hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From li...@apache.org
Subject svn commit: r1553892 - in /hbase/branches/0.89-fb/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket: BucketAllocator.java BucketCache.java
Date Sat, 28 Dec 2013 19:18:21 GMT
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;



Mime
View raw message