You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/07/02 17:43:01 UTC
[GitHub] [hbase] ndimiduk opened a new pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
ndimiduk opened a new pull request #2014:
URL: https://github.com/apache/hbase/pull/2014
… yield when meta is unavailable
One observation from HBASE-24526 is that while meta is unavailable,
other region movement procedures are getting stuck on meta RPCs. Let's
make it so that non-meta transitions check the state of meta before
attempting any RPCs. If meta is known unavailable, release the thread
back to the scheduler.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on a change in pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#discussion_r453174011
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
##########
@@ -200,14 +200,21 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode)
}
}
- private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode) throws IOException {
+ private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode)
+ throws IOException, ProcedureSuspendedException {
ServerName loc = regionNode.getRegionLocation();
if (loc == null) {
LOG.warn("No location specified for {}, jump back to state {} to get one", getRegion(),
RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
return;
}
+ final boolean isMeta = regionNode.getRegionInfo().isMetaRegion();
+ final boolean isMetaAvailable = !env.getAssignmentManager().isMetaRegionInTransition();
+ if (!isMeta && !isMetaAvailable) {
Review comment:
Oh, yes meta should be available by this time. I just saw your stacktrace in Jira. Let me also go through the trace, relevant flow and see what's going on.
One question: you were able to repro this issue easily or this was part of ITBLL run (or live cluster?) and not so easy to repro locally?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on a change in pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#discussion_r449475878
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
##########
@@ -200,14 +200,21 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode)
}
}
- private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode) throws IOException {
+ private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode)
+ throws IOException, ProcedureSuspendedException {
ServerName loc = regionNode.getRegionLocation();
if (loc == null) {
LOG.warn("No location specified for {}, jump back to state {} to get one", getRegion(),
RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
return;
}
+ final boolean isMeta = regionNode.getRegionInfo().isMetaRegion();
+ final boolean isMetaAvailable = !env.getAssignmentManager().isMetaRegionInTransition();
+ if (!isMeta && !isMetaAvailable) {
+ // meta is not assigned yet, so yield
+ throw new ProcedureSuspendedException();
Review comment:
Good to add a message that open region for ${regionNameAsString} is suspended?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#issuecomment-696315779
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] ndimiduk commented on a change in pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#discussion_r492290457
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
##########
@@ -200,14 +200,21 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode)
}
}
- private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode) throws IOException {
+ private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode)
+ throws IOException, ProcedureSuspendedException {
ServerName loc = regionNode.getRegionLocation();
if (loc == null) {
LOG.warn("No location specified for {}, jump back to state {} to get one", getRegion(),
RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
return;
}
+ final boolean isMeta = regionNode.getRegionInfo().isMetaRegion();
+ final boolean isMetaAvailable = !env.getAssignmentManager().isMetaRegionInTransition();
+ if (!isMeta && !isMetaAvailable) {
+ // meta is not assigned yet, so yield
+ throw new ProcedureSuspendedException();
Review comment:
Update the patch to use existing mechanisms in `HMaster` for deciding if meta is assigned and the server hosting it is online.
> So in general, we should have a way to deal with meta update failure(maybe just a retry at procedure level?)and have a smaller timeout on updaing meta operation.
Looks like we deal with meta update failure by aborting the master, at least as of [ee3e2b974b](https://github.com/apache/hbase/blob/ee3e2b974ba0653e0c79d45d6e15fd30b4557167/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java#L221):
```
private void updateRegionLocation(RegionInfo regionInfo, State state, Put put)
throws IOException {
try (Table table = master.getConnection().getTable(TableName.META_TABLE_NAME)) {
table.put(put);
} catch (IOException e) {
// TODO: Revist!!!! Means that if a server is loaded, then we will abort our host!
// In tests we abort the Master!
String msg = String.format("FAILED persisting region=%s state=%s",
regionInfo.getShortNameToLog(), state);
LOG.error(msg, e);
master.abort(msg, e);
throw e;
}
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] ndimiduk commented on a change in pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#discussion_r456725385
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
##########
@@ -200,14 +200,21 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode)
}
}
- private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode) throws IOException {
+ private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode)
+ throws IOException, ProcedureSuspendedException {
ServerName loc = regionNode.getRegionLocation();
if (loc == null) {
LOG.warn("No location specified for {}, jump back to state {} to get one", getRegion(),
RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
return;
}
+ final boolean isMeta = regionNode.getRegionInfo().isMetaRegion();
+ final boolean isMetaAvailable = !env.getAssignmentManager().isMetaRegionInTransition();
+ if (!isMeta && !isMetaAvailable) {
+ // meta is not assigned yet, so yield
+ throw new ProcedureSuspendedException();
Review comment:
I agree, neither this code nor the proposed change provide a satisfactory solution. However, I think if we can avoid turning the TRSP over to the worker thread when we know meta is unavailable, we reduce the window of risk. Another change might alter the configuration of the client that attempts the meta update: instead of retrying for the full loop, it would try a single RPC and if that fails yield the procedure. That way the PEWorker isn't occupied by the default client retry logic.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#issuecomment-696315779
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 7s | Docker mode activated. |
| -0 :warning: | yetus | 0m 6s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ branch-2.3 Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 57s | branch-2.3 passed |
| +1 :green_heart: | compile | 0m 57s | branch-2.3 passed |
| +1 :green_heart: | shadedjars | 5m 28s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | branch-2.3 passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 1m 50s | root in the patch failed. |
| -1 :x: | compile | 0m 37s | hbase-server in the patch failed. |
| -0 :warning: | javac | 0m 37s | hbase-server in the patch failed. |
| -1 :x: | shadedjars | 4m 12s | patch has 16 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 0m 41s | hbase-server in the patch failed. |
| | | 21m 20s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.13 Server=19.03.13 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2014 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 47b1ee2e9f73 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | branch-2.3 / ee3e2b974b |
| Default Java | 1.8.0_232 |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk8-hadoop2-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk8-hadoop2-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk8-hadoop2-check/output/patch-compile-hbase-server.txt |
| shadedjars | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk8-hadoop2-check/output/patch-shadedjars.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk8-hadoop2-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/testReport/ |
| Max. process+thread count | 78 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] ndimiduk commented on a change in pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#discussion_r492290457
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
##########
@@ -200,14 +200,21 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode)
}
}
- private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode) throws IOException {
+ private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode)
+ throws IOException, ProcedureSuspendedException {
ServerName loc = regionNode.getRegionLocation();
if (loc == null) {
LOG.warn("No location specified for {}, jump back to state {} to get one", getRegion(),
RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
return;
}
+ final boolean isMeta = regionNode.getRegionInfo().isMetaRegion();
+ final boolean isMetaAvailable = !env.getAssignmentManager().isMetaRegionInTransition();
+ if (!isMeta && !isMetaAvailable) {
+ // meta is not assigned yet, so yield
+ throw new ProcedureSuspendedException();
Review comment:
Update the patch to use existing mechanisms in `HMaster` for deciding if meta is assigned and the server hosting it is online.
> So in general, we should have a way to deal with meta update failure(maybe just a retry at procedure level?)and have a smaller timeout on updaing meta operation.
Looks like we deal with meta update failure by aborting the master, at least as of [ee3e2b974b](https://github.com/apache/hbase/blob/ee3e2b974ba0653e0c79d45d6e15fd30b4557167/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStateStore.java#L221):
```
private void updateRegionLocation(RegionInfo regionInfo, State state, Put put)
throws IOException {
try (Table table = master.getConnection().getTable(TableName.META_TABLE_NAME)) {
table.put(put);
} catch (IOException e) {
// TODO: Revist!!!! Means that if a server is loaded, then we will abort our host!
// In tests we abort the Master!
String msg = String.format("FAILED persisting region=%s state=%s",
regionInfo.getShortNameToLog(), state);
LOG.error(msg, e);
master.abort(msg, e);
throw e;
}
}
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#discussion_r455880826
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
##########
@@ -200,14 +200,21 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode)
}
}
- private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode) throws IOException {
+ private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode)
+ throws IOException, ProcedureSuspendedException {
ServerName loc = regionNode.getRegionLocation();
if (loc == null) {
LOG.warn("No location specified for {}, jump back to state {} to get one", getRegion(),
RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
return;
}
+ final boolean isMeta = regionNode.getRegionInfo().isMetaRegion();
+ final boolean isMetaAvailable = !env.getAssignmentManager().isMetaRegionInTransition();
+ if (!isMeta && !isMetaAvailable) {
+ // meta is not assigned yet, so yield
+ throw new ProcedureSuspendedException();
Review comment:
This can not solve all the proble, neither the check here, nor the check in waitInitialized. It could always happen that when checking the meta is online,but when you actually write to it, it goes offline... So in general, we should have a way to deal with meta update failure(maybe just a retry at procedure level?)and have a smaller timeout on updaing meta operation.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#issuecomment-696318477
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 13s | Docker mode activated. |
| -0 :warning: | yetus | 0m 7s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ branch-2.3 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 47s | branch-2.3 passed |
| +1 :green_heart: | compile | 1m 13s | branch-2.3 passed |
| +1 :green_heart: | shadedjars | 6m 44s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 45s | hbase-server in branch-2.3 failed. |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 2m 29s | root in the patch failed. |
| -1 :x: | compile | 0m 53s | hbase-server in the patch failed. |
| -0 :warning: | javac | 0m 53s | hbase-server in the patch failed. |
| -1 :x: | shadedjars | 5m 19s | patch has 16 errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 45s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 0m 52s | hbase-server in the patch failed. |
| | | 26m 18s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.13 Server=19.03.13 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2014 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux da9e1d360db0 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | branch-2.3 / ee3e2b974b |
| Default Java | 2020-01-14 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk11-hadoop3-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
| shadedjars | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk11-hadoop3-check/output/patch-shadedjars.txt |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/testReport/ |
| Max. process+thread count | 88 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#issuecomment-653202615
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | Docker mode activated. |
| -0 :warning: | yetus | 0m 7s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ branch-2.3 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 13s | branch-2.3 passed |
| +1 :green_heart: | compile | 1m 2s | branch-2.3 passed |
| +1 :green_heart: | shadedjars | 5m 52s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 42s | hbase-server in branch-2.3 failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 54s | the patch passed |
| +1 :green_heart: | compile | 1m 2s | the patch passed |
| +1 :green_heart: | javac | 1m 3s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 48s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 39s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 126m 29s | hbase-server in the patch passed. |
| | | 152m 15s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2014/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2014 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 40b9e2b1ade8 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | branch-2.3 / 0010fdc90d |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2014/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2014/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2014/1/testReport/ |
| Max. process+thread count | 4040 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2014/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] ndimiduk closed pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
ndimiduk closed pull request #2014:
URL: https://github.com/apache/hbase/pull/2014
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#issuecomment-653158295
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 15s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-2.3 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 1s | branch-2.3 passed |
| +1 :green_heart: | checkstyle | 1m 16s | branch-2.3 passed |
| +1 :green_heart: | spotbugs | 2m 6s | branch-2.3 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 31s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 14s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 30s | Patch does not cause any errors with Hadoop 2.10.0 or 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 14s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 41m 41s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2014/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2014 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux b4b920683b6a 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | branch-2.3 / 0010fdc90d |
| Max. process+thread count | 84 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2014/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] ndimiduk commented on a change in pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#discussion_r453024239
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
##########
@@ -200,14 +200,21 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode)
}
}
- private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode) throws IOException {
+ private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode)
+ throws IOException, ProcedureSuspendedException {
ServerName loc = regionNode.getRegionLocation();
if (loc == null) {
LOG.warn("No location specified for {}, jump back to state {} to get one", getRegion(),
RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
return;
}
+ final boolean isMeta = regionNode.getRegionInfo().isMetaRegion();
+ final boolean isMetaAvailable = !env.getAssignmentManager().isMetaRegionInTransition();
+ if (!isMeta && !isMetaAvailable) {
+ // meta is not assigned yet, so yield
+ throw new ProcedureSuspendedException();
Review comment:
Hmm maybe. This part of the code is not very chatty, and this is kind of an expected scenario. Maybe DEBUG level.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
##########
@@ -200,14 +200,21 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode)
}
}
- private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode) throws IOException {
+ private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode)
+ throws IOException, ProcedureSuspendedException {
ServerName loc = regionNode.getRegionLocation();
if (loc == null) {
LOG.warn("No location specified for {}, jump back to state {} to get one", getRegion(),
RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
return;
}
+ final boolean isMeta = regionNode.getRegionInfo().isMetaRegion();
+ final boolean isMetaAvailable = !env.getAssignmentManager().isMetaRegionInTransition();
+ if (!isMeta && !isMetaAvailable) {
Review comment:
After more reading, I think this approach is wrong. There's already a facility for waiting on meta to be available. My ITBLL experience indicates it doesn't go far enough. I found:
```
@Override
protected boolean waitInitialized(MasterProcedureEnv env) {
if (TableName.isMetaTableName(getTableName())) {
return false;
}
// First we need meta to be loaded, and second, if meta is not online then we will likely to
// fail when updating meta so we wait until it is assigned.
AssignmentManager am = env.getAssignmentManager();
return am.waitMetaLoaded(this) || am.waitMetaAssigned(this, getRegion());
}
```
https://github.com/apache/hbase/blob/809a623/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java#L168-L176
Meta has already been assigned by the time this code executes, it's simply not coming online. Let me poke around some more on the region server side, see if I can make sense from that angle.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#issuecomment-696318668
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 7m 16s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-2.3 Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 1s | branch-2.3 passed |
| +1 :green_heart: | checkstyle | 1m 18s | branch-2.3 passed |
| +1 :green_heart: | spotbugs | 2m 10s | branch-2.3 passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 1m 50s | root in the patch failed. |
| +1 :green_heart: | checkstyle | 1m 14s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| -1 :x: | hadoopcheck | 2m 2s | The patch causes 16 errors with Hadoop v2.10.0. |
| -1 :x: | hadoopcheck | 4m 14s | The patch causes 16 errors with Hadoop v3.1.2. |
| -1 :x: | hadoopcheck | 6m 29s | The patch causes 16 errors with Hadoop v3.2.1. |
| -1 :x: | spotbugs | 0m 32s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 11s | The patch does not generate ASF License warnings. |
| | | 26m 36s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.13 Server=19.03.13 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2014 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 532b38d5f1ef 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | branch-2.3 / ee3e2b974b |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-general-check/output/patch-mvninstall-root.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-general-check/output/patch-javac-2.10.0.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-general-check/output/patch-javac-3.1.2.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-general-check/output/patch-javac-3.2.1.txt |
| spotbugs | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/artifact/yetus-general-check/output/patch-spotbugs-hbase-server.txt |
| Max. process+thread count | 84 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2014/4/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] ndimiduk commented on a change in pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#discussion_r456725385
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
##########
@@ -200,14 +200,21 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode)
}
}
- private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode) throws IOException {
+ private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode)
+ throws IOException, ProcedureSuspendedException {
ServerName loc = regionNode.getRegionLocation();
if (loc == null) {
LOG.warn("No location specified for {}, jump back to state {} to get one", getRegion(),
RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
return;
}
+ final boolean isMeta = regionNode.getRegionInfo().isMetaRegion();
+ final boolean isMetaAvailable = !env.getAssignmentManager().isMetaRegionInTransition();
+ if (!isMeta && !isMetaAvailable) {
+ // meta is not assigned yet, so yield
+ throw new ProcedureSuspendedException();
Review comment:
I agree, neither this code nor the proposed change provide a satisfactory solution. However, I think if we can avoid turning the SRSP over to the worker thread when we know meta is unavailable, we reduce the window of risk. Another change be to alter the configuration of the client that attempts the meta update: instead of retrying for the full loop, it might try a single RPC and if that fails yield the procedure. That way the PEWorker isn't occupied by the default client retry logic.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] ndimiduk commented on pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#issuecomment-653139735
Putting up a PR for eyeballs and buildbot. Not test yet.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#issuecomment-653205985
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | Docker mode activated. |
| -0 :warning: | yetus | 0m 7s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ branch-2.3 Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 34s | branch-2.3 passed |
| +1 :green_heart: | compile | 0m 54s | branch-2.3 passed |
| +1 :green_heart: | shadedjars | 4m 58s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | branch-2.3 passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 12s | the patch passed |
| +1 :green_heart: | compile | 0m 55s | the patch passed |
| +1 :green_heart: | javac | 0m 55s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 2s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 35s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 139m 22s | hbase-server in the patch passed. |
| | | 161m 31s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2014/1/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2014 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 6769bb7e0520 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | branch-2.3 / 0010fdc90d |
| Default Java | 1.8.0_232 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2014/1/testReport/ |
| Max. process+thread count | 4489 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2014/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] ndimiduk commented on a change in pull request #2014: HBASE-24673 TransitionRegionStateProcedure of non-meta regions should…
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #2014:
URL: https://github.com/apache/hbase/pull/2014#discussion_r456725385
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/TransitRegionStateProcedure.java
##########
@@ -200,14 +200,21 @@ private void queueAssign(MasterProcedureEnv env, RegionStateNode regionNode)
}
}
- private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode) throws IOException {
+ private void openRegion(MasterProcedureEnv env, RegionStateNode regionNode)
+ throws IOException, ProcedureSuspendedException {
ServerName loc = regionNode.getRegionLocation();
if (loc == null) {
LOG.warn("No location specified for {}, jump back to state {} to get one", getRegion(),
RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
setNextState(RegionStateTransitionState.REGION_STATE_TRANSITION_GET_ASSIGN_CANDIDATE);
return;
}
+ final boolean isMeta = regionNode.getRegionInfo().isMetaRegion();
+ final boolean isMetaAvailable = !env.getAssignmentManager().isMetaRegionInTransition();
+ if (!isMeta && !isMetaAvailable) {
+ // meta is not assigned yet, so yield
+ throw new ProcedureSuspendedException();
Review comment:
I agree, neither this code nor the proposed change provide a satisfactory solution. However, I think if we can avoid turning the TRSP over to the worker thread when we know meta is unavailable, we reduce the window of risk. Another change be to alter the configuration of the client that attempts the meta update: instead of retrying for the full loop, it might try a single RPC and if that fails yield the procedure. That way the PEWorker isn't occupied by the default client retry logic.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org