You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by be...@apache.org on 2015/08/10 16:37:30 UTC
[2/3] cassandra git commit: Fix race condition on proximal
SecondaryIndex drop/create
Fix race condition on proximal SecondaryIndex drop/create
When a SecondaryIndex is dropped and recreated immediately,
the old files (and transaction logs) can be reused erroneously.
This patch ensures all such files have either been deleted or are
in a transaction log marking them deleted before a SecondaryIndex
is recreated, and that reading a missing TransactionLog does not fail.
patch by stefania; reviewed by benedict for CASSANDRA-9908
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/45c04b72
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/45c04b72
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/45c04b72
Branch: refs/heads/trunk
Commit: 45c04b72753a738cb4a8af7025ad4cb10a12c452
Parents: e98be18
Author: Stefania Alborghetti <st...@datastax.com>
Authored: Wed Jul 29 08:35:33 2015 +0800
Committer: Benedict Elliott Smith <be...@apache.org>
Committed: Mon Aug 10 16:36:41 2015 +0200
----------------------------------------------------------------------
.../apache/cassandra/db/ColumnFamilyStore.java | 5 +++-
.../AbstractSimplePerColumnSecondaryIndex.java | 12 +++++++++
.../cassandra/db/lifecycle/TransactionLogs.java | 2 +-
.../org/apache/cassandra/io/util/FileUtils.java | 6 +++++
.../cassandra/db/lifecycle/TrackerTest.java | 11 ++++++---
.../db/lifecycle/TransactionLogsTest.java | 26 +++++++++++++++++++-
6 files changed, 56 insertions(+), 6 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/45c04b72/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
index c89f16a..797e2c7 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
@@ -310,7 +310,7 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
}
}
- public ColumnFamilyStore(Keyspace keyspace,
+ private ColumnFamilyStore(Keyspace keyspace,
String columnFamilyName,
int generation,
CFMetaData metadata,
@@ -457,7 +457,10 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
latencyCalculator.cancel(false);
compactionStrategyManager.shutdown();
SystemKeyspace.removeTruncationRecord(metadata.cfId);
+
data.dropSSTables();
+ TransactionLogs.waitForDeletions();
+
indexManager.invalidate();
materializedViewManager.invalidate();
http://git-wip-us.apache.org/repos/asf/cassandra/blob/45c04b72/src/java/org/apache/cassandra/db/index/AbstractSimplePerColumnSecondaryIndex.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/index/AbstractSimplePerColumnSecondaryIndex.java b/src/java/org/apache/cassandra/db/index/AbstractSimplePerColumnSecondaryIndex.java
index 4bb0bc4..b5ed7f6 100644
--- a/src/java/org/apache/cassandra/db/index/AbstractSimplePerColumnSecondaryIndex.java
+++ b/src/java/org/apache/cassandra/db/index/AbstractSimplePerColumnSecondaryIndex.java
@@ -18,11 +18,14 @@
package org.apache.cassandra.db.index;
import java.nio.ByteBuffer;
+import java.util.Collection;
+import java.util.Collections;
import java.util.concurrent.Future;
import org.apache.cassandra.config.CFMetaData;
import org.apache.cassandra.config.ColumnDefinition;
import org.apache.cassandra.db.*;
+import org.apache.cassandra.db.compaction.CompactionManager;
import org.apache.cassandra.db.rows.*;
import org.apache.cassandra.db.partitions.*;
import org.apache.cassandra.db.marshal.AbstractType;
@@ -155,6 +158,15 @@ public abstract class AbstractSimplePerColumnSecondaryIndex extends PerColumnSec
public void removeIndex(ByteBuffer columnName)
{
+ // interrupt in-progress compactions
+ Collection<ColumnFamilyStore> cfss = Collections.singleton(indexCfs);
+ CompactionManager.instance.interruptCompactionForCFs(cfss, true);
+ CompactionManager.instance.waitForCessation(cfss);
+
+ indexCfs.keyspace.writeOrder.awaitNewBarrier();
+ indexCfs.forceBlockingFlush();
+
+ indexCfs.readOrdering.awaitNewBarrier();
indexCfs.invalidate();
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/45c04b72/src/java/org/apache/cassandra/db/lifecycle/TransactionLogs.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/lifecycle/TransactionLogs.java b/src/java/org/apache/cassandra/db/lifecycle/TransactionLogs.java
index 80e7831..821f58c 100644
--- a/src/java/org/apache/cassandra/db/lifecycle/TransactionLogs.java
+++ b/src/java/org/apache/cassandra/db/lifecycle/TransactionLogs.java
@@ -479,7 +479,7 @@ public class TransactionLogs extends Transactional.AbstractTransactional impleme
}
catch (NoSuchFileException e)
{
- logger.warn("Unable to delete {} as it does not exist", file);
+ logger.error("Unable to delete {} as it does not exist", file);
}
catch (IOException e)
{
http://git-wip-us.apache.org/repos/asf/cassandra/blob/45c04b72/src/java/org/apache/cassandra/io/util/FileUtils.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/io/util/FileUtils.java b/src/java/org/apache/cassandra/io/util/FileUtils.java
index f415f2b..c3de1db 100644
--- a/src/java/org/apache/cassandra/io/util/FileUtils.java
+++ b/src/java/org/apache/cassandra/io/util/FileUtils.java
@@ -24,6 +24,7 @@ import java.nio.charset.Charset;
import java.nio.file.*;
import java.text.DecimalFormat;
import java.util.Arrays;
+import java.util.Collections;
import java.util.List;
import org.apache.cassandra.config.Config;
@@ -615,6 +616,11 @@ public class FileUtils
{
return Files.readAllLines(file.toPath(), Charset.forName("utf-8"));
}
+ catch (NoSuchFileException ex)
+ {
+ logger.warn("Tried to read non existing file: {}", file);
+ return Collections.emptyList();
+ }
catch (IOException ex)
{
throw new RuntimeException(ex);
http://git-wip-us.apache.org/repos/asf/cassandra/blob/45c04b72/test/unit/org/apache/cassandra/db/lifecycle/TrackerTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/lifecycle/TrackerTest.java b/test/unit/org/apache/cassandra/db/lifecycle/TrackerTest.java
index 89924a5..ea0d9a8 100644
--- a/test/unit/org/apache/cassandra/db/lifecycle/TrackerTest.java
+++ b/test/unit/org/apache/cassandra/db/lifecycle/TrackerTest.java
@@ -205,14 +205,19 @@ public class TrackerTest
try
{
- TransactionLogs.pauseDeletions(true);
+ // TransactionLogs.pauseDeletions(true);
try (LifecycleTransaction txn = tracker.tryModify(readers.get(0), OperationType.COMPACTION))
{
if (invalidate)
+ {
cfs.invalidate(false);
+ }
else
+ {
tracker.dropSSTables();
- Assert.assertEquals(95, cfs.metric.totalDiskSpaceUsed.getCount());
+ TransactionLogs.waitForDeletions();
+ }
+ Assert.assertEquals(9, cfs.metric.totalDiskSpaceUsed.getCount());
Assert.assertEquals(9, cfs.metric.liveDiskSpaceUsed.getCount());
Assert.assertEquals(1, tracker.getView().sstables.size());
}
@@ -253,7 +258,7 @@ public class TrackerTest
}
finally
{
- TransactionLogs.pauseDeletions(false);
+ // TransactionLogs.pauseDeletions(false);
}
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/45c04b72/test/unit/org/apache/cassandra/db/lifecycle/TransactionLogsTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/lifecycle/TransactionLogsTest.java b/test/unit/org/apache/cassandra/db/lifecycle/TransactionLogsTest.java
index 4339877..991eed3 100644
--- a/test/unit/org/apache/cassandra/db/lifecycle/TransactionLogsTest.java
+++ b/test/unit/org/apache/cassandra/db/lifecycle/TransactionLogsTest.java
@@ -502,7 +502,31 @@ public class TransactionLogsTest extends AbstractTransactionalTest
sstable2.selfRef().release();
}
- public static SSTableReader sstable(ColumnFamilyStore cfs, int generation, int size) throws IOException
+ @Test
+ public void testGetTemporaryFilesSafeAfterObsoletion() throws Throwable
+ {
+ ColumnFamilyStore cfs = MockSchema.newCFS(KEYSPACE);
+ SSTableReader sstable = sstable(cfs, 0, 128);
+ File dataFolder = sstable.descriptor.directory;
+
+ TransactionLogs transactionLogs = new TransactionLogs(OperationType.COMPACTION, cfs.metadata);
+ assertNotNull(transactionLogs);
+
+ TransactionLogs.SSTableTidier tidier = transactionLogs.obsoleted(sstable);
+
+ transactionLogs.finish();
+ sstable.markObsolete(tidier);
+ sstable.selfRef().release();
+
+ for (int i = 0; i < 1000; i++)
+ {
+ // This should race with the asynchronous deletion of txn log files
+ // It doesn't matter what it returns but it should not throw
+ TransactionLogs.getTemporaryFiles(cfs.metadata, dataFolder);
+ }
+ }
+
+ private static SSTableReader sstable(ColumnFamilyStore cfs, int generation, int size) throws IOException
{
Directories dir = new Directories(cfs.metadata);
Descriptor descriptor = new Descriptor(dir.getDirectoryForNewSSTables(), cfs.keyspace.getName(), cfs.getColumnFamilyName(), generation);