ifesdjeen commented on code in PR #231:
URL: https://github.com/apache/cassandra-accord/pull/231#discussion_r2235938891
##########
accord-core/src/main/java/accord/local/cfk/Pruning.java:
##########
@@ -539,22 +542,70 @@ static TxnInfo[] removeRedundantById(TxnInfo[] byId,
boolean hasRedundantLoading
TxnInfo[] newById = byId;
int pos = insertPos(byId, newRedundantBefore);
- if (pos != 0 || hasRedundantLoadingPruned)
+ int appliedPos = Arrays.binarySearch(byId, pos, byId.length,
newAppliedBefore);
+ if (appliedPos < 0) appliedPos = -1 - appliedPos;
+ if (pos != 0 || appliedPos != 0 || hasRedundantLoadingPruned)
{
if (Invariants.isParanoid() && testParanoia(LINEAR, NONE, LOW))
{
int startPos = prevBootstrappedAt == null ? 0 :
insertPos(byId, prevBootstrappedAt);
for (int i = startPos ; i < pos ; ++i)
- Invariants.require(byId[i].isNot(COMMITTED) ||
!byId[i].mayExecute() || !reportLinearizabilityViolations(), "%s redundant;
expected to be applied, undecided or to execute in a future epoch", byId[i]);
+ Invariants.require((byId[i].isNot(COMMITTED) &&
byId[i].isNot(STABLE)) || !byId[i].mayExecute() ||
!reportLinearizabilityViolations(), "%s redundant; expected to be applied,
undecided or to execute in a future epoch", byId[i]);
+ }
+
+ int removeUnappliedCount = 0;
+ if (appliedPos > pos)
+ {
+ // we apply additional filtering to remove any transactions we
know would apply locally, but haven't executed
+ // so we know they will not execute. note: we cannot do this
safely for any transactions we don't execute locally!
+ // this is used to handle consistent restore on replay, where
we may have some transaction that is logically
+ // invalidated, but we may only record the invalidation after
the snapshot is created because the RX doesn't
+ // record it as a dependency (and so it doesn't have to be
decided for the RX to execute).
+ // We may then later invalidate and update the CFK, but since
this is not reflected in the snapshot,
+ // after restoring from snapshot we may have an inconsistency
between the command state and the CFK state,
+ // and won't know that we should update the CFK because the
command is already invalidated.
+ // So, to avoid having to read all CFK on startup and process
any potentially invalidated transactions,
+ // we instead apply this filtering aggressively whenever we
know the transaction cannot apply,
+ // and rely on the applied RX to correctly reject recovery of
the transaction.
+ for (int i = pos ; i < appliedPos ; ++i)
+ {
+ if (byId[i].compareTo(APPLIED) < 0)
+ {
+ if (byId[i].mayExecute())
Review Comment:
nit: should we collapse nested ifs into a single if?
--
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: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
