From f48c92d143429d389821be3347de42fe18f60276 Mon Sep 17 00:00:00 2001 From: Sean Busbey Date: Thu, 17 Mar 2016 15:22:07 -0500 Subject: [PATCH] HBASE-15478 add comments to syncRunnerIndex handling explaining constraints on possible values. Signed-off-by: zhangduo --- .../org/apache/hadoop/hbase/regionserver/wal/FSHLog.java | 13 +++++++++++-- 1 file changed, 11 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 f3f869caac..dfbdae5e5c 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 @@ -1785,10 +1785,19 @@ public class FSHLog implements WAL { // If not a batch, return to consume more events from the ring buffer before proceeding; // we want to get up a batch of syncs and appends before we go do a filesystem sync. if (!endOfBatch || this.syncFuturesCount <= 0) return; - // Below expects that the offer 'transfers' responsibility for the outstanding syncs to - // the syncRunner. We should never get an exception in here. + // syncRunnerIndex is bound to the range [0, Integer.MAX_INT - 1] as follows: + // * The maximum value possible for syncRunners.length is Integer.MAX_INT + // * syncRunnerIndex starts at 0 and is incremented only here + // * after the increment, the value is bounded by the '%' operator to [0, syncRunners.length), + // presuming the value was positive prior to the '%' operator. + // * after being bound to [0, Integer.MAX_INT - 1], the new value is stored in syncRunnerIndex + // ensuring that it can't grow without bound and overflow. + // * note that the value after the increment must be positive, because the most it could have + // been prior was Integer.MAX_INT - 1 and we only increment by 1. this.syncRunnerIndex = (this.syncRunnerIndex + 1) % this.syncRunners.length; try { + // Below expects that the offer 'transfers' responsibility for the outstanding syncs to + // the syncRunner. We should never get an exception in here. this.syncRunners[this.syncRunnerIndex].offer(sequence, this.syncFutures, this.syncFuturesCount); } catch (Exception e) { -- 2.11.4.GIT