yashmayya opened a new pull request, #19997:
URL: https://github.com/apache/pinot/pull/19997
## Problem
`BaseBrokerRoutingManager.processInstanceConfigChangeInternal()` has an
inverted check (since #8491). When `_excludedServers` is not empty, a newly
disabled server counts as changed only if it is already excluded:
```java
for (String newDisabledServer : newDisabledServers) {
if (_excludedServers.contains(newDisabledServer)) {
changedServers.add(newDisabledServer);
}
}
```
So while a failure detector (`pinot.broker.failure.detector.type`, e.g.
`CONNECTION`) has any server excluded, a server that starts a graceful shutdown
(`shutdownInProgress=true`) is ignored. The method returns early ("without
instance change"), and the routing entries keep selecting that server. The
broker keeps sending it queries through the shutdown drain, then gets
`Connection refused` when the server closes its ports. The stale
`ServerInstance` also stays in `_enabledServerInstanceMap`, so MSE can dispatch
to the old ports of the restarted server.
We hit this during a rolling server restart. 2 of 4 brokers had an exclusion
in place, ignored every server's shutdown flag, and returned ~1,100 query
errors. The other 2 brokers had none.
## Fix
Count every newly disabled server as changed. Excluded servers now only
affect which servers are routable. Excluded servers that get disabled are still
counted, as before, so the early return can't skip removing them from
`_enabledServerInstanceMap`. Only flipping the check to `!contains` breaks
`testServerReenableCallbackInvokedWhenExcludedServerReenabled`.
Draft PR #19908 makes the same change as part of a larger feature. This PR
ships the fix on its own.
## Testing
Added
`testShuttingDownServerRemovedFromRoutingWhileAnotherServerIsExcluded`. It
fails without the fix. All `BrokerRoutingManagerTest` tests pass.
--
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]
