adelapena commented on code in PR #1891:
URL: https://github.com/apache/cassandra/pull/1891#discussion_r1126369669
##########
src/java/org/apache/cassandra/db/RangeTombstoneList.java:
##########
@@ -58,12 +58,12 @@ public class RangeTombstoneList implements
Iterable<RangeTombstone>, IMeasurable
private ClusteringBound<?>[] starts;
private ClusteringBound<?>[] ends;
private long[] markedAts;
- private int[] delTimes;
+ private long[] delTimes;
Review Comment:
Just took a look at the last commit storing uint, which is very similar to
what I was thinking. However, I think it misses the case where we add a range
tombstone with a `InvalidDeletionTime`.
On on hand, the valid-invalid state of an invalid deletion time is codified
by the implemented class, so we can use a int for codifying the local deletion
time. On the other hand, `RangeTombstoneList` seems to be an attempt at saving
memory by aggregating multiple range tombstones, so we have an array of local
deletion times. If this array is comprised by uints instead of longs, we miss a
representation of the valid/invalid status that we used to represent with the
class.
I think this tiny test demonstrates it:
```java
// create an invalid deletion time
DeletionTime dt = DeletionTime.build(1, -1);
assertFalse(dt.validate());
// use the invalid deletion time for a range tombstone and aggregate it
RangeTombstoneList rtl = new RangeTombstoneList(null, 1);
rtl.add(new RangeTombstone(Slice.ALL, dt));
// undo the aggregation and see if the deletion time is still invalid
dt = rtl.iterator().next().deletionTime();
assertNotNull(dt);
assertFalse(dt.validate()); // fails
```
I guess we could try to overcome this problem by also storing an array of
booleans on `RangeTombstoneList` storing what deletion times are invalid.
Still, I'm not sure whether it's worth it and I wouldn't oppose to leaving
`RangeTombstoneList` with a `long` array. wdyt?
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
