Skip to content

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

Merged
merged 2 commits into from
Jun 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,18 @@ public interface MetricsHeapMemoryManagerSource extends BaseSource {
*/
void setCurMemStoreSizeGauge(long memStoreSize);

/**
* Set the current global memstore on-heap size used gauge
* @param memStoreOnHeapSize the current memory usage in memstore on-heap, in bytes.
*/
void setCurMemStoreOnHeapSizeGauge(long memStoreOnHeapSize);

/**
* Set the current global memstore off-heap size used gauge
* @param memStoreOffHeapSize the current memory usage in memstore off-heap, in bytes.
*/
void setCurMemStoreOffHeapSizeGauge(long memStoreOffHeapSize);

/**
* Update the increase/decrease memstore size histogram
* @param memStoreDeltaSize the tuning result of memstore.
Expand Down Expand Up @@ -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";
Copy link
Contributor

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...

Copy link
Contributor Author

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.

String MEMSTORE_ONHEAP_SIZE_GAUGE_DESC =
"Global MemStore On-heap size in bytes by the RegionServer";
String MEMSTORE_OFFHEAP_SIZE_GAUGE_NAME = "memStoreOffHeapSize";
String MEMSTORE_OFFHEAP_SIZE_GAUGE_DESC =
"Global MemStore Off-heap size in bytes by the RegionServer";

String BLOCKCACHE_SIZE_GAUGE_NAME = "blockCacheSize";
String BLOCKCACHE_SIZE_GAUGE_DESC = "BlockCache used in bytes by the RegionServer";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ public class MetricsHeapMemoryManagerSourceImpl extends BaseSourceImpl
private final MutableGaugeLong blockedFlushGauge;
private final MutableGaugeLong unblockedFlushGauge;
private final MutableGaugeLong memStoreSizeGauge;
private final MutableGaugeLong memStoreOnHeapSizeGauge;
private final MutableGaugeLong memStoreOffHeapSizeGauge;
private final MutableGaugeLong blockCacheSizeGauge;

private final MutableFastCounter doNothingCounter;
Expand Down Expand Up @@ -75,6 +77,10 @@ public MetricsHeapMemoryManagerSourceImpl(String metricsName, String metricsDesc
getMetricsRegistry().newGauge(UNBLOCKED_FLUSH_GAUGE_NAME, UNBLOCKED_FLUSH_GAUGE_DESC, 0L);
memStoreSizeGauge =
getMetricsRegistry().newGauge(MEMSTORE_SIZE_GAUGE_NAME, MEMSTORE_SIZE_GAUGE_DESC, 0L);
memStoreOnHeapSizeGauge = getMetricsRegistry().newGauge(MEMSTORE_ONHEAP_SIZE_GAUGE_NAME,
MEMSTORE_ONHEAP_SIZE_GAUGE_DESC, 0L);
memStoreOffHeapSizeGauge = getMetricsRegistry().newGauge(MEMSTORE_OFFHEAP_SIZE_GAUGE_NAME,
MEMSTORE_OFFHEAP_SIZE_GAUGE_DESC, 0L);
blockCacheSizeGauge =
getMetricsRegistry().newGauge(BLOCKCACHE_SIZE_GAUGE_NAME, BLOCKCACHE_SIZE_GAUGE_DESC, 0L);

Expand Down Expand Up @@ -111,6 +117,16 @@ public void setCurMemStoreSizeGauge(long memstoreSize) {
memStoreSizeGauge.set(memstoreSize);
}

@Override
public void setCurMemStoreOnHeapSizeGauge(long memstoreOnHeapSize) {
memStoreOnHeapSizeGauge.set(memstoreOnHeapSize);
}

@Override
public void setCurMemStoreOffHeapSizeGauge(long memstoreOffHeapSize) {
memStoreOffHeapSizeGauge.set(memstoreOffHeapSize);
}

@Override
public void updateMemStoreDeltaSizeHistogram(int memStoreDeltaSize) {
if (memStoreDeltaSize >= 0) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Contributor

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?

Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Contributor

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

Copy link
Contributor Author

@jinggou jinggou Jun 22, 2023

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

metricsHeapMemoryManager.setCurMemStoreSizeGauge(globalMemstoreDataSize);
metricsHeapMemoryManager.setCurMemStoreOnHeapSizeGauge(globalMemstoreHeapSize);
metricsHeapMemoryManager.setCurMemStoreOffHeapSizeGauge(globalMemStoreOffHeapSize);
tunerContext.setCurBlockCacheSize(blockCachePercent);
tunerContext.setCurMemStoreSize(globalMemStorePercent);
TunerResult result = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,22 @@ public void setCurMemStoreSizeGauge(final long memStoreSize) {
source.setCurMemStoreSizeGauge(memStoreSize);
}

/**
* Set the current global memstore on-heap size gauge
* @param memStoreOnHeapSize the current memory on-heap size in memstore, in bytes.
*/
public void setCurMemStoreOnHeapSizeGauge(final long memStoreOnHeapSize) {
source.setCurMemStoreOnHeapSizeGauge(memStoreOnHeapSize);
}

/**
* Set the current global memstore off-heap size gauge
* @param memStoreOffHeapSize the current memory off-heap size in memstore, in bytes.
*/
public void setCurMemStoreOffHeapSizeGauge(final long memStoreOffHeapSize) {
source.setCurMemStoreOffHeapSizeGauge(memStoreOffHeapSize);
}

/**
* Update the increase/decrease memstore size histogram
* @param memStoreDeltaSize the tuning result of memstore.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,12 @@ public long getGlobalMemStoreDataSize() {
return globalMemStoreDataSize.sum();
}

/** Returns the global memstore heap size in the RegionServer */
/** Returns the global memstore on-heap size in the RegionServer */
public long getGlobalMemStoreHeapSize() {
return this.globalMemStoreHeapSize.sum();
}

/** Returns the global memstore heap size in the RegionServer */
/** Returns the global memstore off-heap size in the RegionServer */
public long getGlobalMemStoreOffHeapSize() {
return this.globalMemStoreOffHeapSize.sum();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,15 @@ public void testGauge() {
hmm.updateBlockedFlushCount(200);
hmm.updateUnblockedFlushCount(50);
hmm.setCurMemStoreSizeGauge(256 * 1024 * 1024);
hmm.setCurMemStoreOnHeapSizeGauge(512 * 1024 * 1024);
hmm.setCurMemStoreOffHeapSizeGauge(128 * 1024 * 1024);
hmm.setCurBlockCacheSizeGauge(100 * 1024 * 1024);

HELPER.assertGauge("blockedFlushGauge", 200, source);
HELPER.assertGauge("unblockedFlushGauge", 50, source);
HELPER.assertGauge("memStoreSize", 256 * 1024 * 1024, source);
HELPER.assertGauge("memStoreOnHeapSize", 512 * 1024 * 1024, source);
HELPER.assertGauge("memStoreOffHeapSize", 128 * 1024 * 1024, source);
HELPER.assertGauge("blockCacheSize", 100 * 1024 * 1024, source);
}
}