priyeshkaratha commented on code in PR #11307:
URL: https://github.com/apache/ozone/pull/11307#discussion_r4324142489
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java:
##########
@@ -619,6 +624,37 @@ public void
commitTransactions(List<DeleteBlockTransactionResult> transactionRes
transactionResult.getTxID(), e);
}
}
+ // A container copy made by ReplicationManager misses this deletion only
if it was exported before the source
+ // recorded the deletion: the datanode serializes export and recording
under the container lock, and ACKs only
+ // after recording. Such a copy was therefore scheduled before the ACKs
above, so its replica ADD is either still
+ // pending here or already completed. ContainerStateManager registers a
replica and clears its pending ADD under
+ // the same container lock, so the pending ADD is checked before the
replica set is read: an ADD that is no
+ // longer pending is already in that replica set. A copy scheduled after
this check is exported after the
+ // deletion was recorded and deletes the blocks without a resend.
+ final Map<ContainerID, List<DatanodeID>> replicaSnapshots = new
HashMap<>();
+ for (ContainerID containerId : new
HashSet<>(purgeCandidateContainers.values())) {
+ if (hasPendingAddReplica(containerId)) {
+ LOG.info("Deferring purge of transactions for container {}: a replica
is being added to it.", containerId);
+ continue;
+ }
+ try {
+ replicaSnapshots.put(containerId,
containerManager.getContainerReplicas(containerId).stream()
+ .map(ContainerReplica::getDatanodeDetails)
+ .map(DatanodeDetails::getID)
+ .collect(Collectors.toList()));
+ } catch (ContainerNotFoundException e) {
+ LOG.warn("Could not re-validate replicas for container {} before
purge; deferring its transactions.",
+ containerId, e);
+ }
+ }
+ txIDsToBeDeleted.removeIf(txID -> !isSafeToPurge(txID,
purgeCandidateContainers.get(txID), replicaSnapshots));
+ for (Long txID : txIDsToBeDeleted) {
+ transactionToDNsCommitMap.remove(txID);
+ transactionToRetryCountMap.remove(txID);
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("Purging txId: {} from block deletion log", txID);
+ }
+ }
Review Comment:
Agreed, thanks. Moved the pending ADD check before the in-loop
getContainerReplicas() and removed the second pass, along with
purgeCandidateContainers, the replica snapshots and isSafeToPurge.
--
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]
