From b54de30a0226daafaa6ecf0257371e6e8c2a0742 Mon Sep 17 00:00:00 2001
From: Jonathan Ellis <jbellis@apache.org>
Date: Mon, 6 Jul 2009 19:58:13 +0000
Subject: [PATCH] per-table key cache size. patch by jbellis; reviewed by Jun
 Rao for CASSANDRA-259

git-svn-id: https://svn.apache.org/repos/asf/incubator/cassandra/trunk@791592 13f79535-47bb-0310-9956-ffa450edef68
---
 conf/storage-conf.xml                         | 16 +++++---
 .../cassandra/config/DatabaseDescriptor.java  | 14 +++----
 .../apache/cassandra/db/BinaryMemtable.java   |  5 +--
 .../cassandra/db/ColumnFamilyStore.java       |  8 ++--
 .../apache/cassandra/db/ColumnIterator.java   |  3 +-
 .../org/apache/cassandra/db/Memtable.java     |  2 +-
 src/java/org/apache/cassandra/db/Table.java   |  2 +-
 .../apache/cassandra/io/SSTableReader.java    | 37 +++++++++++++++----
 .../apache/cassandra/io/SSTableWriter.java    |  8 +++-
 .../org/apache/cassandra/io/SSTableTest.java  |  4 +-
 10 files changed, 64 insertions(+), 35 deletions(-)

diff --git a/conf/storage-conf.xml b/conf/storage-conf.xml
index 8f83ecebf2..e79f4c6ab5 100644
--- a/conf/storage-conf.xml
+++ b/conf/storage-conf.xml
@@ -33,11 +33,17 @@
     -->
     <Tables>
         <Table Name="Table1">
-            <!-- Key cache size is the fraction of keys per sstable whose locations
-                 we keep in memory in "mostly LRU" order.  (JUST the key locations,
-                 NOT any column values.)  Consider increasing this is fine if you have
-                 fewer, wider rows. -->
-            <KeyCacheSize>0.01</KeyCacheSize>
+            <!-- The fraction of keys per sstable whose locations we
+                 keep in memory in "mostly LRU" order.  (JUST the key
+                 locations, NOT any column values.)
+
+                 The amount of memory used by the default setting of
+                 0.01 is comparable to the amount used by the internal
+                 per-sstable key index. Consider increasing this is
+                 fine if you have fewer, wider rows.  Set to 0 to
+                 disable entirely.
+            -->
+            <KeysCachedFraction>0.01</KeysCachedFraction>
             <!-- if FlushPeriodInMinutes is configured and positive, it will be
                  flushed to disk with that period whether it is dirty or not.
                  This is intended for lightly-used columnfamilies so that they
diff --git a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
index de6cd612c3..8d4e35a6f6 100644
--- a/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
+++ b/src/java/org/apache/cassandra/config/DatabaseDescriptor.java
@@ -73,7 +73,7 @@ public class DatabaseDescriptor
     private static String d_columnValue_      = "COLUMN_VALUE";
     private static String d_columnTimestamp_  = "COLUMN_TIMESTAMP";
 
-    private static Map<String, Double> tableKeyCacheSizes_;
+    private static Map<String, Double> tableKeysCachedFractions_;
     /*
      * A map from table names to the set of column families for the table and the
      * corresponding meta data for that column family.
@@ -273,7 +273,7 @@ public class DatabaseDescriptor
                 CommitLog.setSegmentSize(Integer.parseInt(value) * 1024 * 1024);
 
             tableToCFMetaDataMap_ = new HashMap<String, Map<String, CFMetaData>>();
-            tableKeyCacheSizes_ = new HashMap<String, Double>();
+            tableKeysCachedFractions_ = new HashMap<String, Double>();
 
             /* Rack Aware option */
             value = xmlUtils.getNodeValue("/Storage/RackAware");
@@ -300,15 +300,15 @@ public class DatabaseDescriptor
                 tables_.add(tName);
                 tableToCFMetaDataMap_.put(tName, new HashMap<String, CFMetaData>());
 
-                String xqlCacheSize = "/Storage/Tables/Table[@Name='" + tName + "']/KeyCacheSize";
+                String xqlCacheSize = "/Storage/Tables/Table[@Name='" + tName + "']/KeysCachedFraction";
                 value = xmlUtils.getNodeValue(xqlCacheSize);
                 if (value == null)
                 {
-                    tableKeyCacheSizes_.put(tName, 0.01);
+                    tableKeysCachedFractions_.put(tName, 0.01);
                 }
                 else
                 {
-                    tableKeyCacheSizes_.put(tName, Double.valueOf(value));
+                    tableKeysCachedFractions_.put(tName, Double.valueOf(value));
                 }
 
                 String xqlTable = "/Storage/Tables/Table[@Name='" + tName + "']/";
@@ -793,9 +793,9 @@ public class DatabaseDescriptor
         return tableToCFMetaDataMap_;
     }
 
-    public static double getKeyCacheSize(String tableName)
+    public static double getKeysCachedFraction(String tableName)
     {
-        return tableKeyCacheSizes_.get(tableName);
+        return tableKeysCachedFractions_.get(tableName);
     }
 
     private static class ConfigurationException extends Exception
diff --git a/src/java/org/apache/cassandra/db/BinaryMemtable.java b/src/java/org/apache/cassandra/db/BinaryMemtable.java
index c1bc9eb428..c07320ae4d 100644
--- a/src/java/org/apache/cassandra/db/BinaryMemtable.java
+++ b/src/java/org/apache/cassandra/db/BinaryMemtable.java
@@ -28,10 +28,9 @@ import java.util.concurrent.locks.Condition;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
-import org.apache.cassandra.io.SSTableReader;
 import org.apache.cassandra.io.SSTableWriter;
-import org.apache.cassandra.io.SSTable;
 import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.config.DatabaseDescriptor;
 
 import org.apache.log4j.Logger;
 import org.cliffc.high_scale_lib.NonBlockingHashMap;
@@ -157,7 +156,7 @@ public class BinaryMemtable
                 writer.append(key, bytes);
             }
         }
-        cfStore.storeLocation(writer.closeAndOpenReader());
+        cfStore.storeLocation(writer.closeAndOpenReader(DatabaseDescriptor.getKeysCachedFraction(table_)));
         columnFamilies_.clear();       
     }
 }
diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
index dda9f5f649..1e8147c5d2 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
@@ -180,7 +180,7 @@ public final class ColumnFamilyStore implements ColumnFamilyStoreMBean
             String filename = file.getAbsolutePath();
             try
             {
-                SSTableReader sstable = SSTableReader.open(filename, StorageService.getPartitioner());
+                SSTableReader sstable = SSTableReader.open(filename);
                 ssTables_.put(filename, sstable);
             }
             catch (IOException ex)
@@ -985,7 +985,7 @@ public final class ColumnFamilyStore implements ColumnFamilyStoreMBean
                 if (logger_.isDebugEnabled())
                   logger_.debug("New file : " + newfile + " of size " + new File(newfile).length());
                 assert newfile != null;
-                ssTables_.put(newfile, SSTableReader.open(newfile, StorageService.getPartitioner()));
+                ssTables_.put(newfile, SSTableReader.open(newfile));
             }
             SSTableReader.get(file).delete();
         }
@@ -1174,7 +1174,7 @@ public final class ColumnFamilyStore implements ColumnFamilyStoreMBean
 
         if (rangeWriter != null)
         {
-            rangeWriter.closeAndOpenReader();
+            rangeWriter.closeAndOpenReader(DatabaseDescriptor.getKeysCachedFraction(table_));
             if (fileList != null)
             {
                 fileList.add(rangeWriter.getFilename());
@@ -1347,7 +1347,7 @@ public final class ColumnFamilyStore implements ColumnFamilyStoreMBean
         if (writer != null)
         {
             // TODO if all the keys were the same nothing will be done here
-            ssTable = writer.closeAndOpenReader();
+            ssTable = writer.closeAndOpenReader(DatabaseDescriptor.getKeysCachedFraction(table_));
             newfile = writer.getFilename();
         }
         lock_.writeLock().lock();
diff --git a/src/java/org/apache/cassandra/db/ColumnIterator.java b/src/java/org/apache/cassandra/db/ColumnIterator.java
index b28ea5531b..42a184b775 100644
--- a/src/java/org/apache/cassandra/db/ColumnIterator.java
+++ b/src/java/org/apache/cassandra/db/ColumnIterator.java
@@ -25,7 +25,6 @@ import org.apache.cassandra.io.DataInputBuffer;
 import org.apache.cassandra.io.DataOutputBuffer;
 import org.apache.cassandra.io.SSTableReader;
 import org.apache.cassandra.io.SequenceFile.ColumnGroupReader;
-import org.apache.cassandra.service.StorageService;
 import com.google.common.collect.AbstractIterator;
 
 public interface ColumnIterator extends Iterator<IColumn>
@@ -58,7 +57,7 @@ class SSTableColumnIterator extends AbstractIterator<IColumn> implements ColumnI
     throws IOException
     {
         this.isAscending = isAscending;
-        SSTableReader ssTable = SSTableReader.open(filename, StorageService.getPartitioner());
+        SSTableReader ssTable = SSTableReader.open(filename);
         reader = ssTable.getColumnGroupReader(key, cfName, startColumn, isAscending);
         this.startColumn = startColumn;
         curColumnIndex = isAscending ? 0 : -1;
diff --git a/src/java/org/apache/cassandra/db/Memtable.java b/src/java/org/apache/cassandra/db/Memtable.java
index 23c76002b1..6b325ff7a9 100644
--- a/src/java/org/apache/cassandra/db/Memtable.java
+++ b/src/java/org/apache/cassandra/db/Memtable.java
@@ -277,7 +277,7 @@ public class Memtable implements Comparable<Memtable>
                 writer.append(partitioner.decorateKey(key), buffer);
             }
         }
-        SSTableReader ssTable = writer.closeAndOpenReader();
+        SSTableReader ssTable = writer.closeAndOpenReader(DatabaseDescriptor.getKeysCachedFraction(table_));
         cfStore.onMemtableFlush(cLogCtx);
         cfStore.storeLocation(ssTable);
         buffer.close();
diff --git a/src/java/org/apache/cassandra/db/Table.java b/src/java/org/apache/cassandra/db/Table.java
index edc990dda4..6e6d054d75 100644
--- a/src/java/org/apache/cassandra/db/Table.java
+++ b/src/java/org/apache/cassandra/db/Table.java
@@ -185,7 +185,7 @@ public class Table
                  * list of the associated Column Family. Also merge the CBF into the
                  * sampler.
                 */                
-                SSTableReader sstable = SSTableReader.open(streamContext.getTargetFile(), StorageService.getPartitioner());
+                SSTableReader sstable = SSTableReader.open(streamContext.getTargetFile());
                 logger_.debug("Merging the counting bloom filter in the sampler ...");                
                 String[] peices = FBUtilities.strip(fileName, "-");
                 Table.open(peices[0]).getColumnFamilyStore(peices[1]).addToList(sstable);                
diff --git a/src/java/org/apache/cassandra/io/SSTableReader.java b/src/java/org/apache/cassandra/io/SSTableReader.java
index 40c2faee47..41e2f41812 100644
--- a/src/java/org/apache/cassandra/io/SSTableReader.java
+++ b/src/java/org/apache/cassandra/io/SSTableReader.java
@@ -28,6 +28,8 @@ import org.apache.cassandra.dht.IPartitioner;
 import org.apache.cassandra.io.SequenceFile.ColumnGroupReader;
 import org.apache.cassandra.utils.BloomFilter;
 import org.apache.cassandra.utils.FileUtils;
+import org.apache.cassandra.service.StorageService;
+import org.apache.cassandra.config.DatabaseDescriptor;
 import org.cliffc.high_scale_lib.NonBlockingHashMap;
 import com.reardencommerce.kernel.collections.shared.evictable.ConcurrentLinkedHashMap;
 
@@ -78,7 +80,12 @@ public class SSTableReader extends SSTable
         return indexedKeys;
     }
 
-    public static synchronized SSTableReader open(String dataFileName, IPartitioner partitioner) throws IOException
+    public static synchronized SSTableReader open(String dataFileName) throws IOException
+    {
+        return open(dataFileName, StorageService.getPartitioner(), DatabaseDescriptor.getKeysCachedFraction(parseTableName(dataFileName)));
+    }
+
+    public static synchronized SSTableReader open(String dataFileName, IPartitioner partitioner, double cacheFraction) throws IOException
     {
         SSTableReader sstable = openedFiles.get(dataFileName);
         if (sstable == null)
@@ -89,6 +96,10 @@ public class SSTableReader extends SSTable
             long start = System.currentTimeMillis();
             sstable.loadIndexFile();
             sstable.loadBloomFilter();
+            if (cacheFraction > 0)
+            {
+                sstable.keyCache = createKeyCache((int)((sstable.getIndexPositions().size() + 1) * INDEX_INTERVAL * cacheFraction));
+            }
             logger.debug("INDEX LOAD TIME for "  + dataFileName + ": " + (System.currentTimeMillis() - start) + " ms.");
 
             openedFiles.put(dataFileName, sstable);
@@ -103,14 +114,20 @@ public class SSTableReader extends SSTable
         return sstable;
     }
 
+    public static ConcurrentLinkedHashMap<String, Long> createKeyCache(int size)
+    {
+        return ConcurrentLinkedHashMap.create(ConcurrentLinkedHashMap.EvictionPolicy.SECOND_CHANCE, size);
+    }
 
-    private ConcurrentLinkedHashMap<String, Long> keyCache = ConcurrentLinkedHashMap.create(ConcurrentLinkedHashMap.EvictionPolicy.SECOND_CHANCE, 1000);
 
-    SSTableReader(String filename, IPartitioner partitioner, List<KeyPosition> indexPositions, BloomFilter bloomFilter)
+    private ConcurrentLinkedHashMap<String, Long> keyCache;
+
+    SSTableReader(String filename, IPartitioner partitioner, List<KeyPosition> indexPositions, BloomFilter bloomFilter, ConcurrentLinkedHashMap<String, Long> keyCache)
     {
         super(filename, partitioner);
         this.indexPositions = indexPositions;
         this.bf = bloomFilter;
+        this.keyCache = keyCache;
         synchronized (SSTableReader.this)
         {
             openedFiles.put(filename, this);
@@ -183,10 +200,13 @@ public class SSTableReader extends SSTable
     {
         if (!bf.isPresent(decoratedKey))
             return -1;
-        Long cachedPosition = keyCache.get(decoratedKey);
-        if (cachedPosition != null)
+        if (keyCache != null)
         {
-            return cachedPosition;
+            Long cachedPosition = keyCache.get(decoratedKey);
+            if (cachedPosition != null)
+            {
+                return cachedPosition;
+            }
         }
         long start = getIndexScanPosition(decoratedKey, partitioner);
         if (start < 0)
@@ -215,7 +235,8 @@ public class SSTableReader extends SSTable
                 int v = partitioner.getDecoratedKeyComparator().compare(indexDecoratedKey, decoratedKey);
                 if (v == 0)
                 {
-                    keyCache.put(decoratedKey, position);
+                    if (keyCache != null)
+                        keyCache.put(decoratedKey, position);
                     return position;
                 }
                 if (v > 0)
@@ -350,7 +371,7 @@ public class SSTableReader extends SSTable
         openedFiles.clear();
         for (SSTableReader sstable : sstables)
         {
-            SSTableReader.open(sstable.dataFile, sstable.partitioner);
+            SSTableReader.open(sstable.dataFile, sstable.partitioner, 0.01);
         }
     }
 
diff --git a/src/java/org/apache/cassandra/io/SSTableWriter.java b/src/java/org/apache/cassandra/io/SSTableWriter.java
index 2599ba84c7..3a5cb377a3 100644
--- a/src/java/org/apache/cassandra/io/SSTableWriter.java
+++ b/src/java/org/apache/cassandra/io/SSTableWriter.java
@@ -11,6 +11,7 @@ import org.apache.log4j.Logger;
 
 import org.apache.cassandra.dht.IPartitioner;
 import org.apache.cassandra.utils.BloomFilter;
+import com.reardencommerce.kernel.collections.shared.evictable.ConcurrentLinkedHashMap;
 
 public class SSTableWriter extends SSTable
 {
@@ -91,7 +92,7 @@ public class SSTableWriter extends SSTable
     /**
      * Renames temporary SSTable files to valid data, index, and bloom filter files
      */
-    public SSTableReader closeAndOpenReader() throws IOException
+    public SSTableReader closeAndOpenReader(double cacheFraction) throws IOException
     {
         // bloom filter
         FileOutputStream fos = new FileOutputStream(filterFilename());
@@ -112,7 +113,10 @@ public class SSTableWriter extends SSTable
         rename(filterFilename());
         dataFile = rename(dataFile); // important to do this last since index & filter file names are derived from it
 
-        return new SSTableReader(dataFile, partitioner, indexPositions, bf);
+        ConcurrentLinkedHashMap<String,Long> keyCache = cacheFraction > 0
+                                                        ? SSTableReader.createKeyCache((int) (cacheFraction * keysWritten))
+                                                        : null;
+        return new SSTableReader(dataFile, partitioner, indexPositions, bf, keyCache);
     }
 
 }
diff --git a/test/unit/org/apache/cassandra/io/SSTableTest.java b/test/unit/org/apache/cassandra/io/SSTableTest.java
index e7eb83f7de..4a74a3daf7 100644
--- a/test/unit/org/apache/cassandra/io/SSTableTest.java
+++ b/test/unit/org/apache/cassandra/io/SSTableTest.java
@@ -42,7 +42,7 @@ public class SSTableTest extends CleanupHelper
 
         String key = Integer.toString(1);
         writer.append(key, bytes);
-        SSTableReader ssTable = writer.closeAndOpenReader();
+        SSTableReader ssTable = writer.closeAndOpenReader(0.01);
 
         // verify
         verifySingle(ssTable, bytes, key);
@@ -81,7 +81,7 @@ public class SSTableTest extends CleanupHelper
         {
             writer.append(key, map.get(key));
         }
-        SSTableReader ssTable = writer.closeAndOpenReader();
+        SSTableReader ssTable = writer.closeAndOpenReader(0.01);
 
         // verify
         verifyMany(ssTable, map);