priyeshkaratha commented on code in PR #11307:
URL: https://github.com/apache/ozone/pull/11307#discussion_r4324146306
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java:
##########
@@ -628,6 +664,63 @@ public void
commitTransactions(List<DeleteBlockTransactionResult> transactionRes
}
}
+ private boolean hasPendingAddReplica(ContainerID containerId) {
+ return
containerReplicaPendingOpsSupplier.get().getPendingOps(containerId).stream()
+ .anyMatch(op -> op.getOpType() ==
ContainerReplicaOp.PendingOpType.ADD);
+ }
+
+ /**
+ * Whether a transaction whose purge {@link #commitTransactions} deferred
can be purged without another ACK:
+ * no replica ADD is in flight and every current replica has already
committed it. A deferral caused only by
+ * an in-flight copy that later fails or expires is otherwise never retried,
since no replica is left to ACK
+ * a resend and the entry would stay in {@link #transactionToDNsCommitMap}
indefinitely.
+ */
+ boolean isPurgeableWithoutAck(long txID, ContainerID containerId) {
+ final Set<DatanodeID> committedDns = transactionToDNsCommitMap.get(txID);
+ if (committedDns == null || committedDns.isEmpty() ||
hasPendingAddReplica(containerId)) {
+ return false;
+ }
+ try {
+ final Set<ContainerReplica> replicas =
containerManager.getContainerReplicas(containerId);
+ return !replicas.isEmpty()
+ && min(replicas.size(), committedDns.size())
+ >=
containerManager.getContainer(containerId).getReplicationConfig().getRequiredNodes()
+ && replicas.stream().map(r ->
r.getDatanodeDetails().getID()).allMatch(committedDns::contains);
+ } catch (ContainerNotFoundException e) {
+ return false;
+ }
+ }
+
+ /**
+ * Checks that every replica in the pre-purge snapshot has committed the
deletion. A missing snapshot means the
+ * container has an in-flight replica ADD or could not be looked up, so the
transaction is kept for resend.
+ *
+ * <p>Not covered: a copy exported before the deletion was recorded whose
pending ADD expired before its replica
Review Comment:
Thanks, good point about the 12m expiry compared with an import that has no
time limit. The HEALTHY check changes when purges happen, so I'd like to take
it up in HDDS-17152 along with reconciling late replicas, and keep this PR to
the pending ADD ordering. I've updated HDDS-17152 to include it.
--
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]
