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.

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?

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.

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)?

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?

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.

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

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

Some writeup and comments are attached. Thanks.

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?

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

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).

Jimmy Xiang What if you did 1M regions?

Great! Jimmy Xiang - will take a look today.

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

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.

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

Attached v2 which is I am testing now.

-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.

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.

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.

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.

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.

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.

-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.

Jimmy Xiang Is the patch up on rb?

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

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.

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

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.

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?

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?

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.

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.

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.

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.

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?

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

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

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.

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

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.

Nice picture Jimmy.

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.

-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.

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?

-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.

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.

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.

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.

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.

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.

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.

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

I see. Make sense.

-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.

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

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

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

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;
      }

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.

Thanks Jimmy. Read further down the code.

Pushed to 0.98 as part of HBASE-11546

Closing this issue after 0.99.0 release.