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