Details

    • Type: Sub-task
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 1.1.0
    • Fix Version/s: 1.1.0
    • Component/s: master
    • Labels:
      None
    • Release Note:
      Hide
      In 1.1 release, we implemented Procedure V2 to execute table DDL operations (create/delete/modify/truncate/enable/disable table; add/delete/modify column) to replace 1.0 release's handler implementation. By default, Procedure V2 feature is enabled in 1.1 release. We provide a config for customer to go back to 1.0 release implementation.
      The "hbase.master.procedure.tableddl" configuration accepts 2 values to change the behavior (other values treats as default - Procedure "enabled"):
      (1). "unused"
      (1a). uses handler implementation to execute new table DDLs;
      (1b).in case of unclean shutdown (crash), we could have unfinished DDLs in Procedure store, Procedure code will replay those operations and completes them.
      (2). "disabled" - (in case customer run into some problem and want to completely disable the Procedure V2 feature),
      (2a). this value would use handler implementation to execute new table DDLs;
      (2b). in case of unclean shutdown (crash), we could have unfinished DDLs in Procedure store, to prevent possible problem, the files in procedure store (WAL) will be deleted so that we would get into a clean state when the Procedure is enabled.
      Note:
      (A). This configuration is only checked during master start up (in constructor) - so you have to re-start master after changing the value
      (B). In case of crash and unclean shut down, HBCK is needed to clean up corruptions. "disable" case has more chance to lead to half-completed operation and hence customer should not be surprised when running HBCK is needed.
      Show
      In 1.1 release, we implemented Procedure V2 to execute table DDL operations (create/delete/modify/truncate/enable/disable table; add/delete/modify column) to replace 1.0 release's handler implementation. By default, Procedure V2 feature is enabled in 1.1 release. We provide a config for customer to go back to 1.0 release implementation. The "hbase.master.procedure.tableddl" configuration accepts 2 values to change the behavior (other values treats as default - Procedure "enabled"): (1). "unused" (1a). uses handler implementation to execute new table DDLs; (1b).in case of unclean shutdown (crash), we could have unfinished DDLs in Procedure store, Procedure code will replay those operations and completes them. (2). "disabled" - (in case customer run into some problem and want to completely disable the Procedure V2 feature), (2a). this value would use handler implementation to execute new table DDLs; (2b). in case of unclean shutdown (crash), we could have unfinished DDLs in Procedure store, to prevent possible problem, the files in procedure store (WAL) will be deleted so that we would get into a clean state when the Procedure is enabled. Note: (A). This configuration is only checked during master start up (in constructor) - so you have to re-start master after changing the value (B). In case of crash and unclean shut down, HBCK is needed to clean up corruptions. "disable" case has more chance to lead to half-completed operation and hence customer should not be surprised when running HBCK is needed.

      Description

      In branch-1, I think we want proc v2 to be configurable, so that if any non-recoverable issue is found, at least there is a workaround. We already have the handlers and code laying around. It will be just introducing the config to enable / disable. We can even make it dynamically configurable via the new framework.

        Activity

        Hide
        ndimiduk Nick Dimiduk added a comment -

        Closing issues released in 1.1.0.

        Show
        ndimiduk Nick Dimiduk added a comment - Closing issues released in 1.1.0.
        Hide
        hudson Hudson added a comment -

        SUCCESS: Integrated in HBase-1.1.0RC0-JDK7 #2 (See https://builds.apache.org/job/HBase-1.1.0RC0-JDK7/2/)
        HBASE-13469 Procedure v2 - Make procedure v2 configurable in branch-1.1 (Stephen Yuan Jiang) (matteo.bertozzi: rev 3eb8021e38b9e92976f97e2d0c9577d3cff380f3)

        • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableNamespaceManager.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
        • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestProcedureConf.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
        Show
        hudson Hudson added a comment - SUCCESS: Integrated in HBase-1.1.0RC0-JDK7 #2 (See https://builds.apache.org/job/HBase-1.1.0RC0-JDK7/2/ ) HBASE-13469 Procedure v2 - Make procedure v2 configurable in branch-1.1 (Stephen Yuan Jiang) (matteo.bertozzi: rev 3eb8021e38b9e92976f97e2d0c9577d3cff380f3) hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableNamespaceManager.java hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestProcedureConf.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in HBase-1.1 #422 (See https://builds.apache.org/job/HBase-1.1/422/)
        HBASE-13469 Procedure v2 - Make procedure v2 configurable in branch-1.1 (Stephen Yuan Jiang) (matteo.bertozzi: rev 3eb8021e38b9e92976f97e2d0c9577d3cff380f3)

        • hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
        • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableNamespaceManager.java
        • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestProcedureConf.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in HBase-1.1 #422 (See https://builds.apache.org/job/HBase-1.1/422/ ) HBASE-13469 Procedure v2 - Make procedure v2 configurable in branch-1.1 (Stephen Yuan Jiang) (matteo.bertozzi: rev 3eb8021e38b9e92976f97e2d0c9577d3cff380f3) hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableNamespaceManager.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestProcedureConf.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java
        Hide
        hudson Hudson added a comment -

        FAILURE: Integrated in HBase-1.1.0RC0-JDK8 #2 (See https://builds.apache.org/job/HBase-1.1.0RC0-JDK8/2/)
        HBASE-13469 Procedure v2 - Make procedure v2 configurable in branch-1.1 (Stephen Yuan Jiang) (matteo.bertozzi: rev 3eb8021e38b9e92976f97e2d0c9577d3cff380f3)

        • hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java
        • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableNamespaceManager.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java
        • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestProcedureConf.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
        • hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
        Show
        hudson Hudson added a comment - FAILURE: Integrated in HBase-1.1.0RC0-JDK8 #2 (See https://builds.apache.org/job/HBase-1.1.0RC0-JDK8/2/ ) HBASE-13469 Procedure v2 - Make procedure v2 configurable in branch-1.1 (Stephen Yuan Jiang) (matteo.bertozzi: rev 3eb8021e38b9e92976f97e2d0c9577d3cff380f3) hbase-server/src/main/java/org/apache/hadoop/hbase/quotas/MasterQuotaManager.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/TableNamespaceManager.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterServices.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestProcedureConf.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
        Hide
        hadoopqa Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12727052/HBASE-13469.v2-branch-1.1.patch
        against branch-1.1 branch at commit 23960207527b685609cd01259efbeb98f077b209.
        ATTACHMENT ID: 12727052

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

        +1 tests included. The patch appears to include 5 new or modified tests.

        +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0)

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

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

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

        +1 checkstyle. The applied patch does not increase the total number of checkstyle errors

        +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) 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 site. The mvn site goal succeeds with this patch.

        +1 core tests. The patch passed unit tests in .

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13774//testReport/
        Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13774//artifact/patchprocess/newFindbugsWarnings.html
        Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13774//artifact/patchprocess/checkstyle-aggregate.html

        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13774//console

        This message is automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12727052/HBASE-13469.v2-branch-1.1.patch against branch-1.1 branch at commit 23960207527b685609cd01259efbeb98f077b209. ATTACHMENT ID: 12727052 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 5 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 protoc . The applied patch does not increase the total number of protoc compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 checkstyle . The applied patch does not increase the total number of checkstyle errors +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) 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 site . The mvn site goal succeeds with this patch. +1 core tests . The patch passed unit tests in . Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13774//testReport/ Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13774//artifact/patchprocess/newFindbugsWarnings.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13774//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13774//console This message is automatically generated.
        Hide
        syuanjiang Stephen Yuan Jiang added a comment -

        oops, the patch (v1) is slightly out-of-date. Attached the final patch (v2).

        Here is description of the patch:
        "
        Add a config to disable Procedure V2 in HBASE-1.1 release.

        In 1.1 release, we implemented Procedure V2 to execute table DDL operations (create/delete/modify/truncate/enable/disable table; add/delete/modify column) to replace 1.0 release's handler implementation. By default, Procedure V2 feature is enabled in 1.1 release. We provide a config for customer to go back to 1.0 release implementation.

        The "hbase.master.procedure.tableddl" configuration accepts 2 values to change the behavior (other values treats as default - Procedure "enabled"):
        (1). "unused"
        (1a). uses handler implementation to execute new table DDLs;
        (1b).in case of unclean shutdown (crash), we could have unfinished DDLs in Procedure store, Procedure code will replay those operations and completes them.

        (2). "disabled" - (in case customer run into some problem and want to completely disable the Procedure V2 feature),
        (2a). this value would use handler implementation to execute new table DDLs;
        (2b). in case of unclean shutdown (crash), we could have unfinished DDLs in Procedure store, to prevent possible problem, the files in procedure store (WAL) will be deleted so that we would get into a clean state when the Procedure is enabled.

        Note:
        (A). This configuration is only checked during master start up (in constructor) - so you have to re-start master after changing the value
        (B). In case of crash and unclean shut down, HBCK is needed to clean up corruptions. "disable" case has more chance to lead to half-completed operation and hence customer should not be surprised when running HBCK is needed.
        "

        Show
        syuanjiang Stephen Yuan Jiang added a comment - oops, the patch (v1) is slightly out-of-date. Attached the final patch (v2). Here is description of the patch: " Add a config to disable Procedure V2 in HBASE-1 .1 release. In 1.1 release, we implemented Procedure V2 to execute table DDL operations (create/delete/modify/truncate/enable/disable table; add/delete/modify column) to replace 1.0 release's handler implementation. By default, Procedure V2 feature is enabled in 1.1 release. We provide a config for customer to go back to 1.0 release implementation. The "hbase.master.procedure.tableddl" configuration accepts 2 values to change the behavior (other values treats as default - Procedure "enabled"): (1). "unused" (1a). uses handler implementation to execute new table DDLs; (1b).in case of unclean shutdown (crash), we could have unfinished DDLs in Procedure store, Procedure code will replay those operations and completes them. (2). "disabled" - (in case customer run into some problem and want to completely disable the Procedure V2 feature), (2a). this value would use handler implementation to execute new table DDLs; (2b). in case of unclean shutdown (crash), we could have unfinished DDLs in Procedure store, to prevent possible problem, the files in procedure store (WAL) will be deleted so that we would get into a clean state when the Procedure is enabled. Note: (A). This configuration is only checked during master start up (in constructor) - so you have to re-start master after changing the value (B). In case of crash and unclean shut down, HBCK is needed to clean up corruptions. "disable" case has more chance to lead to half-completed operation and hence customer should not be surprised when running HBCK is needed. "
        Hide
        hadoopqa Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12726784/HBASE-13469.v1-branch-1.1.patch
        against branch-1.1 branch at commit 91ab1086d62e0a2de5ca8fc65e98ebcbafc82ba7.
        ATTACHMENT ID: 12726784

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

        +1 tests included. The patch appears to include 5 new or modified tests.

        +1 hadoop versions. The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0)

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

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

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

        +1 checkstyle. The applied patch does not increase the total number of checkstyle errors

        +1 findbugs. The patch does not introduce any new Findbugs (version 2.0.3) 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 site. The mvn site goal succeeds with this patch.

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.master.TestProcedureConf

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13761//testReport/
        Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13761//artifact/patchprocess/newFindbugsWarnings.html
        Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13761//artifact/patchprocess/checkstyle-aggregate.html

        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13761//console

        This message is automatically generated.

        Show
        hadoopqa Hadoop QA added a comment - -1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12726784/HBASE-13469.v1-branch-1.1.patch against branch-1.1 branch at commit 91ab1086d62e0a2de5ca8fc65e98ebcbafc82ba7. ATTACHMENT ID: 12726784 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 5 new or modified tests. +1 hadoop versions . The patch compiles with all supported hadoop versions (2.4.1 2.5.2 2.6.0) +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 protoc . The applied patch does not increase the total number of protoc compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 checkstyle . The applied patch does not increase the total number of checkstyle errors +1 findbugs . The patch does not introduce any new Findbugs (version 2.0.3) 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 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: org.apache.hadoop.hbase.master.TestProcedureConf Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/13761//testReport/ Release Findbugs (version 2.0.3) warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/13761//artifact/patchprocess/newFindbugsWarnings.html Checkstyle Errors: https://builds.apache.org/job/PreCommit-HBASE-Build/13761//artifact/patchprocess/checkstyle-aggregate.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/13761//console This message is automatically generated.
        Hide
        stack stack added a comment -

        Stephen Yuan Jiang

        I think we should spend our energy to clean up handler code in 1.2 and make procedure robust.

        Ok. Sounds reasonable. Took a look at the last patch and not much code and it has a test (only nit comment is why not have the enum name same as the configuration value that turns on the state: i.e. name enums unused, disable, enabled... then you could compare the configuration and the enum toString'd... No biggie)

        Show
        stack stack added a comment - Stephen Yuan Jiang I think we should spend our energy to clean up handler code in 1.2 and make procedure robust. Ok. Sounds reasonable. Took a look at the last patch and not much code and it has a test (only nit comment is why not have the enum name same as the configuration value that turns on the state: i.e. name enums unused, disable, enabled... then you could compare the configuration and the enum toString'd... No biggie)
        Hide
        syuanjiang Stephen Yuan Jiang added a comment -
        Show
        syuanjiang Stephen Yuan Jiang added a comment - Review is posted in https://reviews.apache.org/r/32566/
        Hide
        syuanjiang Stephen Yuan Jiang added a comment -

        stack I discussed with Enis about cleanup handler code - it requires a lot of changes (though not complicated) to make both handler and procedure call the same method. This is only for 1.1 release. I think we should spend our energy to clean up handler code in 1.2 and make procedure robust.

        The reason that we need it for 1.1 is due to the PV2 feature is too new and we are really close to 1.1 release (< 4 weeks based on current schedule); we need to reduce risk for 1.1 release. For 1.2+ - we have more time to test/fix issues, we should be ok to remove the conf and handler code (I changed the title of this Jira only for 1.1).

        Show
        syuanjiang Stephen Yuan Jiang added a comment - stack I discussed with Enis about cleanup handler code - it requires a lot of changes (though not complicated) to make both handler and procedure call the same method. This is only for 1.1 release. I think we should spend our energy to clean up handler code in 1.2 and make procedure robust. The reason that we need it for 1.1 is due to the PV2 feature is too new and we are really close to 1.1 release (< 4 weeks based on current schedule); we need to reduce risk for 1.1 release. For 1.2+ - we have more time to test/fix issues, we should be ok to remove the conf and handler code (I changed the title of this Jira only for 1.1).
        Hide
        stack stack added a comment -

        Enis suggested that we can cleanup the handlers by calling the new Procedure methods, so we avoid the duplicated code and having to do changes in both places.

        I like this.

        Show
        stack stack added a comment - Enis suggested that we can cleanup the handlers by calling the new Procedure methods, so we avoid the duplicated code and having to do changes in both places. I like this.
        Hide
        mbertozzi Matteo Bertozzi added a comment -

        1 on what Enis said. We can remove the handlers in 1.2
        also Enis suggested that we can cleanup the handlers by calling the new Procedure methods, so we avoid the duplicated code and having to do changes in both places.

        Show
        mbertozzi Matteo Bertozzi added a comment - 1 on what Enis said. We can remove the handlers in 1.2 also Enis suggested that we can cleanup the handlers by calling the new Procedure methods, so we avoid the duplicated code and having to do changes in both places.
        Hide
        enis Enis Soztutar added a comment -

        I'd say -0. Cut over. Just kill the old stuff. Test. No safety net!

        Bold! What about keeping it configurable in 1.1, and removing the conf in 1.2+?

        Show
        enis Enis Soztutar added a comment - I'd say -0. Cut over. Just kill the old stuff. Test. No safety net! Bold! What about keeping it configurable in 1.1, and removing the conf in 1.2+?
        Hide
        stack stack added a comment -

        I'd say -0. Cut over. Just kill the old stuff. Test. No safety net!

        Show
        stack stack added a comment - I'd say -0. Cut over. Just kill the old stuff. Test. No safety net!

          People

          • Assignee:
            syuanjiang Stephen Yuan Jiang
            Reporter:
            enis Enis Soztutar
          • Votes:
            0 Vote for this issue
            Watchers:
            7 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development