cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mish...@apache.org
Subject [1/8] git commit: Fix potential SlabAllocator yield-starvation patch by bes; reviewed by jbellis for CASSANDRA-7133
Date Thu, 08 May 2014 22:34:58 GMT
Repository: cassandra
Updated Branches:
  refs/heads/cassandra-2.0 d13f78ab8 -> d48c797f7
  refs/heads/cassandra-2.1 750235931 -> 56136946a
  refs/heads/trunk cbacf8578 -> 211d81c24


Fix potential SlabAllocator yield-starvation
patch by bes; reviewed by jbellis for CASSANDRA-7133


Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/d13f78ab
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/d13f78ab
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/d13f78ab

Branch: refs/heads/trunk
Commit: d13f78ab82c6fe4a6fbb5293126670fb7032203f
Parents: 46f7f84
Author: Jonathan Ellis <jbellis@apache.org>
Authored: Thu May 8 17:04:17 2014 -0500
Committer: Jonathan Ellis <jbellis@apache.org>
Committed: Thu May 8 17:04:17 2014 -0500

----------------------------------------------------------------------
 CHANGES.txt                                     |  2 +
 .../apache/cassandra/utils/SlabAllocator.java   | 53 +++++---------------
 2 files changed, 15 insertions(+), 40 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cassandra/blob/d13f78ab/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index ce30239..2712398 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
 2.0.9
+ * Fix potential SlabAllocator yield-starvation (CASSANDRA-7133)
  * Warn when 'USING TIMESTAMP' is used on a CAS BATCH (CASSANDRA-7067)
  * Starting threads in OutboundTcpConnectionPool constructor causes race conditions (CASSANDRA-7177)
  * return all cpu values from BackgroundActivityMonitor.readAndCompute (CASSANDRA-7183)
@@ -7,6 +8,7 @@
  * Fix potential NumberFormatException when deserializing IntegerType (CASSANDRA-7088)
  * cqlsh can't tab-complete disabling compaction (CASSANDRA-7185)
 
+
 2.0.8
  * Correctly delete scheduled range xfers (CASSANDRA-7143)
  * Make batchlog replica selection rack-aware (CASSANDRA-6551)

http://git-wip-us.apache.org/repos/asf/cassandra/blob/d13f78ab/src/java/org/apache/cassandra/utils/SlabAllocator.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/utils/SlabAllocator.java b/src/java/org/apache/cassandra/utils/SlabAllocator.java
index dedf869..20939fe 100644
--- a/src/java/org/apache/cassandra/utils/SlabAllocator.java
+++ b/src/java/org/apache/cassandra/utils/SlabAllocator.java
@@ -18,11 +18,11 @@
 package org.apache.cassandra.utils;
 
 import java.nio.ByteBuffer;
+import java.util.concurrent.ConcurrentLinkedQueue;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 
-import com.google.common.base.Preconditions;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -47,6 +47,9 @@ public class SlabAllocator extends Allocator
     private final static int REGION_SIZE = 1024 * 1024;
     private final static int MAX_CLONED_SIZE = 128 * 1024; // bigger than this don't go in
the region
 
+    // globally stash any Regions we allocate but are beaten to using, and use these up before
allocating any more
+    private static final ConcurrentLinkedQueue<Region> RACE_ALLOCATED = new ConcurrentLinkedQueue<>();
+
     private final AtomicReference<Region> currentRegion = new AtomicReference<Region>();
     private final AtomicInteger regionCount = new AtomicInteger(0);
     private AtomicLong unslabbed = new AtomicLong(0);
@@ -92,19 +95,20 @@ public class SlabAllocator extends Allocator
                 return region;
 
             // No current region, so we want to allocate one. We race
-            // against other allocators to CAS in an uninitialized region
-            // (which is cheap to allocate)
-            region = new Region(REGION_SIZE);
+            // against other allocators to CAS in a Region, and if we fail we stash the region
for re-use
+            region = RACE_ALLOCATED.poll();
+            if (region == null)
+                region = new Region(REGION_SIZE);
             if (currentRegion.compareAndSet(null, region))
             {
-                // we won race - now we need to actually do the expensive allocation step
-                region.init();
                 regionCount.incrementAndGet();
                 logger.trace("{} regions now allocated in {}", regionCount, this);
                 return region;
             }
+
             // someone else won race - that's fine, we'll try to grab theirs
             // in the next iteration of the loop.
+            RACE_ALLOCATED.add(region);
         }
     }
 
@@ -129,24 +133,18 @@ public class SlabAllocator extends Allocator
         /**
          * Actual underlying data
          */
-        private ByteBuffer data;
+        private final ByteBuffer data;
 
-        private static final int UNINITIALIZED = -1;
         /**
          * Offset for the next allocation, or the sentinel value -1
          * which implies that the region is still uninitialized.
          */
-        private AtomicInteger nextFreeOffset = new AtomicInteger(UNINITIALIZED);
+        private final AtomicInteger nextFreeOffset = new AtomicInteger(0);
 
         /**
          * Total number of allocations satisfied from this buffer
          */
-        private AtomicInteger allocCount = new AtomicInteger();
-
-        /**
-         * Size of region in bytes
-         */
-        private final int size;
+        private final AtomicInteger allocCount = new AtomicInteger();
 
         /**
          * Create an uninitialized region. Note that memory is not allocated yet, so
@@ -156,23 +154,7 @@ public class SlabAllocator extends Allocator
          */
         private Region(int size)
         {
-            this.size = size;
-        }
-
-        /**
-         * Actually claim the memory for this region. This should only be called from
-         * the thread that constructed the region. It is thread-safe against other
-         * threads calling alloc(), who will block until the allocation is complete.
-         */
-        public void init()
-        {
-            assert nextFreeOffset.get() == UNINITIALIZED;
             data = ByteBuffer.allocate(size);
-            assert data.remaining() == data.capacity();
-            // Mark that it's ready for use
-            boolean initted = nextFreeOffset.compareAndSet(UNINITIALIZED, 0);
-            // We should always succeed the above CAS since only one thread calls init()!
-            Preconditions.checkState(initted, "Multiple threads tried to init same region");
         }
 
         /**
@@ -185,15 +167,6 @@ public class SlabAllocator extends Allocator
             while (true)
             {
                 int oldOffset = nextFreeOffset.get();
-                if (oldOffset == UNINITIALIZED)
-                {
-                    // The region doesn't have its data allocated yet.
-                    // Since we found this in currentRegion, we know that whoever
-                    // CAS-ed it there is allocating it right now. So spin-loop
-                    // shouldn't spin long!
-                    Thread.yield();
-                    continue;
-                }
 
                 if (oldOffset + size > data.capacity()) // capacity == remaining
                     return null;


Mime
View raw message