bdeggleston commented on code in PR #109:
URL: https://github.com/apache/cassandra-accord/pull/109#discussion_r1705944169
##########
accord-core/src/main/java/accord/coordinate/Infer.java:
##########
@@ -91,18 +96,26 @@ public enum InvalidIfNot
/**
* If the command has not had its execution timestamp agreed on any
shard
*/
- IfUndecided(IfQuorum, IfQuorum);
+ IfUndecided(IfQuorum, IfQuorum),
+
+ /**
+ * If the command has not had its execution timestamp agreed on any
shard
Review Comment:
this comment is identical to `IfUndecided`
##########
accord-core/src/main/java/accord/local/Commands.java:
##########
@@ -1249,193 +1198,94 @@ private static ProgressShard progressShard(Route<?>
route, @Nullable RoutingKey
return progressKey.equals(route.homeKey()) ? Home : Local;
}
- enum EnsureAction
- {
- /** Don't check */
- Ignore,
- /** Add, but return false if insufficient for any reason */
- TryAdd,
- /** Supplement existing information, asserting that the existing and
additional information are independently sufficient,
- * returning false only if the existing information is absent AND the
new information is insufficient. */
- Add,
- /** Set, but only return false if insufficient */
- TrySet,
- /** Overwrite existing information if sufficient; fail otherwise */
- Set
- }
-
@SuppressWarnings({"unchecked", "rawtypes"})
- private static CommonAttributes set(CommonAttributes attrs,
- Ranges existingRanges, Ranges
additionalRanges,
- ProgressShard shard, Route<?> route,
- @Nullable PartialTxn partialTxn,
EnsureAction ensurePartialTxn,
- @Nullable PartialDeps partialDeps,
EnsureAction ensurePartialDeps)
+ private static CommonAttributes set(SaveStatus newStatus, Command cur,
CommonAttributes upd,
Review Comment:
It would be nice if we wouldn't have to include the status we intend to set
here, but there doesn't seem to be a way to make the Known dependent stuff
happen as part of the state change method without making it both more complex
and less understandable
##########
accord-core/src/main/java/accord/coordinate/ExecuteSyncPoint.java:
##########
@@ -68,17 +77,79 @@ public void start()
}
}
+ public static class ExecuteExclusiveSyncPoint extends
ExecuteSyncPoint<Ranges>
+ {
+ private long retryInFutureEpoch;
+ public ExecuteExclusiveSyncPoint(Node node, SyncPoint<Ranges>
syncPoint, Function<Topologies, AbstractSimpleTracker<?>> trackerSupplier)
+ {
+ super(node, syncPoint, trackerSupplier);
Review Comment:
can we add an counterpart Invariant like
`Invariants.checkArgument(syncPoint.syncId.kind().awaitsOnlyDeps());` or check
the txnId kind against the expected value.
##########
accord-core/src/main/java/accord/coordinate/Invalidate.java:
##########
@@ -148,66 +148,82 @@ private void invalidate()
Invariants.checkState(!isPrepareDone);
isPrepareDone = true;
- // first look to see if it has already been
+ FullRoute<?> fullRoute = InvalidateReply.findRoute(replies);
+ Route<?> someRoute = InvalidateReply.mergeRoutes(replies);
+
+ // first look to see if it has already been decided/invalidated
+ // check each shard independently - if we find any that can be
invalidated, do so
+ InvalidateReply max = InvalidateReply.max(replies);
+ InvalidateReply maxNotTruncated = max;
+ if (max.status == Status.Truncated)
+ maxNotTruncated = InvalidateReply.maxNotTruncated(replies);
+
+ switch (maxNotTruncated.status)
Review Comment:
I think you'd need a null check here in case all replies were truncated?
##########
accord-core/src/main/java/accord/topology/TopologyManager.java:
##########
@@ -538,28 +541,38 @@ public Topologies withUnsyncedEpochs(Unseekables<?>
select, Timestamp min, Times
public Topologies withUnsyncedEpochs(Unseekables<?> select, long minEpoch,
long maxEpoch)
{
Invariants.checkArgument(minEpoch <= maxEpoch, "min epoch %d > max
%d", minEpoch, maxEpoch);
- return withSufficientEpochs(select, minEpoch, maxEpoch, epochState ->
epochState.syncComplete);
+ return withSufficientEpochs(select, minEpoch, maxEpoch, epochState ->
epochState.syncComplete, AT_LEAST);
}
- public Topologies withOpenEpochs(Unseekables<?> select, EpochSupplier min,
EpochSupplier max)
+ public Topologies withOpenEpochs(Unseekables<?> select, EpochSupplier min,
EpochSupplier max, EpochSufficiencyMode mode)
{
- return withSufficientEpochs(select, min.epoch(), max.epoch(),
epochState -> epochState.closed);
+ return withSufficientEpochs(select, min.epoch(), max.epoch(),
epochState -> epochState.closed, mode);
}
- private Topologies withSufficientEpochs(Unseekables<?> select, long
minEpoch, long maxEpoch, Function<EpochState, Ranges> isSufficientFor)
+ public Topologies withUncompletedEpochs(Unseekables<?> select,
EpochSupplier min, EpochSupplier max, EpochSufficiencyMode mode)
+ {
+ return withSufficientEpochs(select, min.epoch(), max.epoch(),
epochState -> epochState.complete, mode);
+ }
+
+ public enum EpochSufficiencyMode { AT_LEAST, AT_MOST }
+
+ private Topologies withSufficientEpochs(Unseekables<?> select, long
minEpoch, long maxEpoch, Function<EpochState, Ranges> isSufficientFor,
EpochSufficiencyMode mode)
Review Comment:
afaict, this is assuming that `mode` will be either `AT_LEAST` or `AT_MOST`
and never `null`. We should add a null check (or make it a boolean arg)
##########
accord-core/src/main/java/accord/coordinate/Invalidate.java:
##########
@@ -148,66 +148,82 @@ private void invalidate()
Invariants.checkState(!isPrepareDone);
isPrepareDone = true;
- // first look to see if it has already been
+ FullRoute<?> fullRoute = InvalidateReply.findRoute(replies);
+ Route<?> someRoute = InvalidateReply.mergeRoutes(replies);
+
+ // first look to see if it has already been decided/invalidated
+ // check each shard independently - if we find any that can be
invalidated, do so
+ InvalidateReply max = InvalidateReply.max(replies);
+ InvalidateReply maxNotTruncated = max;
Review Comment:
nit: could the assignment and conditional be condensed in to a single
ternary assignment?
--
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]
