[GitHub] [hbase] anoopsjohn commented on a change in pull request #2194: HBASE-24665 MultiWAL : Avoid rolling of ALL WALs when one of the WAL needs a roll

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

anoopsjohn commented on a change in pull request #2194:
URL: https://github.com/apache/hbase/pull/2194#discussion_r467850190



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/LogRoller.java
##########
@@ -229,11 +244,52 @@ private void scheduleFlush(final byte [] encodedRegionName) {
    */
   @VisibleForTesting
   public boolean walRollFinished() {
-    for (boolean needRoll : walNeedsRoll.values()) {
-      if (needRoll) {
+    long now = System.currentTimeMillis();
+    for (RollController controller : wals.values()) {
+      if (controller.needsRoll(now)) {
         return false;
       }
     }
     return true;
   }
+
+
+  /**
+   * Independently control the roll of each wal. When use multiwal,
+   * can avoid all wal roll together. see HBASE-24665 for detail
+   */
+  protected class RollController {
+    private final WAL wal;
+    private final AtomicBoolean rollRequest;
+    private long lastRollTime;
+
+    RollController(WAL wal) {
+      this.wal = wal;
+      this.rollRequest = new AtomicBoolean(false);
+      this.lastRollTime = System.currentTimeMillis();
+    }
+
+    public void requestRoll() {
+      this.rollRequest.set(true);
+    }
+
+    public byte[][] rollWal(long now) throws IOException {
+      this.lastRollTime = now;
+      byte[][] regionsToFlush = wal.rollWriter(true);
+      this.rollRequest.set(false);

Review comment:
       This is not as per other branch patches. Why?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/LogRoller.java
##########
@@ -141,9 +150,14 @@ public void run() {
     while (!server.isStopped()) {
       long now = System.currentTimeMillis();
       checkLowReplication(now);
-      boolean periodic = false;
       if (!rollLog.get()) {
-        periodic = (now - this.lastrolltime) > this.rollperiod;
+        boolean periodic = false;
+        for (RollController controller : wals.values()) {
+          if (controller.needsPeriodicRoll(now)) {
+            periodic = true;
+            break;
+          }
+        }

Review comment:
       This logic is not same as per branch-2 patches.  What is different here? why?

##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/LogRoller.java
##########
@@ -56,23 +56,32 @@
   private static final Log LOG = LogFactory.getLog(LogRoller.class);
   private final ReentrantLock rollLock = new ReentrantLock();
   private final AtomicBoolean rollLog = new AtomicBoolean(false);
-  private final ConcurrentHashMap<WAL, Boolean> walNeedsRoll =
-      new ConcurrentHashMap<WAL, Boolean>();
+  private final ConcurrentHashMap<WAL, RollController> wals =
+      new ConcurrentHashMap<WAL, RollController>();
   private final Server server;
   protected final RegionServerServices services;
-  private volatile long lastrolltime = System.currentTimeMillis();
   // Period to roll log.
-  private final long rollperiod;
+  private final long rollPeriod;
   private final int threadWakeFrequency;
   // The interval to check low replication on hlog's pipeline
-  private long checkLowReplicationInterval;
+  private final long checkLowReplicationInterval;
 
   public void addWAL(final WAL wal) {
-    if (null == walNeedsRoll.putIfAbsent(wal, Boolean.FALSE)) {
+    if (null == wals.putIfAbsent(wal, new RollController(wal))) {
       wal.registerWALActionsListener(new WALActionsListener.Base() {
         @Override
         public void logRollRequested(WALActionsListener.RollRequestReason reason) {
-          walNeedsRoll.put(wal, Boolean.TRUE);
+          RollController controller = wals.get(wal);

Review comment:
       I see.. because of jdk 1.7 support, we have to use this way and that is what Findbugs complain. 




----------------------------------------------------------------
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