fix(decommission): run the leader-side start to completion (#8723)

* fix(decommission): run the leader-side start to completion

A forwarded decommission start awaits `ECStore::decommission` inline in
the RPC handler. When the forwarding node's RPC times out, the stream is
reset and the handler future is dropped wherever it is. Dropped after
`pool.bin` records the pool as active but before the peers reload and
the worker is spawned, the start left the pool active with no worker and
peers on the old metadata, and a retry was rejected as already active.
An admin client that disconnects can do the same.

Run the start in its own task and let the caller await only its result.
The start then always reaches its end: it fails before persisting
anything, or persists the pool, has the peers reload and spawns the
local workers, rolling back if the reload fails. A retry still cannot
start twice: it waits on the start gate held by the running start and
either reports the busy gate or is rejected because the pool is active.

`decommission` now takes `self: &Arc<Self>` and spawns workers on that
store instead of looking up the global object store, which is the same
store in production and makes the path testable; the unused
`require_decommission_store` helper is removed.

Refs rustfs/backlog#2873

* fix(decommission): detach the leader-side start from its caller

9ac4c10de carried only the reproduction (the testable entry point, the
peer-reload pause point and the proof test). This adds the fix it
describes: `ECStore::decommission` spawns the start as its own task and
awaits only its result, so a caller that stops waiting, such as a
forwarded start whose internode RPC timed out, can no longer drop the
start between persisting the pool as active and having the peers reload
and the local worker start.

Also prove a retry while the first start runs cannot start the pool
twice, update the operator and architecture docs for the new behavior,
and shrink the error(format!) ratchet baseline for the removed helper.

Refs rustfs/backlog#2873

* fix(decommission): single-flight start, recheck pool state under gate

Review of 3a5276c5b:

- A start re-checks the pool state under the start gate before it
  cancels routines, so a start racing another one can no longer cancel
  the worker the other start just spawned.
- Only one start runs per instance at a time: another request, including
  the retry of an operator whose request timed out, is rejected as
  already running instead of stacking a second detached start and its
  preflight heals. The start runs through `run_detached_mutation`, and a
  failure nobody waits for any more is logged.
- Tests: the proof test pins its drop after the save and requires a
  worker that persists a cancellation; the retry test really drops the
  first caller; new tests cover the under-gate recheck and a peer reload
  failure after the caller is gone. That last one shows the start
  rollback cannot clear a fresh start because the pool metadata
  stale-write guard refuses it; the gap is asserted and documented and
  tracked in rustfs/backlog#2873.
- The start tests run on their own large-stack thread: the io_uring lane
  (`uring_`, which also selects `during_`) runs them with libtest's 2 MiB
  default stack, where the proof test overflowed.
- Docs: single-flight starts, status and cancel during preflight, the
  rollback gap and its recovery, and the release requirement.

Refs rustfs/backlog#2873

* fix(decommission): turn away local start retries at the start slot

Second review of 29253430a:

- The leader-local admin pre-check refreshed rebalance metadata under
  the start gate before the start slot was checked, so a local retry
  waited out its 20 s gate budget behind a running start (which holds
  the gate through its peer reload) and then got a 503. The pre-check
  now checks the slot first and returns `DecommissionAlreadyRunning`,
  which the handler reports as a 400 that keeps the reason.
- Prove the reload-failure path really ran in the detached start (the
  injection is consumed), add a test that a fresh start's failed
  rollback reports both the reload error and the stale-write refusal,
  and assert an abandoned start stays tracked for shutdown.

Refs rustfs/backlog#2873

crates/ecstore/src/runtime/instance.rs

@@ -217,6 +217,10 @@ pub struct InstanceContext {
     /// Readers are held across one publication commit; movement transitions
     /// take the writer at their durable state commit boundary.
     data_movement_operation_gate: Arc<RwLock<()>>,
+    /// Set while an operator decommission start runs on this instance. A
+    /// start runs detached from its caller, so this keeps abandoned requests
+    /// and their retries from stacking concurrent starts and preflight heals.
+    decommission_start_in_flight: AtomicBool,
     /// Remote scanner publication leases own a read guard until explicit
     /// release or bounded expiry.  Keeping the guard in storage-owned state
     /// makes a remote movement transition wait on the same fence as a local
@@ -290,6 +294,7 @@ impl InstanceContext {
             bucket_metadata_sys: std::sync::Mutex::new(None),
             background_cancel_token: OnceLock::new(),
             data_movement_operation_gate: Arc::new(RwLock::new(())),
+            decommission_start_in_flight: AtomicBool::new(false),
             scanner_publication_leases: Arc::new(Mutex::new(HashMap::new())),
             data_movement_operation_epoch: AtomicU64::new(0),
             data_movement_operation_epoch_exhausted: AtomicBool::new(false),
@@ -349,6 +354,22 @@ impl InstanceContext {
         Arc::clone(&self.data_movement_operation_gate)
     }
 
+    /// Claim the single decommission start slot; `false` when a start is
+    /// already running. Release it with [`Self::end_decommission_start`].
+    pub(crate) fn try_begin_decommission_start(&self) -> bool {
+        self.decommission_start_in_flight
+            .compare_exchange(false, true, Ordering::AcqRel, Ordering::Acquire)
+            .is_ok()
+    }
+
+    pub(crate) fn end_decommission_start(&self) {
+        self.decommission_start_in_flight.store(false, Ordering::Release);
+    }
+
+    pub(crate) fn decommission_start_running(&self) -> bool {
+        self.decommission_start_in_flight.load(Ordering::Acquire)
+    }
+
     pub(crate) async fn install_scanner_publication_lease(
         &self,
         token: Uuid,

docs/architecture/decommission-compatibility.md

@@ -24,7 +24,7 @@ Deterministic request rejections (unsupported single-pool operations, missing or
 
 The local-leader-prefix rule keeps the active worker on the leader for the pool being moved while still allowing a request to contain later targets whose leaders are different nodes. Later queued targets are recovered or promoted by the leader that owns that target.
 
-Start, cancel (`POST /v3/pools/cancel`), and clear (`POST /v3/pools/clear`) requests may arrive on any cluster node. When the target pool's first endpoint is remote, RustFS forwards the operation over the authenticated internode RPC channel to that endpoint; the receiving node still enforces the local-leader rule before mutating decommission state.
+Start, cancel (`POST /v3/pools/cancel`), and clear (`POST /v3/pools/clear`) requests may arrive on any cluster node. When the target pool's first endpoint is remote, RustFS forwards the operation over the authenticated internode RPC channel to that endpoint; the receiving node still enforces the local-leader rule before mutating decommission state. On the leader a start runs in its own task (`ECStore::decommission`), so a caller that stops waiting, such as a forwarding node whose RPC timed out, only loses the answer: the start cannot stop between recording the pool as active in `pool.bin` and having the peers reload and the local worker start, or rolling back. One start runs per leader at a time; another start is rejected as already running until it finishes. These guarantees need the leader to run a release that includes them.
 
 ### Persisted Metadata Shape
 

docs/operations/cluster-lifecycle-operations.md

@@ -176,7 +176,7 @@ Contract: [decommission-compatibility.md](../architecture/decommission-compatibi
 - Targets are validated as a batch before anything is persisted: unknown, duplicate, active, queued, or completed targets are rejected; failed or canceled targets must be cleared first unless they hold unresolved listing entries, in which case a new start is the retry path (`DecommissionStartPoolState::Retryable` versus `Blocked` in `crates/ecstore/src/core/pools.rs`).
 - Requests go to any node and are forwarded to the pool leader (§1).
 - In-flight data movement (an object copy or source cleanup, another start, a scanner publication lease) can hold the start, rebalance activation, and data-movement gates. The leader's rebalance pre-check and the start itself each spend at most 20 seconds in total waiting for those gates; preflight work such as bucket heals does not count, and a free gate is always taken. When the budget runs out the start persists nothing and fails with a lock timeout that names the gate: a request served by the leader answers `503 ServiceUnavailable` with `Retry-After`, while a forwarded request reaches the client as a generic internal error because the internode RPC carries only the message. Preflight bucket heals wait for the movement gate without this budget.
-- A start whose caller stops waiting is dropped on the leader wherever it was. This happens to a forwarded start that hits the internode RPC timeout (`RUSTFS_INTERNODE_RPC_TIMEOUT_SECS`, 30 seconds by default) and may happen when an admin client disconnects. Either nothing was persisted, or `pool.bin` already records the pool as active although the leader had not yet started its worker or told the other nodes to reload pool metadata. Check `decommission/status` before retrying: a pool that is not active can be started again, while a new start of an active pool is rejected. For an active pool, the leader's decommission watchdog resumes the missing worker within about 30 seconds once no other decommission worker is running on that node; if progress still does not advance, restart the leader (the pool's first endpoint), whose startup resumes the decommission. Nodes that missed the reload keep their previous view of the pool until they reload pool metadata, for example on restart. Making the leader-side start cancel-safe is tracked in rustfs/backlog#2873.
+- A start keeps running on the leader after its caller stops waiting, for example a forwarded start that hits the internode RPC timeout (`RUSTFS_INTERNODE_RPC_TIMEOUT_SECS`, 30 seconds by default) or an admin client that disconnects; this needs a leader running a release that includes it. Only one start runs on a leader at a time: until it finishes, another start request, including a retry, is rejected as already running. That includes its preflight (bucket heals, capacity scans), while `decommission/status` does not yet show the pool active and a cancel reports that the pool is not decommissioning. A start either fails before persisting anything, or records the pool as active in `pool.bin`, has the other nodes reload pool metadata, and starts the worker. If that reload fails the leader tries to roll the start back. Today that rollback cannot clear a fresh start, because the pool metadata stale-write guard refuses to clear a persisted active decommission (rustfs/backlog#2873). When the rollback fails (`rollback_failed` or `rollback_partial` in the leader log), the pool can stay active without a worker, or nodes can disagree about it: the leader's decommission watchdog resumes a missing worker within about 30 seconds once no other decommission worker runs on that node, restarting the leader resumes it otherwise, and other nodes keep their current view until they reload pool metadata, for example on restart. Check `decommission/status` before retrying.
 - Status reads (`pools/list`, `pools/status`, `decommission/status`) never wait for data movement. While movement holds the gates, a node answers from the persisted `pool.bin` and `rebalance.bin` merged into its in-memory view without publishing them; with the gates free it also publishes the refresh, as before.
 
 ```text

rustfs/src/admin/handlers/pools.rs

@@ -344,14 +344,24 @@ fn decommission_start_api_error(err: crate::storage_api::error::StorageError) ->
             source: Some(Box::new(err)),
         };
     }
+    // Keep the reason: a start rejected because another one is running
+    // should say so, not just "Invalid Request".
+    if matches!(err, crate::storage_api::error::StorageError::DecommissionAlreadyRunning) {
+        return ApiError {
+            code: S3ErrorCode::InvalidRequest,
+            message: err.to_string(),
+            source: Some(Box::new(err)),
+        };
+    }
     ApiError::from(err)
 }
 
 /// The leader-local pre-check refreshes rebalance metadata before the start.
-/// A busy gate there is the same retryable 503 the start itself reports;
-/// any other refresh failure stays an internal error.
+/// A busy gate there is the same retryable 503 the start itself reports, and
+/// a start already running is the same rejection; any other refresh failure
+/// stays an internal error.
 fn decommission_start_rebalance_refresh_error(err: crate::storage_api::error::StorageError, pools_indices: &[usize]) -> S3Error {
-    if is_lock_timeout(&err) {
+    if is_lock_timeout(&err) || matches!(err, crate::storage_api::error::StorageError::DecommissionAlreadyRunning) {
         S3Error::from(contextualize_admin_pool_api_error(
             decommission_start_api_error(err),
             "start decommission",
@@ -1324,6 +1334,17 @@ mod pools_handler_tests {
             "a busy gate is advertised as retryable"
         );
 
+        let running =
+            decommission_start_rebalance_refresh_error(crate::storage_api::error::StorageError::DecommissionAlreadyRunning, &[0]);
+        assert_eq!(running.code(), &s3s::S3ErrorCode::InvalidRequest);
+        assert!(
+            running
+                .message()
+                .is_some_and(|message| message.contains("pools [0]") && message.contains("already running")),
+            "{:?}",
+            running.message()
+        );
+
         let failed = decommission_start_rebalance_refresh_error(
             crate::storage_api::error::StorageError::other("rebalance metadata unreadable"),
             &[0],

scripts/error-other-format-baseline.txt

@@ -24,7 +24,7 @@
 10|crates/ecstore/src/cluster/rpc/remote_disk.rs
 6|crates/ecstore/src/config/com.rs
 14|crates/ecstore/src/config/storageclass.rs
-177|crates/ecstore/src/core/pools.rs
+176|crates/ecstore/src/core/pools.rs
 6|crates/ecstore/src/data_movement/mod.rs
 2|crates/ecstore/src/data_usage/local_snapshot.rs
 12|crates/ecstore/src/data_usage/mod.rs