Return-Path: X-Original-To: apmail-cassandra-commits-archive@www.apache.org Delivered-To: apmail-cassandra-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 057F918BDD for ; Mon, 10 Aug 2015 14:37:52 +0000 (UTC) Received: (qmail 23922 invoked by uid 500); 10 Aug 2015 14:37:29 -0000 Delivered-To: apmail-cassandra-commits-archive@cassandra.apache.org Received: (qmail 23817 invoked by uid 500); 10 Aug 2015 14:37:29 -0000 Mailing-List: contact commits-help@cassandra.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@cassandra.apache.org Delivered-To: mailing list commits@cassandra.apache.org Received: (qmail 23781 invoked by uid 99); 10 Aug 2015 14:37:29 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Mon, 10 Aug 2015 14:37:29 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 97130E0509; Mon, 10 Aug 2015 14:37:29 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: benedict@apache.org To: commits@cassandra.apache.org Date: Mon, 10 Aug 2015 14:37:30 -0000 Message-Id: <7f4ce1468ee94fdfad5c402b3cf29fe5@git.apache.org> In-Reply-To: <28ed4c62dd144980b299395c9b1f0337@git.apache.org> References: <28ed4c62dd144980b299395c9b1f0337@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [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 Authored: Wed Jul 29 08:35:33 2015 +0800 Committer: Benedict Elliott Smith 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 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);