You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by jb...@apache.org on 2009/05/12 21:11:21 UTC
svn commit: r774023 - in /incubator/cassandra/trunk:
src/java/org/apache/cassandra/db/ColumnFamilyStore.java
test/unit/org/apache/cassandra/CleanupHelper.java
test/unit/org/apache/cassandra/db/NameSortTest.java
Author: jbellis
Date: Tue May 12 19:11:21 2009
New Revision: 774023
URL: http://svn.apache.org/viewvc?rev=774023&view=rev
Log:
fix race condition in compaction -- it was possible for a read thread to "snapshot" ssTables_, then have
the compactor thread delete those (after merging them into a new file) before the read thread checked
them. Since the read thread's "snapshot" doesn't include the new merged sstable, it incorrectly tells
the caller that the key does not exist.
patch by jbellis; reviewed by Eric Evans for CASSANDRA-161
Modified:
incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
incubator/cassandra/trunk/test/unit/org/apache/cassandra/CleanupHelper.java
incubator/cassandra/trunk/test/unit/org/apache/cassandra/db/NameSortTest.java
Modified: incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
URL: http://svn.apache.org/viewvc/incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java?rev=774023&r1=774022&r2=774023&view=diff
==============================================================================
--- incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java (original)
+++ incubator/cassandra/trunk/src/java/org/apache/cassandra/db/ColumnFamilyStore.java Tue May 12 19:11:21 2009
@@ -83,7 +83,7 @@
private AtomicReference<BinaryMemtable> binaryMemtable_;
/* SSTables on disk for this column family */
- private Set<String> ssTables_ = new HashSet<String>();
+ private Set<String> ssTables_ = new TreeSet<String>(new FileNameComparator(FileNameComparator.Descending));
/* Modification lock used for protecting reads from compactions. */
private ReentrantReadWriteLock lock_ = new ReentrantReadWriteLock(true);
@@ -557,40 +557,35 @@
*/
private void getColumnFamilyFromDisk(String key, String cf, List<ColumnFamily> columnFamilies, IFilter filter) throws IOException
{
- /* Scan the SSTables on disk first */
- List<String> files = new ArrayList<String>();
lock_.readLock().lock();
try
{
- files.addAll(ssTables_);
- Collections.sort(files, new FileNameComparator(FileNameComparator.Descending));
- }
- finally
- {
- lock_.readLock().unlock();
- }
-
- for (String file : files)
- {
- /*
- * Get the BloomFilter associated with this file. Check if the key
- * is present in the BloomFilter. If not continue to the next file.
- */
- boolean bVal = SSTable.isKeyInFile(key, file);
- if (!bVal)
- {
- continue;
- }
- ColumnFamily columnFamily = fetchColumnFamily(key, cf, filter, file);
- if (columnFamily != null)
+ for (String file : ssTables_)
{
- columnFamilies.add(columnFamily);
- if (filter.isDone())
+ /*
+ * Get the BloomFilter associated with this file. Check if the key
+ * is present in the BloomFilter. If not continue to the next file.
+ */
+ boolean bVal = SSTable.isKeyInFile(key, file);
+ if (!bVal)
{
- break;
+ continue;
+ }
+ ColumnFamily columnFamily = fetchColumnFamily(key, cf, filter, file);
+ if (columnFamily != null)
+ {
+ columnFamilies.add(columnFamily);
+ if (filter.isDone())
+ {
+ break;
+ }
}
}
}
+ finally
+ {
+ lock_.readLock().unlock();
+ }
}
private ColumnFamily fetchColumnFamily(String key, String cf, IFilter filter, String ssTableFile) throws IOException
@@ -1423,7 +1418,6 @@
}
if (newfile != null)
{
- logger_.debug("Inserting bloom filter for file " + newfile);
SSTable.storeBloomFilter(newfile, compactedBloomFilter);
ssTables_.add(newfile);
totalBytesWritten += (new File(newfile)).length();
Modified: incubator/cassandra/trunk/test/unit/org/apache/cassandra/CleanupHelper.java
URL: http://svn.apache.org/viewvc/incubator/cassandra/trunk/test/unit/org/apache/cassandra/CleanupHelper.java?rev=774023&r1=774022&r2=774023&view=diff
==============================================================================
--- incubator/cassandra/trunk/test/unit/org/apache/cassandra/CleanupHelper.java (original)
+++ incubator/cassandra/trunk/test/unit/org/apache/cassandra/CleanupHelper.java Tue May 12 19:11:21 2009
@@ -5,12 +5,12 @@
import org.junit.BeforeClass;
import org.apache.cassandra.config.DatabaseDescriptor;
-import org.apache.cassandra.db.ColumnFamilyStore;
-import org.apache.cassandra.db.CommitLog;
-import org.apache.cassandra.db.Table;
+import org.apache.log4j.Logger;
public class CleanupHelper
{
+ private static Logger logger = Logger.getLogger(CleanupHelper.class);
+
@BeforeClass
public static void cleanup()
{
@@ -32,6 +32,7 @@
}
for (File f : dir.listFiles())
{
+ logger.debug("deleting " + f);
f.delete();
}
}
Modified: incubator/cassandra/trunk/test/unit/org/apache/cassandra/db/NameSortTest.java
URL: http://svn.apache.org/viewvc/incubator/cassandra/trunk/test/unit/org/apache/cassandra/db/NameSortTest.java?rev=774023&r1=774022&r2=774023&view=diff
==============================================================================
--- incubator/cassandra/trunk/test/unit/org/apache/cassandra/db/NameSortTest.java (original)
+++ incubator/cassandra/trunk/test/unit/org/apache/cassandra/db/NameSortTest.java Tue May 12 19:11:21 2009
@@ -89,7 +89,7 @@
}
cf = table.get(key, "Super1");
- assert cf != null;
+ assert cf != null : "key " + key + " is missing!";
Collection<IColumn> superColumns = cf.getAllColumns();
assert superColumns.size() == 8;
for (IColumn superColumn : superColumns)