smengcl commented on code in PR #11307:
URL: https://github.com/apache/ozone/pull/11307#discussion_r4211518741
##########
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 add coverage that exports the container before the source records the
deletion,
then processes the source ACKs and purges the SCM transaction before the
exported copy
is imported and reported?
A successful delete ACK means the source has persisted the pending deletion,
and those
records are copied with the container DB. A replica that never received its
own SCM
delete command can therefore still delete the blocks. This mocked membership
change
checks transaction retention, but does not establish the claimed permanent
block leak.
##########
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 second snapshot still leaves the race open. With candidates A and B, A
can pass
isSafeToPurge(), gain an unacknowledged replica while B is checked, and then
be removed
by removeTransactions(). getContainerReplicas() releases its read lock
before returning,
and the DeletedBlockLog lock does not exclude replica updates.
Can we account for in-flight replication or reconcile deletion progress when
a delayed
replica reports? Another snapshot only moves the window.
--
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]
