gortiz commented on code in PR #10528:
URL: https://github.com/apache/pinot/pull/10528#discussion_r1211205863
##########
pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/MultiNodesOfflineClusterIntegrationTest.java:
##########
@@ -172,7 +174,6 @@ private void testCountStarQuery(int
expectedNumServersQueried, boolean exception
assertEquals(exceptions.size(), 2);
JsonNode firstException = exceptions.get(0);
assertEquals(firstException.get("errorCode").intValue(),
QueryException.BROKER_REQUEST_SEND_ERROR_CODE);
-
assertTrue(firstException.get("message").textValue().contains("Connection
refused"));
Review Comment:
The check is incorrect. I think I already wrote about that somewhere, but
can I do it here.
There is a race condition between the server that is being stopped and the
requests that arrive. Normally, the shutdown is executed before, so the error
that the later request receives is the expected `Connection refused`. But there
are cases where the request arrives before the shutdown is executed and
therefore the broker kills the connection once it was established, producing a
`Connection reset` message. It could even be possible to receive a success if
the request is processed before the server is stopped.
The correct solution would be to break the race condition by sequentializing
the calls. In order to do that the stop request should be blocking. I don't
know how to do that and it may introduce some extra complexity on the PR, so I
thought it would be better to just do not check the message. We are already
checking the error code, which should be good enough.
Also, checking error messages is something problematic in general. It may
depend, for example, on the locale configured by the used, which would make the
tests difficult to reproduce. Instead we should include more specific error
codes.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
