itschrispeck commented on code in PR #13387: URL: https://github.com/apache/pinot/pull/13387#discussion_r1643592776
########## pinot-core/src/test/java/org/apache/pinot/core/transport/QueryRoutingTest.java: ########## @@ -286,6 +293,89 @@ public void testServerDown() assertEquals(_serverRoutingStatsManager.fetchNumInFlightRequestsForServer(serverId).intValue(), 0); } + @Test + public void testSkipUnavailableServer() + throws IOException, InterruptedException { + // Using a different port is a hack to avoid resource conflict with other tests, ideally queryServer.shutdown() + // should ensure there is no possibility of resource conflict. + int port = 12346; Review Comment: I saw that too - the reason why they don't cause test failures is a mix of (1) the order of test execution, and sometimes it is okay to receive the response from another test, and (2) in some tests `queryServer.shutDown()` is called earlier in the tests so it has time to shut down. The behavior should be repeatable when checking out the initial commit in this PR. The flakey test sometimes failed because it receives the response created in the test I added (to be clear, the only flakey test was the one executed right after the test I added). We can also see the `responseDelayMs` set in the test I added (500ms) effected the flakey test only when it failed. A short `Thread.sleep(..)` between tests would also have solved the flakiness issue -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org