SteNicholas commented on code in PR #3858:
URL: https://github.com/apache/celeborn/pull/3858#discussion_r4424998837
##########
master/src/main/scala/org/apache/celeborn/service/deploy/master/Master.scala:
##########
@@ -843,6 +852,7 @@ private[celeborn] class Master(
s" for WorkerLost handler!")
} else {
statusSystem.handleWorkerLost(host, rpcPort, pushPort, fetchPort,
replicatePort, requestId)
+ workerStatsStore.remove(worker)
Review Comment:
**Invalidate local stats across HA leadership changes**: This cleanup only
runs in the leader's RPC handler. If Master A caches worker W's stats, becomes
a follower, and W is lost under leader B, A applies the loss through
`MetaHandler -> updateWorkerLostMeta()` without clearing this cache. If A
becomes leader again, W's old stats remain; W is already absent from
`workersMap`, so the timeout scan cannot remove that entry.
This is the HA concern already raised in the PR discussion. A focused probe
on 7617f38d confirmed that invoking the metadata replay path removes W from
`workersMap` while retaining its stats. Could we invalidate the local cache on
leadership changes, or reconcile it with registered workers, while keeping
metrics outside Raft state? Please cover the former-leader/follower/leader
sequence with an HA regression test.
--
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]
