belliottsmith commented on code in PR #91:
URL: https://github.com/apache/cassandra-accord/pull/91#discussion_r1608142119


##########
accord-core/src/main/java/accord/coordinate/CoordinateSyncPoint.java:
##########
@@ -128,7 +128,7 @@ void onPreAccepted(Topologies topologies, Timestamp 
executeAt, List<PreAcceptOk>
             topologies = node.topology().forEpoch(route, txnId.epoch());
             // TODO (required): consider the required semantics of a SyncPoint
             if (tracker.hasFastPathAccepted() && txnId.kind() == 
Kind.SyncPoint)
-                execute(adapter, node, topologies, route, FAST, txnId, txn, 
executeAt, deps, this);
+                execute(adapter, node, topologies, route, FAST, txnId, txn, 
txnId, deps, this);

Review Comment:
   I don't think this is correct - sync points take an actual executeAt. If we 
intend to coordinate them like ExclusiveSyncPoint then we need to modify them 
to execute the same way (i.e. they need to update their kind to signify 
`awaitsOnlyDeps`, and need to use the same execution path as 
`ExclusiveSyncPoint` and `EphemeralRead`)



##########
accord-core/src/main/java/accord/coordinate/CoordinateSyncPoint.java:
##########
@@ -128,7 +128,7 @@ void onPreAccepted(Topologies topologies, Timestamp 
executeAt, List<PreAcceptOk>
             topologies = node.topology().forEpoch(route, txnId.epoch());
             // TODO (required): consider the required semantics of a SyncPoint
             if (tracker.hasFastPathAccepted() && txnId.kind() == 
Kind.SyncPoint)
-                execute(adapter, node, topologies, route, FAST, txnId, txn, 
executeAt, deps, this);
+                execute(adapter, node, topologies, route, FAST, txnId, txn, 
txnId, deps, this);

Review Comment:
   I don't think this is correct - sync points take an actual `executeAt`. If 
we intend to coordinate them like `ExclusiveSyncPoint` then we need to modify 
them to execute the same way (i.e. they need to update their kind to signify 
`awaitsOnlyDeps`, and need to use the same execution path as 
`ExclusiveSyncPoint` and `EphemeralRead`)



-- 
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]

Reply via email to