You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ozone.apache.org by ri...@apache.org on 2023/03/01 22:30:45 UTC
[ozone] branch master updated: HDDS-7156. Reset pending delete block count (#4324)
This is an automated email from the ASF dual-hosted git repository.
ritesh 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 488e1935a7 HDDS-7156. Reset pending delete block count (#4324)
488e1935a7 is described below
commit 488e1935a7ff5f6a023de1088273672dff3daac7
Author: Ritesh H Shukla <ke...@gmail.com>
AuthorDate: Wed Mar 1 14:30:39 2023 -0800
HDDS-7156. Reset pending delete block count (#4324)
* HDDS-7156: Reset pending delete block count
Reset pending delete block count to 0 if a container has no transactions
Each container has a counter that tracks the number of pending delete
blocks in the delete_txns table for that container. In some cases, this
counter can remain positive when all delete transactions are cleared,
and the counter will never be reset. This means it will continue to be
chosen for deletion instead of other containers that actually have
delete txns.
After this change, if a container is chosen for deletion because it has
>0 pending delete block count but upon deleting from the container, it
is found that there are actually no delete txns, reset the pending
delete block counter to 0. This way it will not be chosen for block
deletion until it gets more delete transactions.
A new unit test for TestBlockDeletingService was added, and the whole
TestBlockDeletingService suite passes.
Author: Ethan Rose
Change-Id: I43e6815962f2db83a95a396020cfa6b7195a8f5f
* Adjust for code changes upstream.
Adjust for renames done in HDDS-7441.
Change-Id: I5e14962e11fa56d959a78ab7f3c99bdd6ab04d18
* Checkstyle
---------
Co-authored-by: Ethan Rose <er...@cloudera.org>
Co-authored-by: Ethan Rose <er...@apache.org>
---
.../container/keyvalue/KeyValueContainerData.java | 8 +
.../background/BlockDeletingService.java | 13 +-
.../container/common/TestBlockDeletingService.java | 184 +++++++++++++++++----
3 files changed, 173 insertions(+), 32 deletions(-)
diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerData.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerData.java
index 58862925c5..b0896d18af 100644
--- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerData.java
+++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/KeyValueContainerData.java
@@ -317,6 +317,14 @@ public class KeyValueContainerData extends ContainerData {
db.getStore().getBatchHandler().commitBatchOperation(batchOperation);
}
+ public void resetPendingDeleteBlockCount(DBHandle db) throws IOException {
+ // Reset the in memory metadata.
+ numPendingDeletionBlocks.set(0);
+ // Reset the metadata on disk.
+ Table<String, Long> metadataTable = db.getStore().getMetadataTable();
+ metadataTable.put(getPendingDeleteBlockCountKey(), 0L);
+ }
+
public int getReplicaIndex() {
return replicaIndex;
}
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 31aee5629e..afea6cdc7a 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
@@ -480,8 +480,15 @@ public class BlockDeletingService extends BackgroundService {
delBlocks.add(delTx);
}
if (delBlocks.isEmpty()) {
- LOG.debug("No transaction found in container : {}",
- containerData.getContainerID());
+ LOG.info("No transaction found in container {} with pending delete " +
+ "block count {}",
+ containerData.getContainerID(),
+ containerData.getNumPendingDeletionBlocks());
+ // If the container was queued for delete, it had a positive
+ // pending delete block count. After checking the DB there were
+ // actually no delete transactions for the container, so reset the
+ // pending delete block count to the correct value of zero.
+ containerData.resetPendingDeleteBlockCount(meta);
return crr;
}
@@ -529,7 +536,7 @@ public class BlockDeletingService extends BackgroundService {
LOG.debug("Container: {}, deleted blocks: {}, space reclaimed: {}, " +
"task elapsed time: {}ms", containerData.getContainerID(),
- deletedBlocksCount, Time.monotonicNow() - startTime);
+ deletedBlocksCount, releasedBytes, Time.monotonicNow() - startTime);
return crr;
} catch (IOException exception) {
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 dfdcef0ee8..c9a54d389d 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
@@ -173,9 +173,16 @@ public class TestBlockDeletingService {
* creates some fake chunk files for testing.
*/
private void createToDeleteBlocks(ContainerSet containerSet,
- int numOfContainers,
- int numOfBlocksPerContainer,
+ int numOfContainers, int numOfBlocksPerContainer,
int numOfChunksPerBlock) throws IOException {
+ for (int i = 0; i < numOfContainers; i++) {
+ createToDeleteBlocks(containerSet, numOfBlocksPerContainer,
+ numOfChunksPerBlock);
+ }
+ }
+
+ private KeyValueContainerData createToDeleteBlocks(ContainerSet containerSet,
+ int numOfBlocksPerContainer, int numOfChunksPerBlock) throws IOException {
ChunkManager chunkManager;
if (layout == FILE_PER_BLOCK) {
chunkManager = new FilePerBlockStrategy(true, null, null);
@@ -185,34 +192,34 @@ public class TestBlockDeletingService {
byte[] arr = randomAlphanumeric(1048576).getBytes(UTF_8);
ChunkBuffer buffer = ChunkBuffer.wrap(ByteBuffer.wrap(arr));
int txnID = 0;
- for (int x = 0; x < numOfContainers; x++) {
- long containerID = ContainerTestHelper.getTestContainerID();
- KeyValueContainerData data =
- new KeyValueContainerData(containerID, layout,
- ContainerTestHelper.CONTAINER_MAX_SIZE,
- UUID.randomUUID().toString(), datanodeUuid);
- data.closeContainer();
- data.setSchemaVersion(schemaVersion);
- KeyValueContainer container = new KeyValueContainer(data, conf);
- container.create(volumeSet,
- new RoundRobinVolumeChoosingPolicy(), scmId);
- containerSet.addContainer(container);
- data = (KeyValueContainerData) containerSet.getContainer(
- containerID).getContainerData();
- data.setSchemaVersion(schemaVersion);
- if (schemaVersion.equals(SCHEMA_V1)) {
- createPendingDeleteBlocksSchema1(numOfBlocksPerContainer, data,
- containerID, numOfChunksPerBlock, buffer, chunkManager, container);
- } else if (schemaVersion.equals(SCHEMA_V2)
- || schemaVersion.equals(SCHEMA_V3)) {
- createPendingDeleteBlocksViaTxn(numOfBlocksPerContainer, txnID,
- containerID, numOfChunksPerBlock, buffer, chunkManager,
- container, data);
- } else {
- throw new UnsupportedOperationException(
- "Only schema version 1,2,3 are supported.");
- }
+ long containerID = ContainerTestHelper.getTestContainerID();
+ KeyValueContainerData data =
+ new KeyValueContainerData(containerID, layout,
+ ContainerTestHelper.CONTAINER_MAX_SIZE,
+ UUID.randomUUID().toString(), datanodeUuid);
+ data.closeContainer();
+ data.setSchemaVersion(schemaVersion);
+ KeyValueContainer container = new KeyValueContainer(data, conf);
+ container.create(volumeSet,
+ new RoundRobinVolumeChoosingPolicy(), scmId);
+ containerSet.addContainer(container);
+ data = (KeyValueContainerData) containerSet.getContainer(
+ containerID).getContainerData();
+ data.setSchemaVersion(schemaVersion);
+ if (schemaVersion.equals(SCHEMA_V1)) {
+ createPendingDeleteBlocksSchema1(numOfBlocksPerContainer, data,
+ containerID, numOfChunksPerBlock, buffer, chunkManager, container);
+ } else if (schemaVersion.equals(SCHEMA_V2)
+ || schemaVersion.equals(SCHEMA_V3)) {
+ createPendingDeleteBlocksViaTxn(numOfBlocksPerContainer, txnID,
+ containerID, numOfChunksPerBlock, buffer, chunkManager,
+ container, data);
+ } else {
+ throw new UnsupportedOperationException(
+ "Only schema version 1,2,3 are supported.");
}
+
+ return data;
}
@SuppressWarnings("checkstyle:parameternumber")
@@ -419,6 +426,125 @@ public class TestBlockDeletingService {
}
+ /**
+ * In some cases, the pending delete blocks metadata will become larger
+ * than the actual number of pending delete blocks in the database. If
+ * there are no delete transactions in the DB, this metadata counter should
+ * be reset to zero.
+ */
+ @Test
+ public void testPendingDeleteBlockReset() throws Exception {
+ // This test is not relevant for schema V1.
+ if (schemaVersion.equals(SCHEMA_V1)) {
+ return;
+ }
+
+ final int blockDeleteLimit = 2;
+ DatanodeConfiguration dnConf = conf.getObject(DatanodeConfiguration.class);
+ dnConf.setBlockDeletionLimit(blockDeleteLimit);
+ this.blockLimitPerInterval = dnConf.getBlockDeletionLimit();
+ conf.setFromObject(dnConf);
+ ContainerSet containerSet = new ContainerSet(1000);
+
+ // Create one container with no actual pending delete blocks, but an
+ // incorrect metadata value indicating it has enough pending deletes to
+ // use up the whole block deleting limit.
+ KeyValueContainerData incorrectData =
+ createToDeleteBlocks(containerSet,
+ 0, 1);
+ try (DBHandle db = BlockUtils.getDB(incorrectData, conf)) {
+ // Check pre-create state.
+ Assert.assertEquals(0, getUnderDeletionBlocksCount(db,
+ incorrectData));
+ Assert.assertEquals(0, db.getStore().getMetadataTable()
+ .get(incorrectData.getPendingDeleteBlockCountKey()).longValue());
+ Assert.assertEquals(0,
+ incorrectData.getNumPendingDeletionBlocks());
+
+ // Alter the pending delete value in memory and the DB.
+ incorrectData.incrPendingDeletionBlocks(blockDeleteLimit);
+ db.getStore().getMetadataTable().put(
+ incorrectData.getPendingDeleteBlockCountKey(),
+ (long)blockDeleteLimit);
+ }
+
+ // Create one container with fewer pending delete blocks than the first.
+ int correctNumBlocksToDelete = blockDeleteLimit - 1;
+ KeyValueContainerData correctData = createToDeleteBlocks(containerSet,
+ correctNumBlocksToDelete, 1);
+ // Check its metadata was set up correctly.
+ Assert.assertEquals(correctNumBlocksToDelete,
+ correctData.getNumPendingDeletionBlocks());
+ try (DBHandle db = BlockUtils.getDB(correctData, conf)) {
+ Assert.assertEquals(correctNumBlocksToDelete,
+ getUnderDeletionBlocksCount(db, correctData));
+ Assert.assertEquals(correctNumBlocksToDelete,
+ db.getStore().getMetadataTable()
+ .get(correctData.getPendingDeleteBlockCountKey()).longValue());
+ }
+
+ // Create the deleting service instance with very large interval between
+ // runs so we can trigger it manually.
+ ContainerMetrics metrics = ContainerMetrics.create(conf);
+ KeyValueHandler keyValueHandler =
+ new KeyValueHandler(conf, datanodeUuid, containerSet, volumeSet,
+ metrics, c -> {
+ });
+ OzoneContainer ozoneContainer =
+ mockDependencies(containerSet, keyValueHandler);
+ BlockDeletingService svc = new BlockDeletingService(ozoneContainer,
+ 1_000_000, 1_000_000, TimeUnit.SECONDS, 1, conf);
+
+ // On the first run, the container with incorrect metadata should consume
+ // the block deletion limit, and the correct container with fewer pending
+ // delete blocks will not be processed.
+ svc.runPeriodicalTaskNow();
+
+ // Pending delete block count in the incorrect container should be fixed
+ // and reset to 0.
+ Assert.assertEquals(0, incorrectData.getNumPendingDeletionBlocks());
+ try (DBHandle db = BlockUtils.getDB(incorrectData, conf)) {
+ Assert.assertEquals(0, getUnderDeletionBlocksCount(db,
+ incorrectData));
+ Assert.assertEquals(0, db.getStore().getMetadataTable()
+ .get(incorrectData.getPendingDeleteBlockCountKey()).longValue());
+ }
+ // Correct container should not have been processed.
+ Assert.assertEquals(correctNumBlocksToDelete,
+ correctData.getNumPendingDeletionBlocks());
+ try (DBHandle db = BlockUtils.getDB(correctData, conf)) {
+ Assert.assertEquals(correctNumBlocksToDelete,
+ getUnderDeletionBlocksCount(db, correctData));
+ Assert.assertEquals(correctNumBlocksToDelete,
+ db.getStore().getMetadataTable()
+ .get(correctData.getPendingDeleteBlockCountKey()).longValue());
+ }
+
+ // On the second run, the correct container should be picked up, because
+ // it now has the most pending delete blocks.
+ svc.runPeriodicalTaskNow();
+
+ // The incorrect container should remain in the same state after being
+ // fixed.
+ Assert.assertEquals(0, incorrectData.getNumPendingDeletionBlocks());
+ try (DBHandle db = BlockUtils.getDB(incorrectData, conf)) {
+ Assert.assertEquals(0, getUnderDeletionBlocksCount(db,
+ incorrectData));
+ Assert.assertEquals(0, db.getStore().getMetadataTable()
+ .get(incorrectData.getPendingDeleteBlockCountKey()).longValue());
+ }
+ // The correct container should have been processed this run and had its
+ // blocks deleted.
+ Assert.assertEquals(0, correctData.getNumPendingDeletionBlocks());
+ try (DBHandle db = BlockUtils.getDB(correctData, conf)) {
+ Assert.assertEquals(0, getUnderDeletionBlocksCount(db,
+ correctData));
+ Assert.assertEquals(0, db.getStore().getMetadataTable()
+ .get(correctData.getPendingDeleteBlockCountKey()).longValue());
+ }
+ }
+
+
@Test
public void testBlockDeletion() throws Exception {
DatanodeConfiguration dnConf = conf.getObject(DatanodeConfiguration.class);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@ozone.apache.org
For additional commands, e-mail: commits-help@ozone.apache.org