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/ca556c44 Tree: http://git-wip-us.apache.org/repos/asf/cassandra/tree/ca556c44 Diff: http://git-wip-us.apache.org/repos/asf/cassandra/diff/ca556c44 Branch: refs/heads/cassandra-3.8 Commit: ca556c442049808c013bd20649d6611eb4a01a61 Parents: e60f741 Author: Branimir Lambov <branimir.lam...@datastax.com> Authored: Thu Aug 11 11:17:37 2016 +0300 Committer: T Jake Luciani <j...@apache.org> Committed: Fri Aug 12 11:29:23 2016 -0400 ---------------------------------------------------------------------- CHANGES.txt | 1 + .../apache/cassandra/db/ColumnFamilyStore.java | 22 +++++------ .../cassandra/db/commitlog/CommitLog.java | 5 ++- .../db/commitlog/CommitLogSegment.java | 5 ++- .../db/commitlog/CommitLogCQLTest.java | 41 ++++++++++++++++++++ 5 files changed, 58 insertions(+), 16 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/cassandra/blob/ca556c44/CHANGES.txt ---------------------------------------------------------------------- diff --git a/CHANGES.txt b/CHANGES.txt index b1b2050..56875ca 100644 --- a/CHANGES.txt +++ b/CHANGES.txt @@ -6,6 +6,7 @@ * cqlsh: Fix handling of $$-escaped strings (CASSANDRA-12189) * Fix SSL JMX requiring truststore containing server cert (CASSANDRA-12109) Merged from 3.0: + * Fix clean interval not sent to commit log for empty memtable flush (CASSANDRA-12436) * Fix potential resource leak in RMIServerSocketFactoryImpl (CASSANDRA-12331) * Make sure compaction stats are updated when compaction is interrupted (CASSANDRA-12100) * Change commitlog and sstables to track dirty and clean intervals (CASSANDRA-11828) http://git-wip-us.apache.org/repos/asf/cassandra/blob/ca556c44/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 21becfe..aa79e90 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -1072,20 +1072,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(); @@ -1105,6 +1094,13 @@ public class ColumnFamilyStore implements ColumnFamilyStoreMBean public Collection<SSTableReader> flushMemtable(Memtable memtable) { + if (memtable.isClean() || truncate) + { + memtable.cfs.replaceFlushed(memtable, Collections.emptyList()); + reclaim(memtable); + return Collections.emptyList(); + } + List<Future<SSTableMultiWriter>> futures = new ArrayList<>(); long totalBytesOnDisk = 0; long maxBytesOnDisk = 0; http://git-wip-us.apache.org/repos/asf/cassandra/blob/ca556c44/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 32f69eb..ce92a0a 100644 --- a/src/java/org/apache/cassandra/db/commitlog/CommitLog.java +++ b/src/java/org/apache/cassandra/db/commitlog/CommitLog.java @@ -321,8 +321,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/ca556c44/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 e32c204..94f0ac8 100644 --- a/src/java/org/apache/cassandra/db/commitlog/CommitLogSegment.java +++ b/src/java/org/apache/cassandra/db/commitlog/CommitLogSegment.java @@ -564,7 +564,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/ca556c44/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..7235600 --- /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. + executeFormattedQuery(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.segmentManager.getActiveSegments()); + CommitLog.instance.forceRecycleAllSegments(); + + // If one of the previous segments remains, it wasn't clean. + active.retainAll(CommitLog.instance.segmentManager.getActiveSegments()); + assert active.isEmpty(); + } +}