-
Notifications
You must be signed in to change notification settings - Fork 3.4k
HBASE-27948 Report memstore on-heap and off-heap size as jmx metrics in sub=Memory bean #5293
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -317,12 +317,15 @@ private void tune() { | |
unblockedFlushCnt = unblockedFlushCount.getAndSet(0); | ||
tunerContext.setUnblockedFlushCount(unblockedFlushCnt); | ||
metricsHeapMemoryManager.updateUnblockedFlushCount(unblockedFlushCnt); | ||
// TODO : add support for offheap metrics | ||
tunerContext.setCurBlockCacheUsed((float) blockCache.getCurrentSize() / maxHeapSize); | ||
metricsHeapMemoryManager.setCurBlockCacheSizeGauge(blockCache.getCurrentSize()); | ||
long globalMemstoreDataSize = regionServerAccounting.getGlobalMemStoreDataSize(); | ||
long globalMemstoreHeapSize = regionServerAccounting.getGlobalMemStoreHeapSize(); | ||
long globalMemStoreOffHeapSize = regionServerAccounting.getGlobalMemStoreOffHeapSize(); | ||
tunerContext.setCurMemStoreUsed((float) globalMemstoreHeapSize / maxHeapSize); | ||
metricsHeapMemoryManager.setCurMemStoreSizeGauge(globalMemstoreHeapSize); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. yes, this is what i was wondering. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 The metrics I was referring to are in There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 FYI @jinggou There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bbeaudreault Sorry I didn't realize that you are referring to 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 |
||
metricsHeapMemoryManager.setCurMemStoreSizeGauge(globalMemstoreDataSize); | ||
metricsHeapMemoryManager.setCurMemStoreOnHeapSizeGauge(globalMemstoreHeapSize); | ||
metricsHeapMemoryManager.setCurMemStoreOffHeapSizeGauge(globalMemStoreOffHeapSize); | ||
tunerContext.setCurBlockCacheSize(blockCachePercent); | ||
tunerContext.setCurMemStoreSize(globalMemStorePercent); | ||
TunerResult result = null; | ||
|
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.