Skip to content

Revert "Retry TestingTrinoServer setup when port is already used"#13247

Merged
findepi merged 5 commits intomasterfrom
revert-13077-origin/master/305_retry
Jul 27, 2022
Merged

Revert "Retry TestingTrinoServer setup when port is already used"#13247
findepi merged 5 commits intomasterfrom
revert-13077-origin/master/305_retry

Conversation

@findepi
Copy link
Copy Markdown
Member

@findepi findepi commented Jul 20, 2022

This reverts commit 5a72170. The change
was not necessary. The rationale was

Even we typically use random port to setup TestingTrinoServer there is a
low likelihood of a race condition where same port is used by two
concurrent tests.

However, in practice, we always bind to 0, and such a bind is guaranteed
to succeed. If a specific test binds to a specific port, it's this
test's responsibility to guarantee the port is free, not
TestingTrinoServer's.

Reverts #13077

This reverts commit 5a72170. The change
was not necessary. The rationale was

    Even we typically use random port to setup TestingTrinoServer there is a
    low likelihood of a race condition where same port is used by two
    concurrent tests.

However, in practice, we always bind to 0, and such a bind is guaranteed
to succeed. If a specific test binds to a specific port, it's this
test's responsibility to guarantee the port is free, not
`TestingTrinoServer`'s.
@findepi findepi requested review from kokosing, sopel39 and wendigo July 20, 2022 08:45
@cla-bot cla-bot bot added the cla-signed label Jul 20, 2022
@findepi
Copy link
Copy Markdown
Member Author

findepi commented Jul 21, 2022

Hive tests fixed in #13252

@findepi
Copy link
Copy Markdown
Member Author

findepi commented Jul 21, 2022

previous Ci: #13288

@findepi findepi force-pushed the revert-13077-origin/master/305_retry branch from 3d28bd3 to e45b8fc Compare July 21, 2022 12:04
@findepi findepi merged commit 1a6ab5a into master Jul 27, 2022
@findepi findepi deleted the revert-13077-origin/master/305_retry branch July 27, 2022 15:08
@github-actions github-actions bot added this to the 392 milestone Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants