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 failing test sometimes failed because it receives the response
created in the test I added. We can also see the `responseDelayMs` set in the
test I added (500ms) effected the failing 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
