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/03/16 06:43:37 UTC
[GitHub] [cassandra] jasonstack opened a new pull request #473: Cassandra
15369
jasonstack opened a new pull request #473: Cassandra 15369
URL: https://github.com/apache/cassandra/pull/473
Changes:
* return range tombstone marker for memtable read instead of row deletion
* remove range-tombstome-marker or row deletion when they don't supersede partition deletion
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #473:
Cassandra 15369
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #473: Cassandra 15369
URL: https://github.com/apache/cassandra/pull/473#discussion_r404756459
##########
File path: test/unit/org/apache/cassandra/db/partition/PartitionImplementationTest.java
##########
@@ -319,26 +318,30 @@ private void testIter(Supplier<Collection<? extends Unfiltered>> contentSupplier
testSlicingOfIterators(sortedContent, partition, cf, true);
}
- void testSearchIterator(NavigableSet<Clusterable> sortedContent, Partition partition, ColumnFilter cf, boolean reversed)
+ void testClusteringsIterator(NavigableSet<Clusterable> sortedContent, Partition partition, ColumnFilter cf, boolean reversed)
Review comment:
Nit: could be private
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #473:
Cassandra 15369
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #473: Cassandra 15369
URL: https://github.com/apache/cassandra/pull/473#discussion_r404761587
##########
File path: test/unit/org/apache/cassandra/db/partition/PartitionImplementationTest.java
##########
@@ -360,6 +363,19 @@ Slices makeSlices()
return builder.build();
}
+ NavigableSet<Clustering> makeClusterings(boolean reversed)
Review comment:
Nit: can be private
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #473:
Cassandra 15369
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #473: Cassandra 15369
URL: https://github.com/apache/cassandra/pull/473#discussion_r404151805
##########
File path: src/java/org/apache/cassandra/db/partitions/AbstractBTreePartition.java
##########
@@ -205,12 +202,17 @@ private RowAndDeletionMergeIterator merge(Iterator<Row> rowIter, Iterator<RangeT
canHaveShadowedData());
}
- private abstract class AbstractIterator extends AbstractUnfilteredRowIterator
+ private class SlicesIterator extends AbstractUnfilteredRowIterator
{
final Holder current;
final ColumnFilter selection;
- private AbstractIterator(Holder current, Row staticRow, ColumnFilter selection, boolean isReversed)
Review comment:
This removed `AbstractIterator`, without any change, could be the base class for both `SlicesIterator` and the new `ClusteringsIterator`, so we can avoid some duplication, especially the long comment about fetched columns.
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org
[GitHub] [cassandra] adelapena commented on a change in pull request #473:
Cassandra 15369
Posted by GitBox <gi...@apache.org>.
adelapena commented on a change in pull request #473: Cassandra 15369
URL: https://github.com/apache/cassandra/pull/473#discussion_r404755199
##########
File path: src/java/org/apache/cassandra/db/rows/RangeTombstoneMarker.java
##########
@@ -160,8 +160,8 @@ private DeletionTime currentOpenDeletionTimeInMerged()
return DeletionTime.LIVE;
DeletionTime biggestDeletionTime = openMarkers[biggestOpenMarker];
- // it's only open in the merged iterator if it's not shadowed by the partition level deletion
- return partitionDeletion.supersedes(biggestDeletionTime) ? DeletionTime.LIVE : biggestDeletionTime;
+ // it's only open in the merged iterator if it doesn't supersedes the partition level deletion
+ return !biggestDeletionTime.supersedes(partitionDeletion) ? DeletionTime.LIVE : biggestDeletionTime;
Review comment:
Do we have a test for this?
----------------------------------------------------------------
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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org