[ 
https://issues.apache.org/jira/browse/HBASE-3909?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13486157#comment-13486157
 ] 

Ted Yu commented on HBASE-3909:
-------------------------------

The first few changes to conf/hbase-site.xml seem to be unrelated to this 
feature.
{code}
+            runtime cluster configuration changes with out the need for 
restarting the cluster.
{code}
'with out' -> 'without'
For HBaseInMemoryConfiguration:
{code}
+  public String get(String configKey) {
+    if (clusterConfigMap.containsKey(configKey)) {
+      return clusterConfigMap.get(configKey);
{code}
Since clusterConfigMap is ConcurrentHashMap, I think we should skip the call to 
containsKey(). If null is returned from get(), we delegate to super.get(). This 
applies to getLong(), etc.
For refreshClusterConfigMap():
{code}
+      LOG.debug("Refreshed cluster configuration map. Current size = "
+              + clusterConfigMap.size());
{code}
Consider recording the size of clusterConfigMap before refreshing and log both 
sizes.
{code}
Running org.apache.hadoop.hbase.TestHBaseDynamicConfiguration
Tests run: 7, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 116.502 sec
{code}
I think TestHBaseDynamicConfiguration should be marked as LargeTest.
{code}
+    updateClusterConfig(cct);
+
+    Thread.sleep(5000);
{code}
Worth considering finding the proper way of detecting completion of config 
update so that we don't need to wait for so long.
testDumpConfiguration() seems like a utility method instead of a test.
testConfigDuplicateAddition() can be named 
testConfigDuplicateAdditionWithSameKey().
testConfigUpdate() seems to be same as testConfigDuplicateAddition().

For JsonConfiguration and JsonProperty, they're defined in ClusterConfigTracker.
You can reuse them in TestHBaseDynamicConfiguration

Please upload patch v3 onto review board so that further review is easier.
                
> Add dynamic config
> ------------------
>
>                 Key: HBASE-3909
>                 URL: https://issues.apache.org/jira/browse/HBASE-3909
>             Project: HBase
>          Issue Type: Bug
>            Reporter: stack
>             Fix For: 0.96.0
>
>         Attachments: 3909_090712-2.patch, 3909-102812.patch, 
> 3909-102912.patch, 3909.v1, 3909-v1.patch, HBase Cluster Config Details.xlsx, 
> patch-v2.patch
>
>
> I'm sure this issue exists already, at least as part of the discussion around 
> making online schema edits possible, but no hard this having its own issue.  
> Ted started a conversation on this topic up on dev and Todd suggested we 
> lookd at how Hadoop did it over in HADOOP-7001

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to