HBase
  1. HBase
  2. HBASE-4451

Improve zk node naming (/hbase/shutdown)

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 0.94.0
    • Fix Version/s: 0.95.0
    • Component/s: master
    • Labels:
    • Hadoop Flags:
      Reviewed
    • Release Note:
      The 'shutdown' znode was renamed 'running'.
    • Tags:
      noob

      Description

      Right now the node /hbase/shutdown is used to indicate cluster status (cluster up, cluster down).

      However, upon a chat with Lars George today, we feel that having a name /hbase/shutdown is possibly bad. The /hbase/shutdown zknode contains a date when the cluster was started. Now that is difficult to understand and digest, given that a person may connect to zk and try to look at what it is about (they may think it 'shutdown' at that date.).

      I feel a better name may simply be: /hbase/running. Thoughts?

      1. 4451-1.patch
        1 kB
        Devaraj Das

        Activity

        Hide
        stack added a comment -

        Marking closed.

        Show
        stack added a comment - Marking closed.
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #351 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/351/)
        HBASE-4451 Improve zk node naming (/hbase/shutdown) (Revision 1434101)

        Result = FAILURE
        stack :
        Files :

        • /hbase/trunk/hbase-protocol/src/main/protobuf/ZooKeeper.proto
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK-on-Hadoop-2.0.0 #351 (See https://builds.apache.org/job/HBase-TRUNK-on-Hadoop-2.0.0/351/ ) HBASE-4451 Improve zk node naming (/hbase/shutdown) (Revision 1434101) Result = FAILURE stack : Files : /hbase/trunk/hbase-protocol/src/main/protobuf/ZooKeeper.proto /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #3759 (See https://builds.apache.org/job/HBase-TRUNK/3759/)
        HBASE-4451 Improve zk node naming (/hbase/shutdown) (Revision 1434101)

        Result = FAILURE
        stack :
        Files :

        • /hbase/trunk/hbase-protocol/src/main/protobuf/ZooKeeper.proto
        • /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #3759 (See https://builds.apache.org/job/HBase-TRUNK/3759/ ) HBASE-4451 Improve zk node naming (/hbase/shutdown) (Revision 1434101) Result = FAILURE stack : Files : /hbase/trunk/hbase-protocol/src/main/protobuf/ZooKeeper.proto /hbase/trunk/hbase-server/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
        Hide
        stack added a comment -

        Committed to trunk. It does not look like this issue responsible for the findbugs warning. Will look into that over in another issue.

        Show
        stack added a comment - Committed to trunk. It does not look like this issue responsible for the findbugs warning. Will look into that over in another issue.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12565061/4451-1.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        -1 tests included. The patch doesn't appear to include any new or modified tests.
        Please justify why no new tests are needed for this patch.
        Also please list what manual steps were performed to verify this patch.

        +1 hadoop2.0. The patch compiles against the hadoop 2.0 profile.

        +1 javadoc. The javadoc tool did not generate any warning messages.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        -1 findbugs. The patch appears to introduce 2 new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 lineLengths. The patch does not introduce lines longer than 100

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.util.TestHBaseFsck
        org.apache.hadoop.hbase.TestLocalHBaseCluster

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12565061/4451-1.patch against trunk revision . +1 @author . The patch does not contain any @author tags. -1 tests included . The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 hadoop2.0 . The patch compiles against the hadoop 2.0 profile. +1 javadoc . The javadoc tool did not generate any warning messages. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 findbugs . The patch appears to introduce 2 new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 lineLengths . The patch does not introduce lines longer than 100 -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.util.TestHBaseFsck org.apache.hadoop.hbase.TestLocalHBaseCluster Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop1-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/4039//console This message is automatically generated.
        Hide
        stack added a comment -

        +1 on patch. Made new issue to see if other znodes we should rename. Will wait on hadoopqa before committing.

        Show
        stack added a comment - +1 on patch. Made new issue to see if other znodes we should rename. Will wait on hadoopqa before committing.
        Hide
        Devaraj Das added a comment -

        Straightforward patch.

        Show
        Devaraj Das added a comment - Straightforward patch.
        Hide
        Harsh J added a comment -

        Sorry I left these assigned to me for long. I may pick it up again later, but unassigning for now to allow others.

        Show
        Harsh J added a comment - Sorry I left these assigned to me for long. I may pick it up again later, but unassigning for now to allow others.
        Hide
        stack added a comment -

        Marking as noob. If we are going to do this rename, 0.96 is the time to do it.

        Show
        stack added a comment - Marking as noob. If we are going to do this rename, 0.96 is the time to do it.
        Hide
        Lars Hofhansl added a comment -

        Moving out of 0.94.

        Show
        Lars Hofhansl added a comment - Moving out of 0.94.
        Hide
        Jonathan Gray added a comment -

        Would changing this have an effect on compatibility?

        If you wanted to support this change over a rolling restart or anything like that, it would probably be rather complicated or impractical. So it would require a full restart of the cluster most likely. In addition, any external ops/monitoring/admin tools people have built might be looking at the specific names. That shouldn't necessarily stop us though.

        Perhaps we can do this as part of a fresh look at the names of the ZK nodes in general. We might make some changes with the root node and such as well in 94. Do you want to look at all the ZK node names and see if there's a new scheme that would be more clear?

        Show
        Jonathan Gray added a comment - Would changing this have an effect on compatibility? If you wanted to support this change over a rolling restart or anything like that, it would probably be rather complicated or impractical. So it would require a full restart of the cluster most likely. In addition, any external ops/monitoring/admin tools people have built might be looking at the specific names. That shouldn't necessarily stop us though. Perhaps we can do this as part of a fresh look at the names of the ZK nodes in general. We might make some changes with the root node and such as well in 94. Do you want to look at all the ZK node names and see if there's a new scheme that would be more clear?
        Hide
        Harsh J added a comment -

        Andrew - Agreed. Am not looking to encourage this, btw.

        Just that, if its a node without which HBase shuts down, (loosely speaking) the condition shouldn't look like if !shutdown: shutdown(), but may look like if !running: shutdown() for better maintainability (code-wise).

        Would changing this have an effect on compatibility? I'd like to change it, but would hate to break any things you know would to possibly be monitoring this?

        Show
        Harsh J added a comment - Andrew - Agreed. Am not looking to encourage this, btw. Just that, if its a node without which HBase shuts down, (loosely speaking) the condition shouldn't look like if !shutdown: shutdown() , but may look like if !running: shutdown() for better maintainability (code-wise). Would changing this have an effect on compatibility? I'd like to change it, but would hate to break any things you know would to possibly be monitoring this?
        Hide
        stack added a comment -

        I'm +1 on changing name and for making whats under the zk node up in zk coherent (you should be discouraged tinkering... but I'd think looking is fine).

        Show
        stack added a comment - I'm +1 on changing name and for making whats under the zk node up in zk coherent (you should be discouraged tinkering... but I'd think looking is fine).
        Hide
        Andrew Purtell added a comment -

        [...] given that a person may connect to zk and try to look at what it is about [...]

        Shouldn't that be discouraged? ("There are no user serviceable parts inside.")

        Show
        Andrew Purtell added a comment - [...] given that a person may connect to zk and try to look at what it is about [...] Shouldn't that be discouraged? ("There are no user serviceable parts inside.")

          People

          • Assignee:
            Devaraj Das
            Reporter:
            Harsh J
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development