You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "rmdmattingly (via GitHub)" <gi...@apache.org> on 2023/11/22 14:53:31 UTC
[PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
rmdmattingly opened a new pull request, #5534:
URL: https://github.com/apache/hbase/pull/5534
https://issues.apache.org/jira/browse/HBASE-28215
The mass reopening of regions caused by a table descriptor modification can be quite disruptive. For latency/error sensitive workloads, like our user facing traffic, we need to be very careful about when we modify table descriptors, and it can be virtually impossible to do it painlessly for busy tables.
This PR introduces two new configurations:
1. hbase.table.regions.reopen.batch.size
* This is an integer which represents the number of reopen procedures to create in a single batch. This defaults to Integer.MAX_VALUE, so it should be a no-op for any table with fewer than billions of regions
3. hbase.table.regions.reopen.batch.backoff.ms
* This is an integer which represents the millis to be waited between reopen batches. Hyperbolically, you could configure your batch size to 1 and your backoff MS to 60_000 — this would result in reopening no more than 1 region per 60s as a result of a single reopening procedure.
@bbeaudreault @hgromer @eab148 @bozzkar
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1409833804
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -133,39 +183,72 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
regionNode.unlock();
}
addChildProcedure(proc);
+ regionsReopened++;
}
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_CONFIRM_REOPENED);
return Flow.HAS_MORE_STATE;
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
- regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
- .filter(l -> l != null).collect(Collectors.toList());
+ // update region lists based on what's been reopened
+ regions = filterReopened(env, regions);
+ currentRegionBatch = filterReopened(env, currentRegionBatch);
+
+ // existing batch didn't fully reopen, so try to resolve that first.
+ // since this is a retry, don't do the batch backoff
+ if (!currentRegionBatch.isEmpty()) {
+ return reopenIfSchedulable(env, currentRegionBatch, false);
+ }
+
if (regions.isEmpty()) {
return Flow.NO_MORE_STATE;
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
- retryCounter = null;
- setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
- return Flow.HAS_MORE_STATE;
- }
- // We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
- // again.
- if (retryCounter == null) {
- retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
- }
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
- LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
- + "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
- throw new ProcedureSuspendedException();
+
+ // current batch is finished, schedule more regions
+ return reopenIfSchedulable(env, regions, true);
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private List<HRegionLocation> filterReopened(MasterProcedureEnv env,
+ List<HRegionLocation> regionsToCheck) {
+ return regionsToCheck.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
+ .filter(l -> l != null).collect(Collectors.toList());
+ }
+
+ private Flow reopenIfSchedulable(MasterProcedureEnv env, List<HRegionLocation> regionsToReopen,
+ boolean shouldBatchBackoff) throws ProcedureSuspendedException {
+ if (regionsToReopen.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ retryCounter = null;
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ reopenBatchSize = Math.min(reopenBatchSizeMax, 2 * reopenBatchSize);
Review Comment:
oh wait, i dont think we want progressive reopen enabled by default right away. i was actually thinking that this progressive batching would only apply if one of the new configs is enabled. i missed that above.
we might want to change our default values so that we can tell if a user actually changed the configs, and only enable progressive if so.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1408061918
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
Review Comment:
This is a little confusing to read, specifically that the way you backoff is by throwing `ProcedureSuspendedException` but below you are returning `Flow.HAS_MORE_STATE`. It would be nice to indicate that the return only happens if we don't back-off. One way to do that would be to add a comment below. But it might be better to rename `backoff` to `setBackoffState()` and throw a ProcedureSuspendedException here. You're already throwing it below (which is redundant given `backoff` throws it now)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1408119641
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
Review Comment:
I think this complexity is necessary in order to differentiate between `are there more regions to reopen` vs `are regions still reopening in the current batch`, one being a state which is considered a retry and exponentially increasing backoff, the other being a state change and fixed backoff
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1832677777
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 36s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 45s | master passed |
| +1 :green_heart: | compile | 0m 48s | master passed |
| +1 :green_heart: | shadedjars | 4m 52s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 43s | the patch passed |
| +1 :green_heart: | compile | 0m 48s | the patch passed |
| +1 :green_heart: | javac | 0m 48s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 51s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 314m 34s | hbase-server in the patch failed. |
| | | 337m 22s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux dd780a61e0ee 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 44eb408a6d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/5/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/5/testReport/ |
| Max. process+thread count | 4453 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/5/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1836963555
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 25s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 38s | master passed |
| +1 :green_heart: | compile | 0m 36s | master passed |
| +1 :green_heart: | shadedjars | 5m 13s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 23s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 17s | the patch passed |
| +1 :green_heart: | compile | 0m 36s | the patch passed |
| +1 :green_heart: | javac | 0m 36s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 7s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 22s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 222m 9s | hbase-server in the patch failed. |
| | | 243m 36s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/8/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux ae8b71ee0239 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/8/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/8/testReport/ |
| Max. process+thread count | 4648 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/8/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1837823062
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 16s | master passed |
| +1 :green_heart: | compile | 0m 41s | master passed |
| +1 :green_heart: | shadedjars | 4m 41s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 24s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 23s | the patch passed |
| +1 :green_heart: | compile | 0m 40s | the patch passed |
| +1 :green_heart: | javac | 0m 40s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 51s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 264m 50s | hbase-server in the patch passed. |
| | | 285m 46s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/12/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 053bac251862 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/12/testReport/ |
| Max. process+thread count | 4482 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/12/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1407632654
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
return Flow.HAS_MORE_STATE;
}
// We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
// again.
if (retryCounter == null) {
retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
}
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
+ long backoffMillis = retryCounter.getBackoffTimeAndIncrementAttempts();
LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
+ "There are still {} region(s) which need to be reopened for table {}. {} are in "
+ "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
+ regions.size(), tableName, currentRegionBatch.size(), backoffMillis / 1000);
+ backoff(backoffMillis);
throw new ProcedureSuspendedException();
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private void backoff(long millis) throws ProcedureSuspendedException {
+ setTimeout(Math.toIntExact(millis));
+ setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
+ skipPersistence();
Review Comment:
Ya so the current region list is serialized into the proto. I think we need to not skip persistence and update the serialization to either remove the current batch or add some sort of offset to the proto. Otherwise let's say this crashed right before finishing -- restarting up, it's have to reopen all of the regions again (in our case often many thousands). If we persist an offset we can pick up right where we left off.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1408061918
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
Review Comment:
This is a little confusing to read. Specifically, the way to backoff is by throwing `ProcedureSuspendedException` but below you are returning `Flow.HAS_MORE_STATE`. It would be nice to indicate that the return only happens if we don't back-off. One way to do that would be to add a comment below. But it might be better to rename `backoff` to `setBackoffState()` and throw a ProcedureSuspendedException here. You're already throwing it below (which is redundant given `backoff` throws it now)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1833262868
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 55s | master passed |
| +1 :green_heart: | compile | 1m 4s | master passed |
| +1 :green_heart: | shadedjars | 5m 25s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 27s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 56s | the patch passed |
| +1 :green_heart: | compile | 1m 0s | the patch passed |
| +1 :green_heart: | javac | 1m 0s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 55s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 225m 27s | hbase-server in the patch passed. |
| | | 249m 42s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/7/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 3f1859d93d54 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 44eb408a6d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/7/testReport/ |
| Max. process+thread count | 4318 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/7/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1409161210
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
Review Comment:
On second thought, after a discussion on https://issues.apache.org/jira/browse/HBASE-25549, i wonder if we should take the safer approach where require each batch to finish before scheduling more. This feature could serve a dual purpose of progressive rollout and rate limiting.
We could also trivially update this code to do an actual progressive deploy -- first batch size 1, then 2, then 4, etc up to the current batch size config you added. At that point it stays at that max concurrency until completion.
Thoughts on adding that? I think we just need one more field for `currentBatchSize` which we increment after each batch up to `reopenBatchSize`. It might make sense to persist this new field in the proto.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1409822211
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -133,39 +183,72 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
regionNode.unlock();
}
addChildProcedure(proc);
+ regionsReopened++;
}
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_CONFIRM_REOPENED);
return Flow.HAS_MORE_STATE;
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
- regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
- .filter(l -> l != null).collect(Collectors.toList());
+ // update region lists based on what's been reopened
+ regions = filterReopened(env, regions);
+ currentRegionBatch = filterReopened(env, currentRegionBatch);
+
+ // existing batch didn't fully reopen, so try to resolve that first.
+ // since this is a retry, don't do the batch backoff
+ if (!currentRegionBatch.isEmpty()) {
+ return reopenIfSchedulable(env, currentRegionBatch, false);
+ }
+
if (regions.isEmpty()) {
return Flow.NO_MORE_STATE;
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
- retryCounter = null;
- setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
- return Flow.HAS_MORE_STATE;
- }
- // We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
- // again.
- if (retryCounter == null) {
- retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
- }
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
- LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
- + "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
- throw new ProcedureSuspendedException();
+
+ // current batch is finished, schedule more regions
+ return reopenIfSchedulable(env, regions, true);
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private List<HRegionLocation> filterReopened(MasterProcedureEnv env,
+ List<HRegionLocation> regionsToCheck) {
+ return regionsToCheck.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
+ .filter(l -> l != null).collect(Collectors.toList());
+ }
+
+ private Flow reopenIfSchedulable(MasterProcedureEnv env, List<HRegionLocation> regionsToReopen,
+ boolean shouldBatchBackoff) throws ProcedureSuspendedException {
+ if (regionsToReopen.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ retryCounter = null;
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ reopenBatchSize = Math.min(reopenBatchSizeMax, 2 * reopenBatchSize);
Review Comment:
i think we should only increase the batch size in the below if block
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1836787833
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 12s | master passed |
| +1 :green_heart: | compile | 2m 57s | master passed |
| +1 :green_heart: | checkstyle | 0m 37s | master passed |
| +1 :green_heart: | spotless | 0m 48s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 30s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 49s | the patch passed |
| +1 :green_heart: | compile | 2m 38s | the patch passed |
| +1 :green_heart: | javac | 2m 38s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 33s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 9m 50s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 45s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 46s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 9s | The patch does not generate ASF License warnings. |
| | | 35m 25s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/8/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 4ca742952943 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 77 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/8/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1823599677
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 23s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 41s | master passed |
| +1 :green_heart: | compile | 2m 23s | master passed |
| +1 :green_heart: | checkstyle | 0m 33s | master passed |
| +1 :green_heart: | spotless | 0m 40s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 23s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 34s | the patch passed |
| +1 :green_heart: | compile | 2m 22s | the patch passed |
| +1 :green_heart: | javac | 2m 22s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 34s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 9m 35s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 39s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 31s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 8s | The patch does not generate ASF License warnings. |
| | | 31m 28s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 43119977ac61 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1203c2014b |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/3/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1823489765
I believe that test failure `TestPrefetchPersistence.testPrefetchPersistence:142->closeStoreFile:148` is unrelated to this changeset
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1407777856
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
return Flow.HAS_MORE_STATE;
}
// We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
// again.
if (retryCounter == null) {
retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
}
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
+ long backoffMillis = retryCounter.getBackoffTimeAndIncrementAttempts();
LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
+ "There are still {} region(s) which need to be reopened for table {}. {} are in "
+ "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
+ regions.size(), tableName, currentRegionBatch.size(), backoffMillis / 1000);
+ backoff(backoffMillis);
throw new ProcedureSuspendedException();
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private void backoff(long millis) throws ProcedureSuspendedException {
+ setTimeout(Math.toIntExact(millis));
+ setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
+ skipPersistence();
Review Comment:
Ok thanks @Apache9 for the helpful context. I just re-read the code, and now I see the checkReopened. I wasn't aware of that, good to know!
In that case, as you say, we don't need persistence. So we can move on to your other feedback on final fields, etc. Thanks!
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1407025651
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
return Flow.HAS_MORE_STATE;
}
// We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
// again.
if (retryCounter == null) {
retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
}
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
+ long backoffMillis = retryCounter.getBackoffTimeAndIncrementAttempts();
LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
+ "There are still {} region(s) which need to be reopened for table {}. {} are in "
+ "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
+ regions.size(), tableName, currentRegionBatch.size(), backoffMillis / 1000);
+ backoff(backoffMillis);
throw new ProcedureSuspendedException();
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private void backoff(long millis) throws ProcedureSuspendedException {
+ setTimeout(Math.toIntExact(millis));
+ setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
+ skipPersistence();
Review Comment:
It is always OK to persist the state, no problem. Skipping persistent is for optimizing the performance, as there is no actual state change when retrying. That's why I say it should be OK to skip persistent here as the reopened flag is not stored in the procedure state.
On the retrying part, checked the code again, you just use a fixed backoff timeout to call the backoff method so it should be OK.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1409885883
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -133,39 +183,72 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
regionNode.unlock();
}
addChildProcedure(proc);
+ regionsReopened++;
}
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_CONFIRM_REOPENED);
return Flow.HAS_MORE_STATE;
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
- regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
- .filter(l -> l != null).collect(Collectors.toList());
+ // update region lists based on what's been reopened
+ regions = filterReopened(env, regions);
+ currentRegionBatch = filterReopened(env, currentRegionBatch);
+
+ // existing batch didn't fully reopen, so try to resolve that first.
+ // since this is a retry, don't do the batch backoff
+ if (!currentRegionBatch.isEmpty()) {
+ return reopenIfSchedulable(env, currentRegionBatch, false);
+ }
+
if (regions.isEmpty()) {
return Flow.NO_MORE_STATE;
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
- retryCounter = null;
- setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
- return Flow.HAS_MORE_STATE;
- }
- // We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
- // again.
- if (retryCounter == null) {
- retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
- }
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
- LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
- + "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
- throw new ProcedureSuspendedException();
+
+ // current batch is finished, schedule more regions
+ return reopenIfSchedulable(env, regions, true);
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private List<HRegionLocation> filterReopened(MasterProcedureEnv env,
+ List<HRegionLocation> regionsToCheck) {
+ return regionsToCheck.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
+ .filter(l -> l != null).collect(Collectors.toList());
+ }
+
+ private Flow reopenIfSchedulable(MasterProcedureEnv env, List<HRegionLocation> regionsToReopen,
+ boolean shouldBatchBackoff) throws ProcedureSuspendedException {
+ if (regionsToReopen.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ retryCounter = null;
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ reopenBatchSize = Math.min(reopenBatchSizeMax, 2 * reopenBatchSize);
Review Comment:
oh interesting, ok that makes sense
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1409817232
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -133,39 +182,66 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
regionNode.unlock();
}
addChildProcedure(proc);
+ regionsReopened++;
}
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_CONFIRM_REOPENED);
return Flow.HAS_MORE_STATE;
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
Review Comment:
Agreed all around about the feedback here, just pushed a simplification that follows this advice almost exactly. The tests are all still passing of course
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1407922652
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -61,20 +69,36 @@ public class ReopenTableRegionsProcedure
private List<HRegionLocation> regions = Collections.emptyList();
+ private List<HRegionLocation> currentRegionBatch = Collections.emptyList();
+
private RetryCounter retryCounter;
+ private final long reopenBatchBackoffMillis;
Review Comment:
yeah good point
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1407617976
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
return Flow.HAS_MORE_STATE;
}
// We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
// again.
if (retryCounter == null) {
retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
}
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
+ long backoffMillis = retryCounter.getBackoffTimeAndIncrementAttempts();
LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
+ "There are still {} region(s) which need to be reopened for table {}. {} are in "
+ "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
+ regions.size(), tableName, currentRegionBatch.size(), backoffMillis / 1000);
+ backoff(backoffMillis);
throw new ProcedureSuspendedException();
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private void backoff(long millis) throws ProcedureSuspendedException {
+ setTimeout(Math.toIntExact(millis));
+ setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
+ skipPersistence();
Review Comment:
I think we need some way to track which regions still have to be restarted after backoff. I don't think we're modifying the proto here yet so agree persistence wouldn't help much yet. But maybe we need to update the proto?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1407876188
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -61,20 +69,36 @@ public class ReopenTableRegionsProcedure
private List<HRegionLocation> regions = Collections.emptyList();
+ private List<HRegionLocation> currentRegionBatch = Collections.emptyList();
+
private RetryCounter retryCounter;
+ private final long reopenBatchBackoffMillis;
Review Comment:
Sounds good. I should probably add these fields to the state data proto too, right?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1830665233
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 54s | master passed |
| +1 :green_heart: | compile | 0m 48s | master passed |
| +1 :green_heart: | shadedjars | 4m 51s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 28s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 43s | the patch passed |
| +1 :green_heart: | compile | 0m 48s | the patch passed |
| +1 :green_heart: | javac | 0m 48s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 52s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 229m 7s | hbase-server in the patch failed. |
| | | 252m 12s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux d40410cb4ca7 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dbfb516a55 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/4/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/4/testReport/ |
| Max. process+thread count | 4502 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/4/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1412589480
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -61,20 +73,43 @@ public class ReopenTableRegionsProcedure
private List<HRegionLocation> regions = Collections.emptyList();
+ private List<HRegionLocation> currentRegionBatch = Collections.emptyList();
+
private RetryCounter retryCounter;
+ private long reopenBatchBackoffMillis;
+ private int reopenBatchSize;
+ private int reopenBatchSizeMax;
+ private long regionsReopened = 0;
+ private long batchesProcessed = 0;
+
public ReopenTableRegionsProcedure() {
- regionNames = Collections.emptyList();
+ this(null);
}
public ReopenTableRegionsProcedure(TableName tableName) {
- this.tableName = tableName;
- this.regionNames = Collections.emptyList();
+ this(tableName, Collections.emptyList());
}
public ReopenTableRegionsProcedure(final TableName tableName, final List<byte[]> regionNames) {
+ this(tableName, regionNames, PROGRESSIVE_BATCH_BACKOFF_MILLIS_DEFAULT,
+ PROGRESSIVE_BATCH_SIZE_MAX_DISABLED);
+ }
+
+ public ReopenTableRegionsProcedure(final TableName tableName, long reopenBatchBackoffMillis,
+ int reopenBatchSizeMax) {
+ this(tableName, Collections.emptyList(), reopenBatchBackoffMillis, reopenBatchSizeMax);
+ }
+
+ public ReopenTableRegionsProcedure(final TableName tableName, final List<byte[]> regionNames,
+ long reopenBatchBackoffMillis, int reopenBatchSizeMax) {
this.tableName = tableName;
this.regionNames = regionNames;
+ this.reopenBatchBackoffMillis = reopenBatchBackoffMillis;
+ this.reopenBatchSize = reopenBatchSizeMax != PROGRESSIVE_BATCH_SIZE_MAX_DISABLED
+ ? 1
+ : PROGRESSIVE_BATCH_SIZE_MAX_DEFAULT_VALUE;
+ this.reopenBatchSizeMax = Math.max(reopenBatchSizeMax, MINIMUM_BATCH_SIZE_MAX);
Review Comment:
sorry, this is now a little confusing. at first i thought this was a bug. if reopenBatchSizeMax was -1, we'd set the batchSize to int.max, and reopenBatchSizeMax to 1. so then below in our batch handling in CONFIRM_REGIONS we'd reset reopenBatchSize to 1 because of the Math.min.... but then I realized, since we use Int.Max it wouldn't go into the code. anyway, it might just be a bit cleaner to do:
```
if (reopenBatchSizeMax == PROGRESSIVE_BATCH_SIZE_MAX_DISABLED) {
this.reopenBatchSize = Integer.MAX_VALUE;
this.reopenBatchSizeMax = Integer.MAX_VALUE;
} else {
this.reopenBatchSize = 1;
this.reopenBatchSizeMax = Math.max(reopenBatchSizeMax, MINIMUM_BATCH_SIZE_MAX);
}
```
And now that I look again below, I actually _do_ think this is a bug. Let's say batch is disabled, and REOPEN_REGIONS tries to reopen all regions at once. But then in CONFIRM_REGIONS some of them did not open. We'd fall into the canSchedule block and `shouldBatchBackoff` would be true. So then we'd do `Math.min(reopenBatchSizeMax, 2 * Integer.Max)`. I actually think the 2 * max would overflow negative and who knows what would happen.
So maybe we should do the clarification above, but we also need to make sure we don't overflow below
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1832550770
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 23s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 15s | master passed |
| +1 :green_heart: | compile | 0m 37s | master passed |
| +1 :green_heart: | shadedjars | 5m 11s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 22s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 17s | the patch passed |
| +1 :green_heart: | compile | 0m 37s | the patch passed |
| +1 :green_heart: | javac | 0m 37s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 9s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 224m 48s | hbase-server in the patch passed. |
| | | 245m 48s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux db41cc9c58a4 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 44eb408a6d |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/5/testReport/ |
| Max. process+thread count | 4651 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/5/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1402830447
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
return Flow.HAS_MORE_STATE;
}
// We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
// again.
if (retryCounter == null) {
retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
}
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
+ long backoffMillis = retryCounter.getBackoffTimeAndIncrementAttempts();
LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
+ "There are still {} region(s) which need to be reopened for table {}. {} are in "
+ "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
+ regions.size(), tableName, currentRegionBatch.size(), backoffMillis / 1000);
+ backoff(backoffMillis);
throw new ProcedureSuspendedException();
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private void backoff(long millis) throws ProcedureSuspendedException {
+ setTimeout(Math.toIntExact(millis));
+ setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
+ skipPersistence();
Review Comment:
This might need more research. I know other examples skip persistence, besides they make no change. But in this case maybe we need persistence so that we save an updated list of regions still needing to reopen
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1402891739
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -61,20 +69,36 @@ public class ReopenTableRegionsProcedure
private List<HRegionLocation> regions = Collections.emptyList();
+ private List<HRegionLocation> currentRegionBatch = Collections.emptyList();
+
private RetryCounter retryCounter;
+ private final long reopenBatchBackoffMillis;
Review Comment:
Procedure should not have final fields which have to be initialized by input. As when loading the procedure from procedure store, we can only initialize them when calling deserialize method...
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
return Flow.HAS_MORE_STATE;
}
// We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
// again.
if (retryCounter == null) {
retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
}
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
+ long backoffMillis = retryCounter.getBackoffTimeAndIncrementAttempts();
LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
+ "There are still {} region(s) which need to be reopened for table {}. {} are in "
+ "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
+ regions.size(), tableName, currentRegionBatch.size(), backoffMillis / 1000);
+ backoff(backoffMillis);
throw new ProcedureSuspendedException();
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private void backoff(long millis) throws ProcedureSuspendedException {
+ setTimeout(Math.toIntExact(millis));
+ setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
+ skipPersistence();
Review Comment:
I think it is OK to skip persistence here as we do not persist the reopen of the region in procedure state, IIRC we will use the openSeqNum to determine whether the region has already been reopened.
But the problem here is that, the old logic is for error retrying, where we can not schedule TRSP for some regions, but here, we are doing throttling, so I do not think we should keep increase the retry number and increase the retry interval while scheduling TRSPs...
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1823575566
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 32s | master passed |
| +1 :green_heart: | compile | 1m 7s | master passed |
| +1 :green_heart: | shadedjars | 7m 43s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 43s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 4s | the patch passed |
| +1 :green_heart: | compile | 1m 7s | the patch passed |
| +1 :green_heart: | javac | 1m 7s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 59s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 34s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 319m 0s | hbase-server in the patch failed. |
| | | 350m 24s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 43de6d9659e6 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1203c2014b |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/2/testReport/ |
| Max. process+thread count | 4386 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/2/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1411456292
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -53,6 +54,16 @@ public class ReopenTableRegionsProcedure
private static final Logger LOG = LoggerFactory.getLogger(ReopenTableRegionsProcedure.class);
+ public static final String PROGRESSIVE_BATCH_BACKOFF_MILLIS_KEY =
+ "hbase.reopen.table.regions.progressive.batch.backoff.ms";
+ public static final long PROGRESSIVE_BATCH_BACKOFF_MILLIS_DEFAULT = 0L;
+ public static final String PROGRESSIVE_BATCH_SIZE_MAX_KEY =
+ "hbase.reopen.table.regions.progressive.batch.size.max";
+ public static final int PROGRESSIVE_BATCH_SIZE_MAX_DEFAULT = Integer.MAX_VALUE;
Review Comment:
Last request :) let's default to -1. That way someone could decide to set the batch size to int max if they just wanted an unlimited progressive deploy (for large tables)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1837042766
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 45s | master passed |
| +1 :green_heart: | compile | 0m 49s | master passed |
| +1 :green_heart: | shadedjars | 4m 54s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 42s | the patch passed |
| +1 :green_heart: | compile | 0m 50s | the patch passed |
| +1 :green_heart: | javac | 0m 50s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 55s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 230m 22s | hbase-server in the patch passed. |
| | | 252m 45s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/9/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 19671a15633b 5.4.0-166-generic #183-Ubuntu SMP Mon Oct 2 11:28:33 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/9/testReport/ |
| Max. process+thread count | 4841 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/9/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache9 (via GitHub)" <gi...@apache.org>.
Apache9 commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1407666801
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
return Flow.HAS_MORE_STATE;
}
// We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
// again.
if (retryCounter == null) {
retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
}
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
+ long backoffMillis = retryCounter.getBackoffTimeAndIncrementAttempts();
LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
+ "There are still {} region(s) which need to be reopened for table {}. {} are in "
+ "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
+ regions.size(), tableName, currentRegionBatch.size(), backoffMillis / 1000);
+ backoff(backoffMillis);
throw new ProcedureSuspendedException();
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private void backoff(long millis) throws ProcedureSuspendedException {
+ setTimeout(Math.toIntExact(millis));
+ setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
+ skipPersistence();
Review Comment:
The flow for ReopenTableRegionsProcedure is
1. Get all regions of the table, and then go to step 2
2. Schedule TRSP to reopen the regions in the list, and then go to step 3
3. (After all the TRSP's are finished) Check whether there are still regions need to be reopened, with RegionStates.checkReopened method, update the regions list. If no regions need to be reopened, we are done. Otherwise, go to step 2.
So in step 3, if we are still in step 3 after suspending, it is OK to not serialize anything, as we rely on the RegionStates.checkReopened to filter out the regions which have already been reopened, not by any procedure states. After master restarts, the region states will be reconstructed by reading meta.
Thanks.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1832933595
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 23s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 22s | master passed |
| +1 :green_heart: | compile | 0m 36s | master passed |
| +1 :green_heart: | shadedjars | 5m 9s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 16s | the patch passed |
| +1 :green_heart: | compile | 0m 35s | the patch passed |
| +1 :green_heart: | javac | 0m 35s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 9s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 223m 52s | hbase-server in the patch passed. |
| | | 244m 46s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/6/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 5327121590e7 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 44eb408a6d |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/6/testReport/ |
| Max. process+thread count | 4644 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/6/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1407617976
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
return Flow.HAS_MORE_STATE;
}
// We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
// again.
if (retryCounter == null) {
retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
}
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
+ long backoffMillis = retryCounter.getBackoffTimeAndIncrementAttempts();
LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
+ "There are still {} region(s) which need to be reopened for table {}. {} are in "
+ "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
+ regions.size(), tableName, currentRegionBatch.size(), backoffMillis / 1000);
+ backoff(backoffMillis);
throw new ProcedureSuspendedException();
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private void backoff(long millis) throws ProcedureSuspendedException {
+ setTimeout(Math.toIntExact(millis));
+ setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
+ skipPersistence();
Review Comment:
I think we need some way to track which regions still have to be restarted after backoff. I don't think we're modifying the proto here yet so agree persistence wouldn't help much yet. But maybe we need to update the proto?
Otherwise in case of master restart, it'll just reopen all regions again?
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1830652731
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 24s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 37s | master passed |
| +1 :green_heart: | compile | 0m 36s | master passed |
| +1 :green_heart: | shadedjars | 5m 11s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 23s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 18s | the patch passed |
| +1 :green_heart: | compile | 0m 37s | the patch passed |
| +1 :green_heart: | javac | 0m 37s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 9s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 223m 33s | hbase-server in the patch passed. |
| | | 245m 6s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 14b7d613e81e 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dbfb516a55 |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/4/testReport/ |
| Max. process+thread count | 4634 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/4/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1822968123
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 27s | master passed |
| +1 :green_heart: | compile | 0m 50s | master passed |
| +1 :green_heart: | shadedjars | 6m 4s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 29s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 1m 28s | root in the patch failed. |
| -1 :x: | compile | 0m 17s | hbase-server in the patch failed. |
| -0 :warning: | javac | 0m 17s | hbase-server in the patch failed. |
| -1 :x: | shadedjars | 3m 48s | patch has 10 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 0m 15s | hbase-server in the patch failed. |
| | | 18m 42s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux a12bb45d8da1 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1203c2014b |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| mvninstall | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-jdk11-hadoop3-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
| shadedjars | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-jdk11-hadoop3-check/output/patch-shadedjars.txt |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/testReport/ |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1822962908
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 34s | master passed |
| +1 :green_heart: | compile | 0m 42s | master passed |
| +1 :green_heart: | shadedjars | 4m 49s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 1m 8s | root in the patch failed. |
| -1 :x: | compile | 0m 15s | hbase-server in the patch failed. |
| -0 :warning: | javac | 0m 15s | hbase-server in the patch failed. |
| -1 :x: | shadedjars | 3m 16s | patch has 10 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 23s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 0m 16s | hbase-server in the patch failed. |
| | | 15m 19s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 73701f566a4a 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1203c2014b |
| Default Java | Temurin-1.8.0_352-b08 |
| mvninstall | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-jdk8-hadoop3-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-hbase-server.txt |
| shadedjars | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-jdk8-hadoop3-check/output/patch-shadedjars.txt |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/testReport/ |
| Max. process+thread count | 64 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1402218181
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,11 +165,29 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ if (reopenBatchBackoffMillis > 0) {
+ Thread.sleep(reopenBatchBackoffMillis);
Review Comment:
I don't think we can sleep within a procedure. There are only a limited number of procedure executors, and this could hold up execution of other procedures.
It's not super well documented, but I think the way to do this is to:
```java
setTimeout(Math.toIntExact(backoff));
setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
throw new ProcedureSuspendedException();
```
Then override setTimeoutFailure:
```java
@Override
protected synchronized boolean setTimeoutFailure(MasterProcedureEnv env) {
setState(ProcedureProtos.ProcedureState.RUNNABLE);
env.getProcedureScheduler().addFront(this);
return false;
}
```
In fact, ReopenTableRegionsProcedure already has some code like this. So maybe you can integrate with it.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1822962791
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 22s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 54s | master passed |
| +1 :green_heart: | compile | 2m 25s | master passed |
| +1 :green_heart: | checkstyle | 0m 34s | master passed |
| +1 :green_heart: | spotless | 0m 42s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 24s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 1m 15s | root in the patch failed. |
| -1 :x: | compile | 0m 13s | hbase-server in the patch failed. |
| -0 :warning: | javac | 0m 13s | hbase-server in the patch failed. |
| -0 :warning: | checkstyle | 0m 32s | hbase-server: The patch generated 4 new + 1 unchanged - 0 fixed = 5 total (was 1) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| -1 :x: | hadoopcheck | 1m 18s | The patch causes 10 errors with Hadoop v3.2.4. |
| -1 :x: | hadoopcheck | 2m 40s | The patch causes 10 errors with Hadoop v3.3.6. |
| -1 :x: | spotless | 0m 32s | patch has 23 errors when running spotless:check, run spotless:apply to fix. |
| -1 :x: | spotbugs | 0m 13s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 9s | The patch does not generate ASF License warnings. |
| | | 15m 13s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux d0b1a06ab9e8 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1203c2014b |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| mvninstall | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-general-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-general-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-general-check/output/patch-compile-hbase-server.txt |
| checkstyle | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| hadoopcheck | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-general-check/output/patch-javac-3.2.4.txt |
| hadoopcheck | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-general-check/output/patch-javac-3.3.6.txt |
| spotless | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-general-check/output/patch-spotless.txt |
| spotbugs | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/artifact/yetus-general-check/output/patch-spotbugs-hbase-server.txt |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/1/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1408070306
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
Review Comment:
One other thing to note -- since we do `skipPersistence()`, we won't persist the setNextState change above. I think this should be fine. The implication is that in the case of a crash/restart while the procedure is suspended, we might backoff twice which is not the end of the world.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1408119641
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
Review Comment:
I think this complexity is necessary in order to differentiate between `are there more regions to reopen` vs `are regions still reopening in the current batch`, the former being a state change and fixed backoff, and the latter being a retry and exponentially increasing backoff
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1409521670
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -133,39 +182,66 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
regionNode.unlock();
}
addChildProcedure(proc);
+ regionsReopened++;
}
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_CONFIRM_REOPENED);
return Flow.HAS_MORE_STATE;
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
Review Comment:
Are you sure we need to do this? What if we did something like this:
```java
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
// update region lists based on what's been reopened
regions = filterReopened(regions);
currentRegionBatch = filterReopened(currentRegionBatch)
// existing batch didn't fully reopen, so try to resolve that first.
// since this is a retry, don't do the batch backoff
if (!currentRegionBatch.isEmpty()) {
return reopenIfSchedulable(currentRegionBatch, false);
}
if (regions.isEmpty()) {
return Flow.NO_MORE_STATE;
}
// no batching or batch is finished, schedule more from main regions
return reopenIfSchedulable(regions, true);
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
...
private List<HRegionLocation> filterReopened(List<HRegionLocation> regionsToCheck) {
return regionsToCheck.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
}
private void reopenIfSchedulable(List<HRegionLocations> regionsToReopen, boolean shouldBatchBackoff) throws ProcedureSuspendedException {
if (regionsToReopen.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
if (shouldBatchBackoff && reopenBatchBackoffMillis > 0) {
reopenBatchSize = Math.min(reopenBatchSizeMax, 2 * reopenBatchSize);
setBackoffStateAndSuspend(reopenBatchBackoffMillis);
} else {
return Flow.HAS_MORE_STATE;
}
}
// We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
// again.
if (retryCounter == null) {
retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
}
long backoffMillis = retryCounter.getBackoffTimeAndIncrementAttempts();
LOG.info(
"There are still {} region(s) which need to be reopened for table {}. {} are in "
+ "OPENING state, suspend {}secs and try again later",
regions.size(), tableName, regionsToCheck.size(), backoffMillis / 1000);
setBackoffStateAndSuspend(backoffMillis);
}
```
It feels cleaner to read and understand what's happening. As mentioned in another comment, I'd recommend updating REOPEN_REGIONS to:
```java
// if we didn't finish reopening the last batch yet, let's keep trying until we do.
// at that point, the batch will be empty and we can generate a new batch
if (currentRegionBatch.isEmpty()) {
currentRegionBatch = regions.stream().limit(reopenBatchSize).collect(Collectors.toList());
}
```
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1409830792
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -53,6 +54,16 @@ public class ReopenTableRegionsProcedure
private static final Logger LOG = LoggerFactory.getLogger(ReopenTableRegionsProcedure.class);
+ public static final String REOPEN_BATCH_BACKOFF_MILLIS_KEY =
+ "hbase.table.regions.reopen.batch.backoff.ms";
+ public static final long REOPEN_BATCH_BACKOFF_MILLIS_DEFAULT = 0L;
+ public static final String REOPEN_BATCH_SIZE_MAX_KEY =
+ "hbase.table.regions.reopen.batch.size.max";
+ public static final int REOPEN_BATCH_SIZE_MAX_DEFAULT = Integer.MAX_VALUE;
Review Comment:
nitpick: we might want to have these names be `hbase.reopen.table.regions.progressive.batch.size.max` and `backoff.ms`. It's more of a progressive reopen system now, and i reordered the table.regions.reopen to align more with the procedure name.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1408119641
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
Review Comment:
I think this complexity is necessary in order to differentiate between `are there more regions to reopen` vs `are there more regions to reopen in our current batch`, one being a state which is considered a retry and exponentially increasing backoff, the other being a state change and fixed backoff
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1832727975
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 45s | master passed |
| +1 :green_heart: | compile | 2m 38s | master passed |
| +1 :green_heart: | checkstyle | 0m 45s | master passed |
| +1 :green_heart: | spotless | 0m 51s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 40s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 18s | the patch passed |
| +1 :green_heart: | compile | 2m 34s | the patch passed |
| +1 :green_heart: | javac | 2m 34s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 33s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 52s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 55s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 2m 11s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 39m 9s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 06aff5e46751 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 44eb408a6d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 80 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/6/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1830280891
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 1s | master passed |
| +1 :green_heart: | compile | 3m 16s | master passed |
| +1 :green_heart: | checkstyle | 0m 47s | master passed |
| +1 :green_heart: | spotless | 0m 49s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 51s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 17s | the patch passed |
| +1 :green_heart: | compile | 2m 46s | the patch passed |
| +1 :green_heart: | javac | 2m 46s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 41s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 10m 37s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 39s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 32s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 7s | The patch does not generate ASF License warnings. |
| | | 37m 12s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 096100cd00f2 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / dbfb516a55 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/4/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1837040443
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 25s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 19s | master passed |
| +1 :green_heart: | compile | 0m 34s | master passed |
| +1 :green_heart: | shadedjars | 5m 5s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 20s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 13s | the patch passed |
| +1 :green_heart: | compile | 0m 36s | the patch passed |
| +1 :green_heart: | javac | 0m 36s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 4s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 219m 36s | hbase-server in the patch passed. |
| | | 240m 18s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/9/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 39c77c676370 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/9/testReport/ |
| Max. process+thread count | 4488 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/9/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1837654302
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 23s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 35s | master passed |
| +1 :green_heart: | compile | 2m 23s | master passed |
| +1 :green_heart: | checkstyle | 0m 33s | master passed |
| +1 :green_heart: | spotless | 0m 39s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 20s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 36s | the patch passed |
| +1 :green_heart: | compile | 2m 26s | the patch passed |
| +1 :green_heart: | javac | 2m 26s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 30s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 9m 38s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 39s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 32s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 7s | The patch does not generate ASF License warnings. |
| | | 31m 17s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/12/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux d6bcc68bd44d 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 80 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/12/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1837641351
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 19s | master passed |
| +1 :green_heart: | compile | 0m 40s | master passed |
| +1 :green_heart: | shadedjars | 4m 43s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 24s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 18s | the patch passed |
| +1 :green_heart: | compile | 0m 40s | the patch passed |
| +1 :green_heart: | javac | 0m 40s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 42s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 23s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 233m 44s | hbase-server in the patch failed. |
| | | 254m 38s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/11/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux bc371e2b77c0 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/11/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/11/testReport/ |
| Max. process+thread count | 4480 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/11/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1837575645
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 41s | master passed |
| +1 :green_heart: | compile | 0m 49s | master passed |
| +1 :green_heart: | shadedjars | 4m 53s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 48s | the patch passed |
| +1 :green_heart: | compile | 0m 50s | the patch passed |
| +1 :green_heart: | javac | 0m 50s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 58s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 231m 16s | hbase-server in the patch passed. |
| | | 253m 55s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/10/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux d7146a52eb9f 5.4.0-166-generic #183-Ubuntu SMP Mon Oct 2 11:28:33 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/10/testReport/ |
| Max. process+thread count | 4909 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/10/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1823483032
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 34s | master passed |
| +1 :green_heart: | compile | 0m 39s | master passed |
| +1 :green_heart: | shadedjars | 4m 44s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 20s | the patch passed |
| +1 :green_heart: | compile | 0m 41s | the patch passed |
| +1 :green_heart: | javac | 0m 41s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 41s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 239m 7s | hbase-server in the patch failed. |
| | | 260m 30s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 027afe8220d2 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1203c2014b |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/2/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/2/testReport/ |
| Max. process+thread count | 4455 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/2/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1823151498
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 28s | master passed |
| +1 :green_heart: | compile | 2m 53s | master passed |
| +1 :green_heart: | checkstyle | 0m 42s | master passed |
| +1 :green_heart: | spotless | 0m 45s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 27s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 42s | the patch passed |
| +1 :green_heart: | compile | 2m 30s | the patch passed |
| +1 :green_heart: | javac | 2m 30s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 33s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 9m 54s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 40s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 33s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 10s | The patch does not generate ASF License warnings. |
| | | 33m 46s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux eff46ea00839 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1203c2014b |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/2/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1832954023
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 50s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 6s | master passed |
| +1 :green_heart: | compile | 1m 12s | master passed |
| +1 :green_heart: | shadedjars | 6m 49s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 32s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 46s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 31s | 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 | 241m 36s | hbase-server in the patch passed. |
| | | 270m 40s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/6/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux a50a91ec2ffd 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 44eb408a6d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/6/testReport/ |
| Max. process+thread count | 4250 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/6/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1409355128
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
Review Comment:
> Now let's look at your new code. I see what you are saying. You are basically doing:
If currentRegionBatch are all reopened, exit or process the next batch
Otherwise, can any of the remaining be scheduled? If not, exponential backoff. If so, process next batch.
I think that's missing a few distinct outcomes of `CONFIRM_REOPENED`, and that's where this complexity comes into play. Here are the possibilities:
1. The current batch has reopened successfully and there are no more regions to produce a next batch, so finish
2. The current batch has reopened successfully and there are more regions to produce a next batch, so set the next state to `REOPEN_REGIONS` and suspend. This will process the next batch
3. The current batch has not reopened successfully, and some of the current batch can still be scheduled. In this case return to `REOPEN_REGIONS` to avoid an infinite loop due to a bypassed proc or something. This could technically reprocess the next batch, or redefine what the "current batch" looks like, but I think that's ok given the exceptional nature of this case
4. The current batch has not reopened successfully, and none of the current batch can still be scheduled (because they have already been scheduled). In this case we're just waiting on TRSPs to complete, so retain current state of `CONFIRM_REOPENED` and retry w/ increasing backoff
If I just revert this diff as described then this procedure will just get stuck in infinite loops (that I think could theoretically be exited if TRSP finishes quickly enough?) and all unit tests fail
> We could also trivially update this code to do an actual progressive deploy -- first batch size 1, then 2, then 4, etc up to the current batch size config you added. At that point it stays at that max concurrency until completion.
I like this idea, and I think it would be trivial to add in the current state of this PR. We'd just need to store a maxReopenBatchSize and increase the reopenBatchSize to Math.min(2*reopenBatchSize, maxReopenBatchSize) each time we enter state 2 above
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1408145896
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
Review Comment:
Let's look back at the original impl of CONFIRM_REOPEN. It only does the backoff if there are still regions to reopen and _none of them are schedulable_. Meaning we back off in order to wait for regions to be schedulable.
Now let's look at your new code. I see what you are saying. You are basically doing:
- If currentRegionBatch are all reopened, exit or process the next batch
- Otherwise, can any of the remaining be scheduled? If not, exponential backoff. If so, _process next batch_.
The key thing there is that "_process next batch_". You'd think that would be "re-schedule existing batch". But as is, when you go to process next batch, the first thing REOPEN_REGIONS does is create a new currentRegionBatch. So lets say for the first batch of 50, only 10 reopen. With the current design, you'd expect it to try again to reopen those remaining 40. But it will actually reopen another 50 (40 of which are likely to have been in the last batch).
TBH I don't think there's a huge problem with that approach. But given that's how it works, we can simplify this code as I described before -- we don't really care about currentRegionBatch here, just that if any regions are left to reopen and schedulable.
If we wanted to do what I think you are intending to do, we probably need to complicate things a bit further. Maybe we update REOPEN_REGIONS to only create currentRegionBatch if it's currently null. Then in CONFIRM_REOPEN, null it out after confirming that they've been reopened. That way, in the above example when we kickoff REOPEN_REGIONS again we'll only open the remaining 40.
Does that make sense? Between these 2 options I might opt for the simpler one where we keep REOPEN_REGIONS as is and remove most of the diff from CONFIRM_REOPEN.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1409395998
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
Review Comment:
Ok lets table the missing outcomes/simplification discussion in that case, since we'll want to keep the current logic for the progressive reopen. We'll also want to do as i said, where in CONFIRM we update currentRegionBatch to filter out the reopened regions, and then in REOPEN we should only create a new currentRegionBatch if the existing isEmpty. That way if there's some problematic region in the batch, we won't move on to the next batch until its been reopened. Right now I think we'd just move on and create a new batch including the problematic region + more?
Once we have that in there, I need to take another look at the logic here. I think it could be a bit clearer, but don't want to make any suggestions until we have the complete impl in place.
In terms of progressive, it'd be nice to start with 1 region but otherwise agree with your Math.min idea.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1409395998
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
Review Comment:
Ok lets table the missing outcomes/simplification discussion in that case, since we'll want to keep the current logic for the progressive reopen. We'll also want to do as i said, where in CONFIRM we update currentRegionBatch to filter out the reopened regions, and then in REOPEN we should only create a new currentRegionBatch if the existing isEmpty.
Once we have that in there, I need to take another look at the logic here. I think it could be a bit clearer, but don't want to make any suggestions until we have the complete impl in place.
In terms of progressive, it'd be nice to start with 1 region but otherwise agree with your Math.min idea.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1412616954
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -61,20 +73,43 @@ public class ReopenTableRegionsProcedure
private List<HRegionLocation> regions = Collections.emptyList();
+ private List<HRegionLocation> currentRegionBatch = Collections.emptyList();
+
private RetryCounter retryCounter;
+ private long reopenBatchBackoffMillis;
+ private int reopenBatchSize;
+ private int reopenBatchSizeMax;
+ private long regionsReopened = 0;
+ private long batchesProcessed = 0;
+
public ReopenTableRegionsProcedure() {
- regionNames = Collections.emptyList();
+ this(null);
}
public ReopenTableRegionsProcedure(TableName tableName) {
- this.tableName = tableName;
- this.regionNames = Collections.emptyList();
+ this(tableName, Collections.emptyList());
}
public ReopenTableRegionsProcedure(final TableName tableName, final List<byte[]> regionNames) {
+ this(tableName, regionNames, PROGRESSIVE_BATCH_BACKOFF_MILLIS_DEFAULT,
+ PROGRESSIVE_BATCH_SIZE_MAX_DISABLED);
+ }
+
+ public ReopenTableRegionsProcedure(final TableName tableName, long reopenBatchBackoffMillis,
+ int reopenBatchSizeMax) {
+ this(tableName, Collections.emptyList(), reopenBatchBackoffMillis, reopenBatchSizeMax);
+ }
+
+ public ReopenTableRegionsProcedure(final TableName tableName, final List<byte[]> regionNames,
+ long reopenBatchBackoffMillis, int reopenBatchSizeMax) {
this.tableName = tableName;
this.regionNames = regionNames;
+ this.reopenBatchBackoffMillis = reopenBatchBackoffMillis;
+ this.reopenBatchSize = reopenBatchSizeMax != PROGRESSIVE_BATCH_SIZE_MAX_DISABLED
+ ? 1
+ : PROGRESSIVE_BATCH_SIZE_MAX_DEFAULT_VALUE;
+ this.reopenBatchSizeMax = Math.max(reopenBatchSizeMax, MINIMUM_BATCH_SIZE_MAX);
Review Comment:
very good catch re: overflow. I've added a test which fails without our overflow protection
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1837583886
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 23s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 39s | master passed |
| +1 :green_heart: | compile | 2m 24s | master passed |
| +1 :green_heart: | checkstyle | 0m 32s | master passed |
| +1 :green_heart: | spotless | 0m 40s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 24s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 38s | the patch passed |
| -1 :x: | compile | 0m 50s | hbase-server in the patch failed. |
| -0 :warning: | javac | 0m 50s | hbase-server in the patch failed. |
| +1 :green_heart: | checkstyle | 0m 32s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 9m 37s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 39s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 30s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 7s | The patch does not generate ASF License warnings. |
| | | 29m 49s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/11/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 819ac7948696 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| compile | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/11/artifact/yetus-general-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/11/artifact/yetus-general-check/output/patch-compile-hbase-server.txt |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/11/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1836998471
The javac warning is legit, will get to that asap. The test failure was for the previous commit I believe
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1407920432
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -61,20 +69,36 @@ public class ReopenTableRegionsProcedure
private List<HRegionLocation> regions = Collections.emptyList();
+ private List<HRegionLocation> currentRegionBatch = Collections.emptyList();
+
private RetryCounter retryCounter;
+ private final long reopenBatchBackoffMillis;
Review Comment:
I don't think so because they're just configuration. Probably preferable for the configuration to update on restart rather than be stuck on what it started with. Maybe see if other procedures take configurations like this
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1836980804
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 25s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 15s | master passed |
| +1 :green_heart: | compile | 2m 49s | master passed |
| +1 :green_heart: | checkstyle | 0m 35s | master passed |
| +1 :green_heart: | spotless | 0m 39s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 23s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 32s | the patch passed |
| -1 :x: | compile | 1m 3s | hbase-server in the patch failed. |
| -0 :warning: | javac | 1m 3s | hbase-server in the patch failed. |
| +1 :green_heart: | checkstyle | 0m 33s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 9m 47s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 39s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 33s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 8s | The patch does not generate ASF License warnings. |
| | | 31m 19s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/9/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 02c6bf96adc1 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| compile | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/9/artifact/yetus-general-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/9/artifact/yetus-general-check/output/patch-compile-hbase-server.txt |
| Max. process+thread count | 76 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/9/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1823752911
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 59s | master passed |
| +1 :green_heart: | compile | 0m 48s | master passed |
| +1 :green_heart: | shadedjars | 4m 58s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 27s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 47s | the patch passed |
| +1 :green_heart: | compile | 0m 50s | the patch passed |
| +1 :green_heart: | javac | 0m 50s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 0s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 27s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 231m 27s | hbase-server in the patch passed. |
| | | 254m 29s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 55123bf1d3fa 5.4.0-166-generic #183-Ubuntu SMP Mon Oct 2 11:28:33 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1203c2014b |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/3/testReport/ |
| Max. process+thread count | 4969 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/3/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1837576527
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 22s | master passed |
| +1 :green_heart: | compile | 0m 41s | master passed |
| +1 :green_heart: | shadedjars | 4m 45s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 24s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 21s | the patch passed |
| +1 :green_heart: | compile | 0m 41s | the patch passed |
| +1 :green_heart: | javac | 0m 41s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 42s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 22s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 235m 35s | hbase-server in the patch passed. |
| | | 256m 40s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/10/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 283b0426202d 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/10/testReport/ |
| Max. process+thread count | 4460 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/10/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1837640995
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 43s | master passed |
| +1 :green_heart: | compile | 0m 49s | master passed |
| +1 :green_heart: | shadedjars | 4m 54s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 42s | the patch passed |
| +1 :green_heart: | compile | 0m 49s | the patch passed |
| +1 :green_heart: | javac | 0m 49s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 55s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 230m 51s | hbase-server in the patch passed. |
| | | 253m 16s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/11/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 2379ea3aeb8e 5.4.0-166-generic #183-Ubuntu SMP Mon Oct 2 11:28:33 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/11/testReport/ |
| Max. process+thread count | 4906 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/11/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault merged PR #5534:
URL: https://github.com/apache/hbase/pull/5534
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1837521403
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 17s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 1s | master passed |
| +1 :green_heart: | compile | 2m 34s | master passed |
| +1 :green_heart: | checkstyle | 0m 38s | master passed |
| +1 :green_heart: | spotless | 0m 49s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 2m 1s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 11s | the patch passed |
| +1 :green_heart: | compile | 2m 54s | the patch passed |
| +1 :green_heart: | javac | 2m 54s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 36s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 10m 30s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 38s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 29s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 8s | The patch does not generate ASF License warnings. |
| | | 36m 34s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/10/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 75bcc4ac2ac9 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 80 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/10/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1837801189
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 44s | master passed |
| +1 :green_heart: | compile | 0m 49s | master passed |
| +1 :green_heart: | shadedjars | 4m 58s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 27s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 43s | the patch passed |
| +1 :green_heart: | compile | 0m 49s | the patch passed |
| +1 :green_heart: | javac | 0m 49s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 56s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 28s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 231m 26s | hbase-server in the patch passed. |
| | | 253m 56s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/12/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux daacd18ae264 5.4.0-166-generic #183-Ubuntu SMP Mon Oct 2 11:28:33 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/12/testReport/ |
| Max. process+thread count | 4861 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/12/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1823753257
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 18s | master passed |
| +1 :green_heart: | compile | 0m 39s | master passed |
| +1 :green_heart: | shadedjars | 4m 42s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 24s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 19s | the patch passed |
| +1 :green_heart: | compile | 0m 41s | the patch passed |
| +1 :green_heart: | javac | 0m 41s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 43s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 23s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 234m 1s | hbase-server in the patch failed. |
| | | 254m 58s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 294530f94d14 5.4.0-163-generic #180-Ubuntu SMP Tue Sep 5 13:21:23 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 1203c2014b |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/3/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/3/testReport/ |
| Max. process+thread count | 4490 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/3/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1406613210
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
return Flow.HAS_MORE_STATE;
}
// We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
// again.
if (retryCounter == null) {
retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
}
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
+ long backoffMillis = retryCounter.getBackoffTimeAndIncrementAttempts();
LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
+ "There are still {} region(s) which need to be reopened for table {}. {} are in "
+ "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
+ regions.size(), tableName, currentRegionBatch.size(), backoffMillis / 1000);
+ backoff(backoffMillis);
throw new ProcedureSuspendedException();
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private void backoff(long millis) throws ProcedureSuspendedException {
+ setTimeout(Math.toIntExact(millis));
+ setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
+ skipPersistence();
Review Comment:
In what event do we rely on the persisted state? Do we rely on it when moving between states, or is it only relevant when recovering from an hmaster roll or something? I agree with @bbeaudreault that we don't reevaluate the `regions` in `REOPEN_TABLE_REGIONS_REOPEN_REGIONS`, so if we rely on the persisted state to decide which regions to transition after a batch backoff then I think this would stick us in an infinite loop. e.g.,:
1. `this.regions = [a, b, c, d, e]`, `batchSize=1`, `batchBackoffMs=1`
2. enter `..._REOPEN_REGIONS` with `this.regions = [a, b, c, d, e]`, derive currentBatch of `a` and kick off TRSP
3. enter `..._CONFIRM_REOPENED`, evaluate `this.regions = [b, c, d, e]` because a has reopened, acknowledge the success by setting current state to `WAITING_TIMEOUT`, next state back to `..._REOPEN_REGIONS` & throw `ProcedureSuspendedException` without persistence of the new this.regions value
4. `..._REOPEN_REGIONS` would be loaded from the persisted state with `this.regions = [a, b, c, d, e]`, so we'd be back at step 2 and destined to repeat this cycle
To avoid this we could just persist our state if the current batch has no regions still in transition. All this said, I feel like the above example misunderstands how proc state is used because otherwise I don't see how [this test](https://github.com/apache/hbase/blob/a3a880439fa553f0b96973bb051884c4a4998ea7/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestReopenTableRegionsProcedureBatchBackoff.java#L74-L88) would pass. It succeeds which pretty clearly indicates the lack of an infinite loop
re:
> I do not think we should keep increase the retry number and increase the retry interval while scheduling TRSPs
We will only increase the retry interval and backoff if our current batch has yet to successfully reopen — the interval will remain fixed and will not increment retry counts if we're successfully moving onto the next batch of regions
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
retryCounter = null;
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
return Flow.HAS_MORE_STATE;
}
// We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
// again.
if (retryCounter == null) {
retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
}
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
+ long backoffMillis = retryCounter.getBackoffTimeAndIncrementAttempts();
LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
+ "There are still {} region(s) which need to be reopened for table {}. {} are in "
+ "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
+ regions.size(), tableName, currentRegionBatch.size(), backoffMillis / 1000);
+ backoff(backoffMillis);
throw new ProcedureSuspendedException();
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private void backoff(long millis) throws ProcedureSuspendedException {
+ setTimeout(Math.toIntExact(millis));
+ setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
+ skipPersistence();
Review Comment:
In what event do we rely on the persisted state? Do we rely on it when moving between states, or is it only relevant when recovering from an hmaster roll or something? I agree with @bbeaudreault that we don't reevaluate the `regions` in `REOPEN_TABLE_REGIONS_REOPEN_REGIONS`, so if we rely on the persisted state to decide which regions to transition after a batch backoff then I think this would stick us in an infinite loop. e.g.,:
1. `this.regions = [a, b, c, d, e]`, `batchSize=1`, `batchBackoffMs=1`
2. enter `..._REOPEN_REGIONS` with `this.regions = [a, b, c, d, e]`, derive currentBatch of `a` and kick off TRSP
3. enter `..._CONFIRM_REOPENED`, evaluate `this.regions = [b, c, d, e]` because a has reopened, acknowledge the success by setting current state to `WAITING_TIMEOUT`, next state back to `..._REOPEN_REGIONS` & throw `ProcedureSuspendedException` without persistence of the new this.regions value
4. `..._REOPEN_REGIONS` would be loaded from the persisted state with `this.regions = [a, b, c, d, e]`, so we'd be back at step 2 and destined to repeat this cycle
To avoid this we could just persist our state if the current batch has no regions still in transition. All this said, I feel like the above example misunderstands how proc state is used because otherwise I don't see how [this test](https://github.com/apache/hbase/blob/a3a880439fa553f0b96973bb051884c4a4998ea7/hbase-server/src/test/java/org/apache/hadoop/hbase/master/procedure/TestReopenTableRegionsProcedureBatchBackoff.java#L74-L88) would pass. It succeeds which pretty clearly indicates the lack of an infinite loop
re:
> I do not think we should keep increase the retry number and increase the retry interval while scheduling TRSPs
We will only increase the retry interval and backoff if our current batch has yet to successfully reopen — the interval will remain fixed and will not increment retry counts if we're successfully moving onto the next batch of regions
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1833078721
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 23s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 34s | master passed |
| +1 :green_heart: | compile | 2m 24s | master passed |
| +1 :green_heart: | checkstyle | 0m 35s | master passed |
| +1 :green_heart: | spotless | 0m 40s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 25s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 39s | the patch passed |
| +1 :green_heart: | compile | 2m 26s | the patch passed |
| +1 :green_heart: | javac | 2m 26s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 32s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 9m 45s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 39s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 31s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 8s | The patch does not generate ASF License warnings. |
| | | 31m 40s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/7/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 5117deced7f3 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 44eb408a6d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 81 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/7/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1833307409
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 54s | master passed |
| +1 :green_heart: | compile | 0m 43s | master passed |
| +1 :green_heart: | shadedjars | 5m 54s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 27s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 39s | the patch passed |
| +1 :green_heart: | compile | 0m 46s | the patch passed |
| +1 :green_heart: | javac | 0m 46s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 51s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 262m 2s | hbase-server in the patch failed. |
| | | 286m 20s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/7/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux f85950c37114 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 44eb408a6d |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/7/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/7/testReport/ |
| Max. process+thread count | 4669 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/7/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1409446005
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
Review Comment:
Sounds good, I just pushed a change which does the following:
1. each reopen proc will start with a batch size of 1
2. the batch size will double after each confirmed batch (until we hit our max)
3. our tests confirm the intended batching behavior, and that each region is only opened once
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1408058012
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -139,33 +170,57 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
.filter(l -> l != null).collect(Collectors.toList());
- if (regions.isEmpty()) {
- return Flow.NO_MORE_STATE;
+ // we need to create a set of region names because the HRegionLocation hashcode is only
+ // based
+ // on the server name
+ Set<byte[]> currentRegionBatchNames = currentRegionBatch.stream()
+ .map(r -> r.getRegion().getRegionName()).collect(Collectors.toSet());
+ currentRegionBatch = regions.stream()
+ .filter(r -> currentRegionBatchNames.contains(r.getRegion().getRegionName()))
+ .collect(Collectors.toList());
+ if (currentRegionBatch.isEmpty()) {
+ if (regions.isEmpty()) {
+ return Flow.NO_MORE_STATE;
+ } else {
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ if (reopenBatchBackoffMillis > 0) {
+ backoff(reopenBatchBackoffMillis);
+ }
+ return Flow.HAS_MORE_STATE;
+ }
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ if (currentRegionBatch.stream().anyMatch(loc -> canSchedule(env, loc))) {
Review Comment:
I think the changes in this method are sort of confusing. I think the division of labor should be:
`REOPEN_TABLE_REGIONS_REOPEN_REGIONS` -- breaks the set of `regions` into a smaller batch, and reopens that batch. The code you have for that looks ok
`REOPEN_TABLE_REGIONS_CONFIRM_REOPENED` -- checks if any regions are still needing reopen. If so, go back to REOPEN_REGIONS. This is how it used to work.
If we totally reverted the changes in REOPEN_TABLE_REGIONS_CONFIRM_REOPENED, I think we'd have successfully broken the full region list into batches and the flow would work, just without backoff. So in this method we really just need to add the backoff.
The backoff as you have it below looks ok at first glance. This is just a long winded way of saying we should drop the highlighted code :)
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1832170061
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 24s | 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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 5s | master passed |
| +1 :green_heart: | compile | 3m 7s | master passed |
| +1 :green_heart: | checkstyle | 0m 36s | master passed |
| +1 :green_heart: | spotless | 0m 49s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 58s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 15s | the patch passed |
| +1 :green_heart: | compile | 2m 54s | the patch passed |
| +1 :green_heart: | javac | 2m 54s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 37s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 10m 40s | Patch does not cause any errors with Hadoop 3.2.4 3.3.6. |
| +1 :green_heart: | spotless | 0m 39s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 33s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 7s | The patch does not generate ASF License warnings. |
| | | 35m 52s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 084ef633e5c9 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May 23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 44eb408a6d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/5/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "rmdmattingly (via GitHub)" <gi...@apache.org>.
rmdmattingly commented on code in PR #5534:
URL: https://github.com/apache/hbase/pull/5534#discussion_r1409823980
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -133,39 +183,72 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
regionNode.unlock();
}
addChildProcedure(proc);
+ regionsReopened++;
}
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_CONFIRM_REOPENED);
return Flow.HAS_MORE_STATE;
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
- regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
- .filter(l -> l != null).collect(Collectors.toList());
+ // update region lists based on what's been reopened
+ regions = filterReopened(env, regions);
+ currentRegionBatch = filterReopened(env, currentRegionBatch);
+
+ // existing batch didn't fully reopen, so try to resolve that first.
+ // since this is a retry, don't do the batch backoff
+ if (!currentRegionBatch.isEmpty()) {
+ return reopenIfSchedulable(env, currentRegionBatch, false);
+ }
+
if (regions.isEmpty()) {
return Flow.NO_MORE_STATE;
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
- retryCounter = null;
- setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
- return Flow.HAS_MORE_STATE;
- }
- // We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
- // again.
- if (retryCounter == null) {
- retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
- }
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
- LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
- + "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
- throw new ProcedureSuspendedException();
+
+ // current batch is finished, schedule more regions
+ return reopenIfSchedulable(env, regions, true);
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private List<HRegionLocation> filterReopened(MasterProcedureEnv env,
+ List<HRegionLocation> regionsToCheck) {
+ return regionsToCheck.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
+ .filter(l -> l != null).collect(Collectors.toList());
+ }
+
+ private Flow reopenIfSchedulable(MasterProcedureEnv env, List<HRegionLocation> regionsToReopen,
+ boolean shouldBatchBackoff) throws ProcedureSuspendedException {
+ if (regionsToReopen.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ retryCounter = null;
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ reopenBatchSize = Math.min(reopenBatchSizeMax, 2 * reopenBatchSize);
Review Comment:
That would mean that the default case of `no batch backoff` is stuck processing single region batches indefinitely
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/ReopenTableRegionsProcedure.java:
##########
@@ -133,39 +183,72 @@ protected Flow executeFromState(MasterProcedureEnv env, ReopenTableRegionsState
regionNode.unlock();
}
addChildProcedure(proc);
+ regionsReopened++;
}
setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_CONFIRM_REOPENED);
return Flow.HAS_MORE_STATE;
case REOPEN_TABLE_REGIONS_CONFIRM_REOPENED:
- regions = regions.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
- .filter(l -> l != null).collect(Collectors.toList());
+ // update region lists based on what's been reopened
+ regions = filterReopened(env, regions);
+ currentRegionBatch = filterReopened(env, currentRegionBatch);
+
+ // existing batch didn't fully reopen, so try to resolve that first.
+ // since this is a retry, don't do the batch backoff
+ if (!currentRegionBatch.isEmpty()) {
+ return reopenIfSchedulable(env, currentRegionBatch, false);
+ }
+
if (regions.isEmpty()) {
return Flow.NO_MORE_STATE;
}
- if (regions.stream().anyMatch(loc -> canSchedule(env, loc))) {
- retryCounter = null;
- setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
- return Flow.HAS_MORE_STATE;
- }
- // We can not schedule TRSP for all the regions need to reopen, wait for a while and retry
- // again.
- if (retryCounter == null) {
- retryCounter = ProcedureUtil.createRetryCounter(env.getMasterConfiguration());
- }
- long backoff = retryCounter.getBackoffTimeAndIncrementAttempts();
- LOG.info(
- "There are still {} region(s) which need to be reopened for table {} are in "
- + "OPENING state, suspend {}secs and try again later",
- regions.size(), tableName, backoff / 1000);
- setTimeout(Math.toIntExact(backoff));
- setState(ProcedureProtos.ProcedureState.WAITING_TIMEOUT);
- skipPersistence();
- throw new ProcedureSuspendedException();
+
+ // current batch is finished, schedule more regions
+ return reopenIfSchedulable(env, regions, true);
default:
throw new UnsupportedOperationException("unhandled state=" + state);
}
}
+ private List<HRegionLocation> filterReopened(MasterProcedureEnv env,
+ List<HRegionLocation> regionsToCheck) {
+ return regionsToCheck.stream().map(env.getAssignmentManager().getRegionStates()::checkReopened)
+ .filter(l -> l != null).collect(Collectors.toList());
+ }
+
+ private Flow reopenIfSchedulable(MasterProcedureEnv env, List<HRegionLocation> regionsToReopen,
+ boolean shouldBatchBackoff) throws ProcedureSuspendedException {
+ if (regionsToReopen.stream().anyMatch(loc -> canSchedule(env, loc))) {
+ retryCounter = null;
+ setNextState(ReopenTableRegionsState.REOPEN_TABLE_REGIONS_REOPEN_REGIONS);
+ reopenBatchSize = Math.min(reopenBatchSizeMax, 2 * reopenBatchSize);
Review Comment:
That would mean that the default case of `reopenBatchBackoffMillis = 0` is stuck processing single region batches indefinitely
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "bbeaudreault (via GitHub)" <gi...@apache.org>.
bbeaudreault commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1836997744
No rush, but looks like test failures are related and new compile/javac warnings
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-28215: region reopen procedure batching/throttling [hbase]
Posted by "Apache-HBase (via GitHub)" <gi...@apache.org>.
Apache-HBase commented on PR #5534:
URL: https://github.com/apache/hbase/pull/5534#issuecomment-1836967457
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 59s | master passed |
| +1 :green_heart: | compile | 0m 49s | master passed |
| +1 :green_heart: | shadedjars | 4m 58s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 28s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 52s | the patch passed |
| +1 :green_heart: | compile | 0m 50s | the patch passed |
| +1 :green_heart: | javac | 0m 50s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 54s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 230m 26s | hbase-server in the patch failed. |
| | | 253m 29s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/8/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/5534 |
| JIRA Issue | HBASE-28215 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux b27d3643db05 5.4.0-166-generic #183-Ubuntu SMP Mon Oct 2 11:28:33 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7dd4d0c532 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/8/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/8/testReport/ |
| Max. process+thread count | 4860 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-5534/8/console |
| versions | git=2.34.1 maven=3.8.6 |
| Powered by | Apache Yetus 0.12.0 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org