)]}'
{
  "commit": "38954aec67a51eead03bb738fd5f7ee2fc0f1bff",
  "tree": "ebfd02b1c3e6605dca61a02007ff4642e43a89bc",
  "parents": [
    "0add9103a78c3486ebdc9c935093ecb13863b5d5"
  ],
  "author": {
    "name": "Xiang Fu",
    "email": "xiangfu@apache.org",
    "time": "Mon Apr 06 12:23:53 2026 -0700"
  },
  "committer": {
    "name": "GitHub",
    "email": "noreply@github.com",
    "time": "Mon Apr 06 12:23:53 2026 -0700"
  },
  "message": "[Flaky Test] Fix QueryRoutingTest port conflicts and race conditions (#18058)\n\n* Fix flaky QueryRoutingTest by using dynamic port allocation\n\nQueryRoutingTest had 12 failures (4x each for testServerDown,\ntestValidResponse, testSkipUnavailableServer) in the weekly CI\ndigest (2026-03-22 to 2026-03-28).\n\nRoot causes:\n- Hardcoded ports (12345/12346/12347) cause binding failures\n  when tests run in parallel on shared CI infrastructure\n- No server startup synchronization — queries sent before\n  server is ready to accept connections\n- Incomplete shutdown — ports not released fast enough between\n  test methods\n\nFixes:\n- Replace static TEST_PORT with dynamic allocation via\n  ServerSocket(0) for OS-assigned free ports\n- Add startQueryServerWithWait() using TestUtils.waitForCondition\n  to ensure server channel is active before proceeding\n- Improve @AfterMethod shutdown with try-catch and 100ms delay\n  to ensure port release between tests\n- Convert static server instances to per-test instance variables\n  for proper test isolation\n\nCo-Authored-By: Claude Opus 4.6 \u003cnoreply@anthropic.com\u003e\n\n* Fix compilation: testSkipUnavailableServer must declare throws Exception\n\nstartQueryServerWithWait() throws Exception but testSkipUnavailableServer\nonly declared throws IOException, InterruptedException. Java 11 rejects\nthis as an unreported checked exception at compile time.\n\nCo-Authored-By: Claude Sonnet 4.6 \u003cnoreply@anthropic.com\u003e\n\n* Address review: eliminate TOCTOU race and duplicate port risk\n\n- Use port 0 (OS-assigned) for QueryServer, then read actual bound port\n  from channel.localAddress() after startup — eliminates the TOCTOU race\n  where another process could grab the port between discovery and bind\n- In testSkipUnavailableServer, ensure port2 !\u003d port1 to prevent\n  duplicate key issues in the routing table\n- Remove getAvailablePort() helper (no longer needed)\n\nCo-Authored-By: Claude Opus 4.6 (1M context) \u003cnoreply@anthropic.com\u003e\n\n* Address review: simplify helpers, remove sleep, deterministic unavailable port\n\n- Remove _testPort field; pass port directly to initializeTestFixtures()\n- Remove try-catch in shutdownServer() — shutDown() already throws on\n  failure; no need to silently swallow exceptions\n- Remove Thread.sleep(100) in @AfterMethod — shutDown() blocks on\n  channel.close().sync() so the port is released when it returns\n- Simplify startQueryServerWithWait() to startAndGetPort() — start()\n  blocks on bind().sync() so no polling loop is needed\n- Use port 1 for the unavailable server in testSkipUnavailableServer\n  instead of ServerSocket(0) — deterministic, no TOCTOU risk, gives\n  immediate \"connection refused\"\n\nCo-Authored-By: Claude Opus 4.6 (1M context) \u003cnoreply@anthropic.com\u003e\n\n---------\n\nCo-authored-by: Claude Opus 4.6 \u003cnoreply@anthropic.com\u003e",
  "tree_diff": [
    {
      "type": "modify",
      "old_id": "57183839b12747201d4a684bc7b2e8799fec17cb",
      "old_mode": 33188,
      "old_path": "pinot-core/src/test/java/org/apache/pinot/core/transport/QueryRoutingTest.java",
      "new_id": "7e9b952cf9de119fe5476ffd54f18e33d13a6a54",
      "new_mode": 33188,
      "new_path": "pinot-core/src/test/java/org/apache/pinot/core/transport/QueryRoutingTest.java"
    }
  ]
}