[Bug](meta-service) Node status flip in decommission is not atomic, readers may observe phantom node removal

[LemonCL]

Search before asking

• I had searched in the issues and found no similar issues.

Version

master, branch-3.1 (cloud mode)

What's Wrong?

In cloud mode meta-service, handle_decommission_node and handle_notify_decommissioned
(cloud/src/meta-service/meta_service_resource.cpp) flip a node's status by calling
ResourceManager::modify_nodes twice in two separate FDB transactions: first deleting
the node from InstanceInfoPB.nodes[], then re-adding it with the new status
(DECOMMISSIONING / DECOMMISSIONED).

Between the two committed transactions, the node is genuinely absent from the cluster
PB. Any concurrent reader of get_cluster/get_instance (e.g. FE CloudClusterChecker,
which polls every 10s) that lands in this window observes a phantom node removal.

Consequences observed in production during a 28-node scale-in:

• FE dropped the backend from memory, then re-registered it as a brand-new backend
  (new backend id) on the next poll;
• The tablet rebalancer treated it as a fresh idle BE and moved ~120k tablets back
  onto it;
• The subsequent DROP_NODE failed, leaving the decommission workflow wedged
  (compute group stuck in "changing" state).

What You Expected?

Node status transitions should be atomic: a reader sees either the old status or the
new status, never "node missing".

How to Reproduce?

Inject a delay (e.g. sleep a few seconds) between the two modify_nodes calls in
handle_notify_decommissioned, then concurrently call get_cluster — the node
disappears from the response during the window.

Anything Else?

Proposed fix: replace the two modify_nodes calls (delete + re-add) with a single
ResourceManager::update_cluster call that mutates the node's status in place within
one FDB transaction — the same pattern already used by handle_rename_cluster /
handle_set_cluster_status. PR: #68119

Are you willing to submit PR?

• Yes I am willing to submit a PR!

Code of Conduct

• I agree to follow this project's Code of Conduct

[Doris-Breakwater]

Breakwater-GitHub-Analysis-Slot: slot_489c0b76c69f

Initial assessment

Verdict: confirmed high-impact cloud scale-in correctness bug. The code-level race is present in both master and branch-3.1. The reported production sequence is consistent with the verified code paths, although the incident-specific timeline has not been independently validated from logs.

Verified facts

• handle_decommission_node and handle_notify_decommissioned each call ResourceManager::modify_nodes twice: once to delete the selected nodes and once to append them with the new status.
• Every modify_nodes invocation creates, reads, writes, and commits its own transaction. Therefore, after the first successful commit and before the second successful commit, the persisted InstanceInfoPB really does not contain those nodes. This is not merely an in-memory cache artifact.
• get_cluster and get_instance read that same instance record through independent transactions, so a reader in this interval can observe the node as absent.
• FE CloudClusterChecker runs at cloud_cluster_check_interval_second (default 10 seconds), diffs nodes by endpoint, removes endpoints absent from the meta-service response, and creates a new Backend with a newly allocated ID when the endpoint reappears. This validates the mechanism behind the reported remove/re-add behavior.
• The two-step implementation also has a failure-atomicity problem: if the re-add transaction fails after deletion commits, the node remains absent. Conversely, an error from the first call can be overwritten by the result of the second call because the handler does not stop between calls.

Assessment of PR #68119

The proposed architectural direction is correct: ResourceManager::update_cluster reads and writes the instance in one transaction, and mutating the existing NodeInfoPB in place removes both the observable missing-node interval and the partial delete/re-add state.

However, the current PR should address these points before merge:

1. Preserve node identity checks. The old path first scopes candidates with get_node(cloud_unique_id) and then matches the endpoint. The new helper scans the requested cluster and matches only ip/host + heartbeat_port. A stale or incorrect cloud_unique_id can therefore update a different node at the same endpoint. Match cloud_unique_id as well as the endpoint, or otherwise retain the old unique-ID scoping explicitly.
2. Do not return success for zero or partial matches. flip_nodes_status_in_place currently only logs when matched_cnt != req_nodes.size(), after which update_cluster commits and the RPC returns OK. Make matching one-to-one and return a non-empty error before commit unless every requested node is found exactly once. Add negative tests for an unknown unique ID, an endpoint mismatch, duplicate request entries, and a partially matched multi-node request.
3. Strengthen the regression coverage. The added test usefully proves final node count, order, and status, and it fails the current delete/append implementation because the order changes. It does not directly exercise a concurrent get_cluster reader. A deterministic sync-point/concurrency test would better lock down the stated invariant; at minimum, retain the order/count assertions and add the identity/error cases above plus both IP and host matching paths.
4. Backport deliberately. PR [fix](meta-service) Make node status flip atomic in decommission and notify_decommissioned #68119 currently targets master, while the same two-transaction code is present on branch-3.1. After the master fix is validated, prepare and test the branch-specific backport rather than assuming identical cache-update internals.

Missing incident evidence

No additional evidence is required to confirm the code defect. To validate the exact production impact and define affected release scope, it would still be useful to attach:

• the exact FE and meta-service build commits;
• timestamp-correlated meta-service logs for both modify_nodes commits and the later DROP_NODE failure;
• FE CloudClusterChecker logs showing the endpoint removal/re-add and old/new backend IDs;
• the failed DROP_NODE response code/message and the compute-group state transition timeline.

The issue currently has no labels, assignee, or milestone. Maintainers should add the project-standard cloud/meta-service bug and release/backport labels, assign a reviewer, amend the PR as above, run the focused cloud meta-service unit tests plus normal CI, and verify the fix on both master and branch-3.1.

[LemonCL]

Thanks for the thorough review! All four points are addressed in the updated commit:

1. Node identity checks: matching now requires cloud_unique_id equality (when the request carries one) in addition to the endpoint (ip/host + heartbeat_port), so a stale or incorrect unique id can no longer flip a different node at the same endpoint.

2. One-to-one matching: flip_nodes_status_in_place now validates the full request first — every request node must match exactly one cluster node, and no two request nodes may claim the same node. Any violation returns a non-empty error before commit, with zero mutation. Negative tests added: unknown/stale unique id at an existing endpoint, endpoint mismatch, duplicated request entries, and a partially matched multi-node request (asserted to fail as a whole with nothing changed).

3. Regression coverage: kept the node count/order/status assertions (the order check deterministically fails the old delete + re-add implementation), and added the host-based endpoint matching path plus all the identity/error cases above. A sync-point based concurrency test is a nice follow-up; the in-place flip within a single FDB transaction makes the invariant structural rather than timing-dependent.

4. Backport: agreed — once this is validated on master, I'll prepare a separate, branch-specific PR for branch-3.1 (the handlers there are still inlined in the alter_cluster switch, so the change needs reshaping, plus its own testing).

For incident evidence, I'll attach the correlated FE/MS log excerpts to the issue shortly.

[LemonCL]

Incident evidence from a production incident (downstream build based on branch-3.1;
the meta-service code in this area is identical to master). All timestamps are from
the same incident, IPs and cluster names are masked.

Scenario: scale-in of a compute group, 28 nodes decommissioned in parallel.

1. MS log — every node is deleted then re-added in two separate transactions
(~4ms apart, note the status flip DECOMMISSIONING → DECOMMISSIONED):

I20260916 15:06:13.303140 resource_manager.cpp:1118] node to modify json={"cloud_unique_id":"1:<instance>:VpS2Z3xR","ip":"10.x.x.103",...,"status":"NODE_STATUS_DECOMMISSIONING",...}
I20260916 15:06:13.307343 resource_manager.cpp:1118] node to modify json={"cloud_unique_id":"1:<instance>:VpS2Z3xR","ip":"10.x.x.103",...,"status":"NODE_STATUS_DECOMMISSIONED",...}

2. FE log — its 10s poll landed exactly in that window for one node, dropped
the backend, and re-registered it with a new backend id on the next poll:

2026-09-16 15:06:13,311 INFO (cloud tablet rebalancer) [CloudTabletRebalancer.checkDecommissionState():607] prepare to notify meta service be 10.x.x.239:9050 decommissioned
2026-09-16 15:06:13,358 INFO (cloud cluster check) [CloudSystemInfoService.updateCloudBackendsUnLock():305] dropped cloud backend=Backend [id=1788251570096, host=10.x.x.239, ...]
2026-09-16 15:06:23,403 INFO (cloud cluster check) [CloudSystemInfoService.updateCloudBackendsUnLock():281] added cloud backend=Backend [id=1788252118413, host=10.x.x.239, ...]

Same host, same BE process (identical process epoch), but backend id changed
1788251570096 → 1788252118413.

3. The subsequent DROP_NODE for a node in this group then failed with:

I20260916 15:06:43.083244 meta_service_helper.h:226] finish alter_cluster ... response=status { code: CLUSTER_NOT_FOUND msg: "not found ,can not find to drop nodes by cloud_unique_id=... help Msg=Ms nodes memory cache may be inconsistent..." }

4. Timeline (correlated):

time	FE	MS
15:06:13.311	rebalancer sends NOTIFY_DECOMMISSIONED for node .239
15:06:13.30x–.31x		txn1: node deleted from nodes[]; txn2: re-added as DECOMMISSIONED
15:06:13.358	checker poll sees node missing → drops backend id ...570096
15:06:23.403	checker re-registers node as new backend id ...118413
15:06:43.083		DROP_NODE fails: CLUSTER_NOT_FOUND

Afterwards the rebalancer treated the re-registered node as a fresh idle BE and
moved ~120k tablets back onto it, and the decommission workflow could not
complete (the compute group stayed in "changing" state).

[LemonCL]

run buildall