Avoid executing commands that have guaranteed empty bounds in slices filter

Patch by Alex Petrov; reviewed by Marcus Eriksson and Aleksey Yeschenko for CASSANDRA-16435.
This commit is contained in:
Alex Petrov 2021-02-10 15:39:41 +01:00
parent 6f13c864a0
commit 9bc8d0b452
9 changed files with 74 additions and 18 deletions

View File

@ -512,7 +512,7 @@ public class SelectStatement implements CQLStatement
return ReadQuery.empty(table);
ClusteringIndexFilter filter = makeClusteringIndexFilter(options, columnFilter);
if (filter == null)
if (filter == null || filter.isEmpty(table.comparator))
return ReadQuery.empty(table);
RowFilter rowFilter = getRowFilter(options);

View File

@ -382,6 +382,9 @@ public class SinglePartitionReadCommand extends ReadCommand implements SinglePar
public PartitionIterator execute(ConsistencyLevel consistency, ClientState clientState, long queryStartNanoTime) throws RequestExecutionException
{
if (clusteringIndexFilter.isEmpty(metadata().comparator))
return EmptyIterators.partition();
return StorageProxy.read(Group.one(this), consistency, clientState, queryStartNanoTime);
}

View File

@ -195,26 +195,37 @@ public class Slice
if (reversed)
{
int cmp = comparator.compare(lastReturned, start);
if (cmp < 0 || (!inclusive && cmp == 0))
assert cmp != 0;
// start is > than lastReturned; got nothing to return no more
if (cmp < 0)
return null;
cmp = comparator.compare(end, lastReturned);
if (cmp < 0 || (inclusive && cmp == 0))
assert cmp != 0;
if (cmp < 0)
return this;
return new Slice(start, inclusive ? ClusteringBound.inclusiveEndOf(lastReturned) : ClusteringBound.exclusiveEndOf(lastReturned));
Slice slice = new Slice(start, inclusive ? ClusteringBound.inclusiveEndOf(lastReturned) : ClusteringBound.exclusiveEndOf(lastReturned));
if (slice.isEmpty(comparator))
return null;
return slice;
}
else
{
int cmp = comparator.compare(end, lastReturned);
if (cmp < 0 || (!inclusive && cmp == 0))
assert cmp != 0;
if (cmp < 0)
return null;
cmp = comparator.compare(lastReturned, start);
if (cmp < 0 || (inclusive && cmp == 0))
assert cmp != 0;
if (cmp < 0)
return this;
return new Slice(inclusive ? ClusteringBound.inclusiveStartOf(lastReturned) : ClusteringBound.exclusiveStartOf(lastReturned), end);
Slice slice = new Slice(inclusive ? ClusteringBound.inclusiveStartOf(lastReturned) : ClusteringBound.exclusiveStartOf(lastReturned), end);
if (slice.isEmpty(comparator))
return null;
return slice;
}
}

View File

@ -42,6 +42,11 @@ public abstract class AbstractClusteringIndexFilter implements ClusteringIndexFi
return reversed;
}
public boolean isEmpty(ClusteringComparator comparator)
{
return false;
}
protected abstract void serializeInternal(DataOutputPlus out, int version) throws IOException;
protected abstract long serializedSizeInternal(int version);

View File

@ -37,7 +37,7 @@ import org.apache.cassandra.schema.TableMetadata;
*/
public interface ClusteringIndexFilter
{
public static Serializer serializer = AbstractClusteringIndexFilter.serializer;
public static final Serializer serializer = AbstractClusteringIndexFilter.serializer;
public enum Kind
{
@ -64,6 +64,8 @@ public interface ClusteringIndexFilter
*/
public boolean isReversed();
public boolean isEmpty(ClusteringComparator comparator);
/**
* Returns a filter for continuing the paging of this filter given the last returned clustering prefix.
*

View File

@ -56,6 +56,13 @@ public class ClusteringIndexSliceFilter extends AbstractClusteringIndexFilter
return slices.size() == 1 && !slices.hasLowerBound() && !slices.hasUpperBound();
}
// Whether or not it is guaranteed that slices are empty. Since we'd like to avoid iteration in general case,
// we rely on Slices#forPaging and SelectStatement#makeSlices to skip empty bounds.
public boolean isEmpty(ClusteringComparator comparator)
{
return slices.isEmpty();
}
public boolean selects(Clustering<?> clustering)
{
return slices.selects(clustering);

View File

@ -18,15 +18,18 @@
package org.apache.cassandra.distributed.test;
import java.io.IOException;
import java.util.Iterator;
import com.google.common.collect.Iterators;
import org.junit.Test;
import org.apache.cassandra.distributed.Cluster;
import org.apache.cassandra.distributed.api.ConsistencyLevel;
import static org.apache.cassandra.distributed.api.ConsistencyLevel.QUORUM;
import static org.apache.cassandra.distributed.shared.AssertUtils.assertRows;
import static org.apache.cassandra.distributed.shared.AssertUtils.row;
public class PagingTest extends TestBaseImpl
{
@ -78,4 +81,27 @@ public class PagingTest extends TestBaseImpl
}
}
}
@Test
public void testPagingWithRangeTombstones() throws IOException
{
try (Cluster cluster = init(Cluster.build(2).start()))
{
cluster.schemaChange("CREATE TABLE " + KEYSPACE + ".tbl (pk int, ck int, regular int, PRIMARY KEY (pk, ck))");
cluster.coordinator(1).execute("DELETE FROM " + KEYSPACE + ".tbl WHERE pk = 1 AND ck > 1 AND ck < 10", ConsistencyLevel.ALL);
cluster.coordinator(1).execute("insert into " + KEYSPACE + ".tbl (pk, ck, regular) values (1,1,1)", ConsistencyLevel.ALL);
cluster.coordinator(1).execute("insert into " + KEYSPACE + ".tbl (pk, ck, regular) values (1,2,2)", ConsistencyLevel.ALL);
cluster.coordinator(1).execute("insert into " + KEYSPACE + ".tbl (pk, ck, regular) values (1,3,3)", ConsistencyLevel.ALL);
cluster.forEach((node) -> node.flush(KEYSPACE));
Iterator<Object[]> iter = cluster.coordinator(1).executeWithPaging("SELECT pk,ck,regular FROM " + KEYSPACE + ".tbl " +
"WHERE pk=? AND ck>=? ORDER BY ck DESC;",
ConsistencyLevel.QUORUM, 1,
1, 1);
assertRows(iter,
row(1, 3, 3),
row(1, 2, 2),
row(1, 1, 1));
}
}
}

View File

@ -135,11 +135,11 @@ public class ReadRepairEmptyRangeTombstonesTest extends TestBaseImpl
"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")
.mutate(1, "INSERT INTO %s (k, s) VALUES (0, 0)")
.assertRowsDistributed("SELECT * FROM %s WHERE k=0 AND c>1 and c<=1", 1)
.assertRowsDistributed("SELECT * FROM %s WHERE k=0 AND c>1 and c<=1", 0)
.assertRowsDistributed("SELECT * FROM %s WHERE k=0 AND c>2 and c<=2", 0)
.assertRowsInternal("SELECT * FROM %s", row(0, null, 0))
.assertRowsInternal("SELECT * FROM %s")
.mutate(2, "DELETE FROM %s WHERE k=0 AND c>0 AND c<3")
.assertRowsInternal("SELECT * FROM %s", row(0, null, 0));
.assertRowsInternal("SELECT * FROM %s");
}
/**
@ -155,10 +155,10 @@ public class ReadRepairEmptyRangeTombstonesTest extends TestBaseImpl
.mutate(1, "INSERT INTO %s (k, c) VALUES (0, 3)")
.mutate(1, "INSERT INTO %s (k, c) VALUES (0, 4)")
.assertRowsDistributed("SELECT * FROM %s WHERE k=0 AND c>=2 AND c<=3",
paging ? (flush ? 3 : 2) : 1,
paging ? 2 : 1,
row(0, 2), row(0, 3))
.assertRowsDistributed("SELECT * FROM %s WHERE k=0 AND c>=3 AND c<=4",
paging && flush ? 2 : 1,
1,
row(0, 3), row(0, 4))
.assertRowsInternal("SELECT * FROM %s", row(0, 2), row(0, 3), row(0, 4))
.mutate(2, "DELETE FROM %s WHERE k=0 AND c>=1 AND c<=5")
@ -169,7 +169,7 @@ public class ReadRepairEmptyRangeTombstonesTest extends TestBaseImpl
* Test range queries asking for a not-empty range targeting rows overlapping with a tombstone range in one replica.
*/
@Test
public void testRangeQueriesWithRowsOverlappingWithTombstoneRangeStart()
public void testRangeQueriesWithRowsOvetrlappingWithTombstoneRangeStart()
{
tester().createTable("CREATE TABLE %s(k int, c int, PRIMARY KEY (k, c)) " +
"WITH CLUSTERING ORDER BY (c %s) AND read_repair='%s'")
@ -181,10 +181,10 @@ public class ReadRepairEmptyRangeTombstonesTest extends TestBaseImpl
.mutate(1, "INSERT INTO %s (k, c) VALUES (0, 5)")
.mutate(1, "INSERT INTO %s (k, c) VALUES (0, 6)")
.assertRowsDistributed("SELECT c FROM %s WHERE k=0 AND c>=1 AND c<=4",
paging ? (flush && !reverse ? 5 : 4) : 1,
paging ? 4 : 1,
row(1), row(2), row(3), row(4))
.assertRowsDistributed("SELECT c FROM %s WHERE k=0 AND c>=2 AND c<=5",
paging && flush && !reverse ? 2 : 1,
1,
row(2), row(3), row(4), row(5))
.assertRowsInternal("SELECT c FROM %s", row(1), row(2), row(3), row(4), row(5))
.mutate(2, "DELETE FROM %s WHERE k=0 AND c>=1 AND c<=6")
@ -207,10 +207,10 @@ public class ReadRepairEmptyRangeTombstonesTest extends TestBaseImpl
.mutate(1, "INSERT INTO %s (k, c) VALUES (0, 5)")
.mutate(1, "INSERT INTO %s (k, c) VALUES (0, 6)")
.assertRowsDistributed("SELECT c FROM %s WHERE k=0 AND c>=2 AND c<=5",
paging ? (flush && reverse ? 5 : 4) : 1,
paging ? 4 : 1,
row(2), row(3), row(4), row(5))
.assertRowsDistributed("SELECT c FROM %s WHERE k=0 AND c>=3 AND c<=6",
paging && flush && reverse ? 2 : 1,
1,
row(3), row(4), row(5), row(6))
.assertRowsInternal("SELECT c FROM %s", row(2), row(3), row(4), row(5), row(6))
.mutate(2, "DELETE FROM %s WHERE k=0 AND c>=1 AND c<=6")

View File

@ -2472,6 +2472,8 @@ public class SelectTest extends CQLTester
assertRows(execute("SELECT * FROM %s WHERE pk = 1 AND c1 > 1 AND c2 > 2 AND c3 = 3 AND v = 3 ALLOW FILTERING;"),
row(1, 3, 3, 3, 3));
assertRows(execute("SELECT * FROM %s WHERE pk = 1 AND c1 IN(0,1,2) AND c2 > 1 AND c2 < 1 AND v = 3 ALLOW FILTERING;"));
assertRows(execute("SELECT * FROM %s WHERE pk = 1 AND c1 IN(0,1,2) AND c2 = 1 AND v = 3 ALLOW FILTERING;"),
row(1, 1, 1, 3, 3));