This is an automated email from the ASF dual-hosted git repository.
ChenSammi pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git
The following commit(s) were added to refs/heads/master by this push:
new 9ac58d9f16c HDDS-16142. Fix double-decrement of deleted-block summary
on Ratis timeout (#11043)
9ac58d9f16c is described below
commit 9ac58d9f16cca23374cc5ec1c01fce1659f36feb
Author: Priyesh Karatha <[email protected]>
AuthorDate: Wed Aug 19 20:25:37 2026 +0530
HDDS-16142. Fix double-decrement of deleted-block summary on Ratis timeout
(#11043)
---
.../SCMDeletedBlockTransactionStatusManager.java | 22 +++
.../hadoop/hdds/scm/block/TestDeletedBlockLog.java | 168 +++++++++++++++++++++
2 files changed, 190 insertions(+)
diff --git
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java
index 43405ed90f7..3df8622608f 100644
---
a/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java
+++
b/hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/block/SCMDeletedBlockTransactionStatusManager.java
@@ -52,6 +52,7 @@
import org.apache.hadoop.hdds.scm.container.ContainerInfo;
import org.apache.hadoop.hdds.scm.container.ContainerManager;
import org.apache.hadoop.hdds.scm.container.ContainerReplica;
+import org.apache.hadoop.hdds.scm.exceptions.SCMException;
import org.apache.hadoop.hdds.upgrade.HDDSLayoutFeature;
import org.apache.hadoop.hdds.utils.db.Table;
import org.apache.hadoop.ozone.container.upgrade.VersionedDatanodeFeatures;
@@ -468,6 +469,9 @@ public void
addTransactions(ArrayList<DeletedBlocksTransaction> txList) throws I
try {
deletedBlockLogStateManager.addTransactionsToDB(txList, getSummary());
} catch (IOException e) {
+ if (isRatisTimeout(e)) {
+ throw e;
+ }
// Revert the in-memory changes if the DB update fails
for (DeletedBlocksTransaction tx: txList) {
if (tx.hasTotalBlockSize()) {
@@ -482,6 +486,21 @@ public void
addTransactions(ArrayList<DeletedBlocksTransaction> txList) throws I
deletedBlockLogStateManager.addTransactionsToDB(txList);
}
+ private static boolean isRatisTimeout(IOException e) {
+ if (!(e instanceof SCMException)) {
+ return false;
+ }
+ SCMException.ResultCodes code = ((SCMException) e).getResult();
+ if (code == SCMException.ResultCodes.TIMEOUT) {
+ return true;
+ }
+ if (code == SCMException.ResultCodes.INTERNAL_ERROR) {
+ Throwable cause = e.getCause();
+ return cause instanceof InterruptedException;
+ }
+ return false;
+ }
+
private void rollbackDeletedBlocksSummary(TxBlockInfo txBlockInfo) {
totalTxCount.addAndGet(1);
totalBlockCount.addAndGet(txBlockInfo.getTotalBlockCount());
@@ -516,6 +535,9 @@ public void removeTransactions(ArrayList<Long> txIDs)
throws IOException {
try {
deletedBlockLogStateManager.removeTransactionsFromDB(txIDs,
getSummary());
} catch (IOException e) {
+ if (isRatisTimeout(e)) {
+ throw e;
+ }
// Revert the in-memory changes if the DB update fails
for (TxBlockInfo txBlockInfo : removedTxBlockInfos) {
txSizeMap.put(txBlockInfo.getTxId(), txBlockInfo);
diff --git
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java
index 2bfa5f379b7..a418e333424 100644
---
a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java
+++
b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/block/TestDeletedBlockLog.java
@@ -21,11 +21,14 @@
import static org.apache.hadoop.ozone.common.BlockGroup.SIZE_NOT_AVAILABLE;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.params.provider.Arguments.arguments;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.atLeast;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -72,6 +75,7 @@
import org.apache.hadoop.hdds.scm.container.ContainerReplica;
import org.apache.hadoop.hdds.scm.container.replication.ContainerHealthResult;
import org.apache.hadoop.hdds.scm.container.replication.ReplicationManager;
+import org.apache.hadoop.hdds.scm.exceptions.SCMException;
import org.apache.hadoop.hdds.scm.ha.SCMHADBTransactionBuffer;
import org.apache.hadoop.hdds.scm.ha.SCMHADBTransactionBufferStub;
import org.apache.hadoop.hdds.scm.ha.SCMHAManagerStub;
@@ -946,6 +950,170 @@ private void mockStandAloneContainerInfo(long
containerID, DatanodeDetails dd)
.thenReturn(replicaSet);
}
+ private SCMException ratisTimeout() {
+ return new SCMException("Simulated Ratis submit timeout",
SCMException.ResultCodes.TIMEOUT);
+ }
+
+ private SCMException hardDbFailure() {
+ return new SCMException("Simulated hard DB failure",
SCMException.ResultCodes.IO_EXCEPTION);
+ }
+
+ private DeletedBlocksTransaction buildTx(long txId, long containerId, long
blockSize, long blockCount) {
+ return DeletedBlocksTransaction.newBuilder()
+ .setTxID(txId)
+ .setContainerID(containerId)
+ .setCount(0)
+ .addLocalID(1L)
+ .setTotalBlockSize(blockSize * blockCount)
+ .setTotalBlockReplicatedSize(blockSize * blockCount * 3L)
+ .build();
+ }
+
+ /**
+ * Ratis timeout on addTransactionsToDB: the abandoned future may still
commit,
+ * so the in-memory increment must NOT be rolled back (HDDS-16142).
+ */
+ @Test
+ public void testSummaryDoesNotDoubleDecrementOnRatisSubmitTimeout() throws
IOException {
+ long txId = 1L;
+ long containerId = 100L;
+ long blockSize = 64L * 1024 * 1024;
+ long blockCount = 5L;
+ ArrayList<DeletedBlocksTransaction> txList = new ArrayList<>();
+ txList.add(buildTx(txId, containerId, blockSize, blockCount));
+
+ DeletedBlockLogStateManager mockStateManager =
mock(DeletedBlockLogStateManager.class);
+
doThrow(ratisTimeout()).when(mockStateManager).addTransactionsToDB(any(ArrayList.class),
any());
+
+ SCMDeletedBlockTransactionStatusManager statusManager =
+ new SCMDeletedBlockTransactionStatusManager(
+ mockStateManager,
+ scm.getScmMetadataStore().getStatefulServiceConfigTable(),
+ containerManager, metrics, Long.MAX_VALUE);
+
+ assertThrows(IOException.class, () ->
statusManager.addTransactions(txList));
+ // Increment must NOT be rolled back: the late commit can still apply.
+ assertEquals(1, statusManager.getSummary().getTotalTransactionCount(),
+ "Ratis timeout on add: summary must stay incremented (no rollback)");
+
+ // Simulate the late Ratis commit landing and the tx being found by
getTransactions.
+ statusManager.getTxSizeMap().put(txId,
+ new TxBlockInfo(txId, containerId, blockCount, blockSize * blockCount,
blockSize * blockCount * 3L));
+ ArrayList<Long> txIds = new ArrayList<>();
+ txIds.add(txId);
+ statusManager.removeTransactions(txIds);
+
+ assertEquals(0, statusManager.getSummary().getTotalTransactionCount(),
+ "One add (no rollback) + one remove must yield 0, not -1");
+ }
+
+ /**
+ * Hard DB failure on addTransactionsToDB: the write definitively did not
land,
+ * so the in-memory increment must be rolled back.
+ */
+ @Test
+ public void testSummaryRollsBackOnHardAddFailure() throws IOException {
+ long txId = 3L;
+ long containerId = 300L;
+ long blockSize = 16L * 1024 * 1024;
+ long blockCount = 2L;
+ ArrayList<DeletedBlocksTransaction> txList = new ArrayList<>();
+ txList.add(buildTx(txId, containerId, blockSize, blockCount));
+
+ DeletedBlockLogStateManager mockStateManager =
mock(DeletedBlockLogStateManager.class);
+
doThrow(hardDbFailure()).when(mockStateManager).addTransactionsToDB(any(ArrayList.class),
any());
+
+ SCMDeletedBlockTransactionStatusManager statusManager =
+ new SCMDeletedBlockTransactionStatusManager(
+ mockStateManager,
+ scm.getScmMetadataStore().getStatefulServiceConfigTable(),
+ containerManager, metrics, Long.MAX_VALUE);
+
+ assertThrows(IOException.class, () ->
statusManager.addTransactions(txList));
+ // Hard failure: write did not land, rollback must restore summary to 0.
+ assertEquals(0, statusManager.getSummary().getTotalTransactionCount(),
+ "Hard add failure: summary must be rolled back to 0");
+ }
+
+ /**
+ * Ratis timeout on removeTransactionsFromDB: the abandoned future may still
commit
+ * and remove the tx durably, so the in-memory decrement must NOT be rolled
back (HDDS-16142).
+ */
+ @Test
+ public void testSummaryDoesNotOvercountOnRatisRemoveTimeout() throws
IOException {
+ long txId = 2L;
+ long containerId = 200L;
+ long blockSize = 32L * 1024 * 1024;
+ long blockCount = 3L;
+
+ DeletedBlockLogStateManager mockStateManager =
mock(DeletedBlockLogStateManager.class);
+
+ SCMDeletedBlockTransactionStatusManager statusManager =
+ new SCMDeletedBlockTransactionStatusManager(
+ mockStateManager,
+ scm.getScmMetadataStore().getStatefulServiceConfigTable(),
+ containerManager, metrics, Long.MAX_VALUE);
+
+ statusManager.addTransactions(new ArrayList<>(
+ Collections.singletonList(buildTx(txId, containerId, blockSize,
blockCount))));
+ assertEquals(1, statusManager.getSummary().getTotalTransactionCount());
+
+ statusManager.getTxSizeMap().put(txId,
+ new TxBlockInfo(txId, containerId, blockCount, blockSize * blockCount,
blockSize * blockCount * 3L));
+
+
doThrow(ratisTimeout()).when(mockStateManager).removeTransactionsFromDB(any(ArrayList.class),
any());
+
+ ArrayList<Long> txIds = new ArrayList<>();
+ txIds.add(txId);
+ assertThrows(IOException.class, () ->
statusManager.removeTransactions(txIds));
+
+ // Decrement must NOT be rolled back: the late commit may have removed the
tx.
+ assertEquals(0, statusManager.getSummary().getTotalTransactionCount(),
+ "Ratis timeout on remove: summary must stay decremented (no
rollback)");
+ // txSizeMap must also not be restored (same reasoning).
+ assertFalse(statusManager.getTxSizeMap().containsKey(txId),
+ "Ratis timeout on remove: txSizeMap entry must not be restored");
+ }
+
+ /**
+ * Hard DB failure on removeTransactionsFromDB: the write definitively did
not land,
+ * so the in-memory decrement must be rolled back and txSizeMap restored.
+ */
+ @Test
+ public void testSummaryRollsBackOnHardRemoveFailure() throws IOException {
+ long txId = 4L;
+ long containerId = 400L;
+ long blockSize = 8L * 1024 * 1024;
+ long blockCount = 4L;
+
+ DeletedBlockLogStateManager mockStateManager =
mock(DeletedBlockLogStateManager.class);
+
+ SCMDeletedBlockTransactionStatusManager statusManager =
+ new SCMDeletedBlockTransactionStatusManager(
+ mockStateManager,
+ scm.getScmMetadataStore().getStatefulServiceConfigTable(),
+ containerManager, metrics, Long.MAX_VALUE);
+
+ statusManager.addTransactions(new ArrayList<>(
+ Collections.singletonList(buildTx(txId, containerId, blockSize,
blockCount))));
+ assertEquals(1, statusManager.getSummary().getTotalTransactionCount());
+
+ statusManager.getTxSizeMap().put(txId,
+ new TxBlockInfo(txId, containerId, blockCount, blockSize * blockCount,
blockSize * blockCount * 3L));
+
+
doThrow(hardDbFailure()).when(mockStateManager).removeTransactionsFromDB(any(ArrayList.class),
any());
+
+ ArrayList<Long> txIds = new ArrayList<>();
+ txIds.add(txId);
+ assertThrows(IOException.class, () ->
statusManager.removeTransactions(txIds));
+
+ // Hard failure: write did not land, rollback must restore summary to 1.
+ assertEquals(1, statusManager.getSummary().getTotalTransactionCount(),
+ "Hard remove failure: summary must be rolled back to 1");
+ assertTrue(statusManager.getTxSizeMap().containsKey(txId),
+ "Hard remove failure: txSizeMap entry must be restored for retry");
+ }
+
private void mockInadequateReplicaUnhealthyContainerInfo(long containerID,
int count) throws IOException {
List<DatanodeDetails> dns = dnList.subList(0, 2);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
