Uploaded image for project: 'HBase'
  1. HBase
  2. HBASE-19017

[AMv2] EnableTableProcedure is not retaining the assignments

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.0-alpha-3
    • Fix Version/s: 2.0.0-beta-1
    • Component/s: Region Assignment
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      Found this while working on HBASE-18946. In branch-1.4 when ever we do enable table we try retain assignment.
      But in branch-2 and trunk the EnableTableProcedure tries to get the location from the existing regionNode. It always returns null because while doing region CLOSE while disabling a table, the regionNode's 'regionLocation' is made NULL but the 'lastHost' is actually having the servername where the region was hosted. But on trying assignment again we try to see what was the last RegionLocation and not the 'lastHost' and we go ahead with new assignment.
      On region CLOSE while disable table

      public void markRegionAsClosed(final RegionStateNode regionNode) throws IOException {
          final RegionInfo hri = regionNode.getRegionInfo();
          synchronized (regionNode) {
            State state = regionNode.transitionState(State.CLOSED, RegionStates.STATES_EXPECTED_ON_CLOSE);
            regionStates.removeRegionFromServer(regionNode.getRegionLocation(), regionNode);
            regionNode.setLastHost(regionNode.getRegionLocation());
            regionNode.setRegionLocation(null);
            regionStateStore.updateRegionLocation(regionNode.getRegionInfo(), state,
              regionNode.getRegionLocation()/*null*/, regionNode.getLastHost(),
              HConstants.NO_SEQNUM, regionNode.getProcedure().getProcId());
            sendRegionClosedNotification(hri);
          }
      

      In AssignProcedure

          ServerName lastRegionLocation = regionNode.offline();
      
      public ServerName setRegionLocation(final ServerName serverName) {
            ServerName lastRegionLocation = this.regionLocation;
            if (LOG.isTraceEnabled() && serverName == null) {
              LOG.trace("Tracking when we are set to null " + this, new Throwable("TRACE"));
            }
            this.regionLocation = serverName;
            this.lastUpdate = EnvironmentEdgeManager.currentTime();
            return lastRegionLocation;
          }
      

      So further code in AssignProcedure

       boolean retain = false;
          if (!forceNewPlan) {
            if (this.targetServer != null) {
              retain = targetServer.equals(lastRegionLocation);
              regionNode.setRegionLocation(targetServer);
            } else {
              if (lastRegionLocation != null) {
                // Try and keep the location we had before we offlined.
                retain = true;
                regionNode.setRegionLocation(lastRegionLocation);
              }
            }
          }
      

      Tries to do retainAssignment but fails because lastRegionLocation is always null.

      1. HBASE-19017.patch
        9 kB
        ramkrishna.s.vasudevan

        Activity

        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build HBase-Trunk_matrix #3903 (See https://builds.apache.org/job/HBase-Trunk_matrix/3903/)
        HBASE-19017 [AMv2] EnableTableProcedure is not retaining the assignments (ramkrishna: rev 9a27ac8781ea09ad0890c3f0a6e6676a785818cf)

        • (add) hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasAreDistributed.java
        • (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build HBase-Trunk_matrix #3903 (See https://builds.apache.org/job/HBase-Trunk_matrix/3903/ ) HBASE-19017 [AMv2] EnableTableProcedure is not retaining the assignments (ramkrishna: rev 9a27ac8781ea09ad0890c3f0a6e6676a785818cf) (add) hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasAreDistributed.java (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in Jenkins build HBase-2.0 #702 (See https://builds.apache.org/job/HBase-2.0/702/)
        HBASE-19017 [AMv2] EnableTableProcedure is not retaining the assignments (ramkrishna: rev 8b8f7a017b05f055a926546e60eab1a238d567cc)

        • (add) hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasAreDistributed.java
        • (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build HBase-2.0 #702 (See https://builds.apache.org/job/HBase-2.0/702/ ) HBASE-19017 [AMv2] EnableTableProcedure is not retaining the assignments (ramkrishna: rev 8b8f7a017b05f055a926546e60eab1a238d567cc) (add) hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionReplicasAreDistributed.java (edit) hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/AssignProcedure.java
        Hide
        ram_krish ramkrishna.s.vasudevan added a comment -

        Pushed to master and branch-2.
        Thanks to all for the reviews.

        Show
        ram_krish ramkrishna.s.vasudevan added a comment - Pushed to master and branch-2. Thanks to all for the reviews.
        Hide
        ram_krish ramkrishna.s.vasudevan added a comment -

        Yi Liang
        Thanks for the comment and review. So will commit this now. The failed test case is unrelated.

        Show
        ram_krish ramkrishna.s.vasudevan added a comment - Yi Liang Thanks for the comment and review. So will commit this now. The failed test case is unrelated.
        Hide
        huaxiang huaxiang sun added a comment -

        +1

        Show
        huaxiang huaxiang sun added a comment - +1
        Hide
        easyliangjob Yi Liang added a comment -

        reviewed your patch, the fix is correct.
        For HBASE-18984, I also add some clean up in the AssignProcedure. You can commit this one first, and I will rebase the patch there.

        And the problem I found seems not related to retain assignment, and try to reproduce and maybe open a new jira for it.

        Show
        easyliangjob Yi Liang added a comment - reviewed your patch, the fix is correct. For HBASE-18984 , I also add some clean up in the AssignProcedure. You can commit this one first, and I will rebase the patch there. And the problem I found seems not related to retain assignment, and try to reproduce and maybe open a new jira for it.
        Hide
        hadoopqa Hadoop QA added a comment -
        -1 overall



        Vote Subsystem Runtime Comment
        0 reexec 8m 51s Docker mode activated.
        +1 hbaseanti 0m 0s Patch does not have any anti-patterns.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
        +1 mvninstall 6m 25s master passed
        +1 compile 1m 6s master passed
        +1 checkstyle 1m 6s master passed
        +1 mvneclipse 0m 29s master passed
        +1 shadedjars 7m 0s branch has no errors when building our shaded downstream artifacts.
        +1 findbugs 3m 25s master passed
        +1 javadoc 0m 42s master passed
        +1 mvninstall 1m 8s the patch passed
        +1 compile 1m 7s the patch passed
        +1 javac 1m 7s the patch passed
        +1 checkstyle 1m 7s the patch passed
        +1 mvneclipse 0m 28s the patch passed
        +1 whitespace 0m 0s The patch has no whitespace issues.
        +1 shadedjars 5m 41s patch has no errors when building our shaded downstream artifacts.
        +1 hadoopcheck 58m 9s Patch does not cause any errors with Hadoop 2.6.1 2.6.2 2.6.3 2.6.4 2.6.5 2.7.1 2.7.2 2.7.3 or 3.0.0-alpha4.
        +1 findbugs 4m 1s the patch passed
        +1 javadoc 0m 45s the patch passed
        -1 unit 149m 39s hbase-server in the patch failed.
        +1 asflicense 0m 38s The patch does not generate ASF License warnings.
        245m 14s



        Reason Tests
        Failed junit tests hadoop.hbase.security.token.TestZKSecretWatcher



        Subsystem Report/Notes
        Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:5d60123
        JIRA Issue HBASE-19017
        JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12892430/HBASE-19017.patch
        Optional Tests asflicense shadedjars javac javadoc unit findbugs hadoopcheck hbaseanti checkstyle compile
        uname Linux 16c9c55bd1a0 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux
        Build tool maven
        Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build@2/component/dev-support/hbase-personality.sh
        git revision master / 51489b20
        Default Java 1.8.0_144
        findbugs v3.1.0-RC3
        unit https://builds.apache.org/job/PreCommit-HBASE-Build/9145/artifact/patchprocess/patch-unit-hbase-server.txt
        Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/9145/testReport/
        modules C: hbase-server U: hbase-server
        Console output https://builds.apache.org/job/PreCommit-HBASE-Build/9145/console
        Powered by Apache Yetus 0.4.0 http://yetus.apache.org

        This message was automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 8m 51s Docker mode activated. +1 hbaseanti 0m 0s Patch does not have any anti-patterns. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 25s master passed +1 compile 1m 6s master passed +1 checkstyle 1m 6s master passed +1 mvneclipse 0m 29s master passed +1 shadedjars 7m 0s branch has no errors when building our shaded downstream artifacts. +1 findbugs 3m 25s master passed +1 javadoc 0m 42s master passed +1 mvninstall 1m 8s the patch passed +1 compile 1m 7s the patch passed +1 javac 1m 7s the patch passed +1 checkstyle 1m 7s the patch passed +1 mvneclipse 0m 28s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 shadedjars 5m 41s patch has no errors when building our shaded downstream artifacts. +1 hadoopcheck 58m 9s Patch does not cause any errors with Hadoop 2.6.1 2.6.2 2.6.3 2.6.4 2.6.5 2.7.1 2.7.2 2.7.3 or 3.0.0-alpha4. +1 findbugs 4m 1s the patch passed +1 javadoc 0m 45s the patch passed -1 unit 149m 39s hbase-server in the patch failed. +1 asflicense 0m 38s The patch does not generate ASF License warnings. 245m 14s Reason Tests Failed junit tests hadoop.hbase.security.token.TestZKSecretWatcher Subsystem Report/Notes Docker Client=17.05.0-ce Server=17.05.0-ce Image:yetus/hbase:5d60123 JIRA Issue HBASE-19017 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12892430/HBASE-19017.patch Optional Tests asflicense shadedjars javac javadoc unit findbugs hadoopcheck hbaseanti checkstyle compile uname Linux 16c9c55bd1a0 3.13.0-119-generic #166-Ubuntu SMP Wed May 3 12:18:55 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /home/jenkins/jenkins-slave/workspace/PreCommit-HBASE-Build@2/component/dev-support/hbase-personality.sh git revision master / 51489b20 Default Java 1.8.0_144 findbugs v3.1.0-RC3 unit https://builds.apache.org/job/PreCommit-HBASE-Build/9145/artifact/patchprocess/patch-unit-hbase-server.txt Test Results https://builds.apache.org/job/PreCommit-HBASE-Build/9145/testReport/ modules C: hbase-server U: hbase-server Console output https://builds.apache.org/job/PreCommit-HBASE-Build/9145/console Powered by Apache Yetus 0.4.0 http://yetus.apache.org This message was automatically generated.
        Hide
        ram_krish ramkrishna.s.vasudevan added a comment -

        Ted Yu
        Yes I have already added that null check. Anyway let me what other issues are there as per Yi Liang.

        Show
        ram_krish ramkrishna.s.vasudevan added a comment - Ted Yu Yes I have already added that null check. Anyway let me what other issues are there as per Yi Liang .
        Hide
        ram_krish ramkrishna.s.vasudevan added a comment -

        Simple patch. Seems Yi Liang also has the same fix but finds some problems. Will check it too.

        Show
        ram_krish ramkrishna.s.vasudevan added a comment - Simple patch. Seems Yi Liang also has the same fix but finds some problems. Will check it too.
        Hide
        yuzhihong@gmail.com Ted Yu added a comment -
                  retain = true;
                  regionNode.setRegionLocation(regionNode.getLastHost());
        

        Shouldn't the value of retain be dependent on whether regionNode.getLastHost() is not null ?

        Show
        yuzhihong@gmail.com Ted Yu added a comment - retain = true ; regionNode.setRegionLocation(regionNode.getLastHost()); Shouldn't the value of retain be dependent on whether regionNode.getLastHost() is not null ?
        Hide
        ram_krish ramkrishna.s.vasudevan added a comment -
            if (!forceNewPlan) {
              if (this.targetServer != null) {
                retain = targetServer.equals(lastRegionLocation);
                regionNode.setRegionLocation(targetServer);
              } else {
                if (lastRegionLocation != null) {
                  // Try and keep the location we had before we offlined.
                  retain = true;
                  regionNode.setRegionLocation(lastRegionLocation);
                } else {
                  retain = true;
                  regionNode.setRegionLocation(regionNode.getLastHost());
                }
              }
            }
        

        this code will solve the problem. I have reproduced with a test case also. Will upload patch shortly.

        Show
        ram_krish ramkrishna.s.vasudevan added a comment - if (!forceNewPlan) { if ( this .targetServer != null ) { retain = targetServer.equals(lastRegionLocation); regionNode.setRegionLocation(targetServer); } else { if (lastRegionLocation != null ) { // Try and keep the location we had before we offlined. retain = true ; regionNode.setRegionLocation(lastRegionLocation); } else { retain = true ; regionNode.setRegionLocation(regionNode.getLastHost()); } } } this code will solve the problem. I have reproduced with a test case also. Will upload patch shortly.
        Hide
        ram_krish ramkrishna.s.vasudevan added a comment -
        Show
        ram_krish ramkrishna.s.vasudevan added a comment - huaxiang sun , Stack FYI.

          People

          • Assignee:
            ram_krish ramkrishna.s.vasudevan
            Reporter:
            ram_krish ramkrishna.s.vasudevan
          • Votes:
            0 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development