Skip to content

test(qwp): fix port collision flake in walk tracker test - #103

Open
bluestreak01 wants to merge 1 commit into
mainfrom
vi_fix_walk_port_race
Open

bluestreak01 wants to merge 1 commit into
mainfrom
vi_fix_walk_port_race

Conversation

@bluestreak01

Copy link
Copy Markdown
Member

Test-only change. It fixes an intermittent failure of QwpQueryClientWalkTrackerTest.testWalk_TransportFailureContinuesWalk.

Failure

The test failed on questdb/questdb#7680 CI, job "SelfHosted Other tests (B) on linux-x64-zfs" (build 274943):

java.lang.IllegalArgumentException: duplicate addr entry: localhost:46809
	at io.questdb.client.impl.ConfigView.parseEntry(ConfigView.java:327)
	at io.questdb.client.impl.ConfigView.getHostPorts(ConfigView.java:169)
	at io.questdb.client.cutlass.qwp.client.QwpQueryClient.validateConfig(QwpQueryClient.java:504)
	at io.questdb.client.cutlass.qwp.client.QwpQueryClient.fromConfig(QwpQueryClient.java:387)
	at io.questdb.client.test.cutlass.qwp.client.QwpQueryClientWalkTrackerTest.testWalk_TransportFailureContinuesWalk(QwpQueryClientWalkTrackerTest.java:451)

Root cause

The test builds addr=localhost:<portDead>,localhost:<portOk>:

  • TestPorts.findUnusedPort() supplies portDead: it binds port 0, closes the probe socket, and returns the number. Nothing holds the port afterwards.
  • new TestWebSocketServer(NOOP_HANDLER) then binds its own port 0 for portOk, and the kernel may hand it the port the probe just released.

In the failing run both values came out as 46809. ConfigView.parseEntry() correctly rejects the duplicate (host, port) pair, so fromConfig() threw before the client attempted any connection. The walk-tracker logic the test targets never ran, and the product code behaved as specified.

The TestPorts.findUnusedPorts(int) javadoc already describes the same race for two back-to-back findUnusedPort() calls. This test pairs one probe with a server's own bind(0), which that helper does not cover.

Change

The test now creates the server first and probes for portDead while the server's listener holds portOk. TestWebSocketServer binds its listener in the constructor and keeps it until close(), and the kernel never assigns a bind(0) port that a live socket on the same address holds, so the two ports cannot collide.

Tradeoff: the test keeps the suite's existing bind-close-reuse exposure, where another process could bind portDead between the probe and the client's connect attempt. Every other pre-selected-port test in the suite accepts the same exposure. The change touches no production code and no shared test helper.

Test plan

  • mvn -pl core test -Dtest=QwpQueryClientWalkTrackerTest: 12/12 green on JDK 25, both on 0b9b5766 (the commit questdb master pins) and on this branch (main plus the fix).
  • A green run cannot prove the absence of a rare random collision; the fix relies on kernel bind semantics rather than on timing.
  • Not built on JDK 8; the change only reorders existing calls and adds a comment.

testWalk_TransportFailureContinuesWalk failed in CI with "duplicate
addr entry: localhost:46809" thrown by ConfigView.parseEntry().

The test called TestPorts.findUnusedPort() to pick a refused port. That
helper binds port 0, closes the probe socket and returns the number.
The test then created a TestWebSocketServer, whose constructor binds
its own port 0. Nothing held the probe port in between, so the kernel
could hand the same port to the server. Both addr entries then pointed
at the same endpoint, and QwpQueryClient.fromConfig() rejected the
config before the walk ever ran.

The test now creates the server first and probes for the dead port
while the server's listener holds its port. The kernel never assigns a
port that a live listener owns, so the two ports always differ. The
remaining exposure - another process taking the dead port before the
client connects - matches every other pre-selected-port test in the
suite.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant