bereng commented on code in PR #1891:
URL: https://github.com/apache/cassandra/pull/1891#discussion_r1111663766
##########
test/unit/org/apache/cassandra/db/compaction/CompactionsCQLTest.java:
##########
@@ -313,16 +314,18 @@ public void testCompactionInvalidTombstone() throws
Throwable
{
DatabaseDescriptor.setCorruptedTombstoneStrategy(Config.CorruptedTombstoneStrategy.exception);
prepare();
+
// write a standard tombstone with negative local deletion time (LDTs
are not set by user and should not be negative):
- RowUpdateBuilder rub = new
RowUpdateBuilder(getCurrentColumnFamilyStore().metadata(), -1,
System.currentTimeMillis() * 1000, 22).clustering(33).delete("b");
- rub.build().apply();
- flush();
- compactAndValidate();
- readAndValidate(true);
- readAndValidate(false);
+ Assertions.assertThatThrownBy(() -> {
+ new RowUpdateBuilder(getCurrentColumnFamilyStore().metadata(),
+ -1,
+ System.currentTimeMillis() * 1000,
+ 22).clustering(33).delete("b");
+ }).isInstanceOf(IllegalArgumentException.class)
+ .hasMessageContaining("is outside the range for an unsigned integer
value");
}
- @Test
+ @Test(expected = RuntimeException.class) // msg = "is outside the range
for an unsigned integer value"
Review Comment:
This should never happen as negative ldts just can't get in now. The
decision in [C14092](https://issues.apache.org/jira/browse/CASSANDRA-14092) was
that you should scrub sstables if there were any negative ldts and _not_
support that logic in the storage engine.
So why does this test exist? digging a bit further
[C14467](https://issues.apache.org/jira/browse/CASSANDRA-14467) seems to bring
in some defensive logic towards negative ldts and other bad scenarios. I can
only think this could happen on corrupted sstables which is what c14467 does:
mark corrupted sstables. So I need to think if upon _decoding_ (bc we will
never encode) a negative ldt outisde the valid uint range we should fail
somehow we trigger the c14467 logic, 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]
