belliottsmith commented on code in PR #2963:
URL: https://github.com/apache/cassandra/pull/2963#discussion_r1422242012
##########
src/java/org/apache/cassandra/service/accord/AccordKeyspace.java:
##########
@@ -363,6 +363,7 @@ public static Row truncatedApply(SaveStatus newSaveStatus,
Row row, long nowInSe
// If durability is not universal we don't want to delete older
versions of the row that might have recorded
// a higher durability value. maybeDropTruncatedCommandColumns
will take care of dropping things even if we don't drop via tombstones.
// durability should be the only column that could have an older
value that is insufficient for propagating forward
+ // TODO (now): with UniversalOrInvalidated should this change?
boolean doDeletion = durability == Durability.Universal;
Review Comment:
I'm not sure such a method would be well-advised, at least in isolation or
without some additional parameters. What we are permitted to do in various
cases of durability depends on how that durability is implied. If it is
recorded against a specific record we are permitted to erase different
information.
By the looks of it this, this particular place is very C* specific, as it
relates to LSMT compaction.
In this case it appears that we are using the record's own durability data,
in which case it should be a bug to even encounter `UniversalOrInvalidated` -
we know it cannot be `Invalidated`, because it has been `Applied`. We should
perhaps enrich our `Command` validation logic to catch this kind of logic
error, if we don't already.
--
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]
