Repository: vespa-engine/vespa
File: clustercontroller-core/src/main/java/com/yahoo/vespa/clustercontroller/core/FleetController.java (Java)

Code before:
        var currentBundle = stateVersionTracker.getVersionedClusterStateBundle();
        context.log(logger, Level.FINE, () -> String.format("All distributors have ACKed cluster state version %d", currentBundle.getVersion()));
        stateChangeHandler.handleAllDistributorsInSync(currentBundle.getBaselineClusterState(), nodes, database, dbContext);
        convergedStates.add(currentBundle);
    }

    private boolean changesConfiguredNodeSet(Collection<ConfiguredNode> newNodes) {
        if (newNodes.size() != cluster.getConfiguredNodes().size()) return true;
        if (! cluster.getConfiguredNodes().values().containsAll(newNodes)) return true;

        // Check retirement changes
        for (ConfiguredNode node : newNodes) {
            if (node.retired() != cluster.getConfiguredNodes().get(node.index()).retired()) {
                return true;
            }
        }

        return false;
    }

    private boolean changesDistributionConfig(Distribution newDistribution) {
        // TODO Ideally we'd also look at searchable-copies, but that's currently
        //  not exposed via the Distribution domain class.
        var currentDistribution = cluster.getDistribution();
        return (newDistribution.getRedundancy() != currentDistribution.getRedundancy()) ||
               !newDistribution.getRootGroup().equals(currentDistribution.getRootGroup());
    }

    /** This is called when the options field has been set to a new set of options */
    private void propagateOptions() {
        verifyInControllerThread();
        selfTerminateIfConfiguredNodeIndexHasChanged();

        if (changesConfiguredNodeSet(options.nodes())) {
            // Force slobrok node re-fetch in case of changes to the set of configured nodes
            cluster.setSlobrokGenerationCount(0);
        }

        stateVersionTracker.setMinMergeCompletionRatio(options.minMergeCompletionRatio());

        communicator.propagateOptions(options);
        indexPageRequestHandler.propagateOptions(options);

        if (nodeLookup instanceof SlobrokClient) {
            ((SlobrokClient) nodeLookup).setSlobrokConnectionSpecs(options.slobrokConnectionSpecs());
        }
        eventLog.setMaxSize(options.eventLogMaxSize(), options.eventNodeLogMaxSize());
        if (changesDistributionConfig(options.storageDistribution())) {
            // Distribution config changes must invalidate any assumptions used for wanted states set on nodes
            cluster.bumpOrchestrationDecisionGeneration();
        }

Review comment (bug):
Two Group.Distribution are equal if their `int[]` is `==`, https://github.com/vespa-engine/vespa/blob/master/vdslib/src/main/java/com/yahoo/vdslib/distribution/Group.java#L280 .  This means they have to be _identical_, as pairwise `int` compares are only done with `Arrays.equals()`.  This seems buggy, and would end up bumping the orchestration decision generation every time ... the StorDistributionConfig changes? ... every redeploy?

Code after:
        var currentBundle = stateVersionTracker.getVersionedClusterStateBundle();
        context.log(logger, Level.FINE, () -> String.format("All distributors have ACKed cluster state version %d", currentBundle.getVersion()));
        stateChangeHandler.handleAllDistributorsInSync(currentBundle.getBaselineClusterState(), nodes, database, dbContext);
        convergedStates.add(currentBundle);
    }

    private boolean changesConfiguredNodeSet(Collection<ConfiguredNode> newNodes) {
        if (newNodes.size() != cluster.getConfiguredNodes().size()) return true;
        if (! cluster.getConfiguredNodes().values().containsAll(newNodes)) return true;

        // Check retirement changes
        for (ConfiguredNode node : newNodes) {
            if (node.retired() != cluster.getConfiguredNodes().get(node.index()).retired()) {
                return true;
            }
        }

        return false;
    }

    private boolean changesDistributionConfig(Distribution newDistribution) {
        // TODO Ideally we'd also look at searchable-copies, but that's currently
        //  not exposed via the Distribution domain class.
        var currentDistribution = cluster.getDistribution();
        return (newDistribution.getRedundancy() != currentDistribution.getRedundancy()) ||
               !newDistribution.getRootGroup().equals(currentDistribution.getRootGroup());
    }

    /** This is called when the options field has been set to a new set of options */
    private void propagateOptions() {
        verifyInControllerThread();
        selfTerminateIfConfiguredNodeIndexHasChanged();

        if (changesConfiguredNodeSet(options.nodes())) {
            // Force slobrok node re-fetch in case of changes to the set of configured nodes
            cluster.setSlobrokGenerationCount(0);
        }

        stateVersionTracker.setMinMergeCompletionRatio(options.minMergeCompletionRatio());

        communicator.propagateOptions(options);
        indexPageRequestHandler.propagateOptions(options);

        if (nodeLookup instanceof SlobrokClient) {
            ((SlobrokClient) nodeLookup).setSlobrokConnectionSpecs(options.slobrokConnectionSpecs());
        }
        eventLog.setMaxSize(options.eventLogMaxSize(), options.eventNodeLogMaxSize());
        if (changesDistributionConfig(options.storageDistribution())) {
            // Distribution config changes must invalidate any assumptions used for wanted states set on nodes
            cluster.bumpOrchestrationGeneration();
        }