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.
In this case it appears that we are in fact 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]
