wombatu-kun opened a new pull request, #17109:
URL: https://github.com/apache/iceberg/pull/17109
## Summary
`RESTServerExtension#beforeAll` starts an embedded REST catalog server for
tests on an OS-assigned free port (`RCKUtils.findFreePort()` opens
`ServerSocket(0)`, reads the port, and closes it). Because Jetty binds to that
port only later in `httpServer.start()`, there is a TOCTOU window in which a
parallel test JVM fork can grab the same ephemeral port, producing
`java.net.BindException: Address already in use`.
PR #13017 added a 10-attempt retry to paper over that window, but it never
fires. It re-rolls the port only inside `catch (BindException e)`, while Jetty
surfaces the failure as a `java.io.IOException("Failed to bind to ...")` whose
cause is the real `BindException`. Since `IOException` is not a
`BindException`, control falls through to the generic `catch (Exception e)`,
which rethrows immediately with no retry, so a single lost port race aborts the
whole test class with `initializationError`.
This changes the catch to detect a bind failure by scanning the exception's
cause chain, so the existing retry actually re-rolls the port on a wrapped
`BindException`. The deeper TOCTOU window is inherent to probe-then-bind and is
left as-is; retry is the agreed mitigation. This is a follow-up to the two
earlier attempts at this same flakiness: #13017 (add the retry) and #13992
(enable `SO_REUSEPORT`, which only covers the `TIME_WAIT` case, not an active
bind by another fork).
Observed as a flaky failure of `TestAddFilesProcedure > initializationError`
in the `spark-tests (17, 3.5, 2.12, extensions)` job (which then fail-fast
cancels the sibling `core` jobs), e.g. on the unrelated PR #17065:
https://github.com/apache/iceberg/actions/runs/28645321282/job/84950457116
```
java.lang.RuntimeException: Failed to start REST server
at
org.apache.iceberg.rest.RESTServerExtension.beforeAll(RESTServerExtension.java:78)
Caused by: java.io.IOException: Failed to bind to 0.0.0.0/0.0.0.0:35673
at
org.apache.iceberg.rest.RESTCatalogServer.start(RESTCatalogServer.java:130)
Caused by: java.net.BindException: Address already in use
```
## Testing
Added `TestRESTServerExtension` covering the cause-chain detection: the
exact CI shape (an `IOException` wrapping a `BindException`, asserting it is
not itself a `BindException` so a direct `catch` would miss it), a direct
`BindException`, non-bind failures, and `null`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
