diff --git a/.build/build-accord.xml b/.build/build-accord.xml index 0d16197c6bea..d47efe55bd16 100644 --- a/.build/build-accord.xml +++ b/.build/build-accord.xml @@ -18,7 +18,11 @@ --> - + + diff --git a/src/java/org/apache/cassandra/db/CassandraKeyspaceWriteHandler.java b/src/java/org/apache/cassandra/db/CassandraKeyspaceWriteHandler.java index 9b68acc708e1..41630a406d0b 100644 --- a/src/java/org/apache/cassandra/db/CassandraKeyspaceWriteHandler.java +++ b/src/java/org/apache/cassandra/db/CassandraKeyspaceWriteHandler.java @@ -40,21 +40,22 @@ public CassandraKeyspaceWriteHandler(Keyspace keyspace) } @Override - public WriteContext beginWrite(Mutation mutation, boolean makeDurable) throws RequestExecutionException + public WriteContext beginWrite(Mutation mutation, boolean makeDurable, boolean isReplay) throws RequestExecutionException { OpOrder.Group group = null; try { group = Keyspace.writeOrder.start(); - MigrationRouter.validateUntrackedMutation(mutation); + MigrationRouter.validateUntrackedMutation(mutation, isReplay); + // write the mutation to the commitlog and memtables CommitLogPosition position = null; if (makeDurable) { position = addToCommitLog(mutation); } - return new CassandraWriteContext(group, position); + return new CassandraWriteContext(group, position, LogDomain.COMMIT_LOG); } catch (Throwable t) { @@ -107,7 +108,9 @@ private WriteContext createEmptyContext() try { group = Keyspace.writeOrder.start(); - return new CassandraWriteContext(group, null); + // Index rebuild and read contexts append to neither log. Commit-log domain because the writes they + // carry are index updates derived from data already durable, never journal appends of their own. + return new CassandraWriteContext(group, null, LogDomain.COMMIT_LOG); } catch (Throwable t) { diff --git a/src/java/org/apache/cassandra/db/CassandraWriteContext.java b/src/java/org/apache/cassandra/db/CassandraWriteContext.java index bac1351fbd4d..aa8fee0aefc5 100644 --- a/src/java/org/apache/cassandra/db/CassandraWriteContext.java +++ b/src/java/org/apache/cassandra/db/CassandraWriteContext.java @@ -27,12 +27,15 @@ public class CassandraWriteContext implements WriteContext { private final OpOrder.Group opGroup; private final CommitLogPosition position; + private final LogDomain domain; - public CassandraWriteContext(OpOrder.Group opGroup, CommitLogPosition position) + public CassandraWriteContext(OpOrder.Group opGroup, CommitLogPosition position, LogDomain domain) { Preconditions.checkArgument(opGroup != null); + Preconditions.checkArgument(domain != null); this.opGroup = opGroup; this.position = position; + this.domain = domain; } public static CassandraWriteContext fromContext(WriteContext context) @@ -51,6 +54,11 @@ public CommitLogPosition getPosition() return position; } + public LogDomain domain() + { + return domain; + } + @Override public void close() { diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java index 162686724593..f3c94a63840f 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -58,6 +58,7 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Joiner; +import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.base.Predicates; import com.google.common.base.Strings; @@ -95,8 +96,10 @@ import org.apache.cassandra.db.lifecycle.Tracker; import org.apache.cassandra.db.lifecycle.View; import org.apache.cassandra.db.memtable.Flushing; +import org.apache.cassandra.db.memtable.LogDomainBounds; import org.apache.cassandra.db.memtable.Memtable; import org.apache.cassandra.db.memtable.ShardBoundaries; +import org.apache.cassandra.db.memtable.SplitDomainMemtable; import org.apache.cassandra.db.partitions.CachedPartition; import org.apache.cassandra.db.partitions.PartitionUpdate; import org.apache.cassandra.db.repair.CassandraTableRepairManager; @@ -145,6 +148,7 @@ import org.apache.cassandra.schema.CompactionParams.TombstoneOption; import org.apache.cassandra.schema.CompressionParams; import org.apache.cassandra.schema.IndexMetadata; +import org.apache.cassandra.schema.ReplicationType; import org.apache.cassandra.schema.Schema; import org.apache.cassandra.schema.SchemaConstants; import org.apache.cassandra.schema.TableId; @@ -520,18 +524,15 @@ public ColumnFamilyStore(Keyspace keyspace, logger.info("Initializing {}.{}", getKeyspaceName(), name); Memtable initialMemtable = null; + LogDomainBounds initialBounds = null; if (DatabaseDescriptor.isDaemonInitialized()) { - CommitLogPosition commitLogPosition; - if (metadata().replicationType().isTracked()) - commitLogPosition = MutationJournal.instance().getCurrentPosition(); - else - commitLogPosition = CommitLog.instance.getCurrentPosition(); - initialMemtable = createMemtable(new AtomicReference<>(commitLogPosition)); + initialBounds = LogDomainBounds.atCurrentPositions(); + initialMemtable = createMemtable(initialBounds); } memtableMetricsReleaser = memtableFactory.createMemtableMetricsReleaser(metadata); - data = new Tracker(this, initialMemtable, loadSSTables); + data = new Tracker(this, initialMemtable, initialBounds, loadSSTables); // Note that this needs to happen before we load the first sstables, or the global sstable tracker will not // be notified on the initial loading. @@ -1050,6 +1051,18 @@ private void switchMemtableOrNotify(FlushReason reason, TableMetadata metadata, elseNotify.accept(currentMemtable); } + /** + * Whether the given memtable is the current generation, accepting an internal of the current split memtable as well + * as the memtable itself. Memtables can flush themselves, so we need to check if they're calling flush from inside + * a split domain memtable. + */ + private boolean isCurrentGeneration(Memtable memtable) + { + Memtable current = data.getView().getCurrentMemtable(); + return current == memtable + || (current instanceof SplitDomainMemtable && ((SplitDomainMemtable) current).isInternal(memtable)); + } + /** * Switches the memtable iff the live memtable is the one provided * @@ -1059,7 +1072,7 @@ public Future switchMemtableIfCurrent(Memtable memtable, Flus { synchronized (data) { - if (data.getView().getCurrentMemtable() == memtable) + if (isCurrentGeneration(memtable)) return switchMemtable(reason); } logger.debug("Memtable is no longer current, returning future that completes when current flushing operation completes"); @@ -1192,14 +1205,21 @@ public CommitLogPosition call() // If a flush errored out but the error was ignored, make sure we don't discard the commit log. if (flushFailure == null && mainMemtable != null) { - CommitLogPosition commitLogLowerBound = mainMemtable.getCommitLogLowerBound(); - commitLogUpperBound = mainMemtable.getFinalCommitLogUpperBound(); TableMetadata metadata = metadata(); - if (metadata().replicationType().isTracked()) - MutationJournal.instance().notifyFlushed(metadata.id, commitLogLowerBound, commitLogUpperBound); + // Each log is told about the span the memtable bounded in that log. + for (Memtable source : mainMemtable.flushSources()) + { + CommitLogPosition lowerBound = source.getCommitLogLowerBound(); + CommitLogPosition upperBound = source.getFinalCommitLogUpperBound(); + + if (source.holds(LogDomain.MUTATION_JOURNAL)) + MutationJournal.instance().notifyFlushed(metadata.id, lowerBound, upperBound); + else + CommitLog.instance.discardCompletedSegments(metadata.id, lowerBound, upperBound); + } - CommitLog.instance.discardCompletedSegments(metadata.id, commitLogLowerBound, commitLogUpperBound); + commitLogUpperBound = mainMemtable.getFinalCommitLogUpperBound(); } metric.pendingFlushes.dec(); @@ -1252,21 +1272,24 @@ private Flush(boolean truncate) memtables = new LinkedHashMap<>(); // submit flushes for the memtable for any indexed sub-cfses, and our own - AtomicReference commitLogUpperBound = new AtomicReference<>(); + LogDomainBounds upperBounds = LogDomainBounds.unset(); for (ColumnFamilyStore cfs : concatWithIndexes()) { // switch all memtables, regardless of their dirty status, setting the barrier // so that we can reach a coordinated decision about cleanliness once they // are no longer possible to be modified - Memtable newMemtable = cfs.createMemtable(commitLogUpperBound); - Memtable oldMemtable = cfs.data.switchMemtable(truncate, newMemtable); - oldMemtable.switchOut(writeBarrier, commitLogUpperBound); + Memtable newMemtable = cfs.createMemtable(upperBounds); + Memtable oldMemtable = cfs.data.switchMemtable(truncate, newMemtable, upperBounds); + oldMemtable.switchOut(writeBarrier, upperBounds); memtables.put(cfs, oldMemtable); } - // we then ensure an atomic decision is made about the upper bound of the continuous range of commit log - // records owned by this memtable - setCommitLogUpperBound(commitLogUpperBound, metadata().replicationType().isTracked()); + // we then ensure an atomic decision is made about the upper bound of the continuous range of records owned + // by this memtable, in each log separately: a position from one log does not compare against a bound from + // the other + upperBounds.seal(); + + Preconditions.checkState(allSealed(upperBounds), "Unsealed bound %s for %d", upperBounds, memtables.values()); // we then issue the barrier; this lets us wait for all operations started prior to the barrier to complete; // since this happens after wiring up the commitLogUpperBound, we also know all operations with earlier @@ -1276,6 +1299,15 @@ private Flush(boolean truncate) postFlushTask = new FutureTask<>(postFlush); } + private boolean allSealed(LogDomainBounds upperBounds) + { + for (Memtable memtable : memtables.values()) + for (LogDomain domain : LogDomain.values()) + if (memtable.holds(domain) && !upperBounds.isSealed(domain)) + return false; + return true; + } + public void run() { if (logger.isTraceEnabled()) @@ -1348,11 +1380,20 @@ public Collection flushMemtable(ColumnFamilyStore cfs, Memtable m try { // flush the memtable - flushRunnables = Flushing.flushRunnables(cfs, memtable, txn); ExecutorPlus[] executors = perDiskflushExecutors.getExecutorsFor(getKeyspaceName(), name); + flushRunnables = new ArrayList<>(); + // One transaction over every log domain's output, so the generation's sstables become visible + // together and PostFlush can't run against a half-persisted memtable generation. + for (Memtable source : memtable.flushSources()) + { + if (source.isClean()) + continue; - for (int i = 0; i < flushRunnables.size(); i++) - futures.add(executors[i].submit(flushRunnables.get(i))); + List perDisk = Flushing.flushRunnables(cfs, source, txn); + flushRunnables.addAll(perDisk); + for (int i = 0; i < perDisk.size(); i++) + futures.add(executors[i].submit(perDisk.get(i))); + } /** * we can flush 2is as soon as the barrier completes, as they will be consistent with (or ahead of) the @@ -1461,32 +1502,26 @@ public String toString() } } - public Memtable createMemtable(AtomicReference commitLogUpperBound) + public Memtable createMemtable(LogDomainBounds lowerBounds) { - return memtableFactory.create(commitLogUpperBound, metadata, this); + LogDomain domain = initialMemtableDomain(); + return createMemtable(lowerBounds.forDomain(domain), domain); } - // atomically set the upper bound for the commit log - private static void setCommitLogUpperBound(AtomicReference commitLogUpperBound, boolean useMutationJournal) + public Memtable createMemtable(AtomicReference commitLogLowerBound, LogDomain domain) { - // we attempt to set the holder to the current commit log context. at the same time all writes to the memtables are - // also maintaining this value, so if somebody sneaks ahead of us somehow (should be rare) we simply retry, - // so that we know all operations prior to the position have not reached it yet - CommitLogPosition lastReplayPosition; - while (true) - { - CommitLogPosition commitLogPosition; - if (useMutationJournal) - commitLogPosition = MutationJournal.instance().getCurrentPosition(); - else - commitLogPosition = CommitLog.instance.getCurrentPosition(); + return memtableFactory.create(commitLogLowerBound, metadata, this, domain); + } - lastReplayPosition = new Memtable.LastCommitLogPosition(commitLogPosition); - CommitLogPosition currentLast = commitLogUpperBound.get(); - if ((currentLast == null || currentLast.compareTo(lastReplayPosition) <= 0) - && commitLogUpperBound.compareAndSet(currentLast, lastReplayPosition)) - break; - } + private LogDomain initialMemtableDomain() + { + // TableMetadata.Builder.kind() nulls keyspaceReplicationType for any non-regular kind, so an index has to ask + // the keyspace. replicationType() cannot stand in: it answers untracked for those kinds, which would make every + // index of a tracked keyspace start in the wrong domain and wrap on the first write of every generation. + ReplicationType replicationType = metadata().keyspaceReplicationType; + if (replicationType == null) + replicationType = keyspace.getMetadata().params.replicationType; + return LogDomain.initialFor(replicationType); } @Override @@ -1527,11 +1562,15 @@ public void apply(MutationId mutationId, PartitionUpdate update, CassandraWriteC long start = nanoTime(); OpOrder.Group opGroup = context.getGroup(); CommitLogPosition commitLogPosition = context.getPosition(); + + // mutation ids aren't passed through to 2i writes, otherwise validate that the mutation id and log domain match + Preconditions.checkState(isIndex() || context.domain().isJournal() != mutationId.isNone(), + "write context domain %s disagrees with mutation id %s", context.domain(), mutationId); try { - Memtable mt = data.getMemtableFor(opGroup, commitLogPosition); + Memtable mt = data.getMemtableFor(opGroup, commitLogPosition, context.domain()); UpdateTransaction indexer = newUpdateTransaction(update, context, updateIndexes, mt); - long timeDelta = mt.put(mutationId, update, indexer, opGroup); + long timeDelta = mt.put(mutationId, update, indexer, opGroup, context.domain()); DecoratedKey key = update.partitionKey(); invalidateCachedPartition(key); metric.topWritePartitionFrequency.addSample(key.getKey(), 1); @@ -2476,6 +2515,9 @@ private SSTableMultiWriter writeMemtableRanges(Supplier> dataSets = new ArrayList<>(ranges.size()); ImmutableCoordinatorLogOffsets.Builder logOffsetsBuilder = new ImmutableCoordinatorLogOffsets.Builder(); IntervalSet.Builder commitLogIntervals = new IntervalSet.Builder(); @@ -2485,7 +2527,7 @@ private SSTableMultiWriter writeMemtableRanges(Supplier dataSet = current.getFlushSet(range.left, range.right); dataSets.add(dataSet); logOffsetsBuilder.addAll(dataSet.coordinatorLogOffsets()); - commitLogIntervals.add(dataSet.commitLogLowerBound(), dataSet.commitLogUpperBound()); + commitLogIntervals.addAll(dataSet.commitLogIntervals()); keys += dataSet.partitionCount(); } if (keys == 0) @@ -2592,7 +2634,9 @@ public void clearUnsafe() for (final ColumnFamilyStore cfs : concatWithIndexes()) { cfs.runWithCompactionsDisabled((Callable) () -> { - cfs.data.reset(memtableFactory.create(new AtomicReference<>(CommitLogPosition.NONE), cfs.metadata, cfs)); + LogDomainBounds bounds = LogDomainBounds.of(CommitLogPosition.NONE); + LogDomain domain = cfs.initialMemtableDomain(); + cfs.data.reset(memtableFactory.create(bounds.forDomain(domain), cfs.metadata, cfs, domain), bounds); return null; }, OperationType.P0, true, false); } diff --git a/src/java/org/apache/cassandra/db/Keyspace.java b/src/java/org/apache/cassandra/db/Keyspace.java index 27072f60a6b9..1364aaa91c1a 100644 --- a/src/java/org/apache/cassandra/db/Keyspace.java +++ b/src/java/org/apache/cassandra/db/Keyspace.java @@ -408,7 +408,7 @@ public void initCf(TableMetadata metadata, boolean loadSSTables, boolean addInde public Future applyFuture(Mutation mutation, boolean writeCommitLog, boolean updateIndexes) { if (mutation.id().isNone()) - return applyInternal(mutation, writeCommitLog, updateIndexes, true, true, new AsyncPromise<>()); + return applyInternal(mutation, writeCommitLog, updateIndexes, true, true, false, new AsyncPromise<>()); else return applyInternalTracked(mutation, false, new AsyncPromise<>()); } @@ -446,7 +446,7 @@ public void apply(final Mutation mutation, applyInternalTracked(mutation, false, null); } else - applyInternal(mutation, makeDurable, updateIndexes, isDroppable, false, null); + applyInternal(mutation, makeDurable, updateIndexes, isDroppable, false, false, null); } /** @@ -458,15 +458,23 @@ public void apply(final Mutation mutation, * @param updateIndexes false to disable index updates (used by CollationController "defragmenting") * @param isDroppable true if this should throw WriteTimeoutException if it does not acquire lock within write_request_timeout * @param isDeferrable true if caller is not waiting for future to complete, so that future may be deferred + * @param isReplay true if this is commit log replay, which skips the routing check */ private Future applyInternal(final Mutation mutation, - final boolean makeDurable, - boolean updateIndexes, - boolean isDroppable, - boolean isDeferrable, - Promise future) + final boolean makeDurable, + boolean updateIndexes, + boolean isDroppable, + boolean isDeferrable, + boolean isReplay, + Promise future) { - Preconditions.checkState(!MigrationRouter.isFullyTracked(mutation) && mutation.id().isNone()); + // An untracked apply must carry no mutation id + Preconditions.checkState(mutation.id().isNone()); + + // Don't check routing for replay. A replayed mutations's domain is determined by which log it came out of. + // We may be replaying a commit log after a migration to tracked has completed + if (!isReplay) + Preconditions.checkState(!MigrationRouter.isFullyTracked(mutation)); if (TEST_FAIL_WRITES && getMetadata().name.equals(TEST_FAIL_WRITES_KS)) throw new RuntimeException("Testing write failures"); @@ -523,9 +531,7 @@ else if (isDeferrable) // This view update can't happen right now. so rather than keep this thread busy // we will re-apply ourself to the queue and try again later - Stage.MUTATION.execute(() -> - applyInternal(mutation, makeDurable, true, isDroppable, true, future) - ); + Stage.MUTATION.execute(() -> applyInternal(mutation, makeDurable, true, isDroppable, true, isReplay, future) ); return future; } else @@ -563,7 +569,7 @@ else if (isDeferrable) columnFamilyStores.get(tableId).metric.viewLockAcquireTime.update(acquireTime, MILLISECONDS); } } - try (WriteContext ctx = getWriteHandler().beginWrite(mutation, makeDurable)) + try (WriteContext ctx = getWriteHandler().beginWrite(mutation, makeDurable, isReplay)) { ConsensusMigrationMutationHelper.validateSafeToExecuteNonTransactionally(mutation); for (PartitionUpdate upd : mutation.getPartitionUpdates()) @@ -615,17 +621,12 @@ else if (isDeferrable) } } - /** - * Apply a tracked mutation read from the {@link org.apache.cassandra.replication.MutationJournal} - * during static-segment replay on startup. - * - * Compared to the normal write apply path, this skips the journal append (since we're replaying from it) - * and always writes to the memtable, even when {@link MutationTrackingService#startWriting} reports the offset - * as already witnessed. - */ public void applyForReplay(Mutation mutation) { - applyInternalTracked(mutation, true, null); + if (mutation.id().isNone()) + applyInternal(mutation, false, true, false, false, true, null); + else + applyInternalTracked(mutation, true, null); } /** @@ -634,19 +635,28 @@ public void applyForReplay(Mutation mutation) private Future applyInternalTracked(Mutation mutation, boolean isReplay, Promise future) { MutationTrackingService.ensureEnabled(); - if (!MigrationRouter.isFullyTracked(mutation) || mutation.id().isNone()) - throw new CoordinatorBehindException("Mutation routing mismatch in applyInternalTracked: isFullyTracked=" + - MigrationRouter.isFullyTracked(mutation) + ", id.isNone=" + mutation.id().isNone() + + // A tracked apply must carry a mutation id, replay or not. + if (mutation.id().isNone()) + throw new CoordinatorBehindException("Mutation routing mismatch in applyInternalTracked: id.isNone=true" + ", keyspace=" + mutation.getKeyspaceName()); + + // Don't check routing for replay. A replayed mutations's domain is determined by which log it came out of. We may + // be replaying a journal after a migration to untracked has completed + if (!isReplay && !MigrationRouter.isFullyTracked(mutation)) + throw new CoordinatorBehindException("Mutation routing mismatch in applyInternalTracked: isFullyTracked=false" + + ", keyspace=" + mutation.getKeyspaceName()); + ClusterMetadata cm = ClusterMetadata.current(); if (TEST_FAIL_WRITES && getMetadata().name.equals(TEST_FAIL_WRITES_KS)) throw new RuntimeException("Testing write failures"); boolean started; - try (WriteContext ctx = trackedWriteHandler.beginWrite(mutation, !isReplay)) + try (WriteContext ctx = trackedWriteHandler.beginWrite(mutation, !isReplay, isReplay)) { - started = MutationTrackingService.instance().startWriting(mutation); + started = isReplay + ? MutationTrackingService.instance().startWritingForReplay(mutation) + : MutationTrackingService.instance().startWriting(mutation); if (started || isReplay) { diff --git a/src/java/org/apache/cassandra/db/KeyspaceWriteHandler.java b/src/java/org/apache/cassandra/db/KeyspaceWriteHandler.java index 19cca7243210..803342ca4afa 100644 --- a/src/java/org/apache/cassandra/db/KeyspaceWriteHandler.java +++ b/src/java/org/apache/cassandra/db/KeyspaceWriteHandler.java @@ -22,8 +22,13 @@ public interface KeyspaceWriteHandler { + default WriteContext beginWrite(Mutation mutation, boolean makeDurable) throws RequestExecutionException + { + return beginWrite(mutation, makeDurable, false); + } + // mutation can be null if makeDurable is false - WriteContext beginWrite(Mutation mutation, boolean makeDurable) throws RequestExecutionException; + WriteContext beginWrite(Mutation mutation, boolean makeDurable, boolean isReplay) throws RequestExecutionException; WriteContext createContextForIndexing(); WriteContext createContextForRead(); } diff --git a/src/java/org/apache/cassandra/db/LogDomain.java b/src/java/org/apache/cassandra/db/LogDomain.java new file mode 100644 index 000000000000..2b1b622cbdba --- /dev/null +++ b/src/java/org/apache/cassandra/db/LogDomain.java @@ -0,0 +1,43 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db; + +import org.apache.cassandra.schema.ReplicationType; + +/** + * Which log a {@link org.apache.cassandra.db.commitlog.CommitLogPosition} came from. + *

+ * The commit log and the mutation journal generate their segment ids independently, so positions from different logs + * can't be compared with each other. + */ +public enum LogDomain +{ + COMMIT_LOG, + MUTATION_JOURNAL; + + public boolean isJournal() + { + return this == MUTATION_JOURNAL; + } + + public static LogDomain initialFor(ReplicationType keyspaceReplicationType) + { + return keyspaceReplicationType.isTracked() ? MUTATION_JOURNAL : COMMIT_LOG; + } +} diff --git a/src/java/org/apache/cassandra/db/commitlog/CommitLogReplayer.java b/src/java/org/apache/cassandra/db/commitlog/CommitLogReplayer.java index 21dc34956394..7654be720132 100644 --- a/src/java/org/apache/cassandra/db/commitlog/CommitLogReplayer.java +++ b/src/java/org/apache/cassandra/db/commitlog/CommitLogReplayer.java @@ -322,7 +322,7 @@ public void runMayThrow() { assert !newPUCollector.isEmpty(); - keyspace.apply(newPUCollector.build(), false, true, false); + keyspace.applyForReplay(newPUCollector.build()); commitLogReplayer.keyspacesReplayed.add(keyspace); } } diff --git a/src/java/org/apache/cassandra/db/compaction/AbstractStrategyHolder.java b/src/java/org/apache/cassandra/db/compaction/AbstractStrategyHolder.java index 29fc82b1f8d1..153a22bd0e94 100644 --- a/src/java/org/apache/cassandra/db/compaction/AbstractStrategyHolder.java +++ b/src/java/org/apache/cassandra/db/compaction/AbstractStrategyHolder.java @@ -155,16 +155,14 @@ final void setStrategy(CompactionParams params, int numTokenPartitions) protected abstract void setStrategyInternal(CompactionParams params, int numTokenPartitions); /** - * SSTables are grouped by their repaired and pending repair status. This method determines if this holder - * holds the sstable for the given repaired/grouped statuses. Holders should be mutually exclusive in the - * groups they deal with. IOW, if one holder returns true for a given isRepaired/isPendingRepair combo, - * none of the others should. + * SSTables are grouped by {@link CompactionGroup}, which is derived from their metadata. This method determines + * whether this holder holds the sstables of a given group. */ - public abstract boolean managesRepairedGroup(boolean isRepaired, boolean isPendingRepair); + public abstract boolean managesGroup(CompactionGroup group); public boolean managesSSTable(SSTableReader sstable) { - return managesRepairedGroup(sstable.isRepaired(), sstable.isPendingRepair()); + return managesGroup(CompactionGroup.of(sstable)); } public abstract AbstractCompactionStrategy getStrategyFor(SSTableReader sstable); diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionGroup.java b/src/java/org/apache/cassandra/db/compaction/CompactionGroup.java new file mode 100644 index 000000000000..116725d14d8f --- /dev/null +++ b/src/java/org/apache/cassandra/db/compaction/CompactionGroup.java @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.compaction; + +import com.google.common.base.Preconditions; + +import org.apache.cassandra.io.sstable.format.SSTableReader; +import org.apache.cassandra.io.sstable.metadata.StatsMetadata; +import org.apache.cassandra.replication.ImmutableCoordinatorLogOffsets; +import org.apache.cassandra.service.ActiveRepairService; +import org.apache.cassandra.utils.TimeUUID; + +/** + * Which compaction strategy holder an sstable belongs to. + *

+ * Sstables are grouped into separate silos that aren't compacted together for correctness reasons related to + * their replication and anti-entropy mechanisms. The group assignment is purely a function of sstable metadata and + * this enum lists the top level buckets and contains the logic for classifying a given sstable + */ +public enum CompactionGroup +{ + UNREPAIRED, // untracked, unrepaired data + PENDING_REPAIR, // sstables currently involved in incremental repair - contains sub buckets per repair session + UNRECONCILED, // tracked data awaiting reconciliation - contains sub buckets per set of activated transfers + REPAIRED; // incrementall repaired or fully reconciled. Data in each bucket should eventually be promoted here + + public static CompactionGroup of(SSTableReader sstable) + { + StatsMetadata metadata = sstable.getSSTableMetadata(); + return of(metadata.repairedAt, metadata.pendingRepair, metadata.coordinatorLogOffsets); + } + + public static CompactionGroup of(long repairedAt, TimeUUID pendingRepair, ImmutableCoordinatorLogOffsets offsets) + { + boolean isRepaired = repairedAt != ActiveRepairService.UNREPAIRED_SSTABLE; + boolean isPendingRepair = pendingRepair != ActiveRepairService.NO_PENDING_REPAIR; + Preconditions.checkArgument(!(isRepaired && isPendingRepair), + "SSTables cannot be both repaired and pending repair"); + + if (isRepaired) + return REPAIRED; + + if (isPendingRepair) + return PENDING_REPAIR; + + // Transfers and mutations both go here. Which sub bucket an sstable lands in is + // TrackedCompactionManager's business, keyed on ImmutableCoordinatorLogOffsets.transferSiloKey(). + if (offsets != null && !offsets.isEmpty()) + return UNRECONCILED; + + return UNREPAIRED; + } +} diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionStrategyHolder.java b/src/java/org/apache/cassandra/db/compaction/CompactionStrategyHolder.java index 34a0181c3c67..5c4715d5ccdc 100644 --- a/src/java/org/apache/cassandra/db/compaction/CompactionStrategyHolder.java +++ b/src/java/org/apache/cassandra/db/compaction/CompactionStrategyHolder.java @@ -39,18 +39,17 @@ import org.apache.cassandra.io.sstable.format.SSTableReader; import org.apache.cassandra.replication.ImmutableCoordinatorLogOffsets; import org.apache.cassandra.schema.CompactionParams; -import org.apache.cassandra.service.ActiveRepairService; import org.apache.cassandra.utils.TimeUUID; public class CompactionStrategyHolder extends AbstractStrategyHolder { private final List strategies = new ArrayList<>(); - private final boolean isRepaired; + private final CompactionGroup group; - public CompactionStrategyHolder(ColumnFamilyStore cfs, DestinationRouter router, boolean isRepaired) + public CompactionStrategyHolder(ColumnFamilyStore cfs, DestinationRouter router, CompactionGroup group) { super(cfs, router); - this.isRepaired = isRepaired; + this.group = group; } @Override @@ -74,18 +73,14 @@ public void setStrategyInternal(CompactionParams params, int numTokenPartitions) } @Override - public boolean managesRepairedGroup(boolean isRepaired, boolean isPendingRepair) + public boolean managesGroup(CompactionGroup group) { - if (!isPendingRepair) - { - return this.isRepaired == isRepaired; - } - else - { - Preconditions.checkArgument(!isRepaired, "SSTables cannot be both repaired and pending repair"); - return false; + return this.group == group; + } - } + boolean isRepaired() + { + return group == CompactionGroup.REPAIRED; } @Override @@ -204,7 +199,7 @@ public List getScanners(GroupedSSTableContainer sstables, Colle Collection> groupForAnticompaction(Iterable sstables) { - Preconditions.checkState(!isRepaired); + Preconditions.checkState(!isRepaired()); GroupedSSTableContainer group = createGroupedSSTableContainer(); sstables.forEach(group::add); @@ -232,18 +227,9 @@ public SSTableMultiWriter createSSTableMultiWriter(Descriptor descriptor, Collection indexGroups, ILifecycleTransaction txn) { - if (isRepaired) - { - Preconditions.checkArgument(repairedAt != ActiveRepairService.UNREPAIRED_SSTABLE, - "Repaired CompactionStrategyHolder can't create unrepaired sstable writers"); - } - else - { - Preconditions.checkArgument(repairedAt == ActiveRepairService.UNREPAIRED_SSTABLE, - "Unrepaired CompactionStrategyHolder can't create repaired sstable writers"); - } - Preconditions.checkArgument(pendingRepair == null, - "CompactionStrategyHolder can't create sstable writer with pendingRepair id"); + Preconditions.checkArgument(managesGroup(CompactionGroup.of(repairedAt, pendingRepair, coordinatorLogOffsets)), + "%s CompactionStrategyHolder can't create a writer for an sstable it would not manage", + group); // to avoid creating a compaction strategy for the wrong pending repair manager, we get the index based on where the sstable is to be written AbstractCompactionStrategy strategy = strategies.get(router.getIndexForSSTableDirectory(descriptor)); return strategy.createSSTableMultiWriter(descriptor, diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java b/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java index 1ceda32ff5e9..36677f312978 100644 --- a/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java +++ b/src/java/org/apache/cassandra/db/compaction/CompactionStrategyManager.java @@ -25,6 +25,7 @@ import java.util.Collections; import java.util.Comparator; import java.util.ConcurrentModificationException; +import java.util.EnumMap; import java.util.HashSet; import java.util.List; import java.util.Map; @@ -40,6 +41,7 @@ import java.util.stream.StreamSupport; import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; @@ -130,8 +132,10 @@ public class CompactionStrategyManager implements INotificationConsumer private final PendingRepairHolder pendingRepairs; private final CompactionStrategyHolder repaired; private final CompactionStrategyHolder unrepaired; + private final TrackedCompactionManager tracked; private final ImmutableList holders; + private final EnumMap holderByGroup; private volatile CompactionParams params; private DiskBoundaries currentBoundaries; @@ -175,9 +179,23 @@ public int getIndexForSSTableDirectory(Descriptor descriptor) } }; pendingRepairs = new PendingRepairHolder(cfs, router); - repaired = new CompactionStrategyHolder(cfs, router, true); - unrepaired = new CompactionStrategyHolder(cfs, router, false); - holders = ImmutableList.of(pendingRepairs, repaired, unrepaired); + repaired = new CompactionStrategyHolder(cfs, router, CompactionGroup.REPAIRED); + unrepaired = new CompactionStrategyHolder(cfs, router, CompactionGroup.UNREPAIRED); + tracked = new TrackedCompactionManager(cfs, router); + holders = ImmutableList.of(pendingRepairs, repaired, unrepaired, tracked); + + holderByGroup = new EnumMap<>(CompactionGroup.class); + for (CompactionGroup group : CompactionGroup.values()) + { + for (AbstractStrategyHolder holder : holders) + { + if (!holder.managesGroup(group)) + continue; + AbstractStrategyHolder previous = holderByGroup.put(group, holder); + Preconditions.checkState(previous == null, "More than one holder claims %s", group); + } + Preconditions.checkState(holderByGroup.containsKey(group), "No holder claims %s", group); + } cfs.getTracker().subscribe(this); logger.trace("Compaction manager for {}.{} subscribed to the data tracker.", cfs.keyspace.getName(), cfs.name); @@ -215,6 +233,11 @@ public Collection getNextBackgroundTasks(long gcBefore) if (repairFinishedTasks != null && !repairFinishedTasks.isEmpty()) return repairFinishedTasks; + // then promote tracked sstables whose mutations or transfers have reconciled + Collection promotionTasks = tracked.getNextPromotionTasks(); + if (promotionTasks != null && !promotionTasks.isEmpty()) + return promotionTasks; + // sort compaction task suppliers by remaining tasks descending List suppliers = new ArrayList<>(numPartitions * holders.size()); for (AbstractStrategyHolder holder : holders) @@ -927,32 +950,20 @@ private int getHolderIndex(SSTableReader sstable) private AbstractStrategyHolder getHolder(SSTableReader sstable) { - for (AbstractStrategyHolder holder : holders) - { - if (holder.managesSSTable(sstable)) - return holder; - } - - throw new IllegalStateException("No holder claimed " + sstable); + return getHolder(CompactionGroup.of(sstable)); } - private AbstractStrategyHolder getHolder(long repairedAt, TimeUUID pendingRepair) + private AbstractStrategyHolder getHolder(long repairedAt, + TimeUUID pendingRepair, + ImmutableCoordinatorLogOffsets coordinatorLogOffsets) { - return getHolder(repairedAt != ActiveRepairService.UNREPAIRED_SSTABLE, - pendingRepair != ActiveRepairService.NO_PENDING_REPAIR); + return getHolder(CompactionGroup.of(repairedAt, pendingRepair, coordinatorLogOffsets)); } @VisibleForTesting - AbstractStrategyHolder getHolder(boolean isRepaired, boolean isPendingRepair) + AbstractStrategyHolder getHolder(CompactionGroup group) { - for (AbstractStrategyHolder holder : holders) - { - if (holder.managesRepairedGroup(isRepaired, isPendingRepair)) - return holder; - } - - throw new IllegalStateException(String.format("No holder claimed isPendingRepair: %s, isPendingRepair %s", - isRepaired, isPendingRepair)); + return holderByGroup.get(group); } @VisibleForTesting @@ -1410,7 +1421,7 @@ public SSTableMultiWriter createSSTableMultiWriter(Descriptor descriptor, readLock.lock(); try { - return getHolder(repairedAt, pendingRepair).createSSTableMultiWriter(descriptor, + return getHolder(repairedAt, pendingRepair, coordinatorLogOffsets).createSSTableMultiWriter(descriptor, keyCount, repairedAt, pendingRepair, @@ -1515,6 +1526,44 @@ public void mutateRepaired(Collection sstables, long repairedAt, } } + /** + * Promote reconciled sstables to repaired and clear their coordinator log offsets in the same metadata mutation, + * then move them between strategies under the write lock as {@link #mutateRepaired} does. + * + * @return the sstables that were promoted + */ + public Set promoteReconciled(Collection sstables, long repairedAt) throws IOException + { + if (sstables.isEmpty()) + return Collections.emptySet(); + Set changed = new HashSet<>(); + + writeLock.lock(); + try + { + for (SSTableReader sstable : sstables) + { + sstable.mutatePromotedToRepairedAndReload(repairedAt); + verifyMetadata(sstable, repairedAt, ActiveRepairService.NO_PENDING_REPAIR); + if (!sstable.getSSTableMetadata().coordinatorLogOffsets.isEmpty()) + throw new IllegalStateException(String.format("Failed clearing coordinator log offsets on %s", sstable)); + changed.add(sstable); + } + } + finally + { + try + { + cfs.getTracker().notifySSTableRepairedStatusChanged(changed); + } + finally + { + writeLock.unlock(); + } + } + return changed; + } + private static void verifyMetadata(SSTableReader sstable, long repairedAt, TimeUUID pendingRepair) { if (!Objects.equals(pendingRepair, sstable.getPendingRepair())) diff --git a/src/java/org/apache/cassandra/db/compaction/CompactionTask.java b/src/java/org/apache/cassandra/db/compaction/CompactionTask.java index 7e41d980de4d..c2949739a808 100644 --- a/src/java/org/apache/cassandra/db/compaction/CompactionTask.java +++ b/src/java/org/apache/cassandra/db/compaction/CompactionTask.java @@ -56,7 +56,6 @@ import org.apache.cassandra.io.sstable.metadata.MetadataCollector; import org.apache.cassandra.io.util.File; import org.apache.cassandra.replication.ImmutableCoordinatorLogOffsets; -import org.apache.cassandra.replication.MutationTrackingService; import org.apache.cassandra.service.ActiveRepairService; import org.apache.cassandra.service.snapshot.SnapshotManager; import org.apache.cassandra.service.snapshot.SnapshotOptions; @@ -443,12 +442,18 @@ public static TimeUUID getPendingRepair(Set sstables) return ids.iterator().next(); } + /** + * Union the inputs' coordinator log offsets. Reconciled ids are not purged here. + * + * Removing ids as they reconcile leaves an sstable that is still journal-derived but no longer says so, and once + * its last id goes it cannot be told apart from commit-log-derived data it must not be combined with while + * unrepaired. {@link SSTableReader#mutatePromotedToRepairedAndReload} removes them in one step at promotion. + */ public static ImmutableCoordinatorLogOffsets getCoordinatorLogOffsets(Set sstables) { ImmutableCoordinatorLogOffsets.Builder builder = new ImmutableCoordinatorLogOffsets.Builder(); for (SSTableReader sstable : sstables) builder.addAll(sstable.getCoordinatorLogOffsets()); - builder.purgeTransfers(id -> MutationTrackingService.instance().isDurablyReconciled(id)); return builder.build(); } diff --git a/src/java/org/apache/cassandra/db/compaction/PendingRepairHolder.java b/src/java/org/apache/cassandra/db/compaction/PendingRepairHolder.java index 417bd04af195..2efd7a63a2b9 100644 --- a/src/java/org/apache/cassandra/db/compaction/PendingRepairHolder.java +++ b/src/java/org/apache/cassandra/db/compaction/PendingRepairHolder.java @@ -73,11 +73,9 @@ public void setStrategyInternal(CompactionParams params, int numTokenPartitions) } @Override - public boolean managesRepairedGroup(boolean isRepaired, boolean isPendingRepair) + public boolean managesGroup(CompactionGroup group) { - Preconditions.checkArgument(!isPendingRepair || !isRepaired, - "SSTables cannot be both repaired and pending repair"); - return isPendingRepair; + return group == CompactionGroup.PENDING_REPAIR; } @Override diff --git a/src/java/org/apache/cassandra/db/compaction/PromoteReconciledTask.java b/src/java/org/apache/cassandra/db/compaction/PromoteReconciledTask.java new file mode 100644 index 000000000000..4bfd19200604 --- /dev/null +++ b/src/java/org/apache/cassandra/db/compaction/PromoteReconciledTask.java @@ -0,0 +1,112 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.compaction; + +import java.util.HashSet; +import java.util.Set; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.lifecycle.LifecycleTransaction; +import org.apache.cassandra.io.sstable.format.SSTableReader; +import org.apache.cassandra.utils.Clock; + +/** + * Promotes reconciled tracked sstables to repaired in place. No data is rewritten. + */ +class PromoteReconciledTask extends AbstractCompactionTask +{ + private static final Logger logger = LoggerFactory.getLogger(PromoteReconciledTask.class); + + private final String reason; + private final Runnable onCompleted; + + static AbstractCompactionTask tryPromote(ColumnFamilyStore cfs, + Set candidates, + String reason, + Runnable onCompleted) + { + if (candidates.isEmpty()) + return null; + + Set available = new HashSet<>(candidates); + available.removeAll(cfs.getTracker().getCompacting()); + if (available.isEmpty()) + { + logger.trace("Deferring promotion of {}.{} {}; all {} sstables are busy", + cfs.metadata.keyspace, cfs.metadata.name, reason, candidates.size()); + return null; + } + + // is txn is only here to serve as a lock, to prevent other compactions from modifying these + // sstables while their metadata is being mutated + LifecycleTransaction txn = cfs.getTracker().tryModify(available, OperationType.COMPACTION); + if (txn == null) + { + // if one or more of the sstables are already marked compacted, remove them and try again. Since we try to + // promote all eligible sstables in a single task, this keeps compaction from preventing and progress in + // promotion + available.removeAll(cfs.getTracker().getCompacting()); + if (available.isEmpty()) + return null; + txn = cfs.getTracker().tryModify(available, OperationType.COMPACTION); + if (txn == null) + { + logger.trace("Deferring promotion of {}.{} {}; lost the race for its sstables", + cfs.metadata.keyspace, cfs.metadata.name, reason); + return null; + } + } + return new PromoteReconciledTask(cfs, txn, reason, onCompleted); + } + + PromoteReconciledTask(ColumnFamilyStore cfs, LifecycleTransaction transaction, String reason, Runnable onCompleted) + { + super(cfs, transaction); + this.reason = reason; + this.onCompleted = onCompleted; + } + + protected void runMayThrow() throws Exception + { + boolean completed = false; + try + { + logger.info("Promoting {} to repaired; {} have reconciled", transaction.originals(), reason); + // One metadata mutation sets repairedAt and clears the offsets, so a repaired sstable never still claims + // journal provenance, and an unrepaired one never loses it. + cfs.getCompactionStrategyManager().promoteReconciled(transaction.originals(), + Clock.Global.currentTimeMillis()); + completed = true; + } + finally + { + transaction.abort(); + if (completed && onCompleted != null) + onCompleted.run(); + } + } + + protected void executeInternal(ActiveCompactionsTracker activeCompactions) + { + run(); + } +} diff --git a/src/java/org/apache/cassandra/db/compaction/TrackedCompactionManager.java b/src/java/org/apache/cassandra/db/compaction/TrackedCompactionManager.java new file mode 100644 index 000000000000..b51cd960ecf3 --- /dev/null +++ b/src/java/org/apache/cassandra/db/compaction/TrackedCompactionManager.java @@ -0,0 +1,506 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.compaction; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; +import com.google.common.collect.Maps; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.SerializationHeader; +import org.apache.cassandra.db.commitlog.CommitLogPosition; +import org.apache.cassandra.db.commitlog.IntervalSet; +import org.apache.cassandra.db.lifecycle.ILifecycleTransaction; +import org.apache.cassandra.dht.Range; +import org.apache.cassandra.dht.Token; +import org.apache.cassandra.index.Index; +import org.apache.cassandra.io.sstable.Descriptor; +import org.apache.cassandra.io.sstable.ISSTableScanner; +import org.apache.cassandra.io.sstable.SSTableMultiWriter; +import org.apache.cassandra.io.sstable.format.SSTableReader; +import org.apache.cassandra.replication.ImmutableCoordinatorLogOffsets; +import org.apache.cassandra.replication.MutationTrackingService; +import org.apache.cassandra.replication.ShortMutationId; +import org.apache.cassandra.schema.CompactionParams; +import org.apache.cassandra.service.ActiveRepairService; +import org.apache.cassandra.utils.TimeUUID; + +/** + * Holds {@link CompactionGroup#UNRECONCILED} sstables, isolating them into one silo per set of activated transfer + * ids, and promoting the contents of each silo to repaired as its contents are durably reconciled - continuously in + * the case of normal writes, all at once in the case of tracked transfers. This is the mutation tracking counterpart + * to {@link PendingRepairManager}. SSTables created by the normal write path don't have transfer ids and are in the + * silo under the key {@link #NONE}. Like {@link PendingRepairManager}, silos are created lazily per key, and discarded + * when empty, with the exception of the {@link #NONE} silo, which is never purged. + */ +public class TrackedCompactionManager extends AbstractStrategyHolder +{ + private static final Logger logger = LoggerFactory.getLogger(TrackedCompactionManager.class); + + /** + * Silo for tracked sstables that carry mutation offsets and no activated transfers. Created with the strategy and + * never pruned, because ordinary tracked flushes land here continuously. + * + * {@link ImmutableCoordinatorLogOffsets#transferSiloKey()} is empty for exactly these sstables, so routing one + * needs no separate classification. + */ + static final ImmutableSet NONE = ImmutableSet.of(); + + private CompactionParams params; + private int numTokenPartitions; + + private volatile ImmutableMap, CompactionStrategyHolder> silos = ImmutableMap.of(); + + public TrackedCompactionManager(ColumnFamilyStore cfs, DestinationRouter router) + { + super(cfs, router); + } + + static ImmutableSet keyOf(SSTableReader sstable) + { + return sstable.getSSTableMetadata().coordinatorLogOffsets.transferSiloKey(); + } + + private static String describe(ImmutableSet key) + { + return key.isEmpty() ? "reconciled mutations" : "tracked transfers " + key; + } + + @Override + public boolean managesGroup(CompactionGroup group) + { + return group == CompactionGroup.UNRECONCILED; + } + + @Override + public void setStrategyInternal(CompactionParams params, int numTokenPartitions) + { + this.params = params; + this.numTokenPartitions = numTokenPartitions; + this.silos = ImmutableMap.of(NONE, newSilo(NONE)); + } + + private CompactionStrategyHolder newSilo(ImmutableSet key) + { + logger.debug("Creating {}.{} compaction strategies for tracked transfers: {}", + cfs.metadata.keyspace, cfs.metadata.name, key); + CompactionStrategyHolder silo = new CompactionStrategyHolder(cfs, router, CompactionGroup.UNRECONCILED); + silo.setStrategy(params, numTokenPartitions); + return silo; + } + + CompactionStrategyHolder getIfPresent(ImmutableSet key) + { + return silos.get(key); + } + + CompactionStrategyHolder getIfPresent(SSTableReader sstable) + { + return getIfPresent(keyOf(sstable)); + } + + CompactionStrategyHolder getOrCreate(ImmutableSet key) + { + CompactionStrategyHolder silo = silos.get(key); + if (silo == null) + { + synchronized (this) + { + silo = silos.get(key); + if (silo == null) + { + silo = newSilo(key); + silos = ImmutableMap., CompactionStrategyHolder>builder() + .putAll(silos).put(key, silo).build(); + } + } + } + return silo; + } + + CompactionStrategyHolder getOrCreate(SSTableReader sstable) + { + return getOrCreate(keyOf(sstable)); + } + + private static Iterable sstablesIn(CompactionStrategyHolder silo) + { + return Iterables.concat(Iterables.transform(silo.allStrategies(), AbstractCompactionStrategy::getSSTables)); + } + + private static boolean isEmpty(CompactionStrategyHolder silo) + { + return Iterables.isEmpty(sstablesIn(silo)); + } + + /** + * Drop every transfer silo that holds no sstables. Called from the paths that walk the map anyway, so teardown does + * not depend on any particular removal notification arriving. The silo that holds normal writes is never pruned + * + * @return true if anything was dropped + */ + synchronized boolean pruneEmpty() + { + Set> empty = null; + for (Map.Entry, CompactionStrategyHolder> entry : silos.entrySet()) + { + // don't prune the normal write silo + if (entry.getKey().isEmpty()) + continue; + + if (isEmpty(entry.getValue())) + { + if (empty == null) + empty = new HashSet<>(); + empty.add(entry.getKey()); + } + } + + if (empty == null) + return false; + + Set> dropped = empty; + logger.debug("Removing {}.{} compaction strategies for reconciled or emptied tracked transfers: {}", + cfs.metadata.keyspace, cfs.metadata.name, dropped); + for (ImmutableSet key : dropped) + silos.get(key).shutdown(); + silos = ImmutableMap.copyOf(Maps.filterKeys(silos, k -> !dropped.contains(k))); + return true; + } + + @Override + public synchronized void startup() + { + silos.values().forEach(CompactionStrategyHolder::startup); + } + + @Override + public synchronized void shutdown() + { + silos.values().forEach(CompactionStrategyHolder::shutdown); + } + + @Override + public AbstractCompactionStrategy getStrategyFor(SSTableReader sstable) + { + Preconditions.checkArgument(managesSSTable(sstable), "Attempting to get compaction strategy from wrong holder"); + return getOrCreate(sstable).getStrategyFor(sstable); + } + + @Override + public Iterable allStrategies() + { + return Iterables.concat(Iterables.transform(silos.values(), AbstractStrategyHolder::allStrategies)); + } + + @Override + public synchronized void addSSTable(SSTableReader sstable) + { + Preconditions.checkArgument(managesSSTable(sstable), "Attempting to add sstable from wrong holder"); + getOrCreate(sstable).addSSTable(sstable); + } + + @VisibleForTesting + synchronized void addSSTables(Iterable sstables) + { + for (SSTableReader sstable : sstables) + addSSTable(sstable); + } + + @Override + public synchronized void addSSTables(GroupedSSTableContainer sstables) + { + for (Map.Entry, GroupedSSTableContainer> entry : splitByKey(sstables).entrySet()) + getOrCreate(entry.getKey()).addSSTables(entry.getValue()); + } + + @Override + public synchronized void removeSSTables(GroupedSSTableContainer sstables) + { + for (CompactionStrategyHolder silo : silos.values()) + silo.removeSSTables(sstables); + pruneEmpty(); + } + + @VisibleForTesting + synchronized void removeSSTable(SSTableReader sstable) + { + for (CompactionStrategyHolder silo : silos.values()) + silo.getStrategyFor(sstable).removeSSTable(sstable); + pruneEmpty(); + } + + @Override + public synchronized void replaceSSTables(GroupedSSTableContainer removed, GroupedSSTableContainer added) + { + Map, GroupedSSTableContainer> addedByKey = splitByKey(added); + + // Removals go to every silo, for the reason given on removeSSTables. + for (Map.Entry, CompactionStrategyHolder> entry : silos.entrySet()) + { + CompactionStrategyHolder silo = entry.getValue(); + GroupedSSTableContainer addedForSilo = addedByKey.get(entry.getKey()); + silo.replaceSSTables(removed, addedForSilo == null ? silo.createGroupedSSTableContainer() : addedForSilo); + } + pruneEmpty(); + } + + /** + * Regroups a container by silo key, creating any silo it does not find + */ + private Map, GroupedSSTableContainer> splitByKey(GroupedSSTableContainer sstables) + { + Map, GroupedSSTableContainer> split = new HashMap<>(); + for (int i = 0; i < sstables.numGroups(); i++) + { + for (SSTableReader sstable : sstables.getGroup(i)) + { + ImmutableSet key = keyOf(sstable); + split.computeIfAbsent(key, k -> getOrCreate(k).createGroupedSSTableContainer()).add(sstable); + } + } + return split; + } + + @Override + public synchronized List getScanners(GroupedSSTableContainer sstables, Collection> ranges) + { + List scanners = new ArrayList<>(); + try + { + for (Map.Entry, GroupedSSTableContainer> entry : splitByKey(sstables).entrySet()) + scanners.addAll(getOrCreate(entry.getKey()).getScanners(entry.getValue(), ranges)); + } + catch (Throwable t) + { + ISSTableScanner.closeAllAndPropagate(scanners, t); + } + return scanners; + } + + @Override + public synchronized Collection getUserDefinedTasks(GroupedSSTableContainer sstables, long gcBefore) + { + List tasks = new ArrayList<>(); + for (Map.Entry, GroupedSSTableContainer> entry : splitByKey(sstables).entrySet()) + { + // CompactionStrategyHolder passes through whatever getUserDefinedTask returns, including null. + for (AbstractCompactionTask task : getOrCreate(entry.getKey()).getUserDefinedTasks(entry.getValue(), gcBefore)) + { + if (task != null) + tasks.add(task); + } + } + return tasks; + } + + @Override + public SSTableMultiWriter createSSTableMultiWriter(Descriptor descriptor, + long keyCount, + long repairedAt, + TimeUUID pendingRepair, + ImmutableCoordinatorLogOffsets coordinatorLogOffsets, + IntervalSet commitLogPositions, + int sstableLevel, + SerializationHeader header, + Collection indexGroups, + ILifecycleTransaction txn) + { + // These guards read pre-write metadata. SSTableWriter.finalizeMetadata() can promote the sstable as it is + // written, so the holder that creates the writer and the holder that ends up with the sstable can differ. + Preconditions.checkArgument(repairedAt == ActiveRepairService.UNREPAIRED_SSTABLE, + "TrackedCompactionManager can't create sstable writer with repaired at set"); + Preconditions.checkArgument(pendingRepair == ActiveRepairService.NO_PENDING_REPAIR, + "TrackedCompactionManager can't create sstable writer with pendingRepair id"); + + // The silo indexes by write destination rather than by token, so this creates no strategy in the wrong place. + return getOrCreate(coordinatorLogOffsets.transferSiloKey()) + .createSSTableMultiWriter(descriptor, + keyCount, + repairedAt, + pendingRepair, + coordinatorLogOffsets, + commitLogPositions, + sstableLevel, + header, + indexGroups, + txn); + } + + @Override + public int getStrategyIndex(AbstractCompactionStrategy strategy) + { + for (CompactionStrategyHolder silo : silos.values()) + { + int idx = silo.getStrategyIndex(strategy); + if (idx >= 0) + return idx; + } + return -1; + } + + @Override + public boolean containsSSTable(SSTableReader sstable) + { + return Iterables.any(silos.values(), silo -> silo.containsSSTable(sstable)); + } + + @VisibleForTesting + boolean isPromotable(SSTableReader sstable) + { + if (!MutationTrackingService.instance().isStarted()) + return false; + + if (sstable.isRepaired() || sstable.isPendingRepair()) + return false; + + ImmutableCoordinatorLogOffsets offsets = sstable.getSSTableMetadata().coordinatorLogOffsets; + if (offsets.isEmpty()) + return false; + + return MutationTrackingService.instance().isDurablyReconciled(offsets); + } + + @VisibleForTesting + synchronized Set promotableSSTables(ImmutableSet key) + { + CompactionStrategyHolder silo = silos.get(key); + if (silo == null) + return Collections.emptySet(); + + Set promotable = new HashSet<>(); + for (SSTableReader sstable : sstablesIn(silo)) + { + if (isPromotable(sstable)) + promotable.add(sstable); + } + return promotable; + } + + @VisibleForTesting + synchronized Set promotableSSTables() + { + // TODO: consider grabbing a snapshot of reconciled offsets and comparing against + // sstable metadata without allocating collections + Set promotable = new HashSet<>(); + for (ImmutableSet key : silos.keySet()) + promotable.addAll(promotableSSTables(key)); + return promotable; + } + + @VisibleForTesting + synchronized Set sstablesFor(ImmutableSet key) + { + CompactionStrategyHolder silo = silos.get(key); + return silo == null ? Collections.emptySet() : ImmutableSet.copyOf(sstablesIn(silo)); + } + + @Override + public synchronized int getEstimatedRemainingTasks() + { + pruneEmpty(); + int tasks = 0; + for (CompactionStrategyHolder silo : silos.values()) + tasks += silo.getEstimatedRemainingTasks(); + return tasks; + } + + /** + * One supplier per strategy, from the silos not awaiting promotion. + * + * The number each supplier carries is its own strategy's estimate, because that is what its callback compacts. + * {@link CompactionStrategyManager#getNextBackgroundTasks} sorts every holder's suppliers together, so a per silo + * or per manager total would let tracked compaction outrank a busier strategy elsewhere. + */ + @Override + public synchronized Collection getBackgroundTaskSuppliers(long gcBefore) + { + pruneEmpty(); + List suppliers = new ArrayList<>(); + for (CompactionStrategyHolder silo : silos.values()) + suppliers.addAll(silo.getBackgroundTaskSuppliers(gcBefore)); + return suppliers; + } + + @Override + public synchronized Collection getMaximalTasks(long gcBefore, boolean splitOutput) + { + pruneEmpty(); + + // Promotion first + List tasks = new ArrayList<>(getNextPromotionTasks()); + for (CompactionStrategyHolder silo : silos.values()) + { + Collection siloTasks = silo.getMaximalTasks(gcBefore, splitOutput); + if (siloTasks != null) + tasks.addAll(siloTasks); + } + return tasks; + } + + @VisibleForTesting + synchronized boolean hasDataFor(ImmutableSet key) + { + CompactionStrategyHolder silo = silos.get(key); + return silo != null && !isEmpty(silo); + } + + @VisibleForTesting + synchronized Set> keys() + { + return ImmutableSet.copyOf(silos.keySet()); + } + + synchronized Collection getNextPromotionTasks() + { + pruneEmpty(); + List tasks = new ArrayList<>(); + for (ImmutableSet key : silos.keySet()) + { + AbstractCompactionTask task = getPromotionTask(key); + if (task != null) + tasks.add(task); + } + return tasks; + } + + @VisibleForTesting + synchronized AbstractCompactionTask getPromotionTask(ImmutableSet key) + { + if (silos.get(key) == null) + return null; + + return PromoteReconciledTask.tryPromote(cfs, promotableSSTables(key), describe(key), this::pruneEmpty); + } +} diff --git a/src/java/org/apache/cassandra/db/lifecycle/Tracker.java b/src/java/org/apache/cassandra/db/lifecycle/Tracker.java index 6636f01256aa..47323c081d11 100644 --- a/src/java/org/apache/cassandra/db/lifecycle/Tracker.java +++ b/src/java/org/apache/cassandra/db/lifecycle/Tracker.java @@ -40,9 +40,12 @@ import org.apache.cassandra.config.DurationSpec; import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.Directories; +import org.apache.cassandra.db.LogDomain; import org.apache.cassandra.db.commitlog.CommitLogPosition; import org.apache.cassandra.db.compaction.OperationType; +import org.apache.cassandra.db.memtable.LogDomainBounds; import org.apache.cassandra.db.memtable.Memtable; +import org.apache.cassandra.db.memtable.SplitDomainMemtable; import org.apache.cassandra.io.sstable.format.SSTableReader; import org.apache.cassandra.io.sstable.metadata.StatsMetadata; import org.apache.cassandra.io.util.File; @@ -107,18 +110,19 @@ public class Tracker /** * @param columnFamilyStore * @param memtable Initial Memtable. Can be null. + * @param bounds The boundary the current memtable starts at. null only if there's no memtable. * @param loadsstables true to indicate to load SSTables (TODO: remove as this is only accessed from 2i) */ - public Tracker(ColumnFamilyStore columnFamilyStore, Memtable memtable, boolean loadsstables) + public Tracker(ColumnFamilyStore columnFamilyStore, Memtable memtable, LogDomainBounds bounds, boolean loadsstables) { this.cfstore = columnFamilyStore; this.loadsstables = loadsstables; - this.reset(memtable); + this.reset(memtable, bounds); } public static Tracker newDummyTracker() { - return new Tracker(null, null, false); + return new Tracker(null, null, null, false); } public LifecycleTransaction tryModify(SSTableReader sstable, OperationType operationType) @@ -319,7 +323,7 @@ private void addSSTablesInternal(Collection sstables, /** (Re)initializes the tracker, purging all references. */ @VisibleForTesting - public void reset(Memtable memtable) + public void reset(Memtable memtable, LogDomainBounds bounds) { viewUpdateLock.lock(); try @@ -328,7 +332,8 @@ public void reset(Memtable memtable) Collections.emptyList(), Collections.emptyMap(), Collections.emptyMap(), - SSTableIntervalTree.empty()); + SSTableIntervalTree.empty(), + bounds); } finally { @@ -415,7 +420,7 @@ public void removeUnreadableSSTables(final File directory) /** * get the Memtable that the ordered writeOp should be directed to */ - public Memtable getMemtableFor(OpOrder.Group opGroup, CommitLogPosition commitLogPosition) + public Memtable getMemtableFor(OpOrder.Group opGroup, CommitLogPosition commitLogPosition, LogDomain domain) { // since any new memtables appended to the list after we fetch it will be for operations started // after us, we can safely assume that we will always find the memtable that 'accepts' us; @@ -427,10 +432,62 @@ public Memtable getMemtableFor(OpOrder.Group opGroup, CommitLogPosition commitLo View view = this.view; for (Memtable memtable : view.liveMemtables) { - if (memtable.accepts(opGroup, commitLogPosition)) + if (memtable.accepts(opGroup, commitLogPosition, domain)) return memtable; } - throw new AssertionError(view.liveMemtables.toString()); + + // the only valid reason to leave the loop above without a memtable is if the current memtable doesn't + // address the domain we're trying to write for. Otherwise, this we have a bug and need to throw an exception + if (view.getCurrentMemtable().holds(domain)) + throw new AssertionError("No live memtable accepted a " + domain + " write, but the current one holds that " + + "domain: " + view.liveMemtables); + + // Otherwise we need to install a split memtable for the current generation + return installSplit(opGroup, commitLogPosition, domain); + } + + private static final int MAX_SPLIT_ATTEMPTS = 64; + + private Memtable installSplit(OpOrder.Group opGroup, CommitLogPosition commitLogPosition, LogDomain domain) + { + SplitDomainMemtable lastRefused = null; + + for (int attempt = 0; attempt < MAX_SPLIT_ATTEMPTS; attempt++) + { + View current = this.view; + Memtable oldMemtable = current.getCurrentMemtable(); + SplitDomainMemtable splitMemtable; + + if (oldMemtable instanceof SplitDomainMemtable) + { + // Split already, by a writer that beat us here. + splitMemtable = (SplitDomainMemtable) oldMemtable; + } + else + { + // The new internal takes the position this memtable generation's boundary holds for its domain, so + // its span begins exactly where the previous generation's ended. + Memtable newMemtable = cfstore.createMemtable(current.currentBounds.sealIfUnset(domain), domain); + splitMemtable = new SplitDomainMemtable(newMemtable, oldMemtable, oldMemtable.getMemtableId()); + + if (apply(View.canSplitMemtable(oldMemtable), View.splitMemtable(oldMemtable, splitMemtable)) == null) + continue; + } + + // Asked of the generation, which delegates to the internal holding the bounds this write is measured + // against. The generation is also what is returned, since put routes inside it. + if (splitMemtable.accepts(opGroup, commitLogPosition, domain)) + return splitMemtable; + + // don't loop on the same generation refusing our write + if (splitMemtable == lastRefused) + throw new AssertionError("A " + domain + " write was refused twice by the same generation " + splitMemtable + + " in " + view.liveMemtables); + lastRefused = splitMemtable; + } + + throw new AssertionError("Gave up routing a " + domain + " write after " + MAX_SPLIT_ATTEMPTS + + " attempts against " + view.liveMemtables); } /** @@ -441,9 +498,9 @@ public Memtable getMemtableFor(OpOrder.Group opGroup, CommitLogPosition commitLo * * @return the previously active memtable */ - public Memtable switchMemtable(boolean truncating, Memtable newMemtable) + public Memtable switchMemtable(boolean truncating, Memtable newMemtable, LogDomainBounds newBounds) { - Pair result = apply(View.switchMemtable(newMemtable)); + Pair result = apply(View.switchMemtable(newMemtable, newBounds)); if (truncating) notifyRenewed(newMemtable); else diff --git a/src/java/org/apache/cassandra/db/lifecycle/View.java b/src/java/org/apache/cassandra/db/lifecycle/View.java index 1d373203e70f..e317d55072f3 100644 --- a/src/java/org/apache/cassandra/db/lifecycle/View.java +++ b/src/java/org/apache/cassandra/db/lifecycle/View.java @@ -29,13 +29,16 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Function; import com.google.common.base.Functions; +import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import org.apache.cassandra.db.DecoratedKey; import org.apache.cassandra.db.PartitionPosition; +import org.apache.cassandra.db.memtable.LogDomainBounds; import org.apache.cassandra.db.memtable.Memtable; +import org.apache.cassandra.db.memtable.SplitDomainMemtable; import org.apache.cassandra.dht.AbstractBounds; import org.apache.cassandra.io.sstable.format.SSTableReader; import org.apache.cassandra.metrics.LatencyMetrics; @@ -87,16 +90,26 @@ public class View final SSTableIntervalTree intervalTree; - View(List liveMemtables, List flushingMemtables, Map sstables, Map compacting, SSTableIntervalTree intervalTree) + // log domain boundaries the current memtable generateion began at + final LogDomainBounds currentBounds; + + View(List liveMemtables, + List flushingMemtables, + Map sstables, + Map compacting, + SSTableIntervalTree intervalTree, + LogDomainBounds currentBounds) { assert liveMemtables != null; assert flushingMemtables != null; assert sstables != null; assert compacting != null; assert intervalTree != null; + assert currentBounds != null || liveMemtables.isEmpty(); this.liveMemtables = liveMemtables; this.flushingMemtables = flushingMemtables; + this.currentBounds = currentBounds; this.sstablesMap = sstables; this.sstables = sstablesMap.keySet(); @@ -279,7 +292,7 @@ public View apply(View view) assert all(mark, Helpers.idIn(view.sstablesMap)); return new View(view.liveMemtables, view.flushingMemtables, view.sstablesMap, replace(view.compactingMap, unmark, mark), - view.intervalTree); + view.intervalTree, view.currentBounds); } }; } @@ -314,7 +327,7 @@ public View apply(View view) SSTableIntervalTree sstableIntervalTree = SSTableIntervalTree.update(view.intervalTree, remove, add); if (sstableIntervalTreeLatency != null) sstableIntervalTreeLatency.addNano(Clock.Global.nanoTime() - treeBuildStart); - return new View(view.liveMemtables, view.flushingMemtables, sstableMap, view.compactingMap, sstableIntervalTree); + return new View(view.liveMemtables, view.flushingMemtables, sstableMap, view.compactingMap, sstableIntervalTree, view.currentBounds); } }; } @@ -331,13 +344,35 @@ public View apply(View view) SSTableIntervalTree sstableIntervalTree = SSTableIntervalTree.replace(view.intervalTree, replacementMap); if (sstableIntervalTreeLatency != null) sstableIntervalTreeLatency.addNano(Clock.Global.nanoTime() - treeBuildStart); - return new View(view.liveMemtables, view.flushingMemtables, sstableMap, view.compactingMap, sstableIntervalTree); + return new View(view.liveMemtables, view.flushingMemtables, sstableMap, view.compactingMap, sstableIntervalTree, view.currentBounds); } }; } - // called prior to initiating flush: add newMemtable to liveMemtables, making it the latest memtable - static Function switchMemtable(final Memtable newMemtable) + /** + * Replace the current memtable with a wrapper holding it, at the same position in the list. + *

+ * This is used when we encounter writes from 2 different backing log domains. + */ + static Function splitMemtable(final Memtable toWrap, final SplitDomainMemtable wrapper) + { + return view -> { + List live = view.liveMemtables; + Preconditions.checkArgument(live.get(live.size() - 1) == toWrap, "Only the current memtable can be split"); + List newLive = ImmutableList.builder() + .addAll(live.subList(0, live.size() - 1)) + .add(wrapper) + .build(); + return new View(newLive, view.flushingMemtables, view.sstablesMap, view.compactingMap, view.intervalTree, view.currentBounds); + }; + } + + static Predicate canSplitMemtable(final Memtable toSplit) + { + return view -> view.getCurrentMemtable() == toSplit; + } + + static Function switchMemtable(final Memtable newMemtable, final LogDomainBounds newBounds) { return new Function() { @@ -345,7 +380,7 @@ public View apply(View view) { List newLive = ImmutableList.builder().addAll(view.liveMemtables).add(newMemtable).build(); assert newLive.size() == view.liveMemtables.size() + 1; - return new View(newLive, view.flushingMemtables, view.sstablesMap, view.compactingMap, view.intervalTree); + return new View(newLive, view.flushingMemtables, view.sstablesMap, view.compactingMap, view.intervalTree, newBounds); } }; } @@ -364,7 +399,7 @@ public View apply(View view) filter(flushing, not(lessThan(toFlush))))); assert newLive.size() == live.size() - 1; assert newFlushing.size() == flushing.size() + 1; - return new View(newLive, newFlushing, view.sstablesMap, view.compactingMap, view.intervalTree); + return new View(newLive, newFlushing, view.sstablesMap, view.compactingMap, view.intervalTree, view.currentBounds); } }; } @@ -381,14 +416,14 @@ public View apply(View view) if (flushed == null || Iterables.isEmpty(flushed)) return new View(view.liveMemtables, flushingMemtables, view.sstablesMap, - view.compactingMap, view.intervalTree); + view.compactingMap, view.intervalTree, view.currentBounds); Map sstableMap = replace(view.sstablesMap, emptySet(), flushed); long treeBuildStart = Clock.Global.nanoTime(); SSTableIntervalTree sstableIntervalTree = SSTableIntervalTree.addSSTables(view.intervalTree, flushed); if (sstableIntervalTreeLatency != null) sstableIntervalTreeLatency.addNano(Clock.Global.nanoTime() - treeBuildStart); - return new View(view.liveMemtables, flushingMemtables, sstableMap, view.compactingMap, sstableIntervalTree); + return new View(view.liveMemtables, flushingMemtables, sstableMap, view.compactingMap, sstableIntervalTree, view.currentBounds); } }; } diff --git a/src/java/org/apache/cassandra/db/memtable/AbstractAllocatorMemtable.java b/src/java/org/apache/cassandra/db/memtable/AbstractAllocatorMemtable.java index 8e296ac6042e..29bf9d36ab15 100644 --- a/src/java/org/apache/cassandra/db/memtable/AbstractAllocatorMemtable.java +++ b/src/java/org/apache/cassandra/db/memtable/AbstractAllocatorMemtable.java @@ -33,6 +33,7 @@ import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.db.ClusteringComparator; import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.LogDomain; import org.apache.cassandra.db.commitlog.CommitLogPosition; import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.schema.TableMetadataRef; @@ -114,9 +115,9 @@ public static MemtablePool createMemtableAllocatorPoolInternal(Config.MemtableAl } // only to be used by init(), to setup the very first memtable for the cfs - public AbstractAllocatorMemtable(AtomicReference commitLogLowerBound, TableMetadataRef metadataRef, Owner owner) + public AbstractAllocatorMemtable(AtomicReference commitLogLowerBound, TableMetadataRef metadataRef, Owner owner, LogDomain domain) { - super(metadataRef, commitLogLowerBound); + super(metadataRef, commitLogLowerBound, domain); this.allocator = MEMORY_POOL.newAllocator(metadataRef.toString()); this.initialComparator = metadata.get().comparator; this.initialFactory = metadata().params.memtable.factory(); @@ -129,6 +130,18 @@ public MemtableAllocator getAllocator() return allocator; } + @Override + public Owner owner() + { + return owner; + } + + @Override + public boolean allocatesFromMemtablePool() + { + return true; + } + @Override public boolean shouldSwitch(ColumnFamilyStore.FlushReason reason, TableMetadata latest) { @@ -161,9 +174,9 @@ public void performSnapshot(String snapshotName) throw new AssertionError("performSnapshot must be implemented if shouldSwitch(SNAPSHOT) can return false."); } - public void switchOut(OpOrder.Barrier writeBarrier, AtomicReference commitLogUpperBound) + public void switchOut(OpOrder.Barrier writeBarrier, LogDomainBounds upperBounds) { - super.switchOut(writeBarrier, commitLogUpperBound); + super.switchOut(writeBarrier, upperBounds); allocator.setDiscarding(); } @@ -217,9 +230,9 @@ private static void scheduleFlush(Owner owner, int period) { protected void runMayThrow() { - Memtable current = owner.getCurrentMemtable(); - if (current instanceof AbstractAllocatorMemtable) - ((AbstractAllocatorMemtable) current).flushIfPeriodExpired(); + // Asked of the memtable rather than gated on its class, so a generation that delegates still gets the + // chance to flush. Gating on AbstractAllocatorMemtable silently stopped periodic flush for one. + owner.getCurrentMemtable().flushIfPeriodExpired(); } @Override @@ -231,7 +244,8 @@ public String toString() ScheduledExecutors.scheduledTasks.scheduleSelfRecurring(runnable, period, TimeUnit.MILLISECONDS); } - private void flushIfPeriodExpired() + @Override + public void flushIfPeriodExpired() { int period = metadata().params.memtableFlushPeriodInMs; if (period > 0 && (Clock.Global.nanoTime() - creationNano >= TimeUnit.MILLISECONDS.toNanos(period))) @@ -257,25 +271,27 @@ private void flushIfPeriodExpired() public static Future flushLargestMemtable() { float largestRatio = 0f; - AbstractAllocatorMemtable largestMemtable = null; + Memtable largestMemtable = null; Memtable.MemoryUsage largestUsage = null; float liveOnHeap = 0, liveOffHeap = 0; // we take a reference to the current main memtable for the CF prior to snapping its ownership ratios // to ensure we have some ordering guarantee for performing the switchMemtableIf(), i.e. we will only // swap if the memtables we are measuring here haven't already been swapped by the time we try to swap them - for (Memtable currentMemtable : ColumnFamilyStore.activeMemtables()) + // Candidacy and ownership are asked of the memtable rather than gated on its class. Gating on + // AbstractAllocatorMemtable skipped a generation that delegates entirely, so it was never reclaimed however + // large it grew, and undercounted a delegating index generation's memory. + for (Memtable current : ColumnFamilyStore.activeMemtables()) { - if (!(currentMemtable instanceof AbstractAllocatorMemtable)) + if (!current.allocatesFromMemtablePool()) continue; - AbstractAllocatorMemtable current = (AbstractAllocatorMemtable) currentMemtable; // find the total ownership ratio for the memtable and all SecondaryIndexes owned by this CF, // both on- and off-heap, and select the largest of the two ratios to weight this CF MemoryUsage usage = Memtable.newMemoryUsage(); current.addMemoryUsageTo(usage); - for (Memtable indexMemtable : current.owner.getIndexMemtables()) - if (indexMemtable instanceof AbstractAllocatorMemtable) + for (Memtable indexMemtable : current.owner().getIndexMemtables()) + if (indexMemtable.allocatesFromMemtablePool()) indexMemtable.addMemoryUsageTo(usage); float ratio = Math.max(usage.ownershipRatioOnHeap, usage.ownershipRatioOffHeap); @@ -299,10 +315,10 @@ public static Future flushLargestMemtable() float flushingOnHeap = MEMORY_POOL.onHeap.reclaimingRatio(); float flushingOffHeap = MEMORY_POOL.offHeap.reclaimingRatio(); logger.info("Flushing largest {} to free up room. Used total: {}, live: {}, flushing: {}, this: {}", - largestMemtable.owner, ratio(usedOnHeap, usedOffHeap), ratio(liveOnHeap, liveOffHeap), + largestMemtable.owner(), ratio(usedOnHeap, usedOffHeap), ratio(liveOnHeap, liveOffHeap), ratio(flushingOnHeap, flushingOffHeap), ratio(largestUsage.ownershipRatioOnHeap, largestUsage.ownershipRatioOffHeap)); - Future flushFuture = largestMemtable.owner.signalFlushRequired(largestMemtable, ColumnFamilyStore.FlushReason.MEMTABLE_LIMIT); + Future flushFuture = largestMemtable.owner().signalFlushRequired(largestMemtable, ColumnFamilyStore.FlushReason.MEMTABLE_LIMIT); flushFuture.addListener(() -> { try { diff --git a/src/java/org/apache/cassandra/db/memtable/AbstractMemtable.java b/src/java/org/apache/cassandra/db/memtable/AbstractMemtable.java index 1b6f999daa33..0a12b0ab561c 100644 --- a/src/java/org/apache/cassandra/db/memtable/AbstractMemtable.java +++ b/src/java/org/apache/cassandra/db/memtable/AbstractMemtable.java @@ -18,7 +18,6 @@ package org.apache.cassandra.db.memtable; -import java.util.Collection; import java.util.HashMap; import java.util.Map; import java.util.concurrent.ConcurrentSkipListSet; @@ -30,7 +29,6 @@ import java.util.function.Supplier; import com.google.common.annotations.VisibleForTesting; -import com.google.common.collect.ImmutableMap; import org.github.jamm.Unmetered; @@ -56,7 +54,7 @@ public abstract class AbstractMemtable implements Memtable // The smallest local deletion time for all partitions in this memtable protected AtomicLong minLocalDeletionTime = new AtomicLong(Long.MAX_VALUE); private final long id = nextId.incrementAndGet(); - private Map> onFlush = ImmutableMap.of(); + private final FlushListeners onFlush = new FlushListeners(); // Note: statsCollector has corresponding statistics to the two above, but starts with an epoch value which is not // correct for their usage. @@ -154,32 +152,14 @@ public LifecycleTransaction setFlushTransaction(LifecycleTransaction flushTransa } @Override - public synchronized > T ensureFlushListener(Object key, Supplier factory) + public > T ensureFlushListener(Object key, Supplier factory) { - if (onFlush == null) - return null; - - T listener = (T)onFlush.get(key); - if (null == listener) - { - listener = factory.get(); - onFlush = ImmutableMap.>builder() - .putAll(onFlush) - .put(key, listener) - .build(); - } - return listener; + return onFlush.ensureFlushListener(key, factory); } public void notifyFlushed() { - Collection> run; - synchronized (this) - { - run = onFlush.values(); - onFlush = null; - } - run.forEach(c -> c.accept(metadata())); + onFlush.notifyFlushed(metadata()); } protected static class ColumnsCollector diff --git a/src/java/org/apache/cassandra/db/memtable/AbstractMemtableWithCommitlog.java b/src/java/org/apache/cassandra/db/memtable/AbstractMemtableWithCommitlog.java index 4fe39a10ca96..1bb3662732c1 100644 --- a/src/java/org/apache/cassandra/db/memtable/AbstractMemtableWithCommitlog.java +++ b/src/java/org/apache/cassandra/db/memtable/AbstractMemtableWithCommitlog.java @@ -20,6 +20,7 @@ import java.util.concurrent.atomic.AtomicReference; +import org.apache.cassandra.db.LogDomain; import org.apache.cassandra.db.commitlog.CommitLog; import org.apache.cassandra.db.commitlog.CommitLogPosition; import org.apache.cassandra.schema.TableMetadataRef; @@ -32,7 +33,7 @@ public abstract class AbstractMemtableWithCommitlog extends AbstractMemtable { // The approximate lower bound by this memtable; must be <= commitLogLowerBound once our predecessor - // has been finalised, and this is enforced in the ColumnFamilyStore.setCommitLogUpperBound + // has been finalised, and this is enforced in LogDomainBounds.seal private final CommitLogPosition approximateCommitLogLowerBound = CommitLog.instance.getCurrentPosition(); // the precise lower bound of CommitLogPosition owned by this memtable; equal to its predecessor's commitLogUpperBound private final AtomicReference commitLogLowerBound; @@ -40,11 +41,36 @@ public abstract class AbstractMemtableWithCommitlog extends AbstractMemtable private volatile OpOrder.Barrier writeBarrier; // the precise upper bound of CommitLogPosition owned by this memtable private volatile AtomicReference commitLogUpperBound; + // which log every position above is drawn from; positions from the other log do not compare against them + private final LogDomain domain; - public AbstractMemtableWithCommitlog(TableMetadataRef metadataRef, AtomicReference commitLogLowerBound) + public AbstractMemtableWithCommitlog(TableMetadataRef metadataRef, + AtomicReference commitLogLowerBound, + LogDomain domain) { super(metadataRef); this.commitLogLowerBound = commitLogLowerBound; + this.domain = domain; + } + + public LogDomain domain() + { + return domain; + } + + @Override + public boolean holds(LogDomain writeDomain) + { + return writeDomain == domain; + } + + /** + * Refuse a write whose position is in the other log. + */ + protected void requireDomain(LogDomain writeDomain) + { + if (writeDomain != domain) + throw new IllegalArgumentException("Cannot put a " + writeDomain + " write into a " + domain + " memtable"); } public CommitLogPosition getApproximateCommitLogLowerBound() @@ -52,12 +78,12 @@ public CommitLogPosition getApproximateCommitLogLowerBound() return approximateCommitLogLowerBound; } - public void switchOut(OpOrder.Barrier writeBarrier, AtomicReference commitLogUpperBound) + public void switchOut(OpOrder.Barrier writeBarrier, LogDomainBounds upperBounds) { // This can prepare the memtable data for deletion; it will still be used while the flush is proceeding. // A setDiscarded call will follow. assert this.writeBarrier == null; - this.commitLogUpperBound = commitLogUpperBound; + this.commitLogUpperBound = upperBounds.forDomain(domain); this.writeBarrier = writeBarrier; } @@ -68,8 +94,12 @@ public void discard() // decide if this memtable should take the write, or if it should go to the next memtable @Override - public boolean accepts(OpOrder.Group opGroup, CommitLogPosition commitLogPosition) + public boolean accepts(OpOrder.Group opGroup, CommitLogPosition commitLogPosition, LogDomain domain) { + // we don't accept writes against other backing log domains + if (domain != this.domain) + return false; + // if the barrier hasn't been set yet, then this memtable is still the newest and is taking ALL writes. OpOrder.Barrier barrier = this.writeBarrier; if (barrier == null) diff --git a/src/java/org/apache/cassandra/db/memtable/AbstractShardedMemtable.java b/src/java/org/apache/cassandra/db/memtable/AbstractShardedMemtable.java index fef13fa667fa..f72d9e6945c2 100644 --- a/src/java/org/apache/cassandra/db/memtable/AbstractShardedMemtable.java +++ b/src/java/org/apache/cassandra/db/memtable/AbstractShardedMemtable.java @@ -24,6 +24,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.apache.cassandra.db.LogDomain; import org.apache.cassandra.db.commitlog.CommitLogPosition; import org.apache.cassandra.schema.TableMetadataRef; import org.apache.cassandra.utils.FBUtilities; @@ -55,9 +56,10 @@ public abstract class AbstractShardedMemtable extends AbstractAllocatorMemtable AbstractShardedMemtable(AtomicReference commitLogLowerBound, TableMetadataRef metadataRef, Owner owner, + LogDomain domain, Integer shardCountOption) { - super(commitLogLowerBound, metadataRef, owner); + super(commitLogLowerBound, metadataRef, owner, domain); int shardCount = shardCountOption != null ? shardCountOption : defaultShardCount; this.boundaries = owner.localRangeSplits(shardCount); } diff --git a/src/java/org/apache/cassandra/db/memtable/FlushListeners.java b/src/java/org/apache/cassandra/db/memtable/FlushListeners.java new file mode 100644 index 000000000000..435aa129a122 --- /dev/null +++ b/src/java/org/apache/cassandra/db/memtable/FlushListeners.java @@ -0,0 +1,65 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.memtable; + +import java.util.Collection; +import java.util.Map; +import java.util.function.Consumer; +import java.util.function.Supplier; + +import com.google.common.collect.ImmutableMap; + +import org.apache.cassandra.schema.TableMetadata; + +/** + * The callbacks to run once a memtable generation is flushed to disk + */ +class FlushListeners +{ + private Map> onFlush = ImmutableMap.of(); + + @SuppressWarnings("unchecked") + synchronized > T ensureFlushListener(Object key, Supplier factory) + { + if (onFlush == null) + return null; + + T listener = (T) onFlush.get(key); + if (null == listener) + { + listener = factory.get(); + onFlush = ImmutableMap.>builder() + .putAll(onFlush) + .put(key, listener) + .build(); + } + return listener; + } + + void notifyFlushed(TableMetadata metadata) + { + Collection> run; + synchronized (this) + { + run = onFlush.values(); + onFlush = null; + } + run.forEach(c -> c.accept(metadata)); + } +} diff --git a/src/java/org/apache/cassandra/db/memtable/Flushing.java b/src/java/org/apache/cassandra/db/memtable/Flushing.java index c53be17a21e3..67f84e5d5343 100644 --- a/src/java/org/apache/cassandra/db/memtable/Flushing.java +++ b/src/java/org/apache/cassandra/db/memtable/Flushing.java @@ -37,7 +37,6 @@ import org.apache.cassandra.db.PartitionPosition; import org.apache.cassandra.db.SerializationHeader; import org.apache.cassandra.db.SystemKeyspace; -import org.apache.cassandra.db.commitlog.IntervalSet; import org.apache.cassandra.db.lifecycle.LifecycleTransaction; import org.apache.cassandra.db.partitions.Partition; import org.apache.cassandra.db.rows.UnfilteredRowIterator; @@ -244,8 +243,7 @@ public static SSTableMultiWriter createFlushWriter(ColumnFamilyStore cfs, ActiveRepairService.UNREPAIRED_SSTABLE, ActiveRepairService.NO_PENDING_REPAIR, flushSet.coordinatorLogOffsets(), - new IntervalSet<>(flushSet.commitLogLowerBound(), - flushSet.commitLogUpperBound()), + flushSet.commitLogIntervals(), new SerializationHeader(true, flushSet.metadata(), flushSet.columns(), diff --git a/src/java/org/apache/cassandra/db/memtable/LogDomainBounds.java b/src/java/org/apache/cassandra/db/memtable/LogDomainBounds.java new file mode 100644 index 000000000000..30c62a7a516f --- /dev/null +++ b/src/java/org/apache/cassandra/db/memtable/LogDomainBounds.java @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.memtable; + +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; + +import com.google.common.annotations.VisibleForTesting; + +import org.apache.cassandra.db.LogDomain; +import org.apache.cassandra.db.commitlog.CommitLog; +import org.apache.cassandra.db.commitlog.CommitLogPosition; +import org.apache.cassandra.replication.MutationJournal; + +/** + * One commit log position per {@link LogDomain}, at the boundary between two memtable generations. + * + * The same instance is the upper bound of the generation being switched out and the lower bound of its replacement, so + * the two generations' spans in each log meet without a gap and without overlapping. + */ +public class LogDomainBounds +{ + private static final Function LIVE_LOGS = + domain -> domain.isJournal() ? MutationJournal.currentPositionOrNull() + : CommitLog.instance.getCurrentPosition(); + + private final AtomicReference commitLog; + private final AtomicReference journal; + + private LogDomainBounds(CommitLogPosition commitLog, CommitLogPosition journal) + { + this.commitLog = new AtomicReference<>(commitLog); + this.journal = new AtomicReference<>(journal); + } + + public static LogDomainBounds unset() + { + return new LogDomainBounds(null, null); + } + + public static LogDomainBounds atCurrentPositions() + { + return new LogDomainBounds(CommitLog.instance.getCurrentPosition(), MutationJournal.currentPositionOrNull()); + } + + public static LogDomainBounds of(CommitLogPosition position) + { + return new LogDomainBounds(position, position); + } + + public AtomicReference forDomain(LogDomain domain) + { + return domain.isJournal() ? journal : commitLog; + } + + public CommitLogPosition get(LogDomain domain) + { + return forDomain(domain).get(); + } + + public boolean isSealed(LogDomain domain) + { + return get(domain) instanceof Memtable.LastCommitLogPosition; + } + + /** + * Fix each bound at the end of the log it names, so that no write can take a position at or below it afterwards. + */ + public void seal() + { + seal(LIVE_LOGS); + } + + public AtomicReference sealIfUnset(LogDomain domain) + { + return sealIfUnset(domain, LIVE_LOGS); + } + + @VisibleForTesting + void seal(Function logs) + { + seal(commitLog, LogDomain.COMMIT_LOG, logs); + seal(journal, LogDomain.MUTATION_JOURNAL, logs); + } + + @VisibleForTesting + AtomicReference sealIfUnset(LogDomain domain, Function logs) + { + AtomicReference bound = forDomain(domain); + if (bound.get() == null) + seal(bound, domain, logs); + return bound; + } + + private static void seal(AtomicReference bound, + LogDomain domain, + Function logs) + { + while (true) + { + // Re-read on every attempt. A write admitted since the last read raises the bound above the position we + // hold, and retrying with that stale position could never satisfy the guard below. + CommitLogPosition position = logs.apply(domain); + if (position == null) + return; + + Memtable.LastCommitLogPosition sealed = new Memtable.LastCommitLogPosition(position); + CommitLogPosition current = bound.get(); + if ((current == null || current.compareTo(sealed) <= 0) && bound.compareAndSet(current, sealed)) + return; + } + } + + @Override + public String toString() + { + return "DomainBounds(commitLog=" + commitLog.get() + ", journal=" + journal.get() + ')'; + } +} diff --git a/src/java/org/apache/cassandra/db/memtable/Memtable.java b/src/java/org/apache/cassandra/db/memtable/Memtable.java index bfa17535f896..38d4968f484a 100644 --- a/src/java/org/apache/cassandra/db/memtable/Memtable.java +++ b/src/java/org/apache/cassandra/db/memtable/Memtable.java @@ -18,6 +18,8 @@ package org.apache.cassandra.db.memtable; +import java.util.Collections; +import java.util.List; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Supplier; @@ -27,9 +29,11 @@ import org.apache.cassandra.db.CellSourceIdentifier; import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.LogDomain; import org.apache.cassandra.db.PartitionPosition; import org.apache.cassandra.db.RegularAndStaticColumns; import org.apache.cassandra.db.commitlog.CommitLogPosition; +import org.apache.cassandra.db.commitlog.IntervalSet; import org.apache.cassandra.db.lifecycle.LifecycleTransaction; import org.apache.cassandra.db.partitions.Partition; import org.apache.cassandra.db.partitions.PartitionUpdate; @@ -82,13 +86,14 @@ interface Factory /** * Create a memtable. * - * @param commitLogLowerBound A commit log lower bound for the new memtable. This will be equal to the previous - * memtable's upper bound and defines the span of positions that any flushed sstable - * will cover. + * @param commitLogLowerBound This memtable's lower bound in the log named by {@code domain}. It is the previous + * generation's upper bound in that log, and defines the span of positions any + * flushed sstable will cover. * @param metadaRef Pointer to the up-to-date table metadata. * @param owner Owning objects that will receive flush requests triggered by the memtable (e.g. on expiration). + * @param domain Which backing log this memtable's bounds reference. */ - Memtable create(AtomicReference commitLogLowerBound, TableMetadataRef metadaRef, Owner owner); + Memtable create(AtomicReference commitLogLowerBound, TableMetadataRef metadaRef, Owner owner, LogDomain domain); /** * Create a release action for the memtable's metrics. This is used to release any resources that are not needed. @@ -186,9 +191,9 @@ interface Owner // Main write and read operations - default long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup) + default long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup, LogDomain domain) { - return put(mutationId, update, indexer, opGroup, false); + return put(mutationId, update, indexer, opGroup, domain, false); } /** @@ -198,6 +203,8 @@ default long put(MutationId mutationId, PartitionUpdate update, UpdateTransactio * @param indexer receives information about the update's effect * @param opGroup write operation group, used to permit the operation to complete if it is needed to complete a * flush to free space. + * @param domain which backing log the write's position reference. Attempting to write to the incorrect domain + * throws an exception. * @param assumeMissing if true, the implementation MAY clone the key and attempt putIfAbsent without first * looking for the keys' presence * @@ -205,7 +212,7 @@ default long put(MutationId mutationId, PartitionUpdate update, UpdateTransactio * timestamp delta being computed as the difference between the cells and DeletionTimes from any existing partition * and those in {@code update}. See CASSANDRA-7979. */ - long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup, boolean assumeMissing); + long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup, LogDomain domain, boolean assumeMissing); /** * Creates a point-in-time snapshot of a partition in this memtable. @@ -342,6 +349,19 @@ interface FlushablePartitionSet

extends Iterable

, SSTabl /** The commit log position at the time that this memtable was switched out */ CommitLogPosition commitLogUpperBound(); + /** + * The *commit log* span the flushed sstable covers, recorded in its {@code StatsMetadata}. + * + * Empty for a journal-domain memtable. Journal bounds have one consumer, {@code MutationJournal.notifyFlushed}, + * and a journal position stored here would be read back as a commit log position; + */ + default IntervalSet commitLogIntervals() + { + return memtable().holds(LogDomain.COMMIT_LOG) + ? new IntervalSet<>(commitLogLowerBound(), commitLogUpperBound()) + : IntervalSet.empty(); + } + /** The set of all columns that have been written */ RegularAndStaticColumns columns(); /** Statistics required for writing an sstable efficiently */ @@ -372,10 +392,20 @@ default boolean isEmpty() * @param writeBarrier The barrier that will signal that all writes to this memtable have completed. That is, the * point after which writes cannot be accepted by this memtable (it is permitted for writes * before this barrier to go into the next; see {@link #accepts}). - * @param commitLogUpperBound The upper commit log position for this memtable. The value may be modified after this - * call and will match the next memtable's lower commit log bound. + * @param upperBounds The generation boundary this memtable ends at. The position for the memtable's own domain is + * its upper bound; it may be modified after this call, and is the next generation's lower bound + * in that log. */ - void switchOut(OpOrder.Barrier writeBarrier, AtomicReference commitLogUpperBound); + void switchOut(OpOrder.Barrier writeBarrier, LogDomainBounds upperBounds); + + /** + * The memtables whose contents are written out when this generation flushes. This is where split domain memtables + * become 2 memtables for flushing to different memtables + */ + default List flushSources() + { + return Collections.singletonList(this); + } /** * This memtable is no longer in use or required for outstanding flushes or operations. @@ -385,13 +415,30 @@ default boolean isEmpty() /** * Decide if this memtable should take a write with the given parameters, or if the write should go to the next - * memtable. This enforces that no writes after the barrier set by {@link #switchOut} can be accepted, and - * is also used to define a shared commit log bound as the upper for this memtable and lower for the next. + * memtable (or split the memtable across domains if the domain doesn't match and this is the current memtable). + * This enforces that no writes after the barrier set by {@link #switchOut} can be accepted, and is also used to + * define a shared commit log bound as the upper for this memtable and lower for the next. */ - boolean accepts(OpOrder.Group opGroup, CommitLogPosition commitLogPosition); + boolean accepts(OpOrder.Group opGroup, CommitLogPosition commitLogPosition, LogDomain domain); long getMemtableId(); + /** + * Whether this memtable can accept writes from the given log domain + */ + boolean holds(LogDomain domain); + + Owner owner(); + + default boolean allocatesFromMemtablePool() + { + return false; + } + + default void flushIfPeriodExpired() + { + } + /** Approximate commit log lower bound, <= getCommitLogLowerBound, used as a time stamp for ordering */ CommitLogPosition getApproximateCommitLogLowerBound(); diff --git a/src/java/org/apache/cassandra/db/memtable/ShardedSkipListMemtable.java b/src/java/org/apache/cassandra/db/memtable/ShardedSkipListMemtable.java index daefbfbf0728..f2781794f8a3 100644 --- a/src/java/org/apache/cassandra/db/memtable/ShardedSkipListMemtable.java +++ b/src/java/org/apache/cassandra/db/memtable/ShardedSkipListMemtable.java @@ -33,6 +33,7 @@ import org.apache.cassandra.db.DataRange; import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.LogDomain; import org.apache.cassandra.db.PartitionPosition; import org.apache.cassandra.db.RegularAndStaticColumns; import org.apache.cassandra.db.Slices; @@ -91,10 +92,11 @@ public class ShardedSkipListMemtable extends AbstractShardedMemtable ShardedSkipListMemtable(AtomicReference commitLogLowerBound, TableMetadataRef metadataRef, Owner owner, + LogDomain domain, Integer shardCountOption, boolean locking) { - super(commitLogLowerBound, metadataRef, owner, shardCountOption); + super(commitLogLowerBound, metadataRef, owner, domain, shardCountOption); this.shards = generatePartitionShards(boundaries.shardCount(), allocator, metadataRef, locking); } @@ -143,8 +145,9 @@ else if (lastToken.compareTo(token) < 0) * * commitLogSegmentPosition should only be null if this is a secondary index, in which case it is *expected* to be null */ - public long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup, boolean assumeMissing) + public long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup, LogDomain domain, boolean assumeMissing) { + requireDomain(domain); DecoratedKey key = update.partitionKey(); MemtableShard shard = shards[boundaries.getShardForKey(key)]; return shard.put(mutationId, key, update, indexer, opGroup, assumeMissing); @@ -531,9 +534,9 @@ public UnfilteredRowIterator next() static class Locking extends ShardedSkipListMemtable { - Locking(AtomicReference commitLogLowerBound, TableMetadataRef metadataRef, Owner owner, Integer shardCountOption) + Locking(AtomicReference commitLogLowerBound, TableMetadataRef metadataRef, Owner owner, LogDomain domain, Integer shardCountOption) { - super(commitLogLowerBound, metadataRef, owner, shardCountOption, true); + super(commitLogLowerBound, metadataRef, owner, domain, shardCountOption, true); } /** @@ -542,8 +545,9 @@ static class Locking extends ShardedSkipListMemtable * * commitLogSegmentPosition should only be null if this is a secondary index, in which case it is *expected* to be null */ - public long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup, boolean assumeMissing) + public long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup, LogDomain domain, boolean assumeMissing) { + requireDomain(domain); DecoratedKey key = update.partitionKey(); MemtableShard shard = shards[boundaries.getShardForKey(key)]; synchronized (shard) @@ -575,11 +579,12 @@ static class Factory implements Memtable.Factory public Memtable create(AtomicReference commitLogLowerBound, TableMetadataRef metadataRef, - Owner owner) + Owner owner, + LogDomain domain) { return isLocking - ? new Locking(commitLogLowerBound, metadataRef, owner, shardCount) - : new ShardedSkipListMemtable(commitLogLowerBound, metadataRef, owner, shardCount, false); + ? new Locking(commitLogLowerBound, metadataRef, owner, domain, shardCount) + : new ShardedSkipListMemtable(commitLogLowerBound, metadataRef, owner, domain, shardCount, false); } public boolean equals(Object o) diff --git a/src/java/org/apache/cassandra/db/memtable/SkipListMemtable.java b/src/java/org/apache/cassandra/db/memtable/SkipListMemtable.java index f68eecd3f628..39d13a4ead48 100644 --- a/src/java/org/apache/cassandra/db/memtable/SkipListMemtable.java +++ b/src/java/org/apache/cassandra/db/memtable/SkipListMemtable.java @@ -34,6 +34,7 @@ import org.apache.cassandra.db.BufferDecoratedKey; import org.apache.cassandra.db.DataRange; import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.LogDomain; import org.apache.cassandra.db.PartitionPosition; import org.apache.cassandra.db.Slices; import org.apache.cassandra.db.commitlog.CommitLogPosition; @@ -93,9 +94,9 @@ public class SkipListMemtable extends AbstractAllocatorMemtable private final AtomicLong liveDataSize = new AtomicLong(0); - protected SkipListMemtable(AtomicReference commitLogLowerBound, TableMetadataRef metadataRef, Owner owner) + protected SkipListMemtable(AtomicReference commitLogLowerBound, TableMetadataRef metadataRef, Owner owner, LogDomain domain) { - super(commitLogLowerBound, metadataRef, owner); + super(commitLogLowerBound, metadataRef, owner, domain); } @Override @@ -120,8 +121,9 @@ public Token lastToken() * commitLogSegmentPosition should only be null if this is a secondary index, in which case it is *expected* to be null */ @Override - public long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup, boolean assumeMissing) + public long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup, LogDomain domain, boolean assumeMissing) { + requireDomain(domain); long initialSize = 0; Cloner cloner = allocator.cloner(opGroup); AtomicBTreePartition previous = assumeMissing ? null : partitions.get(update.partitionKey()); diff --git a/src/java/org/apache/cassandra/db/memtable/SkipListMemtableFactory.java b/src/java/org/apache/cassandra/db/memtable/SkipListMemtableFactory.java index 76dee02be18f..f97a214d1f9d 100644 --- a/src/java/org/apache/cassandra/db/memtable/SkipListMemtableFactory.java +++ b/src/java/org/apache/cassandra/db/memtable/SkipListMemtableFactory.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableMap; import org.apache.cassandra.config.InheritingClass; +import org.apache.cassandra.db.LogDomain; import org.apache.cassandra.db.commitlog.CommitLogPosition; import org.apache.cassandra.schema.TableMetadataRef; @@ -37,9 +38,9 @@ public class SkipListMemtableFactory implements Memtable.Factory { @Override - public Memtable create(AtomicReference commitLogLowerBound, TableMetadataRef metadaRef, Memtable.Owner owner) + public Memtable create(AtomicReference commitLogLowerBound, TableMetadataRef metadaRef, Memtable.Owner owner, LogDomain domain) { - return new SkipListMemtable(commitLogLowerBound, metadaRef, owner); + return new SkipListMemtable(commitLogLowerBound, metadaRef, owner, domain); } public static final SkipListMemtableFactory INSTANCE = new SkipListMemtableFactory(); diff --git a/src/java/org/apache/cassandra/db/memtable/SplitDomainMemtable.java b/src/java/org/apache/cassandra/db/memtable/SplitDomainMemtable.java new file mode 100644 index 000000000000..d11c3a169724 --- /dev/null +++ b/src/java/org/apache/cassandra/db/memtable/SplitDomainMemtable.java @@ -0,0 +1,412 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.memtable; + +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Consumer; +import java.util.function.Supplier; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; + +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.DataRange; +import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.LogDomain; +import org.apache.cassandra.db.PartitionPosition; +import org.apache.cassandra.db.Slices; +import org.apache.cassandra.db.commitlog.CommitLogPosition; +import org.apache.cassandra.db.filter.ColumnFilter; +import org.apache.cassandra.db.lifecycle.LifecycleTransaction; +import org.apache.cassandra.db.partitions.ImmutableBTreePartition; +import org.apache.cassandra.db.partitions.Partition; +import org.apache.cassandra.db.partitions.PartitionUpdate; +import org.apache.cassandra.db.partitions.UnfilteredPartitionIterator; +import org.apache.cassandra.db.partitions.UnfilteredPartitionIterators; +import org.apache.cassandra.db.rows.UnfilteredRowIterator; +import org.apache.cassandra.db.rows.UnfilteredRowIterators; +import org.apache.cassandra.dht.Token; +import org.apache.cassandra.index.transactions.UpdateTransaction; +import org.apache.cassandra.io.sstable.SSTableReadsListener; +import org.apache.cassandra.replication.MutationId; +import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.utils.concurrent.OpOrder; + +/** + * One logical memtable holding a separate internal memtable per {@link LogDomain}. + * + * A table takes writes from both logs while part of its token range is migrating, and a position from one log does not + * compare against a bound from the other. Splitting at the memtable means each write is bounded, flushed and accounted + * for against its own log, without any consumer of a bound having to ask which log it came from. + * + * The internal memtables always flush together and a common set of flush listeners are used. + */ +public class SplitDomainMemtable implements Memtable +{ + private final Memtable commitLogInternal; + private final Memtable journalInternal; + private final List internals; + private final long id; + private final AtomicReference flushTransaction = new AtomicReference<>(null); + private final FlushListeners listeners = new FlushListeners(); + + public SplitDomainMemtable(Memtable left, Memtable right, long id) + { + Preconditions.checkArgument(left.owner() == right.owner()); + if (left.holds(LogDomain.COMMIT_LOG)) + { + Preconditions.checkArgument(right.holds(LogDomain.MUTATION_JOURNAL)); + this.commitLogInternal = left; + this.journalInternal = right; + } + else + { + Preconditions.checkArgument(left.holds(LogDomain.MUTATION_JOURNAL)); + Preconditions.checkArgument(right.holds(LogDomain.COMMIT_LOG)); + this.commitLogInternal = right; + this.journalInternal = left; + } + this.internals = ImmutableList.of(commitLogInternal, journalInternal); + this.id = id; + } + + @Override + public List flushSources() + { + return internals; + } + + public Memtable internalFor(LogDomain domain) + { + return domain.isJournal() ? journalInternal : commitLogInternal; + } + + public boolean isInternal(Memtable memtable) + { + return memtable == commitLogInternal || memtable == journalInternal; + } + + @Override + public boolean accepts(OpOrder.Group opGroup, CommitLogPosition commitLogPosition, LogDomain domain) + { + return internalFor(domain).accepts(opGroup, commitLogPosition, domain); + } + + @Override + public boolean holds(LogDomain domain) + { + return true; + } + + @Override + public Owner owner() + { + // One store created both internals, which is asserted on construction + return commitLogInternal.owner(); + } + + @Override + public boolean allocatesFromMemtablePool() + { + return commitLogInternal.allocatesFromMemtablePool() || journalInternal.allocatesFromMemtablePool(); + } + + @Override + public void flushIfPeriodExpired() + { + commitLogInternal.flushIfPeriodExpired(); + journalInternal.flushIfPeriodExpired(); + } + + @Override + public long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup, LogDomain domain, boolean assumeMissing) + { + return internalFor(domain).put(mutationId, update, indexer, opGroup, domain, assumeMissing); + } + + @Override + public long partitionCount() + { + return commitLogInternal.partitionCount() + journalInternal.partitionCount(); + } + + @Override + public long getLiveDataSize() + { + return commitLogInternal.getLiveDataSize() + journalInternal.getLiveDataSize(); + } + + @Override + public long operationCount() + { + return commitLogInternal.operationCount() + journalInternal.operationCount(); + } + + @Override + public void addMemoryUsageTo(MemoryUsage usage) + { + commitLogInternal.addMemoryUsageTo(usage); + journalInternal.addMemoryUsageTo(usage); + } + + @Override + public long getMinTimestamp() + { + long commitLog = commitLogInternal.getMinTimestamp(); + long journal = journalInternal.getMinTimestamp(); + if (commitLog == NO_MIN_TIMESTAMP) + return journal; + if (journal == NO_MIN_TIMESTAMP) + return commitLog; + return Math.min(commitLog, journal); + } + + @Override + public long getMinLocalDeletionTime() + { + return Math.min(commitLogInternal.getMinLocalDeletionTime(), journalInternal.getMinLocalDeletionTime()); + } + + /** + * Conjoined rather than taken from either internal. Accord's durability path reads this and reports a command store + * durable without registering a flush listener when it is true, so reporting clean while the other internal still + * holds unflushed data would declare a transaction durable that is not. + */ + @Override + public boolean isClean() + { + return commitLogInternal.isClean() && journalInternal.isClean(); + } + + @Override + public void discard() + { + commitLogInternal.discard(); + journalInternal.discard(); + } + + @Override + public void metadataUpdated() + { + commitLogInternal.metadataUpdated(); + journalInternal.metadataUpdated(); + } + + @Override + public void localRangesUpdated() + { + commitLogInternal.localRangesUpdated(); + journalInternal.localRangesUpdated(); + } + + @Override + public void performSnapshot(String snapshotName) + { + commitLogInternal.performSnapshot(snapshotName); + journalInternal.performSnapshot(snapshotName); + } + + @Override + public boolean shouldSwitch(ColumnFamilyStore.FlushReason reason, TableMetadata latest) + { + // Either internal wanting to switch switches the whole generation, since they flush together. + return commitLogInternal.shouldSwitch(reason, latest) || journalInternal.shouldSwitch(reason, latest); + } + + @Override + public void markExtraOnHeapUsed(long additionalSpace, OpOrder.Group opGroup) + { + commitLogInternal.markExtraOnHeapUsed(additionalSpace, opGroup); + } + + @Override + public void markExtraOffHeapUsed(long additionalSpace, OpOrder.Group opGroup) + { + commitLogInternal.markExtraOffHeapUsed(additionalSpace, opGroup); + } + + @Override + public long getMemtableId() + { + return id; + } + + @Override + public TableMetadata metadata() + { + return commitLogInternal.metadata(); + } + + @Override + public LifecycleTransaction getFlushTransaction() + { + return flushTransaction.get(); + } + + @Override + public LifecycleTransaction setFlushTransaction(LifecycleTransaction transaction) + { + return flushTransaction.getAndSet(transaction); + } + + /** + * Held here rather than on an internal, so one listener exists per logical generation and fires once both internals + * are durable. A listener registered on an internal would fire on a partial generation. + */ + @Override + public > T ensureFlushListener(Object key, Supplier factory) + { + return listeners.ensureFlushListener(key, factory); + } + + @Override + public void notifyFlushed() + { + listeners.notifyFlushed(metadata()); + } + + // Commit-log-only accessors + + /** + * The commit-log internal's value, not an aggregate. Its only consumer is commit log segment reclamation, via + * {@link ColumnFamilyStore#forceFlush(CommitLogPosition)} from + * {@code AbstractCommitLogSegmentManager}, and only that internal can hold commit-log-derived rows. + * + * Aggregating would be wrong rather than merely redundant: the field is initialized from + * {@code CommitLog.instance.getCurrentPosition()} on every memtable regardless of domain, so the journal internal's + * value describes when it was created, not what it holds. Folding it in reports commit log data the memtable does + * not have and pins segments that could be recycled. + */ + @Override + public CommitLogPosition getApproximateCommitLogLowerBound() + { + return commitLogInternal.getApproximateCommitLogLowerBound(); + } + + /** Answered by the commit-log internal alone, for the reason on {@link #getApproximateCommitLogLowerBound}. */ + @Override + public boolean mayContainDataBefore(CommitLogPosition position) + { + return commitLogInternal.mayContainDataBefore(position); + } + + @Override + public CommitLogPosition getCommitLogLowerBound() + { + return commitLogInternal.getCommitLogLowerBound(); + } + + @Override + public LastCommitLogPosition getFinalCommitLogUpperBound() + { + return commitLogInternal.getFinalCommitLogUpperBound(); + } + + /** + * Both internals take the same boundary and the same barrier, and each reads the position for its own domain from + * it. One barrier, because the generation flushes as a whole; see the class javadoc. + */ + @Override + public void switchOut(OpOrder.Barrier writeBarrier, LogDomainBounds upperBounds) + { + commitLogInternal.switchOut(writeBarrier, upperBounds); + journalInternal.switchOut(writeBarrier, upperBounds); + } + + @Override + public FlushablePartitionSet getFlushSet(PartitionPosition from, PartitionPosition to) + { + throw new UnsupportedOperationException("Flush iterates flushSources(), so that each output carries one domain's bounds"); + } + + @Override + public Partition snapshotPartition(DecoratedKey key) + { + Partition fromCommitLog = commitLogInternal.snapshotPartition(key); + Partition fromJournal = journalInternal.snapshotPartition(key); + + if (fromCommitLog == null || fromCommitLog.isEmpty()) + return fromJournal; + if (fromJournal == null || fromJournal.isEmpty()) + return fromCommitLog; + + try (UnfilteredRowIterator merged = UnfilteredRowIterators.merge(ImmutableList.of(fromCommitLog.unfilteredIterator(), + fromJournal.unfilteredIterator()))) + { + return ImmutableBTreePartition.create(merged); + } + } + + @Override + public UnfilteredRowIterator rowIterator(DecoratedKey key, + Slices slices, + ColumnFilter columnFilter, + boolean reversed, + SSTableReadsListener listener) + { + UnfilteredRowIterator fromCommitLog = commitLogInternal.rowIterator(key, slices, columnFilter, reversed, listener); + UnfilteredRowIterator fromJournal = journalInternal.rowIterator(key, slices, columnFilter, reversed, listener); + return mergeRows(fromCommitLog, fromJournal); + } + + @Override + public UnfilteredRowIterator rowIterator(DecoratedKey key) + { + return mergeRows(commitLogInternal.rowIterator(key), journalInternal.rowIterator(key)); + } + + private static UnfilteredRowIterator mergeRows(UnfilteredRowIterator fromCommitLog, UnfilteredRowIterator fromJournal) + { + if (fromCommitLog == null) + return fromJournal; + if (fromJournal == null) + return fromCommitLog; + return UnfilteredRowIterators.merge(ImmutableList.of(fromCommitLog, fromJournal)); + } + + @Override + public UnfilteredPartitionIterator partitionIterator(ColumnFilter columnFilter, + DataRange dataRange, + SSTableReadsListener listener) + { + return UnfilteredPartitionIterators.merge(ImmutableList.of(commitLogInternal.partitionIterator(columnFilter, dataRange, listener), + journalInternal.partitionIterator(columnFilter, dataRange, listener)), + UnfilteredPartitionIterators.MergeListener.NOOP); + } + + @Override + public Token lastToken() + { + Token fromCommitLog = commitLogInternal.lastToken(); + Token fromJournal = journalInternal.lastToken(); + + if (fromCommitLog == null) + return fromJournal; + if (fromJournal == null) + return fromCommitLog; + return fromCommitLog.compareTo(fromJournal) >= 0 ? fromCommitLog : fromJournal; + } + + @Override + public String toString() + { + return "DomainSplitMemtable(id=" + id + ", commitLog=" + commitLogInternal + ", journal=" + journalInternal + ')'; + } +} diff --git a/src/java/org/apache/cassandra/db/memtable/TrieMemtable.java b/src/java/org/apache/cassandra/db/memtable/TrieMemtable.java index dc165483d68b..9b38a978b5c6 100644 --- a/src/java/org/apache/cassandra/db/memtable/TrieMemtable.java +++ b/src/java/org/apache/cassandra/db/memtable/TrieMemtable.java @@ -41,6 +41,7 @@ import org.apache.cassandra.db.DataRange; import org.apache.cassandra.db.DecoratedKey; import org.apache.cassandra.db.DeletionInfo; +import org.apache.cassandra.db.LogDomain; import org.apache.cassandra.db.PartitionPosition; import org.apache.cassandra.db.RegularAndStaticColumns; import org.apache.cassandra.db.Slices; @@ -125,9 +126,9 @@ public class TrieMemtable extends AbstractShardedMemtable @Unmetered private final TrieMemtableMetricsView metrics; - TrieMemtable(AtomicReference commitLogLowerBound, TableMetadataRef metadataRef, Owner owner, Integer shardCountOption) + TrieMemtable(AtomicReference commitLogLowerBound, TableMetadataRef metadataRef, Owner owner, LogDomain domain, Integer shardCountOption) { - super(commitLogLowerBound, metadataRef, owner, shardCountOption); + super(commitLogLowerBound, metadataRef, owner, domain, shardCountOption); this.metrics = new TrieMemtableMetricsView(metadataRef.keyspace, metadataRef.name); this.shards = generatePartitionShards(boundaries.shardCount(), allocator, metadataRef, metrics); this.mergedTrie = makeMergedTrie(shards); @@ -186,8 +187,9 @@ public void discard() * commitLogSegmentPosition should only be null if this is a secondary index, in which case it is *expected* to be null */ @Override - public long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup, boolean assumeMissing) + public long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup, LogDomain domain, boolean assumeMissing) { + requireDomain(domain); try { DecoratedKey key = update.partitionKey(); @@ -801,9 +803,10 @@ static class Factory implements Memtable.Factory public Memtable create(AtomicReference commitLogLowerBound, TableMetadataRef metadaRef, - Owner owner) + Owner owner, + LogDomain domain) { - return new TrieMemtable(commitLogLowerBound, metadaRef, owner, shardCount); + return new TrieMemtable(commitLogLowerBound, metadaRef, owner, domain, shardCount); } @Override diff --git a/src/java/org/apache/cassandra/db/tracked/TrackedKeyspaceWriteHandler.java b/src/java/org/apache/cassandra/db/tracked/TrackedKeyspaceWriteHandler.java index 2be356538cc8..6f3caa0a081b 100644 --- a/src/java/org/apache/cassandra/db/tracked/TrackedKeyspaceWriteHandler.java +++ b/src/java/org/apache/cassandra/db/tracked/TrackedKeyspaceWriteHandler.java @@ -20,6 +20,7 @@ import org.apache.cassandra.db.CassandraWriteContext; import org.apache.cassandra.db.Keyspace; import org.apache.cassandra.db.KeyspaceWriteHandler; +import org.apache.cassandra.db.LogDomain; import org.apache.cassandra.db.Mutation; import org.apache.cassandra.db.WriteContext; import org.apache.cassandra.db.commitlog.CommitLogPosition; @@ -32,14 +33,14 @@ public class TrackedKeyspaceWriteHandler implements KeyspaceWriteHandler { @Override - public WriteContext beginWrite(Mutation mutation, boolean makeDurable) throws RequestExecutionException + public WriteContext beginWrite(Mutation mutation, boolean makeDurable, boolean isReplay) throws RequestExecutionException { OpOrder.Group group = null; try { group = Keyspace.writeOrder.start(); - MigrationRouter.validateTrackedMutation(mutation); + MigrationRouter.validateTrackedMutation(mutation, isReplay); CommitLogPosition pointer = null; if (makeDurable) @@ -48,7 +49,7 @@ public WriteContext beginWrite(Mutation mutation, boolean makeDurable) throws Re pointer = MutationJournal.instance().write(mutation.id(), mutation); } - return new CassandraWriteContext(group, pointer); + return new CassandraWriteContext(group, pointer, LogDomain.MUTATION_JOURNAL); } catch (Throwable t) { @@ -75,7 +76,9 @@ private WriteContext createEmptyContext() OpOrder.Group group = Keyspace.writeOrder.start(); try { - return new CassandraWriteContext(group, null); + // Index rebuild and read contexts append to neither log. Commit-log domain because the writes they + // carry are index updates derived from data already durable, never journal appends of their own. + return new CassandraWriteContext(group, null, LogDomain.COMMIT_LOG); } catch (Throwable t) { diff --git a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java index 54bc677a2ad4..3885ccd1ccc4 100644 --- a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java +++ b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java @@ -611,7 +611,13 @@ private void insert(ByteBuffer rowKey, cell)); Row row = BTreeRow.noCellLiveRow(buildIndexClustering(rowKey, clustering, cell), info); PartitionUpdate upd = partitionUpdate(valueKey, row); - indexCfs.getWriteHandler().write(MutationId.fixme(), upd, ctx, false); + + // we always use MutationId.NONE for index writes, even if the write itself is originating from a journal + // write. This is because the journals segment bookkeeping is only against the base table, and the 2i memtables + // are flushed synchronously with the base table, so it doesn't affect segment dropping. Additionally, the 2i + // patch isn't inolved in replication, and sstable compaction is affected by replication status so it's easier + // to just treat all 2i data as untracked. Same way incremental repair doesn't move 2i sstables around. + indexCfs.getWriteHandler().write(MutationId.none(), upd, ctx, false); logger.trace("Inserted entry into index for value {}", valueKey); } @@ -657,7 +663,7 @@ private void doDelete(DecoratedKey indexKey, { Row row = BTreeRow.emptyDeletedRow(indexClustering, Row.Deletion.regular(deletion)); PartitionUpdate upd = partitionUpdate(indexKey, row); - indexCfs.getWriteHandler().write(MutationId.fixme(), upd, ctx, false); + indexCfs.getWriteHandler().write(MutationId.none(), upd, ctx, false); logger.trace("Removed index entry for value {}", indexKey); } diff --git a/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java b/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java index 7262af49cd5a..068c22fc26b8 100644 --- a/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java +++ b/src/java/org/apache/cassandra/io/sstable/format/SSTableReader.java @@ -1413,6 +1413,23 @@ public void mutateCoordinatorLogOffsetsAndReload(ImmutableCoordinatorLogOffsets } } + /** + * Promote a reconciled sstable to repaired and clear its coordinator log offsets + */ + public void mutatePromotedToRepairedAndReload(long newRepairedAt) throws IOException + { + ImmutableCoordinatorLogOffsets cleared = new ImmutableCoordinatorLogOffsets.Builder().build(); + synchronized (tidy.global) + { + descriptor.getMetadataSerializer() + .mutate(descriptor, + "promoted to repaired at " + newRepairedAt + " with offsets cleared", + stats -> stats.mutateRepairedMetadata(newRepairedAt, ActiveRepairService.NO_PENDING_REPAIR) + .mutateCoordinatorLogOffsets(cleared)); + reloadSSTableMetadata(); + } + } + /** * Reloads the sstable metadata from disk. *

diff --git a/src/java/org/apache/cassandra/io/sstable/format/SSTableWriter.java b/src/java/org/apache/cassandra/io/sstable/format/SSTableWriter.java index b635f367753c..48ad03c9cc4c 100644 --- a/src/java/org/apache/cassandra/io/sstable/format/SSTableWriter.java +++ b/src/java/org/apache/cassandra/io/sstable/format/SSTableWriter.java @@ -373,11 +373,22 @@ protected final Map finalizeMetadata() if (MutationTrackingService.instance().isDurablyReconciled(coordinatorLogOffsets)) { repairedAt = Clock.Global.currentTimeMillis(); + // Clear the offsets, as PromoteReconciledTask does. Offsets are only meaningful while an sstable is + // unrepaired, so setting repairedAt and keeping them would leave it claiming journal provenance. + coordinatorLogOffsets = new ImmutableCoordinatorLogOffsets.Builder().build(); logger.debug("Marking SSTable {} as reconciled with repairedAt {}", descriptor, repairedAt); } } } + // Offsets are only meaningful while an sstable is unrepaired, and this runs for compaction and anticompaction + // outputs as well as flushes. Those inherit repairedAt from their inputs, so the branch above is skipped and + // the union of the inputs' offsets would otherwise be written through. An anticompaction during a migration + // does exactly that: it repairs a journal-derived sstable in a pending range without clearing its offsets, and + // a later compaction in the repaired holder can then union it with commit-log-derived data. + if (repairedAt != ActiveRepairService.UNREPAIRED_SSTABLE) + coordinatorLogOffsets = ImmutableCoordinatorLogOffsets.NONE; + return metadataCollector.finalizeMetadata(getPartitioner().getClass().getCanonicalName(), metadata().params.bloomFilterFpChance, repairedAt, diff --git a/src/java/org/apache/cassandra/io/sstable/format/SortedTableVerifier.java b/src/java/org/apache/cassandra/io/sstable/format/SortedTableVerifier.java index 6419695dd9ca..b47f4b485d0f 100644 --- a/src/java/org/apache/cassandra/io/sstable/format/SortedTableVerifier.java +++ b/src/java/org/apache/cassandra/io/sstable/format/SortedTableVerifier.java @@ -125,7 +125,12 @@ protected void markAndThrow(Throwable cause) protected void markAndThrow(Throwable cause, boolean mutateRepaired) { - if (mutateRepaired && options.mutateRepairStatus) // if we are able to mutate repaired flag, an incremental repair should be enough + // Marking the sstable unrepaired exists so that incremental repair notices it and repairs it. That would + // create an unreconcilable table on a tracked table + boolean tracked = cfs.metadata().replicationType().isTracked(); + boolean unRepair = mutateRepaired && options.mutateRepairStatus && !tracked; + + if (unRepair) { try { @@ -137,7 +142,24 @@ protected void markAndThrow(Throwable cause, boolean mutateRepaired) outputHandler.output("Error mutating repairedAt for SSTable %s, as part of markAndThrow", sstable.getFilename()); } } - Exception e = new Exception(String.format("Invalid SSTable %s, please force %srepair", sstable.getFilename(), (mutateRepaired && options.mutateRepairStatus) ? "" : "a full "), cause); + + Exception e; + if (tracked) + { + // Full repair adds correct data but cannot remove the corrupt row: read resolution is last-write-wins on + // timestamp, so a corrupt row with a mangled high timestamp still beats the row repair streams in. + String message = String.format("Invalid SSTable %s on tracked table %s.%s; repairedAt left unchanged. " + + "Run a full repair to restore any missing data, and scrub or replace the " + + "node to remove the corruption - a corrupt row can still win read " + + "resolution on timestamp, so full repair alone may not be sufficient.", + sstable.getFilename(), cfs.metadata.keyspace, cfs.metadata.name); + outputHandler.warn(message); + e = new Exception(message, cause); + } + else + { + e = new Exception(String.format("Invalid SSTable %s, please force %srepair", sstable.getFilename(), unRepair ? "" : "a full "), cause); + } if (options.invokeDiskFailurePolicy) throw new CorruptSSTableException(e, sstable.getFilename()); else diff --git a/src/java/org/apache/cassandra/io/sstable/metadata/StatsMetadata.java b/src/java/org/apache/cassandra/io/sstable/metadata/StatsMetadata.java index 81640ced44a1..05541474d333 100644 --- a/src/java/org/apache/cassandra/io/sstable/metadata/StatsMetadata.java +++ b/src/java/org/apache/cassandra/io/sstable/metadata/StatsMetadata.java @@ -43,6 +43,7 @@ import org.apache.cassandra.io.util.DataOutputPlus; import org.apache.cassandra.replication.ImmutableCoordinatorLogOffsets; import org.apache.cassandra.serializers.AbstractTypeSerializer; +import org.apache.cassandra.service.ActiveRepairService; import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.EstimatedHistogram; import org.apache.cassandra.utils.TimeUUID; @@ -212,8 +213,20 @@ public StatsMetadata mutateLevel(int newLevel) lastKey); } + /** + * Coordinator log offsets are dropped whenever this sets {@code repairedAt}. They are only meaningful while an + * sstable is unrepaired: repairing it asserts the data is consistent on every replica, which is what the offsets + * exist to establish, and {@code TrackedCompactionManager.isPromotable} declines a repaired sstable, so anything + * left behind here could never be cleared afterwards. + * + * A pending-repair session leaves {@code repairedAt} unset, so the offsets survive it. That matters, because a + * failed session returns the sstable to unrepaired and it has to stay promotable. + */ public StatsMetadata mutateRepairedMetadata(long newRepairedAt, TimeUUID newPendingRepair) { + ImmutableCoordinatorLogOffsets newOffsets = newRepairedAt != ActiveRepairService.UNREPAIRED_SSTABLE + ? ImmutableCoordinatorLogOffsets.NONE + : coordinatorLogOffsets; return new StatsMetadata(estimatedPartitionSize, estimatedCellPerPartitionCount, commitLogIntervals, @@ -236,7 +249,7 @@ public StatsMetadata mutateRepairedMetadata(long newRepairedAt, TimeUUID newPend originatingHostId, newPendingRepair, hasPartitionLevelDeletions, - coordinatorLogOffsets, + newOffsets, firstKey, lastKey); } diff --git a/src/java/org/apache/cassandra/metrics/TableMetrics.java b/src/java/org/apache/cassandra/metrics/TableMetrics.java index 356e9c8e92b4..2d4c5cc66d96 100644 --- a/src/java/org/apache/cassandra/metrics/TableMetrics.java +++ b/src/java/org/apache/cassandra/metrics/TableMetrics.java @@ -60,6 +60,7 @@ import org.apache.cassandra.schema.Schema; import org.apache.cassandra.schema.SchemaConstants; import org.apache.cassandra.service.reads.ReplicaFilteringProtection; +import org.apache.cassandra.tcm.ClusterMetadata; import org.apache.cassandra.utils.EstimatedHistogram; import org.apache.cassandra.utils.ExpMovingAverage; import org.apache.cassandra.utils.MovingAverage; @@ -209,6 +210,8 @@ public class TableMetrics public final Gauge bytesRepaired; public final Gauge bytesUnrepaired; public final Gauge bytesPendingRepair; + /** SSTables on a tracked table that can't be promoted, because they are unrepaired and contain no offsets */ + public final Gauge unpromotableSSTables; /** Number of started repairs as coordinator on this table */ public final Counter repairsStarted; /** Number of completed repairs as coordinator on this table */ @@ -632,6 +635,32 @@ public Long getValue() } }); + // An unrepaired sstable with no coordinator log offsets cannot be promoted by reconciliation, because it names + // no mutations to reconcile, and it cannot be promoted by incremental repair once the table is fully migrated. + // It therefore occupies the unrepaired pool indefinitely. During migration this is the ordinary state of + // pre-migration data, so the gauge reports zero until the keyspace leaves migration state. + unpromotableSSTables = createTableGauge("UnpromotableSSTables", new Gauge() + { + public Integer getValue() + { + if (!cfs.metadata().replicationType().isTracked()) + return 0; + + ClusterMetadata metadata = ClusterMetadata.currentNullable(); + if (metadata == null || metadata.mutationTrackingMigrationState.isMigrating(cfs.getKeyspaceName())) + return 0; + + int count = 0; + for (SSTableReader sstable : cfs.getSSTables(SSTableSet.CANONICAL)) + { + if (!sstable.isRepaired() && !sstable.isPendingRepair() + && sstable.getSSTableMetadata().coordinatorLogOffsets.isEmpty()) + count++; + } + return count; + } + }); + bytesPendingRepair = createTableGauge("BytesPendingRepair", new Gauge() { public Long getValue() diff --git a/src/java/org/apache/cassandra/repair/RepairJob.java b/src/java/org/apache/cassandra/repair/RepairJob.java index db7079efc1be..a97fb35d98dd 100644 --- a/src/java/org/apache/cassandra/repair/RepairJob.java +++ b/src/java/org/apache/cassandra/repair/RepairJob.java @@ -295,7 +295,7 @@ public void onSuccess(List stats) cfs.metric.repairsCompleted.inc(); logger.info("Completing repair with excludedDeadNodes {}", session.excludedDeadNodes); ConsensusMigrationRepairResult cmrs = ConsensusMigrationRepairResult.fromRepair(repairStartingEpoch, getUnchecked(accordRepair), session.repairData, doPaxosRepair, doAccordRepair, session.excludedDeadNodes, session.isIncremental); - MutationTrackingMigrationRepairResult mtmrs = MutationTrackingMigrationRepairResult.fromRepair(repairStartingEpoch, session.excludedDeadNodes, session.previewKind.isPreview()); + MutationTrackingMigrationRepairResult mtmrs = MutationTrackingMigrationRepairResult.fromRepair(repairStartingEpoch, session.excludedDeadNodes, session.previewKind.isPreview(), session.isIncremental); trySuccess(new RepairResult(desc, stats, cmrs, mtmrs)); } diff --git a/src/java/org/apache/cassandra/replication/ImmutableCoordinatorLogOffsets.java b/src/java/org/apache/cassandra/replication/ImmutableCoordinatorLogOffsets.java index 441957b1e2b1..f3a9099c43cd 100644 --- a/src/java/org/apache/cassandra/replication/ImmutableCoordinatorLogOffsets.java +++ b/src/java/org/apache/cassandra/replication/ImmutableCoordinatorLogOffsets.java @@ -19,13 +19,17 @@ package org.apache.cassandra.replication; import java.io.IOException; +import java.util.ArrayList; +import java.util.Comparator; import java.util.Iterator; +import java.util.List; import java.util.Map; import java.util.Objects; import java.util.function.Predicate; import javax.annotation.concurrent.NotThreadSafe; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterators; import org.agrona.collections.Long2ObjectHashMap; @@ -45,9 +49,16 @@ public class ImmutableCoordinatorLogOffsets implements CoordinatorLogOffsets EMPTY = ImmutableSet.of(); + private final ImmutableMutations mutations; private final ActivatedTransfers transfers; + /** + * materialized sorted tracked transfer key for unreconciled compaction silo assignment + */ + private final ImmutableSet transferSiloKey; + private ImmutableCoordinatorLogOffsets(Builder builder) { // Important to set shouldAvoidAllocation=false, otherwise iterators are cached and not thread safe, even when immutable and read-only @@ -58,6 +69,24 @@ private ImmutableCoordinatorLogOffsets(Builder builder) this.mutations = new ImmutableMutations(ids); this.transfers = ActivatedTransfers.copyOf(builder.transfers); + this.transferSiloKey = buildTransferSiloKey(this.transfers); + } + + private static ImmutableSet buildTransferSiloKey(ActivatedTransfers transfers) + { + if (transfers == null || transfers.isEmpty()) + return EMPTY; + + List ids = new ArrayList<>(2); + for (ShortMutationId id : transfers) + ids.add(id); + ids.sort(Comparator.naturalOrder()); + return ImmutableSet.copyOf(ids); + } + + public ImmutableSet transferSiloKey() + { + return transferSiloKey; } @Override diff --git a/src/java/org/apache/cassandra/replication/MutationId.java b/src/java/org/apache/cassandra/replication/MutationId.java index 4f0464799e98..c14b9ca8d9e9 100644 --- a/src/java/org/apache/cassandra/replication/MutationId.java +++ b/src/java/org/apache/cassandra/replication/MutationId.java @@ -19,7 +19,6 @@ import java.io.IOException; import java.nio.ByteBuffer; -import java.util.Comparator; import org.apache.cassandra.db.TypeSizes; import org.apache.cassandra.io.UnversionedSerializer; @@ -108,12 +107,6 @@ public String toString() return "MutationId{" + hostId + ", " + hostLogId + ", " + offset + ", " + timestamp + '}'; } - /** - * The comparator is intentionally not overridden by this class, since log id and offset alone - * are meant to uniquely identify a mutation, and only offset determines the order within a log. - */ - public static final Comparator comparator = ShortMutationId.comparator::compare; - public ByteBuffer toByteBuffer() { return ByteBuffer.allocate(16) diff --git a/src/java/org/apache/cassandra/replication/MutationJournal.java b/src/java/org/apache/cassandra/replication/MutationJournal.java index 1fa4909804af..7d3dc91cf1ad 100644 --- a/src/java/org/apache/cassandra/replication/MutationJournal.java +++ b/src/java/org/apache/cassandra/replication/MutationJournal.java @@ -197,6 +197,17 @@ public CommitLogPosition getCurrentPosition() return journal.currentActiveSegment().currentPosition(); } + @Nullable + public static CommitLogPosition currentPositionOrNull() + { + MutationJournal mutationJournal = instance; + if (mutationJournal == null) + return null; + + ActiveSegment segment = mutationJournal.journal.currentActiveSegment(); + return segment == null ? null : segment.currentPosition(); + } + // If all Memtables associated with given segment were flushed by the time we have closed active segment // and opened it as static, the segment is eligible to be marked as not needing replay. The actual durable // recording of needsReplay=false is deferred — we record the segment in pendingClearReplay and let the diff --git a/src/java/org/apache/cassandra/replication/MutationTrackingService.java b/src/java/org/apache/cassandra/replication/MutationTrackingService.java index db87a88f010a..ea6347845781 100644 --- a/src/java/org/apache/cassandra/replication/MutationTrackingService.java +++ b/src/java/org/apache/cassandra/replication/MutationTrackingService.java @@ -91,6 +91,7 @@ import org.apache.cassandra.tcm.ownership.VersionedEndpoints; import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.MBeanWrapper; +import org.apache.cassandra.utils.NoSpamLogger; import static com.google.common.base.Preconditions.checkNotNull; import static java.lang.String.format; @@ -184,6 +185,7 @@ public static void shutdown() throws InterruptedException private static final int SHARD_MULTIPLIER = 1; private static final Logger logger = LoggerFactory.getLogger(MutationTrackingService.class); + private static final NoSpamLogger noSpamLogger = NoSpamLogger.getLogger(logger, 1, TimeUnit.MINUTES); private final TrackedLocalReads localReads = new TrackedLocalReads(); private ConcurrentHashMap keyspaceShards = new ConcurrentHashMap<>(); @@ -514,6 +516,42 @@ public boolean startWriting(Mutation mutation) } } + /** + * Register a mutation being replayed from the journal, tolerating a shard that is no longer present. + *

+ * Unlike {@link #startWriting}, this never creates shards. {@link KeyspaceShards#make} asserts the keyspace is + * still tracked or migrating, which is false once a keyspace has been altered back to untracked — and that flip + * is instant, so unflushed journal records are routinely replayed into exactly that state on restart. Creating + * shards for a keyspace that is no longer tracked would also resurrect tracking state the operator has just turned + * off. + *

+ * A replayed record was already registered with its shard when it was originally coordinated. If the shard is + * gone there is nothing left to reconcile it against, and the only thing that still has to happen is for the + * record to reach the memtable so the data is not lost. + * + * @return true if the record was registered with a shard, false if no shard covers it + */ + public boolean startWritingForReplay(Mutation mutation) + { + shardLock.readLock().lock(); + try + { + Preconditions.checkArgument(!mutation.id().isNone()); + KeyspaceShards shards = keyspaceShards.get(mutation.getKeyspaceName()); + if (shards == null) + { + noSpamLogger.info("Replaying journal record for {} with no shards; applying without registration", + mutation.getKeyspaceName()); + return false; + } + return shards.startWritingIfShardPresent(mutation); + } + finally + { + shardLock.readLock().unlock(); + } + } + public void finishWriting(Mutation mutation) { shardLock.readLock().lock(); @@ -1326,6 +1364,23 @@ boolean startWriting(Mutation mutation) return lookUp(mutation).startWriting(mutation); } + /** + * Register a replayed mutation, returning false rather than throwing when no shard covers its token. + * See {@link MutationTrackingService#startWritingForReplay}. + */ + boolean startWritingIfShardPresent(Mutation mutation) + { + VersionedEndpoints.ForRange forRange = groups.matchToken(mutation.key().getToken()); + Shard shard = forRange == null ? null : shards.get(forRange.range()); + if (shard == null) + { + noSpamLogger.info("Replaying journal record for {} with no shard covering token {}; applying without registration", + keyspace, mutation.key().getToken()); + return false; + } + return shard.startWriting(mutation); + } + void finishWriting(Mutation mutation) { lookUp(mutation).finishWriting(mutation); diff --git a/src/java/org/apache/cassandra/replication/ShortMutationId.java b/src/java/org/apache/cassandra/replication/ShortMutationId.java index 856790a8a1df..6bf464d24a32 100644 --- a/src/java/org/apache/cassandra/replication/ShortMutationId.java +++ b/src/java/org/apache/cassandra/replication/ShortMutationId.java @@ -19,7 +19,6 @@ import java.io.IOException; import java.io.Serializable; -import java.util.Comparator; import org.apache.cassandra.db.TypeSizes; import org.apache.cassandra.io.UnversionedSerializer; @@ -34,7 +33,7 @@ * MutationId without the timestamp component. This is sufficient for uniquely identifying a mutation, * and for lookup in the journal and most tracking data structures. */ -public class ShortMutationId implements Serializable +public class ShortMutationId implements Serializable, Comparable { static final int NONE_OFFSET = Integer.MIN_VALUE; @@ -76,7 +75,19 @@ private ShortMutationId(int hostId, int hostLogId, int offset) public ShortMutationId(MutationId mutationId) { - this(mutationId.hostLogId(), mutationId.hostId(), mutationId.offset()); + this(mutationId.hostId(), mutationId.hostLogId(), mutationId.offset()); + } + + @Override + public int compareTo(ShortMutationId that) + { + int cmp = Integer.compare(this.hostId, that.hostId); + if (cmp != 0) return cmp; + + cmp = Integer.compare(this.hostLogId, that.hostLogId); + if (cmp != 0) return cmp; + + return Integer.compare(this.offset, that.offset); } public int hostId() @@ -130,11 +141,6 @@ public String toString() return "ShortMutationId{" + hostId() + ", " + hostLogId() + ", " + offset() + '}'; } - public static final Comparator comparator = (l, r) -> { - int cmp = Long.compareUnsigned(l.logId(), r.logId()); - return cmp != 0 ? cmp : Integer.compare(l.offset, r.offset); - }; - public static final UnversionedSerializer serializer = new UnversionedSerializer<>() { @Override diff --git a/src/java/org/apache/cassandra/service/accord/AccordKeyspace.java b/src/java/org/apache/cassandra/service/accord/AccordKeyspace.java index 50d1e17d73c1..9adf5bc0213d 100644 --- a/src/java/org/apache/cassandra/service/accord/AccordKeyspace.java +++ b/src/java/org/apache/cassandra/service/accord/AccordKeyspace.java @@ -51,6 +51,7 @@ import org.apache.cassandra.db.DataRange; import org.apache.cassandra.db.DecoratedKey; import org.apache.cassandra.db.Keyspace; +import org.apache.cassandra.db.LogDomain; import org.apache.cassandra.db.PartitionPosition; import org.apache.cassandra.db.ReadExecutionController; import org.apache.cassandra.db.RegularAndStaticColumns; @@ -355,7 +356,8 @@ public static Runnable systemTableUpdater(int storeId, TokenKey key, CommandsFor ColumnFamilyStore cfs = AccordColumnFamilyStores.commandsForKey; try (OpOrder.Group group = Keyspace.writeOrder.start()) { - cfs.getCurrentMemtable().put(MutationId.fixme(), upd, UpdateTransaction.NO_OP, group, true); + cfs.getCurrentMemtable().put(MutationId.fixme(), upd, UpdateTransaction.NO_OP, group, + LogDomain.COMMIT_LOG, true); } }; } diff --git a/src/java/org/apache/cassandra/service/replication/migration/MigrationRouter.java b/src/java/org/apache/cassandra/service/replication/migration/MigrationRouter.java index c500755cd99c..1e0cdfe86893 100644 --- a/src/java/org/apache/cassandra/service/replication/migration/MigrationRouter.java +++ b/src/java/org/apache/cassandra/service/replication/migration/MigrationRouter.java @@ -376,7 +376,7 @@ public static boolean isFullyTracked(IMutation mutation) return getMutationRouting(mutation) == MutationRouting.TRACKED; } - private static void validateMutationReplication(IMutation mutation, MutationRouting expected) + private static void validateMutationReplication(IMutation mutation, MutationRouting expected, boolean isReplay) { switch (expected) { @@ -393,19 +393,22 @@ private static void validateMutationReplication(IMutation mutation, MutationRout } + if (isReplay) + return; + MutationRouting actual = getMutationRouting(mutation); if (expected != actual) throw new CoordinatorBehindException("Mutation replication mismatch: expected " + expected + ", actual " + actual); } - public static void validateTrackedMutation(IMutation mutation) + public static void validateTrackedMutation(IMutation mutation, boolean isReplay) { - validateMutationReplication(mutation, MutationRouting.TRACKED); + validateMutationReplication(mutation, MutationRouting.TRACKED, isReplay); } - public static void validateUntrackedMutation(IMutation mutation) + public static void validateUntrackedMutation(IMutation mutation, boolean isReplay) { - validateMutationReplication(mutation, MutationRouting.UNTRACKED); + validateMutationReplication(mutation, MutationRouting.UNTRACKED, isReplay); } /** diff --git a/src/java/org/apache/cassandra/service/replication/migration/MutationTrackingMigrationRepairResult.java b/src/java/org/apache/cassandra/service/replication/migration/MutationTrackingMigrationRepairResult.java index e0ec93438249..4ac951563a84 100644 --- a/src/java/org/apache/cassandra/service/replication/migration/MutationTrackingMigrationRepairResult.java +++ b/src/java/org/apache/cassandra/service/replication/migration/MutationTrackingMigrationRepairResult.java @@ -33,6 +33,10 @@ public class MutationTrackingMigrationRepairResult new MutationTrackingMigrationRepairResult(Epoch.EMPTY, false, "dead nodes were excluded from the repair"); private static final MutationTrackingMigrationRepairResult PREVIEW = new MutationTrackingMigrationRepairResult(Epoch.EMPTY, false, "the repair was a preview"); + private static final MutationTrackingMigrationRepairResult NOT_INCREMENTAL = + new MutationTrackingMigrationRepairResult(Epoch.EMPTY, false, + "the repair was not incremental, so it synced the pre-migration data " + + "without marking it repaired; migration requires incremental repair"); public final Epoch minEpoch; public final boolean eligible; @@ -48,10 +52,24 @@ private MutationTrackingMigrationRepairResult(Epoch minEpoch, boolean eligible, this.ineligibleReason = ineligibleReason; } - public static MutationTrackingMigrationRepairResult fromRepair(Epoch minEpoch, boolean deadNodesExcluded, boolean isPreview) + /** + * Only an incremental repair may advance migration. A full repair syncs the pre-migration data but leaves it + * unrepaired, and nothing can promote it afterwards: reconciliation has no ids to work from, and once the range is + * migrated incremental repair no longer anticompacts it. Incremental repair marks exactly the ranges it verified, + * which is why migration needs no separate promotion step at completion. + * + * A tracked keyspace with no migration in progress has its incremental flag cleared by + * {@link org.apache.cassandra.repair.RepairCoordinator}, but such a repair never reaches this check: the handler + * returns earlier because the keyspace is not migrating. + */ + public static MutationTrackingMigrationRepairResult fromRepair(Epoch minEpoch, + boolean deadNodesExcluded, + boolean isPreview, + boolean isIncremental) { if (deadNodesExcluded) return DEAD_NODES_EXCLUDED; if (isPreview) return PREVIEW; + if (!isIncremental) return NOT_INCREMENTAL; return new MutationTrackingMigrationRepairResult(minEpoch, true, null); } } diff --git a/test/distributed/org/apache/cassandra/distributed/test/MutationTrackingMigrationTest.java b/test/distributed/org/apache/cassandra/distributed/test/MutationTrackingMigrationTest.java index 0ffe1ede6e9e..8e51af4980b5 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/MutationTrackingMigrationTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/MutationTrackingMigrationTest.java @@ -28,10 +28,14 @@ import org.apache.cassandra.ServerTestUtils; import org.apache.cassandra.config.CassandraRelevantProperties; +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.Keyspace; import org.apache.cassandra.distributed.Cluster; import org.apache.cassandra.distributed.api.ConsistencyLevel; import org.apache.cassandra.distributed.api.Feature; import org.apache.cassandra.distributed.api.ICoordinator; +import org.apache.cassandra.io.sstable.SSTableProvenance; +import org.apache.cassandra.io.sstable.format.SSTableReader; import org.apache.cassandra.replication.MutationJournal; import org.apache.cassandra.schema.KeyspaceMetadata; import org.apache.cassandra.service.replication.migration.MutationTrackingMigrationState; @@ -43,6 +47,7 @@ import static java.lang.String.format; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertTrue; /** @@ -170,32 +175,165 @@ private void verifyKeyspaceState(String keyspace, ExpectedKeyspaceState expected } } + private static void assertNoMixedSSTable(String keyspace, String table) + { + for (int nodeId = 1; nodeId <= NUM_NODES; nodeId++) + { + SHARED_CLUSTER.get(nodeId).runOnInstance(() -> { + ColumnFamilyStore cfs = Keyspace.open(keyspace).getColumnFamilyStore(table); + for (SSTableReader sstable : cfs.getLiveSSTables()) + { + assertNotEquals("an unrepaired sstable carries both journal offsets and a commit log span, so it " + + "mixes two logs: " + sstable + ' ' + sstable.getSSTableMetadata().commitLogIntervals, + SSTableProvenance.BOTH, SSTableProvenance.of(sstable)); + } + }); + } + } + + /** + * Counts, on node 1, how many of a table's sstables came out of each log. + * + * @return the journal-derived count, then the commit-log-derived count + */ + private static int[] countByProvenance(String keyspace, String table) + { + return SHARED_CLUSTER.get(1).callOnInstance(() -> { + int journalDerived = 0; + int commitLogDerived = 0; + ColumnFamilyStore cfs = Keyspace.open(keyspace).getColumnFamilyStore(table); + for (SSTableReader sstable : cfs.getLiveSSTables()) + { + switch (SSTableProvenance.of(sstable)) + { + case MUTATION_JOURNAL: + journalDerived++; + break; + case COMMIT_LOG: + commitLogDerived++; + break; + default: + break; + } + } + return new int[]{ journalDerived, commitLogDerived }; + }); + } + + private static void flushEverywhere(String keyspace, String table) + { + for (int nodeId = 1; nodeId <= NUM_NODES; nodeId++) + SHARED_CLUSTER.get(nodeId).nodetoolResult("flush", keyspace, table).asserts().success(); + } + + private static void insert(String keyspace, String table, int from, int to, String tag) + { + for (int i = from; i < to; i++) + coordinator.execute(format("INSERT INTO %s.%s (pk, value) VALUES (%d, '%s_%d')", + keyspace, table, i, tag, i), + ConsistencyLevel.QUORUM); + } + + private static void createKeyspaceWithTable(String keyspace, String replicationType) + { + coordinator.execute(format("CREATE KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', " + + "'replication_factor': 3} AND replication_type='%s'", keyspace, replicationType), + ConsistencyLevel.ALL); + coordinator.execute(format("CREATE TABLE %s.%s (pk int PRIMARY KEY, value text)", keyspace, TEST_TABLE), + ConsistencyLevel.ALL); + waitForEpochOf(SHARED_CLUSTER, 1); + } + + private static void alterReplicationType(String keyspace, String replicationType) + { + coordinator.execute(format("ALTER KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', " + + "'replication_factor': 3} AND replication_type='%s'", keyspace, replicationType), + ConsistencyLevel.ALL); + waitForEpochOf(SHARED_CLUSTER, 1); + } + + /** + * Check sstable provenance info correctness across migration to and from tracked replication + */ @Test - public void testUntrackedToTrackedMigration() throws Exception + public void sstableProvenanceCorrectnessAcrossMigrationAndReversal() throws Exception { - String testKeyspace = "untracked_to_tracked_test"; + String testKeyspace = "migration_bounds_test"; - // untracked keyspace - coordinator.execute(format("CREATE KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='untracked'", - testKeyspace), - ConsistencyLevel.ALL); + createKeyspaceWithTable(testKeyspace, "untracked"); + // Untracked: everything is commit-log-derived. + insert(testKeyspace, TEST_TABLE, 0, 50, "untracked"); + flushEverywhere(testKeyspace, TEST_TABLE); + assertNoMixedSSTable(testKeyspace, TEST_TABLE); + assertEquals("an untracked table produces no journal-derived sstable", + 0, countByProvenance(testKeyspace, TEST_TABLE)[0]); - coordinator.execute(format("CREATE TABLE %s.%s (pk int PRIMARY KEY, value text)", testKeyspace, TEST_TABLE), - ConsistencyLevel.ALL); + alterReplicationType(testKeyspace, "tracked"); + verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.MIGRATING_TO_TRACKED); + insert(testKeyspace, TEST_TABLE, 50, 100, "migrating"); + flushEverywhere(testKeyspace, TEST_TABLE); + assertNoMixedSSTable(testKeyspace, TEST_TABLE); + + // Repair only the primary range, so some ranges complete and some stay pending. From here the two logs take + // writes for the same table. + SHARED_CLUSTER.get(1).nodetoolResult("repair", "-pr", testKeyspace, TEST_TABLE).asserts().success(); waitForEpochOf(SHARED_CLUSTER, 1); + verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.MIGRATING_TO_TRACKED); + + insert(testKeyspace, TEST_TABLE, 100, 200, "partial"); + flushEverywhere(testKeyspace, TEST_TABLE); + assertNoMixedSSTable(testKeyspace, TEST_TABLE); + int[] partial = countByProvenance(testKeyspace, TEST_TABLE); + assertTrue("a partially migrated table should have taken journal writes; counts were " + + partial[0] + " journal-derived, " + partial[1] + " commit-log-derived", + partial[0] > 0); + + // Repeated flush rounds while both logs are in use, which is the steady state a long migration sits in. + int rowsPerRound = 40; + int rounds = 10; + for (int round = 0; round < rounds; round++) + { + insert(testKeyspace, TEST_TABLE, 200 + round * rowsPerRound, 200 + (round + 1) * rowsPerRound, + "round" + round); + flushEverywhere(testKeyspace, TEST_TABLE); + assertNoMixedSSTable(testKeyspace, TEST_TABLE); + } + + int[] steadyState = countByProvenance(testKeyspace, TEST_TABLE); + assertTrue("the table should still hold sstables from both logs; counts were " + + steadyState[0] + " journal-derived, " + steadyState[1] + " commit-log-derived", + steadyState[0] > 0 && steadyState[1] > 0); + + // tracked -> untracked is instant, so routing goes back to the commit log with no pending window. + int afterRounds = 200 + rowsPerRound * rounds; + alterReplicationType(testKeyspace, "untracked"); + verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.UNTRACKED); + + insert(testKeyspace, TEST_TABLE, afterRounds, afterRounds + 50, "reversed"); + flushEverywhere(testKeyspace, TEST_TABLE); + assertNoMixedSSTable(testKeyspace, TEST_TABLE); + + Object[][] results = coordinator.execute(format("SELECT * FROM %s.%s", testKeyspace, TEST_TABLE), + ConsistencyLevel.QUORUM); + assertEquals("no write is lost across the migration, the steady state and the reversal", + afterRounds + 50, results.length); + } + + @Test + public void testUntrackedToTrackedMigration() throws Exception + { + String testKeyspace = "untracked_to_tracked_test"; + + // untracked keyspace + createKeyspaceWithTable(testKeyspace, "untracked"); verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.UNTRACKED); long journalEntriesBefore = countJournalEntries(); - for (int i = 0; i < 100; i++) - { - coordinator.execute(format("INSERT INTO %s.%s (pk, value) VALUES (%d, 'initial_%d')", - testKeyspace, TEST_TABLE, i, i), - ConsistencyLevel.QUORUM); - } + insert(testKeyspace, TEST_TABLE, 0, 100, "initial"); // no journal entries written while untracked long journalEntriesAfterUntracked = countJournalEntries(); @@ -206,22 +344,13 @@ public void testUntrackedToTrackedMigration() throws Exception assertEquals(100, initialResults.length); // start migration to tracked replication - coordinator.execute(format("ALTER KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='tracked'", - testKeyspace), - ConsistencyLevel.ALL); - - waitForEpochOf(SHARED_CLUSTER, 1); + alterReplicationType(testKeyspace, "tracked"); verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.MIGRATING_TO_TRACKED); long journalEntriesBeforeMigrationWrites = countJournalEntries(); - for (int i = 100; i < 200; i++) - { - coordinator.execute(format("INSERT INTO %s.%s (pk, value) VALUES (%d, 'migration_%d')", - testKeyspace, TEST_TABLE, i, i), - ConsistencyLevel.QUORUM); - } + insert(testKeyspace, TEST_TABLE, 100, 200, "migration"); // writes should be tracked during migration long journalEntriesAfterMigrationWrites = countJournalEntries(); @@ -252,12 +381,7 @@ public void testUntrackedToTrackedMigration() throws Exception long journalEntriesBeforeTracked = countJournalEntries(); - for (int i = 200; i < 210; i++) - { - coordinator.execute(format("INSERT INTO %s.%s (pk, value) VALUES (%d, 'tracked_%d')", - testKeyspace, TEST_TABLE, i, i), - ConsistencyLevel.QUORUM); - } + insert(testKeyspace, TEST_TABLE, 200, 210, "tracked"); // writes should also be tracked after migration long journalEntriesAfterTracked = countJournalEntries(); @@ -282,25 +406,13 @@ public void testTrackedToUntrackedMigration() throws Exception String testKeyspace = "tracked_to_untracked_test"; // tracked keyspace - coordinator.execute(format("CREATE KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='tracked'", - testKeyspace), - ConsistencyLevel.ALL); - - coordinator.execute(format("CREATE TABLE %s.%s (pk int PRIMARY KEY, value text)", testKeyspace, TEST_TABLE), - ConsistencyLevel.ALL); - - waitForEpochOf(SHARED_CLUSTER, 1); + createKeyspaceWithTable(testKeyspace, "tracked"); verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.TRACKED); long journalEntriesBefore = countJournalEntries(); - for (int i = 0; i < 100; i++) - { - coordinator.execute(format("INSERT INTO %s.%s (pk, value) VALUES (%d, 'initial_%d')", - testKeyspace, TEST_TABLE, i, i), - ConsistencyLevel.QUORUM); - } + insert(testKeyspace, TEST_TABLE, 0, 100, "initial"); // writes should be tracked before migration long journalEntriesAfterTracked = countJournalEntries(); @@ -310,24 +422,15 @@ public void testTrackedToUntrackedMigration() throws Exception ConsistencyLevel.QUORUM); assertEquals(100, initialResults.length); - // switch to untracked replication — tracked→untracked is instant, no migration needed - coordinator.execute(format("ALTER KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='untracked'", - testKeyspace), - ConsistencyLevel.ALL); + // switch to untracked replication - tracked→untracked is instant, no migration needed + alterReplicationType(testKeyspace, "untracked"); - waitForEpochOf(SHARED_CLUSTER, 1); - - // Should go directly to UNTRACKED — no migration state + // Should go directly to UNTRACKED - no migration state verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.UNTRACKED); long journalEntriesBeforeUntracked = countJournalEntries(); - for (int i = 100; i < 210; i++) - { - coordinator.execute(format("INSERT INTO %s.%s (pk, value) VALUES (%d, 'untracked_%d')", - testKeyspace, TEST_TABLE, i, i), - ConsistencyLevel.QUORUM); - } + insert(testKeyspace, TEST_TABLE, 100, 210, "untracked"); // writes should not be tracked after instant switch to untracked long journalEntriesAfterUntracked = countJournalEntries(); @@ -349,51 +452,26 @@ public void testMigrationReversal() throws Exception String testKeyspace = "migration_reversal_test"; // untracked keyspace - coordinator.execute(format("CREATE KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='untracked'", - testKeyspace), - ConsistencyLevel.ALL); - - coordinator.execute(format("CREATE TABLE %s.%s (pk int PRIMARY KEY, value text)", testKeyspace, TEST_TABLE), - ConsistencyLevel.ALL); + createKeyspaceWithTable(testKeyspace, "untracked"); - waitForEpochOf(SHARED_CLUSTER, 1); - - for (int i = 0; i < 50; i++) - { - coordinator.execute(format("INSERT INTO %s.%s (pk, value) VALUES (%d, 'initial_%d')", - testKeyspace, TEST_TABLE, i, i), - ConsistencyLevel.QUORUM); - } + insert(testKeyspace, TEST_TABLE, 0, 50, "initial"); // Start migration to tracked - coordinator.execute(format("ALTER KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='tracked'", - testKeyspace), - ConsistencyLevel.ALL); - - waitForEpochOf(SHARED_CLUSTER, 1); + alterReplicationType(testKeyspace, "tracked"); verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.MIGRATING_TO_TRACKED); - for (int i = 50; i < 100; i++) - { - coordinator.execute(format("INSERT INTO %s.%s (pk, value) VALUES (%d, 'migrating_%d')", - testKeyspace, TEST_TABLE, i, i), - ConsistencyLevel.QUORUM); - } + insert(testKeyspace, TEST_TABLE, 50, 100, "migrating"); // only repair the primary range so the migration isn't complete and we have something to reverse SHARED_CLUSTER.get(1).nodetoolResult("repair", "-pr", testKeyspace, TEST_TABLE).asserts().success(); waitForEpochOf(SHARED_CLUSTER, 1); verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.MIGRATING_TO_TRACKED); - // Reverse the migration by changing back to untracked — tracked→untracked is instant - coordinator.execute(format("ALTER KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='untracked'", - testKeyspace), - ConsistencyLevel.ALL); - - waitForEpochOf(SHARED_CLUSTER, 1); + // Reverse the migration by changing back to untracked - tracked→untracked is instant + alterReplicationType(testKeyspace, "untracked"); - // Should go directly to UNTRACKED — no migration state + // Should go directly to UNTRACKED - no migration state verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.UNTRACKED); Object[][] results = coordinator.execute(format("SELECT * FROM %s.%s", testKeyspace, TEST_TABLE), @@ -415,21 +493,10 @@ public void testTableAddedDuringMigrationThenReversed() throws Exception String newTable = "tbl2"; // untracked keyspace - coordinator.execute(format("CREATE KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='untracked'", - testKeyspace), - ConsistencyLevel.ALL); - - coordinator.execute(format("CREATE TABLE %s.%s (pk int PRIMARY KEY, value text)", testKeyspace, TEST_TABLE), - ConsistencyLevel.ALL); - - waitForEpochOf(SHARED_CLUSTER, 1); + createKeyspaceWithTable(testKeyspace, "untracked"); // Start migration to tracked - coordinator.execute(format("ALTER KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='tracked'", - testKeyspace), - ConsistencyLevel.ALL); - - waitForEpochOf(SHARED_CLUSTER, 1); + alterReplicationType(testKeyspace, "tracked"); verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.MIGRATING_TO_TRACKED); @@ -442,14 +509,10 @@ public void testTableAddedDuringMigrationThenReversed() throws Exception coordinator.execute(format("INSERT INTO %s.%s (pk, value) VALUES (1, 'new_table_data')", testKeyspace, newTable), ConsistencyLevel.QUORUM); - // Reverse the migration — tracked→untracked is instant - coordinator.execute(format("ALTER KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='untracked'", - testKeyspace), - ConsistencyLevel.ALL); + // Reverse the migration - tracked→untracked is instant + alterReplicationType(testKeyspace, "untracked"); - waitForEpochOf(SHARED_CLUSTER, 1); - - // Should go directly to UNTRACKED — no migration state + // Should go directly to UNTRACKED - no migration state verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.UNTRACKED); Object[][] results = coordinator.execute(format("SELECT value FROM %s.%s WHERE pk = 1", testKeyspace, newTable), @@ -488,11 +551,7 @@ public void testTableDroppedDuringMigration() throws Exception ConsistencyLevel.QUORUM); // Start migration to tracked - coordinator.execute(format("ALTER KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='tracked'", - testKeyspace), - ConsistencyLevel.ALL); - - waitForEpochOf(SHARED_CLUSTER, 1); + alterReplicationType(testKeyspace, "tracked"); verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.MIGRATING_TO_TRACKED); @@ -529,24 +588,13 @@ public void testKeyspaceDroppedDuringMigration() throws Exception String testKeyspace = "keyspace_dropped_test"; // untracked keyspace - coordinator.execute(format("CREATE KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='untracked'", - testKeyspace), - ConsistencyLevel.ALL); - - coordinator.execute(format("CREATE TABLE %s.%s (pk int PRIMARY KEY, value text)", testKeyspace, TEST_TABLE), - ConsistencyLevel.ALL); - - waitForEpochOf(SHARED_CLUSTER, 1); + createKeyspaceWithTable(testKeyspace, "untracked"); coordinator.execute(format("INSERT INTO %s.%s (pk, value) VALUES (1, 'test_data')", testKeyspace, TEST_TABLE), ConsistencyLevel.QUORUM); // Start migration to tracked - coordinator.execute(format("ALTER KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='tracked'", - testKeyspace), - ConsistencyLevel.ALL); - - waitForEpochOf(SHARED_CLUSTER, 1); + alterReplicationType(testKeyspace, "tracked"); verifyKeyspaceState(testKeyspace, ExpectedKeyspaceState.MIGRATING_TO_TRACKED); diff --git a/test/distributed/org/apache/cassandra/distributed/test/tracking/CommitLogReplayRoutingTest.java b/test/distributed/org/apache/cassandra/distributed/test/tracking/CommitLogReplayRoutingTest.java new file mode 100644 index 000000000000..0e35d71dfccb --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/test/tracking/CommitLogReplayRoutingTest.java @@ -0,0 +1,176 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.distributed.test.tracking; + +import java.io.IOException; +import java.util.Set; + +import org.junit.Test; + +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.Keyspace; +import org.apache.cassandra.distributed.Cluster; +import org.apache.cassandra.distributed.api.ConsistencyLevel; +import org.apache.cassandra.distributed.api.Feature; +import org.apache.cassandra.distributed.api.IInvokableInstance; +import org.apache.cassandra.distributed.shared.ClusterUtils; +import org.apache.cassandra.distributed.test.TestBaseImpl; +import org.apache.cassandra.io.sstable.SSTableProvenance; +import org.apache.cassandra.io.sstable.format.SSTableReader; +import org.apache.cassandra.service.StorageService; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + +/** + * Commit log replay must route onto the untracked apply path, not consult schema + */ +public class CommitLogReplayRoutingTest extends TestBaseImpl +{ + private static final String TABLE = "tbl"; + + /** + * An untracked write, left unflushed, whose keyspace is then migrated to tracked. + */ + @Test + public void replayAppliesUntrackedRecordAfterMigrationToTracked() throws IOException + { + assertRecordsReplayAcrossAlter("untracked", "tracked"); + } + + /** + * A tracked keyspace altered back to untracked with unflushed journal records. + */ + @Test + public void replayAppliesTrackedRecordAfterFlipToUntracked() throws IOException + { + assertRecordsReplayAcrossAlter("tracked", "untracked"); + } + + /** + * Writes ten records under one replication type, alters the keyspace to the other, and restarts. The records are in + * whichever log the first type routes to, and current metadata disagrees with them by the time replay runs. + */ + private void assertRecordsReplayAcrossAlter(String before, String after) throws IOException + { + // NATIVE_PROTOCOL so isNativeTransportRunning() is meaningful; without it the transport never starts. + try (Cluster cluster = init(builder().withNodes(1) + .withConfig(c -> c.with(Feature.NATIVE_PROTOCOL)) + .start())) + { + IInvokableInstance node = cluster.get(1); + + cluster.schemaChange("ALTER KEYSPACE " + KEYSPACE + " WITH replication_type='" + before + '\''); + cluster.schemaChange(withKeyspace("CREATE TABLE %s." + TABLE + " (k int PRIMARY KEY, v int)")); + + for (int k = 0; k < 10; k++) + cluster.coordinator(1).execute(withKeyspace("INSERT INTO %s." + TABLE + " (k, v) VALUES (?, ?)"), + ConsistencyLevel.ALL, k, k); + assertNoSSTables(node); + + cluster.schemaChange("ALTER KEYSPACE " + KEYSPACE + " WITH replication_type='" + after + '\''); + assertNoSSTables(node); + + restart(node); + + assertTrue("replay must not have stopped the native transport", + node.callOnInstance(() -> StorageService.instance.isNativeTransportRunning())); + + Object[][] rows = cluster.coordinator(1).execute(withKeyspace("SELECT k, v FROM %s." + TABLE), + ConsistencyLevel.ALL); + assertEquals("every record written as " + before + " should have been replayed", 10, rows.length); + } + } + + /** + * A token with unflushed writes in both logs replays from both. + */ + @Test + public void replayAppliesBothLogsForSameToken() throws IOException + { + try (Cluster cluster = init(builder().withNodes(1).start())) + { + IInvokableInstance node = cluster.get(1); + + cluster.schemaChange("ALTER KEYSPACE " + KEYSPACE + " WITH replication_type='untracked'"); + cluster.schemaChange(withKeyspace("CREATE TABLE %s." + TABLE + " (k int, c int, v int, PRIMARY KEY (k, c))")); + + // Untracked write for k=0 -> commit log. + cluster.coordinator(1).execute(withKeyspace("INSERT INTO %s." + TABLE + " (k, c, v) VALUES (0, 0, 0)"), + ConsistencyLevel.ALL); + assertNoSSTables(node); + + // Migrating the keyspace routes subsequent writes to the journal. + cluster.schemaChange("ALTER KEYSPACE " + KEYSPACE + " WITH replication_type='tracked'"); + + // Tracked write for the same partition -> mutation journal. + cluster.coordinator(1).execute(withKeyspace("INSERT INTO %s." + TABLE + " (k, c, v) VALUES (0, 1, 1)"), + ConsistencyLevel.ALL); + assertNoSSTables(node); + + restart(node); + + Object[][] rows = cluster.coordinator(1).execute(withKeyspace("SELECT c, v FROM %s." + TABLE + " WHERE k = 0"), + ConsistencyLevel.ALL); + assertEquals("both logs should have replayed for the same token", 2, rows.length); + + assertReplayLandsInSeparateSSTables(node); + } + } + + private static void assertReplayLandsInSeparateSSTables(IInvokableInstance node) + { + node.runOnInstance(() -> { + ColumnFamilyStore cfs = Keyspace.open(KEYSPACE).getColumnFamilyStore(TABLE); + cfs.forceBlockingFlush(ColumnFamilyStore.FlushReason.UNIT_TESTS); + + Set sstables = cfs.getLiveSSTables(); + assertEquals("one sstable per domain: " + sstables, 2, sstables.size()); + + int withSpan = 0; + for (SSTableReader sstable : sstables) + { + SSTableProvenance provenance = SSTableProvenance.of(sstable); + assertNotEquals("an sstable claims both logs: " + sstable, SSTableProvenance.BOTH, provenance); + if (provenance == SSTableProvenance.COMMIT_LOG) + withSpan++; + } + assertEquals("exactly one sstable claims a commit log span, and the journal-derived one claims none: " + + sstables, 1, withSpan); + }); + } + + private static void restart(IInvokableInstance node) + { + ClusterUtils.stopUnchecked(node); + node.startup(); + } + + private static void assertNoSSTables(IInvokableInstance node) + { + node.runOnInstance(() -> { + ColumnFamilyStore cfs = Keyspace.open(KEYSPACE).getColumnFamilyStore(TABLE); + // The records must still be in the log only, or the restart replays nothing and the test proves nothing. + assertTrue("already flushed: " + cfs.getLiveSSTables(), cfs.getLiveSSTables().isEmpty()); + assertFalse(cfs.getTracker().getView().getCurrentMemtable().isClean()); + }); + } +} diff --git a/test/distributed/org/apache/cassandra/distributed/test/tracking/TrackedImportFailureTest.java b/test/distributed/org/apache/cassandra/distributed/test/tracking/TrackedImportFailureTest.java index ee897a31886a..19fc8f9f0f70 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/tracking/TrackedImportFailureTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/tracking/TrackedImportFailureTest.java @@ -21,7 +21,6 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.util.Collections; -import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -34,23 +33,18 @@ import org.slf4j.LoggerFactory; import org.apache.cassandra.db.ColumnFamilyStore; -import org.apache.cassandra.db.marshal.Int32Type; -import org.apache.cassandra.dht.Murmur3Partitioner; import org.apache.cassandra.distributed.Cluster; import org.apache.cassandra.distributed.api.ConsistencyLevel; import org.apache.cassandra.distributed.api.ICoordinator; import org.apache.cassandra.distributed.api.IInvokableInstance; import org.apache.cassandra.distributed.api.IIsolatedExecutor; import org.apache.cassandra.distributed.shared.AssertUtils; -import org.apache.cassandra.distributed.shared.ClusterUtils; import org.apache.cassandra.distributed.shared.Uninterruptibles; import org.apache.cassandra.exceptions.ReadTimeoutException; import org.apache.cassandra.io.UnversionedSerializer; import org.apache.cassandra.io.sstable.format.SSTableReader; import org.apache.cassandra.io.util.File; -import org.apache.cassandra.replication.ActivatedTransfers; import org.apache.cassandra.replication.ActivationRequest; -import org.apache.cassandra.replication.ShortMutationId; import org.apache.cassandra.tcm.ClusterMetadata; import org.apache.cassandra.tcm.ownership.DataPlacement; @@ -206,104 +200,74 @@ public void importMissingOnDataReplicaDuringAugment() throws Throwable } } - /* - * Ensure that activation IDs attached to SSTables aren't spread across Token boundaries by compaction. - * - * For example: - * IMPORT_TOKEN is owned by replicas (A, B) - * OUTSIDE_IMPORT_TOKEN is owned by replicas (B, C) - * Execute import so (A, B) have IMPORT_TOKEN - * Execute plain write so (B, C) have OUTSIDE_IMPORT_TOKEN - * Do a major compaction on B so IMPORT_TOKEN and OUTSIDE_IMPORT_TOKEN are compacted together into the same SSTable - * Execute a data read for OUTSIDE_IMPORT_TOKEN against B, ensure it doesn't contain any activation IDs - */ @Test - public void importActivationMergedByCompaction() throws Throwable + public void majorCompactionPromotesReconciledImportActivation() throws Throwable { try (Cluster cluster = cluster((cl, tg, instance, gen) -> ByteBuddyInjections.SkipPurgeTransfers.install().initialise(cl, tg, instance, gen))) { createSchema(cluster, 2); - Set inImportRange = new HashSet<>(); - cluster.forEach(instance -> { - logger.debug("Instance {} ring is {}", ClusterUtils.instanceId(instance), ClusterUtils.ring(instance)); - boolean isInRange = instance.callOnInstance(() -> { + // The import needs a replica of IMPORT_TOKEN to run on. Any of them will do. + IInvokableInstance importReplica = null; + for (IInvokableInstance instance : cluster) + { + boolean isReplica = instance.callOnInstance(() -> { ColumnFamilyStore cfs = ColumnFamilyStore.getIfExists(KEYSPACE, TABLE); DataPlacement placement = ClusterMetadata.current().placements.get(cfs.keyspace.getMetadata().params.replication); return placement.writes.forToken(IMPORT_TOKEN).get().containsSelf(); }); - if (isInRange) - inImportRange.add(instance); - }); - Assertions.assertThat(inImportRange).hasSize(2); + if (isReplica) + { + importReplica = instance; + break; + } + } + Assertions.assertThat(importReplica).isNotNull(); - // Find a partition key that's not owned by the same replicas as the import - Murmur3Partitioner.LongToken NON_IMPORT_TOKEN = new Murmur3Partitioner.LongToken(IMPORT_TOKEN.getLongValue() * 3); - int NON_IMPORT_PK = Int32Type.instance.compose(Murmur3Partitioner.LongToken.keyForToken(NON_IMPORT_TOKEN)); + doImport(cluster, importReplica); + assertLocalSelect(Collections.singleton(importReplica), + (IIsolatedExecutor.SerializableConsumer) rows -> { + assertRows(rows, row(IMPORT_PK, IMPORT_PK)); + }); - Set inNonImportRange = new HashSet<>(); - cluster.forEach(instance -> { - boolean isInRange = instance.callOnInstance(() -> { - ColumnFamilyStore cfs = ColumnFamilyStore.getIfExists(KEYSPACE, TABLE); - DataPlacement placement = ClusterMetadata.current().placements.get(cfs.keyspace.getMetadata().params.replication); - return placement.writes.forToken(NON_IMPORT_TOKEN).get().containsSelf(); - }); - if (isInRange) - inNonImportRange.add(instance); + // The import has to have left an activation behind, or the promotion asserted below proves nothing. + importReplica.runOnInstance(() -> { + ColumnFamilyStore cfs = ColumnFamilyStore.getIfExists(KEYSPACE, TABLE); + Assertions.assertThat(cfs.getLiveSSTables()) + .describedAs("the import should have left an activated transfer") + .anyMatch(sstable -> !sstable.getCoordinatorLogOffsets().transfers().isEmpty()); }); - Assertions.assertThat(inNonImportRange).hasSize(2); - Assertions.assertThat(inNonImportRange).isNotEqualTo(inImportRange); - - // Import: (A, B) - // Plain: (B, C) - IInvokableInstance A = null; - IInvokableInstance B = null; - IInvokableInstance C = null; - for (IInvokableInstance instance : cluster) - { - boolean isImport = inImportRange.contains(instance); - boolean isNonImport = inNonImportRange.contains(instance); - if (isImport && isNonImport) - B = instance; - else if (isImport) - A = instance; - else if (isNonImport) - C = instance; - } - Assertions.assertThat(A).isNotNull(); - Assertions.assertThat(B).isNotNull(); - Assertions.assertThat(C).isNotNull(); - doImport(cluster, A); - assertLocalSelect(List.of(A, B), (IIsolatedExecutor.SerializableConsumer) rows -> { - assertRows(rows, row(IMPORT_PK, IMPORT_PK)); - }); + // Any second partition will do now that replica placement does not matter. At ALL, so every node holds an + // sstable with no transfer for the compaction to find beside the import's. + int otherPk = IMPORT_PK + 1; + cluster.coordinator(1).execute(withKeyspace("INSERT INTO %s." + TABLE + "(k, v) VALUES (?, ?)"), + ConsistencyLevel.ALL, otherPk, otherPk); + assertCompaction(cluster, Collections.singleton(importReplica), NOOP, NOOP); - ShortMutationId importTransferId = callSerialized(A, () -> ShortMutationId.serializer, () -> { + importReplica.runOnInstance(() -> { ColumnFamilyStore cfs = ColumnFamilyStore.getIfExists(KEYSPACE, TABLE); + int repaired = 0; for (SSTableReader sstable : cfs.getLiveSSTables()) { - ActivatedTransfers transfers = sstable.getCoordinatorLogOffsets().transfers(); - if (!transfers.isEmpty()) - return transfers.iterator().next(); + Assertions.assertThat(sstable.getCoordinatorLogOffsets().transfers()) + .describedAs("promotion should have cleared the offsets of %s", sstable) + .isEmpty(); + if (sstable.isRepaired()) + repaired++; } - return null; + Assertions.assertThat(repaired) + .describedAs("promotion should have taken the transfer and nothing else") + .isEqualTo(1); }); - Assertions.assertThat(importTransferId).isNotNull(); - C.coordinator().execute(withKeyspace("INSERT INTO %s." + TABLE + "(k, v) VALUES (?, ?)"), ConsistencyLevel.ALL, NON_IMPORT_PK, NON_IMPORT_PK); - assertCompaction(cluster, Collections.singleton(B), NOOP, NOOP); - - // Reading from B for a range that doesn't include the import shouldn't include any transfer IDs, even though they've been compacted together - long mark = B.logs().mark(); - Object[][] rows = B.coordinator().execute(withKeyspace("SELECT * FROM %s." + TABLE + " WHERE k = ?"), ConsistencyLevel.ALL, NON_IMPORT_PK); - assertRows(rows, row(NON_IMPORT_PK, NON_IMPORT_PK)); - Assertions.assertThat(B.logs().grep(mark, "Found overlapping activation ID ").getResult()).isEmpty(); - - // But if the read range does include a transfer ID, it should have been added - mark = B.logs().mark(); - rows = B.coordinator().execute(withKeyspace("SELECT * FROM %s." + TABLE + " WHERE k = ?"), ConsistencyLevel.ALL, IMPORT_PK); - assertRows(rows, row(IMPORT_PK, IMPORT_PK)); - Assertions.assertThat(B.logs().grep(mark, "Found overlapping activation ID ").getResult()).isNotEmpty(); + + // Both partitions survive the import and the compaction. + for (int pk : new int[]{ otherPk, IMPORT_PK }) + { + Object[][] rows = cluster.coordinator(1).execute(withKeyspace("SELECT * FROM %s." + TABLE + " WHERE k = ?"), + ConsistencyLevel.ALL, pk); + assertRows(rows, row(pk, pk)); + } } } diff --git a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java index f7921785c644..46bbff5b567f 100644 --- a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java +++ b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java @@ -31,7 +31,6 @@ import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; import java.util.function.Supplier; @@ -57,6 +56,7 @@ import org.apache.cassandra.db.lifecycle.LifecycleTransaction; import org.apache.cassandra.db.lifecycle.SSTableSet; import org.apache.cassandra.db.memtable.AbstractMemtable; +import org.apache.cassandra.db.memtable.LogDomainBounds; import org.apache.cassandra.db.memtable.Memtable; import org.apache.cassandra.db.partitions.FilteredPartition; import org.apache.cassandra.db.partitions.Partition; @@ -847,7 +847,7 @@ private Memtable fakeMemTableWithMinTS(ColumnFamilyStore cfs, long minTS) { @Override - public long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, Group opGroup, boolean assumeMissing) + public long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, Group opGroup, LogDomain domain, boolean assumeMissing) { return 0; } @@ -886,7 +886,7 @@ public FlushablePartitionSet getFlushSet(PartitionPosition from, PartitionPos } @Override - public void switchOut(Barrier writeBarrier, AtomicReference commitLogUpperBound) + public void switchOut(Barrier writeBarrier, LogDomainBounds upperBounds) { } @@ -896,11 +896,23 @@ public void discard() } @Override - public boolean accepts(Group opGroup, CommitLogPosition commitLogPosition) + public boolean accepts(Group opGroup, CommitLogPosition commitLogPosition, LogDomain domain) { return false; } + @Override + public boolean holds(LogDomain domain) + { + return domain == LogDomain.COMMIT_LOG; + } + + @Override + public Owner owner() + { + return cfs; + } + @Override public CommitLogPosition getApproximateCommitLogLowerBound() { diff --git a/test/unit/org/apache/cassandra/db/CoordinatorLogOffsetsLifecycleTest.java b/test/unit/org/apache/cassandra/db/CoordinatorLogOffsetsLifecycleTest.java index 05acb3d87066..a586bbc10ec9 100644 --- a/test/unit/org/apache/cassandra/db/CoordinatorLogOffsetsLifecycleTest.java +++ b/test/unit/org/apache/cassandra/db/CoordinatorLogOffsetsLifecycleTest.java @@ -20,12 +20,10 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.Comparator; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; import org.assertj.core.api.Assertions; import org.junit.Assert; @@ -38,6 +36,7 @@ import org.apache.cassandra.SchemaLoader; import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.db.compaction.CompactionGroup; import org.apache.cassandra.db.lifecycle.View; import org.apache.cassandra.db.marshal.Int32Type; import org.apache.cassandra.db.memtable.Memtable; @@ -195,18 +194,17 @@ public void mutationIdLifecycleTest() assertNumSSTables(view, 1); SSTableReader sstable = Iterables.getOnlyElement(view.liveSSTables()); - ImmutableCoordinatorLogOffsets logOffsets = sstable.getCoordinatorLogOffsets(); - Assertions.assertThat(logOffsets.mutations().size()).isEqualTo(1); - Assertions.assertThat(logOffsets.mutations().offsets(id2.logId()).contains(id2.offset())).isTrue(); - // Single-participant, so mutations are immediately reconciled once applied + // Single-participant, so mutations are immediately reconciled once applied, and a reconciled flush is + // promoted as it is written. Assertions.assertThat(sstable.isRepaired()).isTrue(); + Assertions.assertThat(sstable.getCoordinatorLogOffsets().isEmpty()).isTrue(); + Assertions.assertThat(CompactionGroup.of(sstable)).isEqualTo(CompactionGroup.REPAIRED); } - MutationId id3; MutationId id4; // apply 2 { - id3 = applyMutation(cfs.metadata(), 3, 3); + applyMutation(cfs.metadata(), 3, 3); id4 = applyMutation(cfs.metadata(), 4, 4); View view = cfs.getTracker().getView(); @@ -228,21 +226,11 @@ public void mutationIdLifecycleTest() assertEmptyMemtable(view); assertNumSSTables(view, 2); - List sstables = Lists.newArrayList(view.liveSSTables()); - sstables.sort(Comparator.comparing(sst -> sst.descriptor.id.asBytes())); + for (SSTableReader sstable : view.liveSSTables()) { - ImmutableCoordinatorLogOffsets logOffsets = sstables.get(0).getCoordinatorLogOffsets(); - Assertions.assertThat(logOffsets.mutations().size()).isEqualTo(1); - Assertions.assertThat(logOffsets.mutations().offsets(id2.logId()).contains(id2.offset())).isTrue(); - } - { - - ImmutableCoordinatorLogOffsets logOffsets = sstables.get(1).getCoordinatorLogOffsets(); - Assertions.assertThat(logOffsets.mutations().size()).isEqualTo(1); - Assertions.assertThat(logOffsets.mutations().offsets(id4.logId()).contains(id4.offset())).isTrue(); - } - for (SSTableReader sstable : sstables) Assertions.assertThat(sstable.isRepaired()).isTrue(); + Assertions.assertThat(sstable.getCoordinatorLogOffsets().isEmpty()).isTrue(); + } } // compaction @@ -254,10 +242,9 @@ public void mutationIdLifecycleTest() assertNumSSTables(view, 1); SSTableReader sstable = Iterables.getOnlyElement(view.liveSSTables()); - ImmutableCoordinatorLogOffsets logOffsets = sstable.getCoordinatorLogOffsets(); - Assertions.assertThat(logOffsets.mutations().size()).isEqualTo(1); - Assertions.assertThat(logOffsets.mutations().offsets(id4.logId()).contains(id4.offset())).isTrue(); + // Compacting repaired inputs yields a repaired output, still with no offsets to carry forward. Assertions.assertThat(sstable.isRepaired()).isTrue(); + Assertions.assertThat(sstable.getCoordinatorLogOffsets().isEmpty()).isTrue(); } } } diff --git a/test/unit/org/apache/cassandra/db/compaction/CompactionGroupTest.java b/test/unit/org/apache/cassandra/db/compaction/CompactionGroupTest.java new file mode 100644 index 000000000000..b7d3d10dd7b6 --- /dev/null +++ b/test/unit/org/apache/cassandra/db/compaction/CompactionGroupTest.java @@ -0,0 +1,105 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.compaction; + +import org.junit.Test; + +import org.apache.cassandra.replication.ImmutableCoordinatorLogOffsets; +import org.apache.cassandra.replication.MutationId; +import org.apache.cassandra.service.ActiveRepairService; +import org.apache.cassandra.utils.TimeUUID; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +public class CompactionGroupTest +{ + private static final long REPAIRED_AT = 1234L; + private static final long UNREPAIRED = ActiveRepairService.UNREPAIRED_SSTABLE; + private static final TimeUUID NO_PENDING = ActiveRepairService.NO_PENDING_REPAIR; + + private static ImmutableCoordinatorLogOffsets offsets(long logId, int offset) + { + return new ImmutableCoordinatorLogOffsets.Builder().add(new MutationId(logId, (long) offset)).build(); + } + + private static ImmutableCoordinatorLogOffsets noOffsets() + { + return new ImmutableCoordinatorLogOffsets.Builder().build(); + } + + /** + * Repaired wins over offsets. Promotion clears the offsets in the same metadata mutation that sets {@code repairedAt}, + * so this should be unreachable, but we still don't want repaired and unreconciled data being compacted if that's not + * working properly + */ + @Test + public void repairedOutranksOffsets() + { + assertEquals(CompactionGroup.REPAIRED, CompactionGroup.of(REPAIRED_AT, NO_PENDING, noOffsets())); + assertEquals(CompactionGroup.REPAIRED, CompactionGroup.of(REPAIRED_AT, NO_PENDING, offsets(1, 0))); + } + + /** + * Pending repair outranks offsets too, so an sstable in a session is not routed to a tracked silo. + */ + @Test + public void pendingRepairOutranksOffsets() + { + TimeUUID session = TimeUUID.Generator.nextTimeUUID(); + assertEquals(CompactionGroup.PENDING_REPAIR, CompactionGroup.of(UNREPAIRED, session, noOffsets())); + assertEquals(CompactionGroup.PENDING_REPAIR, CompactionGroup.of(UNREPAIRED, session, offsets(1, 0))); + } + + /** + * Any offsets at all mean tracked data awaiting reconciliation. Transfers and mutations are not told apart here. + */ + @Test + public void offsetsOnUnrepairedDataMeanUnreconciled() + { + assertEquals(CompactionGroup.UNRECONCILED, CompactionGroup.of(UNREPAIRED, NO_PENDING, offsets(1, 0))); + } + + /** + * No offsets on unrepaired data is untracked. A null offsets object reads the same way, which streaming relies on. + */ + @Test + public void noOffsetsOnUnrepairedDataMeansUntracked() + { + assertEquals(CompactionGroup.UNREPAIRED, CompactionGroup.of(UNREPAIRED, NO_PENDING, noOffsets())); + assertEquals(CompactionGroup.UNREPAIRED, CompactionGroup.of(UNREPAIRED, NO_PENDING, null)); + } + + /** + * repairedAt and pendingRepair can't both be set + */ + @Test + public void repairedAndPendingIsRejected() + { + try + { + CompactionGroup.of(REPAIRED_AT, TimeUUID.Generator.nextTimeUUID(), null); + fail("Expected IllegalArgumentException"); + } + catch (IllegalArgumentException expected) + { + // expected + } + } +} diff --git a/test/unit/org/apache/cassandra/db/compaction/CompactionStrategyManagerTest.java b/test/unit/org/apache/cassandra/db/compaction/CompactionStrategyManagerTest.java index 74f27c524d32..5a63ffaa9e69 100644 --- a/test/unit/org/apache/cassandra/db/compaction/CompactionStrategyManagerTest.java +++ b/test/unit/org/apache/cassandra/db/compaction/CompactionStrategyManagerTest.java @@ -22,7 +22,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.EnumMap; +import java.util.EnumSet; import java.util.List; +import java.util.Map; import java.util.Random; import java.util.Set; import java.util.concurrent.CountDownLatch; @@ -73,7 +76,6 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; /** * We use byte ordered partitioner in this test to be able to easily infer an SSTable @@ -275,56 +277,42 @@ public void testAutomaticUpgradeConcurrency2() throws Exception DatabaseDescriptor.setAutomaticSSTableUpgradeEnabled(false); } - private static void assertHolderExclusivity(boolean isRepaired, boolean isPendingRepair, Class expectedType) + @Test + public void testMutualExclusiveHolderClassification() throws Exception { + Map> expected = new EnumMap<>(CompactionGroup.class); + expected.put(CompactionGroup.UNREPAIRED, CompactionStrategyHolder.class); + expected.put(CompactionGroup.REPAIRED, CompactionStrategyHolder.class); + expected.put(CompactionGroup.PENDING_REPAIR, PendingRepairHolder.class); + expected.put(CompactionGroup.UNRECONCILED, TrackedCompactionManager.class); + + assertEquals("a new compaction group needs a holder named here before it can be routed", + EnumSet.allOf(CompactionGroup.class), expected.keySet()); + ColumnFamilyStore cfs = Keyspace.open(KS_PREFIX).getColumnFamilyStore(TABLE_PREFIX); CompactionStrategyManager csm = cfs.getCompactionStrategyManager(); - AbstractStrategyHolder holder = csm.getHolder(isRepaired, isPendingRepair); - assertNotNull(holder); - assertSame(expectedType, holder.getClass()); - - int matches = 0; - for (AbstractStrategyHolder other : csm.getHolders()) + for (CompactionGroup group : CompactionGroup.values()) { - if (other.managesRepairedGroup(isRepaired, isPendingRepair)) + AbstractStrategyHolder holder = csm.getHolder(group); + assertNotNull("no holder claims " + group, holder); + + assertSame(expected.get(group), holder.getClass()); + + int matches = 0; + for (AbstractStrategyHolder other : csm.getHolders()) { - assertSame("holder assignment should be mutually exclusive", holder, other); - matches++; + if (other.managesGroup(group)) + { + assertSame("holder assignment should be mutually exclusive", holder, other); + matches++; + } } - } - assertEquals(1, matches); - } + assertEquals(1, matches); - private static void assertInvalidHolderConfig(boolean isRepaired, boolean isPendingRepair) - { - ColumnFamilyStore cfs = Keyspace.open(KS_PREFIX).getColumnFamilyStore(TABLE_PREFIX); - CompactionStrategyManager csm = cfs.getCompactionStrategyManager(); - try - { - csm.getHolder(isRepaired, isPendingRepair); - fail("Expected IllegalArgumentException"); - } - catch (IllegalArgumentException e) - { - // expected } } - /** - * If an sstable can be be assigned to a strategy holder, it shouldn't be possibly to - * assign it to any of the other holders. - */ - @Test - public void testMutualExclusiveHolderClassification() throws Exception - { - assertHolderExclusivity(false, false, CompactionStrategyHolder.class); - assertHolderExclusivity(true, false, CompactionStrategyHolder.class); - assertHolderExclusivity(false, true, PendingRepairHolder.class); - assertHolderExclusivity(false, true, PendingRepairHolder.class); - assertInvalidHolderConfig(true, true); - } - PartitionPosition forKey(int key) { DecoratedKey dk = Util.dk(String.format("%04d", key)); @@ -364,13 +352,20 @@ public void groupSSTables() throws Exception List grouped = csm.groupSSTables(Iterables.concat( pendingRepair, repaired, unrepaired)); + int placed = 0; for (int x=0; x sstables(int count) + { + Set sstables = new HashSet<>(); + for (int i = 0; i < count; i++) + sstables.add(makeSSTable(true)); + return sstables; + } + + private AbstractCompactionTask tryPromote(Set candidates) + { + return PromoteReconciledTask.tryPromote(cfs, candidates, "test", null); + } + + @Test + public void noCandidatesYieldsNoTask() + { + assertNull(tryPromote(Collections.emptySet())); + } + + @Test + public void allCandidatesBusyYieldsNoTask() + { + Set candidates = sstables(2); + AtomicInteger hookRuns = new AtomicInteger(); + + try (LifecycleTransaction held = cfs.getTracker().tryModify(candidates, OperationType.COMPACTION)) + { + assertNotNull(held); + assertNull("nothing is claimable, so there is nothing to promote", + PromoteReconciledTask.tryPromote(cfs, candidates, "test", hookRuns::incrementAndGet)); + } + assertEquals("no task was produced, so nothing should have been signalled", 0, hookRuns.get()); + } + + @Test + public void allCandidatesFreeClaimsAllOfThem() + { + Set candidates = sstables(3); + + AbstractCompactionTask task = tryPromote(candidates); + assertNotNull(task); + try + { + assertEquals(candidates, task.transaction.originals()); + } + finally + { + task.transaction.abort(); + } + } + + /** + * One busy sstable shouldn't block compaction on the others. The task should try to aquire the remaining tables + * that aren't referenced + */ + @Test + public void busyCandidateDoesntBlockRemainder() + { + Set candidates = sstables(3); + SSTableReader busy = candidates.iterator().next(); + + try (LifecycleTransaction held = cfs.getTracker().tryModify(Collections.singleton(busy), + OperationType.COMPACTION)) + { + assertNotNull(held); + + AbstractCompactionTask task = tryPromote(candidates); + assertNotNull("the free candidates should still have been promoted", task); + try + { + Set claimed = task.transaction.originals(); + // The busy one is left out and every other candidate is claimed. + assertEquals(candidates.size() - 1, claimed.size()); + assertFalse(claimed.contains(busy)); + Set rest = new HashSet<>(candidates); + rest.remove(busy); + assertTrue(claimed.containsAll(rest)); + } + finally + { + task.transaction.abort(); + } + } + } +} diff --git a/test/unit/org/apache/cassandra/db/compaction/TrackedCompactionManagerTest.java b/test/unit/org/apache/cassandra/db/compaction/TrackedCompactionManagerTest.java new file mode 100644 index 000000000000..7f567479d9e8 --- /dev/null +++ b/test/unit/org/apache/cassandra/db/compaction/TrackedCompactionManagerTest.java @@ -0,0 +1,348 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.compaction; + +import java.io.IOException; +import java.util.Collection; +import java.util.Collections; +import java.util.Set; +import java.util.stream.Collectors; + +import com.google.common.collect.ImmutableSet; + +import org.junit.Test; + +import org.apache.cassandra.db.compaction.AbstractStrategyHolder.DestinationRouter; +import org.apache.cassandra.db.compaction.AbstractStrategyHolder.GroupedSSTableContainer; +import org.apache.cassandra.db.compaction.AbstractStrategyHolder.TaskSupplier; +import org.apache.cassandra.dht.Bounds; +import org.apache.cassandra.dht.Token; +import org.apache.cassandra.io.sstable.Descriptor; +import org.apache.cassandra.io.sstable.format.SSTableReader; +import org.apache.cassandra.replication.ImmutableCoordinatorLogOffsets; +import org.apache.cassandra.replication.MutationId; +import org.apache.cassandra.replication.ShortMutationId; +import org.apache.cassandra.schema.CompactionParams; +import org.apache.cassandra.utils.FBUtilities; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNotSame; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +public class TrackedCompactionManagerTest extends AbstractPendingRepairTest +{ + private static ShortMutationId id(long logId, int offset) + { + return new ShortMutationId(logId, offset); + } + + private static ImmutableSet key(ShortMutationId... ids) + { + return ImmutableSet.copyOf(ids); + } + + static final DestinationRouter SINGLE_PARTITION = new DestinationRouter() + { + public int getIndexForSSTable(SSTableReader sstable) { return 0; } + public int getIndexForSSTableDirectory(Descriptor descriptor) { return 0; } + }; + + private TrackedCompactionManager manager() + { + TrackedCompactionManager manager = new TrackedCompactionManager(cfs, SINGLE_PARTITION); + manager.setStrategy(CompactionParams.DEFAULT, 1); + return manager; + } + + private static GroupedSSTableContainer group(TrackedCompactionManager manager, Iterable sstables) + { + GroupedSSTableContainer container = manager.createGroupedSSTableContainer(); + sstables.forEach(container::add); + return container; + } + + private static Collection nextBackgroundTasks(TrackedCompactionManager manager) + { + for (TaskSupplier supplier : manager.getBackgroundTaskSuppliers(FBUtilities.nowInSeconds())) + { + Collection tasks = supplier.getTasks(); + if (tasks != null && !tasks.isEmpty()) + return tasks; + } + return Collections.emptyList(); + } + + /** + * Silo keys other than {@link TrackedCompactionManager#NONE}, which is always present. + * */ + private static Set> transferKeys(TrackedCompactionManager manager) + { + return manager.keys().stream().filter(k -> !k.isEmpty()).collect(Collectors.toSet()); + } + + private static void attachTransfer(SSTableReader sstable, ShortMutationId... ids) throws IOException + { + ImmutableCoordinatorLogOffsets.Builder builder = new ImmutableCoordinatorLogOffsets.Builder(); + for (ShortMutationId id : ids) + { + Bounds bounds = new Bounds<>(sstable.getFirst().getToken(), sstable.getLast().getToken()); + builder.addTransfer(id, bounds); + } + sstable.mutateCoordinatorLogOffsetsAndReload(builder.build()); + } + + private SSTableReader sstableWithTransfers(ShortMutationId... ids) throws IOException + { + SSTableReader sstable = makeSSTable(true); + attachTransfer(sstable, ids); + return sstable; + } + + private SSTableReader sstableWithMutations(long logId, int offset) throws IOException + { + SSTableReader sstable = makeSSTable(true); + ImmutableCoordinatorLogOffsets offsets = new ImmutableCoordinatorLogOffsets.Builder() + .add(new MutationId(logId, (long) offset)) + .build(); + sstable.mutateCoordinatorLogOffsetsAndReload(offsets); + return sstable; + } + + /** + * Any offsets at all put an sstable in this group + */ + @Test + public void anyOffsetsMeanUnreconciled() throws IOException + { + SSTableReader transfer = sstableWithTransfers(id(1, 0)); + SSTableReader mutations = sstableWithMutations(1, 0); + + assertEquals(CompactionGroup.UNRECONCILED, CompactionGroup.of(transfer)); + assertEquals(CompactionGroup.UNRECONCILED, CompactionGroup.of(mutations)); + assertEquals("no offsets is untracked data", CompactionGroup.UNREPAIRED, CompactionGroup.of(makeSSTable(true))); + assertSame(csm.getHolder(CompactionGroup.of(transfer)), csm.getHolder(CompactionGroup.of(mutations))); + } + + /** + * A silo per distinct transfer set, so no strategy can select across the boundary + */ + @Test + public void sstablesAreSiloedByTransferSet() throws IOException + { + TrackedCompactionManager manager = manager(); + + SSTableReader a = sstableWithTransfers(id(1, 0)); + SSTableReader b = sstableWithTransfers(id(1, 1)); + SSTableReader both = sstableWithTransfers(id(1, 0), id(1, 1)); + SSTableReader sameAsA = sstableWithTransfers(id(1, 0)); + + manager.addSSTable(a); + manager.addSSTable(b); + manager.addSSTable(both); + manager.addSSTable(sameAsA); + + assertEquals(3, transferKeys(manager).size()); + assertEquals("an equal transfer set must share a silo rather than making a fourth", + ImmutableSet.of(a, sameAsA), manager.sstablesFor(key(id(1, 0)))); + assertEquals(Collections.singleton(b), manager.sstablesFor(key(id(1, 1)))); + assertEquals(Collections.singleton(both), manager.sstablesFor(key(id(1, 0), id(1, 1)))); + } + + /** + * Sstables carrying no transfers share the empty-key silo + */ + @Test + public void sstablesWithoutTransfersShareOneSilo() throws IOException + { + TrackedCompactionManager manager = manager(); + + SSTableReader a = sstableWithMutations(1, 0); + SSTableReader b = sstableWithMutations(2, 7); + SSTableReader transfer = sstableWithTransfers(id(1, 0)); + manager.addSSTable(a); + manager.addSSTable(b); + manager.addSSTable(transfer); + + assertEquals(ImmutableSet.of(a, b), manager.sstablesFor(TrackedCompactionManager.NONE)); + assertSame(manager.getIfPresent(TrackedCompactionManager.NONE), manager.getIfPresent(a)); + assertSame(manager.getIfPresent(TrackedCompactionManager.NONE), manager.getIfPresent(b)); + + assertNotSame("a transfer must not be compactable with ordinary tracked data", + manager.getIfPresent(transfer), manager.getIfPresent(a)); + assertEquals(Collections.singleton(transfer), manager.sstablesFor(key(id(1, 0)))); + } + + /** + * An empty transfer silo is pruned however it came to be empty, and holds no data and generates no work in the + * meantime. Both routes matter: the delete path reaches past the manager straight to the strategy, and a delete + * notification can resurrect a silo that never held anything. + */ + @Test + public void emptyTransferSilosArePruned() throws IOException + { + TrackedCompactionManager manager = manager(); + SSTableReader sstable = sstableWithTransfers(id(1, 0)); + + // Resurrected by a lookup, never having held anything. + manager.getOrCreate(sstable); + // Holds no data and generates no work, and the walk that reads the estimate prunes it. + assertFalse(manager.hasDataFor(key(id(1, 0)))); + assertTrue(nextBackgroundTasks(manager).isEmpty()); + assertEquals(0, manager.getEstimatedRemainingTasks()); + assertTrue(transferKeys(manager).isEmpty()); + + // Emptied past the manager, so the manager is never told. + manager.addSSTable(sstable); + assertEquals(1, transferKeys(manager).size()); + manager.getIfPresent(sstable).getStrategyFor(sstable).removeSSTable(sstable); + assertEquals("manager was not notified, so the silo is still there", 1, transferKeys(manager).size()); + assertEquals(0, manager.getEstimatedRemainingTasks()); + assertTrue(transferKeys(manager).isEmpty()); + + // Emptied through the manager, which prunes without waiting for a walk. + manager.addSSTable(sstable); + manager.removeSSTable(sstable); + assertTrue("removal through the manager prunes immediately", transferKeys(manager).isEmpty()); + } + + /** The NONE silo takes ordinary unreconciled writes continuously, so unlike a transfer silo it outlives emptying. */ + @Test + public void noneTransfersSiloIsNeverPruned() throws IOException + { + TrackedCompactionManager manager = manager(); + assertNotNull(manager.getIfPresent(TrackedCompactionManager.NONE)); + + SSTableReader sstable = sstableWithMutations(1, 0); + manager.addSSTable(sstable); + manager.removeSSTable(sstable); + + assertTrue("the NONE silo must survive being emptied where a transfer silo would not", + manager.keys().contains(TrackedCompactionManager.NONE)); + assertNotNull(manager.getIfPresent(TrackedCompactionManager.NONE)); + assertFalse(manager.hasDataFor(TrackedCompactionManager.NONE)); + } + + /** + * Membership answers track the sstables held, not whether a silo exists for their key. + * */ + @Test + public void sessionHasData() throws IOException + { + TrackedCompactionManager manager = manager(); + SSTableReader transfer = sstableWithTransfers(id(1, 0)); + SSTableReader unreconciled = sstableWithMutations(1, 0); + + assertFalse(manager.hasDataFor(key(id(1, 0)))); + assertFalse(manager.containsSSTable(transfer)); + assertFalse(manager.containsSSTable(unreconciled)); + assertNull(manager.getIfPresent(transfer)); + + manager.addSSTable(transfer); + manager.addSSTable(unreconciled); + + assertTrue(manager.hasDataFor(key(id(1, 0)))); + assertTrue(manager.containsSSTable(transfer)); + assertTrue(manager.containsSSTable(unreconciled)); + assertNotNull(manager.getIfPresent(transfer)); + } + + /** + * The repair-status notification sends a removal for sstables whose group has already changed, so a removal for an + * sstable this manager never held must not throw. + */ + @Test + public void removingSomethingNeverHeldIsHarmless() throws IOException + { + TrackedCompactionManager manager = manager(); + manager.removeSSTable(sstableWithMutations(1, 0)); + manager.removeSSTable(sstableWithTransfers(id(1, 0))); + } + + /** Compaction output replaces its inputs in whichever silo each belongs to, without creating a new one. */ + @Test + public void replaceRoutesProperly() throws IOException + { + TrackedCompactionManager manager = manager(); + + SSTableReader transfer = sstableWithTransfers(id(1, 0)); + SSTableReader unreconciled = sstableWithMutations(1, 0); + manager.addSSTable(transfer); + manager.addSSTable(unreconciled); + + SSTableReader newTransfer = sstableWithTransfers(id(1, 0)); + SSTableReader newUnreconciled = sstableWithMutations(2, 3); + + manager.replaceSSTables(group(manager, ImmutableSet.of(transfer, unreconciled)), + group(manager, ImmutableSet.of(newTransfer, newUnreconciled))); + + // Each replacement lands in the silo its input came from, and no new silo appears. + assertEquals(Collections.singleton(newTransfer), manager.sstablesFor(key(id(1, 0)))); + assertEquals(Collections.singleton(newUnreconciled), + manager.sstablesFor(TrackedCompactionManager.NONE)); + assertEquals(1, transferKeys(manager).size()); + } + + @Test + public void replaceWithNothingRemovedIsAnAdd() throws IOException + { + TrackedCompactionManager manager = manager(); + SSTableReader added = sstableWithMutations(1, 0); + + manager.replaceSSTables(group(manager, Collections.emptySet()), + group(manager, Collections.singleton(added))); + + assertTrue(manager.containsSSTable(added)); + } + + @Test + public void getNextBackgroundTaskNoSessions() + { + TrackedCompactionManager manager = manager(); + + // Neither the always-present silo when empty, nor a silo that does not exist. + assertNull(manager.getPromotionTask(TrackedCompactionManager.NONE)); + assertNull(manager.getPromotionTask(key(id(9, 9)))); + assertTrue(manager.getNextPromotionTasks().isEmpty()); + } + + @Test + public void userDefinedTaskTest() throws IOException + { + TrackedCompactionManager manager = manager(); + SSTableReader sstable = sstableWithTransfers(id(1, 0)); + + assertNull(manager.getIfPresent(sstable)); + + GroupedSSTableContainer container = group(manager, Collections.singleton(sstable)); + Collection tasks = manager.getUserDefinedTasks(container, FBUtilities.nowInSeconds()); + try + { + assertEquals("the request must be honoured, not dropped", 1, tasks.size()); + assertNotNull(manager.getIfPresent(sstable)); + } + finally + { + tasks.forEach(AbstractCompactionTask::rejected); + } + } +} diff --git a/test/unit/org/apache/cassandra/db/compaction/TrackedUnreconciledPromotionTest.java b/test/unit/org/apache/cassandra/db/compaction/TrackedUnreconciledPromotionTest.java new file mode 100644 index 000000000000..03ed241b6461 --- /dev/null +++ b/test/unit/org/apache/cassandra/db/compaction/TrackedUnreconciledPromotionTest.java @@ -0,0 +1,484 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.compaction; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; + +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Iterables; + +import org.junit.AfterClass; +import org.junit.Assert; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.cassandra.SchemaLoader; +import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.Keyspace; +import org.apache.cassandra.db.Mutation; +import org.apache.cassandra.db.SimpleBuilders; +import org.apache.cassandra.db.lifecycle.LifecycleTransaction; +import org.apache.cassandra.db.marshal.Int32Type; +import org.apache.cassandra.db.partitions.PartitionUpdate; +import org.apache.cassandra.dht.Bounds; +import org.apache.cassandra.dht.Token; +import org.apache.cassandra.io.sstable.Descriptor; +import org.apache.cassandra.io.sstable.format.SSTableReader; +import org.apache.cassandra.replication.ImmutableCoordinatorLogOffsets; +import org.apache.cassandra.replication.MutationId; +import org.apache.cassandra.replication.MutationJournal; +import org.apache.cassandra.replication.MutationTrackingService; +import org.apache.cassandra.replication.Offsets; +import org.apache.cassandra.replication.ShortMutationId; +import org.apache.cassandra.schema.CompactionParams; +import org.apache.cassandra.schema.KeyspaceParams; +import org.apache.cassandra.schema.ReplicationType; +import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.service.ActiveRepairService; +import org.apache.cassandra.utils.ByteBufferUtil; +import org.apache.cassandra.utils.Clock; +import org.apache.cassandra.utils.FBUtilities; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +public class TrackedUnreconciledPromotionTest +{ + private static final AtomicInteger keyspaceNumber = new AtomicInteger(); + + static + { + DatabaseDescriptor.daemonInitialization(); + } + + @BeforeClass + public static void setupClass() + { + SchemaLoader.prepareServer(); + MutationJournal.start(); + MutationTrackingService.start(); + + // we want to drive this manually for testing + MutationTrackingService.instance().pauseOffsetsPersisterForTesting(); + } + + @AfterClass + public static void tearDownClass() + { + MutationTrackingService.instance().resumeOffsetsPersisterForTesting(); + } + + private static void persistLogState() + { + MutationTrackingService.instance().persistLogStateForTesting(true); + } + + private static String nextKeyspaceName() + { + return "tracked_promotion_" + keyspaceNumber.incrementAndGet(); + } + + private static ColumnFamilyStore newTrackedTable() + { + String ks = nextKeyspaceName(); + TableMetadata tableMetadata = + TableMetadata.builder(ks, "tbl") + .addPartitionKeyColumn("k", Int32Type.instance) + .addRegularColumn("v", Int32Type.instance) + .build(); + + SchemaLoader.createKeyspace(ks, KeyspaceParams.simple(1, ReplicationType.tracked), tableMetadata); + + ColumnFamilyStore cfs = Keyspace.open(ks).getColumnFamilyStore("tbl"); + cfs.disableAutoCompaction(); + return cfs; + } + + private static MutationId applyMutation(ColumnFamilyStore cfs, int k, int v) + { + TableMetadata metadata = cfs.metadata(); + DecoratedKey key = metadata.partitioner.decorateKey(ByteBufferUtil.bytes(k)); + MutationId id = MutationTrackingService.instance().nextMutationId(metadata.keyspace, key.getToken()); + SimpleBuilders.MutationBuilder builder = new SimpleBuilders.MutationBuilder(id, metadata.keyspace, key); + PartitionUpdate.SimpleBuilder partition = builder.update(metadata); + partition.row().add("v", v); + Mutation mutation = builder.build(); + Assert.assertFalse(mutation.id().isNone()); + mutation.apply(); + return mutation.id(); + } + + private static TrackedCompactionManager manager(ColumnFamilyStore cfs) + { + return (TrackedCompactionManager) cfs.getCompactionStrategyManager() + .getHolder(CompactionGroup.UNRECONCILED); + } + + private static Set promotable(ColumnFamilyStore cfs) + { + return manager(cfs).promotableSSTables(); + } + + private static Collection promotionTasks(ColumnFamilyStore cfs) + { + return manager(cfs).getNextPromotionTasks(); + } + + private static SSTableReader flushUnreconciled(ColumnFamilyStore cfs, int k) + { + Set before = new HashSet<>(cfs.getLiveSSTables()); + applyMutation(cfs, k, k); + cfs.forceBlockingFlush(ColumnFamilyStore.FlushReason.UNIT_TESTS); + + Set added = new HashSet<>(cfs.getLiveSSTables()); + added.removeAll(before); + assertEquals(1, added.size()); + + SSTableReader written = Iterables.getOnlyElement(added); + assertFalse(written.isRepaired()); + assertFalse(written.getSSTableMetadata().coordinatorLogOffsets.isEmpty()); + assertEquals(CompactionGroup.UNRECONCILED, CompactionGroup.of(written)); + return written; + } + + private static void runPromotion(Collection tasks) + { + for (AbstractCompactionTask task : tasks) + task.execute(ActiveCompactionsTracker.NOOP); + } + + @Test + public void backlogDrainsInOnePass() + { + ColumnFamilyStore cfs = newTrackedTable(); + + int backlog = 4; + Set stranded = new HashSet<>(); + for (int k = 0; k < backlog; k++) + stranded.add(flushUnreconciled(cfs, k)); + + assertEquals(backlog, cfs.getLiveSSTables().size()); + assertTrue(promotable(cfs).isEmpty()); + + persistLogState(); + assertEquals(stranded, promotable(cfs)); + + Collection tasks = promotionTasks(cfs); + assertEquals("the whole backlog should be one task, not one per sstable", 1, tasks.size()); + runPromotion(tasks); + + for (SSTableReader sstable : cfs.getLiveSSTables()) + { + assertTrue(sstable.isRepaired()); + assertTrue(sstable.getSSTableMetadata().coordinatorLogOffsets.isEmpty()); + assertEquals(CompactionGroup.REPAIRED, CompactionGroup.of(sstable)); + // Tombstone purging is gated on repairedAt, so the timestamp has to be real rather than the sentinel. + Assert.assertNotEquals(ActiveRepairService.UNREPAIRED_SSTABLE, sstable.getRepairedAt()); + } + assertTrue(promotable(cfs).isEmpty()); + } + + @Test + public void busySSTableDoesntBlockRemainder() + { + ColumnFamilyStore cfs = newTrackedTable(); + + Set stranded = new HashSet<>(); + for (int k = 0; k < 3; k++) + stranded.add(flushUnreconciled(cfs, k)); + + persistLogState(); + assertEquals(stranded, promotable(cfs)); + + SSTableReader busy = stranded.iterator().next(); + try (LifecycleTransaction held = cfs.getTracker().tryModify(Collections.singleton(busy), + OperationType.COMPACTION)) + { + assertNotNull(held); + + Collection tasks = promotionTasks(cfs); + assertEquals(1, tasks.size()); + runPromotion(tasks); + } + + for (SSTableReader sstable : cfs.getLiveSSTables()) + { + if (sstable.descriptor.equals(busy.descriptor)) + assertFalse("the busy sstable should have been left alone", sstable.isRepaired()); + else + assertTrue("the rest of the backlog should have drained anyway", sstable.isRepaired()); + } + + // The deferred sstable is promoted on a later pass. + assertEquals("the busy sstable is still eligible on a later pass", 1, promotable(cfs).size()); + runPromotion(promotionTasks(cfs)); + for (SSTableReader sstable : cfs.getLiveSSTables()) + assertTrue("the whole backlog should eventually drain", sstable.isRepaired()); + } + + @Test + public void neitherRepairedNorOffsetFreeSSTablesArePromotable() throws IOException + { + ColumnFamilyStore repairedCase = newTrackedTable(); + SSTableReader repaired = flushUnreconciled(repairedCase, 1); + persistLogState(); + assertTrue(manager(repairedCase).isPromotable(repaired)); + + ImmutableCoordinatorLogOffsets offsets = repaired.getSSTableMetadata().coordinatorLogOffsets; + repaired.mutateRepairedAndReload(Clock.Global.currentTimeMillis(), null); + repaired.mutateCoordinatorLogOffsetsAndReload(offsets); + + assertFalse("precondition: it still carries offsets, so the repaired guard is the one under test", + repaired.getSSTableMetadata().coordinatorLogOffsets.isEmpty()); + assertFalse(manager(repairedCase).isPromotable(repaired)); + assertTrue(promotable(repairedCase).isEmpty()); + + ColumnFamilyStore offsetFreeCase = newTrackedTable(); + SSTableReader offsetFree = flushUnreconciled(offsetFreeCase, 1); + offsetFree.mutateCoordinatorLogOffsetsAndReload(new ImmutableCoordinatorLogOffsets.Builder().build()); + + assertFalse("precondition: it stays unrepaired, so the offsets guard is the one under test", + offsetFree.isRepaired()); + assertTrue(offsetFree.getSSTableMetadata().coordinatorLogOffsets.isEmpty()); + assertFalse(manager(offsetFreeCase).isPromotable(offsetFree)); + assertTrue(promotable(offsetFreeCase).isEmpty()); + } + + private static TrackedCompactionManager standaloneManager(ColumnFamilyStore cfs) + { + TrackedCompactionManager manager = + new TrackedCompactionManager(cfs, TrackedCompactionManagerTest.SINGLE_PARTITION); + manager.setStrategy(CompactionParams.DEFAULT, 1); + return manager; + } + + private static ShortMutationId shortIdOf(MutationId id) + { + return new ShortMutationId(id.logId(), id.offset()); + } + + private static void setOffsets(SSTableReader sstable, ShortMutationId transferId, MutationId... mutations) + throws IOException + { + Bounds bounds = new Bounds<>(sstable.getFirst().getToken(), sstable.getLast().getToken()); + ImmutableCoordinatorLogOffsets.Builder builder = new ImmutableCoordinatorLogOffsets.Builder(); + builder.addTransfer(transferId, bounds); + for (MutationId id : mutations) + builder.add(id); + sstable.mutateCoordinatorLogOffsetsAndReload(builder.build()); + } + + /** + * SSTable shouldn't contain transfer and mutation ids offsets, but if they do for some reason, both should + * be taken into account when promoting to repaired + */ + @Test + public void reconciledTransferDoesNotPromoteUnreconciledMutations() throws IOException + { + ColumnFamilyStore cfs = newTrackedTable(); + TrackedCompactionManager manager = standaloneManager(cfs); + + MutationId transferOrigin = applyMutation(cfs, 100, 100); + Set reconciled = new HashSet<>(); + for (int k = 0; k < 3; k++) + reconciled.add(flushUnreconciled(cfs, k)); + + persistLogState(); + ShortMutationId transferId = shortIdOf(transferOrigin); + for (SSTableReader sstable : reconciled) + setOffsets(sstable, transferId); + + // Applied after the persist, so this one has not reconciled. + MutationId behindId = applyMutation(cfs, 99, 99); + SSTableReader behind = flushUnreconciled(cfs, 98); + setOffsets(behind, transferId, behindId); + + ImmutableSet key = ImmutableSet.of(transferId); + manager.addSSTables(reconciled); + manager.addSSTable(behind); + + // It shares the silo, so eligibility is the only thing that can exclude it. + assertEquals(key, TrackedCompactionManager.keyOf(behind)); + assertEquals(4, manager.sstablesFor(key).size()); + assertFalse("its mutation has not reconciled, so it must not be promotable", manager.isPromotable(behind)); + assertEquals("promotion takes the reconciled members only", reconciled, manager.promotableSSTables(key)); + + // One task covers the eligible members of a silo rather than one task each. + AbstractCompactionTask promotion = manager.getPromotionTask(key); + try + { + assertNotNull(promotion); + assertEquals(reconciled, promotion.transaction.originals()); + } + finally + { + // it holds a lifecycle transaction over the silo, which leaks if neither run nor released + if (promotion != null) + promotion.rejected(); + } + } + + @Test + public void partlyEligibleSiloPromotesTheSubset() + { + ColumnFamilyStore cfs = newTrackedTable(); + + Set reconciled = new HashSet<>(); + for (int k = 0; k < 3; k++) + reconciled.add(flushUnreconciled(cfs, k)); + persistLogState(); + SSTableReader behind = flushUnreconciled(cfs, 3); + + TrackedCompactionManager manager = manager(cfs); + assertEquals(4, manager.sstablesFor(TrackedCompactionManager.NONE).size()); + assertEquals("only the earlier flushes are eligible", reconciled, promotable(cfs)); + + runPromotion(promotionTasks(cfs)); + + for (SSTableReader sstable : cfs.getLiveSSTables()) + { + if (sstable.descriptor.equals(behind.descriptor)) + assertFalse("the unreconciled sstable must be left alone", sstable.isRepaired()); + else + assertTrue(sstable.isRepaired()); + } + } + private static AbstractStrategyHolder holderOf(ColumnFamilyStore cfs, SSTableReader sstable) + { + return cfs.getCompactionStrategyManager().getHolder(CompactionGroup.of(sstable)); + } + + @Test + public void promotionMovesTheSSTableToTheRepairedHolder() + { + ColumnFamilyStore cfs = newTrackedTable(); + SSTableReader stranded = flushUnreconciled(cfs, 1); + + AbstractStrategyHolder tracked = manager(cfs); + // Starts in the tracked holder, counted as unrepaired. + assertTrue(tracked.containsSSTable(stranded)); + assertTrue(cfs.metric.bytesUnrepaired.getValue() > 0); + assertEquals(0L, (long) cfs.metric.bytesRepaired.getValue()); + + persistLogState(); + runPromotion(promotionTasks(cfs)); + + SSTableReader promoted = Iterables.getOnlyElement(cfs.getLiveSSTables()); + AbstractStrategyHolder repaired = holderOf(cfs, promoted); + + assertEquals(CompactionGroup.REPAIRED, CompactionGroup.of(promoted)); + // Ownership moves between holders, and the byte metrics follow it. + assertTrue(repaired.containsSSTable(promoted)); + assertFalse(tracked.containsSSTable(promoted)); + assertEquals(0L, (long) cfs.metric.bytesUnrepaired.getValue()); + assertTrue(cfs.metric.bytesRepaired.getValue() > 0); + } + + @Test + public void compactionKeepsOffsetsIncludingReconciledOnes() + { + ColumnFamilyStore cfs = newTrackedTable(); + + SSTableReader first = flushUnreconciled(cfs, 1); + persistLogState(); // first's id reconciles, but nothing rewrites it + SSTableReader second = flushUnreconciled(cfs, 2); + + Set before = new HashSet<>(); + before.addAll(idsOf(first)); + before.addAll(idsOf(second)); + assertEquals(2, before.size()); + + compact(cfs, first, second); + + SSTableReader merged = Iterables.getOnlyElement(cfs.getLiveSSTables()); + assertFalse("one id has not reconciled, so the output must not be promoted at write time", + merged.isRepaired()); + assertEquals("the union must survive, reconciled id included", before, idsOf(merged)); + + // ...and a second compaction of the same sstable is equally non-destructive + compact(cfs, merged); + SSTableReader again = Iterables.getOnlyElement(cfs.getLiveSSTables()); + assertFalse(again.isRepaired()); + assertEquals(before, idsOf(again)); + + persistLogState(); + runPromotion(promotionTasks(cfs)); + + SSTableReader promoted = Iterables.getOnlyElement(cfs.getLiveSSTables()); + assertTrue(promoted.isRepaired()); + assertTrue(promoted.getSSTableMetadata().coordinatorLogOffsets.isEmpty()); + } + + private static Set idsOf(SSTableReader sstable) + { + Set ids = new HashSet<>(); + for (Map.Entry entry : sstable.getSSTableMetadata().coordinatorLogOffsets.entries()) + Iterables.addAll(ids, entry.getValue()); + return ids; + } + + private static void compact(ColumnFamilyStore cfs, SSTableReader... sstables) + { + List descriptors = new ArrayList<>(); + for (SSTableReader sstable : sstables) + descriptors.add(sstable.descriptor); + FBUtilities.waitOnFuture(CompactionManager.instance.submitUserDefined(cfs, descriptors, CompactionManager.NO_GC)); + } + + @Test + public void unpromotableGaugeTest() throws IOException + { + ColumnFamilyStore cfs = newTrackedTable(); + assertEquals(0, (int) cfs.metric.unpromotableSSTables.getValue()); + + SSTableReader stranded = flushUnreconciled(cfs, 1); + assertEquals("carrying offsets, so it is promotable and does not count", + 0, (int) cfs.metric.unpromotableSSTables.getValue()); + + // Clearing the offsets while leaving it unrepaired is the state the gauge is for. Promotion cannot produce it, + // because it sets repairedAt in the same mutation; nodetool verify resetting repairedAt can. + stranded.mutateCoordinatorLogOffsetsAndReload(new ImmutableCoordinatorLogOffsets.Builder().build()); + assertFalse(stranded.isRepaired()); + assertEquals("unrepaired with no offsets must be counted", + 1, (int) cfs.metric.unpromotableSSTables.getValue()); + + // A promoted sstable also has no offsets, but it is repaired, so it must not be counted. + ColumnFamilyStore other = newTrackedTable(); + flushUnreconciled(other, 1); + persistLogState(); + runPromotion(promotionTasks(other)); + SSTableReader promoted = Iterables.getOnlyElement(other.getLiveSSTables()); + assertTrue(promoted.getSSTableMetadata().coordinatorLogOffsets.isEmpty()); + assertTrue(promoted.isRepaired()); + assertEquals("repaired, so not unpromotable however few offsets it carries", + 0, (int) other.metric.unpromotableSSTables.getValue()); + } +} diff --git a/test/unit/org/apache/cassandra/db/lifecycle/LifecycleTransactionTest.java b/test/unit/org/apache/cassandra/db/lifecycle/LifecycleTransactionTest.java index 8a7905dffc2e..eeaaf7224eac 100644 --- a/test/unit/org/apache/cassandra/db/lifecycle/LifecycleTransactionTest.java +++ b/test/unit/org/apache/cassandra/db/lifecycle/LifecycleTransactionTest.java @@ -21,7 +21,6 @@ import java.util.ArrayList; import java.util.List; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicReference; import com.google.common.util.concurrent.Uninterruptibles; @@ -36,6 +35,7 @@ import org.apache.cassandra.db.compaction.OperationType; import org.apache.cassandra.db.lifecycle.LifecycleTransaction.ReaderState; import org.apache.cassandra.db.lifecycle.LifecycleTransaction.ReaderState.Action; +import org.apache.cassandra.db.memtable.LogDomainBounds; import org.apache.cassandra.io.sstable.format.SSTableFormat; import org.apache.cassandra.io.sstable.format.SSTableReader; import org.apache.cassandra.schema.MockSchema; @@ -315,7 +315,8 @@ private static final class TxnTest extends TestableTransaction private static Tracker tracker(ColumnFamilyStore cfs, List readers) { - Tracker tracker = new Tracker(cfs, cfs.createMemtable(new AtomicReference<>(CommitLogPosition.NONE)), false); + LogDomainBounds bounds = LogDomainBounds.of(CommitLogPosition.NONE); + Tracker tracker = new Tracker(cfs, cfs.createMemtable(bounds), bounds, false); tracker.addInitialSSTables(readers); return tracker; } diff --git a/test/unit/org/apache/cassandra/db/lifecycle/TrackerTest.java b/test/unit/org/apache/cassandra/db/lifecycle/TrackerTest.java index 57e76c9a46a4..c89cd24b10d3 100644 --- a/test/unit/org/apache/cassandra/db/lifecycle/TrackerTest.java +++ b/test/unit/org/apache/cassandra/db/lifecycle/TrackerTest.java @@ -23,7 +23,6 @@ import java.util.List; import java.util.Optional; import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; import com.google.common.base.Predicate; import com.google.common.base.Predicates; @@ -37,10 +36,13 @@ import org.apache.cassandra.ServerTestUtils; import org.apache.cassandra.config.DatabaseDescriptor; import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.LogDomain; import org.apache.cassandra.db.commitlog.CommitLog; import org.apache.cassandra.db.commitlog.CommitLogPosition; import org.apache.cassandra.db.compaction.OperationType; +import org.apache.cassandra.db.memtable.LogDomainBounds; import org.apache.cassandra.db.memtable.Memtable; +import org.apache.cassandra.db.memtable.SplitDomainMemtable; import org.apache.cassandra.io.sstable.format.SSTableReader; import org.apache.cassandra.io.sstable.keycache.KeyCacheSupport; import org.apache.cassandra.notifications.INotification; @@ -272,21 +274,23 @@ public void testMemtableReplacement() Tracker tracker = cfs.getTracker(); tracker.subscribe(listener); - Memtable prev1 = tracker.switchMemtable(true, cfs.createMemtable(new AtomicReference<>(CommitLog.instance.getCurrentPosition()))); + LogDomainBounds bounds1 = LogDomainBounds.atCurrentPositions(); + Memtable prev1 = tracker.switchMemtable(true, cfs.createMemtable(bounds1), bounds1); OpOrder.Group write1 = cfs.keyspace.writeOrder.getCurrent(); OpOrder.Barrier barrier1 = cfs.keyspace.writeOrder.newBarrier(); - prev1.switchOut(barrier1, new AtomicReference<>(CommitLog.instance.getCurrentPosition())); + prev1.switchOut(barrier1, LogDomainBounds.atCurrentPositions()); barrier1.issue(); - Memtable prev2 = tracker.switchMemtable(false, cfs.createMemtable(new AtomicReference<>(CommitLog.instance.getCurrentPosition()))); + LogDomainBounds bounds2 = LogDomainBounds.atCurrentPositions(); + Memtable prev2 = tracker.switchMemtable(false, cfs.createMemtable(bounds2), bounds2); OpOrder.Group write2 = cfs.keyspace.writeOrder.getCurrent(); OpOrder.Barrier barrier2 = cfs.keyspace.writeOrder.newBarrier(); - prev2.switchOut(barrier2, new AtomicReference<>(CommitLog.instance.getCurrentPosition())); + prev2.switchOut(barrier2, LogDomainBounds.atCurrentPositions()); barrier2.issue(); Memtable cur = tracker.getView().getCurrentMemtable(); OpOrder.Group writecur = cfs.keyspace.writeOrder.getCurrent(); - Assert.assertEquals(prev1, tracker.getMemtableFor(write1, CommitLogPosition.NONE)); - Assert.assertEquals(prev2, tracker.getMemtableFor(write2, CommitLogPosition.NONE)); - Assert.assertEquals(cur, tracker.getMemtableFor(writecur, CommitLogPosition.NONE)); + Assert.assertEquals(prev1, tracker.getMemtableFor(write1, CommitLogPosition.NONE, LogDomain.COMMIT_LOG)); + Assert.assertEquals(prev2, tracker.getMemtableFor(write2, CommitLogPosition.NONE, LogDomain.COMMIT_LOG)); + Assert.assertEquals(cur, tracker.getMemtableFor(writecur, CommitLogPosition.NONE, LogDomain.COMMIT_LOG)); Assert.assertEquals(2, listener.received.size()); Assert.assertTrue(listener.received.get(0) instanceof MemtableRenewedNotification); Assert.assertTrue(listener.received.get(1) instanceof MemtableSwitchedNotification); @@ -321,8 +325,9 @@ public void testMemtableReplacement() tracker = cfs.getTracker(); listener = new MockListener(false); tracker.subscribe(listener); - Memtable next1 = cfs.createMemtable(new AtomicReference<>(CommitLog.instance.getCurrentPosition())); - prev1 = tracker.switchMemtable(false, next1); + LogDomainBounds nextBounds = LogDomainBounds.atCurrentPositions(); + Memtable next1 = cfs.createMemtable(nextBounds); + prev1 = tracker.switchMemtable(false, next1, nextBounds); tracker.markFlushing(prev1); reader = MockSchema.sstable(0, 10, true, cfs); cfs.invalidate(false); @@ -384,4 +389,37 @@ public void testNotifications() listener.received.clear(); } + /** + * {@code Tracker.installSplit} reads the current memtable outside {@code viewUpdateLock}, so it can race with a + * flush before the split applies. This test emulates that race and confirms that the split fails and nothing breaks + */ + @Test + public void retiredGenerationCantBeSplit() + { + ColumnFamilyStore cfs = MockSchema.newCFS(); + Tracker tracker = cfs.getTracker(); + Memtable retiring = tracker.getView().getCurrentMemtable(); + + // Stand in for the flush that wins the race: switchMemtable retires the generation installSplit had read. + LogDomainBounds replacementBounds = LogDomainBounds.atCurrentPositions(); + tracker.switchMemtable(false, cfs.createMemtable(replacementBounds), replacementBounds); + // The window the guard covers: no longer current, but not yet moved to flushingMemtables. + Assert.assertNotEquals(retiring, tracker.getView().getCurrentMemtable()); + Assert.assertTrue("still live until marked flushing", tracker.getView().liveMemtables.contains(retiring)); + + // The split installSplit would have applied, had it not rechecked under the lock. + Memtable journal = cfs.createMemtable(tracker.getView().currentBounds.forDomain(LogDomain.MUTATION_JOURNAL), + LogDomain.MUTATION_JOURNAL); + SplitDomainMemtable wrapper = new SplitDomainMemtable(journal, retiring, retiring.getMemtableId()); + // Tracker.apply returns null when the permit predicate rejects the view. + Assert.assertNull("the split transform must not have run against a retired generation", + tracker.apply(View.canSplitMemtable(retiring), View.splitMemtable(retiring, wrapper))); + Assert.assertFalse(Iterables.any(tracker.getView().liveMemtables, m -> m instanceof SplitDomainMemtable)); + + // markFlushing is what a split would have broken: it filters retiring out of liveMemtables and asserts the + // list shrank by one, which a wrapper in retiring's place would fail. + tracker.markFlushing(retiring); + Assert.assertTrue(tracker.getView().flushingMemtables.contains(retiring)); + } + } diff --git a/test/unit/org/apache/cassandra/db/lifecycle/ViewTest.java b/test/unit/org/apache/cassandra/db/lifecycle/ViewTest.java index 6bd3896fb950..0f1141a1c026 100644 --- a/test/unit/org/apache/cassandra/db/lifecycle/ViewTest.java +++ b/test/unit/org/apache/cassandra/db/lifecycle/ViewTest.java @@ -37,6 +37,8 @@ import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.PartitionPosition; import org.apache.cassandra.db.commitlog.CommitLog; +import org.apache.cassandra.db.commitlog.CommitLogPosition; +import org.apache.cassandra.db.memtable.LogDomainBounds; import org.apache.cassandra.db.memtable.Memtable; import org.apache.cassandra.dht.AbstractBounds; import org.apache.cassandra.io.sstable.format.SSTableReader; @@ -167,13 +169,13 @@ public void testFlushing() Memtable memtable1 = initialView.getCurrentMemtable(); Memtable memtable2 = MockSchema.memtable(cfs); - cur = View.switchMemtable(memtable2).apply(cur); + cur = View.switchMemtable(memtable2, LogDomainBounds.of(CommitLogPosition.NONE)).apply(cur); Assert.assertEquals(2, cur.liveMemtables.size()); Assert.assertEquals(memtable1, cur.liveMemtables.get(0)); Assert.assertEquals(memtable2, cur.getCurrentMemtable()); Memtable memtable3 = MockSchema.memtable(cfs); - cur = View.switchMemtable(memtable3).apply(cur); + cur = View.switchMemtable(memtable3, LogDomainBounds.of(CommitLogPosition.NONE)).apply(cur); Assert.assertEquals(3, cur.liveMemtables.size()); Assert.assertEquals(0, cur.flushingMemtables.size()); Assert.assertEquals(memtable1, cur.liveMemtables.get(0)); @@ -226,6 +228,8 @@ static View fakeView(int memtableCount, int sstableCount, ColumnFamilyStore cfs, for (int i = 0 ; i < sstableCount ; i++) sstables.add(MockSchema.sstable(i, keepRef, cfs)); return new View(ImmutableList.copyOf(memtables), Collections.emptyList(), Helpers.identityMap(sstables), - Collections.emptyMap(), buildSSTableIntervalTree(sstables)); + Collections.emptyMap(), buildSSTableIntervalTree(sstables), + // View requires a boundary when it holds memtables, and none when it holds none. + memtables.isEmpty() ? null : LogDomainBounds.of(CommitLogPosition.NONE)); } } diff --git a/test/unit/org/apache/cassandra/db/memtable/LogDomainBoundsTest.java b/test/unit/org/apache/cassandra/db/memtable/LogDomainBoundsTest.java new file mode 100644 index 000000000000..5338201c22b6 --- /dev/null +++ b/test/unit/org/apache/cassandra/db/memtable/LogDomainBoundsTest.java @@ -0,0 +1,104 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.memtable; + +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Function; + +import org.junit.Test; + +import org.apache.cassandra.db.LogDomain; +import org.apache.cassandra.db.commitlog.CommitLogPosition; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +public class LogDomainBoundsTest +{ + @Test(timeout = 30_000) + public void basicSeal() + { + LogDomainBounds bounds = LogDomainBounds.unset(); + + bounds.seal(positions(new CommitLogPosition(1, 100), new CommitLogPosition(9000, 0))); + + assertEquals(sealedAt(1, 100), bounds.get(LogDomain.COMMIT_LOG)); + assertEquals(sealedAt(9000, 0), bounds.get(LogDomain.MUTATION_JOURNAL)); + } + + @Test(timeout = 30_000) + public void sealWithRacingWrite() + { + LogDomainBounds bounds = LogDomainBounds.unset(); + AtomicInteger reads = new AtomicInteger(); + + // The first read plants a higher position behind it, standing in for the racing write. + Function log = domain -> { + int read = reads.incrementAndGet(); + if (read == 1) + bounds.forDomain(LogDomain.COMMIT_LOG).set(new CommitLogPosition(1, 500)); + return new CommitLogPosition(1, read == 1 ? 100 : 600); + }; + + bounds.sealIfUnset(LogDomain.COMMIT_LOG, log); + + // Two reads means the loop re-read exactly once + assertEquals(2, reads.get()); + assertEquals(sealedAt(1, 600), bounds.get(LogDomain.COMMIT_LOG)); + } + + /** + * seal shouldn't set positions for log domains not in use + */ + @Test(timeout = 30_000) + public void singleDomainSeal() + { + LogDomainBounds bounds = LogDomainBounds.unset(); + + // The state before the journal starts: it reports no position, while the commit log still has one. + bounds.seal(positions(new CommitLogPosition(1, 100), null)); + + assertEquals(sealedAt(1, 100), bounds.get(LogDomain.COMMIT_LOG)); + assertNull(bounds.get(LogDomain.MUTATION_JOURNAL)); + } + + /** sealIfUnset must not move a bound the previous flush already fixed, or two adjacent spans overlap. */ + @Test(timeout = 30_000) + public void sealIfUnset() + { + LogDomainBounds bounds = LogDomainBounds.unset(); + CommitLogPosition inherited = new CommitLogPosition(7, 42); + bounds.forDomain(LogDomain.COMMIT_LOG).set(inherited); + + bounds.sealIfUnset(LogDomain.COMMIT_LOG, positions(new CommitLogPosition(7, 99), null)); + + // commit log domain was already set, so nothing should have changed + assertEquals(inherited, bounds.get(LogDomain.COMMIT_LOG)); + } + + private static Function positions(CommitLogPosition commitLog, CommitLogPosition journal) + { + return domain -> domain.isJournal() ? journal : commitLog; + } + + private static Memtable.LastCommitLogPosition sealedAt(long segmentId, int position) + { + return new Memtable.LastCommitLogPosition(new CommitLogPosition(segmentId, position)); + } +} diff --git a/test/unit/org/apache/cassandra/db/memtable/SplitDomainMemtableTest.java b/test/unit/org/apache/cassandra/db/memtable/SplitDomainMemtableTest.java new file mode 100644 index 000000000000..b4c8a85a7319 --- /dev/null +++ b/test/unit/org/apache/cassandra/db/memtable/SplitDomainMemtableTest.java @@ -0,0 +1,696 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.db.memtable; + +import java.util.Collections; +import java.util.EnumMap; +import java.util.EnumSet; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.function.Consumer; + +import com.google.common.collect.ImmutableSet; + +import org.assertj.core.api.Assertions; +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.cassandra.SchemaLoader; +import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.DataRange; +import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.Keyspace; +import org.apache.cassandra.db.LogDomain; +import org.apache.cassandra.db.Mutation; +import org.apache.cassandra.db.SimpleBuilders; +import org.apache.cassandra.db.commitlog.CommitLog; +import org.apache.cassandra.db.commitlog.CommitLogPosition; +import org.apache.cassandra.db.commitlog.CommitLogSegment; +import org.apache.cassandra.db.compaction.OperationType; +import org.apache.cassandra.db.filter.ColumnFilter; +import org.apache.cassandra.db.lifecycle.LifecycleTransaction; +import org.apache.cassandra.db.marshal.Int32Type; +import org.apache.cassandra.db.partitions.PartitionUpdate; +import org.apache.cassandra.db.partitions.UnfilteredPartitionIterator; +import org.apache.cassandra.db.rows.EncodingStats; +import org.apache.cassandra.db.rows.UnfilteredRowIterator; +import org.apache.cassandra.dht.Token; +import org.apache.cassandra.index.transactions.UpdateTransaction; +import org.apache.cassandra.io.sstable.SSTableProvenance; +import org.apache.cassandra.io.sstable.SSTableReadsListener; +import org.apache.cassandra.io.sstable.format.SSTableReader; +import org.apache.cassandra.replication.MutationId; +import org.apache.cassandra.replication.MutationJournal; +import org.apache.cassandra.replication.MutationTrackingService; +import org.apache.cassandra.schema.KeyspaceParams; +import org.apache.cassandra.schema.ReplicationType; +import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.utils.ByteBufferUtil; +import org.apache.cassandra.utils.FBUtilities; +import org.apache.cassandra.utils.concurrent.OpOrder; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; +import static org.junit.Assert.assertTrue; + +/** + * A table takes writes from both logs while part of its token range is migrating, and {@link SplitDomainMemtable} holds + * one internal memtable per {@link LogDomain} so that each write is bounded against its own log. Every consumer of a + * {@link Memtable} keeps working without learning it got a wrapper, so the cases here are grouped by consumer, in three + * sections: what a consumer reads off a split generation, how the tracker installs one, and how one flushes. + */ +public class SplitDomainMemtableTest +{ + private static final AtomicInteger keyspaceNumber = new AtomicInteger(); + + static + { + DatabaseDescriptor.daemonInitialization(); + } + + @BeforeClass + public static void setupClass() + { + SchemaLoader.prepareServer(); + MutationJournal.start(); + MutationTrackingService.start(); + } + + private static ColumnFamilyStore newTrackedTable() + { + String ks = "domain_split_" + keyspaceNumber.incrementAndGet(); + TableMetadata metadata = TableMetadata.builder(ks, "tbl") + .addPartitionKeyColumn("k", Int32Type.instance) + .addRegularColumn("v", Int32Type.instance) + .build(); + SchemaLoader.createKeyspace(ks, KeyspaceParams.simple(1, ReplicationType.tracked), metadata); + ColumnFamilyStore cfs = Keyspace.open(ks).getColumnFamilyStore("tbl"); + cfs.disableAutoCompaction(); + return cfs; + } + + private static Memtable internal(ColumnFamilyStore cfs, LogDomainBounds bounds, LogDomain domain) + { + return cfs.createMemtable(bounds.forDomain(domain), domain); + } + + private static SplitDomainMemtable newWrapper(ColumnFamilyStore cfs) + { + LogDomainBounds bounds = LogDomainBounds.atCurrentPositions(); + return new SplitDomainMemtable(internal(cfs, bounds, LogDomain.COMMIT_LOG), + internal(cfs, bounds, LogDomain.MUTATION_JOURNAL), + 1L); + } + + private static DecoratedKey key(ColumnFamilyStore cfs, int k) + { + return cfs.metadata().partitioner.decorateKey(ByteBufferUtil.bytes(k)); + } + + private static void write(ColumnFamilyStore cfs, Memtable memtable, int k, LogDomain domain) + { + write(cfs, memtable, k, domain, FBUtilities.timestampMicros()); + } + + private static void write(ColumnFamilyStore cfs, Memtable memtable, int k, LogDomain domain, long timestamp) + { + TableMetadata metadata = cfs.metadata(); + DecoratedKey key = key(cfs, k); + // An untracked write carries no mutation id, which is what keeps coordinator log offsets off the sstable a + // commit-log-domain memtable flushes. MigrationRouter guarantees the two agree for a base table. + MutationId id = domain.isJournal() + ? MutationTrackingService.instance().nextMutationId(metadata.keyspace, key.getToken()) + : MutationId.none(); + SimpleBuilders.MutationBuilder builder = new SimpleBuilders.MutationBuilder(id, metadata.keyspace, key); + builder.timestamp(timestamp); + builder.update(metadata).row().add("v", k); + Mutation mutation = builder.build(); + try (OpOrder.Group group = Keyspace.writeOrder.start()) + { + memtable.put(id, mutation.getPartitionUpdate(metadata), UpdateTransaction.NO_OP, group, domain); + } + } + + /** The keys a range read finds, which is the one read path that does not take a key. */ + private static Set partitionKeys(Memtable memtable) + { + Set keys = new HashSet<>(); + TableMetadata metadata = memtable.metadata(); + try (UnfilteredPartitionIterator partitions = memtable.partitionIterator(ColumnFilter.all(metadata), + DataRange.allData(metadata.partitioner), + SSTableReadsListener.NOOP_LISTENER)) + { + while (partitions.hasNext()) + { + try (UnfilteredRowIterator rows = partitions.next()) + { + keys.add(rows.partitionKey()); + } + } + } + return keys; + } + + // ---- What a consumer reads off a split generation ---------------------------------------------------------- + + /** + * Commit log segment reclamation asks which memtables still hold data below a position, through + * {@link ColumnFamilyStore#forceFlush(CommitLogPosition)}. Only the commit-log internal can hold + * commit-log-derived rows, so the wrapper answers from it alone. A generation whose journal internal is the dirty + * one holds no commit log data, and must not pin the segment. + *

+ * The ordering matters. Both internals' {@code approximateCommitLogLowerBound} are commit log positions taken at + * construction, whatever the internal's domain. Aggregating across them is therefore only distinguishable from + * delegating when the journal internal is the older of the two. That is also the order an install produces for a + * tracked table, since the memtable already live becomes an internal and the other is created after it. + */ + @Test + public void journalOnlySplitMemtableDoesntPinCommitlogSegments() + { + ColumnFamilyStore cfs = newTrackedTable(); + LogDomainBounds bounds = LogDomainBounds.atCurrentPositions(); + Memtable journalInternal = internal(cfs, bounds, LogDomain.MUTATION_JOURNAL); + write(cfs, journalInternal, 1, LogDomain.MUTATION_JOURNAL); + + // Advance the commit log past the journal internal's creation, so the two internals' bounds differ, then + // create the commit-log internal above the position under test and leave it clean. + appendToCommitLog(); + CommitLogPosition reclaimBelow = CommitLog.instance.getCurrentPosition(); + Memtable commitLogInternal = internal(cfs, bounds, LogDomain.COMMIT_LOG); + SplitDomainMemtable wrapper = new SplitDomainMemtable(commitLogInternal, journalInternal, + journalInternal.getMemtableId()); + + // sanity check + assertNotEquals(commitLogInternal.mayContainDataBefore(reclaimBelow), journalInternal.mayContainDataBefore(reclaimBelow)); + + assertFalse(wrapper.mayContainDataBefore(reclaimBelow)); + assertEquals(commitLogInternal.getApproximateCommitLogLowerBound(), + wrapper.getApproximateCommitLogLowerBound()); + } + + @Test + public void splitDomainMemtablePinsCommitlogSegments() + { + ColumnFamilyStore cfs = newTrackedTable(); + LogDomainBounds bounds = LogDomainBounds.atCurrentPositions(); + Memtable commitLogInternal = internal(cfs, bounds, LogDomain.COMMIT_LOG); + write(cfs, commitLogInternal, 1, LogDomain.COMMIT_LOG); + + appendToCommitLog(); + CommitLogPosition reclaimBelow = CommitLog.instance.getCurrentPosition(); + Memtable journalInternal = internal(cfs, bounds, LogDomain.MUTATION_JOURNAL); + SplitDomainMemtable wrapper = new SplitDomainMemtable(commitLogInternal, journalInternal, + commitLogInternal.getMemtableId()); + + // sanity check + assertNotEquals(commitLogInternal.mayContainDataBefore(reclaimBelow), journalInternal.mayContainDataBefore(reclaimBelow)); + + assertTrue(wrapper.mayContainDataBefore(reclaimBelow)); + assertEquals(commitLogInternal.getCommitLogLowerBound(), wrapper.getCommitLogLowerBound()); + } + + private static void appendToCommitLog() + { + String ks = "domain_split_untracked_" + keyspaceNumber.incrementAndGet(); + TableMetadata metadata = TableMetadata.builder(ks, "tbl") + .addPartitionKeyColumn("k", Int32Type.instance) + .addRegularColumn("v", Int32Type.instance) + .build(); + SchemaLoader.createKeyspace(ks, KeyspaceParams.simple(1, ReplicationType.untracked), metadata); + + DecoratedKey key = metadata.partitioner.decorateKey(ByteBufferUtil.bytes(1)); + SimpleBuilders.MutationBuilder builder = new SimpleBuilders.MutationBuilder(MutationId.none(), ks, key); + builder.update(metadata).row().add("v", 1); + builder.build().apply(); + } + + /** + * What the memtable pool and the read short-circuit read off a generation. Under-reporting a size lets a generation + * grow past its flush threshold. Over-reporting a minimum timestamp lets + * {@code SinglePartitionReadCommand} and {@code CompactionController} skip an sstable that is still needed. + */ + @Test + public void accountingCoversBothInternals() + { + ColumnFamilyStore cfs = newTrackedTable(); + SplitDomainMemtable wrapper = newWrapper(cfs); + Memtable commitLogInternal = wrapper.internalFor(LogDomain.COMMIT_LOG); + Memtable journalInternal = wrapper.internalFor(LogDomain.MUTATION_JOURNAL); + + write(cfs, wrapper, 1, LogDomain.MUTATION_JOURNAL); + + // A clean memtable reports Long.MAX_VALUE, so answering from the commit-log internal alone would report that. + assertEquals(Long.MAX_VALUE, commitLogInternal.getMinTimestamp()); + assertNotEquals(Long.MAX_VALUE, journalInternal.getMinTimestamp()); + assertEquals(journalInternal.getMinTimestamp(), wrapper.getMinTimestamp()); + + write(cfs, commitLogInternal, 2, LogDomain.COMMIT_LOG); + + // One row went to each internal, so a sum differs from either internal's own count. + assertEquals(2, wrapper.partitionCount()); + assertEquals(2, wrapper.operationCount()); + assertEquals(commitLogInternal.getLiveDataSize() + journalInternal.getLiveDataSize(), + wrapper.getLiveDataSize()); + assertTrue("neither internal alone accounts for the size", + wrapper.getLiveDataSize() > Math.max(commitLogInternal.getLiveDataSize(), + journalInternal.getLiveDataSize())); + assertEquals(Math.min(commitLogInternal.getMinTimestamp(), journalInternal.getMinTimestamp()), + wrapper.getMinTimestamp()); + assertEquals(Math.min(commitLogInternal.getMinLocalDeletionTime(), journalInternal.getMinLocalDeletionTime()), + wrapper.getMinLocalDeletionTime()); + } + + /** + * {@code getMinTimestamp} has two kinds of answer and both are numbers: a real timestamp, or + * {@code NO_MIN_TIMESTAMP} (-1) meaning the memtable has no usable timestamp. {@code SinglePartitionReadCommand} and + * {@code CompactionController} test for the sentinel before using the value, so a generation answering -1 while it + * holds timestamped rows makes them skip a comparison they should make. + * + * An internal answers -1 when its tracked minimum equals the epoch {@code EncodingStats} substitutes for an update + * carrying no liveness timestamp, so the test writes that epoch directly. Since -1 sorts below every real timestamp, + * a plain minimum over the two internals returns it. + */ + @Test + public void generationWithOneEpochTimestampInternalStillReportsAMinimum() + { + for (LogDomain epochDomain : LogDomain.values()) + { + ColumnFamilyStore cfs = newTrackedTable(); + SplitDomainMemtable wrapper = newWrapper(cfs); + LogDomain otherDomain = epochDomain.isJournal() ? LogDomain.COMMIT_LOG : LogDomain.MUTATION_JOURNAL; + long realTimestamp = FBUtilities.timestampMicros(); + + write(cfs, wrapper, 1, epochDomain, EncodingStats.NO_STATS.minTimestamp); + write(cfs, wrapper, 2, otherDomain, realTimestamp); + + // we expect the internal memtable to report no timestamp + assertEquals(Memtable.NO_MIN_TIMESTAMP, wrapper.internalFor(epochDomain).getMinTimestamp()); + // but the wrapper needs to report the minimum actual timestamp + assertEquals(realTimestamp, wrapper.getMinTimestamp()); + } + } + + /** + * Check that writes against split memtables are routed to the proper internal memtable, and that the internal + * memtables are presented to readers as a single memtable + */ + @Test + public void writesAndReadsSpanBothDomains() + { + ColumnFamilyStore cfs = newTrackedTable(); + SplitDomainMemtable wrapper = newWrapper(cfs); + Memtable commitLogInternal = wrapper.internalFor(LogDomain.COMMIT_LOG); + Memtable journalInternal = wrapper.internalFor(LogDomain.MUTATION_JOURNAL); + assertTrue(wrapper.isInternal(commitLogInternal)); + assertTrue(wrapper.isInternal(journalInternal)); + assertTrue(wrapper.isClean()); + + // Handed to the generation rather than to an internal, so put() does the routing. + write(cfs, wrapper, 1, LogDomain.MUTATION_JOURNAL); + write(cfs, wrapper, 2, LogDomain.COMMIT_LOG); + + // Each internal took its own domain's row and nothing else. + assertEquals(Collections.singleton(key(cfs, 1)), partitionKeys(journalInternal)); + assertEquals(Collections.singleton(key(cfs, 2)), partitionKeys(commitLogInternal)); + assertFalse(wrapper.isClean()); + + // Every read path answers for both internals, and for neither when the partition is in neither. + assertEquals(ImmutableSet.of(key(cfs, 1), key(cfs, 2)), partitionKeys(wrapper)); + assertNotNull(wrapper.snapshotPartition(key(cfs, 1))); + assertNotNull(wrapper.snapshotPartition(key(cfs, 2))); + assertNotNull(wrapper.rowIterator(key(cfs, 1))); + assertNotNull(wrapper.rowIterator(key(cfs, 2))); + assertNull(wrapper.snapshotPartition(key(cfs, 99))); + assertNull(wrapper.rowIterator(key(cfs, 99))); + assertEquals(maxToken(commitLogInternal.lastToken(), journalInternal.lastToken()), wrapper.lastToken()); + } + + private static Token maxToken(Token left, Token right) + { + return left.compareTo(right) >= 0 ? left : right; + } + + /** + * Both internals take the barrier the generation was switched out with, since the generation flushes as one unit. + * An internal left holding no barrier keeps accepting writes after the switch, and those rows miss the flush they + * were bounded into. + */ + @Test + public void switchingOutGenerationRetiresBothInternals() + { + ColumnFamilyStore cfs = newTrackedTable(); + SplitDomainMemtable wrapper = newWrapper(cfs); + Memtable commitLogInternal = wrapper.internalFor(LogDomain.COMMIT_LOG); + Memtable journalInternal = wrapper.internalFor(LogDomain.MUTATION_JOURNAL); + + OpOrder.Barrier barrier = Keyspace.writeOrder.newBarrier(); + LogDomainBounds upperBounds = LogDomainBounds.atCurrentPositions(); + upperBounds.seal(); + wrapper.switchOut(barrier, upperBounds); + barrier.issue(); + + // A write that starts after the barrier is refused for either domain, which holds only if both internals took + // it. An internal with no barrier reports itself as still the newest and accepts everything. + try (OpOrder.Group after = Keyspace.writeOrder.start()) + { + for (LogDomain domain : LogDomain.values()) + { + CommitLogPosition position = upperBounds.get(domain); + assertFalse(domain + " internal still accepts writes", wrapper.internalFor(domain) + .accepts(after, position, domain)); + assertFalse(domain + " write was accepted by the wrapper", wrapper.accepts(after, position, domain)); + } + } + } + + /** + * The constructor's preconditions are what make {@code internalFor} honest. Two internals holding one domain leave + * the other unroutable, and internals from different stores would put one table's rows in another's flush. + */ + @Test + public void wrapperRefusesInternalsThatCannotCoverBothDomains() + { + ColumnFamilyStore cfs = newTrackedTable(); + LogDomainBounds bounds = LogDomainBounds.atCurrentPositions(); + Memtable journalInternal = internal(cfs, bounds, LogDomain.MUTATION_JOURNAL); + + Assertions.assertThatThrownBy(() -> new SplitDomainMemtable(journalInternal, + internal(cfs, bounds, LogDomain.MUTATION_JOURNAL), + journalInternal.getMemtableId())) + .describedAs("two journal internals leave the commit log domain unroutable") + .isInstanceOf(IllegalArgumentException.class); + + ColumnFamilyStore otherTable = newTrackedTable(); + Assertions.assertThatThrownBy(() -> new SplitDomainMemtable(internal(otherTable, bounds, LogDomain.COMMIT_LOG), + journalInternal, + journalInternal.getMemtableId())) + .describedAs("internals must belong to the same store") + .isInstanceOf(IllegalArgumentException.class); + } + + // An unsplit memtable refuses a write from the other log, because it's bounds are incompatible + @Test + public void normalMemtableRefusesForeignDomainWrite() + { + ColumnFamilyStore cfs = newTrackedTable(); + Memtable journalInternal = internal(cfs, LogDomainBounds.atCurrentPositions(), LogDomain.MUTATION_JOURNAL); + + TableMetadata metadata = cfs.metadata(); + DecoratedKey key = metadata.partitioner.decorateKey(ByteBufferUtil.bytes(1)); + MutationId id = MutationTrackingService.instance().nextMutationId(metadata.keyspace, key.getToken()); + SimpleBuilders.MutationBuilder builder = new SimpleBuilders.MutationBuilder(id, metadata.keyspace, key); + builder.update(metadata).row().add("v", 1); + PartitionUpdate update = builder.build().getPartitionUpdate(metadata); + + Assertions.assertThatThrownBy(() -> { + try (OpOrder.Group group = Keyspace.writeOrder.start()) + { + journalInternal.put(id, update, UpdateTransaction.NO_OP, group, LogDomain.COMMIT_LOG); + } + }) + .describedAs("a commit-log write must not land in a journal memtable") + .isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("COMMIT_LOG"); + } + + // ---- Installing one, through Tracker.getMemtableFor -------------------------------------------------------- + + /** + * A tracked table's memtable refuses an untracked write. The memtable generation splits instead of the write + * failing, and keeps the memtable that was already live as one internal. + */ + @Test + public void foreignDomainWriteSplitsTheCurrentMemtableGeneration() + { + ColumnFamilyStore cfs = newTrackedTable(); + Memtable before = cfs.getTracker().getView().getCurrentMemtable(); + assertTrue(before.holds(LogDomain.MUTATION_JOURNAL)); + + try (OpOrder.Group group = Keyspace.writeOrder.start()) + { + Memtable selected = cfs.getTracker().getMemtableFor(group, + CommitLog.instance.getCurrentPosition(), + LogDomain.COMMIT_LOG); + assertTrue("selection returns the split generation, which routes on put", + selected instanceof SplitDomainMemtable); + assertTrue(((SplitDomainMemtable) selected).internalFor(LogDomain.COMMIT_LOG).holds(LogDomain.COMMIT_LOG)); + } + + Memtable after = cfs.getTracker().getView().getCurrentMemtable(); + assertTrue(after instanceof SplitDomainMemtable); + SplitDomainMemtable wrapper = (SplitDomainMemtable) after; + assertEquals("the memtable already live is kept as the journal internal", + before, wrapper.internalFor(LogDomain.MUTATION_JOURNAL)); + assertEquals("and the wrapper inherits its id, so generation ordering is unbroken", + before.getMemtableId(), wrapper.getMemtableId()); + } + + @Test + public void splitGenerationTakesEitherDomainAndRoutesInside() + { + // memtable should initially be a normal memtable + ColumnFamilyStore cfs = newTrackedTable(); + assertFalse(selectGenerationFor(cfs, LogDomain.MUTATION_JOURNAL) instanceof SplitDomainMemtable); + + //... but requesting a memtable should install a split domain memtable + Memtable installed = selectGenerationFor(cfs, LogDomain.COMMIT_LOG); + assertTrue(selectGenerationFor(cfs, LogDomain.COMMIT_LOG) instanceof SplitDomainMemtable); + + //... and continue returning it for either domain + Memtable forCommitLog = selectGenerationFor(cfs, LogDomain.COMMIT_LOG); + Memtable forJournal = selectGenerationFor(cfs, LogDomain.MUTATION_JOURNAL); + + assertEquals(forCommitLog, forJournal); + assertEquals(installed, forCommitLog); + assertEquals(forCommitLog, cfs.getTracker().getView().getCurrentMemtable()); + SplitDomainMemtable wrapper = (SplitDomainMemtable) forCommitLog; + + // writes against one domain shouldn't touch the other domain + write(cfs, wrapper, 1, LogDomain.MUTATION_JOURNAL); + assertFalse(wrapper.internalFor(LogDomain.MUTATION_JOURNAL).isClean()); + assertTrue("the commit-log internal must not have taken the journal write", + wrapper.internalFor(LogDomain.COMMIT_LOG).isClean()); + } + + /** + * Retires a generation for routing without flushing it. + */ + private static void retireForRouting(Memtable memtable) + { + OpOrder.Barrier barrier = Keyspace.writeOrder.newBarrier(); + LogDomainBounds upperBounds = LogDomainBounds.unset(); + upperBounds.seal(); + memtable.switchOut(barrier, upperBounds); + barrier.issue(); + } + + /** + * Prior to supporting multiple log domains, having no memtable that would accept an incoming write was an error. Now, + * it can mean that we have an illegal condition OR we need to split the current memtable. This tests that we still + * throw if none of the memtables will accept the write AND the current memtable holds the domain of the current write + * we need to split the current memtable. + */ + @Test + public void currentMemtableHoldsDomainRefusesThenThrows() + { + ColumnFamilyStore cfs = newTrackedTable(); + Memtable current = cfs.getTracker().getView().getCurrentMemtable(); + assertTrue(current.holds(LogDomain.MUTATION_JOURNAL)); + + // Retired without a replacement being appended, which no production path does - View.switchMemtable appends + // first. That leaves a current memtable carrying an issued barrier, so it refuses a write it holds the domain + // for. + retireForRouting(current); + + Assertions.assertThatThrownBy(() -> { + try (OpOrder.Group group = Keyspace.writeOrder.start()) + { + cfs.getTracker().getMemtableFor(group, MutationJournal.instance().getCurrentPosition(), + LogDomain.MUTATION_JOURNAL); + } + }) + .describedAs("the invariant must be reported, naming the domain the current memtable holds") + .isInstanceOf(AssertionError.class) + .hasMessageContaining("holds that domain"); + } + + // ---- Flushing one, through ColumnFamilyStore --------------------------------------------------------------- + + @Test + public void splitGenerationFlushesAnSSTablePerDomain() + { + // Can't call getFlushSet on SplitDomainMemtable directly, getFlushSources returns multiple domain memtables + Assertions.assertThatThrownBy(() -> newWrapper(newTrackedTable()).getFlushSet(null, null)) + .isInstanceOf(UnsupportedOperationException.class); + + assertFlushOutput(EnumSet.of(SSTableProvenance.MUTATION_JOURNAL, SSTableProvenance.COMMIT_LOG), + LogDomain.MUTATION_JOURNAL, LogDomain.COMMIT_LOG); + assertFlushOutput(EnumSet.of(SSTableProvenance.MUTATION_JOURNAL), + LogDomain.MUTATION_JOURNAL); + } + + private static void assertFlushOutput(Set expected, LogDomain... dirty) + { + ColumnFamilyStore cfs = newTrackedTable(); + + Memtable generation = selectGenerationFor(cfs, LogDomain.COMMIT_LOG); + assertTrue(generation instanceof SplitDomainMemtable); + int k = 1; + for (LogDomain domain : dirty) + write(cfs, generation, k++, domain); + assertFalse(generation.isClean()); + + cfs.forceBlockingFlush(ColumnFamilyStore.FlushReason.UNIT_TESTS); + + Map byProvenance = new EnumMap<>(SSTableProvenance.class); + for (SSTableReader sstable : cfs.getLiveSSTables()) + assertNull("two sstables from the same log", byProvenance.put(SSTableProvenance.of(sstable), sstable)); + + assertEquals("one sstable per dirty domain, and none claiming both logs", expected, byProvenance.keySet()); + assertFalse("the generation is gone from the view", cfs.getTracker().getView().liveMemtables.contains(generation)); + } + + private static Memtable dirtySplitGeneration(ColumnFamilyStore cfs) + { + Memtable generation = selectGenerationFor(cfs, LogDomain.COMMIT_LOG); + write(cfs, generation, 1, LogDomain.COMMIT_LOG); + write(cfs, generation, 2, LogDomain.MUTATION_JOURNAL); + return generation; + } + + /** + * For accord. Accord assumes a single active memtable and observes them directly when determining durability. + * If the listener fired once per domain memtable, accord would think a memtable was durable before both parts + * actually were + */ + @Test + public void splitDomainMemtableFiresFlushListenerOnce() + { + ColumnFamilyStore cfs = newTrackedTable(); + Memtable generation = dirtySplitGeneration(cfs); + + AtomicInteger fired = new AtomicInteger(); + AtomicInteger liveWhenFired = new AtomicInteger(-1); + Consumer registered = generation.ensureFlushListener("durability", () -> metadata -> { + fired.incrementAndGet(); + liveWhenFired.set(cfs.getLiveSSTables().size()); + }); + assertNotNull(registered); + assertSame("one listener per generation, not per registration", + registered, + generation.ensureFlushListener("durability", () -> { throw new AssertionError("built twice"); })); + + cfs.forceBlockingFlush(ColumnFamilyStore.FlushReason.UNIT_TESTS); + + assertEquals("fires exactly once", 1, fired.get()); + assertEquals("both domains' output must be live before it fires", 2, liveWhenFired.get()); + assertNull("a flushed generation refuses new listeners, which is what ensureDurable retries on", + generation.ensureFlushListener("later", () -> metadata -> {})); + } + + /** + * A flush failure must leave both logs alone, or a segment is marked clean for data that never reached disk. The two + * log calls and the listener share one {@code flushFailure == null} guard in {@code PostFlush}. + */ + @Test + public void flushFailureInOneDomainLeavesTheGenerationUndurable() + { + ColumnFamilyStore cfs = newTrackedTable(); + Memtable generation = dirtySplitGeneration(cfs); + + AtomicInteger fired = new AtomicInteger(); + generation.ensureFlushListener("durability", () -> metadata -> fired.incrementAndGet()); + + // Flushing.flushRunnables refuses a memtable that already holds a flush transaction, so the journal internal + // fails where a writer error would: inside Flush.flushMemtable, after the barrier has issued. + Memtable journalInternal = ((SplitDomainMemtable) generation).internalFor(LogDomain.MUTATION_JOURNAL); + journalInternal.setFlushTransaction(LifecycleTransaction.offline(OperationType.FLUSH)); + + try + { + cfs.forceBlockingFlush(ColumnFamilyStore.FlushReason.UNIT_TESTS); + org.junit.Assert.fail("expected the flush to fail"); + } + catch (RuntimeException expected) + { + // the failure is rethrown by PostFlush + } + + // A failed flush must leave the generation undurable and publish nothing. + assertEquals(0, fired.get()); + assertEquals(0, cfs.getLiveSSTables().size()); + } + + /** + * Checks that the correct log is notified on flush, even if the schema has moved onto a different domain. + */ + @Test + public void originatingLogIsNotifiedOnMemtableFlush() + { + ColumnFamilyStore cfs = newTrackedTable(); + assertTrue(cfs.metadata().replicationType().isTracked()); + + // Split first, so the commit-log internal's lower bound is below the append that follows. + Memtable generation = selectGenerationFor(cfs, LogDomain.COMMIT_LOG); + assertTrue(generation instanceof SplitDomainMemtable); + + CommitLog.instance.add(untrackedMutation(cfs, 1)); + assertTrue(commitLogIsDirtyFor(cfs)); + + write(cfs, generation, 1, LogDomain.COMMIT_LOG); + cfs.forceBlockingFlush(ColumnFamilyStore.FlushReason.UNIT_TESTS); + + assertFalse(commitLogIsDirtyFor(cfs)); + } + + private static Mutation untrackedMutation(ColumnFamilyStore cfs, int k) + { + TableMetadata metadata = cfs.metadata(); + DecoratedKey key = metadata.partitioner.decorateKey(ByteBufferUtil.bytes(k)); + SimpleBuilders.MutationBuilder builder = new SimpleBuilders.MutationBuilder(MutationId.none(), metadata.keyspace, key); + builder.update(metadata).row().add("v", k); + return builder.build(); + } + + private static boolean commitLogIsDirtyFor(ColumnFamilyStore cfs) + { + for (CommitLogSegment segment : CommitLog.instance.segmentManager.getActiveSegments()) + if (segment.getDirtyTableIds().contains(cfs.metadata().id)) + return true; + return false; + } + + private static Memtable selectGenerationFor(ColumnFamilyStore cfs, LogDomain domain) + { + CommitLogPosition position = domain.isJournal() ? MutationJournal.instance().getCurrentPosition() + : CommitLog.instance.getCurrentPosition(); + try (OpOrder.Group group = Keyspace.writeOrder.start()) + { + return cfs.getTracker().getMemtableFor(group, position, domain); + } + } + +} diff --git a/test/unit/org/apache/cassandra/index/internal/TrackedIndexFlushTest.java b/test/unit/org/apache/cassandra/index/internal/TrackedIndexFlushTest.java new file mode 100644 index 000000000000..b3c292e15c83 --- /dev/null +++ b/test/unit/org/apache/cassandra/index/internal/TrackedIndexFlushTest.java @@ -0,0 +1,207 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.index.internal; + +import java.util.Collections; +import java.util.concurrent.atomic.AtomicInteger; + +import com.google.common.collect.Iterables; + +import org.junit.BeforeClass; +import org.junit.Test; + +import org.apache.cassandra.SchemaLoader; +import org.apache.cassandra.config.DatabaseDescriptor; +import org.apache.cassandra.cql3.ColumnIdentifier; +import org.apache.cassandra.cql3.statements.schema.IndexTarget; +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.Keyspace; +import org.apache.cassandra.db.LogDomain; +import org.apache.cassandra.db.Mutation; +import org.apache.cassandra.db.SimpleBuilders; +import org.apache.cassandra.db.commitlog.CommitLogPosition; +import org.apache.cassandra.db.commitlog.IntervalSet; +import org.apache.cassandra.db.compaction.CompactionManager; +import org.apache.cassandra.db.marshal.Int32Type; +import org.apache.cassandra.db.memtable.Memtable; +import org.apache.cassandra.db.partitions.PartitionUpdate; +import org.apache.cassandra.io.sstable.format.SSTableReader; +import org.apache.cassandra.replication.MutationId; +import org.apache.cassandra.replication.MutationJournal; +import org.apache.cassandra.replication.MutationTrackingService; +import org.apache.cassandra.schema.IndexMetadata; +import org.apache.cassandra.schema.Indexes; +import org.apache.cassandra.schema.KeyspaceParams; +import org.apache.cassandra.schema.ReplicationType; +import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.utils.ByteBufferUtil; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +public class TrackedIndexFlushTest +{ + private static final AtomicInteger keyspaceNumber = new AtomicInteger(); + private static final String INDEX_NAME = "tbl_v_index"; + + static + { + DatabaseDescriptor.daemonInitialization(); + } + + @BeforeClass + public static void setupClass() + { + SchemaLoader.prepareServer(); + MutationJournal.start(); + MutationTrackingService.start(); + } + + private static ColumnFamilyStore newTableWithIndex(ReplicationType replicationType) + { + String ks = "tracked_index_flush_" + keyspaceNumber.incrementAndGet(); + + TableMetadata.Builder builder = + TableMetadata.builder(ks, "tbl") + .addPartitionKeyColumn("k", Int32Type.instance) + .addRegularColumn("v", Int32Type.instance); + + builder.indexes(Indexes.of(IndexMetadata.fromIndexTargets( + Collections.singletonList(new IndexTarget(new ColumnIdentifier("v", true), IndexTarget.Type.VALUES)), + INDEX_NAME, + IndexMetadata.Kind.COMPOSITES, + Collections.emptyMap()))); + + SchemaLoader.createKeyspace(ks, KeyspaceParams.simple(1, replicationType), builder); + + ColumnFamilyStore cfs = Keyspace.open(ks).getColumnFamilyStore("tbl"); + cfs.disableAutoCompaction(); + indexStore(cfs).disableAutoCompaction(); + return cfs; + } + + private static ColumnFamilyStore newTrackedTableWithIndex() + { + return newTableWithIndex(ReplicationType.tracked); + } + + private static ColumnFamilyStore newUntrackedTableWithIndex() + { + return newTableWithIndex(ReplicationType.untracked); + } + + private static ColumnFamilyStore indexStore(ColumnFamilyStore baseCfs) + { + return Iterables.getOnlyElement(baseCfs.indexManager.getAllIndexColumnFamilyStores()); + } + + private static ColumnFamilyStore newTrackedTableWithFlushedRow() + { + ColumnFamilyStore cfs = newTrackedTableWithIndex(); + write(cfs, 1, 1); + cfs.forceBlockingFlush(ColumnFamilyStore.FlushReason.UNIT_TESTS); + return cfs; + } + + private static void write(ColumnFamilyStore cfs, int k, int v) + { + TableMetadata metadata = cfs.metadata(); + DecoratedKey key = metadata.partitioner.decorateKey(ByteBufferUtil.bytes(k)); + MutationId id = MutationTrackingService.instance().nextMutationId(metadata.keyspace, key.getToken()); + SimpleBuilders.MutationBuilder builder = new SimpleBuilders.MutationBuilder(id, metadata.keyspace, key); + PartitionUpdate.SimpleBuilder partition = builder.update(metadata); + partition.row().add("v", v); + Mutation mutation = builder.build(); + assertFalse(mutation.id().isNone()); + mutation.apply(); + } + + @Test + public void untrackedIndexMemtableDomain() + { + ColumnFamilyStore cfs = newUntrackedTableWithIndex(); + + Memtable index = indexStore(cfs).getTracker().getView().getCurrentMemtable(); + + assertTrue(index.holds(LogDomain.COMMIT_LOG)); + assertFalse(index.holds(LogDomain.MUTATION_JOURNAL)); + } + + @Test + public void trackedIndexMemtableDomain() + { + ColumnFamilyStore cfs = newTrackedTableWithIndex(); + + Memtable index = indexStore(cfs).getTracker().getView().getCurrentMemtable(); + + assertTrue(index.holds(LogDomain.MUTATION_JOURNAL)); + assertFalse(index.holds(LogDomain.COMMIT_LOG)); + } + + @Test + public void rebuildingAnIndexOnATrackedTableSucceeds() + { + ColumnFamilyStore cfs = newTrackedTableWithFlushedRow(); + ColumnFamilyStore index = indexStore(cfs); + assertFalse(cfs.getLiveSSTables().isEmpty()); + + cfs.indexManager.rebuildIndexesBlocking(Collections.singleton(INDEX_NAME)); + + int withSpan = 0; + int withoutSpan = 0; + for (SSTableReader sstable : index.getLiveSSTables()) + { + IntervalSet intervals = sstable.getSSTableMetadata().commitLogIntervals; + if (intervals.isEmpty()) + { + withoutSpan++; + continue; + } + withSpan++; + for (CommitLogPosition start : intervals.starts()) + assertTrue(start.compareTo(intervals.upperBound().orElseThrow(AssertionError::new)) <= 0); + } + assertEquals(1, withoutSpan); + assertEquals(1, withSpan); + } + + @Test + public void trackedIndexSSTableContainsNoOffsets() + { + ColumnFamilyStore cfs = newTrackedTableWithFlushedRow(); + ColumnFamilyStore index = indexStore(cfs); + + write(cfs, 2, 2); + cfs.forceBlockingFlush(ColumnFamilyStore.FlushReason.UNIT_TESTS); + assertEquals("precondition: two index sstables to union intervals over", 2, index.getLiveSSTables().size()); + for (SSTableReader input : index.getLiveSSTables()) + { + assertTrue(input.getSSTableMetadata().commitLogIntervals.isEmpty()); + assertTrue(input.getSSTableMetadata().coordinatorLogOffsets.isEmpty()); + } + + CompactionManager.instance.performMaximal(index); + + SSTableReader compacted = Iterables.getOnlyElement(index.getLiveSSTables()); + assertTrue(compacted.getSSTableMetadata().commitLogIntervals.isEmpty()); + assertTrue(compacted.getSSTableMetadata().coordinatorLogOffsets.isEmpty()); + } +} diff --git a/test/unit/org/apache/cassandra/io/sstable/SSTableProvenance.java b/test/unit/org/apache/cassandra/io/sstable/SSTableProvenance.java new file mode 100644 index 000000000000..4c626e633c95 --- /dev/null +++ b/test/unit/org/apache/cassandra/io/sstable/SSTableProvenance.java @@ -0,0 +1,52 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.io.sstable; + +import org.apache.cassandra.io.sstable.format.SSTableReader; +import org.apache.cassandra.io.sstable.metadata.StatsMetadata; + +/** + * Convenience enum for testing sstable metadata indicating if it came from the journal, the commit log, or both + */ +public enum SSTableProvenance +{ + MUTATION_JOURNAL, + COMMIT_LOG, + INDETERMINATE, + BOTH; + + public static SSTableProvenance of(SSTableReader sstable) + { + return of(sstable.getSSTableMetadata()); + } + + public static SSTableProvenance of(StatsMetadata metadata) + { + boolean journal = !metadata.coordinatorLogOffsets.isEmpty(); + boolean commitLog = !metadata.commitLogIntervals.isEmpty(); + + if (journal && commitLog) + return BOTH; + if (journal) + return MUTATION_JOURNAL; + if (commitLog) + return COMMIT_LOG; + return INDETERMINATE; + } +} diff --git a/test/unit/org/apache/cassandra/replication/ShortMutationIdTest.java b/test/unit/org/apache/cassandra/replication/ShortMutationIdTest.java new file mode 100644 index 000000000000..ab5d249d0a8b --- /dev/null +++ b/test/unit/org/apache/cassandra/replication/ShortMutationIdTest.java @@ -0,0 +1,79 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.cassandra.replication; + +import org.junit.Test; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + +public class ShortMutationIdTest +{ + private static ShortMutationId id(int hostId, int hostLogId, int offset) + { + return new ShortMutationId(CoordinatorLogId.asLong(hostId, hostLogId), offset); + } + + @Test + public void testComparison() + { + assertTrue(id(1, 0, 0).compareTo(id(2, 0, 0)) < 0); + assertTrue(id(1, 0, 0).compareTo(id(1, 1, 0)) < 0); + assertTrue(id(1, 1, 0).compareTo(id(1, 1, 1)) < 0); + + // host id outranks the components below it + assertTrue(id(2, 0, 0).compareTo(id(1, 9, 9)) > 0); + assertTrue(id(1, 2, 0).compareTo(id(1, 1, 9)) > 0); + } + + @Test + public void compareMatchesEqual() + { + MutationId early = new MutationId(CoordinatorLogId.asLong(1, 2), 3, 100); + MutationId late = new MutationId(CoordinatorLogId.asLong(1, 2), 3, 200); + + ShortMutationId[][] equalPairs = { { id(1, 2, 3), id(1, 2, 3) }, { early, late } }; + for (ShortMutationId[] pair : equalPairs) + { + assertEquals(pair[0], pair[1]); + assertEquals(0, pair[0].compareTo(pair[1])); + assertEquals(pair[0].hashCode(), pair[1].hashCode()); + } + + for (ShortMutationId other : new ShortMutationId[]{ id(9, 2, 3), id(1, 9, 3), id(1, 2, 9) }) + { + assertNotEquals(id(1, 2, 3), other); + assertNotEquals(0, id(1, 2, 3).compareTo(other)); + } + } + + @Test + public void convertingFromMutationIdPreservesTheId() + { + MutationId full = new MutationId(CoordinatorLogId.asLong(1, 2), 3, 100); + ShortMutationId shortened = new ShortMutationId(full); + + assertEquals("host id and host log id must not be transposed", full.hostId(), shortened.hostId()); + assertEquals(full.hostLogId(), shortened.hostLogId()); + assertEquals(full.logId(), shortened.logId()); + assertEquals(full.offset(), shortened.offset()); + assertEquals(full, shortened); + } +} diff --git a/test/unit/org/apache/cassandra/schema/MockSchema.java b/test/unit/org/apache/cassandra/schema/MockSchema.java index fc9e417dfa86..868f9c1088fc 100644 --- a/test/unit/org/apache/cassandra/schema/MockSchema.java +++ b/test/unit/org/apache/cassandra/schema/MockSchema.java @@ -27,6 +27,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; import java.util.function.Function; import java.util.function.Supplier; @@ -44,7 +45,9 @@ import org.apache.cassandra.db.DeletionTime; import org.apache.cassandra.db.Directories; import org.apache.cassandra.db.Keyspace; +import org.apache.cassandra.db.LogDomain; import org.apache.cassandra.db.SerializationHeader; +import org.apache.cassandra.db.commitlog.CommitLogPosition; import org.apache.cassandra.db.marshal.UTF8Type; import org.apache.cassandra.db.memtable.Memtable; import org.apache.cassandra.db.memtable.SkipListMemtable; @@ -121,9 +124,10 @@ public static SSTableId sstableId(int idx) public static final IndexSummary indexSummary; + /** Mock tables are untracked, so every memtable here is commit-log domain and bounded at {@code NONE}. */ public static Memtable memtable(ColumnFamilyStore cfs) { - return SkipListMemtable.FACTORY.create(null, cfs.metadata, cfs); + return SkipListMemtable.FACTORY.create(new AtomicReference<>(CommitLogPosition.NONE), cfs.metadata, cfs, LogDomain.COMMIT_LOG); } public static SSTableReader sstable(int generation, ColumnFamilyStore cfs)