You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by GitBox <gi...@apache.org> on 2022/08/04 12:12:39 UTC
[GitHub] [ozone] kaijchen opened a new pull request, #3655: HDDS-7091. Filter containers with pending deletion blocks before checking policy
kaijchen opened a new pull request, #3655:
URL: https://github.com/apache/ozone/pull/3655
## What changes were proposed in this pull request?
Previously, containers with zero pending deletion blocks will also be sent to check container chosing policy, which is unnecessary. We can filter them out before passing to any policy.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-7091
## How was this patch tested?
Manual tested with debug log added.
Before:
```
2022-08-04 14:14:23,725 [BlockDeletingService#8] INFO org.apache.hadoop.ozone.container.keyvalue.statemachine.background.BlockDeletingService: Found 57 candidates for bl
ock deletion
2022-08-04 14:14:23,725 [BlockDeletingService#8] INFO org.apache.hadoop.ozone.container.common.impl.TopNOrderedContainerDeletionChoosingPolicy: Container 128 has 0 pendi
ng deletion blocks
2022-08-04 14:14:23,725 [BlockDeletingService#8] INFO org.apache.hadoop.ozone.container.common.impl.TopNOrderedContainerDeletionChoosingPolicy: Stop looking for next con
tainer, there is no pending deletion block contained in remaining containers.
2022-08-04 14:14:23,725 [BlockDeletingService#8] INFO org.apache.hadoop.ozone.container.keyvalue.statemachine.background.BlockDeletingService: Found 0 containers for blo
ck deletion
```
After:
```
2022-08-04 20:07:55,288 [BlockDeletingService#2] INFO org.apache.hadoop.ozone.container.keyvalue.statemachine.background.BlockDeletingService: Found 0 containers with pending deletion blocks and allowed for deletion
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on pull request #3655: HDDS-7091. Filter containers with pending deletion blocks before checking policy
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #3655:
URL: https://github.com/apache/ozone/pull/3655#issuecomment-1409016429
@kaijchen Can you rebase with master to rerun CI.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on pull request #3655: HDDS-7091. Filter containers with pending deletion blocks before checking policy
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #3655:
URL: https://github.com/apache/ozone/pull/3655#issuecomment-1406951233
@aswinshakil Can this be merged?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kaijchen commented on a diff in pull request #3655: HDDS-7091. Filter containers with pending deletion blocks before checking policy
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on code in PR #3655:
URL: https://github.com/apache/ozone/pull/3655#discussion_r1089860898
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java:
##########
@@ -180,6 +180,8 @@ public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
throws StorageContainerException {
Map<Long, ContainerData> containerDataMap =
ozoneContainer.getContainerSet().getContainerMap().entrySet().stream()
+ .filter(e -> ((KeyValueContainerData) e.getValue()
+ .getContainerData()).getNumPendingDeletionBlocks() > 0)
Review Comment:
> NIT: I believe this should not be done it is true currently only BlockDeletingService is calling the function but if there is some other implementation it would ending up doing the calculation anyways. But this should also be fine
Good point. However, if there is some other implementation, it would probably check some other condition instead of `entry.getNumPendingDeletionBlocks() > 0`.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai commented on pull request #3655: HDDS-7091. Filter containers with pending deletion blocks before checking policy
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #3655:
URL: https://github.com/apache/ozone/pull/3655#issuecomment-1416801901
Thanks @kaijchen for the patch, @aswinshakil, @swamirishi for the review.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] adoroszlai merged pull request #3655: HDDS-7091. Filter containers with pending deletion blocks before checking policy
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #3655:
URL: https://github.com/apache/ozone/pull/3655
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kaijchen commented on a diff in pull request #3655: HDDS-7091. Filter containers with pending deletion blocks before checking policy
Posted by GitBox <gi...@apache.org>.
kaijchen commented on code in PR #3655:
URL: https://github.com/apache/ozone/pull/3655#discussion_r954668478
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java:
##########
@@ -180,6 +180,8 @@ public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
throws StorageContainerException {
Map<Long, ContainerData> containerDataMap =
ozoneContainer.getContainerSet().getContainerMap().entrySet().stream()
+ .filter(e -> ((KeyValueContainerData) e.getValue()
+ .getContainerData()).getNumPendingDeletionBlocks() > 0)
Review Comment:
Done, thanks for the suggestion.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] swamirishi commented on a diff in pull request #3655: HDDS-7091. Filter containers with pending deletion blocks before checking policy
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on code in PR #3655:
URL: https://github.com/apache/ozone/pull/3655#discussion_r1089316753
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java:
##########
@@ -180,6 +180,8 @@ public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
throws StorageContainerException {
Map<Long, ContainerData> containerDataMap =
ozoneContainer.getContainerSet().getContainerMap().entrySet().stream()
+ .filter(e -> ((KeyValueContainerData) e.getValue()
+ .getContainerData()).getNumPendingDeletionBlocks() > 0)
Review Comment:
NIT: I believe this should not be done it is true currently only BlockDeletingService is calling the function but if there is some other implementation it would ending up doing the calculation anyways. But this should also be fine.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] kerneltime commented on pull request #3655: HDDS-7091. Filter containers with pending deletion blocks before checking policy
Posted by GitBox <gi...@apache.org>.
kerneltime commented on PR #3655:
URL: https://github.com/apache/ozone/pull/3655#issuecomment-1222588049
@aswinshakil
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
[GitHub] [ozone] aswinshakil commented on a diff in pull request #3655: HDDS-7091. Filter containers with pending deletion blocks before checking policy
Posted by GitBox <gi...@apache.org>.
aswinshakil commented on code in PR #3655:
URL: https://github.com/apache/ozone/pull/3655#discussion_r951790063
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java:
##########
@@ -180,6 +180,8 @@ public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
throws StorageContainerException {
Map<Long, ContainerData> containerDataMap =
ozoneContainer.getContainerSet().getContainerMap().entrySet().stream()
+ .filter(e -> ((KeyValueContainerData) e.getValue()
+ .getContainerData()).getNumPendingDeletionBlocks() > 0)
Review Comment:
Since we are already filtering ContainerData based on the pending block deletes. We should refactor the code to not check again for `entry.getNumPendingDeletionBlocks() > 0`
https://github.com/apache/ozone/blob/8d331a371fca43f5f7a4cf027f227f237388dc46/hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/impl/TopNOrderedContainerDeletionChoosingPolicy.java#L76-L83
--
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.
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org