You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@cassandra.apache.org by GitBox <gi...@apache.org> on 2020/06/03 11:03:53 UTC
[GitHub] [cassandra] adelapena commented on a change in pull request #473: Cassandra 15369
adelapena commented on a change in pull request #473:
URL: https://github.com/apache/cassandra/pull/473#discussion_r434476777
##########
File path: src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java
##########
@@ -265,6 +258,55 @@ protected Unfiltered computeNext()
}
}
+ private class ClusteringsIterator extends AbstractIterator
+ {
+ private final Iterator<Clustering> clusteringsInQueryOrder;
+ private final SearchIterator<Clustering, Row> rowSearcher;
+
+ private Iterator<Unfiltered> currentIterator;
+
+ private ClusteringsIterator(ColumnFilter selection, NavigableSet<Clustering> clusteringsInQueryOrder, boolean isReversed, Holder current, Row staticRow)
Review comment:
Perhaps this long line could be broken, feel free to ignore if you don't agree.
```suggestion
private ClusteringsIterator(ColumnFilter selection,
NavigableSet<Clustering> clusteringsInQueryOrder,
boolean isReversed,
Holder current,
Row staticRow)
```
##########
File path: test/unit/org/apache/cassandra/db/SinglePartitionSliceCommandTest.java
##########
@@ -316,6 +294,149 @@ public void staticColumnsAreReturned() throws IOException
}
}
+ /**
+ * Make sure point read on range tombstone returns the same physical data structure regardless
+ * data is in memtable or sstable, so that we can produce the same digest.
+ */
+ @Test
+ public void testReadOnRangeTombstoneMarker()
+ {
+ QueryProcessor.executeOnceInternal("CREATE TABLE IF NOT EXISTS ks.test_read_rt (k int, c1 int, c2 int, c3 int, v int, primary key (k, c1, c2, c3))");
+ TableMetadata metadata = Schema.instance.getTableMetadata("ks", "test_read_rt");
+ ColumnFamilyStore cfs = Schema.instance.getColumnFamilyStoreInstance(metadata.id);
+
+ String template = "SELECT * FROM ks.test_read_rt %s";
+ String pointRead = "WHERE k=1 and c1=1 and c2=1 and c3=1";
+ String sliceReadC1C2 = "WHERE k=1 and c1=1 and c2=1";
+ String sliceReadC1 = "WHERE k=1 and c1=1";
+ String partitionRead = "WHERE k=1";
+
+ for (String postfix : Arrays.asList(pointRead, sliceReadC1C2, sliceReadC1, partitionRead))
+ {
+ String query = String.format(template, postfix);
+ cfs.truncateBlocking();
+ QueryProcessor.executeOnceInternal("DELETE FROM ks.test_read_rt USING TIMESTAMP 10 WHERE k=1 AND c1=1");
+
+ List<Unfiltered> memtableUnfiltereds = assertQueryReturnsSingleRT(query);
+ cfs.forceBlockingFlush();
+ List<Unfiltered> sstableUnfiltereds = assertQueryReturnsSingleRT(query);
+
+ String errorMessage = String.format("Expect %s but got %s with postfix '%s'",
+ toString(memtableUnfiltereds, metadata),
+ toString(sstableUnfiltereds, metadata),
+ postfix);
+ assertEquals(errorMessage, memtableUnfiltereds, sstableUnfiltereds);
+ }
+ }
+
+ /**
+ * Partition deletion should remove row deletion when tie
+ */
+ @Test
+ public void testPartitionDeletionRowDeletionTie()
+ {
+ QueryProcessor.executeOnceInternal("CREATE TABLE ks.partition_row_deletion (k int, c int, v int, primary key (k, c))");
+ TableMetadata metadata = Schema.instance.getTableMetadata("ks", "partition_row_deletion");
+ ColumnFamilyStore cfs = Schema.instance.getColumnFamilyStoreInstance(metadata.id);
+ cfs.disableAutoCompaction();
+
+ BiFunction<Boolean, Boolean, List<Unfiltered>> tester = (flush, multiSSTable)->
+ {
+ cfs.truncateBlocking();
+ QueryProcessor.executeOnceInternal("DELETE FROM ks.partition_row_deletion USING TIMESTAMP 10 WHERE k=1");
+ if (flush && multiSSTable)
+ cfs.forceBlockingFlush();
+ QueryProcessor.executeOnceInternal("DELETE FROM ks.partition_row_deletion USING TIMESTAMP 10 WHERE k=1 and c=1");
+ if (flush)
+ cfs.forceBlockingFlush();
+
+ QueryProcessor.executeOnceInternal("INSERT INTO ks.partition_row_deletion(k,c,v) VALUES(1,1,1) using timestamp 11");
+ if (flush)
+ {
+ cfs.forceBlockingFlush();
+ try
+ {
+ cfs.forceMajorCompaction();
+ }
+ catch (Throwable e)
+ {
+ throw new RuntimeException(e);
+ }
+ }
+
+ try (UnfilteredRowIterator partition = getIteratorFromSinglePartition("SELECT * FROM ks.partition_row_deletion where k=1 and c=1"))
+ {
+ assertEquals(10, partition.partitionLevelDeletion().markedForDeleteAt());
+ return toUnfiltereds(partition);
+ }
+ };
+
+ List<Unfiltered> memtableUnfiltereds = tester.apply(false, false);
+ List<Unfiltered> singleSSTableUnfiltereds = tester.apply(true, false);
+ List<Unfiltered> multiSSTableUnfiltereds = tester.apply(true, true);
+
+ assertEquals(1, singleSSTableUnfiltereds.size());
+ String errorMessage = String.format("Expect %s but got %s", toString(memtableUnfiltereds, metadata), toString(singleSSTableUnfiltereds, metadata));
+ assertEquals(errorMessage, memtableUnfiltereds, singleSSTableUnfiltereds);
+ errorMessage = String.format("Expect %s but got %s", toString(singleSSTableUnfiltereds, metadata), toString(multiSSTableUnfiltereds, metadata));
+ assertEquals(errorMessage, singleSSTableUnfiltereds, multiSSTableUnfiltereds);
+ memtableUnfiltereds.forEach(u -> assertTrue("Expect no row deletion, but got " + u.toString(metadata, true), ((Row) u).deletion().isLive()));
+ }
+
+ /**
+ * Partition deletion should remove range deletion when tie
+ */
+ @Test
+ public void testPartitionDeletionRangeDeletionTie()
+ {
+ QueryProcessor.executeOnceInternal("CREATE TABLE ks.partition_range_deletion (k int, c1 int, c2 int, v int, primary key (k, c1, c2))");
+ TableMetadata metadata = Schema.instance.getTableMetadata("ks", "partition_range_deletion");
+ ColumnFamilyStore cfs = Schema.instance.getColumnFamilyStoreInstance(metadata.id);
+ cfs.disableAutoCompaction();
+
+ BiFunction<Boolean, Boolean, List<Unfiltered>> tester = (flush, multiSSTable) ->
+ {
+ cfs.truncateBlocking();
+ QueryProcessor.executeOnceInternal("DELETE FROM ks.partition_range_deletion USING TIMESTAMP 10 WHERE k=1");
+ if (flush && multiSSTable)
+ cfs.forceBlockingFlush();
+ QueryProcessor.executeOnceInternal("DELETE FROM ks.partition_range_deletion USING TIMESTAMP 10 WHERE k=1 and c1=1");
+ if (flush)
+ cfs.forceBlockingFlush();
+
+ QueryProcessor.executeOnceInternal("INSERT INTO ks.partition_range_deletion(k,c1,c2,v) VALUES(1,1,1,1) using timestamp 11");
+ if (flush)
+ {
+ cfs.forceBlockingFlush();
+ try
+ {
+ cfs.forceMajorCompaction();
+ }
+ catch (Throwable e)
+ {
+ throw new RuntimeException(e);
+ }
+ }
+
+ try (UnfilteredRowIterator partition = getIteratorFromSinglePartition("SELECT * FROM ks.partition_range_deletion where k=1 and c1=1 and c2=1"))
+ {
+ assertEquals(10, partition.partitionLevelDeletion().markedForDeleteAt());
+ return toUnfiltereds(partition);
+ }
+ };
+
+ List<Unfiltered> memtableUnfiltereds = tester.apply(false, false);
+ List<Unfiltered> singleSSTableUnfiltereds = tester.apply(true, false);
+ List<Unfiltered> multiSSTableUnfiltereds = tester.apply(true, true);
+
+ assertEquals(1, singleSSTableUnfiltereds.size());
+ String errorMessage = String.format("Expect %s but got %s", toString(memtableUnfiltereds, metadata), toString(singleSSTableUnfiltereds, metadata));
Review comment:
```suggestion
String errorMessage = String.format("Expected %s but got %s", toString(memtableUnfiltereds, metadata), toString(singleSSTableUnfiltereds, metadata));
```
##########
File path: test/unit/org/apache/cassandra/db/SinglePartitionSliceCommandTest.java
##########
@@ -316,6 +294,149 @@ public void staticColumnsAreReturned() throws IOException
}
}
+ /**
+ * Make sure point read on range tombstone returns the same physical data structure regardless
+ * data is in memtable or sstable, so that we can produce the same digest.
+ */
+ @Test
+ public void testReadOnRangeTombstoneMarker()
+ {
+ QueryProcessor.executeOnceInternal("CREATE TABLE IF NOT EXISTS ks.test_read_rt (k int, c1 int, c2 int, c3 int, v int, primary key (k, c1, c2, c3))");
+ TableMetadata metadata = Schema.instance.getTableMetadata("ks", "test_read_rt");
+ ColumnFamilyStore cfs = Schema.instance.getColumnFamilyStoreInstance(metadata.id);
+
+ String template = "SELECT * FROM ks.test_read_rt %s";
+ String pointRead = "WHERE k=1 and c1=1 and c2=1 and c3=1";
+ String sliceReadC1C2 = "WHERE k=1 and c1=1 and c2=1";
+ String sliceReadC1 = "WHERE k=1 and c1=1";
+ String partitionRead = "WHERE k=1";
+
+ for (String postfix : Arrays.asList(pointRead, sliceReadC1C2, sliceReadC1, partitionRead))
+ {
+ String query = String.format(template, postfix);
+ cfs.truncateBlocking();
+ QueryProcessor.executeOnceInternal("DELETE FROM ks.test_read_rt USING TIMESTAMP 10 WHERE k=1 AND c1=1");
+
+ List<Unfiltered> memtableUnfiltereds = assertQueryReturnsSingleRT(query);
+ cfs.forceBlockingFlush();
+ List<Unfiltered> sstableUnfiltereds = assertQueryReturnsSingleRT(query);
+
+ String errorMessage = String.format("Expect %s but got %s with postfix '%s'",
+ toString(memtableUnfiltereds, metadata),
+ toString(sstableUnfiltereds, metadata),
+ postfix);
+ assertEquals(errorMessage, memtableUnfiltereds, sstableUnfiltereds);
+ }
+ }
+
+ /**
+ * Partition deletion should remove row deletion when tie
+ */
+ @Test
+ public void testPartitionDeletionRowDeletionTie()
+ {
+ QueryProcessor.executeOnceInternal("CREATE TABLE ks.partition_row_deletion (k int, c int, v int, primary key (k, c))");
+ TableMetadata metadata = Schema.instance.getTableMetadata("ks", "partition_row_deletion");
+ ColumnFamilyStore cfs = Schema.instance.getColumnFamilyStoreInstance(metadata.id);
+ cfs.disableAutoCompaction();
+
+ BiFunction<Boolean, Boolean, List<Unfiltered>> tester = (flush, multiSSTable)->
+ {
+ cfs.truncateBlocking();
+ QueryProcessor.executeOnceInternal("DELETE FROM ks.partition_row_deletion USING TIMESTAMP 10 WHERE k=1");
+ if (flush && multiSSTable)
+ cfs.forceBlockingFlush();
+ QueryProcessor.executeOnceInternal("DELETE FROM ks.partition_row_deletion USING TIMESTAMP 10 WHERE k=1 and c=1");
+ if (flush)
+ cfs.forceBlockingFlush();
+
+ QueryProcessor.executeOnceInternal("INSERT INTO ks.partition_row_deletion(k,c,v) VALUES(1,1,1) using timestamp 11");
+ if (flush)
+ {
+ cfs.forceBlockingFlush();
+ try
+ {
+ cfs.forceMajorCompaction();
+ }
+ catch (Throwable e)
+ {
+ throw new RuntimeException(e);
+ }
+ }
+
+ try (UnfilteredRowIterator partition = getIteratorFromSinglePartition("SELECT * FROM ks.partition_row_deletion where k=1 and c=1"))
+ {
+ assertEquals(10, partition.partitionLevelDeletion().markedForDeleteAt());
+ return toUnfiltereds(partition);
+ }
+ };
+
+ List<Unfiltered> memtableUnfiltereds = tester.apply(false, false);
+ List<Unfiltered> singleSSTableUnfiltereds = tester.apply(true, false);
+ List<Unfiltered> multiSSTableUnfiltereds = tester.apply(true, true);
+
+ assertEquals(1, singleSSTableUnfiltereds.size());
+ String errorMessage = String.format("Expect %s but got %s", toString(memtableUnfiltereds, metadata), toString(singleSSTableUnfiltereds, metadata));
Review comment:
```suggestion
String errorMessage = String.format("Expected %s but got %s", toString(memtableUnfiltereds, metadata), toString(singleSSTableUnfiltereds, metadata));
```
##########
File path: test/unit/org/apache/cassandra/db/partition/PartitionImplementationTest.java
##########
@@ -307,10 +306,10 @@ private void testIter(Supplier<Collection<? extends Unfiltered>> contentSupplier
partition.unfilteredIterator(cf, multiSlices, true));
// search iterator
Review comment:
```suggestion
// clustering iterator
```
##########
File path: src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java
##########
@@ -108,10 +112,29 @@ public EncodingStats stats()
public Row getRow(Clustering clustering)
{
- Row row = searchIterator(ColumnFilter.selection(columns()), false).next(clustering);
- // Note that for statics, this will never return null, this will return an empty row. However,
Review comment:
So we can just say that for statics it never returns an empty row, it's either `null` or a not-empty row. Would it make sense to comment this in `Partition#getRow(Clustering)` JavaDoc, and perhaps mark it as `@Nullable`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org