[GitHub] [cassandra-accord] belliottsmith commented on a diff in pull request #5: CASSANDRA-17718: Transaction Invalidation

[GitBox <gi...@apache.org>]

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


##########
accord-core/src/main/java/accord/coordinate/Coordinate.java:
##########
@@ -1,46 +1,283 @@
 package accord.coordinate;
 
-import accord.api.ConfigurationService;
-
-import com.google.common.base.Preconditions;
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+import java.util.function.BiConsumer;
 
 import accord.api.Key;
-import accord.local.Node;
 import accord.api.Result;
-import accord.local.Node;
+import accord.coordinate.tracking.FastPathTracker;
+import accord.topology.Shard;
+import accord.topology.Topologies;
 import accord.txn.Ballot;
+import accord.messages.Callback;
+import accord.local.Node;
+import accord.txn.Dependencies;
+import accord.local.Node.Id;
+import accord.txn.Timestamp;
+import accord.messages.PreAccept;
+import accord.messages.PreAccept.PreAcceptOk;
 import accord.txn.Txn;
 import accord.txn.TxnId;
+import accord.messages.PreAccept.PreAcceptReply;
+import com.google.common.collect.Sets;
+
+import org.apache.cassandra.utils.concurrent.AsyncFuture;
 import org.apache.cassandra.utils.concurrent.Future;
 
-public class Coordinate
+/**
+ * Perform initial rounds of PreAccept and Accept until we have reached agreement about when we should execute.
+ * If we are preempted by a recovery coordinator, we abort and let them complete (and notify us about the execution result)
+ */
+public class Coordinate extends AsyncFuture<Result> implements Callback<PreAcceptReply>, BiConsumer<Result, Throwable>

Review Comment:
   Yeah, I'd be happy simply renaming `Coordinate` to `PreAcceptPhase` (can't be `PreAccept` as we have that already), as that's really all `Coordinate` is right now (plus an `AsyncFuture`, which could be allocated separately if we wanted). Each phase just invokes the next and passes on the callback now.
   
   FWIW, I also think it's fine as it is, as `PreAccept` can only be started by the initial coordinator, and we do allocate slightly less by bundling this into the same class. But I don't really have a strong preference for either, so happy to take yours.



-- 
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-unsubscribe@cassandra.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@cassandra.apache.org
For additional commands, e-mail: pr-help@cassandra.apache.org