You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cassandra.apache.org by ja...@apache.org on 2016/08/12 15:30:28 UTC
[3/8] cassandra git commit: Fix clean interval not sent to commit log
for empty memtable flush
Fix clean interval not sent to commit log for empty memtable flush
patch by Branimir Lambov; reviewed by Jake Luciani for
CASSANDRA-12436
Project: http://git-wip-us.apache.org/repos/asf/cassandra/repo
Commit: http://git-wip-us.apache.org/repos/asf/cassandra/commit/5cef78af
Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/5cef78af
Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/5cef78af
Branch: refs/heads/trunk
Commit: 5cef78af9214308ef817de43ec36054a2549329c
Parents: c85749e
Author: Branimir Lambov <br...@datastax.com>
Authored: Thu Aug 11 11:15:44 2016 +0300
Committer: T Jake Luciani <ja...@apache.org>
Committed: Fri Aug 12 11:24:16 2016 -0400
----------------------------------------------------------------------
CHANGES.txt | 1 +
.../apache/cassandra/db/ColumnFamilyStore.java | 19 +++------
.../cassandra/db/commitlog/CommitLog.java | 5 ++-
.../db/commitlog/CommitLogSegment.java | 5 ++-
.../db/commitlog/CommitLogCQLTest.java | 41 ++++++++++++++++++++
5 files changed, 54 insertions(+), 17 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5cef78af/CHANGES.txt
----------------------------------------------------------------------
diff --git a/CHANGES.txt b/CHANGES.txt
index 959b967..cccb62d 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,4 +1,5 @@
3.0.9
+ * Fix clean interval not sent to commit log for empty memtable flush (CASSANDRA-12436)
* Fix potential resource leak in RMIServerSocketFactoryImpl (CASSANDRA-12331)
* Backport CASSANDRA-12002 (CASSANDRA-12177)
* Make sure compaction stats are updated when compaction is interrupted (CASSANDRA-12100)
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5cef78af/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 82604e2..6c02909 100644
--- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
+++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java
@@ -1039,20 +1039,9 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
writeBarrier.markBlocking();
writeBarrier.await();
- // mark all memtables as flushing, removing them from the live memtable list, and
- // remove any memtables that are already clean from the set we need to flush
- Iterator<Memtable> iter = memtables.iterator();
- while (iter.hasNext())
- {
- Memtable memtable = iter.next();
+ // mark all memtables as flushing, removing them from the live memtable list
+ for (Memtable memtable : memtables)
memtable.cfs.data.markFlushing(memtable);
- if (memtable.isClean() || truncate)
- {
- memtable.cfs.replaceFlushed(memtable, Collections.emptyList());
- reclaim(memtable);
- iter.remove();
- }
- }
metric.memtableSwitchCount.inc();
@@ -1060,7 +1049,9 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean
{
for (Memtable memtable : memtables)
{
- Collection<SSTableReader> readers = memtable.flush();
+ Collection<SSTableReader> readers = Collections.emptyList();
+ if (!memtable.isClean() && !truncate)
+ readers = memtable.flush();
memtable.cfs.replaceFlushed(memtable, readers);
reclaim(memtable);
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5cef78af/src/java/org/apache/cassandra/db/commitlog/CommitLog.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/commitlog/CommitLog.java b/src/java/org/apache/cassandra/db/commitlog/CommitLog.java
index dfe3f91..d167735 100644
--- a/src/java/org/apache/cassandra/db/commitlog/CommitLog.java
+++ b/src/java/org/apache/cassandra/db/commitlog/CommitLog.java
@@ -313,8 +313,9 @@ public class CommitLog implements CommitLogMBean
}
else
{
- logger.trace("Not safe to delete{} commit log segment {}; dirty is {}",
- (iter.hasNext() ? "" : " active"), segment, segment.dirtyString());
+ if (logger.isTraceEnabled())
+ logger.trace("Not safe to delete{} commit log segment {}; dirty is {}",
+ (iter.hasNext() ? "" : " active"), segment, segment.dirtyString());
}
// Don't mark or try to delete any newer segments once we've reached the one containing the
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5cef78af/src/java/org/apache/cassandra/db/commitlog/CommitLogSegment.java
----------------------------------------------------------------------
diff --git a/src/java/org/apache/cassandra/db/commitlog/CommitLogSegment.java b/src/java/org/apache/cassandra/db/commitlog/CommitLogSegment.java
index d2f12bf..0a03c3c 100644
--- a/src/java/org/apache/cassandra/db/commitlog/CommitLogSegment.java
+++ b/src/java/org/apache/cassandra/db/commitlog/CommitLogSegment.java
@@ -536,7 +536,10 @@ public abstract class CommitLogSegment
for (UUID cfId : getDirtyCFIDs())
{
CFMetaData m = Schema.instance.getCFMetaData(cfId);
- sb.append(m == null ? "<deleted>" : m.cfName).append(" (").append(cfId).append("), ");
+ sb.append(m == null ? "<deleted>" : m.cfName).append(" (").append(cfId)
+ .append(", dirty: ").append(cfDirty.get(cfId))
+ .append(", clean: ").append(cfClean.get(cfId))
+ .append("), ");
}
return sb.toString();
}
http://git-wip-us.apache.org/repos/asf/cassandra/blob/5cef78af/test/unit/org/apache/cassandra/db/commitlog/CommitLogCQLTest.java
----------------------------------------------------------------------
diff --git a/test/unit/org/apache/cassandra/db/commitlog/CommitLogCQLTest.java b/test/unit/org/apache/cassandra/db/commitlog/CommitLogCQLTest.java
new file mode 100644
index 0000000..8537ebb
--- /dev/null
+++ b/test/unit/org/apache/cassandra/db/commitlog/CommitLogCQLTest.java
@@ -0,0 +1,41 @@
+package org.apache.cassandra.db.commitlog;
+
+import java.util.ArrayList;
+import java.util.Collection;
+
+import org.junit.Test;
+
+import org.apache.cassandra.cql3.CQLTester;
+import org.apache.cassandra.db.ColumnFamilyStore;
+
+public class CommitLogCQLTest extends CQLTester
+{
+ @Test
+ public void testTruncateSegmentDiscard() throws Throwable
+ {
+ String otherTable = createTable("CREATE TABLE %s (idx INT, data TEXT, PRIMARY KEY(idx));");
+
+ createTable("CREATE TABLE %s (idx INT, data TEXT, PRIMARY KEY(idx));");
+ execute("INSERT INTO %s (idx, data) VALUES (?, ?)", 15, Integer.toString(15));
+ flush();
+
+ // We write something in different table to advance the commit log position. Current table remains clean.
+ execute(String.format("INSERT INTO %s.%s (idx, data) VALUES (?, ?)", keyspace(), otherTable), 16, Integer.toString(16));
+
+ ColumnFamilyStore cfs = getCurrentColumnFamilyStore();
+ assert cfs.getTracker().getView().getCurrentMemtable().isClean();
+ // Calling switchMemtable directly applies Flush even though memtable is empty. This can happen with some races
+ // (flush with recycling by segment manager). It should still tell commitlog that the memtable's region is clean.
+ // CASSANDRA-12436
+ cfs.switchMemtable();
+
+ execute("INSERT INTO %s (idx, data) VALUES (?, ?)", 15, Integer.toString(17));
+
+ Collection<CommitLogSegment> active = new ArrayList<>(CommitLog.instance.allocator.getActiveSegments());
+ CommitLog.instance.forceRecycleAllSegments();
+
+ // If one of the previous segments remains, it wasn't clean.
+ active.retainAll(CommitLog.instance.allocator.getActiveSegments());
+ assert active.isEmpty();
+ }
+}