SteNicholas commented on code in PR #3858:
URL: https://github.com/apache/celeborn/pull/3858#discussion_r4424998818
##########
master/src/main/scala/org/apache/celeborn/service/deploy/master/Master.scala:
##########
@@ -759,6 +764,10 @@ private[celeborn] class Master(
highWorkload,
workerStatus,
requestId)
+ workerStats match {
+ case Some(stats) => workerStatsStore.update(targetWorker, stats)
Review Comment:
@Kalvin2077, **coordinate heartbeat cache writes with worker removal**:
Master processes RPCs concurrently, so the earlier `workersMap.containsKey`
check does not guarantee the worker is still registered when this update runs.
An in-flight heartbeat can pass that check, overlap `handleWorkerLost()`
removing both the worker and its stats, then recreate the cached entry here.
Timeout scans only visit `workersMap`, so the orphaned stats can remain
indefinitely.
I reproduced this on 7617f38d by pausing a heartbeat after the registration
check, completing normal worker-loss cleanup, then resuming the heartbeat: the
worker remained absent, but its stats reappeared. This also occurs in
single-Master mode. Could we coordinate membership validation and cache writes
with removal/re-registration, and add a concurrent lifecycle regression test? A
standalone second membership check would still leave a check-then-write race.
--
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]
