-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HBASE-25653 Add units and round off region size to 2 digits after decimal #3046
Conversation
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
@ndimiduk @virajjasani please review. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Just one question. Have you tried it?
@@ -284,7 +284,7 @@ private double getAverageRegionSizeMb(final List<RegionInfo> tableRegions, | |||
double avgRegionSize; | |||
int targetRegionCount = tableDescriptor.getNormalizerTargetRegionCount(); | |||
long targetRegionSize = tableDescriptor.getNormalizerTargetRegionSize(); | |||
LOG.debug("Table {} configured with target region count {}, target region size {}", table, | |||
LOG.debug("Table {} configured with target region count {}, target region size {} MB", table, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sure these are MB values? I tried looking at code but it doesn't say explicitly...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, there are some checkstyle complaints to fix? See https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3046/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sure these are MB values? I tried looking at code but it doesn't say explicitly...
Yeah, but there is a hint of it in the alter command of hbase shell.
Also, avgRegionSize
is set to targetRegionSize
if its set, else the computed value for avgRegionSize
is in MBs. So its an inference.
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
Tested with hbase in standalone mode.
|
…imal (apache#3046) Signed-off-by: stack <stack@duboce.net> Reviewed-by: Viraj Jasani <vjasani@apache.org>
…imal (apache#3046) Signed-off-by: stack <stack@duboce.net> Reviewed-by: Viraj Jasani <vjasani@apache.org>
…imal (apache#3046) Signed-off-by: stack <stack@duboce.net> Reviewed-by: Viraj Jasani <vjasani@apache.org>
…imal (#3046) Signed-off-by: stack <stack@duboce.net> Reviewed-by: Viraj Jasani <vjasani@apache.org>
…imal (#3046) Signed-off-by: stack <stack@duboce.net> Reviewed-by: Viraj Jasani <vjasani@apache.org>
…imal (#3046) Signed-off-by: stack <stack@duboce.net> Reviewed-by: Viraj Jasani <vjasani@apache.org>
No description provided.