noy-solvin opened a new pull request, #20037:
URL: https://github.com/apache/pinot/pull/20037
## Description
Fixes #19081.
In `BaseBrokerRoutingManager.processInstanceConfigChangeInternal`:
1. Fixed inverted condition when checking `_excludedServers` for
`newDisabledServers`. Previously, `if
(_excludedServers.contains(newDisabledServer))` added the server to
`changedServers` only if it was already excluded. This logic was inverted:
servers not previously in `_excludedServers` were routable and their
routability changed when disabled, so they must be included in `changedServers`
so that table routing entries are updated. Conversely, servers already in
`_excludedServers` were already unroutable, so disabling them does not change
routability.
2. Moved `_enabledServerInstanceMap.remove(newDisabledInstance)` into a
`finally` block to ensure disabled instances are always purged from
`_enabledServerInstanceMap`, even on early termination when
`changedServers.isEmpty()`.
## Testing
- Added unit tests to `BrokerRoutingManagerTest`:
- `testDisabledServerWithNonEmptyExcludedServers`: Verifies that disabling
a server when other servers are excluded properly updates
`_routableServerInstanceMap` and `_enabledServerInstanceMap`.
- `testDisabledServerAlreadyExcludedCleansUpEnabledMap`: Verifies that
disabling a server that was already excluded properly cleans up
`_enabledServerInstanceMap` even when `changedServers` is empty (early return).
- All 25 tests in `BrokerRoutingManagerTest` 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]
