Apache9 commented on a change in pull request #3664:
URL: https://github.com/apache/hbase/pull/3664#discussion_r704419809



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java
##########
@@ -2529,6 +2529,8 @@ protected OffPeakHours getOffPeakHours() {
   public void onConfigurationChange(Configuration conf) {
     this.conf = new CompoundConfiguration()
             .add(conf)
+            .addBytesMap(region.getTableDescriptor().getValues())

Review comment:
       Better abstract a util method to do the merging of the configurations 
and call it from both the constructor and here?

##########
File path: 
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerOnlineConfigChange.java
##########
@@ -259,4 +263,12 @@ public void removeClosedRegionFromConfigurationManager() 
throws Exception {
       });
     }
   }
+
+  @Test
+  public void testStoreConfigurationOnlineChange() {
+    rs1.getConfigurationManager().notifyAllObservers(conf);
+    long actualMaxFileSize =
+        
r1.getStore(COLUMN_FAMILY1).conf.getLong(TableDescriptorBuilder.MAX_FILESIZE, 
-1);

Review comment:
       Strange that we do not provide getConfiguration method for Region and 
Store interface...
   
   Let's open another issue to address it.




-- 
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: [email protected]

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


Reply via email to