diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/RandomContainerDeletionChoosingPolicy.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/RandomContainerDeletionChoosingPolicy.java index 9f2e9e870568..d0773c37754a 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/RandomContainerDeletionChoosingPolicy.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/RandomContainerDeletionChoosingPolicy.java @@ -26,8 +26,9 @@ import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.hadoop.ozone.container.keyvalue.statemachine.background.BlockDeletingService.ContainerBlockInfo; -import java.util.LinkedList; +import java.util.ArrayList; import java.util.List; import java.util.Map; @@ -40,33 +41,41 @@ public class RandomContainerDeletionChoosingPolicy LoggerFactory.getLogger(RandomContainerDeletionChoosingPolicy.class); @Override - public List chooseContainerForBlockDeletion(int count, - Map candidateContainers) + public List chooseContainerForBlockDeletion( + int blockCount, Map candidateContainers) throws StorageContainerException { Preconditions.checkNotNull(candidateContainers, "Internal assertion: candidate containers cannot be null"); - int currentCount = 0; - List result = new LinkedList<>(); + List result = new ArrayList<>(); ContainerData[] values = new ContainerData[candidateContainers.size()]; // to get a shuffle list ContainerData[] shuffled = candidateContainers.values().toArray(values); ArrayUtils.shuffle(shuffled); + + // Here we are returning containers based on totalBlocks which is basically + // number of blocks to be deleted in an interval. We are also considering + // the boundary case where the blocks of the last container exceeds the + // number of blocks to be deleted in an interval, there we return that + // container but with container we also return an integer so that total + // blocks don't exceed the number of blocks to be deleted in an interval. + for (ContainerData entry : shuffled) { - if (currentCount < count) { - result.add(entry); - currentCount++; + if (((KeyValueContainerData) entry).getNumPendingDeletionBlocks() > 0) { + long numBlocksToDelete = Math.min(blockCount, + ((KeyValueContainerData) entry).getNumPendingDeletionBlocks()); + blockCount -= numBlocksToDelete; + result.add(new ContainerBlockInfo(entry, numBlocksToDelete)); if (LOG.isDebugEnabled()) { LOG.debug("Select container {} for block deletion, " - + "pending deletion blocks num: {}.", - entry.getContainerID(), + + "pending deletion blocks num: {}.", entry.getContainerID(), ((KeyValueContainerData) entry).getNumPendingDeletionBlocks()); } - } else { - break; + if (blockCount == 0) { + break; + } } } - return result; } } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/TopNOrderedContainerDeletionChoosingPolicy.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/TopNOrderedContainerDeletionChoosingPolicy.java index 2cee75c00fe8..77befb12f374 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/TopNOrderedContainerDeletionChoosingPolicy.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/TopNOrderedContainerDeletionChoosingPolicy.java @@ -31,6 +31,8 @@ import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.ArrayList; +import org.apache.hadoop.ozone.container.keyvalue.statemachine.background.BlockDeletingService.ContainerBlockInfo; /** * TopN Ordered choosing policy that choosing containers based on pending @@ -49,13 +51,14 @@ public class TopNOrderedContainerDeletionChoosingPolicy c1.getNumPendingDeletionBlocks()); @Override - public List chooseContainerForBlockDeletion(int count, - Map candidateContainers) + public List chooseContainerForBlockDeletion( + int totalBlocks, Map candidateContainers) throws StorageContainerException { + Preconditions.checkNotNull(candidateContainers, "Internal assertion: candidate containers cannot be null"); - List result = new LinkedList<>(); + List result = new ArrayList<>(); List orderedList = new LinkedList<>(); for (ContainerData entry : candidateContainers.values()) { orderedList.add((KeyValueContainerData)entry); @@ -63,29 +66,33 @@ public List chooseContainerForBlockDeletion(int count, Collections.sort(orderedList, KEY_VALUE_CONTAINER_DATA_COMPARATOR); // get top N list ordered by pending deletion blocks' number - int currentCount = 0; + // Here we are returning containers based on totalBlocks which is basically + // number of blocks to be deleted in an interval. We are also considering + // the boundary case where the blocks of the last container exceeds the + // number of blocks to be deleted in an interval, there we return that + // container but with container we also return an integer so that total + // blocks don't exceed the number of blocks to be deleted in an interval. + for (KeyValueContainerData entry : orderedList) { - if (currentCount < count) { - if (entry.getNumPendingDeletionBlocks() > 0) { - result.add(entry); - currentCount++; - if (LOG.isDebugEnabled()) { - LOG.debug( - "Select container {} for block deletion, " - + "pending deletion blocks num: {}.", - entry.getContainerID(), - entry.getNumPendingDeletionBlocks()); - } - } else { - LOG.debug("Stop looking for next container, there is no" - + " pending deletion block contained in remaining containers."); + if (entry.getNumPendingDeletionBlocks() > 0) { + long numBlocksToDelete = + Math.min(totalBlocks, entry.getNumPendingDeletionBlocks()); + totalBlocks -= numBlocksToDelete; + result.add(new ContainerBlockInfo(entry, numBlocksToDelete)); + if (LOG.isDebugEnabled()) { + LOG.debug("Select container {} for block deletion, " + + "pending deletion blocks num: {}.", entry.getContainerID(), + entry.getNumPendingDeletionBlocks()); + } + if (totalBlocks == 0) { break; } } else { + LOG.debug("Stop looking for next container, there is no" + + " pending deletion block contained in remaining containers."); break; } } - return result; } } diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/ContainerDeletionChoosingPolicy.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/ContainerDeletionChoosingPolicy.java index 84c4f903f379..884c6a162724 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/ContainerDeletionChoosingPolicy.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/interfaces/ContainerDeletionChoosingPolicy.java @@ -21,6 +21,7 @@ import org.apache.hadoop.hdds.scm.container.common.helpers .StorageContainerException; import org.apache.hadoop.ozone.container.common.impl.ContainerData; +import org.apache.hadoop.ozone.container.keyvalue.statemachine.background.BlockDeletingService.ContainerBlockInfo; import java.util.List; import java.util.Map; @@ -40,8 +41,8 @@ public interface ContainerDeletionChoosingPolicy { * @return container data list * @throws StorageContainerException */ - List chooseContainerForBlockDeletion(int count, - Map candidateContainers) + List chooseContainerForBlockDeletion( + int count, Map candidateContainers) throws StorageContainerException; /** diff --git a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java index a50c1e4c5c0a..16e2f5f13cc4 100644 --- a/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java +++ b/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java @@ -93,6 +93,23 @@ public void setBlockDeletionInterval(Duration duration) { this.blockDeletionInterval = duration.toMillis(); } + @Config(key = "block.deleting.limit.per.interval", + defaultValue = "5000", + type = ConfigType.INT, + tags = { ConfigTag.SCM, ConfigTag.DELETION }, + description = + "Number of blocks to be deleted in an interval." + ) + private int blockLimitPerInterval = 5000; + + public int getBlockDeletionLimit() { + return blockLimitPerInterval; + } + + public void setBlockDeletionLimit(int limit) { + this.blockLimitPerInterval = limit; + } + @PostConstruct public void validate() { if (replicationMaxStreams < 1) { 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 3dab1fa93814..3454ddff6ba9 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 @@ -58,13 +58,10 @@ import org.apache.hadoop.util.Time; import org.apache.hadoop.hdds.protocol.proto .StorageContainerDatanodeProtocolProtos.DeletedBlocksTransaction; +import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration; import com.google.common.collect.Lists; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_CONTAINER_LIMIT_PER_INTERVAL; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_CONTAINER_LIMIT_PER_INTERVAL_DEFAULT; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER_DEFAULT; import static org.apache.hadoop.ozone.OzoneConsts.SCHEMA_V1; import static org.apache.hadoop.ozone.OzoneConsts.SCHEMA_V2; @@ -86,12 +83,7 @@ public class BlockDeletingService extends BackgroundService { private ContainerDeletionChoosingPolicy containerDeletionPolicy; private final ConfigurationSource conf; - // Throttle number of blocks to delete per task, - // set to 1 for testing - private final int blockLimitPerTask; - - // Throttle the number of containers to process concurrently at a time, - private final int containerLimitPerInterval; + private final int blockLimitPerInterval; // Task priority is useful when a to-delete block has weight. private final static int TASK_PRIORITY_DEFAULT = 1; @@ -113,37 +105,56 @@ public BlockDeletingService(OzoneContainer ozoneContainer, throw new RuntimeException(e); } this.conf = conf; - this.blockLimitPerTask = - conf.getInt(OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER, - OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER_DEFAULT); - this.containerLimitPerInterval = - conf.getInt(OZONE_BLOCK_DELETING_CONTAINER_LIMIT_PER_INTERVAL, - OZONE_BLOCK_DELETING_CONTAINER_LIMIT_PER_INTERVAL_DEFAULT); + DatanodeConfiguration dnConf = conf.getObject(DatanodeConfiguration.class); + this.blockLimitPerInterval = dnConf.getBlockDeletionLimit(); + } + + public static class ContainerBlockInfo { + private final ContainerData containerData; + private final Long numBlocksToDelete; + + public ContainerBlockInfo(ContainerData containerData, Long blocks) { + this.containerData = containerData; + this.numBlocksToDelete = blocks; + } + + public ContainerData getContainerData() { + return containerData; + } + + public Long getBlocks() { + return numBlocksToDelete; + } + } @Override public BackgroundTaskQueue getTasks() { BackgroundTaskQueue queue = new BackgroundTaskQueue(); - List containers = Lists.newArrayList(); + List containers = Lists.newArrayList(); try { // We at most list a number of containers a time, // in case there are too many containers and start too many workers. // We must ensure there is no empty container in this result. // The chosen result depends on what container deletion policy is // configured. - containers = chooseContainerForBlockDeletion(containerLimitPerInterval, - containerDeletionPolicy); - if (containers.size() > 0) { - LOG.info("Plan to choose {} containers for block deletion, " - + "actually returns {} valid containers.", - containerLimitPerInterval, containers.size()); + containers = chooseContainerForBlockDeletion(blockLimitPerInterval, + containerDeletionPolicy); + + BlockDeletingTask containerBlockInfos = null; + long totalBlocks = 0; + for (ContainerBlockInfo containerBlockInfo : containers) { + containerBlockInfos = + new BlockDeletingTask(containerBlockInfo.containerData, + TASK_PRIORITY_DEFAULT, containerBlockInfo.numBlocksToDelete); + queue.add(containerBlockInfos); + totalBlocks += containerBlockInfo.numBlocksToDelete; } - - for(ContainerData container : containers) { - BlockDeletingTask containerTask = - new BlockDeletingTask(container, TASK_PRIORITY_DEFAULT); - queue.add(containerTask); + if (containers.size() > 0) { + LOG.info("Plan to choose {} blocks for block deletion, " + + "actually deleting {} blocks.", blockLimitPerInterval, + totalBlocks); } } catch (StorageContainerException e) { LOG.warn("Failed to initiate block deleting tasks, " @@ -158,8 +169,8 @@ public BackgroundTaskQueue getTasks() { return queue; } - public List chooseContainerForBlockDeletion(int count, - ContainerDeletionChoosingPolicy deletionPolicy) + public List chooseContainerForBlockDeletion( + int blockLimit, ContainerDeletionChoosingPolicy deletionPolicy) throws StorageContainerException { Map containerDataMap = ozoneContainer.getContainerSet().getContainerMap().entrySet().stream() @@ -167,7 +178,7 @@ public List chooseContainerForBlockDeletion(int count, deletionPolicy)).collect(Collectors .toMap(Map.Entry::getKey, e -> e.getValue().getContainerData())); return deletionPolicy - .chooseContainerForBlockDeletion(count, containerDataMap); + .chooseContainerForBlockDeletion(blockLimit, containerDataMap); } private boolean isDeletionAllowed(ContainerData containerData, @@ -246,10 +257,13 @@ private class BlockDeletingTask implements BackgroundTask { private final int priority; private final KeyValueContainerData containerData; + private final long blocksToDelete; - BlockDeletingTask(ContainerData containerName, int priority) { + BlockDeletingTask(ContainerData containerName, int priority, + long blocksToDelete) { this.priority = priority; this.containerData = (KeyValueContainerData) containerName; + this.blocksToDelete = blocksToDelete; } @Override @@ -300,8 +314,8 @@ public ContainerBackgroundTaskResult deleteViaSchema1( // # of blocks to delete is throttled KeyPrefixFilter filter = MetadataKeyFilters.getDeletingKeyFilter(); List> toDeleteBlocks = - blockDataTable.getSequentialRangeKVs(null, blockLimitPerTask, - filter); + blockDataTable + .getSequentialRangeKVs(null, (int) blocksToDelete, filter); if (toDeleteBlocks.isEmpty()) { LOG.debug("No under deletion block found in container : {}", containerData.getContainerID()); @@ -374,16 +388,16 @@ public ContainerBackgroundTaskResult deleteViaSchema2( deleteTxns = dnStoreTwoImpl.getDeleteTransactionTable(); List delBlocks = new ArrayList<>(); int totalBlocks = 0; + int numBlocks = 0; try (TableIterator> iter = dnStoreTwoImpl.getDeleteTransactionTable().iterator()) { - while (iter.hasNext() && (totalBlocks < blockLimitPerTask)) { + while (iter.hasNext() && (numBlocks < blocksToDelete)) { DeletedBlocksTransaction delTx = iter.next().getValue(); - totalBlocks += delTx.getLocalIDList().size(); + numBlocks += delTx.getLocalIDList().size(); delBlocks.add(delTx); } } - if (delBlocks.isEmpty()) { if (LOG.isDebugEnabled()) { LOG.debug("No transaction found in container : {}", @@ -398,7 +412,8 @@ public ContainerBackgroundTaskResult deleteViaSchema2( Handler handler = Objects.requireNonNull(ozoneContainer.getDispatcher() .getHandler(container.getContainerType())); - deleteTransactions(delBlocks, handler, blockDataTable, container); + totalBlocks = + deleteTransactions(delBlocks, handler, blockDataTable, container); // Once blocks are deleted... remove the blockID from blockDataTable // and also remove the transactions from txnTable. @@ -433,9 +448,11 @@ public ContainerBackgroundTaskResult deleteViaSchema2( } } - private void deleteTransactions(List delBlocks, + private int deleteTransactions(List delBlocks, Handler handler, Table blockDataTable, - Container container) throws IOException { + Container container) + throws IOException { + int blocksDeleted = 0; for (DeletedBlocksTransaction entry : delBlocks) { for (Long blkLong : entry.getLocalIDList()) { String blk = blkLong.toString(); @@ -443,6 +460,7 @@ private void deleteTransactions(List delBlocks, LOG.debug("Deleting block {}", blk); try { handler.deleteBlock(container, blkInfo); + blocksDeleted++; } catch (InvalidProtocolBufferException e) { LOG.error("Failed to parse block info for block {}", blk, e); } catch (IOException e) { @@ -450,6 +468,7 @@ private void deleteTransactions(List delBlocks, } } } + return blocksDeleted; } @Override 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 96d4228025ca..f637f41844a0 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 @@ -74,6 +74,7 @@ import org.apache.hadoop.ozone.container.testutils.BlockDeletingServiceTestImpl; import org.apache.hadoop.test.GenericTestUtils; import org.apache.hadoop.test.GenericTestUtils.LogCapturer; +import org.apache.hadoop.ozone.container.common.statemachine.DatanodeConfiguration; import static java.util.stream.Collectors.toList; import static org.apache.commons.lang3.RandomStringUtils.randomAlphanumeric; @@ -87,7 +88,6 @@ import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_CONTAINER_LIMIT_PER_INTERVAL; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER; -import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER_DEFAULT; import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_BLOCK_DELETING_SERVICE_INTERVAL; import static org.apache.hadoop.ozone.OzoneConsts.SCHEMA_VERSIONS; import static org.apache.hadoop.ozone.OzoneConsts.SCHEMA_V1; @@ -113,7 +113,7 @@ public class TestBlockDeletingService { private final ChunkLayOutVersion layout; private final String schemaVersion; - private int blockLimitPerTask; + private int blockLimitPerInterval; private static VolumeSet volumeSet; public TestBlockDeletingService(LayoutInfo layoutInfo) { @@ -206,10 +206,11 @@ private void createToDeleteBlocks(ContainerSet containerSet, containerSet.addContainer(container); data = (KeyValueContainerData) containerSet.getContainer( containerID).getContainerData(); - if (data.getSchemaVersion().equals(SCHEMA_V1)) { + data.setSchemaVersion(schemaVersion); + if (schemaVersion.equals(SCHEMA_V1)) { createPendingDeleteBlocksSchema1(numOfBlocksPerContainer, data, containerID, numOfChunksPerBlock, buffer, chunkManager, container); - } else if (data.getSchemaVersion().equals(SCHEMA_V2)) { + } else if (schemaVersion.equals(SCHEMA_V2)) { createPendingDeleteBlocksSchema2(numOfBlocksPerContainer, txnID, containerID, numOfChunksPerBlock, buffer, chunkManager, container, data); @@ -254,7 +255,6 @@ private void createPendingDeleteBlocksSchema2(int numOfBlocksPerContainer, ChunkManager chunkManager, KeyValueContainer container, KeyValueContainerData data) { List containerBlocks = new ArrayList<>(); - int blockCount = 0; for (int i = 0; i < numOfBlocksPerContainer; i++) { txnID = txnID + 1; BlockID blockID = ContainerTestHelper.getTestBlockID(containerID); @@ -273,20 +273,12 @@ private void createPendingDeleteBlocksSchema2(int numOfBlocksPerContainer, } container.getContainerData().incrPendingDeletionBlocks(1); - // In below if statements we are checking if a single container - // consists of more blocks than 'blockLimitPerTask' then we create - // (totalBlocksInContainer / blockLimitPerTask) transactions which - // consists of blocks equal to blockLimitPerTask and last transaction - // consists of blocks equal to - // (totalBlocksInContainer % blockLimitPerTask). + // Below we are creating one transaction per block just for + // testing purpose + containerBlocks.add(blockID.getLocalID()); - blockCount++; - if (blockCount == blockLimitPerTask || i == (numOfBlocksPerContainer - - 1)) { - createTxn(data, containerBlocks, txnID, containerID); - containerBlocks.clear(); - blockCount = 0; - } + createTxn(data, containerBlocks, txnID, containerID); + containerBlocks.clear(); } updateMetaData(data, container, numOfBlocksPerContainer, numOfChunksPerBlock); @@ -405,11 +397,10 @@ private int getUnderDeletionBlocksCount(ReferenceCountedDB meta, @Test public void testBlockDeletion() throws Exception { - conf.setInt(OZONE_BLOCK_DELETING_CONTAINER_LIMIT_PER_INTERVAL, 10); - conf.setInt(OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER, 2); - this.blockLimitPerTask = - conf.getInt(OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER, - OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER_DEFAULT); + DatanodeConfiguration dnConf = conf.getObject(DatanodeConfiguration.class); + dnConf.setBlockDeletionLimit(3); + this.blockLimitPerInterval = dnConf.getBlockDeletionLimit(); + conf.setFromObject(dnConf); ContainerSet containerSet = new ContainerSet(); createToDeleteBlocks(containerSet, 1, 3, 1); ContainerMetrics metrics = ContainerMetrics.create(conf); @@ -511,11 +502,10 @@ public void testShutdownService() throws Exception { @Test public void testBlockDeletionTimeout() throws Exception { - conf.setInt(OZONE_BLOCK_DELETING_CONTAINER_LIMIT_PER_INTERVAL, 10); - conf.setInt(OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER, 2); - this.blockLimitPerTask = - conf.getInt(OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER, - OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER_DEFAULT); + DatanodeConfiguration dnConf = conf.getObject(DatanodeConfiguration.class); + dnConf.setBlockDeletionLimit(3); + blockLimitPerInterval = dnConf.getBlockDeletionLimit(); + conf.setFromObject(dnConf); ContainerSet containerSet = new ContainerSet(); createToDeleteBlocks(containerSet, 1, 3, 1); ContainerMetrics metrics = ContainerMetrics.create(conf); @@ -546,8 +536,9 @@ public void testBlockDeletionTimeout() throws Exception { svc.shutdown(); // test for normal case that doesn't have timeout limitation - timeout = 0; + createToDeleteBlocks(containerSet, 1, 3, 1); + timeout = 0; svc = new BlockDeletingService(ozoneContainer, TimeUnit.MILLISECONDS.toNanos(1000), timeout, TimeUnit.MILLISECONDS, conf); @@ -558,7 +549,6 @@ public void testBlockDeletionTimeout() throws Exception { (KeyValueContainer) containerSet.getContainerIterator().next(); KeyValueContainerData data = container.getContainerData(); try (ReferenceCountedDB meta = BlockUtils.getDB(data, conf)) { - LogCapturer newLog = LogCapturer.captureLogs(BackgroundService.LOG); GenericTestUtils.waitFor(() -> { try { @@ -611,11 +601,10 @@ public void testContainerThrottle() throws Exception { conf.set( ScmConfigKeys.OZONE_SCM_KEY_VALUE_CONTAINER_DELETION_CHOOSING_POLICY, TopNOrderedContainerDeletionChoosingPolicy.class.getName()); - conf.setInt(OZONE_BLOCK_DELETING_CONTAINER_LIMIT_PER_INTERVAL, 1); - conf.setInt(OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER, 1); - this.blockLimitPerTask = - conf.getInt(OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER, - OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER_DEFAULT); + DatanodeConfiguration dnConf = conf.getObject(DatanodeConfiguration.class); + dnConf.setBlockDeletionLimit(1); + this.blockLimitPerInterval = dnConf.getBlockDeletionLimit(); + conf.setFromObject(dnConf); ContainerSet containerSet = new ContainerSet(); int containerCount = 2; @@ -650,7 +639,7 @@ public void testContainerThrottle() throws Exception { // containers should be zero. deleteAndWait(service, 2); - Assert.assertTrue((containerData.get(1).getBytesUsed() == 0) && ( + Assert.assertTrue((containerData.get(0).getBytesUsed() == 0) && ( containerData.get(1).getBytesUsed() == 0)); } finally { service.shutdown(); @@ -675,12 +664,13 @@ public void testBlockThrottle() throws Exception { // - Container limit per interval : 10 // - Block limit per container : 2 // - // Each time containers can be all scanned, but only 2 blocks - // per container can be actually deleted. So it requires 2 waves - // to cleanup all blocks. - conf.setInt(OZONE_BLOCK_DELETING_CONTAINER_LIMIT_PER_INTERVAL, 10); - blockLimitPerTask = 2; - conf.setInt(OZONE_BLOCK_DELETING_LIMIT_PER_CONTAINER, blockLimitPerTask); + // Each time containers can be all scanned, but only 10 blocks + // can be actually deleted. So it requires 2 waves + // to cleanup all the 15 blocks. + DatanodeConfiguration dnConf = conf.getObject(DatanodeConfiguration.class); + dnConf.setBlockDeletionLimit(10); + this.blockLimitPerInterval = dnConf.getBlockDeletionLimit(); + conf.setFromObject(dnConf); ContainerSet containerSet = new ContainerSet(); ContainerMetrics metrics = ContainerMetrics.create(conf); KeyValueHandler keyValueHandler = @@ -703,14 +693,16 @@ public void testBlockThrottle() throws Exception { try { GenericTestUtils.waitFor(service::isStarted, 100, 3000); // Total blocks = 3 * 5 = 15 - // block per task = 2 - // number of containers = 5 - // each interval will at most runDeletingTasks 5 * 2 = 10 blocks + // blockLimitPerInterval = 10 + // each interval will at most runDeletingTasks = 10 blocks + // but as per of deletion policy (random/topNorder), it will fetch all 3 + // blocks from first 3 containers and 1 block from last container. + // C1 - 3 BLOCKS, C2 - 3 BLOCKS, C3 - 3 BLOCKS, C4 - 1 BLOCK // Deleted space of 10 blocks should be equal to (initial total space // of container - current total space of container). deleteAndWait(service, 1); - Assert.assertEquals(blockLimitPerTask * containerCount * blockSpace, + Assert.assertEquals(blockLimitPerInterval * blockSpace, (totalContainerSpace - currentBlockSpace(containerData, containerCount))); diff --git a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDeletionChoosingPolicy.java b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDeletionChoosingPolicy.java index 4cd0e516f712..482bf95c9f41 100644 --- a/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDeletionChoosingPolicy.java +++ b/hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/container/common/impl/TestContainerDeletionChoosingPolicy.java @@ -25,8 +25,9 @@ import java.util.Map; import java.util.Random; import java.util.UUID; +import java.util.ArrayList; +import java.util.Collections; import java.util.concurrent.TimeUnit; - import org.apache.commons.io.FileUtils; import org.apache.commons.lang3.RandomUtils; import org.apache.hadoop.hdfs.server.datanode.StorageLocation; @@ -38,6 +39,7 @@ import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainer; import org.apache.hadoop.ozone.container.keyvalue.KeyValueContainerData; import org.apache.hadoop.ozone.container.keyvalue.statemachine.background.BlockDeletingService; +import org.apache.hadoop.ozone.container.keyvalue.statemachine.background.BlockDeletingService.ContainerBlockInfo; import org.apache.hadoop.ozone.container.ozoneimpl.OzoneContainer; import org.apache.hadoop.test.GenericTestUtils; import org.junit.Assert; @@ -95,11 +97,13 @@ public void testRandomChoosingPolicy() throws IOException { containerSet = new ContainerSet(); int numContainers = 10; + Random random = new Random(); for (int i = 0; i < numContainers; i++) { KeyValueContainerData data = new KeyValueContainerData(i, layout, ContainerTestHelper.CONTAINER_MAX_SIZE, UUID.randomUUID().toString(), UUID.randomUUID().toString()); + data.incrPendingDeletionBlocks(random.nextInt(numContainers) + 1); data.closeContainer(); KeyValueContainer container = new KeyValueContainer(data, conf); containerSet.addContainer(container); @@ -109,27 +113,35 @@ public void testRandomChoosingPolicy() throws IOException { } blockDeletingService = getBlockDeletingService(); + int blockLimitPerInterval = 5; ContainerDeletionChoosingPolicy deletionPolicy = new RandomContainerDeletionChoosingPolicy(); - List result0 = - blockDeletingService.chooseContainerForBlockDeletion(5, deletionPolicy); - Assert.assertEquals(5, result0.size()); + List result0 = blockDeletingService + .chooseContainerForBlockDeletion(blockLimitPerInterval, deletionPolicy); + + long totPendingBlocks = 0; + for (ContainerBlockInfo pr : result0) { + totPendingBlocks += pr.getBlocks(); + } + Assert.assertTrue(totPendingBlocks >= blockLimitPerInterval); + // test random choosing - List result1 = blockDeletingService + List result1 = blockDeletingService .chooseContainerForBlockDeletion(numContainers, deletionPolicy); - List result2 = blockDeletingService + List result2 = blockDeletingService .chooseContainerForBlockDeletion(numContainers, deletionPolicy); boolean hasShuffled = false; for (int i = 0; i < numContainers; i++) { - if (result1.get(i).getContainerID() - != result2.get(i).getContainerID()) { + if (result1.get(i).getContainerData().getContainerID() != result2.get(i) + .getContainerData().getContainerID()) { hasShuffled = true; break; } } Assert.assertTrue("Chosen container results were same", hasShuffled); + } @Test @@ -150,6 +162,7 @@ public void testTopNOrderedChoosingPolicy() throws IOException { int numContainers = 10; Random random = new Random(); Map name2Count = new HashMap<>(); + List numberOfBlocks = new ArrayList(); // create [numContainers + 1] containers for (int i = 0; i <= numContainers; i++) { long containerId = RandomUtils.nextLong(); @@ -161,6 +174,7 @@ public void testTopNOrderedChoosingPolicy() throws IOException { UUID.randomUUID().toString()); if (i != numContainers) { int deletionBlocks = random.nextInt(numContainers) + 1; + numberOfBlocks.add(deletionBlocks); data.incrPendingDeletionBlocks(deletionBlocks); name2Count.put(containerId, deletionBlocks); } @@ -170,29 +184,47 @@ public void testTopNOrderedChoosingPolicy() throws IOException { Assert.assertTrue( containerSet.getContainerMapCopy().containsKey(containerId)); } - + numberOfBlocks.sort(Collections.reverseOrder()); + int blockLimitPerInterval = 5; blockDeletingService = getBlockDeletingService(); ContainerDeletionChoosingPolicy deletionPolicy = new TopNOrderedContainerDeletionChoosingPolicy(); - List result0 = - blockDeletingService.chooseContainerForBlockDeletion(5, deletionPolicy); - Assert.assertEquals(5, result0.size()); + List result0 = blockDeletingService + .chooseContainerForBlockDeletion(blockLimitPerInterval, deletionPolicy); + long totPendingBlocks = 0; + for (ContainerBlockInfo pr : result0) { + totPendingBlocks += pr.getBlocks(); + } + Assert.assertTrue(totPendingBlocks >= blockLimitPerInterval); + - List result1 = blockDeletingService + List result1 = blockDeletingService .chooseContainerForBlockDeletion(numContainers + 1, deletionPolicy); // the empty deletion blocks container should not be chosen - Assert.assertEquals(numContainers, result1.size()); + int containerCount = 0; + int c = 0; + for (int i = 0; i < numberOfBlocks.size(); i++) { + containerCount++; + c = c + numberOfBlocks.get(i); + if (c >= (numContainers + 1)) { + break; + } + } + Assert.assertEquals(containerCount, result1.size()); // verify the order of return list + int initialName2CountSize = name2Count.size(); int lastCount = Integer.MAX_VALUE; - for (ContainerData data : result1) { - int currentCount = name2Count.remove(data.getContainerID()); + for (ContainerBlockInfo data : result1) { + int currentCount = + name2Count.remove(data.getContainerData().getContainerID()); // previous count should not smaller than next one Assert.assertTrue(currentCount > 0 && currentCount <= lastCount); lastCount = currentCount; } // ensure all the container data are compared - Assert.assertEquals(0, name2Count.size()); + Assert.assertEquals(result1.size(), + initialName2CountSize - name2Count.size()); } private BlockDeletingService getBlockDeletingService() {