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 <branimir.lam...@datastax.com> Authored: Thu Aug 11 11:15:44 2016 +0300 Committer: T Jake Luciani <j...@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(); + } +}