saintstack commented on a change in pull request #1500:
URL: https://github.com/apache/hbase/pull/1500#discussion_r416144243



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -877,6 +877,19 @@ public HRegion(final HRegionFileSystem fs, final WAL wal, 
final Configuration co
     this.maxCellSize = conf.getLong(HBASE_MAX_CELL_SIZE_KEY, 
DEFAULT_MAX_CELL_SIZE);
     this.miniBatchSize = conf.getInt(HBASE_REGIONSERVER_MINIBATCH_SIZE,
         DEFAULT_HBASE_REGIONSERVER_MINIBATCH_SIZE);
+
+    // recover the metrics of read and write requests count if they were 
retained
+    if (rsServices != null) {

Review comment:
       If the Region went to another server and was over there for a week 
before coming to this server, then these metrics would be incorrect? Thats ok?

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java
##########
@@ -8964,4 +8977,12 @@ static void decorateRegionConfiguration(Configuration 
conf) {
       }
     }
   }
+
+  public void setReadRequestsCount(long readRequestsCount) {

Review comment:
       Has to be public? Can't be private or package private?

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerAccounting.java
##########
@@ -67,6 +70,7 @@ public RegionServerAccounting(Configuration conf) {
     this.globalOnHeapMemstoreLimit = 
MemorySizeUtil.getOnheapGlobalMemStoreSize(conf);
     this.globalOnHeapMemstoreLimitLowMark =
         (long) (this.globalOnHeapMemstoreLimit * 
this.globalMemStoreLimitLowMarkPercent);
+    this.retainedRegionRWRequestsCnt = new ConcurrentHashMap<>();

Review comment:
       This data structure can grow w/o bound?
   
   This data structure needs a comment on how it is used and life cycle of 
items. They items should age out I'd think.

##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerAccounting.java
##########
@@ -123,6 +127,13 @@ public long getGlobalMemStoreOffHeapSize() {
     return this.globalMemStoreOffHeapSize.sum();
   }
 
+  /**
+   * @return the retained metrics of region's read and write requests count
+   */
+  public ConcurrentMap<String, Pair<Long, Long>> 
getRetainedRegionRWRequestsCnt() {

Review comment:
       Has to be public? Can't be package private?




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

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to