You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "xichen01 (via GitHub)" <gi...@apache.org> on 2023/05/30 15:15:17 UTC
[GitHub] [ozone] xichen01 opened a new pull request, #4800: HDDS-8729. Add metrics for count of pending Block on DN
xichen01 opened a new pull request, #4800:
URL: https://github.com/apache/ozone/pull/4800
## What changes were proposed in this pull request?
Add a Metrics to monitor the `Pending Block` Count.
`block_deleting_service_metrics_total_pending_block_count{context="dfs",hostname="centos"} 0`
### Example:
```
[root@centos ~/root/ozone]$ curl -s http://127.0.0.1:9882/prom | grep block_deleting_service_metrics | grep -v '#'
block_deleting_service_metrics_failure_count{context="dfs",hostname="centos"} 0
block_deleting_service_metrics_out_of_order_delete_block_transaction_count{context="dfs",hostname="centos"} 0
block_deleting_service_metrics_success_bytes{context="dfs",hostname="-centos"} 0
block_deleting_service_metrics_success_count{context="dfs",hostname="centos"} 0
block_deleting_service_metrics_total_pending_block_count{context="dfs",hostname="centos"} 0
[root@centos ~/root/ozone]$
```
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-8729
## How was this patch tested?
manual test and unit test
--
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] Xushaohong commented on a diff in pull request #4800: HDDS-8729. Add metrics for count of pending Block on DN
Posted by "Xushaohong (via GitHub)" <gi...@apache.org>.
Xushaohong commented on code in PR #4800:
URL: https://github.com/apache/ozone/pull/4800#discussion_r1211295310
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java:
##########
@@ -175,13 +175,19 @@ public BackgroundTaskQueue getTasks() {
public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
int blockLimit, ContainerDeletionChoosingPolicy deletionPolicy)
throws StorageContainerException {
+ long[] totalPendingBlockCount = new long[1];
Review Comment:
Got it.
--
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] xichen01 commented on a diff in pull request #4800: HDDS-8729. Add metrics for count of pending Block on DN
Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #4800:
URL: https://github.com/apache/ozone/pull/4800#discussion_r1218428181
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java:
##########
@@ -175,13 +175,19 @@ public BackgroundTaskQueue getTasks() {
public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
int blockLimit, ContainerDeletionChoosingPolicy deletionPolicy)
throws StorageContainerException {
+ long[] totalPendingBlockCount = new long[1];
Map<Long, ContainerData> containerDataMap =
ozoneContainer.getContainerSet().getContainerMap().entrySet().stream()
.filter(e -> ((KeyValueContainerData) e.getValue()
.getContainerData()).getNumPendingDeletionBlocks() > 0)
.filter(e -> isDeletionAllowed(e.getValue().getContainerData(),
- deletionPolicy)).collect(Collectors
+ deletionPolicy))
+ .peek(e -> totalPendingBlockCount[0] +=
Review Comment:
Do you mean to use `containerDataMap` to do the calculation? which seems clearer, but it may need to iterate through the `Contianer` again if there are more Contianers, which may add some processing time.
--
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] symious commented on a diff in pull request #4800: HDDS-8729. Add metrics for count of pending Block on DN
Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on code in PR #4800:
URL: https://github.com/apache/ozone/pull/4800#discussion_r1217368336
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java:
##########
@@ -175,13 +175,19 @@ public BackgroundTaskQueue getTasks() {
public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
int blockLimit, ContainerDeletionChoosingPolicy deletionPolicy)
throws StorageContainerException {
+ long[] totalPendingBlockCount = new long[1];
Map<Long, ContainerData> containerDataMap =
ozoneContainer.getContainerSet().getContainerMap().entrySet().stream()
.filter(e -> ((KeyValueContainerData) e.getValue()
.getContainerData()).getNumPendingDeletionBlocks() > 0)
.filter(e -> isDeletionAllowed(e.getValue().getContainerData(),
- deletionPolicy)).collect(Collectors
+ deletionPolicy))
+ .peek(e -> totalPendingBlockCount[0] +=
Review Comment:
I'm not sure `peek` is good to use here.
An explicit sum on the result map might be better.
--
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 #4800: HDDS-8729. Add metrics for count of pending Block on DN
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4800:
URL: https://github.com/apache/ozone/pull/4800#issuecomment-1582163829
Thanks @xichen01 for the patch, @symious, @Xushaohong 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 #4800: HDDS-8729. Add metrics for count of pending Block on DN
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai merged PR #4800:
URL: https://github.com/apache/ozone/pull/4800
--
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] symious commented on a diff in pull request #4800: HDDS-8729. Add metrics for count of pending Block on DN
Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on code in PR #4800:
URL: https://github.com/apache/ozone/pull/4800#discussion_r1220670986
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java:
##########
@@ -175,13 +175,19 @@ public BackgroundTaskQueue getTasks() {
public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
int blockLimit, ContainerDeletionChoosingPolicy deletionPolicy)
throws StorageContainerException {
+ long[] totalPendingBlockCount = new long[1];
Map<Long, ContainerData> containerDataMap =
ozoneContainer.getContainerSet().getContainerMap().entrySet().stream()
.filter(e -> ((KeyValueContainerData) e.getValue()
.getContainerData()).getNumPendingDeletionBlocks() > 0)
.filter(e -> isDeletionAllowed(e.getValue().getContainerData(),
- deletionPolicy)).collect(Collectors
+ deletionPolicy))
+ .peek(e -> totalPendingBlockCount[0] +=
Review Comment:
Thanks for the update. LGTM.
--
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] symious commented on a diff in pull request #4800: HDDS-8729. Add metrics for count of pending Block on DN
Posted by "symious (via GitHub)" <gi...@apache.org>.
symious commented on code in PR #4800:
URL: https://github.com/apache/ozone/pull/4800#discussion_r1218802760
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java:
##########
@@ -175,13 +175,19 @@ public BackgroundTaskQueue getTasks() {
public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
int blockLimit, ContainerDeletionChoosingPolicy deletionPolicy)
throws StorageContainerException {
+ long[] totalPendingBlockCount = new long[1];
Map<Long, ContainerData> containerDataMap =
ozoneContainer.getContainerSet().getContainerMap().entrySet().stream()
.filter(e -> ((KeyValueContainerData) e.getValue()
.getContainerData()).getNumPendingDeletionBlocks() > 0)
.filter(e -> isDeletionAllowed(e.getValue().getContainerData(),
- deletionPolicy)).collect(Collectors
+ deletionPolicy))
+ .peek(e -> totalPendingBlockCount[0] +=
Review Comment:
`peek` seems not a stable api to use for now according to its description.
Clearer code can be better for 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] Xushaohong commented on a diff in pull request #4800: HDDS-8729. Add metrics for count of pending Block on DN
Posted by "Xushaohong (via GitHub)" <gi...@apache.org>.
Xushaohong commented on code in PR #4800:
URL: https://github.com/apache/ozone/pull/4800#discussion_r1211016454
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java:
##########
@@ -175,13 +175,19 @@ public BackgroundTaskQueue getTasks() {
public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
int blockLimit, ContainerDeletionChoosingPolicy deletionPolicy)
throws StorageContainerException {
+ long[] totalPendingBlockCount = new long[1];
Review Comment:
Why use an array here for a single variable?
--
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 #4800: HDDS-8729. Add metrics for count of pending Block on DN
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #4800:
URL: https://github.com/apache/ozone/pull/4800#issuecomment-1572100040
@tanvipenumudy please 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] xichen01 commented on a diff in pull request #4800: HDDS-8729. Add metrics for count of pending Block on DN
Posted by "xichen01 (via GitHub)" <gi...@apache.org>.
xichen01 commented on code in PR #4800:
URL: https://github.com/apache/ozone/pull/4800#discussion_r1211221429
##########
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/keyvalue/statemachine/background/BlockDeletingService.java:
##########
@@ -175,13 +175,19 @@ public BackgroundTaskQueue getTasks() {
public List<ContainerBlockInfo> chooseContainerForBlockDeletion(
int blockLimit, ContainerDeletionChoosingPolicy deletionPolicy)
throws StorageContainerException {
+ long[] totalPendingBlockCount = new long[1];
Review Comment:
`totalPendingBlockCount` is an array of size 1 because variables used in lambda expressions must be effectively final. Using an array allows you to modify its content inside the lambda while keeping the array reference unchanged.
--
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