rich7420 commented on code in PR #11307:
URL: https://github.com/apache/ozone/pull/11307#discussion_r4129198614
##########
hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java:
##########
@@ -619,6 +623,18 @@ public void
commitTransactions(List<DeleteBlockTransactionResult> transactionRes
transactionResult.getTxID(), e);
}
}
+ // Re-check replica membership immediately before the durable removal. A
container
+ // copy (ReplicationManager) can add a new replica after the ack-set check
above but
+ // before this point; purging then would leave that replica's blocks
undeleted with
+ // no way to resend the transaction. Transactions that fail this recheck
stay tracked.
+ txIDsToBeDeleted.removeIf(txID -> !isSafeToPurge(txID,
purgeCandidateContainers.get(txID)));
Review Comment:
The race remains after this recheck. In a two-transaction test, adding a
replica to A while rechecking B still purges A. Please coordinate purge with
replica changes instead of taking another snapshot.
##########
hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java:
##########
@@ -412,6 +412,51 @@ public void testCommitTransactions() throws Exception {
assertEquals(0, blocks.size());
}
+ @Test
+ public void testCommitTransactionsDefersPurgeForReplicaAddedDuringCommit()
Review Comment:
Can we cover an export taken before the source records the deletion? Pending
deletion records are copied with the container DB, so the mocked replica change
alone does not establish a block leak.
--
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]
