From 234f31bd4a046a1eb8ecd15b3d5c2844c7916d6a Mon Sep 17 00:00:00 2001
From: zhangduo <zhangduo@apache.org>
Date: Wed, 4 Nov 2015 19:08:26 +0800
Subject: [PATCH] HBASE-14759 Avoid using Math.abs when selecting SyncRunner in
 FSHLog

---
 .../hadoop/hbase/regionserver/wal/FSHLog.java |  5 ++--
 .../hbase/regionserver/wal/TestFSHLog.java    | 29 +++++++++++++++++++
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java
index 7e3465f4f2c..92e7b3c00e9 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSHLog.java
@@ -1766,9 +1766,10 @@ public class FSHLog implements WAL {
         if (this.exception == null) {
           // Below expects that the offer 'transfers' responsibility for the outstanding syncs to
           // the syncRunner. We should never get an exception in here.
-          int index = Math.abs(this.syncRunnerIndex++) % this.syncRunners.length;
+          this.syncRunnerIndex = (this.syncRunnerIndex + 1) % this.syncRunners.length;
           try {
-            this.syncRunners[index].offer(sequence, this.syncFutures, this.syncFuturesCount);
+            this.syncRunners[this.syncRunnerIndex].offer(sequence, this.syncFutures,
+              this.syncFuturesCount);
           } catch (Exception e) {
             // Should NEVER get here.
             requestLogRoll();
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestFSHLog.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestFSHLog.java
index 059d697d104..6ece7006d40 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestFSHLog.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/wal/TestFSHLog.java
@@ -24,6 +24,7 @@ import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 
 import java.io.IOException;
+import java.lang.reflect.Field;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
@@ -419,4 +420,32 @@ public class TestFSHLog {
     }
   }
 
+  @Test
+  public void testSyncRunnerIndexOverflow() throws IOException, NoSuchFieldException,
+      SecurityException, IllegalArgumentException, IllegalAccessException {
+    final String name = "testSyncRunnerIndexOverflow";
+    FSHLog log =
+        new FSHLog(fs, FSUtils.getRootDir(conf), name, HConstants.HREGION_OLDLOGDIR_NAME, conf,
+            null, true, null, null);
+    try {
+      Field ringBufferEventHandlerField = FSHLog.class.getDeclaredField("ringBufferEventHandler");
+      ringBufferEventHandlerField.setAccessible(true);
+      FSHLog.RingBufferEventHandler ringBufferEventHandler =
+          (FSHLog.RingBufferEventHandler) ringBufferEventHandlerField.get(log);
+      Field syncRunnerIndexField =
+          FSHLog.RingBufferEventHandler.class.getDeclaredField("syncRunnerIndex");
+      syncRunnerIndexField.setAccessible(true);
+      syncRunnerIndexField.set(ringBufferEventHandler, Integer.MAX_VALUE - 1);
+      HTableDescriptor htd =
+          new HTableDescriptor(TableName.valueOf("t1")).addFamily(new HColumnDescriptor("row"));
+      HRegionInfo hri =
+          new HRegionInfo(htd.getTableName(), HConstants.EMPTY_START_ROW, HConstants.EMPTY_END_ROW);
+      MultiVersionConcurrencyControl mvcc = new MultiVersionConcurrencyControl();
+      for (int i = 0; i < 10; i++) {
+        addEdits(log, hri, htd, 1, mvcc);
+      }
+    } finally {
+      log.close();
+    }
+  }
 }