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

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

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



##########
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:
       The LogRoller.walRollFinished impl is already buggy. This just checks the status of the boolean. Once we start a roll on a WAL, we reset the boolean (Even before patch HBASE-24665). So it is not clearly telling anything about the roll status. This can return true even while an active wal roll is going on.
   We might need another boolean in Controller which clearly tracks whether we are ongoing a roll.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org