Details

    • Hadoop Flags:
      Reviewed
    • Release Note:
      Hide
      This patch makes it possible to do region assignment without ZK. By default, it is off (i.e. ZK is used for region assignment as before).

      Two setting "hbase.assignment.usezk", "hbase.assignment.usezk.migrating" are introduced to control migration from using ZK for assignment to not using
      ZK for assignment.

      For rolling upgrade from using ZK to not using ZK, it can be done in two steps:

      1. Set both hbase.assignment.usezk and hbase.assignment.usezk.migrating to true, do a rolling upgrade so that both masters and regionservers have
      the new code. Either master or regionserver can be upgraded at first. The order is not important for this step. If you want to keep using ZK for assignment, you'd better set hbase.assignment.usezk to true, and hbase.assignment.usezk.migrating to false so that region states are not persisted in meta table.

      2. Set hbase.assignment.usezk to false, do a rolling restart so that region assignments don't use ZK any more. For this step, masters should be restarted after regionservers have all restarted at first so that they won't update meta table directly and master doesn't know about it.
      Show
      This patch makes it possible to do region assignment without ZK. By default, it is off (i.e. ZK is used for region assignment as before). Two setting "hbase.assignment.usezk", "hbase.assignment.usezk.migrating" are introduced to control migration from using ZK for assignment to not using ZK for assignment. For rolling upgrade from using ZK to not using ZK, it can be done in two steps: 1. Set both hbase.assignment.usezk and hbase.assignment.usezk.migrating to true, do a rolling upgrade so that both masters and regionservers have the new code. Either master or regionserver can be upgraded at first. The order is not important for this step. If you want to keep using ZK for assignment, you'd better set hbase.assignment.usezk to true, and hbase.assignment.usezk.migrating to false so that region states are not persisted in meta table. 2. Set hbase.assignment.usezk to false, do a rolling restart so that region assignments don't use ZK any more. For this step, masters should be restarted after regionservers have all restarted at first so that they won't update meta table directly and master doesn't know about it.

      Description

      It seems that most people don't like region assignment with ZK (HBASE-5487), which causes many uncertainties. This jira is to support ZK-less region assignment. We need to make sure this patch doesn't break backward compatibility/rolling upgrade.

      1. zk-less_am.pdf
        119 kB
        Jimmy Xiang
      2. hbase-11059.patch
        289 kB
        Jimmy Xiang
      3. hbase-11059_v2.patch
        261 kB
        Jimmy Xiang
      4. hbase-11059_v2.1.patch
        286 kB
        Jimmy Xiang
      5. hbase-11059_v2.2.patch
        296 kB
        Jimmy Xiang
      6. zk-less_assignment.png
        20 kB
        Jimmy Xiang
      7. hbase-11059_v3.0.patch
        309 kB
        Jimmy Xiang

        Issue Links

          Activity

          Hide
          Jimmy Xiang added a comment -

          Had several discussions with Matteo and Stack. It's suggested that we can store HBase entities and their states in a system table. For example, we can store tables, regions, and so on in the system table. So we can go without storing transient states in ZK. This table is accessed by master only. We can load it into memory when master starts up.

          Show
          Jimmy Xiang added a comment - Had several discussions with Matteo and Stack. It's suggested that we can store HBase entities and their states in a system table. For example, we can store tables, regions, and so on in the system table. So we can go without storing transient states in ZK. This table is accessed by master only. We can load it into memory when master starts up.
          Hide
          Mikhail Antonov added a comment -

          Jimmy Xiang looks like other jiras like HBASE-7767 would benefit from notion of extended system (meta) tables? Also, if region assignment is ZK-less, HBASE-10909 would benefit too

          without storing transient states in ZK

          Table state isn't transient state, is it?

          Would client need to access this table to find out about assignment/state?

          Show
          Mikhail Antonov added a comment - Jimmy Xiang looks like other jiras like HBASE-7767 would benefit from notion of extended system (meta) tables? Also, if region assignment is ZK-less, HBASE-10909 would benefit too without storing transient states in ZK Table state isn't transient state, is it? Would client need to access this table to find out about assignment/state?
          Hide
          Jimmy Xiang added a comment -

          Table state isn't transient state, is it?

          It's not. But it can be saved in this table too.

          Would client need to access this table to find out about assignment/state?

          Should not. Since the information is already in the master memory, it will be more efficient to ask the master directly.

          Show
          Jimmy Xiang added a comment - Table state isn't transient state, is it? It's not. But it can be saved in this table too. Would client need to access this table to find out about assignment/state? Should not. Since the information is already in the master memory, it will be more efficient to ask the master directly.
          Hide
          Mikhail Antonov added a comment -

          Curious if there's any writeup following up on the discussion you mentioned above? Eager to look at.

          Based on the description, just some thoughts:

          • is that a part of more generic proposal to have system tables, which can be pinned (all regions thereof) to a certain node, or it's just to solve this specific problem?
          • sounds like (correct me if I'm wrong) to make the HA/failover work properly, the consistent replicas of such a table(-s) should be kept on each backup master (and further when we have multiple active master, on active masters)?
          Show
          Mikhail Antonov added a comment - Curious if there's any writeup following up on the discussion you mentioned above? Eager to look at. Based on the description, just some thoughts: is that a part of more generic proposal to have system tables, which can be pinned (all regions thereof) to a certain node, or it's just to solve this specific problem? sounds like (correct me if I'm wrong) to make the HA/failover work properly, the consistent replicas of such a table(-s) should be kept on each backup master (and further when we have multiple active master, on active masters)?
          Hide
          Mikhail Antonov added a comment -

          Actually, as zk-less assignment sounds like something which helps a lot to abstract hbase from ZK (HBASE-10909), I would think we can link these 2 jiras together?

          Show
          Mikhail Antonov added a comment - Actually, as zk-less assignment sounds like something which helps a lot to abstract hbase from ZK ( HBASE-10909 ), I would think we can link these 2 jiras together?
          Hide
          Jimmy Xiang added a comment -

          I don't have a writeup yet. Currently, I was thinking to make sure region assignment works without ZK to simplify the state machine, improve the assignment performance, make it more reliable, so that we can support more regions, and do HA a little bit easier later on. As to pin some system tables to somewhere, like the active master, it's been done in a different issue.

          For backward compatibility, the ZK part will still be there for a while. Yes, it should be of some help.

          Show
          Jimmy Xiang added a comment - I don't have a writeup yet. Currently, I was thinking to make sure region assignment works without ZK to simplify the state machine, improve the assignment performance, make it more reliable, so that we can support more regions, and do HA a little bit easier later on. As to pin some system tables to somewhere, like the active master, it's been done in a different issue. For backward compatibility, the ZK part will still be there for a while. Yes, it should be of some help.
          Hide
          Mikhail Antonov added a comment -

          Jimmy Xiang thanks for desctiption! look forward for the writeup.

          Show
          Mikhail Antonov added a comment - Jimmy Xiang thanks for desctiption! look forward for the writeup.
          Hide
          Konstantin Boudnik added a comment -

          Jimmy Xiang], could you please share the design doc with me as well? Thanks!

          Show
          Konstantin Boudnik added a comment - Jimmy Xiang ], could you please share the design doc with me as well? Thanks!
          Hide
          Jimmy Xiang added a comment -

          Some writeup and comments are attached. Thanks.

          Show
          Jimmy Xiang added a comment - Some writeup and comments are attached. Thanks.
          Hide
          Mikhail Antonov added a comment -

          To recap some of the notes from the doc, to discuss further..

          • the "atomic" short-circuit updates of META are only possible if META is guaranteed to be hosted on active master, which is not the case if HBASE-10569 topology is turned off, unless I'm missing something?
          • if we had system table with statistics (like region size, number of rows in each one etc) we could do master-initiated splits and merges - do you think it would be beneficial in terms of more even load on Master Jimmy Xiang?
          Show
          Mikhail Antonov added a comment - To recap some of the notes from the doc, to discuss further.. the "atomic" short-circuit updates of META are only possible if META is guaranteed to be hosted on active master, which is not the case if HBASE-10569 topology is turned off, unless I'm missing something? if we had system table with statistics (like region size, number of rows in each one etc) we could do master-initiated splits and merges - do you think it would be beneficial in terms of more even load on Master Jimmy Xiang ?
          Hide
          Jimmy Xiang added a comment -

          Attached the first patch, which is also on RB: https://reviews.apache.org/r/21934/

          Show
          Jimmy Xiang added a comment - Attached the first patch, which is also on RB: https://reviews.apache.org/r/21934/
          Hide
          Jimmy Xiang added a comment - - edited

          The patch is backward compatible, rolling upgradable. The performance depends on the setup and the number of regions of course. On my 6 nodes cluster, assigning 13,990 user regions, it took about 8minutes using ZK, and ~1minute with this patch (no ZK).

          Show
          Jimmy Xiang added a comment - - edited The patch is backward compatible, rolling upgradable. The performance depends on the setup and the number of regions of course. On my 6 nodes cluster, assigning 13,990 user regions, it took about 8minutes using ZK, and ~1minute with this patch (no ZK).
          Hide
          stack added a comment -

          Jimmy Xiang What if you did 1M regions?

          Show
          stack added a comment - Jimmy Xiang What if you did 1M regions?
          Hide
          Mikhail Antonov added a comment -

          Great! Jimmy Xiang - will take a look today.

          Show
          Mikhail Antonov added a comment - Great! Jimmy Xiang - will take a look today.
          Hide
          Matteo Bertozzi added a comment -

          Jimmy Xiang is this patch just writing the "state" to meta instead of zookeeper without changing the assignment algorithm at all?

          Show
          Matteo Bertozzi added a comment - Jimmy Xiang is this patch just writing the "state" to meta instead of zookeeper without changing the assignment algorithm at all?
          Hide
          Jimmy Xiang added a comment -

          The old assignment logic is still there for rolling upgrade. If the new logic is turned on, we will store "states" to meta, and regionserver reports region transitions to master via RPC instead of ZK. There are still room for improvement, for example, report aggregation, skip some heart-beat, etc.. Client side logic is not changed very much for compatibility.

          Show
          Jimmy Xiang added a comment - The old assignment logic is still there for rolling upgrade. If the new logic is turned on, we will store "states" to meta, and regionserver reports region transitions to master via RPC instead of ZK. There are still room for improvement, for example, report aggregation, skip some heart-beat, etc.. Client side logic is not changed very much for compatibility.
          Hide
          Jimmy Xiang added a comment -

          By the way, I am going to try 1M regions now. I need a bigger cluster.

          Show
          Jimmy Xiang added a comment - By the way, I am going to try 1M regions now. I need a bigger cluster.
          Hide
          Jimmy Xiang added a comment -

          Attached v2 which is I am testing now.

          Show
          Jimmy Xiang added a comment - Attached v2 which is I am testing now.
          Hide
          stack added a comment -

          -public class RegionState implements org.apache.hadoop.io.Writable {
          +public class RegionState {

          Above is good. We serialize RegionState ever? Nvm... I see RegionStatusProto now.

          On:

          • ZKUtil.createAndFailSilent(this, assignmentZNode);
            + if (conf.getBoolean("hbase.assignment.usezk", false)) { + ZKUtil.createAndFailSilent(this, assignmentZNode); + }

          Should we remove the assignment znode dir if it exists? (NVM... I see you do it on startup).

          Leave of this sentence I'd say: " The other
          + * way around is not recommended also sophisticated users could do it
          + * somehow."

          Put this in release note or lets open issue to ensure the below gets added to the 1.0 migration doc:

          + * For rolling upgrade from using ZK to not using ZK, it can be
          + * done in two steps:
          + * 1. Set hbase.assignment.usezk to true, do a rolling upgrade
          + * so that both masters and regionservers have the new code. Either
          + * master or regionserver can be upgraded at first. The order
          + * is not important for this step.
          + * 2. Set hbase.assignment.usezk to false, do a rolling restart
          + * so that region assignments don't use ZK any more. For this step,
          + * masters should be restarted after regionservers have all
          + * restarted at first so that they won't update meta table
          + * directly and master doesn't know about it.

          This message will come out wrong?

          + LOG.info("Finished region assignment in (failover=" + failover
          + + ")" + (System.currentTimeMillis() - startTime) + "ms");

          It will have 'failover=true' in between 'in' and time in ms... should it be on end?

          It is a pity we have to pass this flag useZKForAssignment down:

          • unassign(regionInfo, rsClosing, expectedVersion, null, true, null);
            + unassign(regionInfo, rsClosing, expectedVersion, null, useZKForAssignment, null);

          Can this be done in a 'transaction'?

          + mergingRegions.remove(encodedName);
          + regionOffline(a, State.MERGED);
          + regionOffline(b, State.MERGED);
          + regionOnline(p, sn, 1);

          We have transactions right as long as all in the same region? Which is the case when only single meta.

          I skimmed the rest.

          This looks amazing Jimmy. I'd have thought it would have taken way more code.

          Looking at tests, I was wondering if the new state transitions could be tested independent of the servers? That possible at all? Should be easier now no zk? Can the state management classes be stood up independent of the Master?

          I can see this going in if your testing works out. We'd have it off on commit. Need to add doc on new assignment state of affairs. You have it written up in code. Would just copy it into releases notes and/or into refguide. We'd open new blocker against 1.0 to discuss whether to have it on for 1.0 (it'd be great if we could have this on... I think it'll save orders of magnitude more headache than it causes). If it is on for 1.0, we'd have to doc the rolling upgrade.

          Show
          stack added a comment - -public class RegionState implements org.apache.hadoop.io.Writable { +public class RegionState { Above is good. We serialize RegionState ever? Nvm... I see RegionStatusProto now. On: ZKUtil.createAndFailSilent(this, assignmentZNode); + if (conf.getBoolean("hbase.assignment.usezk", false)) { + ZKUtil.createAndFailSilent(this, assignmentZNode); + } Should we remove the assignment znode dir if it exists? (NVM... I see you do it on startup). Leave of this sentence I'd say: " The other + * way around is not recommended also sophisticated users could do it + * somehow." Put this in release note or lets open issue to ensure the below gets added to the 1.0 migration doc: + * For rolling upgrade from using ZK to not using ZK, it can be + * done in two steps: + * 1. Set hbase.assignment.usezk to true, do a rolling upgrade + * so that both masters and regionservers have the new code. Either + * master or regionserver can be upgraded at first. The order + * is not important for this step. + * 2. Set hbase.assignment.usezk to false, do a rolling restart + * so that region assignments don't use ZK any more. For this step, + * masters should be restarted after regionservers have all + * restarted at first so that they won't update meta table + * directly and master doesn't know about it. This message will come out wrong? + LOG.info("Finished region assignment in (failover=" + failover + + ")" + (System.currentTimeMillis() - startTime) + "ms"); It will have 'failover=true' in between 'in' and time in ms... should it be on end? It is a pity we have to pass this flag useZKForAssignment down: unassign(regionInfo, rsClosing, expectedVersion, null, true, null); + unassign(regionInfo, rsClosing, expectedVersion, null, useZKForAssignment, null); Can this be done in a 'transaction'? + mergingRegions.remove(encodedName); + regionOffline(a, State.MERGED); + regionOffline(b, State.MERGED); + regionOnline(p, sn, 1); We have transactions right as long as all in the same region? Which is the case when only single meta. I skimmed the rest. This looks amazing Jimmy. I'd have thought it would have taken way more code. Looking at tests, I was wondering if the new state transitions could be tested independent of the servers? That possible at all? Should be easier now no zk? Can the state management classes be stood up independent of the Master? I can see this going in if your testing works out. We'd have it off on commit. Need to add doc on new assignment state of affairs. You have it written up in code. Would just copy it into releases notes and/or into refguide. We'd open new blocker against 1.0 to discuss whether to have it on for 1.0 (it'd be great if we could have this on... I think it'll save orders of magnitude more headache than it causes). If it is on for 1.0, we'd have to doc the rolling upgrade.
          Hide
          Jimmy Xiang added a comment -

          Thanks a lot for the review, Stack!

          This one is fixed:

          Leave of this sentence I'd say: " The other
          + * way around is not recommended also sophisticated users could do it
          + * somehow."

          For migration instruction, I will copy it to the release note.

          This one is fixed:

          This message will come out wrong?
          + LOG.info("Finished region assignment in (failover=" + failover
          + + ")" + (System.currentTimeMillis() - startTime) + "ms");

          That's right. It's better to be done in one single transaction explicitly. Currently, I think we have transactions as long as only one RS is doing the merge/split for the same set of regions.

          We have transactions right as long as all in the same region? Which is the case when only single meta.

          By the way, I did some test. With a patch v3 (to-be-uploaded), it took about 316 seconds (~5 minutes) to assign 1 million regions. All these regions are empty so take no time to open. Including loading the regions from meta (twice, one to restore the state, the other one to init the region assignment snapshot for favored nodes plan. I think the second one can be removed. I can fix it in a separate issue), it took about 395 seconds (~6.5 minutes).

          Without the patch, I don't have a number yet. It's still running. I have increased the ZK jute buffer several times.

          Show
          Jimmy Xiang added a comment - Thanks a lot for the review, Stack! This one is fixed: Leave of this sentence I'd say: " The other + * way around is not recommended also sophisticated users could do it + * somehow." For migration instruction, I will copy it to the release note. This one is fixed: This message will come out wrong? + LOG.info("Finished region assignment in (failover=" + failover + + ")" + (System.currentTimeMillis() - startTime) + "ms"); That's right. It's better to be done in one single transaction explicitly. Currently, I think we have transactions as long as only one RS is doing the merge/split for the same set of regions. We have transactions right as long as all in the same region? Which is the case when only single meta. By the way, I did some test. With a patch v3 (to-be-uploaded), it took about 316 seconds (~5 minutes) to assign 1 million regions. All these regions are empty so take no time to open. Including loading the regions from meta (twice, one to restore the state, the other one to init the region assignment snapshot for favored nodes plan. I think the second one can be removed. I can fix it in a separate issue), it took about 395 seconds (~6.5 minutes). Without the patch, I don't have a number yet. It's still running. I have increased the ZK jute buffer several times.
          Hide
          Jimmy Xiang added a comment -

          I think it is better to have this one in 1.0. So that we can clean up quite some ZK assignment code in 2.0. In the meantime, we can maintain a rolling upgrade path.

          Show
          Jimmy Xiang added a comment - I think it is better to have this one in 1.0. So that we can clean up quite some ZK assignment code in 2.0. In the meantime, we can maintain a rolling upgrade path.
          Hide
          stack added a comment -

          Francis Liu See Jimmy's 1M test results above.

          Sweet Jimmy!

          It should be in 1.0 because it reduces headaches and swelling (smile)!

          How hard to hack a patch for 0.98 for Francis to try next week on his 300node cluster? I could work on it if you think it doable.

          Show
          stack added a comment - Francis Liu See Jimmy's 1M test results above. Sweet Jimmy! It should be in 1.0 because it reduces headaches and swelling (smile)! How hard to hack a patch for 0.98 for Francis to try next week on his 300node cluster? I could work on it if you think it doable.
          Hide
          Jimmy Xiang added a comment -

          For a patch for 0.98, let me give it a try. We need HBASE-10569 too. By the way, in the test, I set both openregion/closeregion thread to 20. The default is 3.

          Show
          Jimmy Xiang added a comment - For a patch for 0.98, let me give it a try. We need HBASE-10569 too. By the way, in the test, I set both openregion/closeregion thread to 20. The default is 3.
          Hide
          Jimmy Xiang added a comment -

          Attached patch v2.1 that passed unit test and itbll locally. For 1million empty regions on 39 nodes, openregion threads set to 20, with the patch it took about 5 minutes to assign them all. Without the patch, it took about 68 minutes.

          Show
          Jimmy Xiang added a comment - Attached patch v2.1 that passed unit test and itbll locally. For 1million empty regions on 39 nodes, openregion threads set to 20, with the patch it took about 5 minutes to assign them all. Without the patch, it took about 68 minutes.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12648832/hbase-11059_v2.1.patch
          against trunk revision .
          ATTACHMENT ID: 12648832

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

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

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

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

          +1 findbugs. The patch does not introduce any 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 introduces the following lines longer than 100:
          + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code>
          + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code>
          + } else if (!useZKForAssignment || asyncSetOfflineInZooKeeper(state, cb, destination)) {

          +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/9719//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//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/12648832/hbase-11059_v2.1.patch against trunk revision . ATTACHMENT ID: 12648832 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 60 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 findbugs . The patch does not introduce any 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 introduces the following lines longer than 100: + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code> + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code> + } else if (!useZKForAssignment || asyncSetOfflineInZooKeeper(state, cb, destination)) { +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/9719//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9719//console This message is automatically generated.
          Hide
          stack added a comment -

          Jimmy Xiang Is the patch up on rb?

          Show
          stack added a comment - Jimmy Xiang Is the patch up on rb?
          Hide
          Jimmy Xiang added a comment - - edited

          Yes, it is on RB: https://reviews.apache.org/r/21934/, the 3rd update.

          Show
          Jimmy Xiang added a comment - - edited Yes, it is on RB: https://reviews.apache.org/r/21934/ , the 3rd update.
          Hide
          stack added a comment -

          ok. I went over RB and left some comments. Minor for the most part. Did not review actual transitions too closely figuring that unit tests and IT will find issues before I would.

          I'm +1 for committing this. Would be cool to get a +1 from a Jeffrey Zhong too... or at least a skim by Jeffrey (Jeffrey, this is sweet work!).

          +1 would be to commit with this feature off. We'd then file an issue against 1.0 to turn it on by default. In that issue we'd make the case for why this should be on.

          Suggest you pull out the comments for rolling restart and shove them in as the release note for this JIRA. Good on you Jimmy.

          Show
          stack added a comment - ok. I went over RB and left some comments. Minor for the most part. Did not review actual transitions too closely figuring that unit tests and IT will find issues before I would. I'm +1 for committing this. Would be cool to get a +1 from a Jeffrey Zhong too... or at least a skim by Jeffrey (Jeffrey, this is sweet work!). +1 would be to commit with this feature off. We'd then file an issue against 1.0 to turn it on by default. In that issue we'd make the case for why this should be on. Suggest you pull out the comments for rolling restart and shove them in as the release note for this JIRA. Good on you Jimmy.
          Hide
          Jimmy Xiang added a comment -

          Cool, thanks. Yes, I will set this feature off by default. Jeffrey Zhong, can you take a look? Thanks.

          Show
          Jimmy Xiang added a comment - Cool, thanks. Yes, I will set this feature off by default. Jeffrey Zhong , can you take a look? Thanks.
          Hide
          Jeffrey Zhong added a comment -

          This is great! The result of improvement is terrific. Let me to take a look at this patch. Does this JIRA depend on master colocating meta otherwise meta recovery needs some bootstrap? Thanks.

          Another idea on this topic is to let RS do all the assignments including meta update and master only need to initiate assignment request & monitor un-assignment regions without even keeping internal memory state.

          Show
          Jeffrey Zhong added a comment - This is great! The result of improvement is terrific. Let me to take a look at this patch. Does this JIRA depend on master colocating meta otherwise meta recovery needs some bootstrap? Thanks. Another idea on this topic is to let RS do all the assignments including meta update and master only need to initiate assignment request & monitor un-assignment regions without even keeping internal memory state.
          Hide
          Francis Liu added a comment -

          Interesting, I suspect zk is signifcantly slower because it syncs to disk (unlike hdfs). Can we have a run without forceSync=no?

          For a patch for 0.98, let me give it a try. We need HBASE-10569 too.

          Does it require the new deployment to make things work? Or is there just some bits of code change that it depends on? We will keep the original deployment supported still correct?

          By the way, in the test, I set both openregion/closeregion thread to 20. The default is 3.

          Are there any other configs you changed?

          Show
          Francis Liu added a comment - Interesting, I suspect zk is signifcantly slower because it syncs to disk (unlike hdfs). Can we have a run without forceSync=no? For a patch for 0.98, let me give it a try. We need HBASE-10569 too. Does it require the new deployment to make things work? Or is there just some bits of code change that it depends on? We will keep the original deployment supported still correct? By the way, in the test, I set both openregion/closeregion thread to 20. The default is 3. Are there any other configs you changed?
          Hide
          stack added a comment -

          Another idea on this topic is to let RS do all the assignments including meta update and master only need to initiate assignment request & monitor un-assignment regions without even keeping internal memory state.

          Tell us more please. Do it in HBASE-11165?

          without forceSync=no

          Francis Liu you mean 'with', not 'without' in above? You want to see what happens if zk is not sync'ing log?

          Show
          stack added a comment - Another idea on this topic is to let RS do all the assignments including meta update and master only need to initiate assignment request & monitor un-assignment regions without even keeping internal memory state. Tell us more please. Do it in HBASE-11165 ? without forceSync=no Francis Liu you mean 'with', not 'without' in above? You want to see what happens if zk is not sync'ing log?
          Hide
          Jimmy Xiang added a comment -

          Jeffrey Zhong, yes, it needs meta/master colocation. This patch makes sure only master updates the meta.
          Francis Liu, I also set hbase.bulk.assignment.waittillallassigned to true since I'd like to get the assignment time only. I didn't change forceSync. So the default value should be used. The reason we need HBASE-10569 is that the patch talks to the meta region directly in persisting region state changes. If meta is on a different server, the performance shouldn't be the same. The new deployment is used in my testing. But the original deployment is still supported, with some performance hurt of course.

          Show
          Jimmy Xiang added a comment - Jeffrey Zhong , yes, it needs meta/master colocation. This patch makes sure only master updates the meta. Francis Liu , I also set hbase.bulk.assignment.waittillallassigned to true since I'd like to get the assignment time only. I didn't change forceSync. So the default value should be used. The reason we need HBASE-10569 is that the patch talks to the meta region directly in persisting region state changes. If meta is on a different server, the performance shouldn't be the same. The new deployment is used in my testing. But the original deployment is still supported, with some performance hurt of course.
          Hide
          Francis Liu added a comment -

          Francis Liu you mean 'with', not 'without' in above? You want to see what happens if zk is not sync'ing log?

          stack sorry typo, yes compare when zk is not sync'ing. Would be good to also have a separate zkwatcher for assignment.

          Show
          Francis Liu added a comment - Francis Liu you mean 'with', not 'without' in above? You want to see what happens if zk is not sync'ing log? stack sorry typo, yes compare when zk is not sync'ing. Would be good to also have a separate zkwatcher for assignment.
          Hide
          Francis Liu added a comment -

          The reason we need HBASE-10569 is that the patch talks to the meta region directly in persisting region state changes. If meta is on a different server, the performance shouldn't be the same. The new deployment is used in my testing. But the original deployment is still supported, with some performance hurt of course.

          I see so it will still work with meta remotely access. IMHO I don't think the perf degradation will be significant, though would be good to quantify.

          Show
          Francis Liu added a comment - The reason we need HBASE-10569 is that the patch talks to the meta region directly in persisting region state changes. If meta is on a different server, the performance shouldn't be the same. The new deployment is used in my testing. But the original deployment is still supported, with some performance hurt of course. I see so it will still work with meta remotely access. IMHO I don't think the perf degradation will be significant, though would be good to quantify.
          Hide
          Jeffrey Zhong added a comment -

          Tell us more please. Do it in HBASE-11165?

          The basic idea in high-level is that master only coordinates the assignment requests without really being involved in the actual assignment work flow(it's better that if we have FATE like library to back it up) and we can do it later if we're agreed.

          1) Master create a new state:"assigning", reason and new destination RS for a region in meta
          2) Master sends an assign request to the new RS
          3) The new RS( and the current RS hosting the region) will assume the duty to do the whole assignment and update meta states along with assignment including: prepare-closing, closing, closed, opening & opened. This needs region server to talk to each other in order to communicate info that current ZK notifications carry and report status back to master by RPC if it fails the assignment.
          4) Master monitors un-assigned & failed open regions periodically

          Thanks.

          Show
          Jeffrey Zhong added a comment - Tell us more please. Do it in HBASE-11165 ? The basic idea in high-level is that master only coordinates the assignment requests without really being involved in the actual assignment work flow(it's better that if we have FATE like library to back it up) and we can do it later if we're agreed. 1) Master create a new state:"assigning", reason and new destination RS for a region in meta 2) Master sends an assign request to the new RS 3) The new RS( and the current RS hosting the region) will assume the duty to do the whole assignment and update meta states along with assignment including: prepare-closing, closing, closed, opening & opened. This needs region server to talk to each other in order to communicate info that current ZK notifications carry and report status back to master by RPC if it fails the assignment. 4) Master monitors un-assigned & failed open regions periodically Thanks.
          Hide
          Mikhail Antonov added a comment -

          Jimmy Xiang the performance improvement is pretty amazing! Let me look at the patch (I looked at v1 before, I assume I can look v1->v3 now?)

          So that we can clean up quite some ZK assignment code in 2.0. In the meantime, we can maintain a rolling upgrade path.

          Do you think we could finish that transition in like 1.1 or it would have to wait till 2.0?

          Also regarding release notes..I'd say that if we can't enforce that people migrate off ZK during rolling upgrade (to be discussed more?), then at least it needs to be worded in release notes in the way like it's strictly advised thing to do, and a note that the old way will be removed soon. What do you think?

          Show
          Mikhail Antonov added a comment - Jimmy Xiang the performance improvement is pretty amazing! Let me look at the patch (I looked at v1 before, I assume I can look v1->v3 now?) So that we can clean up quite some ZK assignment code in 2.0. In the meantime, we can maintain a rolling upgrade path. Do you think we could finish that transition in like 1.1 or it would have to wait till 2.0? Also regarding release notes..I'd say that if we can't enforce that people migrate off ZK during rolling upgrade (to be discussed more?), then at least it needs to be worded in release notes in the way like it's strictly advised thing to do, and a note that the old way will be removed soon. What do you think?
          Hide
          Mikhail Antonov added a comment -

          Sorry, I was going to look at patch earlier but got distracted.

          Show
          Mikhail Antonov added a comment - Sorry, I was going to look at patch earlier but got distracted.
          Hide
          Jimmy Xiang added a comment -

          No problem. Please just look v3. During to rebasing, v1->v3 diff may pull something else in.

          Do you think we could finish that transition in like 1.1 or it would have to wait till 2.0?

          Probably not. As you said, we can't enforce people to migrate off ZK during rolling upgrade, at least inside the same major version. It's up to the community. I think it's fair to move off ZK (assignment) completely in 2.0 instead of 1.1

          Show
          Jimmy Xiang added a comment - No problem. Please just look v3. During to rebasing, v1->v3 diff may pull something else in. Do you think we could finish that transition in like 1.1 or it would have to wait till 2.0? Probably not. As you said, we can't enforce people to migrate off ZK during rolling upgrade, at least inside the same major version. It's up to the community. I think it's fair to move off ZK (assignment) completely in 2.0 instead of 1.1
          Hide
          Jimmy Xiang added a comment -

          I see so it will still work with meta remotely access. IMHO I don't think the perf degradation will be significant, though would be good to quantify.

          The issue is that it is not optimized in this case since I assume meta is co-located on master. Currently, only one meta HTable instance is used in this case in a synchronized way. It is better to use a pool of meta HTable instances.

          Show
          Jimmy Xiang added a comment - I see so it will still work with meta remotely access. IMHO I don't think the perf degradation will be significant, though would be good to quantify. The issue is that it is not optimized in this case since I assume meta is co-located on master. Currently, only one meta HTable instance is used in this case in a synchronized way. It is better to use a pool of meta HTable instances.
          Hide
          Jimmy Xiang added a comment - - edited

          Attached v2.2 with minor changes. It is on RB: https://reviews.apache.org/r/21934/, update #4.

          Show
          Jimmy Xiang added a comment - - edited Attached v2.2 with minor changes. It is on RB: https://reviews.apache.org/r/21934/ , update #4.
          Hide
          Jimmy Xiang added a comment -

          Attached zk-less_assignment.png. It shows the region assignment time comparison between with and without this patch. The x-axis is the number of regions (x1000). The y-axis is the total assignment time in second.

          Show
          Jimmy Xiang added a comment - Attached zk-less_assignment.png. It shows the region assignment time comparison between with and without this patch. The x-axis is the number of regions (x1000). The y-axis is the total assignment time in second.
          Hide
          stack added a comment -

          Nice picture Jimmy.

          Show
          stack added a comment - Nice picture Jimmy.
          Hide
          Jeffrey Zhong added a comment - - edited

          I have reviewed the patch v2.1 in high level. Looks great in general. I have following comments:

          1) Rolling restart when turn hbase.assignment.usezk from OFF to ON.
          At this scenario, rolling restart seems not working. Region server will try to update ZK node during region opening & will fail. Basically all RS with new config can't open any region. Logically it's equivalent stop every thing and then restart. It's also unclear to me when a new master starts where it gets RIT info.

          2) When hbase.assignment.usezk is ON, a region is set RIT to pending_open before it sends region open RPC out and master restarts. During restart, how AM processes left over RIT isn't clear to me. The code seems only process RITs recorded in ZK assignment node.

          3) When a region is opened after master receives region transition response from RS, could we use checkAndPut to update META in order to make sure the region location is the same as the RS who sends the region transition request to prevent potential double assignment.

          4) Inside postOpenDeployTasks, when reportRegionTransition return false we don't offline the region in RS. If this situation happens, when we open a region which isn't supposed to do so, the opening has side effects like removing recovered.edits files when recovery mode is in recover edits mode.

          5) A rare race condition that a RS fails to open a region and it reports to master as FAILED_OPEN but without firstly changing its internal memory state. If master re-assigns the region onto the same RS, the region open RPC will simply return OPENED and the region will be unassigned forever.

          Show
          Jeffrey Zhong added a comment - - edited I have reviewed the patch v2.1 in high level. Looks great in general. I have following comments: 1) Rolling restart when turn hbase.assignment.usezk from OFF to ON. At this scenario, rolling restart seems not working. Region server will try to update ZK node during region opening & will fail. Basically all RS with new config can't open any region. Logically it's equivalent stop every thing and then restart. It's also unclear to me when a new master starts where it gets RIT info. 2) When hbase.assignment.usezk is ON, a region is set RIT to pending_open before it sends region open RPC out and master restarts. During restart, how AM processes left over RIT isn't clear to me. The code seems only process RITs recorded in ZK assignment node. 3) When a region is opened after master receives region transition response from RS, could we use checkAndPut to update META in order to make sure the region location is the same as the RS who sends the region transition request to prevent potential double assignment. 4) Inside postOpenDeployTasks, when reportRegionTransition return false we don't offline the region in RS. If this situation happens, when we open a region which isn't supposed to do so, the opening has side effects like removing recovered.edits files when recovery mode is in recover edits mode. 5) A rare race condition that a RS fails to open a region and it reports to master as FAILED_OPEN but without firstly changing its internal memory state. If master re-assigns the region onto the same RS, the region open RPC will simply return OPENED and the region will be unassigned forever.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12649658/hbase-11059_v2.2.patch
          against trunk revision .
          ATTACHMENT ID: 12649658

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

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

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

          -1 javadoc. The javadoc tool appears to have generated 1 warning messages.

          +1 findbugs. The patch does not introduce any 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 introduces the following lines longer than 100:
          + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code>
          + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code>
          + } else if (!useZKForAssignment || asyncSetOfflineInZooKeeper(state, cb, destination)) {
          + private Map<ServerName, Boolean> requeuedDeadServers = new ConcurrentHashMap<ServerName, Boolean>();

          +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/9736//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//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/12649658/hbase-11059_v2.2.patch against trunk revision . ATTACHMENT ID: 12649658 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 60 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. +1 findbugs . The patch does not introduce any 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 introduces the following lines longer than 100: + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code> + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code> + } else if (!useZKForAssignment || asyncSetOfflineInZooKeeper(state, cb, destination)) { + private Map<ServerName, Boolean> requeuedDeadServers = new ConcurrentHashMap<ServerName, Boolean>(); +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/9736//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9736//console This message is automatically generated.
          Hide
          Jimmy Xiang added a comment -

          Jeffrey Zhong, thanks a lot for the review.

          1) Rolling restart when turn hbase.assignment.usezk from OFF to ON.

          This is not supported. We support rolling restart from ON to OFF, i.e. from using ZK to not using ZK.

          2) When hbase.assignment.usezk is ON, a region...

          This is the existing behavior. Before the master sends region open RPC to RS, the ZK is already created in proper state. So it's enough to process RITs recorded in ZK assignment node only.

          3) When a region is opened after master receives region transition response from RS, could we use checkAndPut to update META...

          This is too conservative. The idea is that in the new scheme, the master should be the source of truth, and have the meta info in memory, that's why we just check against the info in RegionStates.

          4) Inside postOpenDeployTasks, when reportRegionTransition return false we don't offline the region in RS.

          Good catch. In this case, postOpenDeployTasks should throw an exception so the region will be offlined. I will handle this in the next patch.

          5) A rare race condition that a RS fails to open a region and it reports to master as FAILED_OPEN but without firstly changing its internal memory state....

          Right. This seems to be an existing issue, right?

          Show
          Jimmy Xiang added a comment - Jeffrey Zhong , thanks a lot for the review. 1) Rolling restart when turn hbase.assignment.usezk from OFF to ON. This is not supported. We support rolling restart from ON to OFF, i.e. from using ZK to not using ZK. 2) When hbase.assignment.usezk is ON, a region... This is the existing behavior. Before the master sends region open RPC to RS, the ZK is already created in proper state. So it's enough to process RITs recorded in ZK assignment node only. 3) When a region is opened after master receives region transition response from RS, could we use checkAndPut to update META... This is too conservative. The idea is that in the new scheme, the master should be the source of truth, and have the meta info in memory, that's why we just check against the info in RegionStates. 4) Inside postOpenDeployTasks, when reportRegionTransition return false we don't offline the region in RS. Good catch. In this case, postOpenDeployTasks should throw an exception so the region will be offlined. I will handle this in the next patch. 5) A rare race condition that a RS fails to open a region and it reports to master as FAILED_OPEN but without firstly changing its internal memory state.... Right. This seems to be an existing issue, right?
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12649658/hbase-11059_v2.2.patch
          against trunk revision .
          ATTACHMENT ID: 12649658

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

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

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

          -1 javadoc. The javadoc tool appears to have generated 1 warning messages.

          +1 findbugs. The patch does not introduce any 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 introduces the following lines longer than 100:
          + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code>
          + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code>
          + } else if (!useZKForAssignment || asyncSetOfflineInZooKeeper(state, cb, destination)) {
          + private Map<ServerName, Boolean> requeuedDeadServers = new ConcurrentHashMap<ServerName, Boolean>();

          +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/9737//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//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/12649658/hbase-11059_v2.2.patch against trunk revision . ATTACHMENT ID: 12649658 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 60 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. -1 javadoc . The javadoc tool appears to have generated 1 warning messages. +1 findbugs . The patch does not introduce any 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 introduces the following lines longer than 100: + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code> + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code> + } else if (!useZKForAssignment || asyncSetOfflineInZooKeeper(state, cb, destination)) { + private Map<ServerName, Boolean> requeuedDeadServers = new ConcurrentHashMap<ServerName, Boolean>(); +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/9737//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9737//console This message is automatically generated.
          Hide
          Jeffrey Zhong added a comment -

          2) When hbase.assignment.usezk is ON

          Actually I mean hbase.assignment.usezk is OFF(new mode) and PENDING_OPEN RIT state is stored in META but real region open RPC may(or may not) sent out before master restarts. During master starts I didn't see where we handle regions in PENDING_OPEN state from META.

          This seems to be an existing issue, right?

          Before we offline region and then set FAILED_OPEN in ZK. This is likely a new behavior.

          Show
          Jeffrey Zhong added a comment - 2) When hbase.assignment.usezk is ON Actually I mean hbase.assignment.usezk is OFF(new mode) and PENDING_OPEN RIT state is stored in META but real region open RPC may(or may not) sent out before master restarts. During master starts I didn't see where we handle regions in PENDING_OPEN state from META. This seems to be an existing issue, right? Before we offline region and then set FAILED_OPEN in ZK. This is likely a new behavior.
          Hide
          Jimmy Xiang added a comment -

          During master starts I didn't see where we handle regions in PENDING_OPEN state from META.

          I see. We should send the RPC again for both PENDING_OPEN and PENDING_CLOSE at master start in case failover.

          Before we offline region and then set FAILED_OPEN in ZK. This is likely a new behavior.

          I added the offline region part. I mean the race is an existing issue. With ZK, RS can set FAILED_OPEN in ZK, before clean up the RS internal memory state, the master could handle the FAILED_OPEN ZK event and try to open the region on the same RS again and get back already opened. There seems not to have an easy fix. Probably a little synchronization is needed. Perhaps we can address this one in a separate issue.

          Show
          Jimmy Xiang added a comment - During master starts I didn't see where we handle regions in PENDING_OPEN state from META. I see. We should send the RPC again for both PENDING_OPEN and PENDING_CLOSE at master start in case failover. Before we offline region and then set FAILED_OPEN in ZK. This is likely a new behavior. I added the offline region part. I mean the race is an existing issue. With ZK, RS can set FAILED_OPEN in ZK, before clean up the RS internal memory state, the master could handle the FAILED_OPEN ZK event and try to open the region on the same RS again and get back already opened. There seems not to have an easy fix. Probably a little synchronization is needed. Perhaps we can address this one in a separate issue.
          Hide
          Jimmy Xiang added a comment -

          Jeffrey Zhong, can we handle the pending_open/close issue in a separate jira? I will add some tests to cover it. Other issues are simple to fix. I can do it at commit.
          Enis Soztutar, how is HBASE-10070 rebase? Can I get this patch in before it? Otherwise, I have to run all tests again.

          Show
          Jimmy Xiang added a comment - Jeffrey Zhong , can we handle the pending_open/close issue in a separate jira? I will add some tests to cover it. Other issues are simple to fix. I can do it at commit. Enis Soztutar , how is HBASE-10070 rebase? Can I get this patch in before it? Otherwise, I have to run all tests again.
          Hide
          Jeffrey Zhong added a comment -

          can we handle the pending_open/close issue in a separate jira? I will add some tests to cover it.

          Fine with me.

          There maybe a region double assignment issue when we turn the feature on(from hbase.assignment.usezk=ON to hbase.assignment.usezk=OFF) during rolling restart. Below are details:
          1) Since we rolling-restart RS firstly, A RS will use the new config(hbase.assignment.usezk=OFF)
          2) Master is still using hbase.assignment.usezk=ON so AM creates offline znode before sending region open. Though a region will be opened, the OFFLINE znode will be left even after the region is opened.
          3) When master restarts with new config(hbase.assignment.usezk=OFF), AM will process RITs left over in ZK. During RIT processing on offline znode, the region will be assigned somewhere while it's still open in another RS.

          Show
          Jeffrey Zhong added a comment - can we handle the pending_open/close issue in a separate jira? I will add some tests to cover it. Fine with me. There maybe a region double assignment issue when we turn the feature on(from hbase.assignment.usezk=ON to hbase.assignment.usezk=OFF) during rolling restart. Below are details: 1) Since we rolling-restart RS firstly, A RS will use the new config(hbase.assignment.usezk=OFF) 2) Master is still using hbase.assignment.usezk=ON so AM creates offline znode before sending region open. Though a region will be opened, the OFFLINE znode will be left even after the region is opened. 3) When master restarts with new config(hbase.assignment.usezk=OFF), AM will process RITs left over in ZK. During RIT processing on offline znode, the region will be assigned somewhere while it's still open in another RS.
          Hide
          Jimmy Xiang added a comment -

          At step 3, AM will try to send region open to the same RS and get an already_opened response back. The RIT processing won't try to use a different RS.

          Good questions! Do you see more possible issues? Thanks.

          Show
          Jimmy Xiang added a comment - At step 3, AM will try to send region open to the same RS and get an already_opened response back. The RIT processing won't try to use a different RS. Good questions! Do you see more possible issues? Thanks.
          Hide
          Jimmy Xiang added a comment -

          Attached patch v3.0 that addressed the pending_open/close issue Jeffrey Zhong pointed out. If there are new issues, it will be good to address in follow-up issues. If no objection, I would like to commit it this weekend.

          Show
          Jimmy Xiang added a comment - Attached patch v3.0 that addressed the pending_open/close issue Jeffrey Zhong pointed out. If there are new issues, it will be good to address in follow-up issues. If no objection, I would like to commit it this weekend.
          Hide
          Jeffrey Zhong added a comment -

          The RIT processing won't try to use a different RS.

          I see. Make sense.

          Show
          Jeffrey Zhong added a comment - The RIT processing won't try to use a different RS. I see. Make sense.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12649907/hbase-11059_v3.0.patch
          against trunk revision .
          ATTACHMENT ID: 12649907

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

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

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

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

          +1 findbugs. The patch does not introduce any 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 introduces the following lines longer than 100:
          + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code>
          + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code>
          + RegionState newState = new RegionState(hriOnline, State.PENDING_CLOSE, oldState.getServerName());

          +1 site. The mvn site goal succeeds with this patch.

          -1 core tests. The patch failed these unit tests:

          -1 core zombie tests. There are 1 zombie test(s): at org.apache.hadoop.hbase.regionserver.TestHRegion.testWritesWhileGetting(TestHRegion.java:3500)

          Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//testReport/
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html
          Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html
          Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//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/12649907/hbase-11059_v3.0.patch against trunk revision . ATTACHMENT ID: 12649907 +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 60 new or modified tests. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . The javadoc tool did not generate any warning messages. +1 findbugs . The patch does not introduce any 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 introduces the following lines longer than 100: + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code> + * <code>rpc ReportRegionTransition(.ReportRegionTransitionRequest) returns (.ReportRegionTransitionResponse);</code> + RegionState newState = new RegionState(hriOnline, State.PENDING_CLOSE, oldState.getServerName()); +1 site . The mvn site goal succeeds with this patch. -1 core tests . The patch failed these unit tests: -1 core zombie tests . There are 1 zombie test(s): at org.apache.hadoop.hbase.regionserver.TestHRegion.testWritesWhileGetting(TestHRegion.java:3500) Test results: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//testReport/ Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-examples.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-prefix-tree.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-thrift.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-server.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-client.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-hadoop2-compat.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-common.html Findbugs warnings: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//artifact/trunk/patchprocess/newPatchFindbugsWarningshbase-protocol.html Console output: https://builds.apache.org/job/PreCommit-HBASE-Build/9749//console This message is automatically generated.
          Hide
          Jeffrey Zhong added a comment -

          +1 on the v3 patch if you confirm that QA runs fine. Thanks.

          Show
          Jeffrey Zhong added a comment - +1 on the v3 patch if you confirm that QA runs fine. Thanks.
          Hide
          Jimmy Xiang added a comment -

          Integrated into trunk. Thanks Stack, Jeffrey, and Mikhail a lot for the review and discussion.

          Show
          Jimmy Xiang added a comment - Integrated into trunk. Thanks Stack, Jeffrey, and Mikhail a lot for the review and discussion.
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in HBase-TRUNK #5206 (See https://builds.apache.org/job/HBase-TRUNK/5206/)
          HBASE-11059 ZK-less region assignment (jxiang: rev 58549428a66550aed83bfb6f1da405decc1b0f61)

          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestZKLessSplitOnCluster.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java
          • hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStateStore.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/util/ConfigUtil.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerNoMaster.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestZKLessAMOnCluster.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManagerOnCluster.java
          • hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java
          • hbase-protocol/src/main/protobuf/RegionServerStatus.proto
          • hbase-client/src/main/java/org/apache/hadoop/hbase/master/RegionState.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java
          • hbase-client/src/main/java/org/apache/hadoop/hbase/RegionLoad.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScannersFromClientSide.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/CloseRegionHandler.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/OpenRegionHandler.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeTransaction.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/zookeeper/TestRecoverableZooKeeper.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestEndToEndSplitTransaction.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/TestDrainingServer.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/master/UnAssignCallable.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRestartCluster.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java
          • hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignCallable.java
          • hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestZKLessMergeOnCluster.java
          Show
          Hudson added a comment - SUCCESS: Integrated in HBase-TRUNK #5206 (See https://builds.apache.org/job/HBase-TRUNK/5206/ ) HBASE-11059 ZK-less region assignment (jxiang: rev 58549428a66550aed83bfb6f1da405decc1b0f61) hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestZKLessSplitOnCluster.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/MockRegionServer.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStates.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java hbase-client/src/main/java/org/apache/hadoop/hbase/catalog/MetaReader.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterFailover.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/RegionStateStore.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/ServerManager.java hbase-server/src/main/java/org/apache/hadoop/hbase/util/ConfigUtil.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignmentManager.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerNoMaster.java hbase-client/src/main/java/org/apache/hadoop/hbase/HRegionInfo.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/handler/ServerShutdownHandler.java hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperWatcher.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestZKLessAMOnCluster.java hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestHBaseFsck.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManagerOnCluster.java hbase-protocol/src/main/java/org/apache/hadoop/hbase/protobuf/generated/RegionServerStatusProtos.java hbase-protocol/src/main/protobuf/RegionServerStatus.proto hbase-client/src/main/java/org/apache/hadoop/hbase/master/RegionState.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestOpenedRegionHandler.java hbase-client/src/main/java/org/apache/hadoop/hbase/RegionLoad.java hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestScannersFromClientSide.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/CloseRegionHandler.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerServices.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/handler/OpenRegionHandler.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionMergeTransaction.java hbase-server/src/test/java/org/apache/hadoop/hbase/zookeeper/TestRecoverableZooKeeper.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestCloseRegionHandler.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestEndToEndSplitTransaction.java hbase-server/src/test/java/org/apache/hadoop/hbase/TestDrainingServer.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/handler/TestOpenRegionHandler.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/UnAssignCallable.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestZKBasedOpenCloseRegion.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestRestartCluster.java hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestAssignmentManager.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java hbase-server/src/test/java/org/apache/hadoop/hbase/MockRegionServerServices.java hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SplitTransaction.java hbase-server/src/main/java/org/apache/hadoop/hbase/master/AssignCallable.java hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestZKLessMergeOnCluster.java
          Hide
          ramkrishna.s.vasudevan added a comment -

          Jimmy Xiang
          The doc suggests that only the master updates the meta. In the master branch the HMaster itself is am HRS.
          In the OpenRegionHandler we still do updateMeta(). So is that done by the RegionServer or the master?

                if (!updateMeta(region) || this.server.isStopped() ||
                    this.rsServices.isStopping()) {
                  return;
                }
          
          Show
          ramkrishna.s.vasudevan added a comment - Jimmy Xiang The doc suggests that only the master updates the meta. In the master branch the HMaster itself is am HRS. In the OpenRegionHandler we still do updateMeta(). So is that done by the RegionServer or the master? if (!updateMeta(region) || this .server.isStopped() || this .rsServices.isStopping()) { return ; }
          Hide
          Jimmy Xiang added a comment -

          The method name is a little confusing. This method doesn't update meta directly. It sends a RPC call to master to update the meta instead.

          Show
          Jimmy Xiang added a comment - The method name is a little confusing. This method doesn't update meta directly. It sends a RPC call to master to update the meta instead.
          Hide
          ramkrishna.s.vasudevan added a comment -

          Thanks Jimmy. Read further down the code.

          Show
          ramkrishna.s.vasudevan added a comment - Thanks Jimmy. Read further down the code.
          Hide
          Andrew Purtell added a comment -

          Pushed to 0.98 as part of HBASE-11546

          Show
          Andrew Purtell added a comment - Pushed to 0.98 as part of HBASE-11546
          Hide
          Enis Soztutar added a comment -

          Closing this issue after 0.99.0 release.

          Show
          Enis Soztutar added a comment - Closing this issue after 0.99.0 release.

            People

            • Assignee:
              Jimmy Xiang
              Reporter:
              Jimmy Xiang
            • Votes:
              0 Vote for this issue
              Watchers:
              20 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development