jojochuang commented on code in PR #10540:
URL: https://github.com/apache/ozone/pull/10540#discussion_r3585085616
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/replication/ReplicationSupervisor.java:
##########
@@ -276,7 +281,21 @@ private void addToQueue(AbstractReplicationTask task) {
}
queuedCounter.get(task.getMetricName()).incrementAndGet();
executor.execute(new TaskRunner(task));
+ } else {
+ // Duplicate: an identical task is already in-flight; the in-flight copy
will report the real
+ // outcome, so drain this command's PENDING entry now to avoid a
status-map leak.
+ updateCommandStatus(task, CommandStatus::markAsExecuted);
Review Comment:
**[medium] Duplicate replication marks EXECUTED, not FAILED.** When a
duplicate replication task is rejected because an identical container
replication is already in-flight, the supervisor marks the new command
`EXECUTED`. SCM only clears replication pending ops on `FAILED` (via
`ReplicationStatusHandler`), so the superseded command's pending op can remain
until event timeout if the in-flight attempt ultimately fails. Consider whether
`markAsFailed` is more appropriate so SCM can reschedule promptly.
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/replication/ContainerReplicaPendingOps.java:
##########
@@ -380,6 +448,9 @@ private boolean completeOp(ContainerReplicaOp.PendingOpType
opType,
found = true;
completedOps.add(op);
iterator.remove();
+ if (op.getCommand() != null) {
+ commandIdToContainer.remove(op.getCommand().getId());
Review Comment:
**[high] EC command id map premature removal.** For
`reconstructECContainersCommand`, SCM registers multiple ADD pending ops that
share a single command id, but `completeOp` (here) and `removeExpiredEntries`
unconditionally remove that id from `commandIdToContainer` when any one op
completes or expires. A later `FAILED` status report for the same command then
hits the early return in `onReplicationCommandFailed` because the map entry is
already gone, leaving the sibling pending ops uncleared until their deadlines.
Consider only removing the mapping once no ops for that command id remain.
--
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]
