cassandra-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bened...@apache.org
Subject [2/3] cassandra git commit: Fix race condition on proximal SecondaryIndex drop/create
Date Mon, 10 Aug 2015 14:37:30 GMT
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 <stefania.alborghetti@datastax.com>
Authored: Wed Jul 29 08:35:33 2015 +0800
Committer: Benedict Elliott Smith <benedict@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);


Mime
View raw message