You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/07/06 03:50:43 UTC
[GitHub] [hbase] WenFeiYi opened a new pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
WenFeiYi opened a new pull request #2021:
URL: https://github.com/apache/hbase/pull/2021
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] WenFeiYi commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
WenFeiYi commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r459233528
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -58,31 +58,31 @@
protected static final String WAL_ROLL_PERIOD_KEY = "hbase.regionserver.logroll.period";
- protected final ConcurrentMap<WAL, Boolean> walNeedsRoll = new ConcurrentHashMap<>();
+ protected final ConcurrentMap<WAL, RollController> wals = new ConcurrentHashMap<>();
protected final T abortable;
- private volatile long lastRollTime = System.currentTimeMillis();
// Period to roll log.
private final long rollPeriod;
private final int threadWakeFrequency;
// The interval to check low replication on hlog's pipeline
- private long checkLowReplicationInterval;
+ private final long checkLowReplicationInterval;
private volatile boolean running = true;
public void addWAL(WAL wal) {
// check without lock first
- if (walNeedsRoll.containsKey(wal)) {
+ if (wals.containsKey(wal)) {
return;
}
// this is to avoid race between addWAL and requestRollAll.
synchronized (this) {
- if (walNeedsRoll.putIfAbsent(wal, Boolean.FALSE) == null) {
+ if (wals.putIfAbsent(wal, new RollController(wal)) == null) {
wal.registerWALActionsListener(new WALActionsListener() {
@Override
public void logRollRequested(WALActionsListener.RollRequestReason reason) {
// TODO logs will contend with each other here, replace with e.g. DelayedQueue
Review comment:
No, the todo is pre-existing.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-654045181
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | 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 48s | master passed |
| +1 :green_heart: | compile | 0m 57s | master passed |
| +1 :green_heart: | shadedjars | 5m 44s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 29s | the patch passed |
| +1 :green_heart: | compile | 0m 55s | the patch passed |
| +1 :green_heart: | javac | 0m 55s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 35s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 141m 24s | hbase-server in the patch passed. |
| | | 165m 40s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux d7e490663dbe 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 44691ce23c |
| Default Java | 1.8.0_232 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/1/testReport/ |
| Max. process+thread count | 4178 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r457306408
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -249,4 +247,43 @@ public void close() {
running = false;
interrupt();
}
+
+ /**
+ * Independently control the roll of each wal. When use multiwal,
+ * can avoid all wal roll together. see HBASE-24665 for detail
+ */
+ protected class RollController {
+ private final WAL wal;
+ private final AtomicBoolean rollRequest;
+ private long lastRollTime;
+
+ RollController(WAL wal) {
+ this.wal = wal;
+ this.rollRequest = new AtomicBoolean(false);
+ this.lastRollTime = System.currentTimeMillis();
+ }
+
+ public void requestRoll() {
+ this.rollRequest.set(true);
+ }
+
+ public Map<byte[], List<byte[]>> rollWal(long now) throws IOException {
+ this.lastRollTime = now;
+ Map<byte[], List<byte[]>> regionsToFlush = wal.rollWriter(true);
+ this.rollRequest.set(false);
Review comment:
Sorry did not get. How AtomicBoolean is different compared to boolean wrt repeated roll possibility. In the past we used to keep the Boolean state in a Map. Now in this object.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-661025203
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 54s | 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 51s | master passed |
| +1 :green_heart: | compile | 1m 10s | master passed |
| +1 :green_heart: | shadedjars | 6m 29s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 46s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 10s | the patch passed |
| +1 :green_heart: | compile | 1m 21s | the patch passed |
| +1 :green_heart: | javac | 1m 21s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 51s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 44s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 199m 15s | hbase-server in the patch failed. |
| | | 230m 17s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/6/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux b674114857d8 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9b32943697 |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/6/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/6/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/6/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/6/testReport/ |
| Max. process+thread count | 3455 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/6/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-657772616
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 40s | 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 | 5m 21s | master passed |
| +1 :green_heart: | compile | 1m 21s | master passed |
| +1 :green_heart: | shadedjars | 6m 47s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 49s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 15s | the patch passed |
| +1 :green_heart: | compile | 1m 25s | the patch passed |
| +1 :green_heart: | javac | 1m 25s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 32s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 48s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 224m 19s | hbase-server in the patch failed. |
| | | 257m 22s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux bf2bb6b641a6 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3bd54217a0 |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/testReport/ |
| Max. process+thread count | 3537 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-654008844
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | 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 46s | master passed |
| +1 :green_heart: | checkstyle | 1m 4s | master passed |
| +1 :green_heart: | spotbugs | 2m 4s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 27s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 7s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 7s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 52s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| | | 35m 27s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux dfbdc80e3ca0 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 44691ce23c |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn merged pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
anoopsjohn merged pull request #2021:
URL: https://github.com/apache/hbase/pull/2021
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r457920886
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -249,4 +247,43 @@ public void close() {
running = false;
interrupt();
}
+
+ /**
+ * Independently control the roll of each wal. When use multiwal,
+ * can avoid all wal roll together. see HBASE-24665 for detail
+ */
+ protected class RollController {
+ private final WAL wal;
+ private final AtomicBoolean rollRequest;
+ private long lastRollTime;
+
+ RollController(WAL wal) {
+ this.wal = wal;
+ this.rollRequest = new AtomicBoolean(false);
+ this.lastRollTime = System.currentTimeMillis();
+ }
+
+ public void requestRoll() {
+ this.rollRequest.set(true);
+ }
+
+ public Map<byte[], List<byte[]>> rollWal(long now) throws IOException {
+ this.lastRollTime = now;
+ Map<byte[], List<byte[]>> regionsToFlush = wal.rollWriter(true);
+ this.rollRequest.set(false);
Review comment:
Yes. That is happening now also. We intentionally reset the boolean when we start roll itself.
// reset the flag in front to avoid missing roll request before we return from rollWriter.
So there is no change for that right 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r459857809
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -58,31 +58,31 @@
protected static final String WAL_ROLL_PERIOD_KEY = "hbase.regionserver.logroll.period";
- protected final ConcurrentMap<WAL, Boolean> walNeedsRoll = new ConcurrentHashMap<>();
+ protected final ConcurrentMap<WAL, RollController> wals = new ConcurrentHashMap<>();
protected final T abortable;
- private volatile long lastRollTime = System.currentTimeMillis();
// Period to roll log.
private final long rollPeriod;
private final int threadWakeFrequency;
// The interval to check low replication on hlog's pipeline
- private long checkLowReplicationInterval;
+ private final long checkLowReplicationInterval;
private volatile boolean running = true;
public void addWAL(WAL wal) {
// check without lock first
- if (walNeedsRoll.containsKey(wal)) {
+ if (wals.containsKey(wal)) {
return;
}
// this is to avoid race between addWAL and requestRollAll.
synchronized (this) {
- if (walNeedsRoll.putIfAbsent(wal, Boolean.FALSE) == null) {
+ if (wals.putIfAbsent(wal, new RollController(wal)) == null) {
wal.registerWALActionsListener(new WALActionsListener() {
@Override
public void logRollRequested(WALActionsListener.RollRequestReason reason) {
// TODO logs will contend with each other here, replace with e.g. DelayedQueue
synchronized (AbstractWALRoller.this) {
- walNeedsRoll.put(wal, Boolean.TRUE);
+ RollController controller = wals.computeIfAbsent(wal, rc -> new RollController(wal));
Review comment:
Good Q. In fact I also thought when reviewed this. Ideally speaking we should get the addWAL call 1st which will add the instance to the Map. When we get call here the wal should be in the map already. But if u see the cur impl, there is no such contract enforcing. It just add the WAL with True value. So believe while making patch, @WenFeiYi went with similar lines.
We can consider this.. Need to see any chance we get a rollReq before adding.. While RS start, we do some rollReq on WALs.. This introduced some bug in the past. We need to see that closely.. If we can confirm that we can add that contract enforcing and so what u suggested. I would say add a TODO here and raise another issue. This went through multiple cycles of changes. :-) U ok Viraj?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-660985958
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 42s | 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 25s | master passed |
| +1 :green_heart: | compile | 1m 4s | master passed |
| +1 :green_heart: | shadedjars | 7m 30s | 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 25s | the patch passed |
| +1 :green_heart: | compile | 1m 6s | the patch passed |
| +1 :green_heart: | javac | 1m 6s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 3s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 151m 5s | hbase-server in the patch failed. |
| | | 180m 42s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/6/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux a2044920c5f7 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9b32943697 |
| Default Java | 1.8.0_232 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/6/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/6/testReport/ |
| Max. process+thread count | 3726 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/6/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-657464414
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 45s | 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 22s | master passed |
| +1 :green_heart: | compile | 1m 2s | master passed |
| +1 :green_heart: | shadedjars | 6m 22s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 4s | the patch passed |
| +1 :green_heart: | compile | 0m 59s | the patch passed |
| +1 :green_heart: | javac | 0m 59s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 12s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 218m 21s | hbase-server in the patch failed. |
| | | 246m 12s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.9 Server=19.03.9 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 18aa87fd19c2 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 724f0478ed |
| Default Java | 1.8.0_232 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/3/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/3/testReport/ |
| Max. process+thread count | 3088 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/3/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-661907330
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 5s | 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 | 4m 31s | master passed |
| +1 :green_heart: | checkstyle | 1m 25s | master passed |
| +1 :green_heart: | spotbugs | 2m 47s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 54s | the patch passed |
| -0 :warning: | checkstyle | 1m 32s | hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 15m 1s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 51s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 44m 28s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.9 Server=19.03.9 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/8/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 1f20796e215a 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3d270ba6f9 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/8/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 84 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/8/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r456451506
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -249,4 +247,45 @@ public void close() {
running = false;
interrupt();
}
+
+ /**
+ * Independently control the roll of each wal. When use multiwal,
+ * can avoid all wal roll together. see HBASE-24665 for detail
+ */
+ protected class RollController {
+ private final WAL wal;
+ // avoid missing roll request before we return from rollWriter
+ private final AtomicInteger rollRequestCounter;
+ private long lastRollTime;
+
+ RollController(WAL wal) {
+ this.wal = wal;
+ this.rollRequestCounter = new AtomicInteger(0);
+ this.lastRollTime = System.currentTimeMillis();
+ }
+
+ public void requestRoll() {
+ this.rollRequestCounter.incrementAndGet();
Review comment:
Am not sure whether this way of count based thing also correct. Like say 2 times req roll happened before the actual rolling thread picks up this work. Effectively we need to do roll only once right. Even if 2 or more times being asked. But now as per this change it will try do roll again and again.
So a boolean based thing is enough
What we can do is make the requestRoll being synchronized and within rollWal have a synchronized block where you can reset it ad then go ahead with actual roll call.
Or we just need an AtomicBoolean only?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r459256398
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -58,31 +58,31 @@
protected static final String WAL_ROLL_PERIOD_KEY = "hbase.regionserver.logroll.period";
- protected final ConcurrentMap<WAL, Boolean> walNeedsRoll = new ConcurrentHashMap<>();
+ protected final ConcurrentMap<WAL, RollController> wals = new ConcurrentHashMap<>();
Review comment:
This will have all wal entries once the wal instance is created. Not just when it needs a roll. So ya these were wals which needed roll at some point. So wals name also fine IMO.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] WenFeiYi commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
WenFeiYi commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r459238534
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -58,31 +58,31 @@
protected static final String WAL_ROLL_PERIOD_KEY = "hbase.regionserver.logroll.period";
- protected final ConcurrentMap<WAL, Boolean> walNeedsRoll = new ConcurrentHashMap<>();
+ protected final ConcurrentMap<WAL, RollController> wals = new ConcurrentHashMap<>();
protected final T abortable;
- private volatile long lastRollTime = System.currentTimeMillis();
// Period to roll log.
private final long rollPeriod;
private final int threadWakeFrequency;
// The interval to check low replication on hlog's pipeline
- private long checkLowReplicationInterval;
+ private final long checkLowReplicationInterval;
private volatile boolean running = true;
public void addWAL(WAL wal) {
// check without lock first
- if (walNeedsRoll.containsKey(wal)) {
+ if (wals.containsKey(wal)) {
return;
}
// this is to avoid race between addWAL and requestRollAll.
synchronized (this) {
- if (walNeedsRoll.putIfAbsent(wal, Boolean.FALSE) == null) {
+ if (wals.putIfAbsent(wal, new RollController(wal)) == null) {
wal.registerWALActionsListener(new WALActionsListener() {
@Override
public void logRollRequested(WALActionsListener.RollRequestReason reason) {
// TODO logs will contend with each other here, replace with e.g. DelayedQueue
Review comment:
@ramkrish86 the purpose of this PR is to make each wal separate roll when using multiwal. thanks review.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] ramkrish86 commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
ramkrish86 commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r459223780
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -58,31 +58,31 @@
protected static final String WAL_ROLL_PERIOD_KEY = "hbase.regionserver.logroll.period";
- protected final ConcurrentMap<WAL, Boolean> walNeedsRoll = new ConcurrentHashMap<>();
+ protected final ConcurrentMap<WAL, RollController> wals = new ConcurrentHashMap<>();
Review comment:
walRolls? instead of wals?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] WenFeiYi commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
WenFeiYi commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r457965584
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -249,4 +247,43 @@ public void close() {
running = false;
interrupt();
}
+
+ /**
+ * Independently control the roll of each wal. When use multiwal,
+ * can avoid all wal roll together. see HBASE-24665 for detail
+ */
+ protected class RollController {
+ private final WAL wal;
+ private final AtomicBoolean rollRequest;
+ private long lastRollTime;
+
+ RollController(WAL wal) {
+ this.wal = wal;
+ this.rollRequest = new AtomicBoolean(false);
+ this.lastRollTime = System.currentTimeMillis();
+ }
+
+ public void requestRoll() {
+ this.rollRequest.set(true);
+ }
+
+ public Map<byte[], List<byte[]>> rollWal(long now) throws IOException {
+ this.lastRollTime = now;
+ Map<byte[], List<byte[]>> regionsToFlush = wal.rollWriter(true);
+ this.rollRequest.set(false);
Review comment:
yes, I go 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-657731690
: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 | 3m 43s | master passed |
| +1 :green_heart: | compile | 0m 54s | master passed |
| +1 :green_heart: | shadedjars | 5m 37s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 24s | the patch passed |
| +1 :green_heart: | compile | 0m 55s | the patch passed |
| +1 :green_heart: | javac | 0m 55s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 38s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 142m 54s | hbase-server in the patch failed. |
| | | 166m 58s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux ef93c85472c4 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3bd54217a0 |
| Default Java | 1.8.0_232 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/testReport/ |
| Max. process+thread count | 4391 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] WenFeiYi commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
WenFeiYi commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r452658432
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -148,10 +148,9 @@ private void abort(String reason, Throwable cause) {
@Override
public void run() {
while (running) {
- boolean periodic = false;
long now = System.currentTimeMillis();
checkLowReplication(now);
- periodic = (now - this.lastRollTime) > this.rollPeriod;
+ boolean periodic = (now - this.lastRollTime) > this.rollPeriod;
Review comment:
It's like this, I ignored 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r453456133
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -249,4 +244,42 @@ public void close() {
running = false;
interrupt();
}
+
+ /**
+ * Independently control the roll of each wal. When use multiwal,
+ * can avoid all wal roll together. see HBASE-24665 for detail
+ */
+ protected class RollController {
+ private final WAL wal;
+ private boolean isRequestRoll;
+ private long lastRollTime;
+
+ RollController(WAL wal) {
+ this.wal = wal;
+ this.isRequestRoll = false;
+ this.lastRollTime = System.currentTimeMillis();
+ }
+
+ public synchronized void requestRoll() {
+ this.isRequestRoll = true;
+ }
+
+ public synchronized Map<byte[], List<byte[]>> rollWal(long lastRollTime) throws IOException {
Review comment:
This is overall coming good now. Much more clean than what we had before. Thanks
One issue here though. We have rollWal() and requestRoll() under same lock now.. That is not good. The requestRoll should not wait for ongoing roll to be finished. The request call is in hot path.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -115,9 +113,9 @@ protected AbstractWALRoller(String name, Configuration conf, T abortable) {
*/
private void checkLowReplication(long now) {
try {
- for (Entry<WAL, Boolean> entry : walNeedsRoll.entrySet()) {
+ for (Entry<WAL, RollController> entry : wals.entrySet()) {
WAL wal = entry.getKey();
- boolean needRollAlready = entry.getValue();
+ boolean needRollAlready = entry.getValue().isRollRequested();
Review comment:
Previously, when we are going for a roll because of periodic roll, then also we used to keep the value in Map as TRUE right? Means here we checked that also. Because of any reason, the WAL is marked for roll, dont do this check. So here that is missing 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-661051981
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 54s | 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 | 5m 11s | master passed |
| +1 :green_heart: | checkstyle | 1m 36s | master passed |
| +1 :green_heart: | spotbugs | 2m 50s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 24s | the patch passed |
| -0 :warning: | checkstyle | 1m 28s | hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 15m 32s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 38s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 14s | The patch does not generate ASF License warnings. |
| | | 45m 16s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/7/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 60769f4349ba 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / f73b9085ad |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/7/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 84 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/7/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-657388228
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 47s | 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 21s | master passed |
| +1 :green_heart: | checkstyle | 1m 16s | master passed |
| +1 :green_heart: | spotbugs | 2m 20s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 55s | the patch passed |
| -0 :warning: | checkstyle | 1m 16s | hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 55s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| -1 :x: | spotbugs | 2m 56s | hbase-server generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
| | | 39m 45s | |
| Reason | Tests |
|-------:|:------|
| FindBugs | module:hbase-server |
| | Inconsistent synchronization of org.apache.hadoop.hbase.wal.AbstractWALRoller$RollController.lastRollTime; locked 50% of time Unsynchronized access at AbstractWALRoller.java:50% of time Unsynchronized access at AbstractWALRoller.java:[line 278] |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.9 Server=19.03.9 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux b87c149e4bd0 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 724f0478ed |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| spotbugs | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/3/artifact/yetus-general-check/output/new-spotbugs-hbase-server.html |
| Max. process+thread count | 84 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/3/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] ramkrish86 commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
ramkrish86 commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r459384595
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -148,53 +147,49 @@ private void abort(String reason, Throwable cause) {
@Override
public void run() {
while (running) {
- boolean periodic = false;
long now = System.currentTimeMillis();
checkLowReplication(now);
- periodic = (now - this.lastRollTime) > this.rollPeriod;
- if (periodic) {
- // Time for periodic roll, fall through
- LOG.debug("WAL roll period {} ms elapsed", this.rollPeriod);
- } else {
- synchronized (this) {
- if (walNeedsRoll.values().stream().anyMatch(Boolean::booleanValue)) {
- // WAL roll requested, fall through
- LOG.debug("WAL roll requested");
- } else {
- try {
- wait(this.threadWakeFrequency);
- } catch (InterruptedException e) {
- // restore the interrupt state
- Thread.currentThread().interrupt();
- }
- // goto the beginning to check whether again whether we should fall through to roll
- // several WALs, and also check whether we should quit.
- continue;
+ synchronized (this) {
+ if (wals.values().stream().noneMatch(rc -> rc.needsRoll(now))) {
+ try {
+ wait(this.threadWakeFrequency);
+ } catch (InterruptedException e) {
+ // restore the interrupt state
+ Thread.currentThread().interrupt();
}
+ // goto the beginning to check whether again whether we should fall through to roll
+ // several WALs, and also check whether we should quit.
+ continue;
}
}
try {
- this.lastRollTime = System.currentTimeMillis();
- for (Iterator<Entry<WAL, Boolean>> iter = walNeedsRoll.entrySet().iterator(); iter
- .hasNext();) {
- Entry<WAL, Boolean> entry = iter.next();
+ for (Iterator<Entry<WAL, RollController>> iter = wals.entrySet().iterator();
+ iter.hasNext();) {
+ Entry<WAL, RollController> entry = iter.next();
WAL wal = entry.getKey();
- // reset the flag in front to avoid missing roll request before we return from rollWriter.
- walNeedsRoll.put(wal, Boolean.FALSE);
- Map<byte[], List<byte[]>> regionsToFlush = null;
+ RollController controller = entry.getValue();
+ if (controller.isRollRequested()) {
+ // WAL roll requested, fall through
+ LOG.debug("WAL {} roll requested", wal);
+ } else if (controller.needsPeriodicRoll(now)){
+ // Time for periodic roll, fall through
+ LOG.debug("WAL {} roll period {} ms elapsed", wal, this.rollPeriod);
+ } else {
+ continue;
Review comment:
This is they key. We ensure we only roll if one of the condition is met (if either size reach caused a log roll ) or the time elapsed. That is also tracked per wal.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-657668076
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 37s | 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 | 5m 7s | master passed |
| +1 :green_heart: | checkstyle | 1m 25s | master passed |
| +1 :green_heart: | spotbugs | 2m 41s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 30s | the patch passed |
| -0 :warning: | checkstyle | 1m 29s | hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 15m 20s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| -1 :x: | spotbugs | 2m 29s | hbase-server generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
| | | 43m 28s | |
| Reason | Tests |
|-------:|:------|
| FindBugs | module:hbase-server |
| | Inconsistent synchronization of org.apache.hadoop.hbase.wal.AbstractWALRoller$RollController.lastRollTime; locked 50% of time Unsynchronized access at AbstractWALRoller.java:50% of time Unsynchronized access at AbstractWALRoller.java:[line 278] |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 1aa331dc8191 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3bd54217a0 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| spotbugs | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/artifact/yetus-general-check/output/new-spotbugs-hbase-server.html |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/4/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r453212134
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -249,4 +248,35 @@ public void close() {
running = false;
interrupt();
}
+
+ /**
+ * Independently control the roll of each wal. When use multiwal,
+ * can avoid all wal roll together. see HBASE-24665 for detail
+ */
+ protected class RollController {
+ boolean isRequestRoll;
+ long lastRollTime;
+
+ RollController() {
+ this.isRequestRoll = false;
+ this.lastRollTime = System.currentTimeMillis();
+ }
+
+ void requestRoll() {
+ this.isRequestRoll = true;
+ }
+
+ void finishRoll() {
Review comment:
This name is bit confusing. This is not called once roll is finished. We can just call this resetStatus()? Give proper comment that this resets rollReq status as well as lastRollTime. We can pass the ts as param 'lastRollTime' so that this is clear.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -249,4 +248,35 @@ public void close() {
running = false;
interrupt();
}
+
+ /**
+ * Independently control the roll of each wal. When use multiwal,
+ * can avoid all wal roll together. see HBASE-24665 for detail
+ */
+ protected class RollController {
+ boolean isRequestRoll;
+ long lastRollTime;
+
+ RollController() {
+ this.isRequestRoll = false;
+ this.lastRollTime = System.currentTimeMillis();
+ }
+
+ void requestRoll() {
+ this.isRequestRoll = true;
+ }
+
+ void finishRoll() {
+ this.isRequestRoll = false;
+ this.lastRollTime = System.currentTimeMillis();
+ }
+
+ public boolean isRequestRoll() {
Review comment:
isRollRequested() can be the better name?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -249,4 +248,35 @@ public void close() {
running = false;
interrupt();
}
+
+ /**
+ * Independently control the roll of each wal. When use multiwal,
+ * can avoid all wal roll together. see HBASE-24665 for detail
+ */
+ protected class RollController {
+ boolean isRequestRoll;
+ long lastRollTime;
+
+ RollController() {
+ this.isRequestRoll = false;
+ this.lastRollTime = System.currentTimeMillis();
+ }
+
+ void requestRoll() {
+ this.isRequestRoll = true;
+ }
+
+ void finishRoll() {
+ this.isRequestRoll = false;
+ this.lastRollTime = System.currentTimeMillis();
+ }
+
+ public boolean isRequestRoll() {
+ return isRequestRoll;
+ }
+
+ boolean isPeriodRoll(long now) {
Review comment:
A better name ? needsPeriodicRoll ?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -174,18 +170,21 @@ public void run() {
}
}
try {
- this.lastRollTime = System.currentTimeMillis();
- for (Iterator<Entry<WAL, Boolean>> iter = walNeedsRoll.entrySet().iterator(); iter
- .hasNext();) {
- Entry<WAL, Boolean> entry = iter.next();
+ for (Iterator<Entry<WAL, RollController>> iter = walNeedsRoll.entrySet().iterator();
+ iter.hasNext();) {
+ Entry<WAL, RollController> entry = iter.next();
+ RollController controller = entry.getValue();
+ if (!controller.isRequestRoll && !controller.isPeriodRoll(now)) {
+ continue;
+ }
WAL wal = entry.getKey();
// reset the flag in front to avoid missing roll request before we return from rollWriter.
- walNeedsRoll.put(wal, Boolean.FALSE);
+ controller.finishRoll();
Review comment:
Ideally we need this call also within a synchronized block. This is an existing issue.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -174,18 +170,21 @@ public void run() {
}
}
try {
- this.lastRollTime = System.currentTimeMillis();
- for (Iterator<Entry<WAL, Boolean>> iter = walNeedsRoll.entrySet().iterator(); iter
- .hasNext();) {
- Entry<WAL, Boolean> entry = iter.next();
+ for (Iterator<Entry<WAL, RollController>> iter = walNeedsRoll.entrySet().iterator();
+ iter.hasNext();) {
+ Entry<WAL, RollController> entry = iter.next();
+ RollController controller = entry.getValue();
+ if (!controller.isRequestRoll && !controller.isPeriodRoll(now)) {
Review comment:
We can have a single method in RollController which says this WAL needs roll? RollController#needsRoll().
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/LogRoller.java
##########
@@ -66,6 +67,10 @@ protected void scheduleFlush(String encodedRegionName, List<byte[]> families) {
@VisibleForTesting
Map<WAL, Boolean> getWalNeedsRoll() {
Review comment:
This is exposed for test cases only. This is a private class.. So its ok to change the return type. The new RollController gives clear idea whether a wal instance needs roll because of periodic roll or being explicitly asked for. So that is better.
A return type of Map<WAL, RollController>
Any way then we dont need synchronized block. Else, if we have to do as what is being done below in patch, we would need synchronized block
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -148,16 +146,14 @@ private void abort(String reason, Throwable cause) {
@Override
public void run() {
while (running) {
- boolean periodic = false;
long now = System.currentTimeMillis();
checkLowReplication(now);
- periodic = (now - this.lastRollTime) > this.rollPeriod;
- if (periodic) {
+ if (walNeedsRoll.values().stream().anyMatch(rc -> rc.isPeriodRoll(now))) {
// Time for periodic roll, fall through
LOG.debug("WAL roll period {} ms elapsed", this.rollPeriod);
Review comment:
This log is not much value added now as we dont say for which log(s) need this periodic roll. We can clearly say WAL roll period {} elapsed for one of the WAL.
Below we can make sure we log which wal(s) are getting rolled for what purpose
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -58,14 +57,13 @@
protected static final String WAL_ROLL_PERIOD_KEY = "hbase.regionserver.logroll.period";
- protected final ConcurrentMap<WAL, Boolean> walNeedsRoll = new ConcurrentHashMap<>();
+ protected final ConcurrentMap<WAL, RollController> walNeedsRoll = new ConcurrentHashMap<>();
Review comment:
We add all WAL instances into this once it is created. We can just call it wals?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -148,16 +146,14 @@ private void abort(String reason, Throwable cause) {
@Override
public void run() {
while (running) {
- boolean periodic = false;
long now = System.currentTimeMillis();
checkLowReplication(now);
- periodic = (now - this.lastRollTime) > this.rollPeriod;
- if (periodic) {
+ if (walNeedsRoll.values().stream().anyMatch(rc -> rc.isPeriodRoll(now))) {
// Time for periodic roll, fall through
LOG.debug("WAL roll period {} ms elapsed", this.rollPeriod);
} else {
synchronized (this) {
- if (walNeedsRoll.values().stream().anyMatch(Boolean::booleanValue)) {
+ if (walNeedsRoll.values().stream().anyMatch(rc -> rc.isRequestRoll)) {
// WAL roll requested, fall through
LOG.debug("WAL roll requested");
Review comment:
Same as above comment
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] WenFeiYi commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
WenFeiYi commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r457291332
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -249,4 +247,43 @@ public void close() {
running = false;
interrupt();
}
+
+ /**
+ * Independently control the roll of each wal. When use multiwal,
+ * can avoid all wal roll together. see HBASE-24665 for detail
+ */
+ protected class RollController {
+ private final WAL wal;
+ private final AtomicBoolean rollRequest;
+ private long lastRollTime;
+
+ RollController(WAL wal) {
+ this.wal = wal;
+ this.rollRequest = new AtomicBoolean(false);
+ this.lastRollTime = System.currentTimeMillis();
+ }
+
+ public void requestRoll() {
+ this.rollRequest.set(true);
+ }
+
+ public Map<byte[], List<byte[]>> rollWal(long now) throws IOException {
+ this.lastRollTime = now;
+ Map<byte[], List<byte[]>> regionsToFlush = wal.rollWriter(true);
+ this.rollRequest.set(false);
Review comment:
thank you very much! I also noticed this, but in order to avoid repeated roll, so fix to that. if use boolean, roll at most once, the impact can be ignored. Thanks for review
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-661213454
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 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 | 4m 11s | master passed |
| +1 :green_heart: | compile | 1m 0s | master passed |
| +1 :green_heart: | shadedjars | 6m 13s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 50s | the patch passed |
| +1 :green_heart: | compile | 1m 3s | the patch passed |
| +1 :green_heart: | javac | 1m 3s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 10s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 230m 16s | hbase-server in the patch passed. |
| | | 257m 39s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/7/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux c1e361cd7eb7 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / f73b9085ad |
| Default Java | 1.8.0_232 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/7/testReport/ |
| Max. process+thread count | 2685 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/7/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r457991255
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestLogRoller.java
##########
@@ -43,53 +49,104 @@
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestLogRoller.class);
- private static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
+ private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
- private static final int logRollPeriod = 20 * 1000;
+ private static final int LOG_ROLL_PERIOD = 20 * 1000;
+ private static final String LOG_DIR = "WALs";
+ private static final String ARCHIVE_DIR = "archiveWALs";
+ private static final String WAL_PREFIX = "test-log-roller";
+ private static Configuration CONF;
+ private static LogRoller ROLLER;
+ private static Path ROOT_DIR;
+ private static FileSystem FS;
@Before
public void setup() throws Exception {
- TEST_UTIL.getConfiguration().setInt("hbase.regionserver.logroll.period", logRollPeriod);
- TEST_UTIL.startMiniCluster(1);
- TableName name = TableName.valueOf("Test");
- TEST_UTIL.createTable(name, Bytes.toBytes("cf"));
- TEST_UTIL.waitTableAvailable(name);
+ CONF = TEST_UTIL.getConfiguration();
+ CONF.setInt("hbase.regionserver.logroll.period", LOG_ROLL_PERIOD);
+ CONF.setInt(HConstants.THREAD_WAKE_FREQUENCY, 300);
+ ROOT_DIR = TEST_UTIL.getRandomDir();
+ FS = FileSystem.get(CONF);
+ RegionServerServices services = Mockito.mock(RegionServerServices.class);
+ Mockito.when(services.getConfiguration()).thenReturn(CONF);
+ ROLLER = new LogRoller(services);
+ ROLLER.start();
}
@After
public void tearDown() throws Exception {
+ ROLLER.close();
+ FS.close();
TEST_UTIL.shutdownMiniCluster();
}
@Test
public void testRemoveClosedWAL() throws Exception {
- HRegionServer rs = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0);
- Configuration conf = rs.getConfiguration();
- LogRoller logRoller = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0).getWalRoller();
- int originalSize = logRoller.getWalNeedsRoll().size();
- FSHLog wal1 = new FSHLog(rs.getWALFileSystem(), rs.getWALRootDir(),
- AbstractFSWALProvider.getWALDirectoryName(rs.getServerName().getServerName()), conf);
- logRoller.addWAL(wal1);
- FSHLog wal2 = new FSHLog(rs.getWALFileSystem(), rs.getWALRootDir(),
- AbstractFSWALProvider.getWALDirectoryName(rs.getServerName().getServerName()), conf);
- logRoller.addWAL(wal2);
- FSHLog wal3 = new FSHLog(rs.getWALFileSystem(), rs.getWALRootDir(),
- AbstractFSWALProvider.getWALDirectoryName(rs.getServerName().getServerName()), conf);
- logRoller.addWAL(wal3);
-
- assertEquals(originalSize + 3, logRoller.getWalNeedsRoll().size());
- assertTrue(logRoller.getWalNeedsRoll().containsKey(wal1));
-
- wal1.close();
- Thread.sleep(2 * logRollPeriod);
-
- assertEquals(originalSize + 2, logRoller.getWalNeedsRoll().size());
- assertFalse(logRoller.getWalNeedsRoll().containsKey(wal1));
-
- wal2.close();
- wal3.close();
- Thread.sleep(2 * logRollPeriod);
-
- assertEquals(originalSize, logRoller.getWalNeedsRoll().size());
+ assertEquals(0, ROLLER.getWalNeedsRoll().size());
+ for (int i = 1; i <= 3; i++) {
+ FSHLog wal = new FSHLog(FS, ROOT_DIR, LOG_DIR, ARCHIVE_DIR, CONF, null,
+ true, WAL_PREFIX, getWALSuffix(i));
+ ROLLER.addWAL(wal);
+ }
+
+ assertEquals(3, ROLLER.getWalNeedsRoll().size());
+ Iterator<WAL> it = ROLLER.getWalNeedsRoll().keySet().iterator();
+ WAL wal = it.next();
+ assertTrue(ROLLER.getWalNeedsRoll().containsKey(wal));
+
+ wal.close();
+ Thread.sleep(LOG_ROLL_PERIOD + 5000);
+
+ assertEquals(2, ROLLER.getWalNeedsRoll().size());
+ assertFalse(ROLLER.getWalNeedsRoll().containsKey(wal));
+
+ wal = it.next();
+ wal.close();
+ wal = it.next();
+ wal.close();
+ Thread.sleep(LOG_ROLL_PERIOD + 5000);
+
+ assertEquals(0, ROLLER.getWalNeedsRoll().size());
+ }
+
+ /**
+ * verify that each wal roll separately
+ */
+ @Test
+ public void testRequestRollWithMultiWal() throws Exception {
+ // add multiple wal
+ Map<FSHLog, Path> wals = new HashMap<>();
+ for (int i = 1; i <= 3; i++) {
+ FSHLog wal = new FSHLog(FS, ROOT_DIR, LOG_DIR, ARCHIVE_DIR, CONF, null,
+ true, WAL_PREFIX, getWALSuffix(i));
+ wal.init();
+ wals.put(wal, wal.getCurrentFileName());
+ ROLLER.addWAL(wal);
+ Thread.sleep(3000);
Review comment:
We need a 3 sec sleep here? !
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -232,7 +230,7 @@ private boolean isWaiting() {
* @return true if all WAL roll finished
*/
public boolean walRollFinished() {
- return walNeedsRoll.values().stream().allMatch(needRoll -> !needRoll) && isWaiting();
Review comment:
Checking its usage, I think this API impl is already buggy. This just checks the status of the boolean. Once we start a roll on a WAL, we reset the boolean (Even before this patch). So it is not clearly telling anything abt the roll status. This can return true even while an active wal roll is going on. We can keep it as an another jira and fix (if required).. Just add some TODO comments here.
We might need another boolean in Controller which clearly tracks whether we are ongoing a roll. So this really need to check that status as well as a requested roll status.
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestLogRoller.java
##########
@@ -43,53 +49,104 @@
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestLogRoller.class);
- private static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
+ private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
- private static final int logRollPeriod = 20 * 1000;
+ private static final int LOG_ROLL_PERIOD = 20 * 1000;
+ private static final String LOG_DIR = "WALs";
+ private static final String ARCHIVE_DIR = "archiveWALs";
+ private static final String WAL_PREFIX = "test-log-roller";
+ private static Configuration CONF;
+ private static LogRoller ROLLER;
+ private static Path ROOT_DIR;
+ private static FileSystem FS;
@Before
public void setup() throws Exception {
- TEST_UTIL.getConfiguration().setInt("hbase.regionserver.logroll.period", logRollPeriod);
- TEST_UTIL.startMiniCluster(1);
- TableName name = TableName.valueOf("Test");
- TEST_UTIL.createTable(name, Bytes.toBytes("cf"));
- TEST_UTIL.waitTableAvailable(name);
+ CONF = TEST_UTIL.getConfiguration();
+ CONF.setInt("hbase.regionserver.logroll.period", LOG_ROLL_PERIOD);
+ CONF.setInt(HConstants.THREAD_WAKE_FREQUENCY, 300);
+ ROOT_DIR = TEST_UTIL.getRandomDir();
+ FS = FileSystem.get(CONF);
+ RegionServerServices services = Mockito.mock(RegionServerServices.class);
+ Mockito.when(services.getConfiguration()).thenReturn(CONF);
+ ROLLER = new LogRoller(services);
+ ROLLER.start();
}
@After
public void tearDown() throws Exception {
+ ROLLER.close();
+ FS.close();
TEST_UTIL.shutdownMiniCluster();
}
@Test
public void testRemoveClosedWAL() throws Exception {
- HRegionServer rs = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0);
- Configuration conf = rs.getConfiguration();
- LogRoller logRoller = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0).getWalRoller();
- int originalSize = logRoller.getWalNeedsRoll().size();
- FSHLog wal1 = new FSHLog(rs.getWALFileSystem(), rs.getWALRootDir(),
- AbstractFSWALProvider.getWALDirectoryName(rs.getServerName().getServerName()), conf);
- logRoller.addWAL(wal1);
- FSHLog wal2 = new FSHLog(rs.getWALFileSystem(), rs.getWALRootDir(),
- AbstractFSWALProvider.getWALDirectoryName(rs.getServerName().getServerName()), conf);
- logRoller.addWAL(wal2);
- FSHLog wal3 = new FSHLog(rs.getWALFileSystem(), rs.getWALRootDir(),
- AbstractFSWALProvider.getWALDirectoryName(rs.getServerName().getServerName()), conf);
- logRoller.addWAL(wal3);
-
- assertEquals(originalSize + 3, logRoller.getWalNeedsRoll().size());
- assertTrue(logRoller.getWalNeedsRoll().containsKey(wal1));
-
- wal1.close();
- Thread.sleep(2 * logRollPeriod);
-
- assertEquals(originalSize + 2, logRoller.getWalNeedsRoll().size());
- assertFalse(logRoller.getWalNeedsRoll().containsKey(wal1));
-
- wal2.close();
- wal3.close();
- Thread.sleep(2 * logRollPeriod);
-
- assertEquals(originalSize, logRoller.getWalNeedsRoll().size());
+ assertEquals(0, ROLLER.getWalNeedsRoll().size());
+ for (int i = 1; i <= 3; i++) {
+ FSHLog wal = new FSHLog(FS, ROOT_DIR, LOG_DIR, ARCHIVE_DIR, CONF, null,
+ true, WAL_PREFIX, getWALSuffix(i));
+ ROLLER.addWAL(wal);
+ }
+
+ assertEquals(3, ROLLER.getWalNeedsRoll().size());
+ Iterator<WAL> it = ROLLER.getWalNeedsRoll().keySet().iterator();
+ WAL wal = it.next();
+ assertTrue(ROLLER.getWalNeedsRoll().containsKey(wal));
+
+ wal.close();
+ Thread.sleep(LOG_ROLL_PERIOD + 5000);
+
+ assertEquals(2, ROLLER.getWalNeedsRoll().size());
+ assertFalse(ROLLER.getWalNeedsRoll().containsKey(wal));
+
+ wal = it.next();
+ wal.close();
+ wal = it.next();
+ wal.close();
+ Thread.sleep(LOG_ROLL_PERIOD + 5000);
+
+ assertEquals(0, ROLLER.getWalNeedsRoll().size());
+ }
+
+ /**
+ * verify that each wal roll separately
+ */
+ @Test
+ public void testRequestRollWithMultiWal() throws Exception {
+ // add multiple wal
+ Map<FSHLog, Path> wals = new HashMap<>();
+ for (int i = 1; i <= 3; i++) {
+ FSHLog wal = new FSHLog(FS, ROOT_DIR, LOG_DIR, ARCHIVE_DIR, CONF, null,
+ true, WAL_PREFIX, getWALSuffix(i));
+ wal.init();
+ wals.put(wal, wal.getCurrentFileName());
+ ROLLER.addWAL(wal);
+ Thread.sleep(3000);
Review comment:
We want 3 WALs will different periodic roll time period? Why we need? If so, can we limit the time to be 1 sec or lesser?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -232,7 +230,7 @@ private boolean isWaiting() {
* @return true if all WAL roll finished
*/
public boolean walRollFinished() {
- return walNeedsRoll.values().stream().allMatch(needRoll -> !needRoll) && isWaiting();
+ return wals.values().stream().noneMatch(RollController::isRollRequested) && isWaiting();
Review comment:
Here rather than isRollRequested, we need needsRoll?
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestLogRoller.java
##########
@@ -43,53 +49,104 @@
public static final HBaseClassTestRule CLASS_RULE =
HBaseClassTestRule.forClass(TestLogRoller.class);
- private static HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
+ private static final HBaseTestingUtility TEST_UTIL = new HBaseTestingUtility();
- private static final int logRollPeriod = 20 * 1000;
+ private static final int LOG_ROLL_PERIOD = 20 * 1000;
+ private static final String LOG_DIR = "WALs";
+ private static final String ARCHIVE_DIR = "archiveWALs";
+ private static final String WAL_PREFIX = "test-log-roller";
+ private static Configuration CONF;
+ private static LogRoller ROLLER;
+ private static Path ROOT_DIR;
+ private static FileSystem FS;
@Before
public void setup() throws Exception {
- TEST_UTIL.getConfiguration().setInt("hbase.regionserver.logroll.period", logRollPeriod);
- TEST_UTIL.startMiniCluster(1);
- TableName name = TableName.valueOf("Test");
- TEST_UTIL.createTable(name, Bytes.toBytes("cf"));
- TEST_UTIL.waitTableAvailable(name);
+ CONF = TEST_UTIL.getConfiguration();
+ CONF.setInt("hbase.regionserver.logroll.period", LOG_ROLL_PERIOD);
+ CONF.setInt(HConstants.THREAD_WAKE_FREQUENCY, 300);
+ ROOT_DIR = TEST_UTIL.getRandomDir();
+ FS = FileSystem.get(CONF);
+ RegionServerServices services = Mockito.mock(RegionServerServices.class);
+ Mockito.when(services.getConfiguration()).thenReturn(CONF);
+ ROLLER = new LogRoller(services);
+ ROLLER.start();
}
@After
public void tearDown() throws Exception {
+ ROLLER.close();
+ FS.close();
TEST_UTIL.shutdownMiniCluster();
}
@Test
public void testRemoveClosedWAL() throws Exception {
- HRegionServer rs = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0);
- Configuration conf = rs.getConfiguration();
- LogRoller logRoller = TEST_UTIL.getMiniHBaseCluster().getRegionServer(0).getWalRoller();
- int originalSize = logRoller.getWalNeedsRoll().size();
- FSHLog wal1 = new FSHLog(rs.getWALFileSystem(), rs.getWALRootDir(),
- AbstractFSWALProvider.getWALDirectoryName(rs.getServerName().getServerName()), conf);
- logRoller.addWAL(wal1);
- FSHLog wal2 = new FSHLog(rs.getWALFileSystem(), rs.getWALRootDir(),
- AbstractFSWALProvider.getWALDirectoryName(rs.getServerName().getServerName()), conf);
- logRoller.addWAL(wal2);
- FSHLog wal3 = new FSHLog(rs.getWALFileSystem(), rs.getWALRootDir(),
- AbstractFSWALProvider.getWALDirectoryName(rs.getServerName().getServerName()), conf);
- logRoller.addWAL(wal3);
-
- assertEquals(originalSize + 3, logRoller.getWalNeedsRoll().size());
- assertTrue(logRoller.getWalNeedsRoll().containsKey(wal1));
-
- wal1.close();
- Thread.sleep(2 * logRollPeriod);
-
- assertEquals(originalSize + 2, logRoller.getWalNeedsRoll().size());
- assertFalse(logRoller.getWalNeedsRoll().containsKey(wal1));
-
- wal2.close();
- wal3.close();
- Thread.sleep(2 * logRollPeriod);
-
- assertEquals(originalSize, logRoller.getWalNeedsRoll().size());
+ assertEquals(0, ROLLER.getWalNeedsRoll().size());
+ for (int i = 1; i <= 3; i++) {
+ FSHLog wal = new FSHLog(FS, ROOT_DIR, LOG_DIR, ARCHIVE_DIR, CONF, null,
+ true, WAL_PREFIX, getWALSuffix(i));
+ ROLLER.addWAL(wal);
+ }
+
+ assertEquals(3, ROLLER.getWalNeedsRoll().size());
+ Iterator<WAL> it = ROLLER.getWalNeedsRoll().keySet().iterator();
+ WAL wal = it.next();
+ assertTrue(ROLLER.getWalNeedsRoll().containsKey(wal));
+
+ wal.close();
+ Thread.sleep(LOG_ROLL_PERIOD + 5000);
+
+ assertEquals(2, ROLLER.getWalNeedsRoll().size());
+ assertFalse(ROLLER.getWalNeedsRoll().containsKey(wal));
+
+ wal = it.next();
+ wal.close();
+ wal = it.next();
+ wal.close();
+ Thread.sleep(LOG_ROLL_PERIOD + 5000);
+
+ assertEquals(0, ROLLER.getWalNeedsRoll().size());
+ }
+
+ /**
+ * verify that each wal roll separately
+ */
+ @Test
+ public void testRequestRollWithMultiWal() throws Exception {
+ // add multiple wal
+ Map<FSHLog, Path> wals = new HashMap<>();
+ for (int i = 1; i <= 3; i++) {
+ FSHLog wal = new FSHLog(FS, ROOT_DIR, LOG_DIR, ARCHIVE_DIR, CONF, null,
+ true, WAL_PREFIX, getWALSuffix(i));
+ wal.init();
+ wals.put(wal, wal.getCurrentFileName());
+ ROLLER.addWAL(wal);
+ Thread.sleep(3000);
+ }
+
+ // request roll
+ Iterator<Map.Entry<FSHLog, Path>> it = wals.entrySet().iterator();
+ Map.Entry<FSHLog, Path> walEntry = it.next();
+ walEntry.getKey().requestLogRoll();
+ Thread.sleep(5000);
+
+ assertNotEquals(walEntry.getValue(), walEntry.getKey().getCurrentFileName());
+ walEntry.setValue(walEntry.getKey().getCurrentFileName());
+ while (it.hasNext()) {
+ walEntry = it.next();
+ assertEquals(walEntry.getValue(), walEntry.getKey().getCurrentFileName());
Review comment:
Good...
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -148,53 +147,52 @@ private void abort(String reason, Throwable cause) {
@Override
public void run() {
while (running) {
- boolean periodic = false;
long now = System.currentTimeMillis();
checkLowReplication(now);
- periodic = (now - this.lastRollTime) > this.rollPeriod;
- if (periodic) {
- // Time for periodic roll, fall through
- LOG.debug("WAL roll period {} ms elapsed", this.rollPeriod);
- } else {
- synchronized (this) {
- if (walNeedsRoll.values().stream().anyMatch(Boolean::booleanValue)) {
- // WAL roll requested, fall through
- LOG.debug("WAL roll requested");
- } else {
- try {
- wait(this.threadWakeFrequency);
- } catch (InterruptedException e) {
- // restore the interrupt state
- Thread.currentThread().interrupt();
- }
- // goto the beginning to check whether again whether we should fall through to roll
- // several WALs, and also check whether we should quit.
- continue;
+ synchronized (this) {
+ if (wals.values().stream().noneMatch(rc -> rc.needsRoll(now))) {
+ try {
+ wait(this.threadWakeFrequency);
+ } catch (InterruptedException e) {
+ // restore the interrupt state
+ Thread.currentThread().interrupt();
}
+ // goto the beginning to check whether again whether we should fall through to roll
+ // several WALs, and also check whether we should quit.
+ continue;
}
}
try {
- this.lastRollTime = System.currentTimeMillis();
- for (Iterator<Entry<WAL, Boolean>> iter = walNeedsRoll.entrySet().iterator(); iter
- .hasNext();) {
- Entry<WAL, Boolean> entry = iter.next();
+ for (Iterator<Entry<WAL, RollController>> iter = wals.entrySet().iterator();
+ iter.hasNext();) {
+ Entry<WAL, RollController> entry = iter.next();
WAL wal = entry.getKey();
- // reset the flag in front to avoid missing roll request before we return from rollWriter.
- walNeedsRoll.put(wal, Boolean.FALSE);
- Map<byte[], List<byte[]>> regionsToFlush = null;
+ RollController controller = entry.getValue();
+ boolean isRequestRoll;
Review comment:
Seems this boolean is not needed now. Can u pls delete
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r459628419
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -58,31 +58,31 @@
protected static final String WAL_ROLL_PERIOD_KEY = "hbase.regionserver.logroll.period";
- protected final ConcurrentMap<WAL, Boolean> walNeedsRoll = new ConcurrentHashMap<>();
+ protected final ConcurrentMap<WAL, RollController> wals = new ConcurrentHashMap<>();
protected final T abortable;
- private volatile long lastRollTime = System.currentTimeMillis();
// Period to roll log.
private final long rollPeriod;
private final int threadWakeFrequency;
// The interval to check low replication on hlog's pipeline
- private long checkLowReplicationInterval;
+ private final long checkLowReplicationInterval;
private volatile boolean running = true;
public void addWAL(WAL wal) {
// check without lock first
- if (walNeedsRoll.containsKey(wal)) {
+ if (wals.containsKey(wal)) {
return;
}
// this is to avoid race between addWAL and requestRollAll.
synchronized (this) {
- if (walNeedsRoll.putIfAbsent(wal, Boolean.FALSE) == null) {
+ if (wals.putIfAbsent(wal, new RollController(wal)) == null) {
wal.registerWALActionsListener(new WALActionsListener() {
@Override
public void logRollRequested(WALActionsListener.RollRequestReason reason) {
// TODO logs will contend with each other here, replace with e.g. DelayedQueue
synchronized (AbstractWALRoller.this) {
- walNeedsRoll.put(wal, Boolean.TRUE);
+ RollController controller = wals.computeIfAbsent(wal, rc -> new RollController(wal));
Review comment:
We have already done `wals.putIfAbsent(wal, new RollController(wal))` above.
Hence, `wals.computeIfAbsent()` is needed here? Should we not directly get the value with `RollController controller = wals.get(wal)` and expect non-null object?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-656686420
: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 | 4m 24s | master passed |
| +1 :green_heart: | compile | 1m 6s | master passed |
| +1 :green_heart: | shadedjars | 5m 59s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 44s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 6s | the patch passed |
| +1 :green_heart: | compile | 1m 6s | the patch passed |
| +1 :green_heart: | javac | 1m 6s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 50s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 41s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 131m 34s | hbase-server in the patch passed. |
| | | 158m 16s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 223861dcd838 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 67dfbe0406 |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/2/testReport/ |
| Max. process+thread count | 4218 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/2/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-656632846
: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 | 3m 50s | master passed |
| +1 :green_heart: | checkstyle | 1m 7s | master passed |
| +1 :green_heart: | spotbugs | 2m 5s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 23s | the patch passed |
| -0 :warning: | checkstyle | 1m 2s | hbase-server: The patch generated 2 new + 0 unchanged - 0 fixed = 2 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 19s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 59s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
| | | 35m 40s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux d6e713866427 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 67dfbe0406 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/2/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r451688123
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -148,10 +148,9 @@ private void abort(String reason, Throwable cause) {
@Override
public void run() {
while (running) {
- boolean periodic = false;
long now = System.currentTimeMillis();
checkLowReplication(now);
- periodic = (now - this.lastRollTime) > this.rollPeriod;
+ boolean periodic = (now - this.lastRollTime) > this.rollPeriod;
Review comment:
Not on this patch directly.
We have the periodic WAL roll.. Every WAL should get rolled as per this period. When one WAL gets rolled, we will change the lastRollTime and so it can happen that some WALs will ever get rolled!
This issue will be visible now as we will selectively roll WAL files after this patch. Actually we need to track the lastRollTime per WAL instance.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] ramkrish86 commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
ramkrish86 commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r459224188
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -58,31 +58,31 @@
protected static final String WAL_ROLL_PERIOD_KEY = "hbase.regionserver.logroll.period";
- protected final ConcurrentMap<WAL, Boolean> walNeedsRoll = new ConcurrentHashMap<>();
+ protected final ConcurrentMap<WAL, RollController> wals = new ConcurrentHashMap<>();
protected final T abortable;
- private volatile long lastRollTime = System.currentTimeMillis();
// Period to roll log.
private final long rollPeriod;
private final int threadWakeFrequency;
// The interval to check low replication on hlog's pipeline
- private long checkLowReplicationInterval;
+ private final long checkLowReplicationInterval;
private volatile boolean running = true;
public void addWAL(WAL wal) {
// check without lock first
- if (walNeedsRoll.containsKey(wal)) {
+ if (wals.containsKey(wal)) {
return;
}
// this is to avoid race between addWAL and requestRollAll.
synchronized (this) {
- if (walNeedsRoll.putIfAbsent(wal, Boolean.FALSE) == null) {
+ if (wals.putIfAbsent(wal, new RollController(wal)) == null) {
wal.registerWALActionsListener(new WALActionsListener() {
@Override
public void logRollRequested(WALActionsListener.RollRequestReason reason) {
// TODO logs will contend with each other here, replace with e.g. DelayedQueue
Review comment:
Is this talking about what this PR is trying to do?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-658177031
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 48s | 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 | 5m 20s | master passed |
| +1 :green_heart: | compile | 1m 22s | master passed |
| +1 :green_heart: | shadedjars | 7m 5s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 51s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 11s | the patch passed |
| +1 :green_heart: | compile | 1m 23s | the patch passed |
| +1 :green_heart: | javac | 1m 23s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 6s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 51s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 220m 27s | hbase-server in the patch failed. |
| | | 253m 34s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.9 Server=19.03.9 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 50cb73a7561a 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3bd54217a0 |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/5/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/5/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/5/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/5/testReport/ |
| Max. process+thread count | 2971 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/5/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-658085575
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | 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 26s | master passed |
| +1 :green_heart: | checkstyle | 1m 16s | master passed |
| +1 :green_heart: | spotbugs | 2m 16s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 45s | the patch passed |
| -0 :warning: | checkstyle | 1m 14s | hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 18s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 58s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| | | 37m 42s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 3f54eae770db 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3bd54217a0 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 84 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/5/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-662028714
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 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 | 5m 33s | master passed |
| +1 :green_heart: | compile | 1m 30s | master passed |
| +1 :green_heart: | shadedjars | 7m 32s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 56s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 16s | the patch passed |
| +1 :green_heart: | compile | 1m 21s | the patch passed |
| +1 :green_heart: | javac | 1m 21s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 58s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 48s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 217m 52s | hbase-server in the patch passed. |
| | | 251m 37s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/8/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 2dd5fe2fcd8e 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3d270ba6f9 |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/8/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/8/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/8/testReport/ |
| Max. process+thread count | 2910 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/8/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-658174763
: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 | 4m 27s | master passed |
| +1 :green_heart: | compile | 1m 0s | master passed |
| +1 :green_heart: | shadedjars | 6m 13s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 53s | the patch passed |
| +1 :green_heart: | compile | 0m 59s | the patch passed |
| +1 :green_heart: | javac | 0m 59s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 0s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 222m 54s | hbase-server in the patch passed. |
| | | 249m 4s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 8537487cdaa1 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3bd54217a0 |
| Default Java | 1.8.0_232 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/5/testReport/ |
| Max. process+thread count | 3511 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/5/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-662015602
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 7s | 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 3s | master passed |
| +1 :green_heart: | compile | 0m 55s | master passed |
| +1 :green_heart: | shadedjars | 6m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 47s | the patch passed |
| +1 :green_heart: | compile | 0m 57s | the patch passed |
| +1 :green_heart: | javac | 0m 57s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 1s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 207m 16s | hbase-server in the patch failed. |
| | | 233m 26s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/8/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 38b2b4de52ee 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3d270ba6f9 |
| Default Java | 1.8.0_232 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/8/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/8/testReport/ |
| Max. process+thread count | 3303 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/8/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] WenFeiYi commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
WenFeiYi commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r457858574
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -249,4 +247,43 @@ public void close() {
running = false;
interrupt();
}
+
+ /**
+ * Independently control the roll of each wal. When use multiwal,
+ * can avoid all wal roll together. see HBASE-24665 for detail
+ */
+ protected class RollController {
+ private final WAL wal;
+ private final AtomicBoolean rollRequest;
+ private long lastRollTime;
+
+ RollController(WAL wal) {
+ this.wal = wal;
+ this.rollRequest = new AtomicBoolean(false);
+ this.lastRollTime = System.currentTimeMillis();
+ }
+
+ public void requestRoll() {
+ this.rollRequest.set(true);
+ }
+
+ public Map<byte[], List<byte[]>> rollWal(long now) throws IOException {
+ this.lastRollTime = now;
+ Map<byte[], List<byte[]>> regionsToFlush = wal.rollWriter(true);
+ this.rollRequest.set(false);
Review comment:
after reset flag, before roll is complete, if have a roll request, that will also cause an extra roll.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-656722232
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 12s | 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 7s | master passed |
| +1 :green_heart: | compile | 0m 59s | master passed |
| +1 :green_heart: | shadedjars | 6m 4s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 48s | the patch passed |
| +1 :green_heart: | compile | 0m 59s | the patch passed |
| +1 :green_heart: | javac | 0m 59s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 8s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 205m 40s | hbase-server in the patch passed. |
| | | 232m 1s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 2136f7c1411b 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 67dfbe0406 |
| Default Java | 1.8.0_232 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/2/testReport/ |
| Max. process+thread count | 3695 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/2/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-657416379
: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 | 5m 32s | master passed |
| +1 :green_heart: | compile | 1m 22s | master passed |
| +1 :green_heart: | shadedjars | 7m 41s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 53s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 31s | the patch passed |
| +1 :green_heart: | compile | 1m 29s | the patch passed |
| +1 :green_heart: | javac | 1m 29s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 14s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 51s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 134m 1s | hbase-server in the patch passed. |
| | | 167m 8s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 2490864fa976 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 724f0478ed |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/3/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/3/testReport/ |
| Max. process+thread count | 4187 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/3/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] WenFeiYi commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
WenFeiYi commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r452658432
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -148,10 +148,9 @@ private void abort(String reason, Throwable cause) {
@Override
public void run() {
while (running) {
- boolean periodic = false;
long now = System.currentTimeMillis();
checkLowReplication(now);
- periodic = (now - this.lastRollTime) > this.rollPeriod;
+ boolean periodic = (now - this.lastRollTime) > this.rollPeriod;
Review comment:
It's like this, I ignored it. Thanks for reminding
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-661120952
: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 | 4m 2s | master passed |
| +1 :green_heart: | compile | 1m 4s | master passed |
| +1 :green_heart: | shadedjars | 5m 47s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 41s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 6s | the patch passed |
| +1 :green_heart: | compile | 1m 4s | the patch passed |
| +1 :green_heart: | javac | 1m 4s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 52s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 41s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 134m 44s | hbase-server in the patch passed. |
| | | 160m 33s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/7/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 1b4a6e05dae7 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / f73b9085ad |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/7/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/7/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/7/testReport/ |
| Max. process+thread count | 4557 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/7/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] WenFeiYi commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
WenFeiYi commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r453449631
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -174,18 +170,21 @@ public void run() {
}
}
try {
- this.lastRollTime = System.currentTimeMillis();
- for (Iterator<Entry<WAL, Boolean>> iter = walNeedsRoll.entrySet().iterator(); iter
- .hasNext();) {
- Entry<WAL, Boolean> entry = iter.next();
+ for (Iterator<Entry<WAL, RollController>> iter = walNeedsRoll.entrySet().iterator();
+ iter.hasNext();) {
+ Entry<WAL, RollController> entry = iter.next();
+ RollController controller = entry.getValue();
+ if (!controller.isRequestRoll && !controller.isPeriodRoll(now)) {
+ continue;
+ }
WAL wal = entry.getKey();
// reset the flag in front to avoid missing roll request before we return from rollWriter.
- walNeedsRoll.put(wal, Boolean.FALSE);
+ controller.finishRoll();
Review comment:
Thank you very much for your tips
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] WenFeiYi commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
WenFeiYi commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r459882456
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -58,31 +58,31 @@
protected static final String WAL_ROLL_PERIOD_KEY = "hbase.regionserver.logroll.period";
- protected final ConcurrentMap<WAL, Boolean> walNeedsRoll = new ConcurrentHashMap<>();
+ protected final ConcurrentMap<WAL, RollController> wals = new ConcurrentHashMap<>();
protected final T abortable;
- private volatile long lastRollTime = System.currentTimeMillis();
// Period to roll log.
private final long rollPeriod;
private final int threadWakeFrequency;
// The interval to check low replication on hlog's pipeline
- private long checkLowReplicationInterval;
+ private final long checkLowReplicationInterval;
private volatile boolean running = true;
public void addWAL(WAL wal) {
// check without lock first
- if (walNeedsRoll.containsKey(wal)) {
+ if (wals.containsKey(wal)) {
return;
}
// this is to avoid race between addWAL and requestRollAll.
synchronized (this) {
- if (walNeedsRoll.putIfAbsent(wal, Boolean.FALSE) == null) {
+ if (wals.putIfAbsent(wal, new RollController(wal)) == null) {
wal.registerWALActionsListener(new WALActionsListener() {
@Override
public void logRollRequested(WALActionsListener.RollRequestReason reason) {
// TODO logs will contend with each other here, replace with e.g. DelayedQueue
synchronized (AbstractWALRoller.this) {
- walNeedsRoll.put(wal, Boolean.TRUE);
+ RollController controller = wals.computeIfAbsent(wal, rc -> new RollController(wal));
Review comment:
yes, this is my idea, if there is a change, the impact will be greater, it will be far away from the purpose of the issue.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] WenFeiYi commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
WenFeiYi commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r459235284
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -58,31 +58,31 @@
protected static final String WAL_ROLL_PERIOD_KEY = "hbase.regionserver.logroll.period";
- protected final ConcurrentMap<WAL, Boolean> walNeedsRoll = new ConcurrentHashMap<>();
+ protected final ConcurrentMap<WAL, RollController> wals = new ConcurrentHashMap<>();
Review comment:
rollWals? more suitable？
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] ramkrish86 commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
ramkrish86 commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r459374807
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -58,31 +58,31 @@
protected static final String WAL_ROLL_PERIOD_KEY = "hbase.regionserver.logroll.period";
- protected final ConcurrentMap<WAL, Boolean> walNeedsRoll = new ConcurrentHashMap<>();
+ protected final ConcurrentMap<WAL, RollController> wals = new ConcurrentHashMap<>();
Review comment:
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r457274033
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -249,4 +247,43 @@ public void close() {
running = false;
interrupt();
}
+
+ /**
+ * Independently control the roll of each wal. When use multiwal,
+ * can avoid all wal roll together. see HBASE-24665 for detail
+ */
+ protected class RollController {
+ private final WAL wal;
+ private final AtomicBoolean rollRequest;
+ private long lastRollTime;
+
+ RollController(WAL wal) {
+ this.wal = wal;
+ this.rollRequest = new AtomicBoolean(false);
+ this.lastRollTime = System.currentTimeMillis();
+ }
+
+ public void requestRoll() {
+ this.rollRequest.set(true);
+ }
+
+ public Map<byte[], List<byte[]>> rollWal(long now) throws IOException {
+ this.lastRollTime = now;
+ Map<byte[], List<byte[]>> regionsToFlush = wal.rollWriter(true);
+ this.rollRequest.set(false);
Review comment:
Hey this is where we need to reset it even before we do actual roll work.. We were doing that. We had a comment also.
// reset the flag in front to avoid missing roll request before we return from rollWriter.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-654041836
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | 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 17s | master passed |
| +1 :green_heart: | compile | 1m 3s | master passed |
| +1 :green_heart: | shadedjars | 5m 57s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 41s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 2s | the patch passed |
| +1 :green_heart: | compile | 1m 5s | the patch passed |
| +1 :green_heart: | javac | 1m 5s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 42s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 40s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 129m 11s | hbase-server in the patch passed. |
| | | 155m 17s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 1c2b3648a0ba 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 44691ce23c |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/1/testReport/ |
| Max. process+thread count | 4462 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/1/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#issuecomment-660917048
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 16s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 52s | master passed |
| +1 :green_heart: | checkstyle | 1m 14s | master passed |
| +1 :green_heart: | spotbugs | 2m 8s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 41s | the patch passed |
| -0 :warning: | checkstyle | 1m 11s | hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 14s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 17s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 35m 43s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.9 Server=19.03.9 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2021 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux d03b4d50ce04 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9b32943697 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/6/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 84 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2021/6/console |
| versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] anoopsjohn commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r451686449
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -178,14 +177,17 @@ public void run() {
for (Iterator<Entry<WAL, Boolean>> iter = walNeedsRoll.entrySet().iterator(); iter
.hasNext();) {
Entry<WAL, Boolean> entry = iter.next();
+ if (!periodic && !entry.getValue()) {
Review comment:
The actual fix is this alone 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on a change in pull request #2021: HBASE-24665 all wal of RegionGroupingProvider together roll
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2021:
URL: https://github.com/apache/hbase/pull/2021#discussion_r459886231
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -58,31 +58,31 @@
protected static final String WAL_ROLL_PERIOD_KEY = "hbase.regionserver.logroll.period";
- protected final ConcurrentMap<WAL, Boolean> walNeedsRoll = new ConcurrentHashMap<>();
+ protected final ConcurrentMap<WAL, RollController> wals = new ConcurrentHashMap<>();
protected final T abortable;
- private volatile long lastRollTime = System.currentTimeMillis();
// Period to roll log.
private final long rollPeriod;
private final int threadWakeFrequency;
// The interval to check low replication on hlog's pipeline
- private long checkLowReplicationInterval;
+ private final long checkLowReplicationInterval;
private volatile boolean running = true;
public void addWAL(WAL wal) {
// check without lock first
- if (walNeedsRoll.containsKey(wal)) {
+ if (wals.containsKey(wal)) {
return;
}
// this is to avoid race between addWAL and requestRollAll.
synchronized (this) {
- if (walNeedsRoll.putIfAbsent(wal, Boolean.FALSE) == null) {
+ if (wals.putIfAbsent(wal, new RollController(wal)) == null) {
wal.registerWALActionsListener(new WALActionsListener() {
@Override
public void logRollRequested(WALActionsListener.RollRequestReason reason) {
// TODO logs will contend with each other here, replace with e.g. DelayedQueue
synchronized (AbstractWALRoller.this) {
- walNeedsRoll.put(wal, Boolean.TRUE);
+ RollController controller = wals.computeIfAbsent(wal, rc -> new RollController(wal));
Review comment:
Sure, this is critical code anyways, we can live with bit extra atomic calls on Concurrent Map. Sounds good, `TODO` should make this better :)
Nice work @WenFeiYi .
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org