xiangfu0 commented on code in PR #20021:
URL: https://github.com/apache/pinot/pull/20021#discussion_r4366201852
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/manager/BaseBrokerRoutingManager.java:
##########
@@ -451,14 +475,16 @@ private void processInstanceConfigChangeInternal() {
newEnabledServers.add(instanceId);
// NOTE: Remove new enabled server from excluded servers because
the server is likely being restarted
- if (_excludedServers.remove(instanceId)) {
+ boolean wasExcluded = _excludedServers.remove(instanceId);
+ if (wasExcluded) {
LOGGER.info("Got excluded server: {} re-enabled, including it
into the routing", instanceId);
- // We clear any GRPC channel reconnection backoff in this
callback when a server is re-enabled. Otherwise,
- // MSE queries to this server may fail fast until the next
backoff retry succeeds.
- if (_serverReenableCallback != null) {
- LOGGER.info("Calling server re-enable callback for server:
{}", instanceId);
- _serverReenableCallback.accept(serverInstance);
- }
+ }
+ // A server that comes back after having left the enabled set has
most likely been restarted, whether or
+ // not the failure detector had excluded it in between. Let the
listeners drop state tied to the previous
+ // process: cached Netty channels that may be half-open, and the
gRPC connection backoff that would
+ // otherwise make MSE queries fail fast until the next backoff
retry succeeds.
+ if (_previouslyEnabledServers.remove(instanceId) | wasExcluded) {
Review Comment:
The single `|` is load-bearing: both `remove` calls must run for their side
effects, so a short-circuit `||` would be a bug. It reads like a typo at a
glance though -- a tiny `// bitwise OR: both removes must run` would save the
next reader a double-take.
--
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]
