Author: liyin Date: Sat Dec 28 19:18:21 2013 New Revision: 1553892 URL: http://svn.apache.org/r1553892 Log: [HBASE-10205] Fix ConcurrentModificationException in BucketAllocator
Author: arjen Summary: The different WriterThreads in the BucketCache share a reference to a single BucketAllocator. There is race condition between BucketAllocator.allocateBlock(), which is called when flushing each cache entry, and BucketAllocator.getIndexStatistics(), which usually gets called by the WriterThread after the queue with entries has been flushed. This diff adds some synchronization around these operations. Locking here is ok as calls to the BucketAllocator happen outside the critical path of adding to the cache. Removal of items from the cache happen by the compaction thread or on close, both should be ok here. Test Plan: Ran the loadtest on one of dev clusters for 12+ hours. Without this diff exceptions would show up within a few hours. Reviewers: liyintang, aaiyer, rshroff Reviewed By: liyintang CC: hbase-eng@ Differential Revision: https://phabricator.fb.com/D1108464 Task ID: 3337650 Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketAllocator.java hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketAllocator.java URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketAllocator.java?rev=1553892&r1=1553891&r2=1553892&view=diff ============================================================================== --- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketAllocator.java (original) +++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketAllocator.java Sat Dec 28 19:18:21 2013 @@ -20,7 +20,7 @@ package org.apache.hadoop.hbase.io.hfile.bucket; -import java.util.ArrayList; +import java.util.LinkedList; import java.util.List; import com.google.common.base.Preconditions; @@ -150,9 +150,9 @@ public final class BucketAllocator { private int sizeIndex; BucketSizeInfo(int sizeIndex) { - bucketList = new ArrayList<Bucket>(); - freeBuckets = new ArrayList<Bucket>(); - completelyFreeBuckets = new ArrayList<Bucket>(); + bucketList = new LinkedList<Bucket>(); + freeBuckets = new LinkedList<Bucket>(); + completelyFreeBuckets = new LinkedList<Bucket>(); this.sizeIndex = sizeIndex; } @@ -186,7 +186,7 @@ public final class BucketAllocator { return result; } - void blockAllocated(Bucket b) { + private void blockAllocated(Bucket b) { if (!b.isCompletelyFree()) completelyFreeBuckets.remove(b); if (!b.hasFreeSpace()) freeBuckets.remove(b); } @@ -248,7 +248,6 @@ public final class BucketAllocator { return null; } - static public final int FEWEST_ITEMS_IN_BUCKET = 4; // The capacity size for each bucket @@ -349,7 +348,7 @@ public final class BucketAllocator { return targetBucket.itemAllocationSize(); } - public int sizeOfAllocation(long offset) { + public synchronized int sizeOfAllocation(long offset) { int bucketNo = (int) (offset / bucketCapacity); Preconditions.checkPositionIndex(bucketNo, buckets.length); Bucket targetBucket = buckets[bucketNo]; @@ -421,10 +420,10 @@ public final class BucketAllocator { IndexStatistics total = new IndexStatistics(); IndexStatistics[] stats = getIndexStatistics(total); LOG.info("Bucket allocator statistics follow:\n"); - LOG.info(" Free bytes=" + total.freeBytes() + "+; used bytes=" + LOG.info(" Free bytes=" + total.freeBytes() + "; used bytes=" + total.usedBytes() + "; total bytes=" + total.totalBytes()); for (IndexStatistics s : stats) { - LOG.info(" Object size " + s.itemSize() + " used=" + s.usedCount() + LOG.info(" Object size " + s.itemSize() + "; used=" + s.usedCount() + "; free=" + s.freeCount() + "; total=" + s.totalCount()); } } @@ -440,7 +439,7 @@ public final class BucketAllocator { return stats; } - public IndexStatistics[] getIndexStatistics() { + public synchronized IndexStatistics[] getIndexStatistics() { IndexStatistics[] stats = new IndexStatistics[bucketSizes.length]; for (int i = 0; i < stats.length; ++i) stats[i] = bucketSizeInfos[i].statistics(); Modified: hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java URL: http://svn.apache.org/viewvc/hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java?rev=1553892&r1=1553891&r2=1553892&view=diff ============================================================================== --- hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java (original) +++ hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java Sat Dec 28 19:18:21 2013 @@ -616,6 +616,8 @@ public class BucketCache implements Heap + StringUtils.byteDesc(memory)); } + } catch (Throwable t) { + LOG.warn("Failed freeing space", t); } finally { cacheStats.evict(); freeInProgress = false;
