You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by ck...@apache.org on 2022/10/08 08:34:48 UTC

[ozone] branch master updated: HDDS-7259. Fix uncounted blocksDeleted in BlockDeletingService (#3779)

This is an automated email from the ASF dual-hosted git repository.

ckj 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 e0ea7df9e5 HDDS-7259. Fix uncounted blocksDeleted in BlockDeletingService (#3779)
e0ea7df9e5 is described below

commit e0ea7df9e581fdb112f292c458883a395a162831
Author: Nibiru <ax...@qq.com>
AuthorDate: Sat Oct 8 16:34:41 2022 +0800

    HDDS-7259. Fix uncounted blocksDeleted in BlockDeletingService (#3779)
---
 .../background/BlockDeletingService.java           | 21 ++++++-
 .../container/common/TestBlockDeletingService.java | 69 ++++++++++++++++++++++
 2 files changed, 87 insertions(+), 3 deletions(-)

diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java
index 7234527acf..22d4f253c3 100644
--- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java
+++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java
@@ -556,17 +556,32 @@ public class BlockDeletingService extends BackgroundService {
             LOG.warn("Missing delete block(Container = " +
                 container.getContainerData().getContainerID() + ", Block = " +
                 blkLong);
+            blocksDeleted++;
             continue;
           }
+
+          boolean deleted = false;
           try {
             handler.deleteBlock(container, blkInfo);
             blocksDeleted++;
-            bytesReleased += KeyValueContainerUtil.getBlockLength(blkInfo);
-          } catch (InvalidProtocolBufferException e) {
-            LOG.error("Failed to parse block info for block {}", blkLong, e);
+            deleted = true;
           } catch (IOException e) {
+            // TODO: if deletion of certain block retries exceed the certain
+            //  number of times, service should skip deleting it,
+            //  otherwise invalid numPendingDeletionBlocks could accumulate
+            //  beyond the limit and the following deletion will stop.
             LOG.error("Failed to delete files for block {}", blkLong, e);
           }
+
+          if (deleted) {
+            try {
+              bytesReleased += KeyValueContainerUtil.getBlockLength(blkInfo);
+            } catch (IOException e) {
+              // TODO: handle the bytesReleased correctly for the unexpected
+              //  exception.
+              LOG.error("Failed to get block length for block {}", blkLong, e);
+            }
+          }
         }
       }
       return Pair.of(blocksDeleted, bytesReleased);
diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java
index 9539d5bf03..4ae420dc72 100644
--- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java
+++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/TestBlockDeletingService.java
@@ -79,6 +79,7 @@ import java.nio.ByteBuffer;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.UUID;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -492,6 +493,74 @@ public class TestBlockDeletingService {
     svc.shutdown();
   }
 
+  @Test
+  public void testWithUnrecordedBlocks() throws Exception {
+    // Skip schemaV1, when markBlocksForDeletionSchemaV1, the unrecorded blocks
+    // from received TNXs will be skipped to delete and will not be added into
+    // the NumPendingDeletionBlocks
+    if (Objects.equals(schemaVersion, SCHEMA_V1)) {
+      return;
+    }
+
+    DatanodeConfiguration dnConf = conf.getObject(DatanodeConfiguration.class);
+    dnConf.setBlockDeletionLimit(2);
+    this.blockLimitPerInterval = dnConf.getBlockDeletionLimit();
+    conf.setFromObject(dnConf);
+    ContainerSet containerSet = new ContainerSet(1000);
+
+    createToDeleteBlocks(containerSet, 2, 3, 1);
+
+    ContainerMetrics metrics = ContainerMetrics.create(conf);
+    KeyValueHandler keyValueHandler =
+        new KeyValueHandler(conf, datanodeUuid, containerSet, volumeSet,
+            metrics, c -> {
+        });
+    BlockDeletingServiceTestImpl svc =
+        getBlockDeletingService(containerSet, conf, keyValueHandler);
+    svc.start();
+    GenericTestUtils.waitFor(svc::isStarted, 100, 3000);
+
+    // Ensure 2 container was created
+    List<ContainerData> containerData = Lists.newArrayList();
+    containerSet.listContainer(0L, 2, containerData);
+    Assert.assertEquals(2, containerData.size());
+    KeyValueContainerData ctr1 = (KeyValueContainerData) containerData.get(0);
+    KeyValueContainerData ctr2 = (KeyValueContainerData) containerData.get(1);
+
+    try (DBHandle meta = BlockUtils.getDB(ctr1, conf)) {
+      // create unrecorded blocks in a new txn and update metadata,
+      // service shall first choose the top pendingDeletion container
+      // if using the TopNOrderedContainerDeletionChoosingPolicy
+      List<Long> unrecordedBlocks = new ArrayList<>();
+      int numUnrecorded = 4;
+      for (int i = 0; i < numUnrecorded; i++) {
+        unrecordedBlocks.add(System.nanoTime() + i);
+      }
+      createTxn(ctr1, unrecordedBlocks, 100, ctr1.getContainerID());
+      ctr1.updateDeleteTransactionId(100);
+      ctr1.incrPendingDeletionBlocks(numUnrecorded);
+      updateMetaData(ctr1, (KeyValueContainer) containerSet.getContainer(
+          ctr1.getContainerID()), 3, 1);
+      // Ensure there are 3 + 4 = 7 blocks under deletion
+      Assert.assertEquals(7, getUnderDeletionBlocksCount(meta, ctr1));
+    }
+
+    Assert.assertEquals(3, ctr2.getNumPendingDeletionBlocks());
+
+    // Totally 2 container * 3 blocks + 4 unrecorded block = 10 blocks
+    // So we shall experience 5 rounds to delete all blocks
+    // Unrecorded blocks should not affect the actual NumPendingDeletionBlocks
+    deleteAndWait(svc, 1);
+    deleteAndWait(svc, 2);
+    deleteAndWait(svc, 3);
+    deleteAndWait(svc, 4);
+    deleteAndWait(svc, 5);
+    GenericTestUtils.waitFor(() -> ctr2.getNumPendingDeletionBlocks() == 0,
+        200, 2000);
+
+    svc.shutdown();
+  }
+
   @Test
   @SuppressWarnings("java:S2699") // waitFor => assertion with timeout
   public void testShutdownService() throws Exception {


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org