SteNicholas commented on code in PR #3719:
URL: https://github.com/apache/celeborn/pull/3719#discussion_r3457745821
##########
common/src/main/proto/TransportMessages.proto:
##########
@@ -219,6 +221,7 @@ message PbMetaRegisterWorkerRequest {
map<string, PbResourceConsumption> userResourceConsumption = 7;
int32 internalPort = 8;
string networkLocation = 9;
+ repeated string tags = 12;
Review Comment:
`tags = 12` skips field numbers 10 and 11 here — this message only goes up
to `networkLocation = 9`, so it leaves a confusing gap (looks copy-pasted from
`PbRegisterWorker` above, where `12` is correct). `PbMetaRegisterWorkerRequest`
also has no Java/Scala callers: it's only embedded in the proto
(`registerWorkerRequest = 17`), and the master register path actually flows
through `Resource.proto`'s `RegisterWorkerRequest` (which correctly got `tags =
10`). So this edit is functionally dead — I'd drop it, or use `tags = 10` if
you keep it for symmetry.
##########
master/src/main/java/org/apache/celeborn/service/deploy/master/clustermeta/AbstractMetaManager.java:
##########
@@ -370,6 +371,7 @@ public void updateRegisterWorkerMeta(
disks,
new HashMap<>());
workerInfo.lastHeartbeat_$eq(System.currentTimeMillis());
+ workerInfo.tags_$eq(new HashSet<>(tags));
Review Comment:
Anchoring the re-register refresh discussion here. On an immediate
re-register over a still-live entry, the `putIfAbsent` below keeps the old
`WorkerInfo`, so these fresh tags are silently dropped until the stale entry is
evicted (worker-lost). Smallest fix that mirrors the heartbeat's in-place model
and avoids the collateral of a full `put`:
```java
WorkerInfo existing = workersMap.putIfAbsent(workerInfo.toUniqueId(),
workerInfo);
if (existing != null) {
existing.tags_$eq(new HashSet<>(tags));
}
```
No `availableWorkers` remove/add needed — it already holds the live object,
and tags don't affect availability.
--
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]
