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:
[email protected]


Reply via email to