[GitHub] [hbase] Apache9 commented on a change in pull request #2169: HBASE-24792 LogRoller.walRollFinished fix

[GitBox <gi...@apache.org>]

Apache9 commented on a change in pull request #2169:
URL: https://github.com/apache/hbase/pull/2169#discussion_r467653135



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/wal/AbstractWALRoller.java
##########
@@ -255,22 +250,32 @@ public void close() {
     private final WAL wal;
     private final AtomicBoolean rollRequest;
     private long lastRollTime;
+    private boolean isRolling;
 
     RollController(WAL wal) {
       this.wal = wal;
       this.rollRequest = new AtomicBoolean(false);
       this.lastRollTime = System.currentTimeMillis();
+      this.isRolling = false;
     }
 
     public void requestRoll() {
       this.rollRequest.set(true);
     }
 
     public Map<byte[], List<byte[]>> rollWal(long now) throws IOException {
-      this.lastRollTime = now;
-      // reset the flag in front to avoid missing roll request before we return from rollWriter.
-      this.rollRequest.set(false);
-      return wal.rollWriter(true);
+      this.isRolling = true;
+      try {
+        this.lastRollTime = now;
+        // reset the flag in front to avoid missing roll request before we return from rollWriter.
+        this.rollRequest.set(false);
+        return wal.rollWriter(true);
+      } finally {
+        this.isRolling = false;
+        synchronized (RollController.class) {
+          RollController.class.notifyAll();

Review comment:
       Usually a dummy notifyAll without any flag in the same synchronized block will have race... Mind explain a bit what is the problem we want to fix 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