Jackie-Jiang commented on code in PR #19178:
URL: https://github.com/apache/pinot/pull/19178#discussion_r4225397505
##########
pinot-broker/src/main/java/org/apache/pinot/broker/routing/manager/BaseBrokerRoutingManager.java:
##########
@@ -507,6 +537,138 @@ private void processInstanceConfigChangeInternal() {
newDisabledServers, _excludedServers);
}
+ /// Replaces each server's publication marker with the exact tables whose
primary selector can route to it. Must run
+ /// before the server is published in `_routableServerInstanceMap` so the
lock-free readiness check cannot observe a
+ /// routable server without also observing its pending acknowledgements.
+ @GuardedBy("_globalLock.writeLock()")
+ private void markServersPendingRelevantTableUpdates(List<String> servers) {
+ for (String server : servers) {
+ Set<String> relevantTables = new HashSet<>();
+ for (RoutingEntry routingEntry : _routingEntryMap.values()) {
+ if (routingEntry.isServerAssigned(server)) {
+ relevantTables.add(routingEntry.getTableNameWithType());
+ }
+ }
+ _serversPendingRoutingUpdate.put(server, Set.copyOf(relevantTables));
Review Comment:
[MAJOR] This replaces the pending set on every retry, discarding completed
acknowledgements. For a newly enabled server S assigned to tables A and B: A
succeeds and B fails, leaving only B pending; the next retry resets the set to
{A, B}. With no intervening routing change, A can then fail while B succeeds,
leaving S unready even though both selectors have incorporated it. A concrete
built-in case is `MultiStageReplicaGroupSelector`: a failed instance-partitions
read retains its previously valid partition snapshot, so a redundant refresh
failure need not invalidate its routing. Alternating read failures can keep S
returning 503 indefinitely. Please preserve completed acknowledgements for the
existing server-enable update and distinguish retries from genuinely new
routing changes, rather than re-adding every relevant table.
--
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]
