lokeshj1703 commented on a change in pull request #1885:
URL: https://github.com/apache/ozone/pull/1885#discussion_r595038005
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/statemachine/DatanodeConfiguration.java
##########
@@ -58,6 +58,7 @@
private int replicationMaxStreams = REPLICATION_MAX_STREAMS_DEFAULT;
static final int CONTAINER_DELETE_THREADS_DEFAULT = 2;
+ static final int OZONE_BLOCK_DELETING_LIMIT_PER_INTERVAL = 1000;
Review comment:
We can remove this field.
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/RandomContainerDeletionChoosingPolicy.java
##########
@@ -40,33 +41,41 @@
LoggerFactory.getLogger(RandomContainerDeletionChoosingPolicy.class);
@Override
- public List<ContainerData> chooseContainerForBlockDeletion(int count,
- Map<Long, ContainerData> candidateContainers)
+ public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
+ int blockCount, Map<Long, ContainerData> candidateContainers)
throws StorageContainerException {
Preconditions.checkNotNull(candidateContainers,
"Internal assertion: candidate containers cannot be null");
- int currentCount = 0;
- List<ContainerData> result = new LinkedList<>();
+ List<ContainerBlockInfo> 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) {
+ blockCount -=
+ ((KeyValueContainerData) entry).getNumPendingDeletionBlocks();
+ result.add(new ContainerBlockInfo(entry,
+ ((KeyValueContainerData) entry).getNumPendingDeletionBlocks()));
+ if (blockCount <= 0) {
+ break;
+ }
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());
Review comment:
We will need to move this above the if condition for if statement.
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/RandomContainerDeletionChoosingPolicy.java
##########
@@ -40,33 +41,41 @@
LoggerFactory.getLogger(RandomContainerDeletionChoosingPolicy.class);
@Override
- public List<ContainerData> chooseContainerForBlockDeletion(int count,
- Map<Long, ContainerData> candidateContainers)
+ public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
+ int blockCount, Map<Long, ContainerData> candidateContainers)
throws StorageContainerException {
Preconditions.checkNotNull(candidateContainers,
"Internal assertion: candidate containers cannot be null");
- int currentCount = 0;
- List<ContainerData> result = new LinkedList<>();
+ List<ContainerBlockInfo> 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) {
+ blockCount -=
+ ((KeyValueContainerData) entry).getNumPendingDeletionBlocks();
+ result.add(new ContainerBlockInfo(entry,
+ ((KeyValueContainerData) entry).getNumPendingDeletionBlocks()));
Review comment:
Similar for TopN Policy.
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java
##########
@@ -314,11 +325,13 @@ public ContainerBackgroundTaskResult deleteViaSchema1(
Handler handler = Objects.requireNonNull(ozoneContainer.getDispatcher()
.getHandler(container.getContainerType()));
+ int blocksDeleted = 0;
Review comment:
We don't need this variable.
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java
##########
@@ -113,38 +105,54 @@ 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 subject =
conf.getObject(DatanodeConfiguration.class);
Review comment:
Let's rename it to dnConf or sth similar?
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/RandomContainerDeletionChoosingPolicy.java
##########
@@ -40,33 +41,41 @@
LoggerFactory.getLogger(RandomContainerDeletionChoosingPolicy.class);
@Override
- public List<ContainerData> chooseContainerForBlockDeletion(int count,
- Map<Long, ContainerData> candidateContainers)
+ public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
+ int blockCount, Map<Long, ContainerData> candidateContainers)
throws StorageContainerException {
Preconditions.checkNotNull(candidateContainers,
"Internal assertion: candidate containers cannot be null");
- int currentCount = 0;
- List<ContainerData> result = new LinkedList<>();
+ List<ContainerBlockInfo> 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) {
+ blockCount -=
+ ((KeyValueContainerData) entry).getNumPendingDeletionBlocks();
+ result.add(new ContainerBlockInfo(entry,
+ ((KeyValueContainerData) entry).getNumPendingDeletionBlocks()));
+ if (blockCount <= 0) {
+ break;
+ }
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());
Review comment:
Similar for TopN policy.
##########
File path:
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/RandomContainerDeletionChoosingPolicy.java
##########
@@ -40,33 +41,41 @@
LoggerFactory.getLogger(RandomContainerDeletionChoosingPolicy.class);
@Override
- public List<ContainerData> chooseContainerForBlockDeletion(int count,
- Map<Long, ContainerData> candidateContainers)
+ public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
+ int blockCount, Map<Long, ContainerData> candidateContainers)
throws StorageContainerException {
Preconditions.checkNotNull(candidateContainers,
"Internal assertion: candidate containers cannot be null");
- int currentCount = 0;
- List<ContainerData> result = new LinkedList<>();
+ List<ContainerBlockInfo> 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) {
+ blockCount -=
+ ((KeyValueContainerData) entry).getNumPendingDeletionBlocks();
+ result.add(new ContainerBlockInfo(entry,
+ ((KeyValueContainerData) entry).getNumPendingDeletionBlocks()));
Review comment:
We should delete a `min(blockCount, ((KeyValueContainerData)
entry).getNumPendingDeletionBlocks())`
```suggestion
int numBlocksToDelete = min(blockCount, ((KeyValueContainerData)
entry).getNumPendingDeletionBlocks());
blockCount -= numBlocksToDelete;
result.add(new ContainerBlockInfo(entry, numBlocksToDelete);
```
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
