Details

    • Type: Sub-task Sub-task
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: HA Branch (HDFS-1623)
    • Fix Version/s: HA Branch (HDFS-1623)
    • Component/s: ha
    • Labels:
      None

      Description

      Common side of HDFS-2679.

      1. hadoop-7925.txt
        5 kB
        Eli Collins
      2. hadoop-7925.txt
        5 kB
        Eli Collins
      3. hadoop-7925.txt
        5 kB
        Eli Collins

        Activity

        Hide
        Eli Collins added a comment -

        Patch attached.

        Show
        Eli Collins added a comment - Patch attached.
        Hide
        Todd Lipcon added a comment -

        Does the new test pass? It calls -getState but the new command is -getServiceState. If it does pass there's something wrong with our test harness

        Show
        Todd Lipcon added a comment - Does the new test pass? It calls -getState but the new command is -getServiceState. If it does pass there's something wrong with our test harness
        Hide
        Eli Collins added a comment -

        Doh! Updated patch.

        Show
        Eli Collins added a comment - Doh! Updated patch.
        Hide
        Uma Maheswara Rao G added a comment -

        Eli, Patch looks good to me.

        One small comment:
        Looks we are spreading the "active", "standby" string literals in common and hdfs. If some one changes toString value of ActiveState in HDFS and prints the value somewhere that may give the new value of state, at the same time we must change common also.
        My suggestion is to keep this literals in Enum itself and provide the toString of it. We can use this in both the places now. This is just suggestion, what do you say?
        Also we need not have the state comparision checks.

        enum HAServiceState {
              ACTIVE("active"),
             STANDBY("standby");
              
              private String state = null;
              
              HAServiceState(String state)
              {
                this.state = state;
              }
              public String toString()
              {
                return this.state;
              }
              
           }
        
        Show
        Uma Maheswara Rao G added a comment - Eli, Patch looks good to me. One small comment: Looks we are spreading the "active", "standby" string literals in common and hdfs. If some one changes toString value of ActiveState in HDFS and prints the value somewhere that may give the new value of state, at the same time we must change common also. My suggestion is to keep this literals in Enum itself and provide the toString of it. We can use this in both the places now. This is just suggestion, what do you say? Also we need not have the state comparision checks. enum HAServiceState { ACTIVE( "active" ), STANDBY( "standby" ); private String state = null ; HAServiceState( String state) { this .state = state; } public String toString() { return this .state; } }
        Hide
        Eli Collins added a comment -

        Uma, good suggestion. Updated patch attached.

        Show
        Eli Collins added a comment - Uma, good suggestion. Updated patch attached.
        Hide
        Uma Maheswara Rao G added a comment -

        +1, lgtm

        Show
        Uma Maheswara Rao G added a comment - +1, lgtm
        Hide
        Todd Lipcon added a comment -

        +1 from me too, committing momentarily

        Show
        Todd Lipcon added a comment - +1 from me too, committing momentarily
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-HAbranch-build #21 (See https://builds.apache.org/job/Hadoop-Hdfs-HAbranch-build/21/)
        HADOOP-7925. Add interface and update CLI to query current state to HAServiceProtocol. Contributed by Eli Collins.

        todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1220611
        Files :

        • /hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/CHANGES.HDFS-1623.txt
        • /hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HAAdmin.java
        • /hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HAServiceProtocol.java
        • /hadoop/common/branches/HDFS-1623/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHAAdmin.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-HAbranch-build #21 (See https://builds.apache.org/job/Hadoop-Hdfs-HAbranch-build/21/ ) HADOOP-7925 . Add interface and update CLI to query current state to HAServiceProtocol. Contributed by Eli Collins. todd : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1220611 Files : /hadoop/common/branches/ HDFS-1623 /hadoop-common-project/hadoop-common/CHANGES. HDFS-1623 .txt /hadoop/common/branches/ HDFS-1623 /hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HAAdmin.java /hadoop/common/branches/ HDFS-1623 /hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ha/HAServiceProtocol.java /hadoop/common/branches/ HDFS-1623 /hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ha/TestHAAdmin.java

          People

          • Assignee:
            Eli Collins
            Reporter:
            Eli Collins
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development