HBASE-26441: Add metrics for BrokenStoreFileCleaner #3861
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Apache9
reviewed
Nov 19, 2021
|
|
||
| /** | ||
| * Increment the deleted files counter | ||
| * @param deletes |
Contributor
There was a problem hiding this comment.
nits: avoid empty javadoc element, it will lead to a checkstyle warning.
Contributor
Author
There was a problem hiding this comment.
Thanks for pointing it out!
| String FAILED_DELETES_DESC = | ||
| "Number of files BrokenStoreFileCleaner tried but failed to delete"; | ||
| String RUNS = "BrokenStoreFileCleanerRuns"; | ||
| String RUNS_DESC = "Number of time the BrokenStoreFileCleaner chore run"; |
| required uint64 runtime = 2; | ||
| optional uint64 deleted_files = 3; | ||
| optional uint64 failed_deletes = 4; | ||
| required uint32 runs = 5; |
Contributor
There was a problem hiding this comment.
In the above metrics class we use long to runs, why here we change to use uint32 instead of uint64?
Contributor
Author
There was a problem hiding this comment.
I'm not sure how it ended up as uint32. Changing it to uint64.
| returns(GetLiveRegionServersResponse); | ||
|
|
||
| /** Report BrokenStoreFileCleaner chore metrics to master */ | ||
| rpc ReportBrokenStoreFileCleanerUsage(BrokenStoreFileCleanerUsageRequest) |
Contributor
There was a problem hiding this comment.
Is it worth a separated method? How do we report the metrics for other background tasks on region server to master?
Contributor
Author
There was a problem hiding this comment.
Usually we do not report metrics. I do this here for 2 reasons:
- Convenience. Aggregating the rs metrics is possible and usually done by queries from the db, storing aggregated metrics here is for ease of use.
- Preparing things for the next PR where I'll add logic to store the reported metrics in-memory and extend the REST service to allow checking these values without having to access the metrics db.
Contributor
|
Any updates here? Thanks. |
…racking logic Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
…g from different store file tracker implementations (apache#3656) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
…iptor when creating table (apache#3666) Signed-off-by: Duo Zhang <zhangduo@apache.org>
… file tracker implementation (apache#3665) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
… tracker (apache#3681) Signed-off-by: Josh Elser <elserj@apache.org>
Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Reviewed-by: Josh Elser <elserj@apache.org>
apache#3721) Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Josh Elser <elserj@apache.org>
…et method (apache#3774) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
… impl (apache#3749) Signed-off-by: Duo Zhang <zhangduo@apache.org>
…ations to TableDescriptor for existing tables (apache#3700) Signed-off-by: Duo Zhang <zhangduo@apache.org> Reviewed-by: Wellington Ramos Chevreuil <wchevreuil@apache.org>
…he#3786) Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Josh Elser <elserj@apache.org> Signed-off-by: Wellington Ramos Chevreuil <wchevreuil@apache.org>
apache#3845) Signed-off-by: Duo Zhang <zhangduo@apache.org>
Contributor
|
I've rebased HBASE-26067 to the newest master. For addressing the UT problem so we can run most of UTs in the nightly job. Please rebase the PR. Thanks. |
add rs metricsfor BrokenStoreFileCleaner aggregate the rs metrics on master for easier access
BukrosSzabolcs
force-pushed
the
HBASE-26441
branch
from
31a5852 to
e9a86ef
Compare
Contributor
Author
|
@Apache9 Sorry for the slow response I was sick |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
Contributor
Oh, sorry... |
checkstlye fixes
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
…racking logic Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
…g from different store file tracker implementations (apache#3656) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
…iptor when creating table (apache#3666) Signed-off-by: Duo Zhang <zhangduo@apache.org>
… file tracker implementation (apache#3665) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
… tracker (apache#3681) Signed-off-by: Josh Elser <elserj@apache.org>
Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org> Reviewed-by: Josh Elser <elserj@apache.org>
apache#3721) Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Josh Elser <elserj@apache.org>
…et method (apache#3774) Signed-off-by: Wellington Chevreuil <wchevreuil@apache.org>
… impl (apache#3749) Signed-off-by: Duo Zhang <zhangduo@apache.org>
…ations to TableDescriptor for existing tables (apache#3700) Signed-off-by: Duo Zhang <zhangduo@apache.org> Reviewed-by: Wellington Ramos Chevreuil <wchevreuil@apache.org>
…he#3786) Signed-off-by: Duo Zhang <zhangduo@apache.org> Signed-off-by: Josh Elser <elserj@apache.org> Signed-off-by: Wellington Ramos Chevreuil <wchevreuil@apache.org>
apache#3845) Signed-off-by: Duo Zhang <zhangduo@apache.org>
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
|
🎊 +1 overall
This message was automatically generated. |
Contributor
Author
|
@Apache9 The failing unit test is unrelated it works just fine for me. Do you have any other feedback or can we wrap this up? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add rs metricsfor BrokenStoreFileCleaner
aggregate the rs metrics on master for easier access