belliottsmith commented on code in PR #65:
URL: https://github.com/apache/cassandra-accord/pull/65#discussion_r1445965204
##########
accord-core/src/main/java/accord/messages/Commit.java:
##########
@@ -275,6 +275,22 @@ public static void commitInvalidate(Node node, TxnId
txnId, Unseekables<?> infor
// TODO (expected, safety): this kind of check needs to be
inserted in all equivalent methods
Invariants.checkState(untilEpoch >= txnId.epoch());
Invariants.checkState(node.topology().hasEpoch(untilEpoch));
+ // Possible that the invalidation is due to the range no longer
existing in untilEpoch, which means that
Review Comment:
Ok, so the mistake is that we use a later epoch that is voted on (and
recorded as such) on preaccept that wasn't proposed. So I guess we really want
to replicate the original epoch we used for sending, which means for any Accept
we see, we use the highest timestamp. But for PreAccept we only use TxnId. In
this case we *should* never use a future epoch, and we should still
invalidate/clear any records we might have created.
So, something like
`Timestamp invalidateUntil = recoverOks.stream().map(ok ->
ok.saveStatus.hasBeen(Accepted) ? ok.executeAt : ok.txnId).reduce(txnId,
Timestamp::max);`
This isn't perfect, as an `AcceptInvalidate` could mask an `Accept` with a
higher timestamp, and cause us to not clear some state. But I'm not sure it's
worth agonising over, as eventually it all gets cleaned up. Better to keep
things simple.
--
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]
