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/08/13 11:34:13 UTC
[GitHub] [hbase] wchevreuil opened a new pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
wchevreuil opened a new pull request #2255:
URL: https://github.com/apache/hbase/pull/2255
…eptions happen on replication source
----------------------------------------------------------------
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] wchevreuil commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r470734916
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -583,16 +614,27 @@ private void initialize() {
PriorityBlockingQueue<Path> queue = entry.getValue();
tryStartNewShipper(walGroupId, queue);
}
+ this.startupOngoing.set(false);
}
@Override
public void startup() {
// mark we are running now
this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ this.retryStartup.set(true);
+ do {
+ if(retryStartup.get()) {
+ retryStartup.set(false);
+ startupOngoing.set(true);
+ initThread = new Thread(this::initialize);
+ Threads.setDaemonThreadRunning(initThread,
+ Thread.currentThread().getName() + ".replicationSource," + this.queueId,
+ (t,e) -> {
+ uncaughtException(t, e);
+ retryStartup.set(true);
Review comment:
Addressed the nits and added some comments to the two flags.
----------------------------------------------------------------
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] wchevreuil commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r475665485
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -587,12 +617,25 @@ private void initialize() {
@Override
public void startup() {
- // mark we are running now
- this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ //Flag that signalizes uncaught error happening while starting up the source
+ // and a retry should be attempted
+ AtomicBoolean retryStartup = new AtomicBoolean(false);
Review comment:
It's been modified by the lambda expression on line #635, since lambdas expressions requires variables to be final, I can't use a simple, local primitive boolean or wrapper.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -587,12 +617,25 @@ private void initialize() {
@Override
public void startup() {
- // mark we are running now
- this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ //Flag that signalizes uncaught error happening while starting up the source
+ // and a retry should be attempted
+ AtomicBoolean retryStartup = new AtomicBoolean(false);
Review comment:
It's been modified by the lambda expression on line #635, since lambdas expressions require variables to be final, I can't use a simple, local primitive boolean or wrapper.
----------------------------------------------------------------
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] wchevreuil commented on pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-687251804
Latest UT failure seems unrelated, have it passing locally.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-679894883
: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 | 4m 20s | master passed |
| +1 :green_heart: | compile | 1m 10s | master passed |
| +1 :green_heart: | shadedjars | 6m 43s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 43s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 1m 57s | root in the patch failed. |
| -1 :x: | compile | 0m 20s | hbase-server in the patch failed. |
| -0 :warning: | javac | 0m 20s | hbase-server in the patch failed. |
| -1 :x: | shadedjars | 4m 49s | patch has 10 errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 23s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 0m 19s | hbase-server in the patch failed. |
| | | 22m 22s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux d4e9c7d6c08b 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 / 6b0707f541 |
| Default Java | 2020-01-14 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk11-hadoop3-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
| shadedjars | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk11-hadoop3-check/output/patch-shadedjars.txt |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/testReport/ |
| Max. process+thread count | 97 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-679994452
: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 | 5m 13s | master passed |
| +1 :green_heart: | checkstyle | 1m 35s | master passed |
| +1 :green_heart: | spotbugs | 2m 53s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 42s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 27s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 15m 13s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 3m 4s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 19s | The patch does not generate ASF License warnings. |
| | | 44m 2s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux fd2850d05503 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 / 6b0707f541 |
| Max. process+thread count | 94 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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] Apache9 commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r472636537
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceShipper.java
##########
@@ -290,7 +290,22 @@ private boolean updateLogPosition(WALEntryBatch batch) {
public void startup(UncaughtExceptionHandler handler) {
String name = Thread.currentThread().getName();
Threads.setDaemonThreadRunning(this,
- name + ".replicationSource.shipper" + walGroupId + "," + source.getQueueId(), handler);
+ name + ".replicationSource.shipper" + walGroupId + "," + source.getQueueId(),
+ (t,e) -> {
Review comment:
OK, the code is almost the same... Then I think we could move the logic into uncaughtException method? If abortOnError is true, we about, otherwise we will try to refresh the source.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -373,7 +389,21 @@ private void tryStartNewShipper(String walGroupId, PriorityBlockingQueue<Path> q
Threads.setDaemonThreadRunning(
walReader, Thread.currentThread().getName()
+ ".replicationSource.wal-reader." + walGroupId + "," + queueId,
- this::uncaughtException);
+ (t,e) -> {
Review comment:
So here it is for wal reader. I think refreshSources and retry is an acceptable way. Then let's just test the abortOnError flag here? If it is true, we will call uncaughtException, otherwise we will try to refresh the replication source.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -583,16 +617,27 @@ private void initialize() {
PriorityBlockingQueue<Path> queue = entry.getValue();
tryStartNewShipper(walGroupId, queue);
}
+ this.startupOngoing.set(false);
}
@Override
public void startup() {
// mark we are running now
this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ this.retryStartup.set(true);
Review comment:
This flag is only used in this method? Let's use a local var instead of a class member field?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -583,16 +617,27 @@ private void initialize() {
PriorityBlockingQueue<Path> queue = entry.getValue();
tryStartNewShipper(walGroupId, queue);
}
+ this.startupOngoing.set(false);
}
@Override
public void startup() {
// mark we are running now
this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ this.retryStartup.set(true);
+ do {
+ if(retryStartup.get()) {
+ retryStartup.set(false);
+ startupOngoing.set(true);
Review comment:
So this one is exactly the same with source.isActive? Can we just make use of that flag instead of introducing a new one?
----------------------------------------------------------------
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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-679105675
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 39s | 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 34s | master passed |
| +1 :green_heart: | checkstyle | 1m 21s | master passed |
| +1 :green_heart: | spotbugs | 2m 25s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 9s | the patch passed |
| -0 :warning: | checkstyle | 1m 16s | hbase-server: The patch generated 6 new + 2 unchanged - 0 fixed = 8 total (was 2) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 13m 34s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 40s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 40m 13s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 527c94414576 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2874f00a2f |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/3/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://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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] virajjasani commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r469904081
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -583,16 +614,27 @@ private void initialize() {
PriorityBlockingQueue<Path> queue = entry.getValue();
tryStartNewShipper(walGroupId, queue);
}
+ this.startupOngoing.set(false);
}
@Override
public void startup() {
// mark we are running now
this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ this.retryStartup.set(true);
+ do {
+ if(retryStartup.get()) {
+ retryStartup.set(false);
+ startupOngoing.set(true);
+ initThread = new Thread(this::initialize);
+ Threads.setDaemonThreadRunning(initThread,
+ Thread.currentThread().getName() + ".replicationSource," + this.queueId,
+ (t,e) -> {
+ uncaughtException(t, e);
+ retryStartup.set(true);
Review comment:
I hope if we encounter uncaughtException here, we want to retry the loop again.
If so, shall we also add `startupOngoing.set(true);` here explicitely? Just in case if it is set to false in `initialize()`?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -35,6 +35,8 @@
import java.util.concurrent.PriorityBlockingQueue;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
Review comment:
nit: redundant
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -120,6 +122,13 @@
// ReplicationEndpoint which will handle the actual replication
private volatile ReplicationEndpoint replicationEndpoint;
+ private AtomicBoolean retryStartup = new AtomicBoolean(false);
+
+ private AtomicBoolean startupOngoing = new AtomicBoolean(false);
Review comment:
nit: `final` for both?
----------------------------------------------------------------
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] wchevreuil commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r475666601
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -119,6 +120,14 @@
private int logQueueWarnThreshold;
// ReplicationEndpoint which will handle the actual replication
private volatile ReplicationEndpoint replicationEndpoint;
+ //This is needed for the startup loop to identify when there's already
+ // an initialization happening (but not finished yet),
+ // so that it doesn't try submit another initialize thread.
+ // NOTE: this should only be set to false at the end of initialize method, prior to return.
+// private final AtomicBoolean startupOngoing = new AtomicBoolean(false);
Review comment:
Actually, this whole block can be removed.
----------------------------------------------------------------
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] wchevreuil commented on pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-673960675
> What's next if we ignore the exception? We will retry later? Or we will just go on without this replication source?
As you can see on `ReplicationSource.startup`, it keeps looping until `initialize` succeeds without throwing any uncaught exceptions.
> Users will then find out that the cluster is fine but data has not been replicated out?
It's common practice to verify replication status after a maintenance.
> I'm not sure if this is correct way, we fix an issue but introduce another hard to find issue?
It does not fail silently, errors will get logged, and it gives operators the chance to look after what's going wrong without a complete downtime of their source clusters.
>Adding a flag can keep the old behavior but we give users an impression that the exception can be ignored? Still not sure if this is the correct way to fix this...
Mind explaining more on your real usage?
We do use some custom replication endpoints that under certain unavailability of some target peer hosts ended up throwing uncaught exception and aborting the source RSes. Sure, there could be improvements on the custom code, and it was an internal infra issue, but with a flag like this, we wouldn't need to face a period of outage at the source.
----------------------------------------------------------------
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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r470485706
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -583,16 +614,27 @@ private void initialize() {
PriorityBlockingQueue<Path> queue = entry.getValue();
tryStartNewShipper(walGroupId, queue);
}
+ this.startupOngoing.set(false);
}
@Override
public void startup() {
// mark we are running now
this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ this.retryStartup.set(true);
+ do {
+ if(retryStartup.get()) {
+ retryStartup.set(false);
+ startupOngoing.set(true);
+ initThread = new Thread(this::initialize);
+ Threads.setDaemonThreadRunning(initThread,
+ Thread.currentThread().getName() + ".replicationSource," + this.queueId,
+ (t,e) -> {
+ uncaughtException(t, e);
+ retryStartup.set(true);
Review comment:
Yeah right, it is not set to `false` if it completes successfully, anyone touching the same code in future should realize this.
Although not a strong point but If you don't mind, maybe we can comment here indicating `startupOngoing` is expected to be true when we are here handling Exception.
----------------------------------------------------------------
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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-679286764
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 4s | 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 44s | master passed |
| +1 :green_heart: | checkstyle | 1m 5s | master passed |
| +1 :green_heart: | spotbugs | 1m 59s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 19s | the patch passed |
| -0 :warning: | checkstyle | 1m 3s | hbase-server: The patch generated 6 new + 2 unchanged - 0 fixed = 8 total (was 2) |
| +1 :green_heart: | whitespace | 0m 1s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 11m 1s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 6s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| | | 32m 54s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux d3c67955b0bd 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 6ad73b9668 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/4/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://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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] wchevreuil commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r475551956
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSourceShipper.java
##########
@@ -290,7 +290,22 @@ private boolean updateLogPosition(WALEntryBatch batch) {
public void startup(UncaughtExceptionHandler handler) {
String name = Thread.currentThread().getName();
Threads.setDaemonThreadRunning(this,
- name + ".replicationSource.shipper" + walGroupId + "," + source.getQueueId(), handler);
+ name + ".replicationSource.shipper" + walGroupId + "," + source.getQueueId(),
+ (t,e) -> {
Review comment:
Moved it to _uncaughtException_. This is true when handling potential errors on both the reader and shipper threads, but _uncaughtException_ is also used when handling issues from _ReplicationSource.initialize_ method, which might blow before reader/shipper is even started. So needed to add extra check in _uncaughtException_ to decide when to invoke _refreshSources_.
----------------------------------------------------------------
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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-679183399
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 41s | 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 23s | master passed |
| +1 :green_heart: | compile | 1m 3s | master passed |
| +1 :green_heart: | shadedjars | 6m 30s | 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 2s | 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 33s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 150m 4s | hbase-server in the patch passed. |
| | | 177m 48s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux a0ab0e6d6aaa 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 / 2874f00a2f |
| Default Java | 1.8.0_232 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/3/testReport/ |
| Max. process+thread count | 3867 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-679217560
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 16s | 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 53s | master passed |
| +1 :green_heart: | compile | 1m 17s | master passed |
| +1 :green_heart: | shadedjars | 6m 51s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 50s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 46s | the patch passed |
| +1 :green_heart: | compile | 1m 19s | the patch passed |
| +1 :green_heart: | javac | 1m 19s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 30s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 42s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 205m 29s | hbase-server in the patch passed. |
| | | 235m 33s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux c5f659124d33 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 2874f00a2f |
| Default Java | 2020-01-14 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/3/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/3/testReport/ |
| Max. process+thread count | 3670 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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] Apache9 commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r475685292
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -587,12 +617,25 @@ private void initialize() {
@Override
public void startup() {
- // mark we are running now
- this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ //Flag that signalizes uncaught error happening while starting up the source
+ // and a retry should be attempted
+ AtomicBoolean retryStartup = new AtomicBoolean(false);
Review comment:
Then use MutableBoolean? We do not need to use atomic here.
----------------------------------------------------------------
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] wchevreuil commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r475666601
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -119,6 +120,14 @@
private int logQueueWarnThreshold;
// ReplicationEndpoint which will handle the actual replication
private volatile ReplicationEndpoint replicationEndpoint;
+ //This is needed for the startup loop to identify when there's already
+ // an initialization happening (but not finished yet),
+ // so that it doesn't try submit another initialize thread.
+ // NOTE: this should only be set to false at the end of initialize method, prior to return.
+// private final AtomicBoolean startupOngoing = new AtomicBoolean(false);
Review comment:
Actually, these whole block can be removed.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -119,6 +120,14 @@
private int logQueueWarnThreshold;
// ReplicationEndpoint which will handle the actual replication
private volatile ReplicationEndpoint replicationEndpoint;
+ //This is needed for the startup loop to identify when there's already
+ // an initialization happening (but not finished yet),
+ // so that it doesn't try submit another initialize thread.
+ // NOTE: this should only be set to false at the end of initialize method, prior to return.
+// private final AtomicBoolean startupOngoing = new AtomicBoolean(false);
Review comment:
Actually, this whole block can be removed.
----------------------------------------------------------------
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] wchevreuil commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r475670615
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -587,12 +617,25 @@ private void initialize() {
@Override
public void startup() {
- // mark we are running now
- this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ //Flag that signalizes uncaught error happening while starting up the source
+ // and a retry should be attempted
+ AtomicBoolean retryStartup = new AtomicBoolean(false);
+ retryStartup.set(true);
+ do {
+ if(retryStartup.get()) {
+ retryStartup.set(false);
+ // mark we are running now
+ this.sourceRunning = true;
+ initThread = new Thread(this::initialize);
+ Threads.setDaemonThreadRunning(initThread,
+ Thread.currentThread().getName() + ".replicationSource," + this.queueId,
+ (t,e) -> {
+ sourceRunning = false;
+ uncaughtException(t, e, null, null);
+ retryStartup.set(true);
+ });
+ }
+ } while (!this.sourceRunning);
Review comment:
I had a second thought on this here, we can't simply re-use this boolean, because in case of failure, we risk reach this point before the exception handler has updated it to false. I'm bringing back the original _startupOngoing_ in the next commit,
----------------------------------------------------------------
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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-673445925
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 41s | 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 57s | master passed |
| +1 :green_heart: | checkstyle | 1m 20s | master passed |
| +1 :green_heart: | spotbugs | 2m 28s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 19s | the patch passed |
| -0 :warning: | checkstyle | 1m 19s | hbase-server: The patch generated 18 new + 2 unchanged - 0 fixed = 20 total (was 2) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 14m 25s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 47s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| | | 42m 29s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 8e2d0f0fe0ac 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / c81ef7368e |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/1/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://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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] wchevreuil commented on pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-680766011
retest build
----------------------------------------------------------------
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] wchevreuil merged pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil merged pull request #2255:
URL: https://github.com/apache/hbase/pull/2255
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-681217536
> retest build
This does not work, you need to go to the jenkins page to manually start a build, or just do a rebase and force push here, if you want to trigger a new build.
----------------------------------------------------------------
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] wchevreuil commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r470477805
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -583,16 +614,27 @@ private void initialize() {
PriorityBlockingQueue<Path> queue = entry.getValue();
tryStartNewShipper(walGroupId, queue);
}
+ this.startupOngoing.set(false);
}
@Override
public void startup() {
// mark we are running now
this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ this.retryStartup.set(true);
+ do {
+ if(retryStartup.get()) {
+ retryStartup.set(false);
+ startupOngoing.set(true);
+ initThread = new Thread(this::initialize);
+ Threads.setDaemonThreadRunning(initThread,
+ Thread.currentThread().getName() + ".replicationSource," + this.queueId,
+ (t,e) -> {
+ uncaughtException(t, e);
+ retryStartup.set(true);
Review comment:
>I hope if we encounter uncaughtException here, we want to retry the loop again.
Yes, we want to keep trying until we succeed.
> If so, shall we also add startupOngoing.set(true); here explicitely? Just in case if it is set to false in initialize()?
The only cases `startupOngoing` would had been set to `false` in `initialize` is if it completes fine without any uncaught exception, so adding it here would be redundant.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-674008001
OK, thank you for your reply. Let me take a look at the code more carefully.
----------------------------------------------------------------
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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-679894592
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 0s | 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 46s | master passed |
| +1 :green_heart: | compile | 0m 56s | master passed |
| +1 :green_heart: | shadedjars | 6m 35s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 1m 40s | root in the patch failed. |
| -1 :x: | compile | 0m 20s | hbase-server in the patch failed. |
| -0 :warning: | javac | 0m 20s | hbase-server in the patch failed. |
| -1 :x: | shadedjars | 4m 49s | patch has 10 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 0m 20s | hbase-server in the patch failed. |
| | | 21m 52s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 59807960fd2b 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 6b0707f541 |
| Default Java | 1.8.0_232 |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk8-hadoop3-check/output/patch-mvninstall-root.txt |
| compile | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-hbase-server.txt |
| javac | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-hbase-server.txt |
| shadedjars | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk8-hadoop3-check/output/patch-shadedjars.txt |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/testReport/ |
| Max. process+thread count | 88 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-687240214
: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 22s | master passed |
| +1 :green_heart: | compile | 1m 9s | master passed |
| +1 :green_heart: | shadedjars | 6m 58s | 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 | 4m 11s | 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 | 6m 58s | 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 | 130m 57s | hbase-server in the patch passed. |
| | | 159m 42s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/8/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux d0cd28c1e9f7 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 19d0140562 |
| Default Java | 2020-01-14 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/8/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/8/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/8/testReport/ |
| Max. process+thread count | 4428 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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] wchevreuil commented on pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-681034324
retest build
----------------------------------------------------------------
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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-679893077
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 15s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 19s | master passed |
| +1 :green_heart: | checkstyle | 1m 15s | master passed |
| +1 :green_heart: | spotbugs | 2m 37s | master passed |
||| _ Patch Compile Tests _ |
| -1 :x: | mvninstall | 1m 59s | root in the patch failed. |
| -0 :warning: | checkstyle | 1m 18s | hbase-server: The patch generated 1 new + 2 unchanged - 0 fixed = 3 total (was 2) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| -1 :x: | hadoopcheck | 2m 6s | The patch causes 10 errors with Hadoop v3.1.2. |
| -1 :x: | hadoopcheck | 4m 7s | The patch causes 10 errors with Hadoop v3.2.1. |
| -1 :x: | spotbugs | 0m 18s | hbase-server in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 11s | The patch does not generate ASF License warnings. |
| | | 18m 54s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux ac1cd28493bf 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 6b0707f541 |
| mvninstall | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-general-check/output/patch-mvninstall-root.txt |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-general-check/output/patch-javac-3.1.2.txt |
| hadoopcheck | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-general-check/output/patch-javac-3.2.1.txt |
| spotbugs | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/5/artifact/yetus-general-check/output/patch-spotbugs-hbase-server.txt |
| Max. process+thread count | 84 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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] wchevreuil commented on pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-679087403
Thanks for the suggestions, @Apache9, had a pushed a new commit addressing those, let me know on your thoughts.
----------------------------------------------------------------
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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-687163437
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 9s | 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 7s | master passed |
| +1 :green_heart: | checkstyle | 1m 13s | master passed |
| +1 :green_heart: | spotbugs | 2m 10s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 45s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 9s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 12m 29s | 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. |
| | | 36m 17s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/8/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 0447e2d9fef6 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 19d0140562 |
| Max. process+thread count | 84 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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] wchevreuil commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r476272301
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -587,12 +617,25 @@ private void initialize() {
@Override
public void startup() {
- // mark we are running now
- this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ //Flag that signalizes uncaught error happening while starting up the source
+ // and a retry should be attempted
+ AtomicBoolean retryStartup = new AtomicBoolean(false);
Review comment:
Sure, thanks for suggesting MutableBoolean, am not too familiar with apache commons lib.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r475573989
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -450,11 +463,28 @@ WALEntryFilter getWalEntryFilter() {
return walEntryFilter;
}
- protected final void uncaughtException(Thread t, Throwable e) {
+ protected final void uncaughtException(Thread t, Throwable e,
+ ReplicationSourceManager manager, String peerId) {
RSRpcServices.exitIfOOME(e);
LOG.error("Unexpected exception in {} currentPath={}",
t.getName(), getCurrentPath(), e);
- server.abort("Unexpected exception in " + t.getName(), e);
+ if(abortOnError){
+ server.abort("Unexpected exception in " + t.getName(), e);
+ }
+ if(manager!=null){
Review comment:
Need to update the formatter config? Usually it should be 'if (manager != null) {'.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -119,6 +120,14 @@
private int logQueueWarnThreshold;
// ReplicationEndpoint which will handle the actual replication
private volatile ReplicationEndpoint replicationEndpoint;
+ //This is needed for the startup loop to identify when there's already
+ // an initialization happening (but not finished yet),
+ // so that it doesn't try submit another initialize thread.
+ // NOTE: this should only be set to false at the end of initialize method, prior to return.
+// private final AtomicBoolean startupOngoing = new AtomicBoolean(false);
Review comment:
nit: indent?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -587,12 +617,25 @@ private void initialize() {
@Override
public void startup() {
- // mark we are running now
- this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ //Flag that signalizes uncaught error happening while starting up the source
+ // and a retry should be attempted
+ AtomicBoolean retryStartup = new AtomicBoolean(false);
Review comment:
Why we need a AtomicBoolean here? It is only used locally, so a simple boolean is enough?
----------------------------------------------------------------
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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-687245528
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 37s | 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 26s | master passed |
| +1 :green_heart: | compile | 0m 57s | master passed |
| +1 :green_heart: | shadedjars | 6m 35s | 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 25s | 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 | 6m 32s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 143m 15s | hbase-server in the patch failed. |
| | | 169m 55s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/8/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 83ed99eb7cf0 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 / 19d0140562 |
| Default Java | 1.8.0_232 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/8/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/8/testReport/ |
| Max. process+thread count | 4872 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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] wchevreuil commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r474533514
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -583,16 +617,27 @@ private void initialize() {
PriorityBlockingQueue<Path> queue = entry.getValue();
tryStartNewShipper(walGroupId, queue);
}
+ this.startupOngoing.set(false);
}
@Override
public void startup() {
// mark we are running now
this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ this.retryStartup.set(true);
Review comment:
Yeah, originally I was also referring it on _uncaughtException_, but it was not actually needed.
----------------------------------------------------------------
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] wchevreuil commented on pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-679884736
Pushed a new commit addressing latest suggestion and checkstyle issues.
----------------------------------------------------------------
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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-682437651
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 57s | 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 50s | master passed |
| +1 :green_heart: | checkstyle | 1m 34s | master passed |
| +1 :green_heart: | spotbugs | 2m 37s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 43s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 22s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 14m 44s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 31s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 43m 9s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/7/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux 42074e508888 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7909e29de5 |
| Max. process+thread count | 84 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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] wchevreuil commented on a change in pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#discussion_r474544355
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/replication/regionserver/ReplicationSource.java
##########
@@ -583,16 +617,27 @@ private void initialize() {
PriorityBlockingQueue<Path> queue = entry.getValue();
tryStartNewShipper(walGroupId, queue);
}
+ this.startupOngoing.set(false);
}
@Override
public void startup() {
// mark we are running now
this.sourceRunning = true;
- initThread = new Thread(this::initialize);
- Threads.setDaemonThreadRunning(initThread,
- Thread.currentThread().getName() + ".replicationSource," + this.queueId,
- this::uncaughtException);
+ this.retryStartup.set(true);
+ do {
+ if(retryStartup.get()) {
+ retryStartup.set(false);
+ startupOngoing.set(true);
Review comment:
Not exactly the same, but we may use the negation of _source.isActive_ to control this retry loop, and change _this.sourceRunning_ value properly along _startup_ and _initialize_ method.
----------------------------------------------------------------
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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-673440803
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 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 | 3m 55s | master passed |
| +1 :green_heart: | compile | 0m 56s | master passed |
| +1 :green_heart: | shadedjars | 5m 59s | 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 32s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 35s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 6m 48s | hbase-server in the patch failed. |
| | | 31m 7s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 2f5e5acb36da 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 / c81ef7368e |
| Default Java | 1.8.0_232 |
| unit | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/1/testReport/ |
| Max. process+thread count | 771 (vs. ulimit of 12500) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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 #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-674177875
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 24s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 11s | master passed |
| +1 :green_heart: | checkstyle | 1m 12s | master passed |
| +1 :green_heart: | spotbugs | 2m 12s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 50s | the patch passed |
| -0 :warning: | checkstyle | 1m 10s | hbase-server: The patch generated 17 new + 2 unchanged - 0 fixed = 19 total (was 2) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 14m 22s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 2m 40s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 18s | The patch does not generate ASF License warnings. |
| | | 39m 2s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2255 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
| uname | Linux a185969d3af4 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / c81ef7368e |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/2/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://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2255/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] wchevreuil commented on pull request #2255: HBASE-24877 Add option to avoid aborting RS process upon uncaught exc…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #2255:
URL: https://github.com/apache/hbase/pull/2255#issuecomment-683274583
It looks like this is causing some of the UTs to timeout. Let me dig into it further.
----------------------------------------------------------------
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