[hbase] branch branch-1 updated: HBASE-26435 The log rolling request maybe canceled immediately in LogRoller due to a race (#3832)

[reidchan]

This is an automated email from the ASF dual-hosted git repository.

reidchan pushed a commit to branch branch-1
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-1 by this push:
     new f712a4c  HBASE-26435 The log rolling request maybe canceled 
immediately in LogRoller due to a race (#3832)
f712a4c is described below

commit f712a4ceb540935081e7d84da52803843ed06156
Author: Rushabh Shah <shahr...@gmail.com>
AuthorDate: Thu Mar 3 04:49:07 2022 -0500

    HBASE-26435 The log rolling request maybe canceled immediately in LogRoller 
due to a race (#3832)
    
    Co-authored-by: Rushabh Shah <shahr...@apache.org>
    Signed-off-by: Reid Chan <reidc...@apache.org>
---
 .../apache/hadoop/hbase/regionserver/LogRoller.java | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/LogRoller.java
 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/LogRoller.java
index 0b06f41..52d7099 100644
--- 
a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/LogRoller.java
+++ 
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/LogRoller.java
@@ -143,7 +143,12 @@ public class LogRoller extends HasThread {
     while (!server.isStopped()) {
       long now = EnvironmentEdgeManager.currentTime();
       checkLowReplication(now);
-      if (!rollLog.get()) {
+      boolean rollLogLocal;
+      synchronized (rollLog) {
+        rollLogLocal = rollLog.get();
+        rollLog.set(false);
+      }
+      if (!rollLogLocal) {
         boolean periodic = false;
         for (RollController controller : wals.values()) {
           if (controller.needsPeriodicRoll(now)) {
@@ -154,11 +159,10 @@ public class LogRoller extends HasThread {
         if (!periodic) {
           synchronized (rollLog) {
             try {
-              if (!rollLog.get()) {
-                rollLog.wait(this.threadWakeFrequency);
-              }
+              rollLog.wait(this.threadWakeFrequency);
             } catch (InterruptedException e) {
               // Fall through
+              LOG.info("LogRoller interrupted ", e);
             }
           }
           continue;
@@ -199,11 +203,7 @@ public class LogRoller extends HasThread {
         LOG.error(msg, ex);
         server.abort(msg, ex);
       } finally {
-        try {
-          rollLog.set(false);
-        } finally {
-          rollLock.unlock();
-        }
+        rollLock.unlock();
       }
     }
     LOG.info("LogRoller exiting.");
@@ -266,8 +266,9 @@ public class LogRoller extends HasThread {
 
     public byte[][] rollWal(long now) throws IOException {
       this.lastRollTime = now;
-      byte[][] regionsToFlush = wal.rollWriter(true);
+      // reset the flag in front to avoid missing roll request before we 
return from rollWriter.
       this.rollRequest.set(false);
+      byte[][] regionsToFlush = wal.rollWriter(true);
       return regionsToFlush;
     }