You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "Apache9 (via GitHub)" <gi...@apache.org> on 2023/06/16 14:23:39 UTC

[GitHub] [hbase] Apache9 commented on a diff in pull request #5293: HBASE-27892 Report memstore on-heap and off-heap size as jmx metrics

Apache9 commented on code in PR #5293:
URL: https://github.com/apache/hbase/pull/5293#discussion_r1232319452


##########
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsHeapMemoryManagerSource.java:
##########
@@ -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";

Review Comment:
   Use OnHeap or just Heap? I'm not an English expert, just asking...



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HeapMemoryManager.java:
##########
@@ -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);

Review Comment:
   So in the old time we just use heap size as memstore size? This should be bug?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org