Fix flaky PeersV2NodeRefreshIT: retry on Simulacron port-bind race

[roydahan]

Summary

PeersV2NodeRefreshIT occasionally fails in CI with:

java.lang.RuntimeException: com.datastax.oss.simulacron.server.BindNodeException: Failed to bind NodeSpec{id=0, name='node1', address=null} to /127.0.0.1:49152
	at com.datastax.oss.simulacron.server.CompletableFutures.getUninterruptibly(CompletableFutures.java:41)
	at com.datastax.oss.simulacron.server.Server.register(Server.java:189)
	at com.datastax.oss.driver.core.PeersV2NodeRefreshIT.setup(PeersV2NodeRefreshIT.java:45)

Re-running the same job with no code changes typically makes it pass, which is what makes this "just flaky" -- but it's a real, reproducible race, not test infra noise.

Root cause

PeersV2NodeRefreshIT.setup() builds its own Simulacron Server with withMultipleNodesPerIp(true) (needed because it emulates several nodes sharing one IP, since system.peers_v2 distinguishes nodes by (peer, peer_port)). That mode binds its nodes starting at 127.0.0.1:49152 -- which is the first port of the OS ephemeral/dynamic port range on both Linux (ip_local_port_range, typically 32768-60999) and macOS (49152-65535).

That same range is what the kernel hands out as the source port for any outbound client socket on the machine -- including other integration tests' driver connections running concurrently in the same build (this module's failsafe config runs up to 16 test classes in parallel via <parallel>classes</parallel> / <threadCountClasses>16</threadCountClasses>, generating a lot of concurrent socket churn). When the kernel happens to hand out 49152 as an ephemeral source port at the exact moment this test tries to bind() it as a listening socket, the bind fails and Simulacron surfaces it as BindNodeException.

This was reported before in #951 and closed as "fixed by #942 and #943" -- but those two PRs only address the unrelated TupleTest.simpleWriteReadTest flake that was bundled into the same issue. Neither touches Simulacron or port binding. The Simulacron race was never actually fixed; the issue was closed because a CI re-run happened to pass, exactly like the case that prompted this PR.

I also tried to just fix the starting port directly (Server.builder().withMultipleNodesPerIp(true).withAddressResolver(new NodePerPortResolver(nonEphemeralPort))), which is what the public Simulacron API is meant to support. It doesn't work with the version this repo depends on: decompiling the vendored com.scylladb.oss.simulacron:simulacron-native-server:0.14.0.0 jar shows that Server.Builder#build() unconditionally replaces the configured resolver with a brand new new NodePerPortResolver() (hardcoded to port 49152) whenever multipleNodesPerIp is true, discarding anything passed to withAddressResolver(...). Upstream datastax/simulacron does not do this (there, withMultipleNodesPerIp(true) only sets the resolver as a default that a later withAddressResolver(...) call can still override) -- this looks like a behavior regression specific to the Scylla fork of Simulacron used here. I didn't have access to that fork's source repo to fix it there, so this PR works around it at the call site instead.

Fix

Since the starting port can't be changed through the public API in the vendored Simulacron version, setup() now retries the whole register-and-bind attempt (fresh Server + fresh resolver state each time) with a short backoff (up to 5 attempts, 200ms * attempt) whenever the failure is specifically a BindNodeException. This gives the OS's ephemeral port allocator a chance to move off of 49152 before the next attempt. Any other exception is rethrown immediately (no masking of real failures), and each failed Server is closed before retrying so its event-loop threads aren't leaked.

Testing

Since this doesn't require CCM/a real cluster (just Simulacron), I verified it directly against the compiled test class:

• Reproduced the exact reported failure: with the original code, holding 127.0.0.1:49152 with an external socket for 1s causes setup() to fail immediately with the identical BindNodeException stack trace shown above.
• Confirmed the fix recovers: with the patched code, the same external hold on port 49152 causes setup() to retry and pass once the port frees up (observed total time increases proportionally to the hold duration, confirming the retry path is actually exercised, not just a lucky timing coincidence).
• Confirmed no regression in the normal case: with no port collision, the patched test passes in the same time as before (single attempt, no retry overhead).
• Ran PeersV2NodeRefreshIT directly via org.junit.runner.JUnitCore (JDK 17) after mvn -pl integration-tests -am install -DskipTests, both with and without the artificial port collision.
• Ran the project's fmt-maven-plugin formatter on the changed file; no reformatting was needed.

Not verified: the full CCM/Docker-backed integration suite (make test-integration-scylla) -- this sandbox doesn't have Docker/CCM available. The change is scoped to a single test's @BeforeClass, doesn't touch any shared test-infra code, and compiles cleanly with the rest of the integration-tests module, so this risk is low, but a full CI run is the last remaining check.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: QUIET

Plan: Pro Plus

Run ID: 51661251-45bb-4354-9d75-1d5875d05923

📥 Commits

Reviewing files that changed from the base of the PR and between d8f6dd3 and 17b3cd3.

📒 Files selected for processing (1)

• integration-tests/src/test/java/com/datastax/oss/driver/core/PeersV2NodeRefreshIT.java

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

• scylladb/scylladb (auto-detected)
• scylladb/github-automation (auto-detected)

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

---

📝 Walkthrough

Walkthrough

PeersV2NodeRefreshIT.setup() now retries Simulacron node binding failures up to five times. Each attempt creates and registers a fresh server. Failed servers are closed. Binding failures trigger incremental delays. Non-binding failures and the final binding failure are rethrown. The method now declares InterruptedException.

Merge Risk: ⚪ Minimal · up to 17b3c

This localized integration-test change retries a specific transient port-binding failure without affecting production behavior; no actionable merge-blocking risk remains beyond normal checks and review.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly identifies the flaky test and the retry-based fix for the Simulacron port-binding race.
Description check	✅ Passed	The description directly explains the port-binding race, retry implementation, cleanup behavior, and validation performed.
Docstring Coverage	✅ Passed	Docstring check was indeterminate for this PR — some files could not be analyzed in time. Not blocking.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

---

_{Comment @coderabbitai help to get the list of available commands.}

[dkropachev]

Thanks for investigating this flake. Closing this PR because retrying here does not reliably solve the collision: every new Server recreates NodePerPortResolver and retries the same ports (49152/49153), while active or TIME_WAIT sockets can occupy them longer than the total backoff.

Please create a PR against scylladb/java-simulacron that fixes Server.Builder so withMultipleNodesPerIp(true) does not overwrite an explicitly configured withAddressResolver(...) during build(). The problematic code is here. This should follow upstream semantics, where callers can override the default resolver.

After releasing the Simulacron fix, please open a follow-up PR here to bump the dependency and configure a collision-safe resolver. withMultipleNodesPerIp(true) should remain enabled because it also configures peer metadata behavior.

[roydahan]

Follow-up: opened scylladb/java-simulacron#11 against Server.Builder#build(), fixing it so withMultipleNodesPerIp(true) no longer silently overrides an explicitly configured withAddressResolver(...). The fix matches upstream datastax/simulacron semantics (eager, order-dependent assignment in withMultipleNodesPerIp, rather than a forced override at build() time), and adds unit test coverage for both call orders.

Once this is reviewed and released, I'll open a follow-up PR here to bump the Simulacron dependency and configure a collision-safe resolver alongside withMultipleNodesPerIp(true), as suggested.