belliottsmith commented on code in PR #50:
URL: https://github.com/apache/cassandra-accord/pull/50#discussion_r1254165898
##########
accord-core/src/main/java/accord/local/Commands.java:
##########
@@ -900,47 +978,98 @@ else if (cur.has(until))
break;
}
- TxnId directlyBlockedOnCommit = firstWaitingOnCommit(cur);
- TxnId directlyBlockedOnApply = firstWaitingOnApply(cur,
directlyBlockedOnCommit);
+ WaitingOn waitingOn = cur.hasBeen(Committed) ?
cur.asCommitted().waitingOn() : WaitingOn.EMPTY;
+ TxnId directlyBlockedOnCommit;
+ TxnId directlyBlockedOnApply = waitingOn.nextWaitingOnApply();
if (directlyBlockedOnApply != null)
{
+ // preferentially block on apply, as this probably saves
us additional bookkeeping
+ // by giving other dependencies time to complete
push(directlyBlockedOnApply, Known.Done);
+ prevSafe = curSafe;
}
- else if (directlyBlockedOnCommit != null)
+ else if ((directlyBlockedOnCommit =
waitingOn.nextWaitingOnCommit()) != null)
{
push(directlyBlockedOnCommit, ExecuteAtOnly);
+ prevSafe = curSafe;
}
else
{
- if (cur.hasBeen(Committed) && !cur.hasBeen(ReadyToExecute)
&& !cur.asCommitted().isWaitingOnDependency())
+ if (cur.hasBeen(Committed) && !cur.is(ReadyToExecute) &&
!cur.is(Applying) && !cur.asCommitted().isWaitingOnDependency())
{
- if (!maybeExecute(safeStore, curSafe,
progressShard(safeStore, cur), false, false))
+ if (!maybeExecute(safeStore, curSafe,
cur.progressShard(), false, false))
throw new AssertionError("Is able to Apply, but
has not done so");
// loop and re-test the command's status; we may still
want to notify blocking, esp. if not homeShard
continue;
}
- Unseekables<?, ?> someKeys = cur.maxUnseekables();
- if (someKeys == null && prev != null) someKeys =
prev.partialDeps().someUnseekables(cur.txnId());
- Invariants.checkState(someKeys != null);
- logger.trace("{} blocked on {} until {}", txnIds[0],
cur.txnId(), until);
- safeStore.progressLog().waiting(cur.txnId(), until,
someKeys);
- return;
+ Participants<?> someParticipants = null;
+ if (cur.route() != null)
+ {
+ someParticipants = cur.route().participants();
+ }
+ else if (prev != null)
+ {
+ someParticipants =
prev.partialDeps().participants(cur.txnId());
Review Comment:
I think you may have confused about the meaning of `prev` here - we're
exploring dependencies, so `prev` is the one that depends on `cur`, and we
check for this at the start, stepping up the stack as we find things are
redundant.
##########
accord-core/src/main/java/accord/local/Commands.java:
##########
@@ -900,47 +978,98 @@ else if (cur.has(until))
break;
}
- TxnId directlyBlockedOnCommit = firstWaitingOnCommit(cur);
- TxnId directlyBlockedOnApply = firstWaitingOnApply(cur,
directlyBlockedOnCommit);
+ WaitingOn waitingOn = cur.hasBeen(Committed) ?
cur.asCommitted().waitingOn() : WaitingOn.EMPTY;
+ TxnId directlyBlockedOnCommit;
+ TxnId directlyBlockedOnApply = waitingOn.nextWaitingOnApply();
if (directlyBlockedOnApply != null)
{
+ // preferentially block on apply, as this probably saves
us additional bookkeeping
+ // by giving other dependencies time to complete
push(directlyBlockedOnApply, Known.Done);
+ prevSafe = curSafe;
}
- else if (directlyBlockedOnCommit != null)
+ else if ((directlyBlockedOnCommit =
waitingOn.nextWaitingOnCommit()) != null)
{
push(directlyBlockedOnCommit, ExecuteAtOnly);
+ prevSafe = curSafe;
}
else
{
- if (cur.hasBeen(Committed) && !cur.hasBeen(ReadyToExecute)
&& !cur.asCommitted().isWaitingOnDependency())
+ if (cur.hasBeen(Committed) && !cur.is(ReadyToExecute) &&
!cur.is(Applying) && !cur.asCommitted().isWaitingOnDependency())
{
- if (!maybeExecute(safeStore, curSafe,
progressShard(safeStore, cur), false, false))
+ if (!maybeExecute(safeStore, curSafe,
cur.progressShard(), false, false))
throw new AssertionError("Is able to Apply, but
has not done so");
// loop and re-test the command's status; we may still
want to notify blocking, esp. if not homeShard
continue;
}
- Unseekables<?, ?> someKeys = cur.maxUnseekables();
- if (someKeys == null && prev != null) someKeys =
prev.partialDeps().someUnseekables(cur.txnId());
- Invariants.checkState(someKeys != null);
- logger.trace("{} blocked on {} until {}", txnIds[0],
cur.txnId(), until);
- safeStore.progressLog().waiting(cur.txnId(), until,
someKeys);
- return;
+ Participants<?> someParticipants = null;
+ if (cur.route() != null)
+ {
+ someParticipants = cur.route().participants();
+ }
+ else if (prev != null)
+ {
+ someParticipants =
prev.partialDeps().participants(cur.txnId());
Review Comment:
I think you may have confused about the meaning of `prev` here - we're
exploring dependencies, so `prev` is the one that depends on `cur`, and we
check for this at the start, stepping up the stack as we find things are
redundant (and so may have been truncated).
--
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]
