hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Zhihong Yu (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-3909) Add dynamic config
Date Thu, 05 Apr 2012 00:01:41 GMT

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

Zhihong Yu commented on HBASE-3909:
-----------------------------------

For new files, please add license at the top of them.

Please add class javadoc for HBaseInMemoryConfiguration. Nicolas is introducing CompoundConfiguration
in HBASE-5335. You may want to take a look.
{code}
+    public long getLong(String name, long defaultValue) {
+        if (clusterConfigMap.containsKey(name)) {
+            String longValue = clusterConfigMap.get(name);
+            return longValue == null ? defaultValue : Long.parseLong(longValue);
{code}
What if Long.parseLong() throws NumberFormatException ? Should you catch NFE and delegate
to super.getLong() ?
Similar comment for getInt(), getFloat() and getBoolean().

For the new method in HMasterInterface:
{code}
+  public boolean updateConfig(String configKey, String configValue) {
{code}
Should List<Pair<String, String>> be the parameter type so that the number of
calls to master can be lowered ?
Similar comment for getConfig().
{code}
+    this.zooKeeper = new ZooKeeperWatcher(conf, MASTER + ":" + "isa.port", this, true);
{code}
What does "isa.port" represent above ?
getClusterConfiguration() is not a good name: clusterConfigTracker is started in the method.

If "hbase.dynamic.configuration.enabled" has value of false, do we need to start the tracker
?
Similar comment for starting the tracker in initializeZKBasedSystemTrackers().
{code}
-    this.zooKeeper.reconnectAfterExpiration();
-
+    this.zooKeeper = new ZooKeeperWatcher(conf, MASTER + ":"
+      + this.serverName.getPort(), this, true);
+    // Set cluster config tracker to null to trigger a reload.
+    this.clusterConfigTracker = null;
{code}
Is the change above to how zooKeeper is recovered intentional ?
tryRecoveringExpiredZKSession() is called by abortNow(). Can you explain what the comment
above setting this.clusterConfigTracker to null means ?
                
> 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-v1.patch, 3909.v1
>
>
> 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: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message