From 19be8aeac2bdf4a0002da289600aefc4e0fb9b8d Mon Sep 17 00:00:00 2001 From: Blake Eggleston Date: Thu, 9 Apr 2026 10:04:24 -0700 Subject: [PATCH] CEP-45: Add mutation tracking support for secondary index reads This patch enables mutation tracking for queries using secondary indexes (both legacy 2i and SAI). Key changes include the addition of the MultiStepSearcher interface, which allows existing index implementations to iterate over matches and filter augmented data, proper Memtable snapshotting, and the integration of these in PartialTrackedIndexRead. patch by Blake Eggleston; reviewed by Aleksey Yeschenko and Caleb Rackliffe for CASSANDRA-20374 Co-authored-by: Blake Eggleston Co-authored-by: Caleb Rackliffe Co-authored-by: Aleksey Yeschenko --- .../cql3/statements/SelectStatement.java | 3 +- .../cassandra/db/ColumnFamilyStore.java | 15 +- .../db/PartitionRangeReadCommand.java | 3 +- .../org/apache/cassandra/db/ReadCommand.java | 88 +- .../org/apache/cassandra/db/ReadableView.java | 30 + .../db/SinglePartitionReadCommand.java | 58 +- .../cassandra/db/memtable/Memtable.java | 13 +- .../db/memtable/ShardedSkipListMemtable.java | 9 +- .../db/memtable/SkipListMemtable.java | 9 +- .../cassandra/db/memtable/TrieMemtable.java | 23 +- .../db/partitions/AtomicBTreePartition.java | 9 +- .../partitions/ImmutableBTreePartition.java | 18 +- .../UnfilteredPartitionIterators.java | 12 + .../org/apache/cassandra/index/Index.java | 74 ++ .../index/internal/CassandraIndex.java | 302 +++--- .../internal/CassandraIndexSearcher.java | 196 +++- .../cassandra/index/internal/IndexEntry.java | 47 +- .../composites/ClusteringColumnIndex.java | 2 +- .../composites/CollectionKeyIndexBase.java | 2 +- .../composites/CollectionValueIndex.java | 2 +- .../composites/CompositesSearcher.java | 249 +++-- .../composites/PartitionKeyIndex.java | 2 +- .../composites/RegularColumnIndex.java | 2 +- .../index/internal/keys/KeysSearcher.java | 175 ++-- .../cassandra/index/sai/plan/FilterTree.java | 13 +- .../index/sai/plan/QueryController.java | 12 +- .../plan/StorageAttachedIndexSearcher.java | 833 +++++++++++------ .../cassandra/index/sai/utils/PrimaryKey.java | 9 +- .../replication/BroadcastLogOffsets.java | 8 +- .../cassandra/schema/ReplicationType.java | 6 - .../tracked/AbstractPartialTrackedRead.java | 266 ------ .../reads/tracked/ExtendingCompletedRead.java | 92 +- .../reads/tracked/FilteredFollowupRead.java | 2 +- .../tracked/PartialTrackedIndexRead.java | 863 ++++++++++++++++++ .../tracked/PartialTrackedRangeRead.java | 52 +- .../reads/tracked/PartialTrackedRead.java | 310 ++++++- .../PartialTrackedSinglePartitionRead.java | 7 +- .../reads/tracked/TrackedLocalReads.java | 93 +- .../service/reads/tracked/TrackedRead.java | 35 +- .../cassandra/utils/AbstractIterator.java | 5 +- .../utils/CloseablePeekingIterator.java | 40 + ...eadRepairEmptyRangeTombstonesTestBase.java | 38 +- .../distributed/test/ReadRepairIndexTest.java | 227 +++++ .../test/ReadRepairQueryTester.java | 99 +- .../distributed/test/ReadRepairTester.java | 5 +- ...NodeTableWalkWithMutationTrackingTest.java | 22 +- .../test/cql3/SingleNodeTableWalkTest.java | 67 +- .../test/sai/PartialUpdateHandlingTest.java | 98 +- .../test/sai/StrictFilteringTest.java | 36 +- .../MutationTrackingPartitionReadTest.java | 218 +++++ .../sai/MultiNodeSAIMutationTrackingTest.java | 49 + .../fuzz/sai/SingleNodeSAITestBase.java | 21 +- .../cassandra/db/ColumnFamilyStoreTest.java | 7 + 53 files changed, 3556 insertions(+), 1320 deletions(-) create mode 100644 src/java/org/apache/cassandra/db/ReadableView.java delete mode 100644 src/java/org/apache/cassandra/service/reads/tracked/AbstractPartialTrackedRead.java create mode 100644 src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedIndexRead.java create mode 100644 src/java/org/apache/cassandra/utils/CloseablePeekingIterator.java create mode 100644 test/distributed/org/apache/cassandra/distributed/test/ReadRepairIndexTest.java create mode 100644 test/distributed/org/apache/cassandra/distributed/test/tracking/MutationTrackingPartitionReadTest.java create mode 100644 test/distributed/org/apache/cassandra/fuzz/sai/MultiNodeSAIMutationTrackingTest.java diff --git a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java index 1752fe0bc4..95ac721366 100644 --- a/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java +++ b/src/java/org/apache/cassandra/cql3/statements/SelectStatement.java @@ -116,6 +116,7 @@ import org.apache.cassandra.index.Index; import org.apache.cassandra.index.IndexRegistry; import org.apache.cassandra.metrics.ClientRequestSizeMetrics; import org.apache.cassandra.schema.ColumnMetadata; +import org.apache.cassandra.schema.ReplicationType; import org.apache.cassandra.schema.Schema; import org.apache.cassandra.schema.SchemaConstants; import org.apache.cassandra.schema.TableMetadata; @@ -483,7 +484,7 @@ public class SelectStatement implements CQLStatement.SingleKeyspaceCqlStatement, return getRangeCommand(options, state, columnFilter, rowFilter, limit, nowInSec, potentialTxnConflicts); } - if (restrictions.usesSecondaryIndexing() && !rowFilter.isStrict()) + if (restrictions.usesSecondaryIndexing() && !rowFilter.isStrict() && table.keyspaceReplicationType != ReplicationType.tracked) return getRangeCommand(options, state, columnFilter, rowFilter, limit, nowInSec, potentialTxnConflicts); return getSliceCommands(options, state, columnFilter, rowFilter, limit, nowInSec, potentialTxnConflicts); diff --git a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java index 851711a128..d7488a36d6 100644 --- a/src/java/org/apache/cassandra/db/ColumnFamilyStore.java +++ b/src/java/org/apache/cassandra/db/ColumnFamilyStore.java @@ -101,6 +101,7 @@ import org.apache.cassandra.db.partitions.CachedPartition; import org.apache.cassandra.db.partitions.PartitionUpdate; import org.apache.cassandra.db.repair.CassandraTableRepairManager; import org.apache.cassandra.db.rows.CellPath; +import org.apache.cassandra.db.rows.UnfilteredSource; import org.apache.cassandra.db.streaming.CassandraStreamManager; import org.apache.cassandra.db.view.TableViews; import org.apache.cassandra.dht.AbstractBounds; @@ -3098,7 +3099,7 @@ public T withAllSSTables(final OperationType operationType, Function sstables; public final Iterable memtables; @@ -3108,6 +3109,18 @@ public T withAllSSTables(final OperationType operationType, Function memtables() + { + return memtables; + } + + @Override + public List sstables() + { + return sstables; + } } public static class RefViewFragment extends ViewFragment implements AutoCloseable diff --git a/src/java/org/apache/cassandra/db/PartitionRangeReadCommand.java b/src/java/org/apache/cassandra/db/PartitionRangeReadCommand.java index 8028b92900..962065c3a6 100644 --- a/src/java/org/apache/cassandra/db/PartitionRangeReadCommand.java +++ b/src/java/org/apache/cassandra/db/PartitionRangeReadCommand.java @@ -449,11 +449,10 @@ public class PartitionRangeReadCommand extends ReadCommand implements PartitionR @Override protected PartialTrackedRead createInProgressRead(UnfilteredPartitionIterator iterator, ReadExecutionController executionController, - Index.Searcher searcher, ColumnFamilyStore cfs, long startTimeNanos) { - return PartialTrackedRangeRead.create(executionController, searcher, cfs, startTimeNanos, this, iterator); + return PartialTrackedRangeRead.create(executionController, null, cfs, startTimeNanos, this, iterator); } @Override diff --git a/src/java/org/apache/cassandra/db/ReadCommand.java b/src/java/org/apache/cassandra/db/ReadCommand.java index 673dea71be..12917e398a 100644 --- a/src/java/org/apache/cassandra/db/ReadCommand.java +++ b/src/java/org/apache/cassandra/db/ReadCommand.java @@ -130,7 +130,42 @@ public abstract class ReadCommand extends AbstractReadQuery { private interface ReadCompleter { - T complete(UnfilteredPartitionIterator iterator, ReadExecutionController executionController, Index.Searcher searcher, ColumnFamilyStore cfs, long startTimeNanos); + T complete(ReadCommand command, UnfilteredPartitionIterator iterator, ReadExecutionController executionController, ColumnFamilyStore cfs, long startTimeNanos); + T complete(ReadCommand command, Index.Searcher searcher, ReadExecutionController executionController, ColumnFamilyStore cfs, long startTimeNanos); + + ReadCompleter IMMEDIATE = new ReadCompleter<>() + { + @Override + public UnfilteredPartitionIterator complete(ReadCommand command, UnfilteredPartitionIterator iterator, ReadExecutionController executionController, ColumnFamilyStore cfs, long startTimeNanos) + { + return command.completeRead(iterator, executionController, null, cfs, startTimeNanos); + } + + @Override + public UnfilteredPartitionIterator complete(ReadCommand command, Index.Searcher searcher, ReadExecutionController executionController, ColumnFamilyStore cfs, long startTimeNanos) + { + UnfilteredPartitionIterator iterator = searcher.search(executionController); + return command.completeRead(iterator, executionController, searcher, cfs, startTimeNanos); + } + }; + + ReadCompleter TRACKED = new ReadCompleter<>() + { + @Override + public PartialTrackedRead complete(ReadCommand command, UnfilteredPartitionIterator iterator, ReadExecutionController executionController, ColumnFamilyStore cfs, long startTimeNanos) + { + return command.createInProgressRead(iterator, executionController, cfs, startTimeNanos); + } + + @Override + public PartialTrackedRead complete(ReadCommand command, Index.Searcher searcher, ReadExecutionController executionController, ColumnFamilyStore cfs, long startTimeNanos) + { + if (!searcher.isMultiStep()) + throw new IllegalStateException("Cannot use " + searcher.getClass().getName() + " with tracked reads"); + + return searcher.asMultiStep().beginRead(executionController, cfs, startTimeNanos); + } + }; } private static final int TEST_ITERATION_DELAY_MILLIS = CassandraRelevantProperties.TEST_READ_ITERATION_DELAY_MS.getInt(); @@ -483,28 +518,26 @@ public abstract class ReadCommand extends AbstractReadQuery ConsensusRequestRouter.validateSafeToReadNonTransactionally(this, cm); Index.QueryPlan indexQueryPlan = indexQueryPlan(); - Index.Searcher searcher = null; if (indexQueryPlan != null) { cfs.indexManager.checkQueryability(indexQueryPlan); - searcher = indexQueryPlan.searcherFor(this); - Tracing.trace("Executing read on {}.{} using index{} {}", - cfs.metadata.keyspace, - cfs.metadata.name, - indexQueryPlan.getIndexes().size() == 1 ? "" : "es", - indexQueryPlan.getIndexes() - .stream() - .map(i -> i.getIndexMetadata().name) - .collect(Collectors.joining(","))); + if (Tracing.isTracing()) + { + Tracing.trace("Executing read on {}.{} using index{} {}", + cfs.metadata.keyspace, + cfs.metadata.name, + indexQueryPlan.getIndexes().size() == 1 ? "" : "es", + indexQueryPlan.getIndexes() + .stream() + .map(i -> i.getIndexMetadata().name) + .collect(Collectors.joining(","))); + } + Index.Searcher searcher = indexQueryPlan.searcherFor(this); + return completer.complete(this, searcher, executionController, cfs, startTimeNanos); } - - if (searcher != null && metadata().replicationType().isTracked()) - throw new UnsupportedOperationException("TODO: support tracked index reads"); - - UnfilteredPartitionIterator iterator = (null == searcher) ? queryStorage(cfs, executionController) : searcher.search(executionController); - - return completer.complete(iterator, executionController, searcher, cfs, startTimeNanos); + UnfilteredPartitionIterator iterator = queryStorage(cfs, executionController); + return completer.complete(this, iterator, executionController, cfs, startTimeNanos); } finally { @@ -584,13 +617,12 @@ public abstract class ReadCommand extends AbstractReadQuery */ protected abstract PartialTrackedRead createInProgressRead(UnfilteredPartitionIterator iterator, ReadExecutionController executionController, - Index.Searcher searcher, ColumnFamilyStore cfs, long startTimeNanos); public PartialTrackedRead beginTrackedRead(ReadExecutionController executionController) { - return beginRead(executionController, null, this::createInProgressRead); + return beginRead(executionController, null, ReadCompleter.TRACKED); } public UnfilteredPartitionIterator completeTrackedRead(UnfilteredPartitionIterator iterator, PartialTrackedRead read) @@ -608,12 +640,12 @@ public abstract class ReadCommand extends AbstractReadQuery // iterators created inside the try as long as we do close the original resultIterator), or by closing the result. public UnfilteredPartitionIterator executeLocally(ReadExecutionController executionController) { - return beginRead(executionController, null, this::completeRead); + return beginRead(executionController, null, ReadCompleter.IMMEDIATE); } public UnfilteredPartitionIterator executeLocally(ReadExecutionController executionController, @Nullable ClusterMetadata cm) { - return beginRead(executionController, cm, this::completeRead); + return beginRead(executionController, cm, ReadCompleter.IMMEDIATE); } protected abstract void recordLatency(TableMetrics metric, long latencyNanos); @@ -1148,7 +1180,7 @@ public abstract class ReadCommand extends AbstractReadQuery return toCQLString(); } - InputCollector iteratorsForPartition(ColumnFamilyStore.ViewFragment view, ReadExecutionController controller) + InputCollector iteratorsForPartition(ReadableView view, ReadExecutionController controller) { final BiFunction, RepairedDataInfo, UnfilteredRowIterator> merge = (unfilteredRowIterators, repairedDataInfo) -> { @@ -1198,7 +1230,7 @@ public abstract class ReadCommand extends AbstractReadQuery List repairedIters; List unrepairedIters; - InputCollector(ColumnFamilyStore.ViewFragment view, + InputCollector(ReadableView view, ReadExecutionController controller, BiFunction, RepairedDataInfo, T> repairedMerger, Function postLimitAdditionalPartitions) @@ -1208,12 +1240,12 @@ public abstract class ReadCommand extends AbstractReadQuery if (isTrackingRepairedStatus) { - for (SSTableReader sstable : view.sstables) + for (SSTableReader sstable : view.sstables()) { if (considerRepairedForTracking(sstable)) { if (repairedSSTables == null) - repairedSSTables = Sets.newHashSetWithExpectedSize(view.sstables.size()); + repairedSSTables = Sets.newHashSetWithExpectedSize(view.sstables().size()); repairedSSTables.add(sstable); } } @@ -1221,14 +1253,14 @@ public abstract class ReadCommand extends AbstractReadQuery if (repairedSSTables == null) { repairedIters = Collections.emptyList(); - unrepairedIters = new ArrayList<>(view.sstables.size()); + unrepairedIters = new ArrayList<>(view.sstables().size()); } else { repairedIters = new ArrayList<>(repairedSSTables.size()); // when we're done collating, we'll merge the repaired iters and add the // result to the unrepaired list, so size that list accordingly - unrepairedIters = new ArrayList<>((view.sstables.size() - repairedSSTables.size()) + Iterables.size(view.memtables) + 1); + unrepairedIters = new ArrayList<>((view.sstables().size() - repairedSSTables.size()) + Iterables.size(view.memtables()) + 1); } this.repairedMerger = repairedMerger; this.postLimitAdditionalPartitions = postLimitAdditionalPartitions; diff --git a/src/java/org/apache/cassandra/db/ReadableView.java b/src/java/org/apache/cassandra/db/ReadableView.java new file mode 100644 index 0000000000..d68335a2d4 --- /dev/null +++ b/src/java/org/apache/cassandra/db/ReadableView.java @@ -0,0 +1,30 @@ +/* + * 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 java.util.List; + +import org.apache.cassandra.db.rows.UnfilteredSource; +import org.apache.cassandra.io.sstable.format.SSTableReader; + +public interface ReadableView +{ + Iterable memtables(); + List sstables(); +} diff --git a/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java b/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java index 59cd5b6db7..ed2fbdc645 100644 --- a/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java +++ b/src/java/org/apache/cassandra/db/SinglePartitionReadCommand.java @@ -65,6 +65,7 @@ import org.apache.cassandra.db.rows.Unfiltered; import org.apache.cassandra.db.rows.UnfilteredRowIterator; import org.apache.cassandra.db.rows.UnfilteredRowIteratorWithLowerBound; import org.apache.cassandra.db.rows.UnfilteredRowIterators; +import org.apache.cassandra.db.rows.UnfilteredSource; import org.apache.cassandra.db.rows.WrappingUnfilteredRowIterator; import org.apache.cassandra.db.transform.RTBoundValidator; import org.apache.cassandra.db.transform.Transformation; @@ -558,11 +559,10 @@ public class SinglePartitionReadCommand extends ReadCommand implements SinglePar @Override protected PartialTrackedRead createInProgressRead(UnfilteredPartitionIterator iterator, ReadExecutionController executionController, - Index.Searcher searcher, ColumnFamilyStore cfs, long startTimeNanos) { - return PartialTrackedSinglePartitionRead.create(executionController, searcher, cfs, startTimeNanos, this, iterator); + return PartialTrackedSinglePartitionRead.create(executionController, null, cfs, startTimeNanos, this, iterator); } /** @@ -726,29 +726,32 @@ public class SinglePartitionReadCommand extends ReadCommand implements SinglePar * Also note that one must have created a {@code ReadExecutionController} on the queried table and we require it as * a parameter to enforce that fact, even though it's not explicitlly used by the method. */ - public UnfilteredRowIterator queryMemtableAndDisk(ColumnFamilyStore cfs, ReadExecutionController executionController) + public UnfilteredRowIterator queryMemtableAndDisk(ReadableView view, ColumnFamilyStore cfs, ReadExecutionController executionController) { assert executionController != null && executionController.validForReadOn(cfs); Tracing.trace("Executing single-partition query on {}", cfs.name); - Tracing.trace("Acquiring sstable references"); - ColumnFamilyStore.ViewFragment view = cfs.select(View.select(SSTableSet.LIVE, partitionKey())); - return queryMemtableAndDiskInternal(cfs, view, null, executionController); + return queryMemtableAndDiskInternal(view, cfs, null, executionController); } - public UnfilteredRowIterator queryMemtableAndDisk(ColumnFamilyStore cfs, - ColumnFamilyStore.ViewFragment view, + public UnfilteredRowIterator queryMemtableAndDisk(ColumnFamilyStore cfs, ReadExecutionController executionController) + { + return queryMemtableAndDisk(cfs.select(View.select(SSTableSet.LIVE, partitionKey())), cfs, executionController); + } + + public UnfilteredRowIterator queryMemtableAndDisk(ReadableView view, + ColumnFamilyStore cfs, Function>> rowTransformer, ReadExecutionController executionController) { assert executionController != null && executionController.validForReadOn(cfs); Tracing.trace("Executing single-partition query on {}", cfs.name); - return queryMemtableAndDiskInternal(cfs, view, rowTransformer, executionController); + return queryMemtableAndDiskInternal(view, cfs, rowTransformer, executionController); } - private UnfilteredRowIterator queryMemtableAndDiskInternal(ColumnFamilyStore cfs, - ColumnFamilyStore.ViewFragment view, + private UnfilteredRowIterator queryMemtableAndDiskInternal(ReadableView view, + ColumnFamilyStore cfs, Function>> rowTransformer, ReadExecutionController controller) { @@ -774,10 +777,12 @@ public class SinglePartitionReadCommand extends ReadCommand implements SinglePar && !queriesMulticellType() && !controller.isTrackingRepairedStatus()) { - return queryMemtableAndSSTablesInTimestampOrder(cfs, view, rowTransformer, (ClusteringIndexNamesFilter)clusteringIndexFilter(), controller); + return queryMemtableAndSSTablesInTimestampOrder(view, cfs, rowTransformer, (ClusteringIndexNamesFilter)clusteringIndexFilter(), controller); } - view.sstables.sort(SSTableReader.maxTimestampDescending); + Tracing.trace("Acquiring sstable references"); + List sstables = view.sstables(); + sstables.sort(SSTableReader.maxTimestampDescending); ClusteringIndexFilter filter = clusteringIndexFilter(); long minTimestamp = Long.MAX_VALUE; long mostRecentPartitionTombstone = Long.MIN_VALUE; @@ -786,7 +791,7 @@ public class SinglePartitionReadCommand extends ReadCommand implements SinglePar { SSTableReadMetricsCollector metricsCollector = new SSTableReadMetricsCollector(); - for (Memtable memtable : view.memtables) + for (UnfilteredSource memtable : view.memtables()) { UnfilteredRowIterator iter = memtable.rowIterator(partitionKey(), filter.getSlices(metadata()), columnFilter(), filter.isReversed(), metricsCollector); if (iter == null) @@ -795,8 +800,8 @@ public class SinglePartitionReadCommand extends ReadCommand implements SinglePar if (memtable.getMinTimestamp() != Memtable.NO_MIN_TIMESTAMP) minTimestamp = Math.min(minTimestamp, memtable.getMinTimestamp()); - if (rowTransformer != null) - iter = Transformation.apply(iter, rowTransformer.apply(memtable)); + if (rowTransformer != null && memtable instanceof CellSourceIdentifier) + iter = Transformation.apply(iter, rowTransformer.apply((CellSourceIdentifier) memtable)); // Memtable data is always considered unrepaired controller.updateMinOldestUnrepairedTombstone(memtable.getMinLocalDeletionTime()); @@ -818,14 +823,14 @@ public class SinglePartitionReadCommand extends ReadCommand implements SinglePar * In other words, iterating in descending maxTimestamp order allow to do our mostRecentPartitionTombstone * elimination in one pass, and minimize the number of sstables for which we read a partition tombstone. */ - view.sstables.sort(SSTableReader.maxTimestampDescending); + sstables.sort(SSTableReader.maxTimestampDescending); int nonIntersectingSSTables = 0; int includedDueToTombstones = 0; if (controller.isTrackingRepairedStatus()) Tracing.trace("Collecting data from sstables and tracking repaired status"); - for (SSTableReader sstable : view.sstables) + for (SSTableReader sstable : sstables) { // if we've already seen a partition tombstone with a timestamp greater // than the most recent update to this sstable, we can skip it @@ -900,7 +905,7 @@ public class SinglePartitionReadCommand extends ReadCommand implements SinglePar if (Tracing.isTracing()) Tracing.trace("Skipped {}/{} non-slice-intersecting sstables, included {} due to tombstones", - nonIntersectingSSTables, view.sstables.size(), includedDueToTombstones); + nonIntersectingSSTables, sstables.size(), includedDueToTombstones); if (inputCollector.isEmpty()) return EmptyIterators.unfilteredRow(cfs.metadata(), partitionKey(), filter.isReversed()); @@ -1030,21 +1035,23 @@ public class SinglePartitionReadCommand extends ReadCommand implements SinglePar * no collection or counters are included). * This method assumes the filter is a {@code ClusteringIndexNamesFilter}. */ - private UnfilteredRowIterator queryMemtableAndSSTablesInTimestampOrder(ColumnFamilyStore cfs, ColumnFamilyStore.ViewFragment view, Function>> rowTransformer, ClusteringIndexNamesFilter filter, ReadExecutionController controller) + private UnfilteredRowIterator queryMemtableAndSSTablesInTimestampOrder(ReadableView view, ColumnFamilyStore cfs, Function>> rowTransformer, ClusteringIndexNamesFilter filter, ReadExecutionController controller) { + Tracing.trace("Acquiring sstable references"); ImmutableBTreePartition result = null; SSTableReadMetricsCollector metricsCollector = new SSTableReadMetricsCollector(); Tracing.trace("Merging memtable contents"); - for (Memtable memtable : view.memtables) + for (UnfilteredSource memtable : view.memtables()) { try (UnfilteredRowIterator iter = memtable.rowIterator(partitionKey, filter.getSlices(metadata()), columnFilter(), isReversed(), metricsCollector)) { if (iter == null) continue; - UnfilteredRowIterator wrapped = rowTransformer != null ? Transformation.apply(iter, rowTransformer.apply(memtable)) - : iter; + UnfilteredRowIterator wrapped = rowTransformer != null && memtable instanceof CellSourceIdentifier + ? Transformation.apply(iter, rowTransformer.apply((CellSourceIdentifier) memtable)) + : iter; result = add(RTBoundValidator.validate(wrapped, RTBoundValidator.Stage.MEMTABLE, false), result, filter, @@ -1054,9 +1061,10 @@ public class SinglePartitionReadCommand extends ReadCommand implements SinglePar } /* add the SSTables on disk */ - view.sstables.sort(SSTableReader.maxTimestampDescending); + List sstables = view.sstables(); + sstables.sort(SSTableReader.maxTimestampDescending); // read sorted sstables - for (SSTableReader sstable : view.sstables) + for (SSTableReader sstable : sstables) { // if we've already seen a partition tombstone with a timestamp greater // than the most recent update to this sstable, we're done, since the rest of the sstables diff --git a/src/java/org/apache/cassandra/db/memtable/Memtable.java b/src/java/org/apache/cassandra/db/memtable/Memtable.java index 35a2ebf0f5..bfa17535f8 100644 --- a/src/java/org/apache/cassandra/db/memtable/Memtable.java +++ b/src/java/org/apache/cassandra/db/memtable/Memtable.java @@ -26,6 +26,7 @@ import javax.annotation.concurrent.NotThreadSafe; import org.apache.cassandra.db.CellSourceIdentifier; import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.DecoratedKey; import org.apache.cassandra.db.PartitionPosition; import org.apache.cassandra.db.RegularAndStaticColumns; import org.apache.cassandra.db.commitlog.CommitLogPosition; @@ -206,7 +207,17 @@ public interface Memtable extends Comparable, UnfilteredSource, CellSo */ long put(MutationId mutationId, PartitionUpdate update, UpdateTransaction indexer, OpOrder.Group opGroup, boolean assumeMissing); - // Read operations are provided by the UnfilteredSource interface. + /** + * Creates a point-in-time snapshot of a partition in this memtable. + *

+ * This method returns an immutable view of the partition as it exists at the time of the call. + * The snapshot is isolated from subsequent writes to the memtable and can be safely read + * concurrently with ongoing mutations. + * + * @param key the partition key to snapshot + * @return an immutable snapshot of the partition, or {@code null} if the partition does not exist + */ + Partition snapshotPartition(DecoratedKey key); // Statistics diff --git a/src/java/org/apache/cassandra/db/memtable/ShardedSkipListMemtable.java b/src/java/org/apache/cassandra/db/memtable/ShardedSkipListMemtable.java index a803811d59..316ac8bfab 100644 --- a/src/java/org/apache/cassandra/db/memtable/ShardedSkipListMemtable.java +++ b/src/java/org/apache/cassandra/db/memtable/ShardedSkipListMemtable.java @@ -262,7 +262,7 @@ public class ShardedSkipListMemtable extends AbstractShardedMemtable return iterator; } - private Partition getPartition(DecoratedKey key) + private AtomicBTreePartition getPartition(DecoratedKey key) { int shardIndex = boundaries.getShardForKey(key); return shards[shardIndex].partitions.get(key); @@ -285,6 +285,13 @@ public class ShardedSkipListMemtable extends AbstractShardedMemtable return p != null ? p.unfilteredIterator() : null; } + @Override + public Partition snapshotPartition(DecoratedKey partitionKey) + { + AtomicBTreePartition partition = getPartition(partitionKey); + return partition == null ? null : partition.asImmutable(); + } + public FlushablePartitionSet getFlushSet(PartitionPosition from, PartitionPosition to) { long keySize = 0; diff --git a/src/java/org/apache/cassandra/db/memtable/SkipListMemtable.java b/src/java/org/apache/cassandra/db/memtable/SkipListMemtable.java index 24072f0a9a..acbc0bf65d 100644 --- a/src/java/org/apache/cassandra/db/memtable/SkipListMemtable.java +++ b/src/java/org/apache/cassandra/db/memtable/SkipListMemtable.java @@ -207,7 +207,7 @@ public class SkipListMemtable extends AbstractAllocatorMemtable } } - Partition getPartition(DecoratedKey key) + AtomicBTreePartition getPartition(DecoratedKey key) { return partitions.get(key); } @@ -229,6 +229,13 @@ public class SkipListMemtable extends AbstractAllocatorMemtable return p != null ? p.unfilteredIterator() : null; } + @Override + public Partition snapshotPartition(DecoratedKey partitionKey) + { + AtomicBTreePartition partition = getPartition(partitionKey); + return partition == null ? null : partition.asImmutable(); + } + private static int estimateRowOverhead(final int count) { // calculate row overhead diff --git a/src/java/org/apache/cassandra/db/memtable/TrieMemtable.java b/src/java/org/apache/cassandra/db/memtable/TrieMemtable.java index 1603c0b3bc..3ada16a75a 100644 --- a/src/java/org/apache/cassandra/db/memtable/TrieMemtable.java +++ b/src/java/org/apache/cassandra/db/memtable/TrieMemtable.java @@ -319,15 +319,26 @@ public class TrieMemtable extends AbstractShardedMemtable } private Partition getPartition(DecoratedKey key) + { + return getPartition(key, false); + } + + private Partition getPartition(DecoratedKey key, boolean canHaveShadowedData) { int shardIndex = boundaries.getShardForKey(key); BTreePartitionData data = shards[shardIndex].data.get(key); if (data != null) - return createPartition(metadata(), allocator.ensureOnHeap(), key, data); + return createPartition(metadata(), allocator.ensureOnHeap(), key, data, canHaveShadowedData); else return null; } + @Override + public Partition snapshotPartition(DecoratedKey partitionKey) + { + return getPartition(partitionKey, true); + } + @Override public UnfilteredRowIterator rowIterator(DecoratedKey key, Slices slices, ColumnFilter selectedColumns, boolean reversed, SSTableReadsListener listener) { @@ -345,9 +356,9 @@ public class TrieMemtable extends AbstractShardedMemtable return p != null ? p.unfilteredIterator() : null; } - private static MemtablePartition createPartition(TableMetadata metadata, EnsureOnHeap ensureOnHeap, DecoratedKey key, BTreePartitionData data) + private static MemtablePartition createPartition(TableMetadata metadata, EnsureOnHeap ensureOnHeap, DecoratedKey key, BTreePartitionData data, boolean canHaveShadowedData) { - return new MemtablePartition(metadata, ensureOnHeap, key, data); + return new MemtablePartition(metadata, ensureOnHeap, key, data, canHaveShadowedData); } private static MemtablePartition getPartitionFromTrieEntry(TableMetadata metadata, EnsureOnHeap ensureOnHeap, Map.Entry en) @@ -355,7 +366,7 @@ public class TrieMemtable extends AbstractShardedMemtable DecoratedKey key = BufferDecoratedKey.fromByteComparable(en.getKey(), BYTE_COMPARABLE_VERSION, metadata.partitioner); - return createPartition(metadata, ensureOnHeap, key, en.getValue()); + return createPartition(metadata, ensureOnHeap, key, en.getValue(), false); } @@ -696,9 +707,9 @@ public class TrieMemtable extends AbstractShardedMemtable private final EnsureOnHeap ensureOnHeap; - private MemtablePartition(TableMetadata table, EnsureOnHeap ensureOnHeap, DecoratedKey key, BTreePartitionData data) + private MemtablePartition(TableMetadata table, EnsureOnHeap ensureOnHeap, DecoratedKey key, BTreePartitionData data, boolean canHaveShadowedData) { - super(table, key, data); + super(table, key, data, canHaveShadowedData); this.ensureOnHeap = ensureOnHeap; } diff --git a/src/java/org/apache/cassandra/db/partitions/AtomicBTreePartition.java b/src/java/org/apache/cassandra/db/partitions/AtomicBTreePartition.java index b90ebccdff..505c0ac3d9 100644 --- a/src/java/org/apache/cassandra/db/partitions/AtomicBTreePartition.java +++ b/src/java/org/apache/cassandra/db/partitions/AtomicBTreePartition.java @@ -46,7 +46,7 @@ import static org.apache.cassandra.utils.Clock.Global.nanoTime; /** * A thread-safe and atomic Partition implementation. - * + *

* Operations (in particular addAll) on this implementation are atomic and * isolated (in the sense of ACID). Typically a addAll is guaranteed that no * other thread can see the state where only parts but not all rows have @@ -77,7 +77,7 @@ public final class AtomicBTreePartition extends AbstractBTreePartition /** * (clock + allocation) granularity are combined to give us an acceptable (waste) allocation rate that is defined by * the passage of real time of ALLOCATION_GRANULARITY_BYTES/CLOCK_GRANULARITY, or in this case 7.63KiB/ms, or 7.45Mb/s - * + *

* in wasteTracker we maintain within EXCESS_WASTE_OFFSET before the current time; whenever we waste bytes * we increment the current value if it is within this window, and set it to the min of the window plus our waste * otherwise. @@ -101,6 +101,11 @@ public final class AtomicBTreePartition extends AbstractBTreePartition this.ref = BTreePartitionData.EMPTY; } + public ImmutableBTreePartition asImmutable() + { + return new ImmutableBTreePartition(metadata(), partitionKey, holder(), true); + } + protected BTreePartitionData holder() { return ref; diff --git a/src/java/org/apache/cassandra/db/partitions/ImmutableBTreePartition.java b/src/java/org/apache/cassandra/db/partitions/ImmutableBTreePartition.java index 2fe8b95375..a2e8654d9b 100644 --- a/src/java/org/apache/cassandra/db/partitions/ImmutableBTreePartition.java +++ b/src/java/org/apache/cassandra/db/partitions/ImmutableBTreePartition.java @@ -32,6 +32,8 @@ public class ImmutableBTreePartition extends AbstractBTreePartition protected final BTreePartitionData holder; protected final TableMetadata metadata; + private final boolean canHaveShadowedData; + public ImmutableBTreePartition(TableMetadata metadata, DecoratedKey partitionKey, RegularAndStaticColumns columns, @@ -43,15 +45,25 @@ public class ImmutableBTreePartition extends AbstractBTreePartition super(partitionKey); this.metadata = metadata; this.holder = new BTreePartitionData(columns, tree, deletionInfo, staticRow, stats); + this.canHaveShadowedData = false; } protected ImmutableBTreePartition(TableMetadata metadata, - DecoratedKey partitionKey, - BTreePartitionData holder) + DecoratedKey partitionKey, + BTreePartitionData holder) + { + this(metadata, partitionKey, holder, false); + } + + public ImmutableBTreePartition(TableMetadata metadata, + DecoratedKey partitionKey, + BTreePartitionData holder, + boolean canHaveShadowedData) { super(partitionKey); this.metadata = metadata; this.holder = holder; + this.canHaveShadowedData = canHaveShadowedData; } /** @@ -128,6 +140,6 @@ public class ImmutableBTreePartition extends AbstractBTreePartition protected boolean canHaveShadowedData() { - return false; + return canHaveShadowedData; } } diff --git a/src/java/org/apache/cassandra/db/partitions/UnfilteredPartitionIterators.java b/src/java/org/apache/cassandra/db/partitions/UnfilteredPartitionIterators.java index aec80ba1cd..572396fedc 100644 --- a/src/java/org/apache/cassandra/db/partitions/UnfilteredPartitionIterators.java +++ b/src/java/org/apache/cassandra/db/partitions/UnfilteredPartitionIterators.java @@ -427,4 +427,16 @@ public abstract class UnfilteredPartitionIterators }; } } + + public static void consume(UnfilteredPartitionIterator iterator) + { + while (iterator.hasNext()) + { + try (UnfilteredRowIterator partition = iterator.next()) + { + while (partition.hasNext()) + partition.next(); + } + } + } } \ No newline at end of file diff --git a/src/java/org/apache/cassandra/index/Index.java b/src/java/org/apache/cassandra/index/Index.java index 73c4ffe480..05a3826f70 100644 --- a/src/java/org/apache/cassandra/index/Index.java +++ b/src/java/org/apache/cassandra/index/Index.java @@ -29,6 +29,7 @@ import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.Callable; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; @@ -36,6 +37,8 @@ import java.util.function.Supplier; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import com.google.common.collect.PeekingIterator; + import org.apache.cassandra.cql3.Operator; import org.apache.cassandra.cql3.QueryOptions; import org.apache.cassandra.cql3.restrictions.Restriction; @@ -45,6 +48,7 @@ import org.apache.cassandra.db.DeletionTime; import org.apache.cassandra.db.RangeTombstone; import org.apache.cassandra.db.ReadCommand; import org.apache.cassandra.db.ReadExecutionController; +import org.apache.cassandra.db.ReadableView; import org.apache.cassandra.db.RegularAndStaticColumns; import org.apache.cassandra.db.WriteContext; import org.apache.cassandra.db.filter.RowFilter; @@ -55,6 +59,7 @@ import org.apache.cassandra.db.partitions.PartitionIterator; import org.apache.cassandra.db.partitions.PartitionUpdate; import org.apache.cassandra.db.partitions.UnfilteredPartitionIterator; import org.apache.cassandra.db.rows.Row; +import org.apache.cassandra.db.rows.UnfilteredRowIterator; import org.apache.cassandra.exceptions.InvalidRequestException; import org.apache.cassandra.index.internal.CollatedViewIndexBuilder; import org.apache.cassandra.index.transactions.IndexTransaction; @@ -68,6 +73,8 @@ import org.apache.cassandra.schema.ColumnMetadata; import org.apache.cassandra.schema.IndexMetadata; import org.apache.cassandra.schema.TableMetadata; import org.apache.cassandra.service.ClientState; +import org.apache.cassandra.service.reads.tracked.PartialTrackedRead; +import org.apache.cassandra.utils.CloseablePeekingIterator; /** * Consisting of a top level Index interface and two sub-interfaces which handle read and write operations, @@ -750,6 +757,73 @@ public interface Index { return command().rowFilter().filter(fullResponse, command().metadata(), command().nowInSec()); } + + default boolean isMultiStep() + { + return false; + } + + default MultiStepSearcher asMultiStep() + { + throw new IllegalStateException(getClass().getSimpleName() + " is not a multi-step searcher"); + } + } + + interface IndexMatch + { + DecoratedKey key(); + } + + /** + * Extended searcher capable of participating in tracked reads + */ + interface MultiStepSearcher extends Searcher + { + + PartialTrackedRead beginRead(ReadExecutionController executionController, ColumnFamilyStore cfs, long startTimeNanos); + + CloseablePeekingIterator matchIterator(ReadExecutionController executionController); + MatchIndexer matchIndexer(); + + UnfilteredRowIterator queryNextMatches(ReadExecutionController executionController, DecoratedKey partitionKey, ReadableView view, PeekingIterator matches); + + /** + * Since partition updates may not contain all the info the index query needs to know if it will create a hit + * it may return false positives. This filter is meant to catch and remove them from the augmented result + */ + UnfilteredPartitionIterator filterCompletedRead(UnfilteredPartitionIterator iterator); + + MatchComparator matchComparator(); + + @Override + default boolean isMultiStep() + { + return true; + } + + @Override + default MultiStepSearcher asMultiStep() + { + return this; + } + + interface MatchIndexer + { + void index(PartitionUpdate update, Consumer indexTo); + } + + interface MatchComparator extends Comparator + { + int compare(Match a, Match b, boolean strict); + + @Override + default int compare(Match a, Match b) + { + return compare(a, b, true); + } + + default void consumeDuplicates(Match original, PeekingIterator iterator) { } + } } /** diff --git a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java index 373ce67d18..54bc677a2a 100644 --- a/src/java/org/apache/cassandra/index/internal/CassandraIndex.java +++ b/src/java/org/apache/cassandra/index/internal/CassandraIndex.java @@ -356,6 +356,162 @@ public abstract class CassandraIndex implements Index } } + static abstract class AbstractIndexer implements Indexer + { + public void begin() + { + } + + public void partitionDelete(DeletionTime deletionTime) + { + } + + public void rangeTombstone(RangeTombstone tombstone) + { + } + + abstract CassandraIndex index(); + + ColumnMetadata indexedColumn() + { + return index().indexedColumn; + } + + boolean isPrimaryKeyIndex() + { + return index().isPrimaryKeyIndex(); + } + + abstract long nowInSec(); + abstract DecoratedKey key(); + + abstract void insert(DecoratedKey rowKey, Clustering clustering, Cell cell, LivenessInfo info); + abstract void delete(DecoratedKey rowKey, Clustering clustering, Cell cell, long nowInSec); + abstract void delete(DecoratedKey rowKey, Clustering clustering, DeletionTime deletion); + + public void insertRow(Row row) + { + if (row.isStatic() && !indexedColumn().isStatic() && !indexedColumn().isPartitionKey()) + return; + + if (isPrimaryKeyIndex()) + { + indexPrimaryKey(row.clustering(), + getPrimaryKeyIndexLiveness(row), + row.deletion()); + } + else + { + if (indexedColumn().isComplex()) + indexCells(row.clustering(), row.getComplexColumnData(indexedColumn())); + else + indexCell(row.clustering(), row.getCell(indexedColumn())); + } + } + + public void removeRow(Row row) + { + if (isPrimaryKeyIndex()) + return; + + if (indexedColumn().isComplex()) + removeCells(row.clustering(), row.getComplexColumnData(indexedColumn())); + else + removeCell(row.clustering(), row.getCell(indexedColumn())); + } + + public void updateRow(Row oldRow, Row newRow) + { + assert oldRow.isStatic() == newRow.isStatic(); + if (newRow.isStatic() != indexedColumn().isStatic()) + return; + + if (isPrimaryKeyIndex()) + indexPrimaryKey(newRow.clustering(), + getPrimaryKeyIndexLiveness(newRow), + newRow.deletion()); + + if (indexedColumn().isComplex()) + { + indexCells(newRow.clustering(), newRow.getComplexColumnData(indexedColumn())); + removeCells(oldRow.clustering(), oldRow.getComplexColumnData(indexedColumn())); + } + else + { + indexCell(newRow.clustering(), newRow.getCell(indexedColumn())); + removeCell(oldRow.clustering(), oldRow.getCell(indexedColumn())); + } + } + + public void finish() + { + } + + private void indexCells(Clustering clustering, Iterable> cells) + { + if (cells == null) + return; + + for (Cell cell : cells) + indexCell(clustering, cell); + } + + private void indexCell(Clustering clustering, Cell cell) + { + if (cell == null || !cell.isLive(nowInSec())) + return; + + insert(key(), + clustering, + cell, + LivenessInfo.withExpirationTime(cell.timestamp(), cell.ttl(), cell.localDeletionTime())); + } + + private void removeCells(Clustering clustering, Iterable> cells) + { + if (cells == null) + return; + + for (Cell cell : cells) + removeCell(clustering, cell); + } + + private void removeCell(Clustering clustering, Cell cell) + { + if (cell == null || !cell.isLive(nowInSec())) + return; + + delete(key(), clustering, cell, nowInSec()); + } + + private void indexPrimaryKey(final Clustering clustering, + final LivenessInfo liveness, + final Row.Deletion deletion) + { + if (liveness.timestamp() != LivenessInfo.NO_TIMESTAMP) + insert(key(), clustering, null, liveness); + + if (!deletion.isLive()) + delete(key(), clustering, deletion.time()); + } + + private LivenessInfo getPrimaryKeyIndexLiveness(Row row) + { + long timestamp = row.primaryKeyLivenessInfo().timestamp(); + int ttl = row.primaryKeyLivenessInfo().ttl(); + for (Cell cell : row.cells()) + { + long cellTimestamp = cell.timestamp(); + if (cell.isLive(nowInSec())) + { + if (cellTimestamp > timestamp) + timestamp = cellTimestamp; + } + } + return LivenessInfo.create(timestamp, ttl, nowInSec()); + } + } + public Indexer indexerFor(final DecoratedKey key, final RegularAndStaticColumns columns, final long nowInSec, @@ -375,141 +531,42 @@ public abstract class CassandraIndex implements Index if (!isPrimaryKeyIndex() && !columns.contains(indexedColumn)) return null; - return new Indexer() + return new AbstractIndexer() { - public void begin() + @Override + CassandraIndex index() { + return CassandraIndex.this; } - public void partitionDelete(DeletionTime deletionTime) + @Override + long nowInSec() { + return nowInSec; } - public void rangeTombstone(RangeTombstone tombstone) + @Override + DecoratedKey key() { + return key; } - public void insertRow(Row row) + @Override + void insert(DecoratedKey rowKey, Clustering clustering, Cell cell, LivenessInfo info) { - if (row.isStatic() && !indexedColumn.isStatic() && !indexedColumn.isPartitionKey()) - return; - - if (isPrimaryKeyIndex()) - { - indexPrimaryKey(row.clustering(), - getPrimaryKeyIndexLiveness(row), - row.deletion()); - } - else - { - if (indexedColumn.isComplex()) - indexCells(row.clustering(), row.getComplexColumnData(indexedColumn)); - else - indexCell(row.clustering(), row.getCell(indexedColumn)); - } + CassandraIndex.this.insert(rowKey.getKey(), clustering, cell, info, ctx); } - public void removeRow(Row row) + @Override + void delete(DecoratedKey rowKey, Clustering clustering, Cell cell, long nowInSec) { - if (isPrimaryKeyIndex()) - return; - - if (indexedColumn.isComplex()) - removeCells(row.clustering(), row.getComplexColumnData(indexedColumn)); - else - removeCell(row.clustering(), row.getCell(indexedColumn)); + CassandraIndex.this.delete(rowKey.getKey(), clustering, cell, ctx, nowInSec); } - public void updateRow(Row oldRow, Row newRow) + @Override + void delete(DecoratedKey rowKey, Clustering clustering, DeletionTime deletion) { - assert oldRow.isStatic() == newRow.isStatic(); - if (newRow.isStatic() != indexedColumn.isStatic()) - return; - - if (isPrimaryKeyIndex()) - indexPrimaryKey(newRow.clustering(), - getPrimaryKeyIndexLiveness(newRow), - newRow.deletion()); - - if (indexedColumn.isComplex()) - { - indexCells(newRow.clustering(), newRow.getComplexColumnData(indexedColumn)); - removeCells(oldRow.clustering(), oldRow.getComplexColumnData(indexedColumn)); - } - else - { - indexCell(newRow.clustering(), newRow.getCell(indexedColumn)); - removeCell(oldRow.clustering(), oldRow.getCell(indexedColumn)); - } - } - - public void finish() - { - } - - private void indexCells(Clustering clustering, Iterable> cells) - { - if (cells == null) - return; - - for (Cell cell : cells) - indexCell(clustering, cell); - } - - private void indexCell(Clustering clustering, Cell cell) - { - if (cell == null || !cell.isLive(nowInSec)) - return; - - insert(key.getKey(), - clustering, - cell, - LivenessInfo.withExpirationTime(cell.timestamp(), cell.ttl(), cell.localDeletionTime()), - ctx); - } - - private void removeCells(Clustering clustering, Iterable> cells) - { - if (cells == null) - return; - - for (Cell cell : cells) - removeCell(clustering, cell); - } - - private void removeCell(Clustering clustering, Cell cell) - { - if (cell == null || !cell.isLive(nowInSec)) - return; - - delete(key.getKey(), clustering, cell, ctx, nowInSec); - } - - private void indexPrimaryKey(final Clustering clustering, - final LivenessInfo liveness, - final Row.Deletion deletion) - { - if (liveness.timestamp() != LivenessInfo.NO_TIMESTAMP) - insert(key.getKey(), clustering, null, liveness, ctx); - - if (!deletion.isLive()) - delete(key.getKey(), clustering, deletion.time(), ctx); - } - - private LivenessInfo getPrimaryKeyIndexLiveness(Row row) - { - long timestamp = row.primaryKeyLivenessInfo().timestamp(); - int ttl = row.primaryKeyLivenessInfo().ttl(); - for (Cell cell : row.cells()) - { - long cellTimestamp = cell.timestamp(); - if (cell.isLive(nowInSec)) - { - if (cellTimestamp > timestamp) - timestamp = cellTimestamp; - } - } - return LivenessInfo.create(timestamp, ttl, nowInSec); + CassandraIndex.this.delete(rowKey.getKey(), clustering, deletion, ctx); } }; } @@ -531,6 +588,15 @@ public abstract class CassandraIndex implements Index logger.trace("Removed index entry for stale value {}", indexKey); } + public IndexEntry createIndexEntry(DecoratedKey rowKey, Clustering clustering, Cell cell, LivenessInfo info) + { + DecoratedKey indexKey = getIndexKeyFor(getIndexedValue(rowKey.getKey(), + clustering, + cell)); + Clustering indexClustering = buildIndexClustering(rowKey.getKey(), clustering, cell); + return new IndexEntry(indexKey, indexClustering, info.timestamp(), rowKey, clustering); + } + /** * Called when adding a new entry to the index */ diff --git a/src/java/org/apache/cassandra/index/internal/CassandraIndexSearcher.java b/src/java/org/apache/cassandra/index/internal/CassandraIndexSearcher.java index 61d446674e..ee4641471a 100644 --- a/src/java/org/apache/cassandra/index/internal/CassandraIndexSearcher.java +++ b/src/java/org/apache/cassandra/index/internal/CassandraIndexSearcher.java @@ -21,8 +21,11 @@ package org.apache.cassandra.index.internal; import java.nio.ByteBuffer; +import java.util.Optional; import java.util.SortedSet; +import java.util.function.Consumer; +import com.google.common.base.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -32,9 +35,12 @@ import org.apache.cassandra.db.ClusteringBound; import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.DataRange; import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.DeletionTime; +import org.apache.cassandra.db.LivenessInfo; import org.apache.cassandra.db.PartitionPosition; import org.apache.cassandra.db.ReadCommand; import org.apache.cassandra.db.ReadExecutionController; +import org.apache.cassandra.db.ReadableView; import org.apache.cassandra.db.SinglePartitionReadCommand; import org.apache.cassandra.db.Slice; import org.apache.cassandra.db.Slices; @@ -43,23 +49,112 @@ import org.apache.cassandra.db.filter.ClusteringIndexNamesFilter; import org.apache.cassandra.db.filter.ClusteringIndexSliceFilter; import org.apache.cassandra.db.filter.ColumnFilter; import org.apache.cassandra.db.filter.RowFilter; +import org.apache.cassandra.db.lifecycle.SSTableSet; +import org.apache.cassandra.db.lifecycle.View; +import org.apache.cassandra.db.partitions.PartitionUpdate; import org.apache.cassandra.db.partitions.UnfilteredPartitionIterator; +import org.apache.cassandra.db.rows.Cell; +import org.apache.cassandra.db.rows.Row; import org.apache.cassandra.db.rows.RowIterator; +import org.apache.cassandra.db.rows.Rows; import org.apache.cassandra.db.rows.UnfilteredRowIterator; import org.apache.cassandra.db.rows.UnfilteredRowIterators; +import org.apache.cassandra.db.transform.Transformation; import org.apache.cassandra.dht.AbstractBounds; import org.apache.cassandra.index.Index; import org.apache.cassandra.index.internal.composites.CollectionValueIndex; import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.service.reads.tracked.PartialTrackedIndexRead; +import org.apache.cassandra.service.reads.tracked.PartialTrackedRead; +import org.apache.cassandra.utils.AbstractIterator; +import org.apache.cassandra.utils.CloseablePeekingIterator; import org.apache.cassandra.utils.btree.BTreeSet; -public abstract class CassandraIndexSearcher implements Index.Searcher +public abstract class CassandraIndexSearcher implements Index.MultiStepSearcher { + protected abstract class AbstractMatchIndexer extends CassandraIndex.AbstractIndexer implements MatchIndexer + { + protected DecoratedKey key; + protected Consumer indexTo; + + @Override + long nowInSec() + { + return command.nowInSec(); + } + + @Override + CassandraIndex index() + { + return index; + } + + @Override + DecoratedKey key() + { + return key; + } + + protected abstract M createMatch(DecoratedKey rowKey, Clustering clustering, Cell cell, LivenessInfo info); + + @Override + void insert(DecoratedKey rowKey, Clustering clustering, Cell cell, LivenessInfo info) + { + indexTo.accept(createMatch(rowKey, clustering, cell, info)); + } + + @Override + void delete(DecoratedKey rowKey, Clustering clustering, Cell cell, long nowInSec) + { + + } + + @Override + void delete(DecoratedKey rowKey, Clustering clustering, DeletionTime deletion) + { + + } + + @Override + public void insertRow(Row row) + { + if (!expression.isSatisfiedBy(command.metadata(), key, row, nowInSec())) + return; + + if (!command.selectsClustering(key, row.clustering())) + return; + + super.insertRow(row); + } + + @Override + public void index(PartitionUpdate update, Consumer indexTo) + { + this.key = update.partitionKey(); + this.indexTo = indexTo; + + try + { + Row staticRow = update.staticRow(); + if (staticRow != Rows.EMPTY_STATIC_ROW) + insertRow(staticRow); + + update.forEach(this::insertRow); + } + finally + { + this.key = null; + this.indexTo = null; + } + } + } + private static final Logger logger = LoggerFactory.getLogger(CassandraIndexSearcher.class); private final RowFilter.Expression expression; protected final CassandraIndex index; protected final ReadCommand command; + protected final DecoratedKey indexedKey; public CassandraIndexSearcher(ReadCommand command, RowFilter.Expression expression, @@ -68,6 +163,9 @@ public abstract class CassandraIndexSearcher implements Index.Searcher this.command = command; this.expression = expression; this.index = index; + Optional backingTable = index.getBackingTable(); + Preconditions.checkState(backingTable.isPresent()); + this.indexedKey = backingTable.get().decorateKey(expression.getIndexValue()); } @Override @@ -76,19 +174,96 @@ public abstract class CassandraIndexSearcher implements Index.Searcher return command; } - // of this method. + @Override + public PartialTrackedRead beginRead(ReadExecutionController executionController, ColumnFamilyStore cfs, long startTimeNanos) + { + return PartialTrackedIndexRead.create(executionController, cfs, startTimeNanos, command, this); + } + + @Override + public UnfilteredPartitionIterator filterCompletedRead(UnfilteredPartitionIterator iterator) + { + return Transformation.apply(iterator, new Transformation() + { + DecoratedKey key = null; + @Override + protected DecoratedKey applyToPartitionKey(DecoratedKey key) + { + this.key = key; + return super.applyToPartitionKey(key); + } + + @Override + protected Row applyToRow(Row row) + { + if (!expression.isSatisfiedBy(command.metadata(), key, row, command.nowInSec())) + return null; + return row; + } + }); + } + + protected RowIterator queryIndex(DecoratedKey indexKey, ReadExecutionController executionController) + { + UnfilteredRowIterator indexIter = queryIndex(indexKey, command, executionController); + return UnfilteredRowIterators.filter(indexIter, command.nowInSec()); + } + + protected class ResultIterator extends AbstractIterator implements UnfilteredPartitionIterator + { + private final CloseablePeekingIterator matchIterator; + private final ReadExecutionController executionController; + + public ResultIterator(CloseablePeekingIterator matchIterator, ReadExecutionController executionController) + { + this.matchIterator = matchIterator; + this.executionController = executionController; + } + + @Override + protected UnfilteredRowIterator computeNext() + { + while (matchIterator.hasNext()) + { + DecoratedKey key = matchIterator.peek().key(); + ReadableView view = index.baseCfs.select(View.select(SSTableSet.LIVE, key)); + UnfilteredRowIterator partition = queryNextMatches(executionController, key, view, matchIterator); + + if (partition == null) + continue; + + if (!partition.isEmpty()) + return partition; + + partition.close(); + } + return endOfData(); + } + + @Override + public TableMetadata metadata() + { + return command.metadata(); + } + + @Override + public void close() + { + matchIterator.close(); + } + } + + @Override public UnfilteredPartitionIterator search(ReadExecutionController executionController) { - // the value of the index expression is the partition key in the index table - DecoratedKey indexKey = index.getBackingTable().get().decorateKey(expression.getIndexValue()); - UnfilteredRowIterator indexIter = queryIndex(indexKey, command, executionController); + CloseablePeekingIterator matchIterator = matchIterator(executionController); try { - return queryDataFromIndex(indexKey, UnfilteredRowIterators.filter(indexIter, command.nowInSec()), command, executionController); + return new ResultIterator(matchIterator, executionController); } - catch (RuntimeException | Error e) + catch (Throwable e) { - indexIter.close(); + matchIterator.close(); throw e; } } @@ -217,9 +392,4 @@ public abstract class CassandraIndexSearcher implements Index.Searcher { return index.buildIndexClusteringPrefix(rowKey, clustering, null).build(); } - - protected abstract UnfilteredPartitionIterator queryDataFromIndex(DecoratedKey indexKey, - RowIterator indexHits, - ReadCommand command, - ReadExecutionController executionController); } diff --git a/src/java/org/apache/cassandra/index/internal/IndexEntry.java b/src/java/org/apache/cassandra/index/internal/IndexEntry.java index c8e9955d57..6cb4f82362 100644 --- a/src/java/org/apache/cassandra/index/internal/IndexEntry.java +++ b/src/java/org/apache/cassandra/index/internal/IndexEntry.java @@ -20,29 +20,29 @@ */ package org.apache.cassandra.index.internal; -import java.nio.ByteBuffer; - import org.apache.cassandra.db.Clustering; import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.index.Index; +import org.apache.cassandra.schema.TableMetadata; /** * Entries in indexes on non-compact tables (tables with composite comparators) * can be encapsulated as IndexedEntry instances. These are not used when dealing * with indexes on static/compact tables (i.e. KEYS indexes). */ -public final class IndexEntry +public final class IndexEntry implements Index.IndexMatch { public final DecoratedKey indexValue; public final Clustering indexClustering; public final long timestamp; - public final ByteBuffer indexedKey; + public final DecoratedKey indexedKey; public final Clustering indexedEntryClustering; public IndexEntry(DecoratedKey indexValue, Clustering indexClustering, long timestamp, - ByteBuffer indexedKey, + DecoratedKey indexedKey, Clustering indexedEntryClustering) { this.indexValue = indexValue; @@ -51,4 +51,41 @@ public final class IndexEntry this.indexedKey = indexedKey; this.indexedEntryClustering = indexedEntryClustering; } + + @Override + public DecoratedKey key() + { + return indexedKey; + } + + public static int compare(TableMetadata indexMetadata, TableMetadata baseMetadata, IndexEntry left, IndexEntry right) + { + int cmp = left.indexValue.compareTo(right.indexValue); + if (cmp != 0) + return cmp; + + cmp = indexMetadata.comparator.compare(left.indexClustering, right.indexClustering); + if (cmp != 0) + return cmp; + + cmp = left.indexedKey.compareTo(right.indexedKey); + if (cmp != 0) + return cmp; + + // Take STATIC rows into account... + if (!left.indexedEntryClustering.isEmpty() || !right.indexedEntryClustering.isEmpty()) + { + if (left.indexedEntryClustering.isEmpty()) + return -1; + + if (right.indexedEntryClustering.isEmpty()) + return 1; + + cmp = baseMetadata.comparator.compare(left.indexedEntryClustering, right.indexedEntryClustering); + if (cmp != 0) + return cmp; + } + + return Long.compare(left.timestamp, right.timestamp); + } } diff --git a/src/java/org/apache/cassandra/index/internal/composites/ClusteringColumnIndex.java b/src/java/org/apache/cassandra/index/internal/composites/ClusteringColumnIndex.java index c8a63f457a..874b4b46ba 100644 --- a/src/java/org/apache/cassandra/index/internal/composites/ClusteringColumnIndex.java +++ b/src/java/org/apache/cassandra/index/internal/composites/ClusteringColumnIndex.java @@ -98,7 +98,7 @@ public class ClusteringColumnIndex extends CassandraIndex return new IndexEntry(indexedValue, clustering, indexEntry.primaryKeyLivenessInfo().timestamp(), - clustering.bufferAt(0), + baseCfs.decorateKey(clustering.bufferAt(0)), builder.build()); } diff --git a/src/java/org/apache/cassandra/index/internal/composites/CollectionKeyIndexBase.java b/src/java/org/apache/cassandra/index/internal/composites/CollectionKeyIndexBase.java index 1222eaf6a3..c3f182d218 100644 --- a/src/java/org/apache/cassandra/index/internal/composites/CollectionKeyIndexBase.java +++ b/src/java/org/apache/cassandra/index/internal/composites/CollectionKeyIndexBase.java @@ -87,7 +87,7 @@ public abstract class CollectionKeyIndexBase extends CassandraIndex return new IndexEntry(indexedValue, clustering, indexEntry.primaryKeyLivenessInfo().timestamp(), - clustering.bufferAt(0), + baseCfs.decorateKey(clustering.bufferAt(0)), indexedEntryClustering); } } diff --git a/src/java/org/apache/cassandra/index/internal/composites/CollectionValueIndex.java b/src/java/org/apache/cassandra/index/internal/composites/CollectionValueIndex.java index f5d290d827..4ecde0b459 100644 --- a/src/java/org/apache/cassandra/index/internal/composites/CollectionValueIndex.java +++ b/src/java/org/apache/cassandra/index/internal/composites/CollectionValueIndex.java @@ -97,7 +97,7 @@ public class CollectionValueIndex extends CassandraIndex return new IndexEntry(indexedValue, clustering, indexEntry.primaryKeyLivenessInfo().timestamp(), - clustering.bufferAt(0), + baseCfs.decorateKey(clustering.bufferAt(0)), indexedEntryClustering); } diff --git a/src/java/org/apache/cassandra/index/internal/composites/CompositesSearcher.java b/src/java/org/apache/cassandra/index/internal/composites/CompositesSearcher.java index 6629c06256..94ba834fb5 100644 --- a/src/java/org/apache/cassandra/index/internal/composites/CompositesSearcher.java +++ b/src/java/org/apache/cassandra/index/internal/composites/CompositesSearcher.java @@ -21,18 +21,23 @@ import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.List; +import com.google.common.base.Preconditions; +import com.google.common.collect.PeekingIterator; + import org.apache.cassandra.db.Clustering; import org.apache.cassandra.db.ClusteringComparator; import org.apache.cassandra.db.DecoratedKey; import org.apache.cassandra.db.DeletionTime; +import org.apache.cassandra.db.LivenessInfo; import org.apache.cassandra.db.ReadCommand; import org.apache.cassandra.db.ReadExecutionController; +import org.apache.cassandra.db.ReadableView; import org.apache.cassandra.db.SinglePartitionReadCommand; import org.apache.cassandra.db.WriteContext; import org.apache.cassandra.db.filter.ClusteringIndexNamesFilter; import org.apache.cassandra.db.filter.DataLimits; import org.apache.cassandra.db.filter.RowFilter; -import org.apache.cassandra.db.partitions.UnfilteredPartitionIterator; +import org.apache.cassandra.db.rows.Cell; import org.apache.cassandra.db.rows.Row; import org.apache.cassandra.db.rows.RowIterator; import org.apache.cassandra.db.rows.Rows; @@ -43,11 +48,12 @@ import org.apache.cassandra.index.Index; import org.apache.cassandra.index.internal.CassandraIndex; import org.apache.cassandra.index.internal.CassandraIndexSearcher; import org.apache.cassandra.index.internal.IndexEntry; -import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.utils.AbstractIterator; +import org.apache.cassandra.utils.CloseablePeekingIterator; import org.apache.cassandra.utils.btree.BTreeSet; -public class CompositesSearcher extends CassandraIndexSearcher +public class CompositesSearcher extends CassandraIndexSearcher { public CompositesSearcher(ReadCommand command, RowFilter.Expression expression, @@ -56,6 +62,25 @@ public class CompositesSearcher extends CassandraIndexSearcher super(command, expression, index); } + @Override + public MatchIndexer matchIndexer() + { + return new AbstractMatchIndexer() + { + @Override + protected IndexEntry createMatch(DecoratedKey rowKey, Clustering clustering, Cell cell, LivenessInfo info) + { + return index.createIndexEntry(rowKey, clustering, cell, info); + } + }; + } + + @Override + public MatchComparator matchComparator() + { + return (left, right, strict) -> IndexEntry.compare(index.getIndexCfs().metadata(), command.metadata(), left, right); + } + private boolean isMatchingEntry(DecoratedKey partitionKey, IndexEntry entry, ReadCommand command) { return command.selectsKey(partitionKey) && command.selectsClustering(partitionKey, entry.indexedEntryClustering); @@ -66,145 +91,105 @@ public class CompositesSearcher extends CassandraIndexSearcher return index.getIndexedColumn().isStatic(); } - protected UnfilteredPartitionIterator queryDataFromIndex(final DecoratedKey indexKey, - final RowIterator indexHits, - final ReadCommand command, - final ReadExecutionController executionController) + @Override + public CloseablePeekingIterator matchIterator(ReadExecutionController executionController) { - assert indexHits.staticRow() == Rows.EMPTY_STATIC_ROW; - - return new UnfilteredPartitionIterator() + RowIterator indexHits = queryIndex(indexedKey, executionController); + try { - private IndexEntry nextEntry; - - private UnfilteredRowIterator next; - - public TableMetadata metadata() + Preconditions.checkState(indexHits.staticRow() == Rows.EMPTY_STATIC_ROW); + return new AbstractIterator() { - return command.metadata(); - } - - public boolean hasNext() - { - return prepareNext(); - } - - public UnfilteredRowIterator next() - { - if (next == null) - prepareNext(); - - UnfilteredRowIterator toReturn = next; - next = null; - return toReturn; - } - - private boolean prepareNext() - { - while (true) + @Override + protected IndexEntry computeNext() { - if (next != null) - return true; - - if (nextEntry == null) + while (indexHits.hasNext()) { - if (!indexHits.hasNext()) - return false; - - nextEntry = index.decodeEntry(indexKey, indexHits.next()); - } - - SinglePartitionReadCommand dataCmd; - DecoratedKey partitionKey = index.baseCfs.decorateKey(nextEntry.indexedKey); - List entries = new ArrayList<>(); - if (isStaticColumn()) - { - // The index hit may not match the commad key constraint - if (!isMatchingEntry(partitionKey, nextEntry, command)) { - nextEntry = indexHits.hasNext() ? index.decodeEntry(indexKey, indexHits.next()) : null; - continue; - } - - // If the index is on a static column, we just need to do a full read on the partition. - // Note that we want to re-use the command.columnFilter() in case of future change. - dataCmd = SinglePartitionReadCommand.create(index.baseCfs.metadata(), - command.nowInSec(), - command.columnFilter(), - RowFilter.none(), - DataLimits.NONE, - partitionKey, - command.clusteringIndexFilter(partitionKey)); - entries.add(nextEntry); - nextEntry = indexHits.hasNext() ? index.decodeEntry(indexKey, indexHits.next()) : null; - } - else - { - // Gather all index hits belonging to the same partition and query the data for those hits. - // TODO: it's much more efficient to do 1 read for all hits to the same partition than doing - // 1 read per index hit. However, this basically mean materializing all hits for a partition - // in memory so we should consider adding some paging mechanism. However, index hits should - // be relatively small so it's much better than the previous code that was materializing all - // *data* for a given partition. - BTreeSet.Builder> clusterings = BTreeSet.builder(index.baseCfs.getComparator()); - while (nextEntry != null && partitionKey.getKey().equals(nextEntry.indexedKey)) - { - // We're queried a slice of the index, but some hits may not match some of the clustering column constraints - if (isMatchingEntry(partitionKey, nextEntry, command)) - { - clusterings.add(nextEntry.indexedEntryClustering); - entries.add(nextEntry); - } - - nextEntry = indexHits.hasNext() ? index.decodeEntry(indexKey, indexHits.next()) : null; - } - - // Because we've eliminated entries that don't match the clustering columns, it's possible we added nothing - if (clusterings.isEmpty()) + IndexEntry nextEntry = index.decodeEntry(indexedKey, indexHits.next()); + DecoratedKey partitionKey = nextEntry.indexedKey; + if (!isMatchingEntry(partitionKey, nextEntry, command)) continue; - // Query the gathered index hits. We still need to filter stale hits from the resulting query. - ClusteringIndexNamesFilter filter = new ClusteringIndexNamesFilter(clusterings.build(), false); - dataCmd = SinglePartitionReadCommand.create(index.baseCfs.metadata(), - command.nowInSec(), - command.columnFilter(), - command.rowFilter(), - DataLimits.NONE, - partitionKey, - filter, - (Index.QueryPlan) null); + return nextEntry; } - - // by the next caller of next, or through closing this iterator is this come before. - UnfilteredRowIterator dataIter = - filterStaleEntries(dataCmd.queryMemtableAndDisk(index.baseCfs, executionController), - indexKey.getKey(), - entries, - executionController.getWriteContext(), - command.nowInSec()); - - if (dataIter.isEmpty()) - { - dataIter.close(); - continue; - } - - next = dataIter; - return true; + return endOfData(); } - } - public void remove() - { - throw new UnsupportedOperationException(); - } - - public void close() - { + @Override + public void close() + { + if (indexHits != null) + indexHits.close(); + } + }; + } + catch (Throwable e) + { + if (indexHits != null) indexHits.close(); - if (next != null) - next.close(); + throw e; + } + } + + @Override + public UnfilteredRowIterator queryNextMatches(ReadExecutionController executionController, DecoratedKey partitionKey, ReadableView view, PeekingIterator matches) + { + Preconditions.checkArgument(matches.hasNext()); + SinglePartitionReadCommand dataCmd; + List entries = new ArrayList<>(); + if (isStaticColumn()) + { + + // If the index is on a static column, we just need to do a full read on the partition. + // Note that we want to re-use the command.columnFilter() in case of future change. + dataCmd = SinglePartitionReadCommand.create(index.baseCfs.metadata(), + command.nowInSec(), + command.columnFilter(), + RowFilter.none(), + DataLimits.NONE, + partitionKey, + command.clusteringIndexFilter(partitionKey)); + entries.add(matches.next()); + } + else + { + // Gather all index hits belonging to the same partition and query the data for those hits. + // TODO: it's much more efficient to do 1 read for all hits to the same partition than doing + // 1 read per index hit. However, this basically mean materializing all hits for a partition + // in memory so we should consider adding some paging mechanism. However, index hits should + // be relatively small so it's much better than the previous code that was materializing all + // *data* for a given partition. + BTreeSet.Builder> clusterings = BTreeSet.builder(index.baseCfs.getComparator()); + while (matches.hasNext() && partitionKey.equals(matches.peek().indexedKey)) + { + // We're queried a slice of the index, and some hits may not match some of the clustering column constraints, + // but they will have been filtered out upstream + IndexEntry nextEntry = matches.next(); + clusterings.add(nextEntry.indexedEntryClustering); + entries.add(nextEntry); } - }; + + // since non-matching entries will have been filtered out by matchIterator, it should not be possible to have empty clusterings + Preconditions.checkArgument(!clusterings.isEmpty()); + + // Query the gathered index hits. We still need to filter stale hits from the resulting query. + ClusteringIndexNamesFilter filter = new ClusteringIndexNamesFilter(clusterings.build(), false); + dataCmd = SinglePartitionReadCommand.create(index.baseCfs.metadata(), + command.nowInSec(), + command.columnFilter(), + command.rowFilter(), + DataLimits.NONE, + partitionKey, + filter, + (Index.QueryPlan) null); + } + + // by the next caller of next, or through closing this iterator is this come before. + return filterStaleEntries(dataCmd.queryMemtableAndDisk(view, index.baseCfs, executionController), + indexedKey.getKey(), + entries, + executionController.getWriteContext(), + command.nowInSec()); } private void deleteAllEntries(final List entries, final WriteContext ctx, final long nowInSec) @@ -297,8 +282,8 @@ public class CompositesSearcher extends CassandraIndexSearcher // those tables do not support static columns. By consequence if a table // has some static columns and all its clustering key elements are null // it means that the partition exists and contains only static data - if (!dataIter.metadata().hasStaticColumns() || !containsOnlyNullValues(indexedEntryClustering)) - staleEntries.add(entry); + if (!dataIter.metadata().hasStaticColumns() || !containsOnlyNullValues(indexedEntryClustering)) + staleEntries.add(entry); } // entries correspond to the rows we've queried, so we shouldn't have a row that has no corresponding entry. throw new AssertionError(); diff --git a/src/java/org/apache/cassandra/index/internal/composites/PartitionKeyIndex.java b/src/java/org/apache/cassandra/index/internal/composites/PartitionKeyIndex.java index 4280ee65a1..5d44f386da 100644 --- a/src/java/org/apache/cassandra/index/internal/composites/PartitionKeyIndex.java +++ b/src/java/org/apache/cassandra/index/internal/composites/PartitionKeyIndex.java @@ -90,7 +90,7 @@ public class PartitionKeyIndex extends CassandraIndex return new IndexEntry(indexedValue, clustering, indexEntry.primaryKeyLivenessInfo().timestamp(), - clustering.bufferAt(0), + baseCfs.decorateKey(clustering.bufferAt(0)), builder.build()); } diff --git a/src/java/org/apache/cassandra/index/internal/composites/RegularColumnIndex.java b/src/java/org/apache/cassandra/index/internal/composites/RegularColumnIndex.java index f68862e114..777c2b34f2 100644 --- a/src/java/org/apache/cassandra/index/internal/composites/RegularColumnIndex.java +++ b/src/java/org/apache/cassandra/index/internal/composites/RegularColumnIndex.java @@ -101,7 +101,7 @@ public class RegularColumnIndex extends CassandraIndex return new IndexEntry(indexedValue, clustering, indexEntry.primaryKeyLivenessInfo().timestamp(), - clustering.bufferAt(0), + baseCfs.decorateKey(clustering.bufferAt(0)), indexedEntryClustering); } diff --git a/src/java/org/apache/cassandra/index/internal/keys/KeysSearcher.java b/src/java/org/apache/cassandra/index/internal/keys/KeysSearcher.java index 98e4e02160..ad5c09f6ff 100644 --- a/src/java/org/apache/cassandra/index/internal/keys/KeysSearcher.java +++ b/src/java/org/apache/cassandra/index/internal/keys/KeysSearcher.java @@ -19,33 +19,34 @@ package org.apache.cassandra.index.internal.keys; import java.nio.ByteBuffer; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; +import com.google.common.base.Preconditions; +import com.google.common.collect.PeekingIterator; import org.apache.cassandra.db.Clustering; import org.apache.cassandra.db.DecoratedKey; import org.apache.cassandra.db.DeletionTime; +import org.apache.cassandra.db.LivenessInfo; import org.apache.cassandra.db.ReadCommand; import org.apache.cassandra.db.ReadExecutionController; +import org.apache.cassandra.db.ReadableView; import org.apache.cassandra.db.SinglePartitionReadCommand; import org.apache.cassandra.db.WriteContext; import org.apache.cassandra.db.filter.ColumnFilter; import org.apache.cassandra.db.filter.DataLimits; import org.apache.cassandra.db.filter.RowFilter; -import org.apache.cassandra.db.partitions.UnfilteredPartitionIterator; +import org.apache.cassandra.db.rows.Cell; import org.apache.cassandra.db.rows.Row; import org.apache.cassandra.db.rows.RowIterator; import org.apache.cassandra.db.rows.Rows; import org.apache.cassandra.db.rows.UnfilteredRowIterator; -import org.apache.cassandra.index.Index; import org.apache.cassandra.index.internal.CassandraIndex; import org.apache.cassandra.index.internal.CassandraIndexSearcher; -import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.index.internal.IndexEntry; +import org.apache.cassandra.utils.AbstractIterator; +import org.apache.cassandra.utils.CloseablePeekingIterator; -public class KeysSearcher extends CassandraIndexSearcher +public class KeysSearcher extends CassandraIndexSearcher { - private static final Logger logger = LoggerFactory.getLogger(KeysSearcher.class); - public KeysSearcher(ReadCommand command, RowFilter.Expression expression, CassandraIndex indexer) @@ -53,89 +54,91 @@ public class KeysSearcher extends CassandraIndexSearcher super(command, expression, indexer); } - protected UnfilteredPartitionIterator queryDataFromIndex(final DecoratedKey indexKey, - final RowIterator indexHits, - final ReadCommand command, - final ReadExecutionController executionController) + @Override + public MatchIndexer matchIndexer() { - assert indexHits.staticRow() == Rows.EMPTY_STATIC_ROW; - - return new UnfilteredPartitionIterator() + return new AbstractMatchIndexer() { - private UnfilteredRowIterator next; - - public TableMetadata metadata() + @Override + protected IndexEntry createMatch(DecoratedKey rowKey, Clustering clustering, Cell cell, LivenessInfo info) { - return command.metadata(); - } - - public boolean hasNext() - { - return prepareNext(); - } - - public UnfilteredRowIterator next() - { - if (next == null) - prepareNext(); - - UnfilteredRowIterator toReturn = next; - next = null; - return toReturn; - } - - private boolean prepareNext() - { - while (next == null && indexHits.hasNext()) - { - Row hit = indexHits.next(); - DecoratedKey key = index.baseCfs.decorateKey(hit.clustering().bufferAt(0)); - if (!command.selectsKey(key)) - continue; - - ColumnFilter extendedFilter = getExtendedFilter(command.columnFilter()); - SinglePartitionReadCommand dataCmd = SinglePartitionReadCommand.create(index.baseCfs.metadata(), - command.nowInSec(), - extendedFilter, - command.rowFilter(), - DataLimits.NONE, - key, - command.clusteringIndexFilter(key), - (Index.QueryPlan) null); - - // Otherwise, we close right away if empty, and if it's assigned to next it will be called either - // by the next caller of next, or through closing this iterator is this come before. - UnfilteredRowIterator dataIter = filterIfStale(dataCmd.queryMemtableAndDisk(index.baseCfs, executionController), - hit, - indexKey.getKey(), - executionController.getWriteContext(), - command.nowInSec()); - - if (dataIter != null) - { - if (dataIter.isEmpty()) - dataIter.close(); - else - next = dataIter; - } - } - return next != null; - } - - public void remove() - { - throw new UnsupportedOperationException(); - } - - public void close() - { - indexHits.close(); - if (next != null) - next.close(); + return index.createIndexEntry(rowKey, clustering, cell, info); } }; } + @Override + public MatchComparator matchComparator() + { + return (left, right, strict) -> IndexEntry.compare(index.getIndexCfs().metadata(), command.metadata(), left, right); + } + + @Override + public CloseablePeekingIterator matchIterator(ReadExecutionController executionController) + { + RowIterator indexHits = queryIndex(indexedKey, executionController); + try + { + Preconditions.checkState(indexHits.staticRow() == Rows.EMPTY_STATIC_ROW); + return new AbstractIterator() + { + @Override + protected IndexEntry computeNext() + { + while (indexHits.hasNext()) + { + Row hit = indexHits.next(); + DecoratedKey key = index.baseCfs.decorateKey(hit.clustering().bufferAt(0)); + if (!command.selectsKey(key)) + continue; + + return new IndexEntry(indexedKey, hit.clustering(), hit.primaryKeyLivenessInfo().timestamp(), key, Clustering.EMPTY); + } + return endOfData(); + } + + @Override + public void close() + { + if (indexHits != null) + indexHits.close(); + } + }; + + } + catch (Throwable e) + { + if (indexHits != null) + indexHits.close(); + throw e; + } + } + + @Override + public UnfilteredRowIterator queryNextMatches(ReadExecutionController executionController, DecoratedKey key, ReadableView view, PeekingIterator matches) + { + Preconditions.checkArgument(matches.hasNext()); + + IndexEntry entry = matches.next(); + + ColumnFilter extendedFilter = getExtendedFilter(command.columnFilter()); + SinglePartitionReadCommand dataCmd = SinglePartitionReadCommand.create(index.baseCfs.metadata(), + command.nowInSec(), + extendedFilter, + command.rowFilter(), + DataLimits.NONE, + key, + command.clusteringIndexFilter(key)); + + // Otherwise, we close right away if empty, and if it's assigned to next it will be called either + // by the next caller of next, or through closing this iterator is this come before. + return filterIfStale(dataCmd.queryMemtableAndDisk(index.baseCfs, executionController), + entry.timestamp, + indexedKey.getKey(), + executionController.getWriteContext(), + command.nowInSec()); + } + private ColumnFilter getExtendedFilter(ColumnFilter initialFilter) { if (command.columnFilter().fetches(index.getIndexedColumn())) @@ -148,7 +151,7 @@ public class KeysSearcher extends CassandraIndexSearcher } private UnfilteredRowIterator filterIfStale(UnfilteredRowIterator iterator, - Row indexHit, + long timestamp, ByteBuffer indexedValue, WriteContext ctx, long nowInSec) @@ -159,7 +162,7 @@ public class KeysSearcher extends CassandraIndexSearcher // Index is stale, remove the index entry and ignore index.deleteStaleEntry(index.getIndexCfs().decorateKey(indexedValue), makeIndexClustering(iterator.partitionKey().getKey(), Clustering.EMPTY), - DeletionTime.build(indexHit.primaryKeyLivenessInfo().timestamp(), nowInSec), + DeletionTime.build(timestamp, nowInSec), ctx); iterator.close(); return null; diff --git a/src/java/org/apache/cassandra/index/sai/plan/FilterTree.java b/src/java/org/apache/cassandra/index/sai/plan/FilterTree.java index 03ce9daf3b..17571fa1a6 100644 --- a/src/java/org/apache/cassandra/index/sai/plan/FilterTree.java +++ b/src/java/org/apache/cassandra/index/sai/plan/FilterTree.java @@ -79,7 +79,12 @@ public class FilterTree public boolean isSatisfiedBy(DecoratedKey key, Row row, Row staticRow) { - boolean result = localSatisfiedBy(key, row, staticRow); + return isSatisfiedBy(key, row, staticRow, context.hasUnrepairedMatches); + } + + public boolean isSatisfiedBy(DecoratedKey key, Row row, Row staticRow, boolean hasUnrepairedMatches) + { + boolean result = localSatisfiedBy(key, row, staticRow, hasUnrepairedMatches); for (FilterTree child : children) result = baseOperator.apply(result, child.isSatisfiedBy(key, row, staticRow)); @@ -87,19 +92,19 @@ public class FilterTree return result; } - private boolean localSatisfiedBy(DecoratedKey key, Row row, Row staticRow) + private boolean localSatisfiedBy(DecoratedKey key, Row row, Row staticRow, boolean hasUnrepairedMatches) { if (row == null) return false; final long now = FBUtilities.nowInSeconds(); // Downgrade AND to OR unless the coordinator indicates strict filtering is safe or all matches are repaired: - BooleanOperator localOperator = (isStrict || !context.hasUnrepairedMatches) ? baseOperator : BooleanOperator.OR; + BooleanOperator localOperator = (isStrict || !hasUnrepairedMatches) ? baseOperator : BooleanOperator.OR; boolean result = localOperator == BooleanOperator.AND; // If all matches on indexed columns are repaired, strict filtering is not allowed, and there are multiple // unindexed column expressions, isolate the expressions on unindexed columns and union their results: - boolean isolateUnindexed = !context.hasUnrepairedMatches && !isStrict && expressions.hasMultipleUnindexedColumns(); + boolean isolateUnindexed = !hasUnrepairedMatches && !isStrict && expressions.hasMultipleUnindexedColumns(); boolean unindexedResult = false; Iterator columnIterator = expressions.columns().iterator(); diff --git a/src/java/org/apache/cassandra/index/sai/plan/QueryController.java b/src/java/org/apache/cassandra/index/sai/plan/QueryController.java index 00147a54bc..74d79b1b02 100644 --- a/src/java/org/apache/cassandra/index/sai/plan/QueryController.java +++ b/src/java/org/apache/cassandra/index/sai/plan/QueryController.java @@ -42,6 +42,7 @@ import org.apache.cassandra.db.PartitionPosition; import org.apache.cassandra.db.PartitionRangeReadCommand; import org.apache.cassandra.db.ReadCommand; import org.apache.cassandra.db.ReadExecutionController; +import org.apache.cassandra.db.ReadableView; import org.apache.cassandra.db.SinglePartitionReadCommand; import org.apache.cassandra.db.filter.ClusteringIndexFilter; import org.apache.cassandra.db.filter.ClusteringIndexNamesFilter; @@ -127,6 +128,11 @@ public class QueryController this.lastPrimaryKey = keyFactory.create(mergeRange.right.getToken()); this.nextClusterings = new InsertionOrderedNavigableSet<>(cfs.metadata().comparator); } + + public ReadCommand command() + { + return command; + } public PrimaryKey.Factory primaryKeyFactory() { @@ -183,7 +189,7 @@ public class QueryController return index != null && index.hasAnalyzer(); } - public UnfilteredRowIterator queryStorage(List keys, ReadExecutionController executionController) + public UnfilteredRowIterator queryStorage(ReadableView view, List keys, ReadExecutionController executionController) { if (keys.isEmpty()) throw new IllegalArgumentException("At least one primary key is required!"); @@ -196,7 +202,7 @@ public class QueryController keys.get(0).partitionKey(), makeFilter(keys)); - return partition.queryMemtableAndDisk(cfs, executionController); + return partition.queryMemtableAndDisk(view, cfs, executionController); } private static Runnable getIndexReleaser(Set referencedIndexes) @@ -237,7 +243,7 @@ public class QueryController key.partitionKey(), makeFilter(List.of(key))); - return partition.queryMemtableAndDisk(cfs, view, SOURCE_TABLE_ROW_TRANSFORMER, executionController); + return partition.queryMemtableAndDisk(view, cfs, SOURCE_TABLE_ROW_TRANSFORMER, executionController); } /** diff --git a/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexSearcher.java b/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexSearcher.java index 2e8b7d8f71..654ed20cf9 100644 --- a/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexSearcher.java +++ b/src/java/org/apache/cassandra/index/sai/plan/StorageAttachedIndexSearcher.java @@ -31,12 +31,16 @@ import java.util.NoSuchElementException; import java.util.PriorityQueue; import java.util.Queue; import java.util.concurrent.TimeUnit; -import java.util.function.Supplier; +import java.util.function.Consumer; import java.util.stream.Collectors; import javax.annotation.Nonnull; import javax.annotation.Nullable; +import com.google.common.base.Preconditions; +import com.google.common.collect.PeekingIterator; + +import io.netty.util.concurrent.FastThreadLocal; import org.apache.cassandra.cql3.Operator; import org.apache.cassandra.db.Clustering; import org.apache.cassandra.db.ClusteringBound; @@ -47,19 +51,25 @@ import org.apache.cassandra.db.DecoratedKey; import org.apache.cassandra.db.PartitionPosition; import org.apache.cassandra.db.ReadCommand; import org.apache.cassandra.db.ReadExecutionController; +import org.apache.cassandra.db.ReadableView; import org.apache.cassandra.db.RegularAndStaticColumns; import org.apache.cassandra.db.Slices; import org.apache.cassandra.db.filter.ClusteringIndexFilter; import org.apache.cassandra.db.filter.ClusteringIndexNamesFilter; import org.apache.cassandra.db.filter.ClusteringIndexSliceFilter; +import org.apache.cassandra.db.filter.DataLimits; import org.apache.cassandra.db.filter.RowFilter; +import org.apache.cassandra.db.lifecycle.SSTableSet; +import org.apache.cassandra.db.lifecycle.View; import org.apache.cassandra.db.partitions.PartitionIterator; +import org.apache.cassandra.db.partitions.PartitionUpdate; import org.apache.cassandra.db.partitions.UnfilteredPartitionIterator; import org.apache.cassandra.db.rows.AbstractUnfilteredRowIterator; import org.apache.cassandra.db.rows.Row; import org.apache.cassandra.db.rows.RowIterator; import org.apache.cassandra.db.rows.Unfiltered; import org.apache.cassandra.db.rows.UnfilteredRowIterator; +import org.apache.cassandra.db.transform.Transformation; import org.apache.cassandra.dht.AbstractBounds; import org.apache.cassandra.dht.Token; import org.apache.cassandra.exceptions.RequestTimeoutException; @@ -73,21 +83,28 @@ import org.apache.cassandra.index.sai.utils.PrimaryKey; import org.apache.cassandra.index.sai.utils.RangeUtil; import org.apache.cassandra.io.util.FileUtils; import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.service.reads.tracked.PartialTrackedIndexRead; +import org.apache.cassandra.service.reads.tracked.PartialTrackedRead; import org.apache.cassandra.utils.AbstractIterator; import org.apache.cassandra.utils.Clock; import org.apache.cassandra.utils.CloseableIterator; +import org.apache.cassandra.utils.CloseablePeekingIterator; import org.apache.cassandra.utils.FBUtilities; -import io.netty.util.concurrent.FastThreadLocal; - -public class StorageAttachedIndexSearcher implements Index.Searcher +public class StorageAttachedIndexSearcher implements Index.MultiStepSearcher { private static final int PARTITION_ROW_BATCH_SIZE = 100; + private final ColumnFamilyStore cfs; private final ReadCommand command; private final QueryController queryController; private final QueryContext queryContext; private final TableQueryMetrics tableQueryMetrics; + private final int partitionRowBatchSize; + private final FilterTree filterTree; + private final FilterTree strictFilterTree; + private final PrimaryKey.Factory keyFactory; + private final boolean topK; private static final FastThreadLocal> nextKeys = new FastThreadLocal<>() { @@ -104,10 +121,24 @@ public class StorageAttachedIndexSearcher implements Index.Searcher RowFilter indexFilter, long executionQuotaMs) { + this.cfs = cfs; this.command = command; this.queryContext = new QueryContext(command, executionQuotaMs); this.queryController = new QueryController(cfs, command, indexFilter, queryContext); this.tableQueryMetrics = tableQueryMetrics; + + // Use PER PARTITION LIMIT if it exists, otherwise use global LIMIT + int effectiveLimit = command.limits().perPartitionCount() != DataLimits.NO_LIMIT + ? command.limits().perPartitionCount() + : command.limits().count(); + this.partitionRowBatchSize = Math.min(PARTITION_ROW_BATCH_SIZE, effectiveLimit); + + boolean useStrictFiltering = queryController.usesStrictFiltering(); + this.filterTree = Operation.buildFilter(queryController, useStrictFiltering); + this.strictFilterTree = useStrictFiltering ? this.filterTree : Operation.buildFilter(queryController, true); + + this.keyFactory = queryController.primaryKeyFactory(); + this.topK = command.isTopK(); } @Override @@ -126,7 +157,69 @@ public class StorageAttachedIndexSearcher implements Index.Searcher } // if no analyzer does transformation - return Index.Searcher.super.filterReplicaFilteringProtection(fullResponse); + return Index.MultiStepSearcher.super.filterReplicaFilteringProtection(fullResponse); + } + + // ========================================== + // MultiStepSearcher interface (non-topK path) + // ========================================== + + protected class ResultIterator extends AbstractIterator implements UnfilteredPartitionIterator + { + private final CloseablePeekingIterator matchIterator; + private final ReadExecutionController executionController; + + public ResultIterator(CloseablePeekingIterator matchIterator, ReadExecutionController executionController) + { + this.matchIterator = matchIterator; + this.executionController = executionController; + } + + @Override + protected UnfilteredRowIterator computeNext() + { + while (matchIterator.hasNext()) + { + DecoratedKey key = matchIterator.peek().partitionKey(); + ReadableView view = cfs.select(View.select(SSTableSet.LIVE, key)); + UnfilteredRowIterator partition = queryNextMatches(executionController, key, view, matchIterator); + + if (partition == null) + continue; + + if (!partition.isEmpty()) + return partition; + + partition.close(); + } + return endOfData(); + } + + @Override + public TableMetadata metadata() + { + return command.metadata(); + } + + @Override + public void close() + { + matchIterator.close(); + } + } + + private ResultIterator searchInternal(ReadExecutionController executionController) + { + CloseablePeekingIterator matchIterator = matchIterator(executionController); + try + { + return new ResultIterator(matchIterator, executionController); + } + catch (Throwable e) + { + matchIterator.close(); + throw e; + } } @Override @@ -134,7 +227,7 @@ public class StorageAttachedIndexSearcher implements Index.Searcher { if (!command.isTopK()) { - return new ResultRetriever(executionController); + return searchInternal(executionController); } else { @@ -171,26 +264,48 @@ public class StorageAttachedIndexSearcher implements Index.Searcher return new QueryViewBuilder(Collections.singleton(planExpression), queryController.mergeRange()).build(); } - private abstract class AbstractRetreiver extends AbstractIterator implements UnfilteredPartitionIterator + private class MatchIndexer implements Index.MultiStepSearcher.MatchIndexer { - final FilterTree filterTree; - final ReadExecutionController executionController; + private final PrimaryKey.Factory keyFactory; - AbstractRetreiver(ReadExecutionController executionController) + public MatchIndexer() { - this.executionController = executionController; - this.filterTree = Operation.buildFilter(queryController, queryController.usesStrictFiltering()); + this.keyFactory = queryController.primaryKeyFactory(); } @Override - public TableMetadata metadata() + public void index(PartitionUpdate update, Consumer indexTo) { - return queryController.metadata(); - } + DecoratedKey key = update.partitionKey(); + Row staticRow = update.staticRow(); + boolean hasClustering = cfs.getComparator().size() > 0; + if (!filterTree.restrictsNonStaticRow()) + { + if (filterTree.isSatisfiedBy(key, staticRow, staticRow, true)) + indexTo.accept(keyFactory.create(key, Clustering.STATIC_CLUSTERING)); + } + else + { + if (hasClustering && filterTree.isSatisfiedBy(key, staticRow, staticRow, true)) + { + indexTo.accept(keyFactory.create(key, Clustering.STATIC_CLUSTERING)); + return; + } + + for (Row row : update) + { + if (filterTree.isSatisfiedBy(key, row, staticRow, true)) + { + PrimaryKey primaryKey = hasClustering ? keyFactory.create(key, row.clustering()) : keyFactory.create(key); + indexTo.accept(primaryKey); + } + } + } + } } - private class ResultRetriever extends AbstractRetreiver + private class MatchIterator extends AbstractIterator { private final PrimaryKey firstPrimaryKey; private final PrimaryKey lastPrimaryKey; @@ -199,157 +314,91 @@ public class StorageAttachedIndexSearcher implements Index.Searcher private AbstractBounds currentKeyRange; private final KeyRangeIterator resultKeyIterator; - private final PrimaryKey.Factory keyFactory; - private final int partitionRowBatchSize; private PrimaryKey lastKey; - private ResultRetriever(ReadExecutionController executionController) + private MatchIterator() { - super(executionController); this.keyRanges = queryController.dataRanges().iterator(); this.firstDataRange = keyRanges.next(); this.currentKeyRange = firstDataRange.keyRange(); this.resultKeyIterator = Operation.buildIterator(queryController); - this.keyFactory = queryController.primaryKeyFactory(); this.firstPrimaryKey = queryController.firstPrimaryKeyInRange(); this.lastPrimaryKey = queryController.lastPrimaryKeyInRange(); - - // Ensure we don't fetch larger batches than the provided LIMIT to avoid fetching keys we won't use: - this.partitionRowBatchSize = Math.min(PARTITION_ROW_BATCH_SIZE, command.limits().count()); } @Override - public UnfilteredRowIterator computeNext() + public PrimaryKey computeNext() { if (resultKeyIterator == null) - return endOfData(); + throw new IllegalStateException("Result key iterator should be non-null. (Query: '" + queryController.command().toCQLString() + "')"); - // If being called for the first time, skip to the beginning of the range. - // We can't put this code in the constructor because it may throw and the caller - // may not be prepared for that. - if (lastKey == null) + while (true) { - PrimaryKey skipTarget = firstPrimaryKey; - ClusteringComparator comparator = command.metadata().comparator; - - // If there are no clusterings, the first data range selects an entire partitions, or we have static - // expressions, don't bother trying to skip forward within the partition. - if (comparator.size() > 0 && !firstDataRange.selectsAllPartition() && !command.rowFilter().hasStaticExpression()) + // If being called for the first time, skip to the beginning of the range. + // We can't put this code in the constructor because it may throw and the caller + // may not be prepared for that. + if (lastKey == null) { - // Only attempt to skip if the first data range covers a single partition. - if (currentKeyRange.left.equals(currentKeyRange.right) && currentKeyRange.left instanceof DecoratedKey) + PrimaryKey skipTarget = firstPrimaryKey; + ClusteringComparator comparator = command.metadata().comparator; + + // If there are no clusterings, the first data range selects an entire partitions, or we have static + // expressions, don't bother trying to skip forward within the partition. + if (comparator.size() > 0 && !firstDataRange.selectsAllPartition() && !command.rowFilter().hasStaticExpression()) { - DecoratedKey decoratedKey = (DecoratedKey) currentKeyRange.left; - ClusteringIndexFilter filter = firstDataRange.clusteringIndexFilter(decoratedKey); - - if (filter instanceof ClusteringIndexSliceFilter) + // Only attempt to skip if the first data range covers a single partition. + if (currentKeyRange.left.equals(currentKeyRange.right) && currentKeyRange.left instanceof DecoratedKey) { - Slices slices = ((ClusteringIndexSliceFilter) filter).requestedSlices(); + DecoratedKey decoratedKey = (DecoratedKey) currentKeyRange.left; + ClusteringIndexFilter filter = firstDataRange.clusteringIndexFilter(decoratedKey); - if (!slices.isEmpty()) + if (filter instanceof ClusteringIndexSliceFilter) { - ClusteringBound startBound = slices.get(0).start(); + Slices slices = ((ClusteringIndexSliceFilter) filter).requestedSlices(); - if (!startBound.isEmpty()) + if (!slices.isEmpty()) { - ByteBuffer[] rawValues = startBound.getBufferArray(); + ClusteringBound startBound = slices.get(0).start(); - if (rawValues.length == comparator.size()) - skipTarget = keyFactory.create(decoratedKey, Clustering.make(rawValues)); + if (!startBound.isEmpty()) + { + ByteBuffer[] rawValues = startBound.getBufferArray(); + + if (rawValues.length == comparator.size()) + skipTarget = keyFactory.create(decoratedKey, Clustering.make(rawValues)); + } + } + } + else if (filter instanceof ClusteringIndexNamesFilter) + { + ClusteringIndexNamesFilter namesFilter = (ClusteringIndexNamesFilter) filter; + + if (!namesFilter.requestedRows().isEmpty()) + { + Clustering skipClustering = namesFilter.requestedRows().iterator().next(); + skipTarget = keyFactory.create(decoratedKey, skipClustering); } } } - else if (filter instanceof ClusteringIndexNamesFilter) - { - ClusteringIndexNamesFilter namesFilter = (ClusteringIndexNamesFilter) filter; - - if (!namesFilter.requestedRows().isEmpty()) - { - Clustering skipClustering = namesFilter.requestedRows().iterator().next(); - skipTarget = keyFactory.create(decoratedKey, skipClustering); - } - } } + + resultKeyIterator.skipTo(skipTarget); } - resultKeyIterator.skipTo(skipTarget); + PrimaryKey nextKey = nextKeyInRange(); + if (nextKey == null) + return endOfData(); + + if (queryController.doesNotSelect(nextKey) || nextKey.equals(lastKey, false)) + continue; + + lastKey = nextKey; + return nextKey; } - - // Theoretically we wouldn't need this if the caller of computeNext always ran the - // returned iterators to the completion. Unfortunately, we have no control over the caller behavior here. - // Hence, we skip to the next partition in order to comply to the unwritten partition iterator contract - // saying this iterator must not return the same partition twice. - skipToNextPartition(); - - UnfilteredRowIterator iterator = nextRowIterator(this::nextSelectedKeysInRange); - return iterator != null ? iteratePartition(iterator) : endOfData(); } - /** - * Tries to obtain a row iterator for the supplied keys by repeatedly calling - * {@link ResultRetriever#queryStorageAndFilter} until it gives a non-null result. - * The keysSupplier should return the next batch of keys with every call to get() - * and null when there are no more keys to try. - * - * @return an iterator or null if all keys were tried with no success - */ - private @Nullable UnfilteredRowIterator nextRowIterator(@Nonnull Supplier> keysSupplier) - { - UnfilteredRowIterator iterator = null; - while (iterator == null) - { - List keys = keysSupplier.get(); - if (keys.isEmpty()) - return null; - iterator = queryStorageAndFilter(keys); - } - return iterator; - } - /** - * Retrieves the next batch of primary keys (i.e. up to {@link #partitionRowBatchSize} of them) that are - * contained by one of the query key ranges and selected by the {@link QueryController}. If the next key falls - * out of the current key range, it skips to the next key range, and so on. If no more keys accepted by - * the controller are available, and empty list is returned. - * - * @return a list of up to {@link #partitionRowBatchSize} primary keys - */ - private List nextSelectedKeysInRange() - { - List threadLocalNextKeys = nextKeys.get(); - threadLocalNextKeys.clear(); - PrimaryKey firstKey; - - do - { - firstKey = nextKeyInRange(); - - if (firstKey == null) - return Collections.emptyList(); - } - while (queryController.doesNotSelect(firstKey) || firstKey.equals(lastKey, false)); - - lastKey = firstKey; - threadLocalNextKeys.add(firstKey); - fillNextSelectedKeysInPartition(firstKey.partitionKey(), threadLocalNextKeys); - return threadLocalNextKeys; - } - - /** - * Retrieves the next batch of primary keys (i.e. up to {@link #partitionRowBatchSize} of them) that belong to - * the given partition and are selected by the query controller, advancing the underlying iterator only while - * the next key belongs to that partition. - * - * @return a list of up to {@link #partitionRowBatchSize} primary keys within the given partition - */ - private List nextSelectedKeysInPartition(DecoratedKey partitionKey) - { - List threadLocalNextKeys = nextKeys.get(); - threadLocalNextKeys.clear(); - fillNextSelectedKeysInPartition(partitionKey, threadLocalNextKeys); - return threadLocalNextKeys; - } /** * Returns the next available key contained by one of the keyRanges. @@ -379,25 +428,6 @@ public class StorageAttachedIndexSearcher implements Index.Searcher return key; } - private void fillNextSelectedKeysInPartition(DecoratedKey partitionKey, List nextPrimaryKeys) - { - while (resultKeyIterator.hasNext() - && resultKeyIterator.peek().partitionKey().equals(partitionKey) - && nextPrimaryKeys.size() < partitionRowBatchSize) - { - PrimaryKey key = nextKey(); - - if (key == null) - break; - - if (queryController.doesNotSelect(key) || key.equals(lastKey, false)) - continue; - - nextPrimaryKeys.add(key); - lastKey = key; - } - } - /** * Gets the next key from the underlying operation. * Returns null if there are no more keys <= lastPrimaryKey. @@ -434,87 +464,6 @@ public class StorageAttachedIndexSearcher implements Index.Searcher resultKeyIterator.skipTo(keyFactory.create(token)); } - /** - * Skips to the key that belongs to a different partition than the last key we fetched. - */ - private void skipToNextPartition() - { - if (lastKey == null) - return; - DecoratedKey lastPartitionKey = lastKey.partitionKey(); - while (resultKeyIterator.hasNext() && resultKeyIterator.peek().partitionKey().equals(lastPartitionKey)) - resultKeyIterator.next(); - } - - - /** - * Returns an iterator over the rows in the partition associated with the given iterator. - * Initially, it retrieves the rows from the given iterator until it runs out of data. - * Then it iterates the remaining primary keys obtained from the index in batches until the end of the - * partition, lazily constructing an itertor for each batch. Only one row iterator is open at a time. - *

- * The rows are retrieved in the order of primary keys provided by the underlying index. - * The iterator is complete when the next key to be fetched belongs to different partition - * (but the iterator does not consume that key). - * - * @param startIter an iterator positioned at the first row in the partition that we want to return - */ - private @Nonnull UnfilteredRowIterator iteratePartition(@Nonnull UnfilteredRowIterator startIter) - { - return new AbstractUnfilteredRowIterator(startIter.metadata(), - startIter.partitionKey(), - startIter.partitionLevelDeletion(), - startIter.columns(), - startIter.staticRow(), - startIter.isReverseOrder(), - startIter.stats()) - { - private UnfilteredRowIterator currentIter = startIter; - private final DecoratedKey partitionKey = startIter.partitionKey(); - - @Override - protected Unfiltered computeNext() - { - while (!currentIter.hasNext()) - { - currentIter.close(); - currentIter = nextRowIterator(() -> nextSelectedKeysInPartition(partitionKey)); - if (currentIter == null) - return endOfData(); - } - return currentIter.next(); - } - - @Override - public void close() - { - FileUtils.closeQuietly(currentIter); - super.close(); - } - }; - } - - private UnfilteredRowIterator queryStorageAndFilter(List keys) - { - long startTimeNanos = Clock.Global.nanoTime(); - - try (UnfilteredRowIterator partition = queryController.queryStorage(keys, executionController)) - { - queryContext.partitionsRead++; - queryContext.checkpoint(); - - List filtered = filterPartition(partition, filterTree, queryContext); - - // Note that we record the duration of the read after post-filtering, which actually - // materializes the rows from disk. - tableQueryMetrics.postFilteringReadLatency.update(Clock.Global.nanoTime() - startTimeNanos, TimeUnit.NANOSECONDS); - - return filtered != null - ? new SinglePartitionIterator(partition, partition.staticRow(), filtered.iterator()) - : null; - } - } - @Override public void close() { @@ -523,76 +472,67 @@ public class StorageAttachedIndexSearcher implements Index.Searcher } } - private static List filterPartition(UnfilteredRowIterator partition, FilterTree tree, QueryContext context) + @Override + public CloseablePeekingIterator matchIterator(ReadExecutionController executionController) { - Row staticRow = partition.staticRow(); - DecoratedKey partitionKey = partition.partitionKey(); - List matches = new ArrayList<>(); - boolean hasMatch = false; - - while (partition.hasNext()) - { - Unfiltered unfiltered = partition.next(); - - if (unfiltered.isRow()) - { - context.rowsFiltered++; - - if (tree.isSatisfiedBy(partitionKey, (Row) unfiltered, staticRow)) - { - matches.add((Row) unfiltered); - hasMatch = true; - } - } - } - - // We may not have any non-static row data to filter... - if (!hasMatch) - { - context.rowsFiltered++; - - if (tree.isSatisfiedBy(partitionKey, staticRow, staticRow)) - { - hasMatch = true; - } - } - - if (!hasMatch) - { - // If there are no matches, return an empty partition. If reconciliation is required at the - // coordinator, replica filtering protection may make a second round trip to complete its view - // of the partition. - return null; - } - - // Return all matches found - return matches; + return new MatchIterator(); } - private static class SinglePartitionIterator extends AbstractUnfilteredRowIterator + @Override + public MatchComparator matchComparator() { - private final Iterator rows; - - public SinglePartitionIterator(UnfilteredRowIterator partition, Row staticRow, Iterator rows) + return new MatchComparator() { - super(partition.metadata(), - partition.partitionKey(), - partition.partitionLevelDeletion(), - partition.columns(), - staticRow, - partition.isReverseOrder(), - partition.stats()); + @Override + public int compare(PrimaryKey a, PrimaryKey b, boolean strict) + { + return a.compareTo(b, strict); + } - this.rows = rows; - } + @Override + public void consumeDuplicates(PrimaryKey original, PeekingIterator iterator) + { + while (original.kind() == PrimaryKey.Kind.STATIC && iterator.hasNext() && original.equals(iterator.peek(), false)) + iterator.next(); + } + }; + } - @Override - protected Unfiltered computeNext() + @Override + public Index.MultiStepSearcher.MatchIndexer matchIndexer() + { + return new MatchIndexer(); + } + + @Override + public PartialTrackedRead beginRead(ReadExecutionController executionController, ColumnFamilyStore cfs, long startTimeNanos) + { + return PartialTrackedIndexRead.create(executionController, cfs, startTimeNanos, command, this); + } + + private UnfilteredRowIterator queryStorageAndFilter(ReadableView view, ReadExecutionController executionController, List keys) + { + long startTimeNanos = Clock.Global.nanoTime(); + + try (UnfilteredRowIterator partition = queryController.queryStorage(view, keys, executionController)) { - return rows.hasNext() ? rows.next() : endOfData(); + queryContext.partitionsRead++; + queryContext.checkpoint(); + + UnfilteredRowIterator filtered = filterPartition(keys, partition, filterTree); + + // Note that we record the duration of the read after post-filtering, which actually + // materializes the rows from disk. + tableQueryMetrics.postFilteringReadLatency.update(Clock.Global.nanoTime() - startTimeNanos, TimeUnit.NANOSECONDS); + + return filtered; } } + // ========================================== + // TopK / ANN path (ScoreOrderedResultRetriever) + // ========================================== + /** * A result retriever that consumes an iterator primary keys sorted by some score, materializes the row for each * primary key (currently, each primary key is required to be fully qualified and should only point to one row), @@ -603,8 +543,11 @@ public class StorageAttachedIndexSearcher implements Index.Searcher * The resulting {@link UnfilteredRowIterator} objects are not guaranteed to be in any particular order. It is * the responsibility of the caller to sort the results if necessary. */ - public class ScoreOrderedResultRetriever extends AbstractRetreiver + public class ScoreOrderedResultRetriever extends AbstractIterator implements UnfilteredPartitionIterator { + private final FilterTree filterTree; + private final ReadExecutionController executionController; + private final ColumnFamilyStore.ViewFragment view; private final List> keyRanges; private final boolean coversFullRing; @@ -624,7 +567,9 @@ public class StorageAttachedIndexSearcher implements Index.Searcher private ScoreOrderedResultRetriever(ReadExecutionController executionController, QueryViewBuilder.QueryView queryView) { - super(executionController); + this.filterTree = Operation.buildFilter(queryController, queryController.usesStrictFiltering()); + this.executionController = executionController; + assert queryView.view.size() == 1; QueryViewBuilder.QueryExpressionView queryExpressionView = queryView.view.stream().findFirst().get(); this.view = queryExpressionView.computeViewFragment(); @@ -639,6 +584,12 @@ public class StorageAttachedIndexSearcher implements Index.Searcher this.pendingRows = new ArrayDeque<>(softLimit); } + @Override + public TableMetadata metadata() + { + return queryController.metadata(); + } + @Override public UnfilteredRowIterator computeNext() { @@ -766,7 +717,7 @@ public class StorageAttachedIndexSearcher implements Index.Searcher queryContext.partitionsRead++; queryContext.checkpoint(); - List clusters = filterPartition(partition, filterTree, queryContext); + List clusters = filterPartitionRows(partition, filterTree, queryContext); if (clusters == null) { @@ -823,6 +774,198 @@ public class StorageAttachedIndexSearcher implements Index.Searcher } } + /** + * Filters a partition returning just the matching rows as a list. + * Used by {@link ScoreOrderedResultRetriever#readAndValidatePartition} which needs + * individual row access for validity checking. + */ + private static List filterPartitionRows(UnfilteredRowIterator partition, FilterTree tree, QueryContext context) + { + Row staticRow = partition.staticRow(); + DecoratedKey partitionKey = partition.partitionKey(); + List matches = new ArrayList<>(); + boolean hasMatch = false; + + while (partition.hasNext()) + { + Unfiltered unfiltered = partition.next(); + + if (unfiltered.isRow()) + { + context.rowsFiltered++; + + if (tree.isSatisfiedBy(partitionKey, (Row) unfiltered, staticRow)) + { + matches.add((Row) unfiltered); + hasMatch = true; + } + } + } + + // We may not have any non-static row data to filter... + if (!hasMatch) + { + context.rowsFiltered++; + + if (tree.isSatisfiedBy(partitionKey, staticRow, staticRow)) + { + hasMatch = true; + } + } + + if (!hasMatch) + { + // If there are no matches, return an empty partition. If reconciliation is required at the + // coordinator, replica filtering protection may make a second round trip to complete its view + // of the partition. + return null; + } + + // Return all matches found + return matches; + } + + private static class SinglePartitionIterator extends AbstractUnfilteredRowIterator + { + private final Iterator rows; + + public SinglePartitionIterator(UnfilteredRowIterator partition, Row staticRow, Iterator rows) + { + super(partition.metadata(), + partition.partitionKey(), + partition.partitionLevelDeletion(), + partition.columns(), + staticRow, + partition.isReverseOrder(), + partition.stats()); + + this.rows = rows; + } + + @Override + protected Unfiltered computeNext() + { + return rows.hasNext() ? rows.next() : endOfData(); + } + } + + // ========================================== + // Partition iteration helpers (non-topK path) + // ========================================== + + /** + * Returns an iterator over the rows in the partition associated with the given iterator. + * Initially, it retrieves the rows from the given iterator until it runs out of data. + * Then it iterates the remaining primary keys obtained from the index in batches until the end of the + * partition, lazily constructing an itertor for each batch. Only one row iterator is open at a time. + *

+ * The rows are retrieved in the order of primary keys provided by the underlying index. + * The iterator is complete when the next key to be fetched belongs to different partition + * (but the iterator does not consume that key). + * + * @param startIter an iterator positioned at the first row in the partition that we want to return + */ + private @Nonnull UnfilteredRowIterator iteratePartition(ReadExecutionController executionController, ReadableView view, PeekingIterator matchIter, @Nonnull UnfilteredRowIterator startIter) + { + return new AbstractUnfilteredRowIterator(startIter.metadata(), + startIter.partitionKey(), + startIter.partitionLevelDeletion(), + startIter.columns(), + startIter.staticRow(), + startIter.isReverseOrder(), + startIter.stats()) + { + private UnfilteredRowIterator currentIter = startIter; + private final DecoratedKey partitionKey = startIter.partitionKey(); + + @Override + protected Unfiltered computeNext() + { + while (!currentIter.hasNext()) + { + currentIter.close(); + currentIter = nextRowIterator(executionController, partitionKey, view, matchIter); + if (currentIter == null) + return endOfData(); + } + return currentIter.next(); + } + + @Override + public void close() + { + // skip to the next partition key if the matchIterator hasn't been exhausted + while (matchIter.hasNext() && matchIter.peek().partitionKey().equals(partitionKey)) + matchIter.next(); + + FileUtils.closeQuietly(currentIter); + super.close(); + } + }; + } + + private void fillNextSelectedKeysInPartition(DecoratedKey partitionKey, List nextPrimaryKeys, PeekingIterator resultKeyIterator) + { + while (resultKeyIterator.hasNext() + && resultKeyIterator.peek().partitionKey().equals(partitionKey) + && nextPrimaryKeys.size() < partitionRowBatchSize) + { + nextPrimaryKeys.add(resultKeyIterator.next()); + } + } + + /** + * Retrieves the next batch of primary keys (i.e. up to {@link #partitionRowBatchSize} of them) that belong to + * the given partition and are selected by the query controller, advancing the underlying iterator only while + * the next key belongs to that partition. + * + * @return a list of up to {@link #partitionRowBatchSize} primary keys within the given partition + */ + private List nextSelectedKeysInPartition(DecoratedKey partitionKey, PeekingIterator matches) + { + List threadLocalNextKeys = nextKeys.get(); + threadLocalNextKeys.clear(); + fillNextSelectedKeysInPartition(partitionKey, threadLocalNextKeys, matches); + return threadLocalNextKeys; + } + + /** + * Tries to obtain a row iterator for the supplied keys by repeatedly calling + * {@link StorageAttachedIndexSearcher#queryStorageAndFilter} until it gives a non-null result. + * The keysSupplier should return the next batch of keys with every call to get() + * and null when there are no more keys to try. + * + * @return an iterator or null if all keys were tried with no success + */ + private @Nullable UnfilteredRowIterator nextRowIterator(ReadExecutionController executionController, DecoratedKey partitionKey, ReadableView view, PeekingIterator matches) + { + UnfilteredRowIterator iterator = null; + while (iterator == null) + { + List keys = nextSelectedKeysInPartition(partitionKey, matches); + if (keys.isEmpty()) + return null; + iterator = queryStorageAndFilter(view, executionController, keys); + } + return iterator; + } + + @Override + public UnfilteredRowIterator queryNextMatches(ReadExecutionController executionController, DecoratedKey partitionKey, ReadableView view, PeekingIterator matchIter) + { + Preconditions.checkArgument(matchIter.hasNext()); + Preconditions.checkArgument(matchIter.peek().partitionKey().equals(partitionKey)); + + UnfilteredRowIterator iterator = nextRowIterator(executionController, partitionKey, view, matchIter); + if (iterator == null) + return null; + return iteratePartition(executionController, view, matchIter, iterator); + } + + // ========================================== + // Filtering + // ========================================== + /** * Used by {@link StorageAttachedIndexSearcher#filterReplicaFilteringProtection} to filter rows for columns that * have transformations so won't get handled correctly by the row filter. @@ -934,4 +1077,106 @@ public class StorageAttachedIndexSearcher implements Index.Searcher } }; } + + @Override + public UnfilteredPartitionIterator filterCompletedRead(UnfilteredPartitionIterator iterator) + { + return Transformation.apply(iterator, new Transformation() + { + DecoratedKey key = null; + Row staticRow = null; + + @Override + protected DecoratedKey applyToPartitionKey(DecoratedKey key) + { + this.key = key; + return super.applyToPartitionKey(key); + } + + @Override + protected UnfilteredRowIterator applyToPartition(UnfilteredRowIterator partition) + { + this.staticRow = partition.staticRow(); + if (!strictFilterTree.restrictsNonStaticRow()) + return strictFilterTree.isSatisfiedBy(partition.partitionKey(), staticRow, staticRow) ? partition : null; + + return Transformation.apply(partition, this); + } + + @Override + protected Row applyToRow(Row row) + { + queryContext.rowsFiltered++; + + ClusteringIndexFilter clusteringFilter = command.clusteringIndexFilter(key); + if (!clusteringFilter.selects(row.clustering())) + return null; + + if (!strictFilterTree.isSatisfiedBy(key, row, staticRow)) + return null; + return super.applyToRow(row); + } + }); + } + + private UnfilteredRowIterator filterPartition(List keys, UnfilteredRowIterator partition, FilterTree tree) + { + Row staticRow = partition.staticRow(); + DecoratedKey partitionKey = partition.partitionKey(); + List matches = new ArrayList<>(); + boolean hasMatch = false; + + while (partition.hasNext()) + { + Unfiltered unfiltered = partition.next(); + + if (unfiltered.isRow()) + { + queryContext.rowsFiltered++; + + if (tree.isSatisfiedBy(partitionKey, (Row) unfiltered, staticRow)) + { + matches.add(unfiltered); + hasMatch = true; + } + } + } + + // We may not have any non-static row data to filter... + if (!hasMatch) + { + queryContext.rowsFiltered++; + + if (tree.isSatisfiedBy(partitionKey, staticRow, staticRow)) + { + hasMatch = true; + } + } + + if (!hasMatch) + { + // If there are no matches, return an empty partition. If reconciliation is required at the + // coordinator, replica filtering protection may make a second round trip to complete its view + // of the partition. + return null; + } + + // Return all matches found, along with the static row... + return new AbstractUnfilteredRowIterator(partition.metadata(), + partition.partitionKey(), + partition.partitionLevelDeletion(), + partition.columns(), + staticRow, + partition.isReverseOrder(), + partition.stats()) + { + private final Iterator rows = matches.iterator(); + + @Override + protected Unfiltered computeNext() + { + return rows.hasNext() ? rows.next() : endOfData(); + } + }; + } } diff --git a/src/java/org/apache/cassandra/index/sai/utils/PrimaryKey.java b/src/java/org/apache/cassandra/index/sai/utils/PrimaryKey.java index 6de7a6c884..0dcef7c715 100644 --- a/src/java/org/apache/cassandra/index/sai/utils/PrimaryKey.java +++ b/src/java/org/apache/cassandra/index/sai/utils/PrimaryKey.java @@ -29,6 +29,7 @@ import org.apache.cassandra.db.DecoratedKey; import org.apache.cassandra.db.marshal.ByteBufferAccessor; import org.apache.cassandra.dht.IPartitioner; import org.apache.cassandra.dht.Token; +import org.apache.cassandra.index.Index; import org.apache.cassandra.utils.ByteBufferUtil; import org.apache.cassandra.utils.bytecomparable.ByteComparable; import org.apache.cassandra.utils.bytecomparable.ByteSource; @@ -40,7 +41,7 @@ import org.apache.cassandra.utils.bytecomparable.ByteSourceInverse; * The {@link Factory.TokenOnlyPrimaryKey} is used by the {@link org.apache.cassandra.index.sai.plan.StorageAttachedIndexSearcher} to * position the search within the query range. */ -public interface PrimaryKey extends Comparable, ByteComparable +public interface PrimaryKey extends Comparable, ByteComparable, Index.IndexMatch { /** * See the javadoc for {@link #kind()} for how this enum is used. @@ -474,6 +475,12 @@ public interface PrimaryKey extends Comparable, ByteComparable */ DecoratedKey partitionKey(); + @Override + default DecoratedKey key() + { + return partitionKey(); + } + /** * Returns the {@link Clustering} representing the clustering component of the {@link PrimaryKey}. *

diff --git a/src/java/org/apache/cassandra/replication/BroadcastLogOffsets.java b/src/java/org/apache/cassandra/replication/BroadcastLogOffsets.java index ce1d4b672f..abd23dc3f2 100644 --- a/src/java/org/apache/cassandra/replication/BroadcastLogOffsets.java +++ b/src/java/org/apache/cassandra/replication/BroadcastLogOffsets.java @@ -21,9 +21,6 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import org.apache.cassandra.db.TypeSizes; import org.apache.cassandra.dht.AbstractBounds; import org.apache.cassandra.dht.IPartitioner; @@ -36,8 +33,6 @@ import org.apache.cassandra.net.IVerbHandler; public class BroadcastLogOffsets { - private static final Logger logger = LoggerFactory.getLogger(BroadcastLogOffsets.class); - private final String keyspace; private final Range range; private final List replicatedOffsets; @@ -59,7 +54,7 @@ public class BroadcastLogOffsets @Override public String toString() { - StringBuilder sb = new StringBuilder('['); + StringBuilder sb = new StringBuilder("["); boolean isFirst = true; for (Offsets.Immutable logOffsets : replicatedOffsets) { @@ -73,7 +68,6 @@ public class BroadcastLogOffsets public static final IVerbHandler verbHandler = message -> { BroadcastLogOffsets replicatedOffsets = message.payload; - logger.trace("Received replicated offsets {} from {}", replicatedOffsets, message.from()); MutationTrackingService.instance.updateReplicatedOffsets(replicatedOffsets.keyspace, replicatedOffsets.range, replicatedOffsets.replicatedOffsets, diff --git a/src/java/org/apache/cassandra/schema/ReplicationType.java b/src/java/org/apache/cassandra/schema/ReplicationType.java index 575e9b7f78..d57f68f5af 100644 --- a/src/java/org/apache/cassandra/schema/ReplicationType.java +++ b/src/java/org/apache/cassandra/schema/ReplicationType.java @@ -83,10 +83,4 @@ public enum ReplicationType { return this == tracked; } - - // FIXME: used in lieu of adding support for tracked reads in parameterized tests, fix usages of this method - public static ReplicationType[] fixmeValues() - { - return new ReplicationType[]{ untracked }; - } } diff --git a/src/java/org/apache/cassandra/service/reads/tracked/AbstractPartialTrackedRead.java b/src/java/org/apache/cassandra/service/reads/tracked/AbstractPartialTrackedRead.java deleted file mode 100644 index 2819068a43..0000000000 --- a/src/java/org/apache/cassandra/service/reads/tracked/AbstractPartialTrackedRead.java +++ /dev/null @@ -1,266 +0,0 @@ -/* - * 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.service.reads.tracked; - -import com.google.common.base.Preconditions; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import org.apache.cassandra.db.ColumnFamilyStore; -import org.apache.cassandra.db.Mutation; -import org.apache.cassandra.db.ReadExecutionController; -import org.apache.cassandra.db.partitions.PartitionUpdate; -import org.apache.cassandra.db.partitions.UnfilteredPartitionIterator; -import org.apache.cassandra.db.transform.RTBoundValidator; - - -public abstract class AbstractPartialTrackedRead implements PartialTrackedRead -{ - private static final Logger logger = LoggerFactory.getLogger(AbstractPartialTrackedRead.class); - - protected interface Augmentable - { - State augment(PartitionUpdate update); - } - - protected static abstract class State - { - protected static final State CLOSED = new State() - { - @Override - String name() - { - return "closed"; - } - - @Override - boolean isClosed() - { - return true; - } - }; - - abstract String name(); - - boolean isInitialized() - { - return false; - } - - Initialized asInitialized() - { - throw new IllegalStateException("State is " + name() + ", not " + Initialized.NAME); - } - - boolean isPrepared() - { - return false; - } - - Prepared asPrepared() - { - throw new IllegalStateException("State is " + name() + ", not " + Prepared.NAME); - } - - boolean isCompleted() - { - return false; - } - - Completed asCompleted() - { - throw new IllegalStateException("State is " + name() + ", not " + Completed.NAME); - } - - boolean isAugmentable() - { - return isPrepared() || isInitialized(); - } - - Augmentable asAugmentable() - { - if (isPrepared()) return asPrepared(); - throw new IllegalStateException("State is " + name() + ", not augmentable"); - } - - boolean isClosed() - { - return false; - } - - void close() - { - } - } - - // TODO (expected): this is a redundant state, never exposed - protected final class Initialized extends State - { - static final String NAME = "initialized"; - - @Override - String name() - { - return NAME; - } - - @Override - boolean isInitialized() - { - return true; - } - - @Override - Initialized asInitialized() - { - return this; - } - - Prepared prepare(UnfilteredPartitionIterator initialData) - { - return prepareInternal(initialData); - } - } - - protected abstract Prepared prepareInternal(UnfilteredPartitionIterator initialData); - - protected abstract class Prepared extends State implements Augmentable - { - private static final String NAME = "prepared"; - - @Override - String name() - { - return NAME; - } - - @Override - boolean isPrepared() - { - return true; - } - - @Override - Prepared asPrepared() - { - return this; - } - - abstract Completed complete(); - - } - - protected abstract class Completed extends State - { - private static final String NAME = "completed"; - - @Override - String name() - { - return NAME; - } - - protected abstract UnfilteredPartitionIterator iterator(); - protected abstract CompletedRead createResult(UnfilteredPartitionIterator iterator); - - protected CompletedRead getResult() - { - UnfilteredPartitionIterator result = command().completeTrackedRead(iterator(), AbstractPartialTrackedRead.this); - // validate that the sequence of RT markers is correct: open is followed by close, deletion times for both - // ends equal, and there are no dangling RT bound in any partition. - result = RTBoundValidator.validate(result, RTBoundValidator.Stage.PROCESSED, true); - return createResult(result); - } - } - - final ReadExecutionController executionController; - final ColumnFamilyStore cfs; - final long startTimeNanos; - private State state = new Initialized(); - - public AbstractPartialTrackedRead(ReadExecutionController executionController, ColumnFamilyStore cfs, long startTimeNanos) - { - this.executionController = executionController; - this.cfs = cfs; - this.startTimeNanos = startTimeNanos; - } - - @Override - public ReadExecutionController executionController() - { - return executionController; - } - - @Override - public ColumnFamilyStore cfs() - { - return cfs; - } - - @Override - public long startTimeNanos() - { - return startTimeNanos; - } - - protected synchronized State state() - { - return state; - } - - /** - * Implementors need to call this before returning this from createInProgressRead - * TODO (expected): this is a redundant transition from a redundant state (INITIALIZED) - */ - synchronized void prepare(UnfilteredPartitionIterator initialData) - { - logger.trace("Preparing read {}", this); - state = state.asInitialized().prepare(initialData); - } - - @Override - public synchronized void augment(Mutation mutation) - { - PartitionUpdate update = mutation.getPartitionUpdate(command().metadata()); - if (update != null) - state = state.asAugmentable().augment(update); - } - - @Override - public synchronized CompletedRead complete() - { - Preconditions.checkState(state.isPrepared()); - Completed completed = state.asPrepared().complete(); - state = completed; - return completed.getResult(); - } - - @Override - public synchronized void close() - { - if (state.isClosed()) - return; - - logger.trace("Closing read {}", this); - state.close(); - executionController.close(); - state = State.CLOSED; - } -} diff --git a/src/java/org/apache/cassandra/service/reads/tracked/ExtendingCompletedRead.java b/src/java/org/apache/cassandra/service/reads/tracked/ExtendingCompletedRead.java index 88f4b496e8..21a552e677 100644 --- a/src/java/org/apache/cassandra/service/reads/tracked/ExtendingCompletedRead.java +++ b/src/java/org/apache/cassandra/service/reads/tracked/ExtendingCompletedRead.java @@ -18,7 +18,7 @@ package org.apache.cassandra.service.reads.tracked; -import org.apache.cassandra.transport.Dispatcher; +import com.google.common.base.Preconditions; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -35,48 +35,31 @@ import org.apache.cassandra.db.transform.EmptyPartitionsDiscarder; import org.apache.cassandra.db.transform.Transformation; import org.apache.cassandra.dht.AbstractBounds; import org.apache.cassandra.tracing.Tracing; +import org.apache.cassandra.transport.Dispatcher; import org.apache.cassandra.utils.FBUtilities; import org.apache.cassandra.utils.concurrent.AsyncPromise; import org.apache.cassandra.utils.concurrent.Future; -class ExtendingCompletedRead implements PartialTrackedRead.CompletedRead +public abstract class ExtendingCompletedRead implements PartialTrackedRead.CompletedRead { private static final Logger logger = LoggerFactory.getLogger(ExtendingCompletedRead.class); - final PartitionRangeReadCommand command; - final UnfilteredPartitionIterator iterator; // merged end-result counter final DataLimits.Counter mergedResultCounter; - private final boolean partitionsFetched; private final boolean initialIteratorExhausted; - protected final AbstractBounds followUpBounds; - public ExtendingCompletedRead(PartitionRangeReadCommand command, - UnfilteredPartitionIterator iterator, - boolean partitionsFetched, - boolean initialIteratorExhausted, - AbstractBounds followUpBounds) + public ExtendingCompletedRead(ReadCommand command, boolean partitionsFetched, boolean initialIteratorExhausted) { - this.command = command; - this.iterator = iterator; - mergedResultCounter = command.limits().newCounter(command.nowInSec(), - true, - command.selectsFullPartition(), - command.metadata().enforceStrictLiveness()); + this.mergedResultCounter = command.limits().newCounter(command.nowInSec(), + true, + command.selectsFullPartition(), + command.metadata().enforceStrictLiveness()); this.partitionsFetched = partitionsFetched; this.initialIteratorExhausted = initialIteratorExhausted; - this.followUpBounds = followUpBounds; } - @Override - public TrackedDataResponse response() - { - PartitionIterator filtered = UnfilteredPartitionIterators.filter(iterator, command.nowInSec()); - PartitionIterator counted = Transformation.apply(filtered, mergedResultCounter); - PartitionIterator result = Transformation.apply(counted, new EmptyPartitionsDiscarder()); - return TrackedDataResponse.create(result, command.columnFilter()); - } + abstract ReadCommand command(); static boolean followUpReadRequired(ReadCommand command, DataLimits.Counter mergedResultCounter, boolean initialIteratorExhausted, boolean partitionsFetched) { @@ -116,7 +99,7 @@ class ExtendingCompletedRead implements PartialTrackedRead.CompletedRead protected boolean followUpRequired() { - return followUpReadRequired(command, mergedResultCounter, initialIteratorExhausted, partitionsFetched); + return followUpReadRequired(command(), mergedResultCounter, initialIteratorExhausted, partitionsFetched); } static int toQuery(ReadCommand command, DataLimits.Counter mergedResultCounter) @@ -145,18 +128,21 @@ class ExtendingCompletedRead implements PartialTrackedRead.CompletedRead * the total # of rows remaining - if it has some. If we don't grab enough rows in some of the partitions, * then future ShortReadRowsProtection.moreContents() calls will fetch the missing ones. */ - int toQuery = toQuery(command, mergedResultCounter); + int toQuery = toQuery(command(), mergedResultCounter); - ColumnFamilyStore.metricsFor(command.metadata().id).shortReadProtectionRequests.mark(); + ColumnFamilyStore.metricsFor(command().metadata().id).shortReadProtectionRequests.mark(); Tracing.trace("Requesting {} extra rows from {} for short read protection", toQuery, FBUtilities.getBroadcastAddressAndPort()); logger.info("Requesting {} extra rows from {} for short read protection", toQuery, FBUtilities.getBroadcastAddressAndPort()); return makeFollowupRead(initialResponse, toQuery, consistencyLevel, requestTime); } + protected abstract AbstractBounds followUpBounds(); + protected Future makeFollowupRead(TrackedDataResponse initialResponse, int toQuery, ConsistencyLevel consistencyLevel, Dispatcher.RequestTime requestTime) { - TrackedRead.Range followUpRead = PartialTrackedRangeRead.makeFollowUpRead(command, followUpBounds, toQuery, consistencyLevel, requestTime); + Preconditions.checkState(command() instanceof PartitionRangeReadCommand); + TrackedRead.Range followUpRead = PartialTrackedRangeRead.makeFollowUpRead((PartitionRangeReadCommand) command(), followUpBounds(), toQuery, consistencyLevel, requestTime); logger.trace("Short read detected, starting followup read {}", followUpRead); followUpRead.start(requestTime); AsyncPromise combinedRead = new AsyncPromise<>(); @@ -180,9 +166,49 @@ class ExtendingCompletedRead implements PartialTrackedRead.CompletedRead return combinedRead; } - @Override - public void close() + static class RangeRead extends ExtendingCompletedRead { - iterator.close(); + final PartitionRangeReadCommand command; + final UnfilteredPartitionIterator iterator; + protected final AbstractBounds followUpBounds; + + public RangeRead(PartitionRangeReadCommand command, + UnfilteredPartitionIterator iterator, + boolean partitionsFetched, + boolean initialIteratorExhausted, + AbstractBounds followUpBounds) + { + super(command, partitionsFetched, initialIteratorExhausted); + this.command = command; + this.iterator = iterator; + this.followUpBounds = followUpBounds; + } + + @Override + ReadCommand command() + { + return command; + } + + @Override + protected AbstractBounds followUpBounds() + { + return followUpBounds; + } + + @Override + public TrackedDataResponse response() + { + PartitionIterator filtered = UnfilteredPartitionIterators.filter(iterator, command.nowInSec()); + PartitionIterator counted = Transformation.apply(filtered, mergedResultCounter); + PartitionIterator result = Transformation.apply(counted, new EmptyPartitionsDiscarder()); + return TrackedDataResponse.create(result, command.columnFilter()); + } + + @Override + public void close() + { + iterator.close(); + } } } diff --git a/src/java/org/apache/cassandra/service/reads/tracked/FilteredFollowupRead.java b/src/java/org/apache/cassandra/service/reads/tracked/FilteredFollowupRead.java index fe87dbad0f..d07539a460 100644 --- a/src/java/org/apache/cassandra/service/reads/tracked/FilteredFollowupRead.java +++ b/src/java/org/apache/cassandra/service/reads/tracked/FilteredFollowupRead.java @@ -109,7 +109,7 @@ class FilteredFollowupRead extends AsyncPromise { partialRead = new AtomicReference<>(); TrackedRead.Range rangeRead = makeFollowUpRead(command, followUpBounds, remaining, consistencyLevel, requestTime); - rangeRead.startLocal(requestTime, partialRead::set); + rangeRead.startLocal(requestTime, partialRead::set, TrackedLocalReads.Completer.DEFAULT); futures.add(rangeRead.future()); } else diff --git a/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedIndexRead.java b/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedIndexRead.java new file mode 100644 index 0000000000..e335104458 --- /dev/null +++ b/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedIndexRead.java @@ -0,0 +1,863 @@ +/* + * 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.service.reads.tracked; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.SortedMap; +import java.util.SortedSet; +import java.util.TreeMap; +import java.util.TreeSet; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.atomic.AtomicBoolean; + +import com.google.common.base.Preconditions; +import com.google.common.collect.Iterables; +import com.google.common.collect.Iterators; +import com.google.common.collect.PeekingIterator; + +import org.apache.cassandra.db.ColumnFamilyStore; +import org.apache.cassandra.db.ConsistencyLevel; +import org.apache.cassandra.db.DataRange; +import org.apache.cassandra.db.DecoratedKey; +import org.apache.cassandra.db.PartitionPosition; +import org.apache.cassandra.db.PartitionRangeReadCommand; +import org.apache.cassandra.db.ReadCommand; +import org.apache.cassandra.db.ReadExecutionController; +import org.apache.cassandra.db.ReadableView; +import org.apache.cassandra.db.SinglePartitionReadCommand; +import org.apache.cassandra.db.Slices; +import org.apache.cassandra.db.filter.ColumnFilter; +import org.apache.cassandra.db.lifecycle.SSTableSet; +import org.apache.cassandra.db.lifecycle.View; +import org.apache.cassandra.db.memtable.Memtable; +import org.apache.cassandra.db.partitions.Partition; +import org.apache.cassandra.db.partitions.PartitionIterator; +import org.apache.cassandra.db.partitions.PartitionUpdate; +import org.apache.cassandra.db.partitions.SimpleBTreePartition; +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.UnfilteredSource; +import org.apache.cassandra.db.transform.EmptyPartitionsDiscarder; +import org.apache.cassandra.db.transform.Transformation; +import org.apache.cassandra.dht.AbstractBounds; +import org.apache.cassandra.dht.ExcludingBounds; +import org.apache.cassandra.dht.Range; +import org.apache.cassandra.index.Index; +import org.apache.cassandra.index.Index.IndexMatch; +import org.apache.cassandra.index.transactions.UpdateTransaction; +import org.apache.cassandra.io.sstable.SSTableReadsListener; +import org.apache.cassandra.io.sstable.format.SSTableReader; +import org.apache.cassandra.io.util.FileUtils; +import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.tcm.ClusterMetadata; +import org.apache.cassandra.transport.Dispatcher; +import org.apache.cassandra.utils.AbstractIterator; +import org.apache.cassandra.utils.ByteBufferUtil; +import org.apache.cassandra.utils.CloseablePeekingIterator; +import org.apache.cassandra.utils.concurrent.AsyncPromise; +import org.apache.cassandra.utils.concurrent.Future; +import org.apache.cassandra.utils.concurrent.FutureCombiner; +import org.apache.cassandra.utils.concurrent.UncheckedInterruptedException; + +public class PartialTrackedIndexRead> extends PartialTrackedRead +{ + private final ReadCommand command; + private final Searcher searcher; + + private ConsistencyLevel consistencyLevel; + private Dispatcher.RequestTime requestTime; + + PartialTrackedIndexRead(ReadExecutionController executionController, ColumnFamilyStore cfs, long startTimeNanos, ReadCommand command, Searcher searcher) + { + super(executionController, cfs, startTimeNanos); + this.command = command; + this.searcher = searcher; + } + + public static > PartialTrackedIndexRead create(ReadExecutionController executionController, ColumnFamilyStore cfs, long startTimeNanos, ReadCommand command, Searcher searcher) + { + PartialTrackedIndexRead read = new PartialTrackedIndexRead<>(executionController, cfs, startTimeNanos, command, searcher); + read.prepare(null); + return read; + } + + @Override + public ReadCommand command() + { + return command; + } + + @Override + public Searcher searcher() + { + return searcher; + } + + @Override + public void setFollowUpReadContext(ConsistencyLevel consistencyLevel, Dispatcher.RequestTime requestTime) + { + this.consistencyLevel = consistencyLevel; + this.requestTime = requestTime; + } + + public interface CompletedIndexPartitionRead + { + UnfilteredRowIterator matchingRows(CloseablePeekingIterator matchIterator); + } + + public interface CompletedIndexRead extends CompletedRead + { + CompletedIndexPartitionRead partitionRead(DecoratedKey key); + CloseablePeekingIterator matchIterator(); + } + + private static DecoratedKey maxKey(DecoratedKey left, DecoratedKey right) + { + if (left == null) + return right; + if (right == null) + return left; + + return right.compareTo(left) > 0 ? right : left; + } + + private static class FollowUpRead> implements CompletedIndexPartitionRead, AutoCloseable + { + private final DecoratedKey key; + private final PartialTrackedIndexRead read; + private final CompletedIndexRead completedRead; + private final CompletedIndexPartitionRead partitionRead; + + public FollowUpRead(DecoratedKey key, PartialTrackedIndexRead read) + { + Preconditions.checkArgument(!read.command.isRangeRequest()); + this.key = key; + this.read = read; + this.completedRead = (CompletedIndexRead) read.complete(); + this.partitionRead = Preconditions.checkNotNull(completedRead.partitionRead(key)); + } + + static > Future> start(ReadCommand command, DecoratedKey key, ConsistencyLevel consistencyLevel, Dispatcher.RequestTime requestTime) + { + ClusterMetadata metadata = ClusterMetadata.current(); + Preconditions.checkState(command instanceof PartitionRangeReadCommand, "additional reads can only be made with range reads"); + + PartitionRangeReadCommand rangeReadCommand = (PartitionRangeReadCommand) command; + SinglePartitionReadCommand partitionReadCommand = SinglePartitionReadCommand.fromRangeRead(key, rangeReadCommand, rangeReadCommand.limits()); + + AsyncPromise> followUpPromise = new AsyncPromise<>(); + TrackedRead.Partition trackedRead = TrackedRead.Partition.create(metadata, partitionReadCommand, consistencyLevel, requestTime); + + trackedRead.startLocal(requestTime, null, ((promise1, read, consistencyLevel1, rt) -> { + try + { + followUpPromise.trySuccess(new FollowUpRead<>(key, (PartialTrackedIndexRead) read)); + } + catch (Exception e) + { + followUpPromise.tryFailure(e); + } + })); + return followUpPromise; + } + + @Override + public UnfilteredRowIterator matchingRows(CloseablePeekingIterator matchIterator) + { + Preconditions.checkState(matchIterator.hasNext()); + Preconditions.checkState(matchIterator.peek().key().equals(key)); + return partitionRead.matchingRows(matchIterator); + } + + @Override + public void close() + { + read.close(); + } + + static > void close(Map>> followUpReads) + { + for (Future> future : followUpReads.values()) + { + future.addCallback((followup, failure) -> { + if (failure != null) + followup.close(); + }); + } + } + + static > Map> getResults(Map>> futures, List> matchIterators) + { + Map> followupReads = new HashMap<>(); + for (Future> future : futures.values()) + { + try + { + FollowUpRead followUpRead = future.get(); + matchIterators.add(followUpRead.completedRead.matchIterator()); + followupReads.put(followUpRead.key, followUpRead); + } + catch (ExecutionException e) + { + throw new RuntimeException(e); + } + catch (InterruptedException e) + { + throw new UncheckedInterruptedException(e); + } + } + return followupReads; + } + + } + + private static class SnapshotView implements ReadableView + { + final List snapshots; + final List sstables; + private AugmentedPartition augmentedPartition = null; + + public SnapshotView(List snapshots, List sstables) + { + this.snapshots = snapshots; + this.sstables = sstables; + } + + public static SnapshotView create(DecoratedKey key, ColumnFamilyStore cfs) + { + ColumnFamilyStore.ViewFragment view = cfs.select(View.select(SSTableSet.LIVE, key)); + return new SnapshotView(MemtableSnapshot.create(key, view.memtables), view.sstables()); + } + + @Override + public Iterable memtables() + { + return snapshots; + } + + @Override + public List sstables() + { + return sstables; + } + + public void augment(PartitionUpdate update) + { + if (augmentedPartition == null) + { + augmentedPartition = new AugmentedPartition(update.partitionKey(), update.metadata()); + snapshots.add(augmentedPartition); + } + + augmentedPartition.augment(update); + } + } + + private static abstract class SinglePartitionSource implements UnfilteredSource + { + abstract Partition partition(); + + @Override + public UnfilteredRowIterator rowIterator(DecoratedKey key, Slices slices, ColumnFilter columnFilter, boolean reversed, SSTableReadsListener listener) + { + Partition partition = partition(); + Preconditions.checkState(key.equals(partition.partitionKey())); + return partition.unfilteredIterator(columnFilter, slices, reversed); + } + + @Override + public UnfilteredPartitionIterator partitionIterator(ColumnFilter columnFilter, DataRange dataRange, SSTableReadsListener listener) + { + throw new IllegalStateException("Range scans not supported"); + } + + @Override + public long getMinTimestamp() + { + return partition().stats().minTimestamp; + } + + @Override + public long getMinLocalDeletionTime() + { + return partition().stats().minLocalDeletionTime; + } + } + + private static class MemtableSnapshot extends SinglePartitionSource + { + private final Partition partition; + + public MemtableSnapshot(Partition partition) + { + this.partition = partition; + } + + static List create(DecoratedKey key, Iterable memtables) + { + List snapshots = new ArrayList<>(); + for (Memtable memtable : memtables) + { + Partition partition = memtable.snapshotPartition(key); + if (partition != null) + snapshots.add(new MemtableSnapshot(partition)); + } + return snapshots; + } + + @Override + Partition partition() + { + return partition; + } + } + + private static class AugmentedPartition extends SinglePartitionSource + { + private final SimpleBTreePartition data; + + AugmentedPartition(DecoratedKey key, TableMetadata metadata) + { + this.data = new SimpleBTreePartition(key, metadata, UpdateTransaction.NO_OP); + } + + void augment(PartitionUpdate update) + { + data.update(update); + } + + @Override + Partition partition() + { + return data; + } + } + + class AugmentableIndexPartitionRead implements CompletedIndexPartitionRead + { + private final DecoratedKey partitionKey; + private final SnapshotView view; + + AugmentableIndexPartitionRead(DecoratedKey partitionKey, SnapshotView view) + { + this.partitionKey = partitionKey; + this.view = view; + } + + void augment(PartitionUpdate update) + { + Preconditions.checkArgument(update.partitionKey().equals(partitionKey)); + view.augment(update); + } + + @Override + public UnfilteredRowIterator matchingRows(CloseablePeekingIterator matchIterator) + { + Preconditions.checkArgument(matchIterator.hasNext()); + Preconditions.checkArgument(matchIterator.peek().key().equals(partitionKey)); + return searcher.queryNextMatches(executionController, partitionKey, view, matchIterator); + } + } + + AugmentableIndexPartitionRead createRead(DecoratedKey key, ColumnFamilyStore cfs) + { + SnapshotView view = SnapshotView.create(key, cfs); + return new AugmentableIndexPartitionRead(key, view); + } + + @Override + protected Prepared prepareInternal(UnfilteredPartitionIterator initialData) + { + DecoratedKey maxKey = null; + SortedMap reads = new TreeMap<>(); + + if (command instanceof SinglePartitionReadCommand) + { + SinglePartitionReadCommand cmd = (SinglePartitionReadCommand) command; + DecoratedKey key = cmd.partitionKey(); + AugmentableIndexPartitionRead partitionRead = createRead(key, cfs); + reads.put(key, partitionRead); + maxKey = key; + } + + CloseablePeekingIterator matchIterator = searcher.matchIterator(executionController); + try + { + SortedSet materializedMatches = new TreeSet<>(searcher.matchComparator()); + while (matchIterator.hasNext() && materializedMatches.size() < command.limits().count()) + { + Match match = matchIterator.next(); + materializedMatches.add(match); + if (!reads.containsKey(match.key())) + { + DecoratedKey key = match.key(); + maxKey = maxKey(maxKey, key); + AugmentableIndexPartitionRead partitionRead = createRead(key, cfs); + reads.put(key, partitionRead); + } + } + return new IndexPrepared(maxKey, materializedMatches, matchIterator, reads); + } + catch (Throwable t) + { + FileUtils.closeQuietly(matchIterator); + throw t; + } + } + + @Override + public synchronized void complete(AsyncPromise promise, ConsistencyLevel consistencyLevel, Dispatcher.RequestTime requestTime) + { + Preconditions.checkState(state().isPrepared()); + IndexPrepared prepared = (IndexPrepared) state(); + + if (prepared.isCompletable()) + { + super.complete(promise, consistencyLevel, requestTime); + return; + } + + IndexPreComplete preComplete = prepared.preComplete(); + state = preComplete; + + // simple listener - completion will handle any failed futures + preComplete.future().addListener(() -> super.complete(promise, consistencyLevel, requestTime)); + } + + private abstract class AbstractIndexPrepared extends Prepared + { + protected DecoratedKey maxKey; + protected final SortedSet materializedMatches; + // there may be additional matches for keys we've already scanned, this allows us to read them before + // starting a short read + protected final CloseablePeekingIterator additionalMatches; + + protected final SortedMap reads; + + // for range scans, if we learn of new keys with matching contents as part of reconciliation, we need + // to do follow up reads against them since we didn't snapshot memtable contents for the keys during + // the prepare phase of the read. Futures for those reads are kept here + protected final Map>> followUpReads; + + public AbstractIndexPrepared(DecoratedKey maxKey, + SortedSet materializedMatches, + CloseablePeekingIterator additionalMatches, + SortedMap reads, + Map>> followUpReads) + { + this.maxKey = maxKey; + this.materializedMatches = materializedMatches; + this.reads = reads; + this.additionalMatches = additionalMatches; + this.followUpReads = followUpReads; + } + + boolean isCompletable() + { + return Iterables.all(followUpReads.values(), Future::isDone); + } + + @Override + Completed complete() + { + Preconditions.checkState(isCompletable()); + List> matchIterators = new ArrayList<>(followUpReads.size() + 1); + matchIterators.add(CloseablePeekingIterator.wrap(materializedMatches.iterator())); + Map> followUpResults = FollowUpRead.getResults(followUpReads, matchIterators); + return new IndexCompleted(maxKey, new MergingMatchIterator(matchIterators), additionalMatches, reads, followUpResults); + } + + @Override + void close() + { + FollowUpRead.close(followUpReads); + super.close(); + } + } + + private class IndexPrepared extends AbstractIndexPrepared + { + private Index.MultiStepSearcher.MatchIndexer matchIndexer = null; + + public IndexPrepared(DecoratedKey maxKey, SortedSet materializedMatches, CloseablePeekingIterator additionalMatches, SortedMap reads) + { + super(maxKey, materializedMatches, additionalMatches, reads, new HashMap<>()); + } + + private Index.MultiStepSearcher.MatchIndexer matchIndexer() + { + if (matchIndexer == null) + matchIndexer = searcher.matchIndexer(); + return matchIndexer; + } + + private boolean indexNewKey(PartitionUpdate update) + { + AtomicBoolean hasMatches = new AtomicBoolean(false); + matchIndexer().index(update, e -> hasMatches.set(true)); + return hasMatches.get(); + } + + private boolean indexUpdate(PartitionUpdate update) + { + int startingSize = materializedMatches.size(); + matchIndexer().index(update, materializedMatches::add); + return materializedMatches.size() > startingSize; + } + + @Override + public void augment(PartitionUpdate update) + { + Preconditions.checkState(consistencyLevel != null, + "PartialTrackedRead#setFollowUpReadContext needs to be called before making reads available for augmenting mutation"); + DecoratedKey key = update.partitionKey(); + AugmentableIndexPartitionRead read = reads.get(key); + if (read == null) + { + // TODO: maybe we should immediately start a follow up read if it's likely this key will be included in the response + if (!followUpReads.containsKey(key) && indexNewKey(update)) + { + maxKey = maxKey(maxKey, update.partitionKey()); + Future> followUpRead = FollowUpRead.start(command, update.partitionKey(), consistencyLevel, requestTime); + followUpReads.put(key, followUpRead); + } + return; + } + + read.augment(update); + indexUpdate(update); + } + + IndexPreComplete preComplete() + { + return new IndexPreComplete(maxKey, materializedMatches, additionalMatches, reads, followUpReads); + } + } + + private class IndexPreComplete extends AbstractIndexPrepared + { + public IndexPreComplete(DecoratedKey maxKey, SortedSet materializedMatches, CloseablePeekingIterator additionalMatches, SortedMap reads, Map>> followUpReads) + { + super(maxKey, materializedMatches, additionalMatches, reads, followUpReads); + } + + @Override + public void augment(PartitionUpdate update) + { + throw new IllegalStateException("Cannot augment reads pending completion"); + } + + Future>> future() + { + return FutureCombiner.allOf(followUpReads.values()); + } + } + + private class IndexCompleted extends Completed + { + private final DecoratedKey maxKey; + private final CloseablePeekingIterator materializedMatchIterator; + private final CloseablePeekingIterator additionalMatchIterator; + private final SortedMap reads; + private final Map> followUpReads; + + public IndexCompleted(DecoratedKey maxKey, CloseablePeekingIterator materializedMatchIterator, CloseablePeekingIterator additionalMatchIterator, SortedMap reads, Map> followUpReads) + { + this.maxKey = maxKey; + this.materializedMatchIterator = materializedMatchIterator; + this.additionalMatchIterator = additionalMatchIterator; + this.reads = reads; + this.followUpReads = followUpReads; + } + + @Override + protected CompletedRead getResult() + { + return new FilteringCompletedIndexRead(maxKey, materializedMatchIterator, additionalMatchIterator, reads, followUpReads); + } + } + + protected class MergingMatchIterator extends AbstractIterator + { + private final List> iterators; + private Match last; + + public MergingMatchIterator(List> iterators) + { + this.iterators = iterators; + } + + @Override + protected Match computeNext() + { + int minIdx = -1; + Match minMatch = null; + for (int i = 0, mi = iterators.size(); i < mi; i++) + { + CloseablePeekingIterator iterator = iterators.get(i); + + if (last != null) + searcher.matchComparator().consumeDuplicates(last, iterator); + + if (!iterator.hasNext()) + continue; + + if (minMatch == null) + { + minMatch = iterator.peek(); + minIdx = i; + continue; + } + + Match thisMatch = iterator.peek(); + int cmp = searcher.matchComparator().compare(thisMatch, minMatch); + if (cmp < 0) + { + minMatch = thisMatch; + minIdx = i; + } + else if (cmp == 0) + { + // if this iterator equals the current minimum, advance the iterator - we don't merge equal matches + iterator.next(); + } + } + + if (minMatch != null) + { + iterators.get(minIdx).next(); + last = minMatch; + return minMatch; + } + + return endOfData(); + } + + @Override + public void close() + { + FileUtils.closeQuietly(iterators); + } + } + + /** + * Merges a materialized iterator and an additional iterator. The additional iterator is meant to be the initial + * match iterator from the searcher. If we encounter previously unseen keys from the initial match iterator, it + * means that we're in a short read and need to start a follow-up read, which this iterator signals to the caller + */ + private class MergingStoppingMatchIterator extends AbstractIterator + { + private final DecoratedKey maxKey; + private final PeekingIterator materializedIterator; + private final CloseablePeekingIterator additionalIterator; + private boolean followUpRequired = false; + + public MergingStoppingMatchIterator(DecoratedKey maxKey, Iterator materializedIterator, CloseablePeekingIterator additionalIterator) + { + this.maxKey = maxKey; + this.materializedIterator = Iterators.peekingIterator(materializedIterator); + this.additionalIterator = additionalIterator; + } + + @Override + protected Match computeNext() + { + if (materializedIterator.hasNext() && additionalIterator.hasNext()) + { + int cmp = searcher.matchComparator().compare(materializedIterator.peek(), additionalIterator.peek(), true); + if (cmp == 0) + { + additionalIterator.next(); + return materializedIterator.next(); + } + else if (cmp < 0) + { + Match match = materializedIterator.next(); + searcher.matchComparator().consumeDuplicates(match, additionalIterator); + return match; + } + else + { + Match match = additionalIterator.next(); + searcher.matchComparator().consumeDuplicates(match, materializedIterator); + + DecoratedKey key = match.key(); + Preconditions.checkArgument(key.compareTo(maxKey) <= 0); + return match; + } + } + + if (materializedIterator.hasNext()) + return materializedIterator.next(); + + if (additionalIterator.hasNext()) + { + Match match = additionalIterator.next(); + DecoratedKey key = match.key(); + if (key.compareTo(maxKey) > 0) + { + Preconditions.checkArgument(command.isRangeRequest()); + followUpRequired = true; + return endOfData(); + } + return match; + } + + return endOfData(); + } + + @Override + public void close() + { + additionalIterator.close(); + } + } + + private class FilteringCompletedIndexRead extends ExtendingCompletedRead implements CompletedIndexRead + { + private final DecoratedKey maxKey; + private final MergingStoppingMatchIterator matchIterator; + private final SortedMap reads; + + final Map> followUpReads; + + public FilteringCompletedIndexRead(DecoratedKey maxKey, CloseablePeekingIterator materializedMatches, CloseablePeekingIterator additionalMatches, SortedMap reads, Map> followupReads) + { + super(command, materializedMatches.hasNext(), true); + this.maxKey = maxKey; + this.matchIterator = new MergingStoppingMatchIterator(maxKey, materializedMatches, additionalMatches); + this.reads = reads; + this.followUpReads = followupReads; + } + + @Override + public CloseablePeekingIterator matchIterator() + { + return matchIterator; + } + + @Override + ReadCommand command() + { + return command; + } + + @Override + protected AbstractBounds followUpBounds() + { + Preconditions.checkState(command.isRangeRequest()); + Preconditions.checkNotNull(maxKey); + AbstractBounds bounds = command.dataRange().keyRange(); + return bounds.inclusiveRight() + ? new Range<>(maxKey, bounds.right) + : new ExcludingBounds<>(maxKey, bounds.right); + } + + private class UnfilteredResultIterator extends AbstractIterator implements UnfilteredPartitionIterator + { + private final CloseablePeekingIterator matchIter; + + public UnfilteredResultIterator(CloseablePeekingIterator matchIter) + { + this.matchIter = matchIter; + } + + @Override + public TableMetadata metadata() + { + return command.metadata(); + } + + @Override + protected UnfilteredRowIterator computeNext() + { + for (;;) + { + if (!matchIter.hasNext()) + return endOfData(); + + DecoratedKey nextKey = matchIter.peek().key(); + AugmentableIndexPartitionRead read = reads.get(nextKey); + if (read != null) + return read.matchingRows(matchIter); + + FollowUpRead followUpRead = followUpReads.get(nextKey); + if (followUpRead == null) + throw new IllegalStateException("Received match for key without initial or followup read: " + ByteBufferUtil.bytesToHex(nextKey.getKey())); + + UnfilteredRowIterator next = followUpRead.matchingRows(matchIter); + if (next != null) + return next; + } + } + + @Override + public void close() + { + matchIter.close(); + } + } + + private PartitionIterator filter(UnfilteredPartitionIterator iterator) + { + iterator = searcher.filterCompletedRead(iterator); + iterator = command.completeTrackedRead(iterator, PartialTrackedIndexRead.this); + PartitionIterator filtered = UnfilteredPartitionIterators.filter(iterator, command.nowInSec()); + PartitionIterator counted = Transformation.apply(filtered, mergedResultCounter); + PartitionIterator result = Transformation.apply(counted, new EmptyPartitionsDiscarder()); + return result; + } + + @Override + public TrackedDataResponse response() + { + try (UnfilteredResultIterator iterator = new UnfilteredResultIterator(matchIterator)) + { + PartitionIterator filtered = filter(iterator); + return TrackedDataResponse.create(filtered, command.columnFilter()); + } + } + + @Override + protected boolean followUpRequired() + { + if (!command.isRangeRequest()) + return false; + return matchIterator.followUpRequired || super.followUpRequired(); + } + + @Override + public void close() + { + FileUtils.closeQuietly(matchIterator); + FileUtils.closeQuietly(followUpReads.values()); + } + + @Override + public CompletedIndexPartitionRead partitionRead(DecoratedKey key) + { + return reads.get(key); + } + } +} diff --git a/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedRangeRead.java b/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedRangeRead.java index 2f3b58dfdc..154b828370 100644 --- a/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedRangeRead.java +++ b/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedRangeRead.java @@ -25,7 +25,6 @@ import java.util.SortedMap; import java.util.TreeMap; import com.google.common.base.Preconditions; -import org.apache.cassandra.transport.Dispatcher; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -45,6 +44,7 @@ import org.apache.cassandra.db.partitions.AbstractUnfilteredPartitionIterator; import org.apache.cassandra.db.partitions.PartitionUpdate; import org.apache.cassandra.db.partitions.SimpleBTreePartition; 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.transform.Transformation; import org.apache.cassandra.dht.AbstractBounds; @@ -55,9 +55,10 @@ import org.apache.cassandra.index.transactions.UpdateTransaction; import org.apache.cassandra.locator.ReplicaPlan; import org.apache.cassandra.locator.ReplicaPlans; import org.apache.cassandra.schema.TableMetadata; +import org.apache.cassandra.transport.Dispatcher; import org.apache.cassandra.utils.concurrent.Future; -public abstract class PartialTrackedRangeRead extends AbstractPartialTrackedRead +public abstract class PartialTrackedRangeRead extends PartialTrackedRead { private static final Logger logger = LoggerFactory.getLogger(PartialTrackedRangeRead.class); @@ -111,19 +112,15 @@ public abstract class PartialTrackedRangeRead extends AbstractPartialTrackedRead protected static class ShortReadSupport { - final DecoratedKey lastPartitionKey; // key of the last observed partition final boolean partitionsFetched; // whether we've seen any new partitions since iteration start or last moreContents() call final boolean initialIteratorExhausted; final AbstractBounds followUpBounds; - boolean wasAugmented; ShortReadSupport(Builder builder, boolean initialIteratorExhausted, AbstractBounds followUpBounds) { - this.lastPartitionKey = builder.lastPartitionKey; this.partitionsFetched = builder.partitionsFetched; this.initialIteratorExhausted = initialIteratorExhausted; this.followUpBounds = followUpBounds; - this.wasAugmented = false; } protected static class Builder @@ -175,34 +172,19 @@ public abstract class PartialTrackedRangeRead extends AbstractPartialTrackedRead RangePrepared materialize(UnfilteredPartitionIterator inputIterator) { - try + try (inputIterator) { - UnfilteredPartitionIterator materialized = Transformation.apply(inputIterator, new Transformation() - { - @Override - protected UnfilteredRowIterator applyToPartition(UnfilteredRowIterator partition) - { - SimpleBTreePartition materialized = data.computeIfAbsent(partition.partitionKey(), key -> new SimpleBTreePartition(key, partition.metadata(), UpdateTransaction.NO_OP)); - materialized.update(PartitionUpdate.fromIterator(partition, command.columnFilter())); - shortReadSupport.lastPartitionKey = partition.partitionKey(); - shortReadSupport.partitionsFetched = true; - return queryPartition(materialized); - } - }); + UnfilteredPartitionIterator materialized = Transformation.apply(inputIterator, this); UnfilteredPartitionIterator filtered = filter(materialized); try (UnfilteredPartitionIterator iterator = shortReadSupport.counter.applyTo(filtered)) { - consume(iterator); + UnfilteredPartitionIterators.consume(iterator); } return createRangePrepared(); } - finally - { - inputIterator.close(); - } } @Override @@ -241,7 +223,7 @@ public abstract class PartialTrackedRangeRead extends AbstractPartialTrackedRead } @Override - public State augment(PartitionUpdate update) + public void augment(PartitionUpdate update) { // if the input iterator reached the row limit, then we can't apply any augmenting mutations that are past // the last materialized key. Since we wouldn't have materialized the local data for that key, applying an @@ -256,11 +238,10 @@ public abstract class PartialTrackedRangeRead extends AbstractPartialTrackedRead logger.trace("Ignoring unacceptable update from key {} on read {}", update.partitionKey(), PartialTrackedRangeRead.this); } wasAugmented = true; - return this; } } - protected abstract class RangeCompleted extends Completed + protected abstract class RangeCompleted extends AbstractCompleted { protected final SortedMap data; protected final ShortReadSupport shortReadSupport; @@ -337,18 +318,6 @@ public abstract class PartialTrackedRangeRead extends AbstractPartialTrackedRead command.clusteringIndexFilter(partition.partitionKey()).isReversed()); } - private static void consume(UnfilteredPartitionIterator iterator) - { - while (iterator.hasNext()) - { - try (UnfilteredRowIterator partition = iterator.next()) - { - while (partition.hasNext()) - partition.next(); - } - } - } - public AbstractBounds followUpBounds() { RangeCompleted completed = (RangeCompleted) state().asCompleted(); @@ -401,7 +370,7 @@ public abstract class PartialTrackedRangeRead extends AbstractPartialTrackedRead @Override protected CompletedRead extendRead(UnfilteredPartitionIterator iterator) { - return new ExtendingCompletedRead(command, iterator, shortReadSupport.partitionsFetched, shortReadSupport.initialIteratorExhausted, shortReadSupport.followUpBounds); + return new ExtendingCompletedRead.RangeRead(command, iterator, shortReadSupport.partitionsFetched, shortReadSupport.initialIteratorExhausted, shortReadSupport.followUpBounds); } } @@ -523,7 +492,7 @@ public abstract class PartialTrackedRangeRead extends AbstractPartialTrackedRead } } - static class FilteredCompletedRead extends ExtendingCompletedRead + static class FilteredCompletedRead extends ExtendingCompletedRead.RangeRead { private final DecoratedKey lastMatchingKey; private final SortedMap followUpReadInfo; @@ -537,7 +506,6 @@ public abstract class PartialTrackedRangeRead extends AbstractPartialTrackedRead /** * Even if we reached the limit during materialization, if there are keys ahead of the first materialized key * or interleaved with them, then we need to read them - * @return */ private boolean hasInterleavedFollowupKeys() { diff --git a/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedRead.java b/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedRead.java index 32409e59d0..11cb4cdffc 100644 --- a/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedRead.java +++ b/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedRead.java @@ -15,10 +15,11 @@ * See the License for the specific language governing permissions and * limitations under the License. */ - package org.apache.cassandra.service.reads.tracked; import com.google.common.base.Preconditions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.apache.cassandra.db.ColumnFamilyStore; import org.apache.cassandra.db.ConsistencyLevel; @@ -27,22 +28,257 @@ import org.apache.cassandra.db.ReadCommand; import org.apache.cassandra.db.ReadExecutionController; import org.apache.cassandra.db.filter.DataLimits; import org.apache.cassandra.db.partitions.PartitionIterator; +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.transform.RTBoundValidator; import org.apache.cassandra.index.Index; +import org.apache.cassandra.utils.concurrent.AsyncPromise; import org.apache.cassandra.utils.concurrent.Future; import org.apache.cassandra.replication.Log2OffsetsMap; import org.apache.cassandra.replication.MutationJournal; import org.apache.cassandra.replication.ShortMutationId; import org.apache.cassandra.transport.Dispatcher; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; -public interface PartialTrackedRead +public abstract class PartialTrackedRead { - Logger logger = LoggerFactory.getLogger(PartialTrackedRead.class); - - interface CompletedRead extends AutoCloseable + private static final Logger logger = LoggerFactory.getLogger(PartialTrackedRead.class); + + final ReadExecutionController executionController; + final ColumnFamilyStore cfs; + final long startTimeNanos; + + public PartialTrackedRead(ReadExecutionController executionController, ColumnFamilyStore cfs, long startTimeNanos) + { + this.executionController = executionController; + this.cfs = cfs; + this.startTimeNanos = startTimeNanos; + } + + public ReadExecutionController executionController() + { + return executionController; + } + + public ColumnFamilyStore cfs() + { + return cfs; + } + + public long startTimeNanos() + { + return startTimeNanos; + } + + abstract ReadCommand command(); + + public abstract Index.Searcher searcher(); + + protected interface Augmentable + { + void augment(PartitionUpdate update); + } + + protected static abstract class State + { + protected static final State CLOSED = new State() + { + @Override + String name() + { + return "closed"; + } + + @Override + boolean isClosed() + { + return true; + } + }; + + abstract String name(); + + Initialized asInitialized() + { + throw new IllegalStateException("State is " + name() + ", not " + Initialized.NAME); + } + + boolean isPrepared() + { + return false; + } + + Prepared asPrepared() + { + throw new IllegalStateException("State is " + name() + ", not " + Prepared.NAME); + } + + Completed asCompleted() + { + throw new IllegalStateException("State is " + name() + ", not " + Completed.NAME); + } + + Augmentable asAugmentable() + { + if (isPrepared()) return asPrepared(); + throw new IllegalStateException("State is " + name() + ", not augmentable"); + } + + boolean isClosed() + { + return false; + } + + void close() + { + } + } + + // TODO (expected): this is a redundant state, never exposed + protected final class Initialized extends State + { + static final String NAME = "initialized"; + + @Override + String name() + { + return NAME; + } + + @Override + Initialized asInitialized() + { + return this; + } + + Prepared prepare(UnfilteredPartitionIterator initialData) + { + return prepareInternal(initialData); + } + } + + protected abstract Prepared prepareInternal(UnfilteredPartitionIterator initialData); + + protected static abstract class Prepared extends State implements Augmentable + { + private static final String NAME = "prepared"; + + @Override + String name() + { + return NAME; + } + + @Override + boolean isPrepared() + { + return true; + } + + @Override + Prepared asPrepared() + { + return this; + } + + abstract Completed complete(); + } + + protected static abstract class Completed extends State + { + private static final String NAME = "completed"; + + @Override + String name() + { + return NAME; + } + + protected abstract CompletedRead getResult(); + } + + protected abstract class AbstractCompleted extends Completed + { + protected abstract UnfilteredPartitionIterator iterator(); + protected abstract CompletedRead createResult(UnfilteredPartitionIterator iterator); + + @Override + protected CompletedRead getResult() + { + UnfilteredPartitionIterator result = command().completeTrackedRead(iterator(), PartialTrackedRead.this); + // validate that the sequence of RT markers is correct: open is followed by close, deletion times for both + // ends equal, and there are no dangling RT bound in any partition. + result = RTBoundValidator.validate(result, RTBoundValidator.Stage.PROCESSED, true); + return createResult(result); + } + } + + protected State state = new Initialized(); + + protected synchronized State state() + { + return state; + } + + /** + * Implementors need to call this before returning this from createInProgressRead + * TODO (expected): this is a redundant transition from a redundant state (INITIALIZED) + */ + synchronized void prepare(UnfilteredPartitionIterator initialData) + { + logger.trace("Preparing read {}", this); + state = state.asInitialized().prepare(initialData); + } + + void augment(PartitionUpdate update) + { + state.asAugmentable().augment(update); + } + + public synchronized void augment(Mutation mutation) + { + PartitionUpdate update = mutation.getPartitionUpdate(command().metadata()); + if (update != null) + augment(update); + } + + void augment(Log2OffsetsMap augmentingOffsets) + { + augmentingOffsets.forEach(this::augment); + } + + void augment(ShortMutationId mutationId) + { + Mutation mutation = MutationJournal.instance.read(mutationId); + Preconditions.checkNotNull(mutation, "Missing mutation %s", mutationId); + if (!command().selectsKey(mutation.key())) + { + logger.trace("Skipping mutation {} - {} not in read range", mutationId, mutation.key()); + return; + } + augment(mutation); + } + + public synchronized CompletedRead complete() + { + Preconditions.checkState(state.isPrepared()); + Completed completed = state.asPrepared().complete(); + state = completed; + return completed.getResult(); + } + + public synchronized void close() + { + if (state.isClosed()) + return; + + logger.trace("Closing read {}", this); + state.close(); + executionController.close(); + state = State.CLOSED; + } + + public interface CompletedRead extends AutoCloseable { TrackedDataResponse response(); // must be called from the read stage Future followupRead(TrackedDataResponse initialResponse, ConsistencyLevel consistencyLevel, Dispatcher.RequestTime requestTime); @@ -86,36 +322,48 @@ public interface PartialTrackedRead } } - CompletedRead complete(); + /** + * Sets consistency level and expiration info to be used for follow up reads. Needs to be called before making the + * read available for receiving augmenting mutations + */ + void setFollowUpReadContext(ConsistencyLevel consistencyLevel, Dispatcher.RequestTime requestTime) {} - void augment(Mutation mutation); - - default void augment(Log2OffsetsMap augmentingOffsets) + void complete(AsyncPromise promise, ConsistencyLevel consistencyLevel, Dispatcher.RequestTime requestTime) { - augmentingOffsets.forEach(this::augment); + complete(promise, this, consistencyLevel, requestTime); } - default void augment(ShortMutationId mutationId) + static void complete(AsyncPromise promise, PartialTrackedRead read, ConsistencyLevel consistencyLevel, Dispatcher.RequestTime requestTime) { - Mutation mutation = MutationJournal.instance.read(mutationId); - Preconditions.checkNotNull(mutation, "Missing mutation %s", mutationId); - if (!command().selectsKey(mutation.key())) + try (CompletedRead completedRead = read.complete()) { - logger.trace("Skipping mutation {} - {} not in read range", mutationId, mutation.key()); - return; + TrackedDataResponse response = completedRead.response(); + Future followUp = completedRead.followupRead(response, consistencyLevel, requestTime); + + if (followUp != null) + { + followUp.addCallback((newResponse, error) -> { + if (error != null) + { + promise.tryFailure(error); + return; + } + promise.trySuccess(newResponse); + }); + } + else + { + promise.trySuccess(response); + } + } + catch (Exception e) + { + promise.tryFailure(e); + throw e; + } + finally + { + read.close(); } - augment(mutation); } - - ReadExecutionController executionController(); - - Index.Searcher searcher(); - - ColumnFamilyStore cfs(); - - long startTimeNanos(); - - ReadCommand command(); - - void close(); } diff --git a/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedSinglePartitionRead.java b/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedSinglePartitionRead.java index 895706f056..9a655ff354 100644 --- a/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedSinglePartitionRead.java +++ b/src/java/org/apache/cassandra/service/reads/tracked/PartialTrackedSinglePartitionRead.java @@ -36,7 +36,7 @@ import org.apache.cassandra.index.transactions.UpdateTransaction; import static org.apache.cassandra.db.partitions.UnfilteredPartitionIterators.MergeListener.NOOP; -public class PartialTrackedSinglePartitionRead extends AbstractPartialTrackedRead +public class PartialTrackedSinglePartitionRead extends PartialTrackedRead { private final Index.Searcher searcher; private final SinglePartitionReadCommand command; @@ -74,7 +74,7 @@ public class PartialTrackedSinglePartitionRead extends AbstractPartialTrackedRea } @Override - public State augment(PartitionUpdate update) + public void augment(PartitionUpdate update) { if (!update.partitionKey().equals(command.partitionKey())) throw new IllegalArgumentException(String.format("Received update for partition key %s but command was for %s", @@ -85,7 +85,6 @@ public class PartialTrackedSinglePartitionRead extends AbstractPartialTrackedRea augmentedData = new SimpleBTreePartition(command.partitionKey(), command.metadata(), UpdateTransaction.NO_OP); augmentedData.update(update); - return this; } @Override @@ -95,7 +94,7 @@ public class PartialTrackedSinglePartitionRead extends AbstractPartialTrackedRea } } - private class SinglePartitionCompleted extends Completed + private class SinglePartitionCompleted extends AbstractCompleted { private final UnfilteredPartitionIterator initialData; private final SimpleBTreePartition augmentedData; diff --git a/src/java/org/apache/cassandra/service/reads/tracked/TrackedLocalReads.java b/src/java/org/apache/cassandra/service/reads/tracked/TrackedLocalReads.java index fa6c3193b3..aa58dc3319 100644 --- a/src/java/org/apache/cassandra/service/reads/tracked/TrackedLocalReads.java +++ b/src/java/org/apache/cassandra/service/reads/tracked/TrackedLocalReads.java @@ -22,6 +22,8 @@ import java.util.Map; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.apache.cassandra.concurrent.Stage; import org.apache.cassandra.db.*; @@ -37,12 +39,8 @@ import org.apache.cassandra.service.reads.SpeculativeRetryPolicy; import org.apache.cassandra.tcm.ClusterMetadata; import org.apache.cassandra.transport.Dispatcher; import org.apache.cassandra.utils.concurrent.AsyncPromise; -import org.apache.cassandra.utils.concurrent.Future; import org.jctools.maps.NonBlockingHashMap; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - /** * Since the read reconciliations don't use 2 way callbacks, maps of active reads and reconciliations * are maintained and expired here. @@ -53,6 +51,12 @@ public class TrackedLocalReads implements ExpiredStatePurger.Expireable { private static final Logger logger = LoggerFactory.getLogger(TrackedLocalReads.class); + public interface Completer + { + void complete(AsyncPromise promise, PartialTrackedRead read, ConsistencyLevel consistencyLevel, Dispatcher.RequestTime requestTime); + TrackedLocalReads.Completer DEFAULT = (promise, read, consistencyLevel, requestTime) -> read.complete(promise, consistencyLevel, requestTime); + } + private final NonBlockingHashMap coordinators = new NonBlockingHashMap<>(); public TrackedLocalReads() @@ -66,7 +70,8 @@ public class TrackedLocalReads implements ExpiredStatePurger.Expireable ReadCommand command, ConsistencyLevel consistencyLevel, int[] summaryNodes, - Dispatcher.RequestTime requestTime) + Dispatcher.RequestTime requestTime, + TrackedLocalReads.Completer completer) { Keyspace keyspace = Keyspace.open(command.metadata().keyspace); ColumnFamilyStore cfs = keyspace.getColumnFamilyStore(command.metadata().id); @@ -96,18 +101,18 @@ public class TrackedLocalReads implements ExpiredStatePurger.Expireable } // TODO: confirm all summaryNodes are present in the replica plan AsyncPromise promise = new AsyncPromise<>(); - beginReadInternal(readId, command, replicaPlan, summaryNodes, requestTime, promise); + beginReadInternal(readId, command, replicaPlan, summaryNodes, requestTime, promise, completer); return promise; } // TODO (expected): skip local summaries and reconcile when summaryNodes is empty (e.g. for CL.ONE) - private void beginReadInternal( - TrackedRead.Id readId, - ReadCommand command, - ReplicaPlan.AbstractForRead replicaPlan, - int[] summaryNodes, - Dispatcher.RequestTime requestTime, - AsyncPromise promise) + private void beginReadInternal(TrackedRead.Id readId, + ReadCommand command, + ReplicaPlan.AbstractForRead replicaPlan, + int[] summaryNodes, + Dispatcher.RequestTime requestTime, + AsyncPromise promise, + TrackedLocalReads.Completer completer) { PartialTrackedRead read = null; MutationSummary secondarySummary; @@ -118,6 +123,7 @@ public class TrackedLocalReads implements ExpiredStatePurger.Expireable try { read = command.beginTrackedRead(controller); + read.setFollowUpReadContext(replicaPlan.consistencyLevel(), requestTime); // Create another summary once initial data has been read fully. We do this to catch // any mutations that may have arrived during initial read execution. secondarySummary = command.createMutationSummary(true); @@ -131,8 +137,7 @@ public class TrackedLocalReads implements ExpiredStatePurger.Expireable throw e; } - Coordinator coordinator = - new Coordinator(readId, promise, read, replicaPlan.consistencyLevel(), requestTime); + Coordinator coordinator = new Coordinator(readId, promise, read, replicaPlan.consistencyLevel(), requestTime, completer); coordinators.put(readId, coordinator); // TODO (expected): reconsider the approach to tracked mutation metrics @@ -185,19 +190,21 @@ public class TrackedLocalReads implements ExpiredStatePurger.Expireable private final PartialTrackedRead read; private final ConsistencyLevel consistencyLevel; private final Dispatcher.RequestTime requestTime; + private final Completer completer; - Coordinator( - TrackedRead.Id readId, - AsyncPromise promise, - PartialTrackedRead read, - ConsistencyLevel consistencyLevel, - Dispatcher.RequestTime requestTime) + Coordinator(TrackedRead.Id readId, + AsyncPromise promise, + PartialTrackedRead read, + ConsistencyLevel consistencyLevel, + Dispatcher.RequestTime requestTime, + Completer completer) { this.readId = readId; this.promise = promise; this.read = Preconditions.checkNotNull(read); this.consistencyLevel = consistencyLevel; this.requestTime = requestTime; + this.completer = completer; } boolean isPurgeable(long nanoTime) @@ -215,52 +222,20 @@ public class TrackedLocalReads implements ExpiredStatePurger.Expireable { logger.trace("Reconciliation completed for {}, missing {}", readId, augmentingOffsets); - Stage.READ.submit(() -> { - + Stage.READ.submit(() -> { try { read.augment(augmentingOffsets); - complete(); - } catch (Throwable t) { + // TODO: Does the implementation of this in FollowUpRead in PartialTrackedIndexRead need to be on a READ Stage thread? + completer.complete(promise, read, consistencyLevel, requestTime); + } + catch (Throwable t) + { logger.error("Exception thrown during read completion", t); promise.tryFailure(t); throw t; } }); } - - private void complete() - { - try (PartialTrackedRead.CompletedRead completedRead = read.complete()) - { - TrackedDataResponse response = completedRead.response(); - Future followUp = completedRead.followupRead(response, consistencyLevel, requestTime); - - if (followUp != null) - { - followUp.addCallback((newResponse, error) -> { - if (error != null) - { - promise.tryFailure(error); - return; - } - promise.trySuccess(newResponse); - }); - } - else - { - promise.trySuccess(response); - } - } - catch (Exception e) - { - promise.tryFailure(e); - throw e; - } - finally - { - read.close(); - } - } } } diff --git a/src/java/org/apache/cassandra/service/reads/tracked/TrackedRead.java b/src/java/org/apache/cassandra/service/reads/tracked/TrackedRead.java index 975c2742fc..27642583b2 100644 --- a/src/java/org/apache/cassandra/service/reads/tracked/TrackedRead.java +++ b/src/java/org/apache/cassandra/service/reads/tracked/TrackedRead.java @@ -18,8 +18,19 @@ package org.apache.cassandra.service.reads.tracked; +import java.io.IOException; +import java.util.Collections; +import java.util.Map; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.atomic.AtomicLong; +import java.util.function.Consumer; + import com.google.common.base.Preconditions; import com.google.common.collect.Iterables; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.apache.cassandra.concurrent.Stage; import org.apache.cassandra.db.*; @@ -47,18 +58,6 @@ import org.apache.cassandra.utils.concurrent.AsyncPromise; import org.apache.cassandra.utils.concurrent.Future; import org.apache.cassandra.utils.concurrent.UncheckedInterruptedException; -import java.io.IOException; -import java.util.Collections; -import java.util.Map; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.TimeoutException; -import java.util.concurrent.atomic.AtomicLong; -import java.util.function.Consumer; - -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; - import static java.util.concurrent.TimeUnit.NANOSECONDS; import static org.apache.cassandra.metrics.ClientRequestsMetricsHolder.readMetrics; @@ -249,7 +248,7 @@ public abstract class TrackedRead, P extends ReplicaPlan. return metadata.directory.peerId(replica.endpoint()).id(); } - private void start(Dispatcher.RequestTime requestTime, Consumer partialReadConsumer) + private void start(Dispatcher.RequestTime requestTime, Consumer partialReadConsumer, TrackedLocalReads.Completer completer) { // TODO: skip local coordination if this node knows its recovering from an outage // TODO: read speculation @@ -279,7 +278,7 @@ public abstract class TrackedRead, P extends ReplicaPlan. logger.trace("Locally coordinating {}", readId); Stage.READ.submit(() -> { AsyncPromise promise = - MutationTrackingService.instance.localReads().beginRead(readId, ClusterMetadata.current(), command, consistencyLevel, summaryNodes, requestTime); + MutationTrackingService.instance.localReads().beginRead(readId, ClusterMetadata.current(), command, consistencyLevel, summaryNodes, requestTime, completer); promise.addCallback((response, error) -> { if (error != null) { @@ -326,12 +325,12 @@ public abstract class TrackedRead, P extends ReplicaPlan. public void start(Dispatcher.RequestTime requestTime) { - start(requestTime, null); + start(requestTime, null, TrackedLocalReads.Completer.DEFAULT); } - public void startLocal(Dispatcher.RequestTime requestTime, Consumer partialReadConsumer) + public void startLocal(Dispatcher.RequestTime requestTime, Consumer partialReadConsumer, TrackedLocalReads.Completer completer) { - start(requestTime, partialReadConsumer); + start(requestTime, partialReadConsumer, completer); } private void onResponse(TrackedDataResponse response) @@ -450,7 +449,7 @@ public abstract class TrackedRead, P extends ReplicaPlan. AsyncPromise promise = MutationTrackingService.instance .localReads() - .beginRead(readId, metadata, command, consistencyLevel, summaryNodes, requestTime); + .beginRead(readId, metadata, command, consistencyLevel, summaryNodes, requestTime, TrackedLocalReads.Completer.DEFAULT); promise.addCallback((response, error) -> { if (error != null) { diff --git a/src/java/org/apache/cassandra/utils/AbstractIterator.java b/src/java/org/apache/cassandra/utils/AbstractIterator.java index 7dd32b8490..4a89001c19 100644 --- a/src/java/org/apache/cassandra/utils/AbstractIterator.java +++ b/src/java/org/apache/cassandra/utils/AbstractIterator.java @@ -18,12 +18,9 @@ */ package org.apache.cassandra.utils; -import java.util.Iterator; import java.util.NoSuchElementException; -import com.google.common.collect.PeekingIterator; - -public abstract class AbstractIterator implements Iterator, PeekingIterator, CloseableIterator +public abstract class AbstractIterator implements CloseablePeekingIterator { private static enum State { MUST_FETCH, HAS_NEXT, DONE, FAILED } diff --git a/src/java/org/apache/cassandra/utils/CloseablePeekingIterator.java b/src/java/org/apache/cassandra/utils/CloseablePeekingIterator.java new file mode 100644 index 0000000000..fa5d4dd7cb --- /dev/null +++ b/src/java/org/apache/cassandra/utils/CloseablePeekingIterator.java @@ -0,0 +1,40 @@ +/* + * 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.utils; + +import java.util.Iterator; + +import com.google.common.collect.PeekingIterator; + +public interface CloseablePeekingIterator extends PeekingIterator, CloseableIterator +{ + static CloseablePeekingIterator wrap(Iterator iterator) + { + return new AbstractIterator<>() + { + @Override + protected V computeNext() + { + if (!iterator.hasNext()) + return endOfData(); + return iterator.next(); + } + }; + } +} diff --git a/test/distributed/org/apache/cassandra/distributed/test/ReadRepairEmptyRangeTombstonesTestBase.java b/test/distributed/org/apache/cassandra/distributed/test/ReadRepairEmptyRangeTombstonesTestBase.java index d3ed8d0d7b..24c0adf44e 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/ReadRepairEmptyRangeTombstonesTestBase.java +++ b/test/distributed/org/apache/cassandra/distributed/test/ReadRepairEmptyRangeTombstonesTestBase.java @@ -81,9 +81,12 @@ public abstract class ReadRepairEmptyRangeTombstonesTestBase extends TestBaseImp for (int coordinator = 1; coordinator <= NUM_NODES; coordinator++) for (boolean paging : BOOLEANS) for (boolean reverse : BOOLEANS) - for (ReplicationType replication : ReplicationType.fixmeValues()) - result.add(new Object[]{ ReadRepairStrategy.BLOCKING, coordinator, paging, reverse, replication }); - result.add(new Object[]{ ReadRepairStrategy.NONE, 1, false, false, ReplicationType.untracked }); + for (ReplicationType replication : ReplicationType.values()) + { + ReadRepairStrategy rrStrategy = replication == ReplicationType.untracked ? ReadRepairStrategy.BLOCKING : ReadRepairStrategy.NONE; + result.add(new Object[]{ rrStrategy, coordinator, paging, reverse, replication }); + } + return result; } @@ -170,6 +173,11 @@ public abstract class ReadRepairEmptyRangeTombstonesTestBase extends TestBaseImp @Test public void testRangeQueriesWithRowsOvetrlappingWithTombstoneRangeStart() { + // Read-repair covers only the queried range, while mutation tracking pulls all missing mutations. + Object[][] postRepairInternalRows = replicationType == ReplicationType.tracked + ? new Object[][] { row(1), row(2), row(3), row(4), row(5), row(6) } + : new Object[][] { row(1), row(2), row(3), row(4), row(5) }; + tester().createTable("CREATE TABLE %s(k int, c int, PRIMARY KEY (k, c)) " + "WITH CLUSTERING ORDER BY (c %s) AND read_repair='%s'") .mutate(1, "DELETE FROM %s USING TIMESTAMP 1 WHERE k=0 AND c>=3 AND c<=6") @@ -185,7 +193,7 @@ public abstract class ReadRepairEmptyRangeTombstonesTestBase extends TestBaseImp .assertRowsDistributed("SELECT c FROM %s WHERE k=0 AND c>=2 AND c<=5", 1, row(2), row(3), row(4), row(5)) - .assertRowsInternal("SELECT c FROM %s", row(1), row(2), row(3), row(4), row(5)) + .assertRowsInternal("SELECT c FROM %s", postRepairInternalRows) .mutate(2, "DELETE FROM %s WHERE k=0 AND c>=1 AND c<=6") .assertRowsInternal("SELECT * FROM %s"); } @@ -196,6 +204,11 @@ public abstract class ReadRepairEmptyRangeTombstonesTestBase extends TestBaseImp @Test public void testRangeQueriesWithRowsOverlappingWithTombstoneRangeEnd() { + // Read-repair covers only the queried range, while mutation tracking pulls all missing mutations. + Object[][] postRepairInternalRows = replicationType == ReplicationType.tracked + ? new Object[][] { row(1), row(2), row(3), row(4), row(5), row(6) } + : new Object[][] { row(2), row(3), row(4), row(5), row(6) }; + tester().createTable("CREATE TABLE %s(k int, c int, PRIMARY KEY (k, c)) " + "WITH CLUSTERING ORDER BY (c %s) AND read_repair='%s'") .mutate(1, "DELETE FROM %s USING TIMESTAMP 1 WHERE k=0 AND c>=1 AND c<=4") @@ -211,7 +224,7 @@ public abstract class ReadRepairEmptyRangeTombstonesTestBase extends TestBaseImp .assertRowsDistributed("SELECT c FROM %s WHERE k=0 AND c>=3 AND c<=6", 1, row(3), row(4), row(5), row(6)) - .assertRowsInternal("SELECT c FROM %s", row(2), row(3), row(4), row(5), row(6)) + .assertRowsInternal("SELECT c FROM %s", postRepairInternalRows) .mutate(2, "DELETE FROM %s WHERE k=0 AND c>=1 AND c<=6") .assertRowsInternal("SELECT * FROM %s"); } @@ -222,6 +235,15 @@ public abstract class ReadRepairEmptyRangeTombstonesTestBase extends TestBaseImp @Test public void testPointQueriesWithRowsContainedInTombstoneRange() { + // Read-repair covers only the queried range, while mutation tracking pulls all missing mutations. + Object[][] postRepairInternalRows = replicationType == ReplicationType.tracked + ? new Object[][] { row(0, 0), row(0, 1), row(0, 2) } + : new Object[][] { row(0, 1), row(0, 2) }; + + @SuppressWarnings("ZeroLengthArrayAllocation") Object[][] postDeleteInternalRows = replicationType == ReplicationType.tracked + ? new Object[][] { row(0, 0) } + : new Object[0][0]; + tester().createTable("CREATE TABLE %s(k int, c int, PRIMARY KEY (k, c)) " + "WITH CLUSTERING ORDER BY (c %s) AND read_repair='%s'") .mutate(1, "DELETE FROM %s USING TIMESTAMP 1 WHERE k=0 AND c>0 AND c<3") @@ -231,9 +253,9 @@ public abstract class ReadRepairEmptyRangeTombstonesTestBase extends TestBaseImp .assertRowsDistributed("SELECT * FROM %s WHERE k=0 AND c=1", 1, row(0, 1)) .assertRowsDistributed("SELECT * FROM %s WHERE k=0 AND c=2", 1, row(0, 2)) .assertRowsDistributed("SELECT * FROM %s WHERE k=0 AND c=3", 0) - .assertRowsInternal("SELECT * FROM %s", row(0, 1), row(0, 2)) + .assertRowsInternal("SELECT * FROM %s", postRepairInternalRows) .mutate(2, "DELETE FROM %s WHERE k=0 AND c>0 AND c<3") - .assertRowsInternal("SELECT * FROM %s"); + .assertRowsInternal("SELECT * FROM %s", postDeleteInternalRows); } /** @@ -275,7 +297,7 @@ public abstract class ReadRepairEmptyRangeTombstonesTestBase extends TestBaseImp { String formattedQuery = String.format(query, qualifiedTableName); - if (strategy == ReadRepairStrategy.NONE) + if (strategy == ReadRepairStrategy.NONE && replicationType == ReplicationType.untracked) expectedRows = EMPTY_ROWS; else if (reverse) expectedRows = reverse(expectedRows); diff --git a/test/distributed/org/apache/cassandra/distributed/test/ReadRepairIndexTest.java b/test/distributed/org/apache/cassandra/distributed/test/ReadRepairIndexTest.java new file mode 100644 index 0000000000..37a6769f4e --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/test/ReadRepairIndexTest.java @@ -0,0 +1,227 @@ +/* + * 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; + +import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import org.junit.AfterClass; +import org.junit.Assume; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import org.apache.cassandra.distributed.Cluster; +import org.apache.cassandra.index.internal.CassandraIndex; +import org.apache.cassandra.index.sai.StorageAttachedIndex; +import org.apache.cassandra.schema.ReplicationType; +import org.apache.cassandra.service.reads.repair.ReadRepairStrategy; + +import static org.apache.cassandra.distributed.shared.AssertUtils.row; + +@RunWith(Parameterized.class) +public class ReadRepairIndexTest extends TestBaseImpl +{ + private static final int NUM_NODES = 2; + + enum IndexType + { + SECONDARY(CassandraIndex.NAME), + SAI(StorageAttachedIndex.NAME); + + final String name; + + IndexType(String name) + { + this.name = name; + } + + @Override + public String toString() + { + return name; + } + } + + /** + * The read repair strategy to be used + */ + @Parameterized.Parameter + public ReadRepairStrategy strategy; + + /** + * The node to be used as coordinator + */ + @Parameterized.Parameter(1) + public int coordinator; + + /** + * Whether to flush data after mutations + */ + @Parameterized.Parameter(2) + public boolean flush; + + /** + * Whether paging is used for the distributed queries + */ + @Parameterized.Parameter(3) + public boolean paging; + + @Parameterized.Parameter(4) + public ReplicationType replicationType; + + @SuppressWarnings("ClassEscapesDefinedScope") + @Parameterized.Parameter(5) + public IndexType indexType; + + + @Parameterized.Parameters(name = "{index}: strategy={0} coordinator={1} flush={2} paging={3} replication={4} index={5}") + public static Collection data() + { + List result = new ArrayList<>(); + for (int coordinator = 1; coordinator <= NUM_NODES; coordinator++) + for (boolean flush : BOOLEANS) + for (boolean paging : BOOLEANS) + for (ReplicationType replication : ReplicationType.values()) + for (IndexType indexType : IndexType.values()) + result.add(new Object[]{ ReadRepairStrategy.BLOCKING, coordinator, flush, paging, replication, indexType}); + return result; + } + + private static Cluster cluster; + + @BeforeClass + public static void setupCluster() throws IOException + { + cluster = Cluster.build(NUM_NODES) + .withConfig(config -> config.set("read_request_timeout", "1m") + .set("write_request_timeout", "1m")) + .start(); + } + + @AfterClass + public static void teardownCluster() + { + if (cluster != null) + cluster.close(); + } + + protected Tester tester(String restriction) + { + return new Tester(restriction, cluster, strategy, coordinator, flush, paging, replicationType, indexType); + } + + protected static class Tester extends ReadRepairQueryTester.AbstractTester + { + private int nameSeq = 0; + private final IndexType indexType; + + @SuppressWarnings("ClassEscapesDefinedScope") + public Tester(String restriction, Cluster cluster, ReadRepairStrategy strategy, int coordinator, boolean flush, boolean paging, ReplicationType replicationType, IndexType indexType) + { + super(restriction, cluster, strategy, coordinator, flush, paging, replicationType); + this.indexType = indexType; + } + + @Override + Tester self() + { + return this; + } + + Tester createIndex(String column) + { + String query = String.format("CREATE INDEX %s_index_%d ON %s(%s) USING '%s'", tableName, nameSeq++, qualifiedTableName, column, indexType); + cluster.schemaChange(query); + return this; + } + } + + /** + * A partition that would not be an index hit on one node would be on the other + */ + @Test + public void singlePartitionUpdatedPartition() + { + tester("WHERE k=1 AND v=2") + .createTable("CREATE TABLE %s (k int, c int, v int, PRIMARY KEY (k, c))") + .createIndex("v") + .mutate(2, "INSERT INTO %s (k, c, v) VALUES (1, 2, 2)") + .mutate(1, "INSERT INTO %s (k, c, v) VALUES (1, 1, 1)") + .queryColumns("k, c, v", 1, 0, + rows(row(1, 2, 2)), + rows(row(1, 2, 2)), + rows(row(1, 2, 2))) + .tearDown(1, + rows(row(1, 1, 1), row(1, 2, 2)), + rows(row(1, 2, 2))); + + } + @Test + public void rangeReadTest() + { + tester("WHERE v=2") + .createTable("CREATE TABLE %s (k int, v int, PRIMARY KEY (k))") + .createIndex("v") + .mutate(2, "INSERT INTO %s (k, v) VALUES (1, 2)") + .mutate(1, "INSERT INTO %s (k, v) VALUES (2, 1)") + .mutate(2, "INSERT INTO %s (k, v) VALUES (3, 1)") + .mutate(1, "INSERT INTO %s (k, v) VALUES (4, 2)") + .queryColumns("k, v", 2, 0, + rows(row(1, 2), row(4, 2)), + rows(row(1, 2), row(4, 2)), + rows(row(1, 2), row(4, 2))) + .tearDown(2, + rows(row(1, 2), row(2, 1), row(4, 2), row(3, 1)), + (replicationType.isTracked() + ? rows(row(1, 2), row(2, 1), row(4, 2), row(3, 1)) + : rows(row(1, 2), row(2, 1), row(4, 2))), + (replicationType.isTracked() + ? rows(row(1, 2), row(2, 1), row(4, 2), row(3, 1)) + : rows(row(1, 2), row(4, 2), row(3, 1)))); + } + + @Test + public void sortedRangeRead() + { + Assume.assumeTrue("CassandraIndex doesn't support numerical ranges", indexType == IndexType.SAI); + + tester("WHERE v>2") + .createTable("CREATE TABLE %s (k int, c int, v int, PRIMARY KEY (k, c))") + .createIndex("v") + .mutate(2, "INSERT INTO %s (k, c, v) VALUES (1, 2, 2)") + .mutate(1, "INSERT INTO %s (k, c, v) VALUES (1, 4, 4)") + .mutate(2, "INSERT INTO %s (k, c, v) VALUES (5, 2, 1)") + .mutate(1, "INSERT INTO %s (k, c, v) VALUES (8, 4, 3)") + .queryColumns("k, c, v", 2, 0, + rows(row(1, 4, 4), row(8, 4, 3)), + rows(row(1, 4, 4), row(8, 4, 3)), + rows(row(1, 4, 4), row(8, 4, 3))) + .tearDown(2, + rows(row(5, 2, 1), row(1, 2, 2), row(1, 4, 4), row(8, 4, 3)), + replicationType.isTracked() + ? rows(row(5, 2, 1), row(1, 2, 2), row(1, 4, 4), row(8, 4, 3)) + : rows(row(1, 4, 4), row(8, 4, 3)), + rows(row(5, 2, 1), row(1, 2, 2), row(1, 4, 4), row(8, 4, 3))); + + } +} diff --git a/test/distributed/org/apache/cassandra/distributed/test/ReadRepairQueryTester.java b/test/distributed/org/apache/cassandra/distributed/test/ReadRepairQueryTester.java index 447bdbde88..cbf711f203 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/ReadRepairQueryTester.java +++ b/test/distributed/org/apache/cassandra/distributed/test/ReadRepairQueryTester.java @@ -34,6 +34,7 @@ import org.apache.cassandra.service.reads.repair.ReadRepairStrategy; import static org.apache.cassandra.distributed.shared.AssertUtils.assertEquals; import static org.apache.cassandra.distributed.shared.AssertUtils.assertRows; +import static org.apache.cassandra.service.reads.repair.ReadRepairStrategy.BLOCKING; import static org.apache.cassandra.service.reads.repair.ReadRepairStrategy.NONE; /** @@ -109,8 +110,8 @@ public abstract class ReadRepairQueryTester extends TestBaseImpl for (boolean flush : BOOLEANS) for (boolean paging : BOOLEANS) for (ReplicationType replication : ReplicationType.values()) - result.add(new Object[]{ ReadRepairStrategy.BLOCKING, coordinator, flush, paging, replication }); - result.add(new Object[]{ ReadRepairStrategy.NONE, 1, false, false, ReplicationType.untracked }); + result.add(new Object[] { replication.isTracked() ? NONE : BLOCKING, coordinator, flush, paging, replication }); + return result; } @@ -137,12 +138,12 @@ public abstract class ReadRepairQueryTester extends TestBaseImpl return new Tester(restriction, cluster, strategy, coordinator, flush, paging, replicationType); } - protected static class Tester extends ReadRepairTester + static abstract class AbstractTester> extends ReadRepairTester { private final String restriction; // the tested CQL query WHERE restriction private final String allColumnsQuery; // a SELECT * query for the table using the tested restriction - Tester(String restriction, Cluster cluster, ReadRepairStrategy strategy, int coordinator, boolean flush, boolean paging, ReplicationType replicationType) + AbstractTester(String restriction, Cluster cluster, ReadRepairStrategy strategy, int coordinator, boolean flush, boolean paging, ReplicationType replicationType) { super(cluster, strategy, coordinator, flush, paging, false, replicationType); this.restriction = restriction; @@ -150,12 +151,6 @@ public abstract class ReadRepairQueryTester extends TestBaseImpl allColumnsQuery = String.format("SELECT * FROM %s %s", qualifiedTableName, restriction); } - @Override - Tester self() - { - return this; - } - /** * Runs the tested query with CL=ALL selectig only the specified columns and verifies that it returns the * specified rows. Then, it runs the query again selecting all the columns, and verifies that the first query @@ -168,12 +163,12 @@ public abstract class ReadRepairQueryTester extends TestBaseImpl * @param node1Rows the rows in the first node, which is the one with the most updated data * @param node2Rows the rows in the second node, which is the one meant to receive the RR writes */ - Tester queryColumns(String columns, - long columnsQueryRepairedRows, - long rowsQueryRepairedRows, - Object[][] columnsQueryResults, - Object[][] node1Rows, - Object[][] node2Rows) + T queryColumns(String columns, + long columnsQueryRepairedRows, + long rowsQueryRepairedRows, + Object[][] columnsQueryResults, + Object[][] node1Rows, + Object[][] node2Rows) { // query only the selected columns with CL=ALL to trigger partial read repair on that column String columnsQuery = String.format("SELECT %s FROM %s %s", columns, qualifiedTableName, restriction); @@ -206,7 +201,7 @@ public abstract class ReadRepairQueryTester extends TestBaseImpl assertRowsDistributed(columnsQuery, columnsQueryRepairedRows, columnsQueryResults); // query entire rows to repair the rest of the columns, that might trigger new repairs for those columns - return verifyQuery(allColumnsQuery, rowsQueryRepairedRows, node1Rows, node2Rows); + return verifyQuery(allColumnsQuery, rowsQueryRepairedRows, node1Rows, node1Rows, node2Rows); } /** @@ -223,13 +218,13 @@ public abstract class ReadRepairQueryTester extends TestBaseImpl * @param node1Rows the rows in the first node, which is the one with the most updated data * @param node2Rows the rows in the second node, which is the one meant to receive the RR writes */ - Tester deleteColumn(String columnDeletion, - String columns, - long columnsQueryRepairedRows, - long rowsQueryRepairedRows, - Object[][] columnsQueryResults, - Object[][] node1Rows, - Object[][] node2Rows) + T deleteColumn(String columnDeletion, + String columns, + long columnsQueryRepairedRows, + long rowsQueryRepairedRows, + Object[][] columnsQueryResults, + Object[][] node1Rows, + Object[][] node2Rows) { assert restriction != null; @@ -250,7 +245,7 @@ public abstract class ReadRepairQueryTester extends TestBaseImpl * Executes the specified row deletion on just one node and verifies the tested query, to ensure that the tested * query propagates the row deletion. */ - Tester deleteRows(String rowDeletion, long repairedRows, Object[][] node1Rows, Object[][] node2Rows) + T deleteRows(String rowDeletion, long repairedRows, Object[][] node1Rows, Object[][] node2Rows) { mutate(1, rowDeletion); @@ -259,28 +254,28 @@ public abstract class ReadRepairQueryTester extends TestBaseImpl if (replicationType.isTracked()) repairedRows = Math.min(repairedRows, 1); - return verifyQuery(allColumnsQuery, repairedRows, node1Rows, node2Rows); + return verifyQuery(allColumnsQuery, repairedRows, node1Rows, node1Rows, node2Rows); } - Tester mutate(String... queries) + T mutate(String... queries) { return mutate(1, queries); } - private Tester verifyQuery(String query, long expectedRepairedRows, Object[][] node1Rows, Object[][] node2Rows) + private T verifyQuery(String query, long expectedRepairedRows, Object[][] allRows, Object[][] node1Rows, Object[][] node2Rows) { // verify the per-replica status before running the query distributedly assertRows(cluster.get(1).executeInternal(query), node1Rows); - assertRows(cluster.get(2).executeInternal(query), strategy == NONE ? EMPTY_ROWS : node2Rows); + assertRows(cluster.get(2).executeInternal(query), strategy == NONE && !replicationType.isTracked() ? EMPTY_ROWS : node2Rows); // now, run the query with CL=ALL to reconcile and repair the replicas - assertRowsDistributed(query, expectedRepairedRows, node1Rows); + assertRowsDistributed(query, expectedRepairedRows, allRows); // run the query locally again to verify that the distributed query has repaired everything - assertRows(cluster.get(1).executeInternal(query), node1Rows); - assertRows(cluster.get(2).executeInternal(query), strategy == NONE ? EMPTY_ROWS : node1Rows); + assertRows(cluster.get(1).executeInternal(query), allRows); + assertRows(cluster.get(2).executeInternal(query), strategy == NONE && !replicationType.isTracked() ? EMPTY_ROWS : allRows); - return this; + return self(); } /** @@ -295,11 +290,11 @@ public abstract class ReadRepairQueryTester extends TestBaseImpl * Verifies the final status of the nodes with an unrestricted query, to ensure that the main tested query * hasn't triggered any unexpected repairs. Then, it verifies that the node that hasn't been used as coordinator * hasn't triggered any unexpected repairs. Finally, it drops the table. - * + *

* The expectUnrepaired flag is meant for range query tests where logged replication table special casing * doesn't apply since we do expect the final query to find and repair missing mutations */ - void tearDown(long repairedRows, Object[][] node1Rows, Object[][] node2Rows, boolean expectUnrepaired) + void tearDown(long repairedRows, Object[][] allRows, Object[][] node1Rows, Object[][] node2Rows, boolean expectUnrepaired) { if (replicationType.isTracked() && !expectUnrepaired) { @@ -321,7 +316,7 @@ public abstract class ReadRepairQueryTester extends TestBaseImpl // we also expect all pending mutations to be reconciled in the initial read, and none to be reconciled on the verification step repairedRows = 0; } - verifyQuery("SELECT * FROM " + qualifiedTableName, repairedRows, node1Rows, node2Rows); + verifyQuery("SELECT * FROM " + qualifiedTableName, repairedRows, allRows, node1Rows, node2Rows); for (int n = 1; n <= cluster.size(); n++) { if (n == coordinator) @@ -337,7 +332,37 @@ public abstract class ReadRepairQueryTester extends TestBaseImpl void tearDown(long repairedRows, Object[][] node1Rows, Object[][] node2Rows) { - tearDown(repairedRows, node1Rows, node2Rows, false); + tearDown(repairedRows, node1Rows, node1Rows, node2Rows, false); + } + + void tearDown(long repairedRows, Object[][] allRows, Object[][] node1Rows, Object[][] node2Rows) + { + tearDown(repairedRows, allRows, node1Rows, node2Rows, false); + } + + void tearDown(long repairedRows, Object[][] node1Rows, Object[][] node2Rows, boolean expectUnrepaired) + { + tearDown(repairedRows, node1Rows, node1Rows, node2Rows, expectUnrepaired); + } + } + + void tearDown(long repairedRows, Object[][] node1Rows, Object[][] node2Rows, boolean expectUnrepaired) + { + + } + + + protected static class Tester extends AbstractTester + { + public Tester(String restriction, Cluster cluster, ReadRepairStrategy strategy, int coordinator, boolean flush, boolean paging, ReplicationType replicationType) + { + super(restriction, cluster, strategy, coordinator, flush, paging, replicationType); + } + + @Override + Tester self() + { + return this; } } } diff --git a/test/distributed/org/apache/cassandra/distributed/test/ReadRepairTester.java b/test/distributed/org/apache/cassandra/distributed/test/ReadRepairTester.java index e599b982a1..5f92173306 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/ReadRepairTester.java +++ b/test/distributed/org/apache/cassandra/distributed/test/ReadRepairTester.java @@ -36,7 +36,6 @@ import org.apache.cassandra.service.reads.repair.ReadRepairStrategy; import static org.apache.cassandra.distributed.api.ConsistencyLevel.ALL; import static org.apache.cassandra.distributed.shared.AssertUtils.assertEquals; -import static org.apache.cassandra.distributed.test.TestBaseImpl.KEYSPACE; /** * Extensible helper class for read repair tests. @@ -46,7 +45,7 @@ public abstract class ReadRepairTester> private static final AtomicInteger seqNumber = new AtomicInteger(); private final String keyspaceName = "ks_" + seqNumber.getAndIncrement(); - private static final String tableName = "tbl"; + static final String tableName = "tbl"; final String qualifiedTableName = keyspaceName + '.' + tableName; protected final Cluster cluster; @@ -117,7 +116,7 @@ public abstract class ReadRepairTester> // flush the update node to ensure reads come from sstables if (flush) - cluster.get(node).flush(KEYSPACE); + cluster.get(node).flush(keyspaceName); return self(); } diff --git a/test/distributed/org/apache/cassandra/distributed/test/cql3/MultiNodeTableWalkWithMutationTrackingTest.java b/test/distributed/org/apache/cassandra/distributed/test/cql3/MultiNodeTableWalkWithMutationTrackingTest.java index 7d28f96e44..d5ea279521 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/cql3/MultiNodeTableWalkWithMutationTrackingTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/cql3/MultiNodeTableWalkWithMutationTrackingTest.java @@ -38,6 +38,7 @@ import org.apache.cassandra.utils.LoggingCommand; import static accord.utils.Property.commands; import static accord.utils.Property.stateful; +import static org.apache.cassandra.cql3.KnownIssue.AF_MULTI_NODE_MULTI_COLUMN_AND_NODE_LOCAL_WRITES; public class MultiNodeTableWalkWithMutationTrackingTest extends MultiNodeTableWalkBase { @@ -74,23 +75,27 @@ public class MultiNodeTableWalkWithMutationTrackingTest extends MultiNodeTableWa // if a failing seed is detected, populate here // Example: builder.withSeed(42L); // builder.withExamples(10); + // if a failing seed is detected, populate here + // Example: builder.withSeed(42L); // CQL operations may have opertors such as +, -, and / (example 4 + 4), to "apply" them to get a constant value // CQL_DEBUG_APPLY_OPERATOR = true; // When mutations look to be lost as seen by more complex SELECTs, it can be useful to just SELECT the partition/row right after to write to see if it was safe at the time. // READ_AFTER_WRITE = true; } + // TODO: Remove this override entirely when range reads and indexing are working properly together. @Override protected List supportedIndexers() { - return Collections.emptyList(); + return Collections.singletonList(CreateIndexDDL.SAI); } @Override protected void clusterConfig(IInstanceConfig c) { super.clusterConfig(c); - c.set("mutation_tracking_enabled", "true"); + c.set("mutation_tracking_enabled", true); + IGNORED_ISSUES.remove(AF_MULTI_NODE_MULTI_COLUMN_AND_NODE_LOCAL_WRITES); } @Test @@ -100,17 +105,20 @@ public class MultiNodeTableWalkWithMutationTrackingTest extends MultiNodeTableWa { Property.StatefulBuilder statefulBuilder = stateful().withExamples(10).withSteps(400); preCheck(cluster, statefulBuilder); + + // TODO: Uncomment the commented bits below to test range queries w/ the seeds above. statefulBuilder.check(commands(() -> rs -> createState(rs, cluster)) .add(StatefulASTBase::insert) - .add(StatefulASTBase::fullTableScan) - .addIf(State::allowUsingTimestamp, StatefulASTBase::validateUsingTimestamp) +// .add(StatefulASTBase::fullTableScan) +// .addIf(State::allowUsingTimestamp, StatefulASTBase::validateUsingTimestamp) .addIf(State::hasPartitions, this::selectExisting) - .addAllIf(State::supportTokens, this::selectToken, this::selectTokenRange, StatefulASTBase::selectMinTokenRange) +// .addAllIf(State::supportTokens, this::selectToken, this::selectTokenRange, StatefulASTBase::selectMinTokenRange) .addIf(State::hasEnoughMemtable, StatefulASTBase::flushTable) .addIf(State::hasEnoughSSTables, StatefulASTBase::compactTable) - .addIf(State::allowNonPartitionQuery, this::nonPartitionQuery) - .addIf(State::allowNonPartitionMultiColumnQuery, this::multiColumnQuery) +// .addIf(State::allowNonPartitionQuery, this::nonPartitionQuery) +// .addIf(State::allowNonPartitionMultiColumnQuery, this::multiColumnQuery) .addIf(State::allowPartitionQuery, this::partitionRestrictedQuery) + .addIf(State::allowPartitionMultiColumnQuery, this::multiColumnPartitionQuery) .destroyState(State::close) .commandsTransformer(LoggingCommand.factory()) .onSuccess(onSuccess(logger)) diff --git a/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java b/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java index dc71e8f96f..ec9df94d47 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/cql3/SingleNodeTableWalkTest.java @@ -135,16 +135,10 @@ public class SingleNodeTableWalkTest extends StatefulASTBase public Property.Command selectExisting(RandomSource rs, State state) { - NavigableSet keys = state.model.partitionKeys(); - BytesPartitionState.Ref ref = rs.pickOrderedSet(keys); - Clustering key = ref.key; - Select.Builder builder = Select.builder().table(state.metadata); - ImmutableUniqueList pks = state.model.factory.partitionColumns; - ImmutableUniqueList cks = state.model.factory.clusteringColumns; - for (Symbol pk : pks) - builder.value(pk, key.bufferAt(pks.indexOf(pk))); + BytesPartitionState.Ref ref = restrictPartition(rs, state, builder); + ImmutableUniqueList cks = state.model.factory.clusteringColumns; boolean wholePartition = cks.isEmpty() || rs.nextBoolean(); if (!wholePartition) { @@ -228,16 +222,8 @@ public class SingleNodeTableWalkTest extends StatefulASTBase public Property.Command partitionRestrictedQuery(RandomSource rs, State state) { - //TODO (now): remove duplicate logic - NavigableSet keys = state.model.partitionKeys(); - BytesPartitionState.Ref ref = rs.pickOrderedSet(keys); - Clustering key = ref.key; - Select.Builder builder = Select.builder().table(state.metadata); - ImmutableUniqueList pks = state.model.factory.partitionColumns; - for (Symbol pk : pks) - builder.value(pk, key.bufferAt(pks.indexOf(pk))); - + BytesPartitionState.Ref ref = restrictPartition(rs, state, builder); List searchableColumns = state.searchableNonPartitionColumns; Symbol symbol = rs.pick(searchableColumns); @@ -269,6 +255,18 @@ public class SingleNodeTableWalkTest extends StatefulASTBase return eqSearch(rs, state, symbol, value, builder); } + private static BytesPartitionState.Ref restrictPartition(RandomSource rs, State state, Select.Builder builder) + { + NavigableSet keys = state.model.partitionKeys(); + BytesPartitionState.Ref ref = rs.pickOrderedSet(keys); + Clustering key = ref.key; + + ImmutableUniqueList pks = state.model.factory.partitionColumns; + for (Symbol pk : pks) + builder.value(pk, key.bufferAt(pks.indexOf(pk))); + return ref; + } + public Property.Command nonPartitionQuery(RandomSource rs, State state) { Symbol symbol = rs.pick(state.searchableColumns); @@ -300,22 +298,31 @@ public class SingleNodeTableWalkTest extends StatefulASTBase public Property.Command multiColumnQuery(RandomSource rs, State state) { + Select.Builder builder = Select.builder().table(state.metadata).allowFiltering(); List allowedColumns = state.multiColumnQueryColumns(); + return multiColumnQuery(rs, state, builder, null, allowedColumns); + } + public Property.Command multiColumnPartitionQuery(RandomSource rs, State state) + { + Select.Builder builder = Select.builder().table(state.metadata).allowFiltering(); + BytesPartitionState.Ref ref = restrictPartition(rs, state, builder); + List allowedColumns = state.multiColumnPartitionQueryColumns(); + return multiColumnQuery(rs, state, builder, ref, allowedColumns); + } + + private static Property.Command multiColumnQuery(RandomSource rs, State state, Select.Builder builder, BytesPartitionState.Ref ref, List allowedColumns) + { if (allowedColumns.size() <= 1) throw new IllegalArgumentException("Unable to do multiple column query when there is only a single column"); int numColumns = rs.nextInt(1, allowedColumns.size()) + 1; - List cols = Gens.lists(Gens.pick(allowedColumns)).unique().ofSize(numColumns).next(rs); - Select.Builder builder = Select.builder().table(state.metadata).allowFiltering(); - for (Symbol symbol : cols) { - TreeMap> universe = state.model.index(symbol); + TreeMap> universe = ref == null ? state.model.index(symbol) : state.model.index(ref, symbol); NavigableSet allowed = Sets.filter(universe.navigableKeySet(), b -> !ByteBufferUtil.EMPTY_BYTE_BUFFER.equals(b)); - //TODO (now): support if (allowed.isEmpty()) return Property.ignoreCommand(); ByteBuffer value = rs.pickOrderedSet(allowed); @@ -456,6 +463,7 @@ public class SingleNodeTableWalkTest extends StatefulASTBase .addIf(State::allowNonPartitionMultiColumnQuery, this::multiColumnQuery) .addIf(State::allowPartitionQuery, this::partitionRestrictedQuery) .addIf(State::allowClusteringBetweenQuery, this::clusteringBetweenQuery) + .addIf(State::allowPartitionMultiColumnQuery, this::multiColumnPartitionQuery) .destroyState(State::close) .commandsTransformer(LoggingCommand.factory()) .onSuccess(onSuccess(logger)) @@ -661,6 +669,11 @@ public class SingleNodeTableWalkTest extends StatefulASTBase return allowNonPartitionQuery() && multiColumnQueryColumns().size() > 1; } + public boolean allowPartitionMultiColumnQuery() + { + return allowPartitionQuery() && multiColumnPartitionQueryColumns().size() > 1; + } + private List multiColumnQueryColumns() { List allowedColumns = searchableColumns; @@ -671,6 +684,16 @@ public class SingleNodeTableWalkTest extends StatefulASTBase return allowedColumns; } + private List multiColumnPartitionQueryColumns() + { + List allowedColumns = searchableNonPartitionColumns; + if (hasMultiNodeMultiColumnAllowFilteringWithLocalWritesIssue()) + allowedColumns = nonPkIndexedColumns; + if (IGNORED_ISSUES.contains(KnownIssue.SAI_AND_VECTOR_COLUMNS) && !indexes.isEmpty()) + allowedColumns = allowedColumns.stream().filter(s -> !s.type().isVector()).collect(Collectors.toList()); + return allowedColumns; + } + private boolean hasMultiNodeMultiColumnAllowFilteringWithLocalWritesIssue() { return isMultiNode() && IGNORED_ISSUES.contains(KnownIssue.AF_MULTI_NODE_MULTI_COLUMN_AND_NODE_LOCAL_WRITES); diff --git a/test/distributed/org/apache/cassandra/distributed/test/sai/PartialUpdateHandlingTest.java b/test/distributed/org/apache/cassandra/distributed/test/sai/PartialUpdateHandlingTest.java index d59b42b7cd..0bb326acca 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/sai/PartialUpdateHandlingTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/sai/PartialUpdateHandlingTest.java @@ -42,6 +42,7 @@ import org.apache.cassandra.distributed.Cluster; import org.apache.cassandra.distributed.api.ConsistencyLevel; import org.apache.cassandra.distributed.test.TestBaseImpl; import org.apache.cassandra.index.sai.plan.Expression; +import org.apache.cassandra.schema.ReplicationType; import static org.apache.cassandra.distributed.api.ConsistencyLevel.ALL; import static org.apache.cassandra.distributed.api.Feature.GOSSIP; @@ -70,7 +71,7 @@ import static org.junit.Assert.assertEquals; * across different partitions). * 5.) Whether data resides in SSTables or Memtables. The latter is implicitly unrepaired. * 6.) Interaction w/ existing mechanisms on the distributed read path that deal with short reads, replica filtering - * protection, etc. + * protection, mutation tracking, etc. * 7.) The relationship between columns selected and columns restricted by queries. (If coordinator filtering is * involved at the implementation level, retrieving enough information to do that filtering is important.) * 8.) The timestamps of partial updates and deletes, especially for single-column queries that might produce @@ -88,22 +89,28 @@ public class PartialUpdateHandlingTest extends TestBaseImpl @BeforeClass public static void setUpCluster() throws IOException { - CLUSTER = init(Cluster.build(NODES).withConfig(config -> config.set("hinted_handoff_enabled", false).with(GOSSIP).with(NETWORK)).start()); + CLUSTER = Cluster.build(NODES).withConfig(config -> config.set("hinted_handoff_enabled", false).with(GOSSIP).with(NETWORK)).start(); - // All parameterized test scenarios share the same table and attached indexes, but write to different partitions - // that are deleted after each scenario completes. - String createTableDDL = String.format("CREATE TABLE %s.%s (pk int, pk2 int, ck int, s int static, y int static, a int, b int, x int, PRIMARY KEY ((pk, pk2), ck)) WITH read_repair = 'NONE'", - KEYSPACE, TEST_TABLE_NAME); - CLUSTER.schemaChange(createTableDDL); - CLUSTER.disableAutoCompaction(KEYSPACE); + for (ReplicationType replicationType : ReplicationType.values()) + { + String keyspace = KEYSPACE + '_' + replicationType; + CLUSTER.schemaChange("CREATE KEYSPACE " + keyspace + " WITH replication = {'class': 'SimpleStrategy', 'replication_factor': " + CLUSTER.size() + "} AND replication_type = '" + replicationType + '\''); - CLUSTER.schemaChange(String.format("CREATE INDEX pk2_idx ON %s.%s(pk2) USING 'sai'", KEYSPACE, TEST_TABLE_NAME)); - CLUSTER.schemaChange(String.format("CREATE INDEX ck_idx ON %s.%s(ck) USING 'sai'", KEYSPACE, TEST_TABLE_NAME)); - CLUSTER.schemaChange(String.format("CREATE INDEX s_idx ON %s.%s(s) USING 'sai'", KEYSPACE, TEST_TABLE_NAME)); - CLUSTER.schemaChange(String.format("CREATE INDEX a_idx ON %s.%s(a) USING 'sai'", KEYSPACE, TEST_TABLE_NAME)); - CLUSTER.schemaChange(String.format("CREATE INDEX b_idx ON %s.%s(b) USING 'sai'", KEYSPACE, TEST_TABLE_NAME)); + // All parameterized test scenarios share the same table and attached indexes, but write to different partitions + // that are deleted after each scenario completes. + String createTableDDL = String.format("CREATE TABLE %s.%s (pk int, pk2 int, ck int, s int static, y int static, a int, b int, x int, PRIMARY KEY ((pk, pk2), ck)) WITH read_repair = 'NONE'", + keyspace, TEST_TABLE_NAME); + CLUSTER.schemaChange(createTableDDL); + CLUSTER.disableAutoCompaction(keyspace); - SAIUtil.waitForIndexQueryable(CLUSTER, KEYSPACE); + CLUSTER.schemaChange(String.format("CREATE INDEX pk2_idx ON %s.%s(pk2) USING 'sai'", keyspace, TEST_TABLE_NAME)); + CLUSTER.schemaChange(String.format("CREATE INDEX ck_idx ON %s.%s(ck) USING 'sai'", keyspace, TEST_TABLE_NAME)); + CLUSTER.schemaChange(String.format("CREATE INDEX s_idx ON %s.%s(s) USING 'sai'", keyspace, TEST_TABLE_NAME)); + CLUSTER.schemaChange(String.format("CREATE INDEX a_idx ON %s.%s(a) USING 'sai'", keyspace, TEST_TABLE_NAME)); + CLUSTER.schemaChange(String.format("CREATE INDEX b_idx ON %s.%s(b) USING 'sai'", keyspace, TEST_TABLE_NAME)); + + SAIUtil.waitForIndexQueryable(CLUSTER, keyspace); + } } static class Specification @@ -115,6 +122,7 @@ public class PartialUpdateHandlingTest extends TestBaseImpl final int partitionKey; final boolean flushPartials; final Expression.IndexOperator validationMode; + final ReplicationType replicationType; Specification(boolean restrictPartitionKey, String[] columns, @@ -122,7 +130,8 @@ public class PartialUpdateHandlingTest extends TestBaseImpl StatementType partialUpdateType, int partitionKey, boolean flushPartials, - Expression.IndexOperator validationMode) + Expression.IndexOperator validationMode, + ReplicationType replicationType) { this.restrictPartitionKey = restrictPartitionKey; this.columns = columns; @@ -131,12 +140,18 @@ public class PartialUpdateHandlingTest extends TestBaseImpl this.partitionKey = partitionKey; this.flushPartials = flushPartials; this.validationMode = validationMode; + this.replicationType = replicationType; } public String[] nonKeyColumns() { return Arrays.stream(columns).filter(c -> !c.equals("ck") && !c.equals("pk") && !c.equals("pk2")).toArray(String[]::new); } + + public String keyspaceName() + { + return KEYSPACE + '_' + replicationType; + } public String tableName() { @@ -152,7 +167,8 @@ public class PartialUpdateHandlingTest extends TestBaseImpl ", partialUpdateType=" + partialUpdateType + ", partitionKey=" + partitionKey + ", flushPartials=" + flushPartials + - ", validationMode=" + validationMode; + ", validationMode=" + validationMode + + ", replicationType=" + replicationType; } @Override @@ -163,13 +179,14 @@ public class PartialUpdateHandlingTest extends TestBaseImpl Specification that = (Specification) o; return Arrays.equals(columns, that.columns) && existing == that.existing && restrictPartitionKey == that.restrictPartitionKey - && partialUpdateType == that.partialUpdateType && partitionKey == that.partitionKey && flushPartials == that.flushPartials; + && partialUpdateType == that.partialUpdateType && partitionKey == that.partitionKey + && flushPartials == that.flushPartials && replicationType == that.replicationType; } @Override public int hashCode() { - int result = Objects.hash(existing, restrictPartitionKey, partialUpdateType, partitionKey, flushPartials); + int result = Objects.hash(existing, restrictPartitionKey, partialUpdateType, partitionKey, flushPartials, replicationType); result = 31 * result + Arrays.hashCode(columns); return result < 0 ? -result : result; } @@ -194,7 +211,7 @@ public class PartialUpdateHandlingTest extends TestBaseImpl { for (int i = 0; i < PARTITIONS_PER_TEST; i++) { - StringBuilder insert = new StringBuilder("INSERT INTO ").append(KEYSPACE).append('.').append(specification.tableName()); + StringBuilder insert = new StringBuilder("INSERT INTO ").append(specification.keyspaceName()).append('.').append(specification.tableName()); insert.append("(pk, pk2, ck"); for (Object column : specification.nonKeyColumns()) @@ -220,7 +237,7 @@ public class PartialUpdateHandlingTest extends TestBaseImpl CLUSTER.coordinator(1).execute(insert.toString(), ConsistencyLevel.ALL); } - CLUSTER.get(1).nodetoolResult("repair", KEYSPACE).asserts().success(); + CLUSTER.get(1).nodetoolResult("repair", specification.keyspaceName()).asserts().success(); } public void writeUnrepairedRows() @@ -269,7 +286,7 @@ public class PartialUpdateHandlingTest extends TestBaseImpl } String dml = String.format("INSERT INTO %s.%s(pk, pk2, ck, %s) VALUES (?, ?, 0, ?) USING TIMESTAMP %d", - KEYSPACE, specification.tableName(), column, nextTimestamp++); + specification.keyspaceName(), specification.tableName(), column, nextTimestamp++); CLUSTER.get(node).executeInternal(dml, partitionKey, partitionKey, value); node = nextNode(node); return node; @@ -283,11 +300,11 @@ public class PartialUpdateHandlingTest extends TestBaseImpl int partitionKey = specification.partitionKey + partitionIndex; String dml = String.format("DELETE %s FROM %s.%s USING TIMESTAMP %d WHERE pk = %d AND pk2 = %d AND ck = 0", - column, KEYSPACE, specification.tableName(), nextTimestamp++, partitionKey, partitionKey); + column, specification.keyspaceName(), specification.tableName(), nextTimestamp++, partitionKey, partitionKey); if (isStatic((String) column)) dml = String.format("DELETE %s FROM %s.%s USING TIMESTAMP %d WHERE pk = %d AND pk2 = %d", - column, KEYSPACE, specification.tableName(), nextTimestamp++, partitionKey, partitionKey); + column, specification.keyspaceName(), specification.tableName(), nextTimestamp++, partitionKey, partitionKey); CLUSTER.get(node).executeInternal(dml); node = nextNode(node); @@ -341,7 +358,7 @@ public class PartialUpdateHandlingTest extends TestBaseImpl for (Object column : specification.nonKeyColumns()) select.append(", ").append(column); - select.append(" FROM ").append(KEYSPACE).append('.').append(specification.tableName()).append(" WHERE "); + select.append(" FROM ").append(specification.keyspaceName()).append('.').append(specification.tableName()).append(" WHERE "); ArrayList restricted = Lists.newArrayList(specification.columns); @@ -400,6 +417,7 @@ public class PartialUpdateHandlingTest extends TestBaseImpl } } + @SuppressWarnings("ClassEscapesDefinedScope") @Parameterized.Parameter public Specification specification; @@ -417,33 +435,49 @@ public class PartialUpdateHandlingTest extends TestBaseImpl for (boolean restrictPartitionKey : new boolean[] { false, true }) { for (String[] columns : new String[][] { { "ck", "a" }, { "ck", "s" }, { "s", "a" }, { "a", "b" }, { "s", "x" }, { "s", "y" }, { "a", "x" }, { "a", "y" }, { "a" }, { "s" } }) - for (boolean existing : new boolean[] { false, true }) + { + for (boolean existing : new boolean[]{ false, true }) { - parameters.add(new Object[] { new Specification(restrictPartitionKey, columns, existing, StatementType.INSERT, nextPartitionKey, flushPartials, EQ) }); + parameters.add(new Object[]{ new Specification(restrictPartitionKey, columns, existing, StatementType.INSERT, nextPartitionKey, flushPartials, EQ, ReplicationType.untracked) }); nextPartitionKey += PARTITIONS_PER_TEST; + + if (restrictPartitionKey) + { + parameters.add(new Object[]{ new Specification(true, columns, existing, StatementType.INSERT, nextPartitionKey, flushPartials, EQ, ReplicationType.tracked) }); + nextPartitionKey += PARTITIONS_PER_TEST; + } } + } // Deletion scenarios assume existing data. for (String[] columns : new String[][] { { "s", "a" }, { "a", "b" }, { "s", "x" }, { "a", "x" }, { "a", "y" }, { "a" }, { "s" } }) { - parameters.add(new Object[] { new Specification(restrictPartitionKey, columns, true, StatementType.DELETE, nextPartitionKey, flushPartials, EQ) }); + parameters.add(new Object[] { new Specification(restrictPartitionKey, columns, true, StatementType.DELETE, nextPartitionKey, flushPartials, EQ, ReplicationType.untracked) }); nextPartitionKey += PARTITIONS_PER_TEST; + + if (restrictPartitionKey) + { + parameters.add(new Object[] { new Specification(true, columns, true, StatementType.DELETE, nextPartitionKey, flushPartials, EQ, ReplicationType.tracked) }); + nextPartitionKey += PARTITIONS_PER_TEST; + } } } // Note that scenarios around indexes on a partition key element only appear here where we neither // delete nor restrict on partition, as both would be nonsensical. for (String[] columns : new String[][] { { "pk2", "a" }, { "s", "a" }, { "a", "b" }, { "s", "x" }, { "a", "x" }, { "a", "y" }, { "a" }, { "s" } }) - for (boolean existing : new boolean[] { false, true }) + { + for (boolean existing : new boolean[]{ false, true }) { - parameters.add(new Object[]{ new Specification(false, columns, existing, StatementType.INSERT, nextPartitionKey, flushPartials, RANGE) }); + parameters.add(new Object[]{ new Specification(false, columns, existing, StatementType.INSERT, nextPartitionKey, flushPartials, RANGE, ReplicationType.untracked) }); nextPartitionKey += PARTITIONS_PER_TEST; } + } // Deletion scenarios assume existing data. for (String[] columns : new String[][] { { "s", "a" }, { "a", "b" }, { "s", "x" }, { "a", "x" }, { "a", "y" }, { "a" }, { "s" } }) { - parameters.add(new Object[]{ new Specification(false, columns, true, StatementType.DELETE, nextPartitionKey, flushPartials, RANGE) }); + parameters.add(new Object[]{ new Specification(false, columns, true, StatementType.DELETE, nextPartitionKey, flushPartials, RANGE, ReplicationType.untracked) }); nextPartitionKey += PARTITIONS_PER_TEST; } } @@ -468,7 +502,7 @@ public class PartialUpdateHandlingTest extends TestBaseImpl if (specification.flushPartials) // Flushg partial rows from Memtable-attached indexes to SSTable indexes: - CLUSTER.stream().forEach(i -> i.flush(KEYSPACE)); + CLUSTER.stream().forEach(i -> i.flush(specification.keyspaceName())); // If we wrote an initial (repaired) version of the row, do negative validation. // (i.e. Ensure queries that would have initially produced matches no longer do.) @@ -483,7 +517,7 @@ public class PartialUpdateHandlingTest extends TestBaseImpl @After public void truncateTable() { - CLUSTER.coordinator(1).execute(String.format("TRUNCATE TABLE %s.%s ", KEYSPACE, specification.tableName()), ALL); + CLUSTER.coordinator(1).execute(String.format("TRUNCATE TABLE %s.%s ", specification.keyspaceName(), specification.tableName()), ALL); } @AfterClass diff --git a/test/distributed/org/apache/cassandra/distributed/test/sai/StrictFilteringTest.java b/test/distributed/org/apache/cassandra/distributed/test/sai/StrictFilteringTest.java index cece824fa7..ffcfcccf7b 100644 --- a/test/distributed/org/apache/cassandra/distributed/test/sai/StrictFilteringTest.java +++ b/test/distributed/org/apache/cassandra/distributed/test/sai/StrictFilteringTest.java @@ -19,21 +19,29 @@ package org.apache.cassandra.distributed.test.sai; import java.io.IOException; +import java.util.ArrayList; +import java.util.Collection; import java.util.Iterator; +import java.util.List; import org.junit.AfterClass; +import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; import org.apache.cassandra.db.Keyspace; import org.apache.cassandra.distributed.Cluster; import org.apache.cassandra.distributed.api.ConsistencyLevel; import org.apache.cassandra.distributed.test.TestBaseImpl; +import org.apache.cassandra.schema.ReplicationType; import static org.apache.cassandra.distributed.api.Feature.GOSSIP; import static org.apache.cassandra.distributed.api.Feature.NETWORK; import static org.apache.cassandra.distributed.shared.AssertUtils.assertRows; import static org.apache.cassandra.distributed.shared.AssertUtils.row; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; /** @@ -42,16 +50,38 @@ import static org.junit.Assert.assertEquals; * * @see CASSANDRA-19018 */ +@RunWith(Parameterized.class) public class StrictFilteringTest extends TestBaseImpl { private static Cluster CLUSTER; + private static int keyspaceIdx; + + @Parameterized.Parameter + public ReplicationType replicationType; + @BeforeClass public static void setUpCluster() throws IOException { CLUSTER = init(Cluster.build(2).withConfig(config -> config.set("hinted_handoff_enabled", false).with(GOSSIP).with(NETWORK)).start()); } + @Parameterized.Parameters(name = "{index}: replication={0}") + public static Collection data() + { + List result = new ArrayList<>(); + for (ReplicationType replication : ReplicationType.values()) + result.add(new Object[]{replication}); + return result; + } + + @Before + public void setup() + { + KEYSPACE = "ks_" + keyspaceIdx++; + CLUSTER.schemaChange("CREATE KEYSPACE " + KEYSPACE + " WITH replication = {'class': 'SimpleStrategy', 'replication_factor': " + CLUSTER.size() + "} AND replication_type='" + replicationType + "';"); + } + @Test public void shouldDegradeToUnionOnSingleStatic() { @@ -260,7 +290,11 @@ public class StrictFilteringTest extends TestBaseImpl assertRows(initialRows, row(0, 4, 1)); Long srpRequestsAfter = CLUSTER.get(1).callOnInstance(() -> Keyspace.open(KEYSPACE).getColumnFamilyStore("necessary_short_read").metric.shortReadProtectionRequests.getCount()); - assertEquals(srpRequestsBefore + 2L, srpRequestsAfter.longValue()); + + if (replicationType == ReplicationType.untracked) + assertEquals(srpRequestsBefore + 2L, srpRequestsAfter.longValue()); + else if (replicationType == ReplicationType.tracked) + assertThat(srpRequestsAfter).isGreaterThanOrEqualTo(0L).isLessThanOrEqualTo(2L); } @Test diff --git a/test/distributed/org/apache/cassandra/distributed/test/tracking/MutationTrackingPartitionReadTest.java b/test/distributed/org/apache/cassandra/distributed/test/tracking/MutationTrackingPartitionReadTest.java new file mode 100644 index 0000000000..119ec61523 --- /dev/null +++ b/test/distributed/org/apache/cassandra/distributed/test/tracking/MutationTrackingPartitionReadTest.java @@ -0,0 +1,218 @@ +/* + * 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.math.BigInteger; +import java.util.Map; +import java.util.UUID; + +import org.junit.AfterClass; +import org.junit.BeforeClass; +import org.junit.Test; + +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.test.TestBaseImpl; +import org.apache.cassandra.distributed.test.sai.SAIUtil; + +import static org.apache.cassandra.distributed.shared.AssertUtils.assertRows; +import static org.apache.cassandra.distributed.shared.AssertUtils.row; +import static org.junit.Assert.assertEquals; + +public class MutationTrackingPartitionReadTest extends TestBaseImpl +{ + private static final int REPLICAS = 3; + + private static Cluster cluster; + + @BeforeClass + public static void setup() throws IOException + { + cluster = Cluster.build() + .withNodes(REPLICAS) + .withConfig(cfg -> cfg.with(Feature.NETWORK, Feature.GOSSIP) + .set("mutation_tracking_enabled", true) + .set("hinted_handoff_enabled", false)) + .start(); + } + + @AfterClass + public static void teardown() + { + if (cluster != null) + cluster.close(); + } + + @Test + public void testEqQueryOnStaticColumn() + { + String keyspace = "test_eq_query_on_static_column"; + cluster.schemaChange(withKeyspace("CREATE KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='tracked'", keyspace)); + + cluster.schemaChange(withKeyspace("CREATE TABLE %s.tbl (pk0 varint, pk1 uuid, ck0 time, s0 ascii static, v1 double, PRIMARY KEY ((pk0, pk1), ck0)) " + + "WITH CLUSTERING ORDER BY (ck0 DESC) AND read_repair = 'NONE'", keyspace)); + cluster.schemaChange(withKeyspace("CREATE INDEX tbl_s0 ON %s.tbl(s0) USING 'sai'", keyspace)); + SAIUtil.waitForIndexQueryable(cluster, keyspace); + cluster.forEach(i -> i.nodetoolResult("disableautocompaction", keyspace, "tbl").asserts().success()); + + cluster.get(3).executeInternal(withKeyspace("INSERT INTO %s.tbl (pk0, pk1, s0) VALUES (-58, 00000000-0000-4d00-8600-000000000000, 'foo') USING TIMESTAMP 5", keyspace)); + cluster.get(1).executeInternal(withKeyspace("DELETE s0, s0 FROM %s.tbl USING TIMESTAMP 13 WHERE pk0 = 7 AND pk1 = 00000000-0000-4e00-9600-000000000000", keyspace)); + cluster.get(1).executeInternal(withKeyspace("INSERT INTO %s.tbl (pk0, pk1, ck0, v1) VALUES (-58, 00000000-0000-4d00-8600-000000000000, '16:40:27.677919817', 1.6896613611522374E184) USING TIMESTAMP 14", keyspace)); + cluster.get(1).executeInternal(withKeyspace("UPDATE %s.tbl USING TIMESTAMP 15 SET v1=8.05223257349057E-164 WHERE pk0 = -58 AND pk1 = 00000000-0000-4d00-8600-000000000000 AND ck0 = '20:02:33.822429155'", keyspace)); + cluster.get(2).executeInternal(withKeyspace("INSERT INTO %s.tbl (pk0, pk1, s0) VALUES (-58, 00000000-0000-4d00-8600-000000000000, 'bar') USING TIMESTAMP 18", keyspace)); + + String select = withKeyspace("SELECT pk0, pk1, ck0 FROM %s.tbl WHERE pk0 = -58 AND pk1 = 00000000-0000-4d00-8600-000000000000 AND s0 = 'bar'", keyspace); + Object[][] result = cluster.coordinator(1).execute(select, ConsistencyLevel.ALL); + assertRows(result, row(BigInteger.valueOf(-58), UUID.fromString("00000000-0000-4d00-8600-000000000000"), 72153822429155L), + row(BigInteger.valueOf(-58), UUID.fromString("00000000-0000-4d00-8600-000000000000"), 60027677919817L)); + } + + @Test + public void testMissingPartitionDelete() + { + String keyspace = "test_missing_partition_delete"; + cluster.schemaChange(withKeyspace("CREATE KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='tracked'", keyspace)); + + cluster.schemaChange(withKeyspace("CREATE TABLE %s.tbl (pk0 text, pk1 bigint, ck0 smallint, v0 timestamp, v1 int, PRIMARY KEY ((pk0, pk1), ck0)) " + + "WITH CLUSTERING ORDER BY (ck0 ASC) AND read_repair = 'NONE'", keyspace)); + cluster.schemaChange(withKeyspace("CREATE INDEX tbl_v1_idx ON %s.tbl(v1) USING 'sai'", keyspace)); + SAIUtil.waitForIndexQueryable(cluster, keyspace); + cluster.forEach(i -> i.nodetoolResult("disableautocompaction", keyspace, "tbl").asserts().success()); + + cluster.get(2).executeInternal(withKeyspace("UPDATE %s.tbl USING TIMESTAMP 8 SET v1=1778069545 WHERE pk0 = 'ad1b:bbdc:e712:574:e7ca:104e:5abb:d9e1' AND pk1 = -5572993830691022649 AND ck0 = 32379", keyspace)); + cluster.get(1).executeInternal(withKeyspace("DELETE FROM %s.tbl USING TIMESTAMP 12 WHERE pk0 = 'ad1b:bbdc:e712:574:e7ca:104e:5abb:d9e1' AND pk1 = -5572993830691022649", keyspace)); + cluster.get(2).executeInternal(withKeyspace("UPDATE %s.tbl USING TIMESTAMP 14 SET v0=null, v1=1353378764 WHERE pk0 = 'ad1b:bbdc:e712:574:e7ca:104e:5abb:d9e1' AND pk1 = -5572993830691022649 AND ck0 = 29521", keyspace)); + + String select = withKeyspace("SELECT pk0, pk1, ck0, v1 FROM %s.tbl WHERE pk0 = 'ad1b:bbdc:e712:574:e7ca:104e:5abb:d9e1' AND pk1 = -5572993830691022649 AND v1 <= 1353378764 LIMIT 136", keyspace); + cluster.coordinator(1).execute(select, ConsistencyLevel.ALL); + + select = withKeyspace("SELECT pk0, pk1, ck0, v1 FROM %s.tbl WHERE pk0 = 'ad1b:bbdc:e712:574:e7ca:104e:5abb:d9e1' AND pk1 = -5572993830691022649 AND v1 >= 1353378764 LIMIT 116", keyspace); + Object[][] result = cluster.coordinator(1).execute(select, ConsistencyLevel.ALL); + + assertRows(result, row("ad1b:bbdc:e712:574:e7ca:104e:5abb:d9e1", -5572993830691022649L, (short) 29521, 1353378764)); + } + + @Test + public void testMultiColumnPartitionRestrictedQuery() + { + String keyspace = "test_multi_column_partition_restricted"; + cluster.schemaChange(withKeyspace("CREATE KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '3'} AND replication_type='tracked'", keyspace)); + + cluster.schemaChange(withKeyspace("CREATE TABLE %s.tbl (pk0 varint, ck0 timeuuid,ck1 uuid, s0 int static, s1 vector static, v0 frozen>>, v2 date, v3 int, v1 set>>, PRIMARY KEY (pk0, ck0, ck1)) " + + "WITH CLUSTERING ORDER BY (ck0 ASC, ck1 DESC) AND read_repair = 'NONE'", keyspace)); + + cluster.schemaChange(withKeyspace("CREATE INDEX tbl_ck0 ON %s.tbl(ck0) USING 'sai'", keyspace)); + cluster.schemaChange(withKeyspace("CREATE INDEX tbl_ck1 ON %s.tbl(ck1) USING 'sai'", keyspace)); + cluster.schemaChange(withKeyspace("CREATE INDEX tbl_v3 ON %s.tbl(v3) USING 'sai'", keyspace)); + SAIUtil.waitForIndexQueryable(cluster, keyspace); + cluster.forEach(i -> i.nodetoolResult("disableautocompaction", keyspace, "tbl").asserts().success()); + + cluster.get(1).executeInternal(withKeyspace("DELETE FROM %s.tbl USING TIMESTAMP 160 WHERE pk0 = -320778557", keyspace)); + cluster.get(3).executeInternal(withKeyspace("INSERT INTO %s.tbl (pk0, s0, s1) VALUES (-320778557, 1363549784, [00000000-0000-1d00-8300-000000000000, 00000000-0000-1a00-8900-000000000000]) USING TIMESTAMP 166", keyspace)); + cluster.get(1).executeInternal(withKeyspace("UPDATE %s.tbl USING TIMESTAMP 168 SET v1 = {{false}, {false, true}}, v3 = -1669443995, s0 = 1234171012 " + + "WHERE pk0 = -320778557 AND ck0 = 00000000-0000-1400-9600-000000000000 " + + "AND ck1 IN (00000000-0000-4e00-bf00-000000000000, 00000000-0000-4500-a100-000000000000, 00000000-0000-4000-9100-000000000000)", keyspace)); + + cluster.forEach(i -> i.nodetoolResult("flush", keyspace, "tbl").asserts().success()); + + String select = withKeyspace("SELECT * FROM %s.tbl WHERE pk0 = -320778557 AND ck1 = 00000000-0000-4c00-a700-000000000000 AND v3 = -1669443995 AND ck0 = 00000000-0000-1300-b800-000000000000 ALLOW FILTERING", keyspace); + Object[][] result = cluster.coordinator(2).execute(select, ConsistencyLevel.ALL); + assertRows(result); + } + + @Test + public void testMissingRowWithMultipleIndexedColumns() + { + String keyspace = "test_missing_row"; + cluster.schemaChange(withKeyspace("CREATE KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': '3'} AND replication_type='tracked'", keyspace)); + + cluster.schemaChange(withKeyspace("CREATE TABLE %s.tbl (pk0 timeuuid, pk1 timestamp, ck0 boolean, ck1 varint, v0 frozen>, v1 text, v2 frozen>, frozen>>>, v3 ascii, PRIMARY KEY ((pk0, pk1), ck0, ck1)) " + + "WITH CLUSTERING ORDER BY (ck0 DESC, ck1 ASC) AND read_repair = 'NONE'", keyspace)); + + cluster.schemaChange(withKeyspace("CREATE INDEX tbl_v2 ON %s.tbl(v2) USING 'sai'", keyspace)); + cluster.schemaChange(withKeyspace("CREATE INDEX tbl_v3 ON %s.tbl(v3) USING 'sai'", keyspace)); + SAIUtil.waitForIndexQueryable(cluster, keyspace); + + cluster.forEach(i -> i.nodetoolResult("disableautocompaction", keyspace, "tbl").asserts().success()); + + // Insert row on node3 at ts=11 with v2 and v3 + cluster.get(3).executeInternal(withKeyspace("INSERT INTO %s.tbl (pk0, pk1, ck0, ck1, v0, v1, v2, v3) " + + "VALUES (00000000-0000-1200-8700-000000000000, '2028-05-17T03:51:34.765Z', true, 0, {1, 2, 3}, 'test', ({13, 55}, [true]), 'original_value') " + + "USING TIMESTAMP 11", keyspace)); + + cluster.get(3).executeInternal(withKeyspace("DELETE FROM %s.tbl USING TIMESTAMP 12 " + + "WHERE pk0 = 00000000-0000-1200-8700-000000000000 AND pk1 = '2028-05-17T03:51:34.765Z' AND ck0 = false AND ck1 = 0", keyspace)); + + cluster.forEach(i -> i.nodetoolResult("flush", keyspace, "tbl").asserts().success()); + + cluster.get(3).executeInternal(withKeyspace("UPDATE %s.tbl USING TIMESTAMP 15 SET v0 = {10, 20}, v2 = ({-13, 44}, [false]), v1 = 'updated' " + + "WHERE pk0 = 00000000-0000-1200-8700-000000000000 AND pk1 = '2028-05-17T03:51:34.765Z' AND ck0 = true AND ck1 = 0", keyspace)); + + cluster.forEach(i -> i.nodetoolResult("flush", keyspace, "tbl").asserts().success()); + + String select = withKeyspace("SELECT pk0, pk1, ck0, ck1, v3 FROM %s.tbl " + + "WHERE pk0 = 00000000-0000-1200-8700-000000000000 AND pk1 = '2028-05-17T03:51:34.765Z' AND v2 = ({-13, 44}, [false]) AND v3 = 'original_value' " + + "LIMIT 47 ALLOW FILTERING", keyspace); + + Object[][] result = cluster.coordinator(1).execute(select, ConsistencyLevel.ALL); + + assertEquals("Query should return exactly 1 row", 1, result.length); + assertEquals("ck0 should be true", true, result[0][2]); + assertEquals("ck1 should be 0", BigInteger.ZERO, result[0][3]); + assertEquals("v3 should be 'original_value'", "original_value", result[0][4]); + } + + @Test + public void testStaticColumnUpdateWithRowQuery() + { + String keyspace = "test_static_column_update"; + cluster.schemaChange(withKeyspace("CREATE KEYSPACE %s WITH replication = {'class': 'SimpleStrategy', 'replication_factor': 3} AND replication_type='tracked'", keyspace)); + + cluster.schemaChange(withKeyspace("CREATE TABLE %s.tbl (pk0 int, pk1 text, ck0 int, s0 frozen> static, v0 int, v1 text, v3 int, PRIMARY KEY ((pk0, pk1), ck0)) " + + "WITH CLUSTERING ORDER BY (ck0 ASC) AND read_repair = 'NONE'", keyspace)); + + cluster.schemaChange(withKeyspace("CREATE INDEX tbl_ck0 ON %s.tbl(ck0) USING 'sai'", keyspace)); + cluster.schemaChange(withKeyspace("CREATE INDEX tbl_s0 ON %s.tbl(FULL(s0)) USING 'sai'", keyspace)); + SAIUtil.waitForIndexQueryable(cluster, keyspace); + cluster.forEach(i -> i.nodetoolResult("disableautocompaction", keyspace, "tbl").asserts().success()); + + // Step 1: UPDATE on node2 @ ts=6 - sets s0=null and regular columns for ck0=100 + cluster.get(2).executeInternal(withKeyspace("UPDATE %s.tbl USING TIMESTAMP 6 SET s0=null, v0=42, v1='value_from_ts6', v3=999 WHERE pk0 = 1 AND pk1 = 'partition1' AND ck0 = 100", keyspace)); + + // Flush to ensure data is in SSTables + cluster.forEach(i -> i.nodetoolResult("flush", keyspace, "tbl").asserts().success()); + + // Step 2: INSERT on node1 @ ts=23 - updates static column s0 only + cluster.get(1).executeInternal(withKeyspace("INSERT INTO %s.tbl (pk0, pk1, s0) VALUES (1, 'partition1', {'key1': 10, 'key2': 20, 'key3': 30}) USING TIMESTAMP 23", keyspace)); + cluster.forEach(i -> i.nodetoolResult("flush", keyspace, "tbl").asserts().success()); + + String select = withKeyspace("SELECT pk0, pk1, ck0, s0, v0, v1, v3 FROM %s.tbl " + + "WHERE pk0 = 1 AND pk1 = 'partition1' AND ck0 = 100 AND v0 = 42 AND v3 = 999 AND s0 = {'key1': 10, 'key2': 20, 'key3': 30} ALLOW FILTERING", keyspace); + + Object[][] result = cluster.coordinator(2).execute(select, ConsistencyLevel.ALL); + assertRows(result, row(1, "partition1", 100, Map.of("key1", 10, "key2", 20, "key3", 30), 42, "value_from_ts6", 999)); + } + + public static String withKeyspace(String replaceIn, String keyspace) + { + return String.format(replaceIn, keyspace); + } +} diff --git a/test/distributed/org/apache/cassandra/fuzz/sai/MultiNodeSAIMutationTrackingTest.java b/test/distributed/org/apache/cassandra/fuzz/sai/MultiNodeSAIMutationTrackingTest.java new file mode 100644 index 0000000000..a12c0fca4b --- /dev/null +++ b/test/distributed/org/apache/cassandra/fuzz/sai/MultiNodeSAIMutationTrackingTest.java @@ -0,0 +1,49 @@ +/* + * 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.fuzz.sai; + +import org.apache.cassandra.harry.SchemaSpec; +import org.apache.cassandra.harry.gen.Generator; + +public class MultiNodeSAIMutationTrackingTest extends MultiNodeSAITestBase +{ + public MultiNodeSAIMutationTrackingTest() + { + super(null); + } + + @Override + protected String replicationType() + { + return "tracked"; + } + + @Override + protected Generator schemaGenerator(boolean disableReadRepair) + { + // Mutation tracking does not require read-repair + return super.schemaGenerator(true); + } + + @Override + protected void repair(SchemaSpec schema) + { + // Mutation tracking should not require normal repairs + } +} \ No newline at end of file diff --git a/test/distributed/org/apache/cassandra/fuzz/sai/SingleNodeSAITestBase.java b/test/distributed/org/apache/cassandra/fuzz/sai/SingleNodeSAITestBase.java index bbe74c34a1..9aa7cc7ebd 100644 --- a/test/distributed/org/apache/cassandra/fuzz/sai/SingleNodeSAITestBase.java +++ b/test/distributed/org/apache/cassandra/fuzz/sai/SingleNodeSAITestBase.java @@ -67,8 +67,6 @@ public abstract class SingleNodeSAITestBase extends TestBaseImpl private static final int COMPACTION_SKIP = 4435; private static final int DEFAULT_REPAIR_SKIP = 8869; - private static final int OPERATIONS_PER_RUN = 30_000; - private static final int NUM_PARTITIONS = 64; private static final int NUM_VISITED_PARTITIONS = 16; protected static final int MAX_PARTITION_SIZE = 2000; @@ -90,6 +88,11 @@ public abstract class SingleNodeSAITestBase extends TestBaseImpl return 5; } + protected int operationsPerRun() + { + return 30_000; + } + @BeforeClass public static void before() throws Throwable { @@ -113,6 +116,7 @@ public abstract class SingleNodeSAITestBase extends TestBaseImpl cluster.startup(); cluster = init(cluster); } + @AfterClass public static void afterClass() { @@ -122,8 +126,7 @@ public abstract class SingleNodeSAITestBase extends TestBaseImpl @Before public void beforeEach() { - cluster.schemaChange("DROP KEYSPACE IF EXISTS " + KEYSPACE); - cluster.schemaChange("CREATE KEYSPACE " + KEYSPACE + " WITH replication = {'class': 'SimpleStrategy', 'replication_factor': " + rf() + "};"); + cluster.schemaChange("CREATE KEYSPACE IF NOT EXISTS " + KEYSPACE + " WITH replication = {'class': 'SimpleStrategy', 'replication_factor': " + rf() + "} AND replication_type = '" + replicationType() + "';"); } protected int rf() @@ -131,6 +134,11 @@ public abstract class SingleNodeSAITestBase extends TestBaseImpl return 1; } + protected String replicationType() + { + return "untracked"; + } + @Test public void simplifiedSaiTest() { @@ -235,7 +243,7 @@ public abstract class SingleNodeSAITestBase extends TestBaseImpl if (IndexTermType.isEqOnlyType(schema.clusteringKeys.get(i).type.asServerType())) eqOnlyClusteringColumns.add(i); - for (int i = 0; i < OPERATIONS_PER_RUN; i++) + for (int i = 0; i < operationsPerRun(); i++) { int partitionIndex = pkGen.generate(rng); HistoryBuilderHelper.insertRandomData(schema, partitionIndex, ckGen.generate(rng), rng, 0.5d, history); @@ -307,8 +315,7 @@ public abstract class SingleNodeSAITestBase extends TestBaseImpl protected Generator schemaGenerator(boolean disableReadRepair) { - SchemaSpec.OptionsBuilder builder = SchemaSpec.optionsBuilder().disableReadRepair(disableReadRepair) - .compactionStrategy("LeveledCompactionStrategy"); + SchemaSpec.OptionsBuilder builder = SchemaSpec.optionsBuilder().disableReadRepair(disableReadRepair).compactionStrategy("LeveledCompactionStrategy"); return SchemaGenerators.schemaSpecGen(KEYSPACE, "basic_sai", MAX_PARTITION_SIZE, builder); } diff --git a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java index 56f35fe180..f7921785c6 100644 --- a/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java +++ b/test/unit/org/apache/cassandra/db/ColumnFamilyStoreTest.java @@ -59,6 +59,7 @@ import org.apache.cassandra.db.lifecycle.SSTableSet; import org.apache.cassandra.db.memtable.AbstractMemtable; import org.apache.cassandra.db.memtable.Memtable; import org.apache.cassandra.db.partitions.FilteredPartition; +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.rows.Cell; @@ -968,6 +969,12 @@ public class ColumnFamilyStoreTest { } + @Override + public Partition snapshotPartition(DecoratedKey partitionKey) + { + return null; + } + @Override public UnfilteredRowIterator rowIterator(DecoratedKey key, Slices slices,