[CICP-5409] Fix data race between async global rebalance and the controller pipeline

[thestreak101]

Problem

GenericHelixController owns a single ResourceControllerDataProvider instance (built once, held in a final field) and refreshes it in place on every event-loop iteration.

With async global rebalance — which is the default (ClusterConfig.DEFAULT_GLOBAL_REBALANCE_ASYNC_MODE_ENABLED = true) — the pipeline thread submits the baseline calculation and immediately continues. The async task then read clusterData, resourceMap and currentStateOutput while the pipeline thread was concurrently mutating the very same objects on the next iteration.

The baseline could therefore be computed from a half-refreshed view of the cluster: a torn mix of old and new instance/resource state. This is a live defect on the default configuration, not a theoretical one.

Fix

Materialize the ClusterModel on the calling (pipeline) thread, before the task is submitted.

getBaselineAssignment and generateClusterModelForBaseline were the only two places the async task touched shared pipeline state, plus clusterData.getClusterName(). Both are hoisted into a new buildClusterModel(...), and the cluster name is captured in a local.

doGlobalRebalance is reduced to:

doGlobalRebalance(ClusterModel clusterModel, String clusterName,
                  RebalanceAlgorithm algorithm, boolean shouldTriggerMainPipeline)

so it is no longer able to reach shared mutable state — the invariant is enforced by the signature rather than by convention.

The expensive part — the rebalance algorithm itself — still runs asynchronously. This does not turn async mode back into sync mode.

Prerequisite verified

The fix is only sound if ClusterModel is a true snapshot. Confirmed that neither ClusterModel nor ClusterContext, AssignableNode or AssignableReplica holds a reference back to the data provider. Without that property the race would simply move rather than be fixed.

Behaviour change (please review)

Cluster-model construction failures (INVALID_CLUSTER_STATUS / INVALID_CLUSTER_CONFIG) now surface synchronously to the pipeline caller, instead of being recorded in _lastAsyncFailure and reported on a later iteration. This is arguably more correct — the failure is attributed to the iteration that caused it — but it is a change in where the exception appears.

Verification

• mvn -pl helix-core -am test-compile passes (includes existing helix-core tests).
• No new test added: reliably reproducing a torn-read data race in a unit test is not practical. The change is structural — it removes the shared-state access rather than guarding it.
• Suggest a canary before broad rollout, given this is on the default path.

🤖 Drafted with GitHub Copilot CLI. Filed from an audit of CICP Helix bug tickets — please review carefully before merging.