belliottsmith commented on code in PR #3781:
URL: https://github.com/apache/cassandra/pull/3781#discussion_r1908409877


##########
src/java/org/apache/cassandra/service/accord/AccordConfigurationService.java:
##########
@@ -217,48 +233,40 @@ protected EpochHistory createEpochHistory()
         return new EpochHistory();
     }
 
-    @VisibleForTesting
+    /**
+     * On restart, loads topologies. On bootstrap, discovers existing 
topologies and initializes the node.
+     */
     public synchronized void start()
-    {
-        start(ignore -> {});
-    }
-
-    public synchronized void start(Consumer<OptionalLong> callback)
     {
         Invariants.checkState(state == State.INITIALIZED, "Expected state to 
be INITIALIZED but was %s", state);
         state = State.LOADING;
-        EndpointMapping snapshot = mapping;
-        //TODO (restart): if there are topologies loaded then there is likely 
failures if reporting is needed, as mapping is not setup yet
-        AtomicReference<Topology> previousRef = new AtomicReference<>(null);
-        diskState = diskStateManager.loadTopologies(((epoch, metadata, 
topology, syncStatus, pendingSyncNotify, remoteSyncComplete, closed, redundant) 
-> {
-            updateMapping(metadata);
-            reportTopology(topology, syncStatus == SyncStatus.NOT_STARTED, 
true);
-
-            Topology previous = previousRef.get();
-            if (previous != null)
-            {
-                // for all nodes removed, or pending removal, mark them as 
removed so we don't wait on their replies
-                Sets.SetView<Node.Id> removedNodes = 
Sets.difference(previous.nodes(), topology.nodes());
-                if (!removedNodes.isEmpty())
-                    onNodesRemoved(topology.epoch(), currentTopology(), 
removedNodes);
-            }
-            previousRef.set(topology);
 
+        EndpointMapping snapshot = mapping;
+        diskStateManager.loadLocalTopologyState((epoch, syncStatus, 
pendingSyncNotify, remoteSyncComplete, closed, redundant) -> {
             getOrCreateEpochState(epoch).setSyncStatus(syncStatus);
-            if (syncStatus == SyncStatus.NOTIFYING)
+            switch (syncStatus)

Review Comment:
   Either handle all cases and throw exception on unhandled, or simply test 
`syncStatus == NOTIFYING`?
   
   Should we be doing anything in the `NOT_STARTED` case? We might need to 
fetch information from peers about what has been synced, though this will 
naturally start arriving from later epochs, so perhaps not a problem.



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

Reply via email to