-
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-27948 Report memstore on-heap and off-heap size as jmx metrics in sub=Memory bean #5293
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. |
@@ -118,6 +130,13 @@ public interface MetricsHeapMemoryManagerSource extends BaseSource { | |||
String UNBLOCKED_FLUSH_GAUGE_DESC = "Gauge for the unblocked flush count before tuning"; | |||
String MEMSTORE_SIZE_GAUGE_NAME = "memStoreSize"; | |||
String MEMSTORE_SIZE_GAUGE_DESC = "Global MemStore used in bytes by the RegionServer"; | |||
String MEMSTORE_ONHEAP_SIZE_GAUGE_NAME = "memStoreOnHeapSize"; |
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.
Use OnHeap or just Heap? I'm not an English expert, just asking...
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.
Both Heap and OnHeap are used to referring to on-heap size in our code, so I think both are okay, and choose OnHeap here to differentiate it with OffHeap.
tunerContext.setCurMemStoreUsed((float) globalMemstoreHeapSize / maxHeapSize); | ||
metricsHeapMemoryManager.setCurMemStoreSizeGauge(globalMemstoreHeapSize); |
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.
So in the old time we just use heap size as memstore size? This should be bug?
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.
yes, this is what i was wondering.
FYI @bbeaudreault as he discovered this
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.
Ah interesting.
So I wasn't talking about these metrics at all. These metrics show up in JMX under sub=Memory
(which for us is all 0's for some reason, and we don't use them).
The metrics I was referring to are in sub=Server
and also per-region and per-table metrics. These also have a memStoreSize
field, and it is derived from the DataSize rather than HeapSize. These are calculated in a few places, you have to sort of dig in based on usages of MetricsRegionServerSource.MEMSTORE_SIZE
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.
Sorry for the confusion, I didn't realize these metrics existed. Maybe they are better? I need to read more.
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.
i was not aware either, realized these metrics exist and this is likely bug only after Jing created this PR, i also need to do some digging here.
Thanks for pointing out sub=Server
@bbeaudreault
FYI @jinggou
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.
@bbeaudreault Sorry I didn't realize that you are referring to sub=Server
. This change is made for sub=Memory
, so I create a new pr (#5308) for HBASE-27892.
For the code change here, I think it might be a bug because memStoreSize should refer to memstore data size instead of memstore heap size, and also noticed that its value is 0 under sub=Memory
(while not 0 under sub=Server
). I've created a new jira HBASE-27948 to figure out this issue. @Apache9 @virajjasani
@jinggou what about the new metrics that you introduced with this PR? they are also being reported as 0 under Memory section? |
Yes. |
this needs work, maybe in separate Jira |
…in sub=Memory bean (#5293) Signed-off-by: Viraj Jasani <[email protected]>
…in sub=Memory bean (#5293) Signed-off-by: Viraj Jasani <[email protected]>
…in sub=Memory bean (#5293) Signed-off-by: Viraj Jasani <[email protected]>
Jira: HBASE-27948