HBase
  1. HBase
  2. HBASE-5454

Refuse operations from Admin before master is initialized

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.94.0
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      In our testing environment,
      When master is initializing, we found conflict problems between master#assignAllUserRegions and EnableTable event, causing assigning region throw exception so that master abort itself.

      We think we'd better refuse operations from Admin, such as CreateTable, EnableTable,etc, It could reduce error.

      1. hbase-5454v2.patch
        5 kB
        chunhui shen
      2. hbase-5454.patch
        7 kB
        chunhui shen

        Activity

        Hide
        xufeng added a comment -

        @chunhui
        Does it need to be added in HMaster#createTable?

        Show
        xufeng added a comment - @chunhui Does it need to be added in HMaster#createTable?
        Hide
        Hudson added a comment -

        Integrated in HBase-TRUNK #2671 (See https://builds.apache.org/job/HBase-TRUNK/2671/)
        HBASE-5454 Refuse operations from Admin before master is initialized (Revision 1295433)

        Result = FAILURE
        stack :
        Files :

        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/PleaseHoldException.java
        • /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
        Show
        Hudson added a comment - Integrated in HBase-TRUNK #2671 (See https://builds.apache.org/job/HBase-TRUNK/2671/ ) HBASE-5454 Refuse operations from Admin before master is initialized (Revision 1295433) Result = FAILURE stack : Files : /hbase/trunk/src/main/java/org/apache/hadoop/hbase/PleaseHoldException.java /hbase/trunk/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
        Hide
        Hudson added a comment -

        Integrated in HBase-0.94 #6 (See https://builds.apache.org/job/HBase-0.94/6/)
        HBASE-5454 Refuse operations from Admin before master is initialized (Revision 1295693)

        Result = SUCCESS
        stack :
        Files :

        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/PleaseHoldException.java
        • /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
        Show
        Hudson added a comment - Integrated in HBase-0.94 #6 (See https://builds.apache.org/job/HBase-0.94/6/ ) HBASE-5454 Refuse operations from Admin before master is initialized (Revision 1295693) Result = SUCCESS stack : Files : /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/PleaseHoldException.java /hbase/branches/0.94/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
        Hide
        stack added a comment -

        Committed to 0.94.0 too.

        The patch is incomplete in that it disables the public table modification apis only during master initialization. Other stuff like zk callbacks can still run. I think though that even though incomplete, its helps.

        Show
        stack added a comment - Committed to 0.94.0 too. The patch is incomplete in that it disables the public table modification apis only during master initialization. Other stuff like zk callbacks can still run. I think though that even though incomplete, its helps.
        Hide
        Lars Hofhansl added a comment -

        0.94?

        Show
        Lars Hofhansl added a comment - 0.94?
        Hide
        stack added a comment -

        Committed to trunk. Thanks for the patch Chunhui.

        Show
        stack added a comment - Committed to trunk. Thanks for the patch Chunhui.
        Hide
        stack added a comment -

        I'm +1 on commit for this. We can figure something similar for handling of zk callbacks in another issue.

        Show
        stack added a comment - I'm +1 on commit for this. We can figure something similar for handling of zk callbacks in another issue.
        Hide
        stack added a comment -

        Reopening since Chunhui added a v2.

        Show
        stack added a comment - Reopening since Chunhui added a v2.
        Hide
        Hadoop QA added a comment -

        -1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12516635/hbase-5454v2.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 javadoc. The javadoc tool appears to have generated -131 warning messages.

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

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

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

        -1 core tests. The patch failed these unit tests:
        org.apache.hadoop.hbase.mapreduce.TestImportTsv
        org.apache.hadoop.hbase.mapred.TestTableMapReduce
        org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat

        Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1065//testReport/
        Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1065//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html
        Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1065//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/12516635/hbase-5454v2.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 javadoc. The javadoc tool appears to have generated -131 warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. -1 findbugs. The patch appears to introduce 155 new Findbugs (version 1.3.9) warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. -1 core tests. The patch failed these unit tests: org.apache.hadoop.hbase.mapreduce.TestImportTsv org.apache.hadoop.hbase.mapred.TestTableMapReduce org.apache.hadoop.hbase.mapreduce.TestHFileOutputFormat Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/1065//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/1065//artifact/trunk/patchprocess/newPatchFindbugsWarnings.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/1065//console This message is automatically generated.
        Hide
        chunhui shen added a comment -

        In patch v2, master will throw PleaseHoldException when master is initializing and client call admin operations.

        Show
        chunhui shen added a comment - In patch v2, master will throw PleaseHoldException when master is initializing and client call admin operations.
        Hide
        chunhui shen added a comment -

        Yes, I think so, close this one as won't fix.
        Thanks.

        Show
        chunhui shen added a comment - Yes, I think so, close this one as won't fix. Thanks.
        Hide
        stack added a comment -

        So, you want to mash this patch into hbase-5270? If so, close this one as won't fix?

        Show
        stack added a comment - So, you want to mash this patch into hbase-5270? If so, close this one as won't fix?
        Hide
        chunhui shen added a comment -

        @stack
        I add safe mode for master in HBASE-5270’s V4 patch to fix this issue too.
        Please review it.
        Thanks.

        Show
        chunhui shen added a comment - @stack I add safe mode for master in HBASE-5270 ’s V4 patch to fix this issue too. Please review it. Thanks.
        Hide
        stack added a comment -

        Chunhui Want to do that other stuff in a different issue? This one is nice and simple as is if you make the changes suggested I can commit.

        Show
        stack added a comment - Chunhui Want to do that other stuff in a different issue? This one is nice and simple as is if you make the changes suggested I can commit.
        Hide
        Ted Yu added a comment -

        Instead expireIfOnline, add a method splitLogIfOnline.

        The approach should work.

        We just need to cleanly refactor ServerShutdownHandler so that maximum code reuse is achieved.

        Show
        Ted Yu added a comment - Instead expireIfOnline, add a method splitLogIfOnline. The approach should work. We just need to cleanly refactor ServerShutdownHandler so that maximum code reuse is achieved.
        Hide
        chunhui shen added a comment -

        To be exact, we don't refuse SSH, but delay process SSH after master is initialized.

        Show
        chunhui shen added a comment - To be exact, we don't refuse SSH, but delay process SSH after master is initialized.
        Hide
        chunhui shen added a comment -

        What is the server carrying .META. needs to run ServerShutdownHandler ?

        In that case, we could just split log. Instead expireIfOnline, add a method splitLogIfOnline.

        Show
        chunhui shen added a comment - What is the server carrying .META. needs to run ServerShutdownHandler ? In that case, we could just split log. Instead expireIfOnline, add a method splitLogIfOnline.
        Hide
        Ted Yu added a comment -

        could also refuse ServerShutdownHandler

        What is the server carrying .META. needs to run ServerShutdownHandler ?

        Show
        Ted Yu added a comment - could also refuse ServerShutdownHandler What is the server carrying .META. needs to run ServerShutdownHandler ?
        Hide
        chunhui shen added a comment -

        @Stack
        What about we define the safemode for master like namenode, where master is aborting and initializing.

        In the safemode, we maybe could also refuse ServerShutdownHandler which simplify solution of HBASE-5270.

        Show
        chunhui shen added a comment - @Stack What about we define the safemode for master like namenode, where master is aborting and initializing. In the safemode, we maybe could also refuse ServerShutdownHandler which simplify solution of HBASE-5270 .
        Hide
        stack added a comment -

        This patch is a good idea Chunhui.

        I don't think you need to have this message "Master is not initialized" on an exception whose type is: MasterNotInitializedException. It seems redundant.

        Remove this line:

        + * Copyright 2007 The Apache Software Foundation

        Maybe you don't need these two methods?

        + public MasterNotInitializedException(String s)

        { + super(s); + }

        +
        + /**
        + * Constructor taking another exception.
        + *
        + * @param e Exception to grab data from.
        + */
        + public MasterNotInitializedException(Exception e) {

        Show
        stack added a comment - This patch is a good idea Chunhui. I don't think you need to have this message "Master is not initialized" on an exception whose type is: MasterNotInitializedException. It seems redundant. Remove this line: + * Copyright 2007 The Apache Software Foundation Maybe you don't need these two methods? + public MasterNotInitializedException(String s) { + super(s); + } + + /** + * Constructor taking another exception. + * + * @param e Exception to grab data from. + */ + public MasterNotInitializedException(Exception e) {

          People

          • Assignee:
            chunhui shen
            Reporter:
            chunhui shen
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development