gsadmin/rustfs
1
0
Fork 0
mirror of https://github.com/rustfs/rustfs.git synced 2026-08-13 00:26:53 +00:00
Code Issues Packages Projects Releases Wiki Activity

docs: remove agent-generated planning docs and forbid committing them (#4771)

Browse Source 
docs: remove agent-generated planning docs, forbid committing them

Delete one-shot planning/progress artifacts that were checked into the tree:
the 14 superpowers plan/tracker docs under docs/superpowers/plans/, plus
issue-scoped implementation plans, optimization conclusions, and dated
benchmark-result snapshots under docs/ (issue-4003 ListObjectsV2 plans,
get-small-file conclusion, issue824/issue829 benchmark results, issue-713
>1GiB GET baseline summary and ops guide).

Codify the rule so they do not come back:
- .gitignore drops the docs/superpowers whitelist, so anything new under
  docs/ stays ignored unless force-added.
- AGENTS.md gains an explicit 'do not commit planning-type documents' rule
  scoping version control to the durable architecture/operations/testing sets.
- docs/architecture/README.md, overview.md, arch-checks SKILL.md, and
  check_doc_paths.sh drop their references to the removed archive.
This commit is contained in:
Zhengchao An
2026-07-12 14:14:15 +08:00
committed by GitHub
parent b235762fdb
commit c4c198670d
27 changed files with 21 additions and 16103 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/superpowers/plans/2026-06-25-site-replication-hardening.md
-328
View File
@@ -1,328 +0,0 @@
# Site Replication Hardening Implementation Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Harden RustFS site replication against credential exposure, unsafe diagnostic authorization, incomplete remove cleanup, and weak operational visibility while preserving MinIO-compatible API behavior.
**Architecture:** Start with low-risk guardrails that reduce blast radius, then add deterministic cleanup and status surfaces that support later durable replay work. Keep changes local to existing admin, madmin, and bucket replication modules before introducing larger state-machine refactors.
**Tech Stack:** Rust, Axum admin handlers, serde madmin contracts, ecstore bucket replication metadata, RustFS route policy tests, focused cargo tests.
---
## File Map
- `docs/architecture/site-replication-hardening-pr.md`: PR body draft and implementation log.
- `crates/ecstore/src/bucket/target/bucket_target.rs`: credential redaction helpers and safe `Debug`.
- `rustfs/src/admin/handlers/replication.rs`: remote target response/log redaction.
- `rustfs/src/admin/handlers/bucket_meta.rs`: bucket metadata export redaction.
- `rustfs/src/admin/route_policy.rs`: site replication diagnostic route action mapping.
- `rustfs/src/admin/handlers/site_replication.rs`: diagnostic authorization, remove cleanup, status detail, and focused tests.
- `crates/madmin/src/site_replication.rs`: response DTO fields for peer errors and pending operation visibility when needed.
## Task 1: Documentation Baseline
**Files:**
- Create: `docs/superpowers/plans/2026-06-25-site-replication-hardening.md`
- Create: `docs/architecture/site-replication-hardening-pr.md`
- [ ] **Step 1: Write the implementation plan and PR draft**
Record the phased plan, implementation log, and PR template sections.
- [ ] **Step 2: Commit**
```bash
git add docs/superpowers/plans/2026-06-25-site-replication-hardening.md docs/architecture/site-replication-hardening-pr.md
git commit -m "docs: plan site replication hardening"
```
## Task 2: Redact Site Replication Credentials
**Files:**
- Modify: `crates/ecstore/src/bucket/target/bucket_target.rs`
- Modify: `rustfs/src/admin/handlers/replication.rs`
- Modify: `rustfs/src/admin/handlers/bucket_meta.rs`
- Modify: `docs/architecture/site-replication-hardening-pr.md`
- [ ] **Step 1: Add safe credential formatting**
Implement custom `Debug` for credential-bearing bucket target structures and expose redacted response/export helpers where existing APIs serialize targets.
- [ ] **Step 2: Replace unsafe logs and admin responses**
Stop returning or logging `secret_key` in remote target listing and bucket metadata export unless an existing privileged import path requires the raw stored form.
- [ ] **Step 3: Add focused tests**
Add assertions that serialized admin/export output and `Debug` output do not contain the configured secret value.
- [ ] **Step 4: Run focused tests**
```bash
cargo test -p rustfs replication --lib
cargo test -p rustfs bucket_meta --lib
cargo test -p rustfs site_replication --lib
```
- [ ] **Step 5: Commit**
```bash
git add crates/ecstore/src/bucket/target/bucket_target.rs rustfs/src/admin/handlers/replication.rs rustfs/src/admin/handlers/bucket_meta.rs docs/architecture/site-replication-hardening-pr.md
git commit -m "fix: redact site replication target secrets"
```
## Task 3: Tighten Diagnostic Route Authorization
**Files:**
- Modify: `rustfs/src/admin/route_policy.rs`
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
- Modify: `docs/architecture/site-replication-hardening-pr.md`
- [ ] **Step 1: Move devnull and netperf off read-only info authorization**
Map diagnostic POST endpoints to `SiteReplicationOperationAction` or a stricter existing action and update handlers to validate the same action.
- [ ] **Step 2: Add request bounds**
Ensure diagnostic body or duration inputs cannot be used as unbounded work by a low-privilege caller.
- [ ] **Step 3: Add route policy tests**
Assert `devnull` and `netperf` require operation-level permission.
- [ ] **Step 4: Run focused tests**
```bash
cargo test -p rustfs route_policy --lib
cargo test -p rustfs site_replication --lib
```
- [ ] **Step 5: Commit**
```bash
git add rustfs/src/admin/route_policy.rs rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
git commit -m "fix: require operation access for replication diagnostics"
```
## Task 4: Clean Replication Targets on Site Remove
**Files:**
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
- Modify: `docs/architecture/site-replication-hardening-pr.md`
- [ ] **Step 1: Add removed-peer target cleanup**
When a site is removed, traverse buckets and remove `site-repl-{deployment_id}` rules and matching bucket targets before final state is saved.
- [ ] **Step 2: Preserve runtime cache consistency**
After changing `bucket-targets.json`, call existing target refresh paths so replication clients stop using removed peers.
- [ ] **Step 3: Add focused tests**
Create a bucket target/rule fixture and assert remove cleanup prunes only the removed deployment.
- [ ] **Step 4: Run focused tests**
```bash
cargo test -p rustfs site_replication --lib
```
- [ ] **Step 5: Commit**
```bash
git add rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
git commit -m "fix: clean site replication targets on remove"
```
## Task 5: Improve Status Diagnostics
**Files:**
- Modify: `crates/madmin/src/site_replication.rs`
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
- Modify: `docs/architecture/site-replication-hardening-pr.md`
- [ ] **Step 1: Expose peer fetch errors**
Add machine-readable peer error fields to status responses without failing the entire status call.
- [ ] **Step 2: Expose pending operations**
Return pending remove or credential rotation state with acked and pending peers.
- [ ] **Step 3: Add focused tests**
Assert status includes peer fetch error details and pending operation summaries.
- [ ] **Step 4: Run focused tests**
```bash
cargo test -p rustfs site_replication --lib
```
- [ ] **Step 5: Commit**
```bash
git add crates/madmin/src/site_replication.rs rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
git commit -m "feat: expose site replication status diagnostics"
```
## Task 6: Compatibility and Release Verification
**Files:**
- Modify: `docs/architecture/site-replication-hardening-pr.md`
- [ ] **Step 1: Run focused verification**
```bash
cargo fmt --all --check
cargo test -p rustfs route_policy --lib
cargo test -p rustfs route_registration_test --lib
cargo test -p rustfs site_replication --lib
```
- [ ] **Step 2: Run broader gate if feasible**
```bash
make pre-commit
```
- [ ] **Step 3: Record verification and residual risk in PR draft**
Update the PR draft with commands run, failures, skips, and remaining follow-up items.
- [ ] **Step 4: Commit**
```bash
git add docs/architecture/site-replication-hardening-pr.md
git commit -m "docs: finalize site replication hardening notes"
```
## Task 7: Extend Scope for Full Single-PR Completion
**Files:**
- Modify: `docs/superpowers/plans/2026-06-25-site-replication-hardening.md`
- Modify: `docs/architecture/site-replication-hardening-pr.md`
- [ ] **Step 1: Record the expanded single-PR scope**
Add follow-up tasks for MinIO wire compatibility, add preflight validation, bootstrap sync, lifecycle compatibility, durable retry, and repair.
- [ ] **Step 2: Commit**
```bash
git add docs/superpowers/plans/2026-06-25-site-replication-hardening.md docs/architecture/site-replication-hardening-pr.md
git commit -m "docs: expand site replication hardening scope"
```
## Task 8: MinIO-Compatible Peer Transport Contract
**Files:**
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
- Modify: `docs/architecture/site-replication-hardening-pr.md`
- [ ] **Step 1: Add peer admin path mapping**
Route outbound peer requests through MinIO-compatible `/minio/admin/v3/site-replication/...` paths when talking to a MinIO-compatible peer, while preserving RustFS compatibility through the existing alias handling.
- [ ] **Step 2: Add focused tests**
Assert peer path mapping preserves the request path used for signing and URL construction.
- [ ] **Step 3: Commit**
```bash
git add rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
git commit -m "fix: align site replication peer paths with minio"
```
## Task 9: Add-Time Topology Preflight
**Files:**
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
- Modify: `docs/architecture/site-replication-hardening-pr.md`
- [x] **Step 1: Validate self, deployment identity, IDP, and initial data shape**
Before persisting add state, fetch remote metainfo and IDP settings, reject duplicate deployment IDs, missing local site, existing site-replication topology gaps, IDP mismatch, and multiple non-empty initial sites.
- [x] **Step 2: Add focused tests**
Cover pure topology validation for duplicate deployments, missing self, IDP mismatch, and more than one non-empty site.
- [x] **Step 3: Commit**
```bash
git add rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
git commit -m "fix: validate site replication add topology"
```
## Task 10: Full Bootstrap Sync
**Files:**
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
- Modify: `docs/architecture/site-replication-hardening-pr.md`
- [x] **Step 1: Add snapshot bootstrap after add/join**
Use `build_sr_info` as the canonical local snapshot and sync IAM and bucket metadata to peers after add, before object resync.
- [x] **Step 2: Add focused tests**
Cover bootstrap task planning order and item counts without requiring live peers.
- [x] **Step 3: Commit**
```bash
git add rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
git commit -m "feat: bootstrap site replication metadata on add"
```
## Task 11: Lifecycle Compatibility
**Files:**
- Modify: `rustfs/src/app/bucket_usecase.rs`
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
- Modify: `docs/architecture/site-replication-hardening-pr.md`
- [x] **Step 1: Stop default full lifecycle replication**
Only replicate lifecycle expiry metadata when `replicate_ilm_expiry` is enabled for site replication peers.
- [x] **Step 2: Add focused tests**
Assert lifecycle metadata is skipped by default and included only for expiry-enabled peers.
- [x] **Step 3: Commit**
```bash
git add rustfs/src/app/bucket_usecase.rs rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
git commit -m "fix: align lifecycle replication with minio semantics"
```
## Task 12: Durable Retry and Repair MVP
**Files:**
- Modify: `crates/madmin/src/site_replication.rs`
- Modify: `rustfs/src/admin/handlers/site_replication.rs`
- Modify: `docs/architecture/site-replication-hardening-pr.md`
- [x] **Step 1: Add minimal persistent retry queue**
Record failed bucket/IAM/state replication events with peer, path, payload, retry count, and last error; expose pending/failed counts in status.
- [x] **Step 2: Add repair once operation**
Provide an operation-level admin path that compares local snapshot to peer metainfo and replays missing metadata.
- [x] **Step 3: Add focused tests**
Cover queue serialization, retry status summaries, and repair task generation.
- [x] **Step 4: Commit**
```bash
git add crates/madmin/src/site_replication.rs rustfs/src/admin/handlers/site_replication.rs docs/architecture/site-replication-hardening-pr.md
git commit -m "feat: add site replication retry and repair MVP"
```
docs/superpowers/plans/kms-development-defaults-inventory.md
-105
View File
@@ -1,105 +0,0 @@
> **Archived migration snapshot** — moved from `docs/architecture/` (2026-07)
> when the architecture-review ledger it fed closed out. Kept for history; not
> maintained.
# KMS Development Defaults Inventory
This inventory tracks `KMSD-001` for
[`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660). It records
current KMS development defaults before any production default hardening.
## Scope
- Source files reviewed: `crates/kms/src/config.rs` and
`crates/kms/src/api_types.rs`.
- This is a `docs-only` task.
- No runtime behavior, config serialization, admin authorization, startup order,
global state, storage path, or crate boundary changes are included.
- Follow-up hardening must be done in separate `security-change` PRs with focused
tests.
## Current Defaults
| Source | Default | Current behavior | Classification |
|---|---|---|---|
| `KmsConfig::default()` | Local backend | Uses `LocalConfig::default()` and validates successfully. | dev-only |
| `LocalConfig::default().key_dir` | OS temp dir plus `rustfs_kms_keys` | Keys are stored under the process temp directory. | dev-only |
| `LocalConfig::default().master_key` | `None` | Local key files are stored in plaintext when no master key is configured. | invalid for production |
| `LocalConfig::default().file_permissions` | `0o600` | Owner read/write only for key files. | production-safe as a permission default, but not sufficient without encrypted key storage |
| `KmsConfig::local(key_dir)` | caller-provided key dir, default local fields | Keeps `master_key = None` unless the caller supplies one later. | dev-only unless explicit encryption material is configured |
| `KmsConfig::from_env()` local key dir | `./kms_keys` | The env loader builds a relative path, then existing validation rejects it because local key dirs must be absolute. | invalid as a standalone default |
| `KmsConfig::from_env()` local master key | absent | Leaves `master_key = None`. | invalid for production |
| `VaultConfig::default().address` | `http://localhost:8200` | HTTP is accepted by validation. | dev-only |
| `VaultTransitConfig::default().address` | `http://localhost:8200` | HTTP is accepted by validation. | dev-only |
| `VaultConfig::default().auth_method` | token dev-token | The default token is accepted if used as-is. | invalid for production |
| `VaultTransitConfig::default().auth_method` | token dev-token | The default token is accepted if used as-is. | invalid for production |
| `VaultConfig::default().tls` | `None` | No custom TLS settings. HTTPS without custom TLS relies on system CA when no skip flag is set. | production-safe only when HTTPS and system trust are intended |
| `VaultTransitConfig::default().tls` | `None` | Same TLS behavior as Vault KV2. | production-safe only when HTTPS and system trust are intended |
| `ConfigureVaultKmsRequest.skip_tls_verify` | omitted means false | `to_kms_config()` leaves TLS config as `None` unless the request explicitly sets true. | production-safe when omitted |
| `ConfigureVaultTransitKmsRequest.skip_tls_verify` | omitted means false | Same behavior as Vault KV2 configure requests. | production-safe when omitted |
| `skip_tls_verify = true` in configure requests | explicit insecure opt-in | Creates a TLS config with `skip_verify = true`. | invalid for production |
## Existing Validation Boundary
- Local key directories must be absolute.
- Timeout and retry attempts must be greater than zero.
- Vault addresses must use HTTP or HTTPS.
- Vault mount paths must be non-empty.
- HTTPS with custom TLS config and verification enabled warns when relying on
system CA instead of custom CA/client certificates.
- Existing validation does not fail closed for HTTP Vault addresses, dev-token,
missing local master key, temp key dirs, or explicit `skip_tls_verify = true`.
## Hardening Behavior
`KMSD-002` makes Local KMS unsafe defaults explicit development opt-ins or
production failures:
- no local master key fails validation unless
`allow_insecure_dev_defaults = true`;
- local key directories under the process temp directory fail validation unless
`allow_insecure_dev_defaults = true`;
- `RUSTFS_KMS_LOCAL_MASTER_KEY` is the production-safe local CLI/env path for
encrypted local key files;
- `RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS=true` is the development-only escape
hatch for local plaintext or temp-dir setups.
`KMSD-003` makes Vault unsafe defaults explicit development opt-ins or
production failures:
- HTTP Vault addresses fail validation unless explicit development opt-in is set;
- default `dev-token` credentials fail validation unless explicit development
opt-in is set;
- explicit `skip_tls_verify = true` fails validation unless explicit development
opt-in is set;
- `RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS=true` applies to `KmsConfig::from_env`;
- admin configure requests can set `allow_insecure_dev_defaults = true` for the
same development-only behavior.
These checks run in `KmsConfig::validate()` so CLI startup, persisted dynamic
configuration, service-manager start/reconfigure, and direct backend
construction use the same fail-closed behavior.
## Compatibility Notes
- Production Local KMS deployments should configure an absolute key directory
outside the process temp directory and set `RUSTFS_KMS_LOCAL_MASTER_KEY`.
- Local development setups that intentionally store plaintext key files or use
temp directories must set `RUSTFS_KMS_ALLOW_INSECURE_DEV_DEFAULTS=true` or the
admin request field `allow_insecure_dev_defaults = true`.
- Production Vault deployments should use HTTPS, non-default credentials, and
TLS verification.
- Local Vault development setups that intentionally use HTTP, `dev-token`, or
skip TLS verification must set the same explicit development opt-in.
- Legacy persisted KMS config JSON remains deserializable; old unsafe persisted
values default to production mode and fail validation until secured or
explicitly marked development-only.
## Test Coverage
- Local production fail-closed and development opt-in validation.
- Vault HTTP, default token, and skip-TLS fail-closed validation plus explicit
development opt-in.
- `KmsConfig::from_env()` development default rejection and opt-in behavior.
- Admin configure request conversion to the same validation behavior.
- KMS service manager rejects unsafe configs before moving to `Configured`.
docs/superpowers/plans/migration-progress.md
-10016
View File
File diff suppressed because it is too large Load Diff
docs/superpowers/plans/profiling-numa-capability-inventory.md
-99
View File
@@ -1,99 +0,0 @@
> **Archived migration snapshot** — moved from `docs/architecture/` (2026-07)
> when the architecture-review ledger it fed closed out. Kept for history; not
> maintained.
# Profiling And NUMA Capability Inventory
This inventory covers `G-013` for `rustfs/backlog#667`. It records the current
profiling, memory sampling, allocator, and NUMA baseline before optional runtime
sidecars are designed.
## Platform Support Matrix
| Capability | Current support | Current owner | Baseline recommendation |
|---|---|---|---|
| CPU pprof dump | Linux and macOS builds use `pprof`; other targets return an unsupported-platform error. | `rustfs/src/profiling.rs` | Keep CPU profiling opt-in through existing env flags and cancellation token. |
| Continuous CPU profiling | Linux and macOS builds can hold a continuous `ProfilerGuard` when enabled. | `rustfs/src/profiling.rs` | Preserve single-guard ownership and avoid starting multiple continuous guards. |
| Periodic CPU profiling | Linux and macOS builds can spawn a periodic sampling loop. | `rustfs/src/profiling.rs` | Keep the loop cancellation-driven and non-fatal. |
| Jemalloc memory pprof | Only `linux` + `gnu` + `x86_64` exposes jemalloc pprof dumping. Other supported builds return an unsupported-target error. | `rustfs/src/profiling.rs` | Treat memory pprof as optional and target-gated. |
| Periodic memory pprof | Only runs where jemalloc profiling control is available and active. | `rustfs/src/profiling.rs` | Keep inactive jemalloc as a skipped dump, not a startup failure. |
| Process/system memory sampling | Uses `rustfs_io_metrics::snapshot_process_resource_and_system` plus `sysinfo` total memory. | `rustfs/src/memory_observability.rs` | Keep sampling portable and metric-gated. |
| cgroup memory sampling | Reads Linux cgroup v2 or v1 memory files when present. Missing files produce no cgroup split. | `rustfs/src/memory_observability.rs` | Keep cgroup data opportunistic and absent-safe. |
| Allocator reclaim | Uses jemalloc backend on `linux` + `gnu` + `x86_64`; otherwise mimalloc variants. | `rustfs/src/allocator_reclaim.rs` | Keep backend detection read-only and preserve effective-force behavior. |
| eBPF | No runtime eBPF sidecar is currently wired into startup. | N/A | Treat eBPF as future optional Linux-only inventory, never as a required baseline. |
| NUMA | No NUMA placement or topology controller is currently wired into startup. | N/A | Treat NUMA as future optional capability with no-op fallback. |
## Cross-Platform Baseline
The current safe baseline is:
- Profiling is opt-in through env flags and must not make startup fatal.
- Startup and shutdown call profiling through `startup_profiling` lifecycle
hooks; `profiling.rs` remains the CPU/memory profiling implementation and
admin dump API owner.
- Unsupported profiling targets return structured unsupported errors or skip
startup tasks.
- Memory observability records process/system metrics and adds cgroup split
only when cgroup files exist.
- Allocator reclaim observes active HTTP, delete-tail, scanner, heal, erasure,
and GET-buffer activity before reclaiming.
- Runtime thread sizing remains owned by the Tokio runtime builder and sysinfo
core detection, not NUMA topology.
## Optional Sidecar Invariants
Future sidecars for profiling, eBPF, or NUMA must preserve these invariants:
- Sidecars must be disabled by default or target-gated until explicitly enabled.
- Unsupported targets must degrade to no-op status, not panic or fail startup.
- Sidecars must use the runtime cancellation token or an equivalent explicit
shutdown handle.
- Sidecars must not mutate Tokio worker counts after runtime creation.
- Profiling output directory fallback must stay local to profiling and must not
affect object storage paths.
- NUMA fallback must preserve current runtime thread defaults, storage set
placement, and request admission behavior.
## First Implementation Candidates
`API-013`:
- Define a read-only capability contract for profiling, cgroup memory, eBPF,
allocator backend, and NUMA availability.
- Keep the contract in a low-dependency crate and report unsupported states
explicitly.
`R-016`:
- Wire storage runtime startup to consume capability snapshots read-only.
- Do not start sidecars or mutate runtime worker ownership in the same PR.
`X-012`:
- Define the `ops.profiler.v1` extension schema for profiling capability
reporting, backend status, redaction requirements, and provenance.
- Keep the schema capability-only; it must not request profiler execution,
start sidecars, or change profile export behavior.
- Keep unsupported targets, disabled sidecars, and unknown future backends
representable as no-op capability states.
`X-013`:
- Add the extension capability snapshot contract for disabled, unsupported, and
enabled profiler backends.
- Verify optional profiler sidecar and Wasm runtimes stay disabled by default
and cannot declare a startup fatal boundary.
`R-021`:
- If a runtime service sidecar is added later, enter it through the optional
runtime boundary with explicit shutdown ownership.
- Preserve current service order, KMS/audit/notification fatal boundaries, and
scanner/heal startup semantics.
`R-022`:
- Keep optional runtime startup handoff in `startup_optional_runtimes` while
leaving concrete protocol adapters in `startup_protocols`.
- Preserve KMS-before-protocol startup ordering and disabled protocol no-op
behavior.
docs/superpowers/plans/rebalance-decommission-followup-review-plan.md
-1795
View File
File diff suppressed because it is too large Load Diff
docs/superpowers/plans/rebalance-decommission-implementation-plan-index.md
-81
View File
@@ -1,81 +0,0 @@
> **Archived implementation plan/tracker** — moved from `docs/architecture/` (2026-07).
> Kept for history; not maintained. File paths inside may reflect the pre-#3929
> module layout (e.g. `crates/ecstore/src/rebalance.rs` is now
> `crates/ecstore/src/store/rebalance.rs`; `set_disk.rs` is now `set_disk/`).
# Rebalance and Decommission Implementation Plan Index
> This index is based on `docs/architecture/rebalance-decommission-remediation-plan.md`. The remediation scope is intentionally split into smaller implementation plans because the fixes touch independent risk areas: object-version safety, distributed operation semantics, data movement internals, and operational hardening.
## Why Split the Work
The remediation backlog has fourteen fix blocks. Implementing them in one PR would make review risky and would mix unrelated failure modes. The safer path is:
1. Fix the data semantics that can lose or corrupt object-version meaning.
2. Fix distributed start/stop/recovery semantics so operators can trust cluster state.
3. Fix resource and metadata correctness in shared data movement.
4. Add observability and compatibility hardening.
Each plan below should be reviewed and executed independently unless the plan explicitly says two fixes must share the same implementation.
## Plan Set
| Plan | Fixes | Status | Purpose |
| --- | --- | --- | --- |
| `rebalance-decommission-phase1-safety-plan.md` | F01, F02, F03, F04, F05 | Drafted | Protect object-version semantics and cluster operation safety |
| `rebalance-decommission-phase2-data-movement-plan.md` | F06, F07, F08, F09, F10 | Drafted | Stream multipart migration, preserve metadata, and improve convergence |
| `rebalance-decommission-phase3-hardening-plan.md` | F11, F12, F13, F14 | Drafted | Improve cleanup reporting, auditability, metadata decoding, and threshold docs |
| `rebalance-decommission-followup-review-plan.md` | R01-R16 | Reviewed | Close post-implementation review gaps found after F01-F14 |
## Execution Recommendation
Start with `rebalance-decommission-phase1-safety-plan.md`. Phase 1 contains the highest-risk issues and defines semantics that later fixes depend on.
Within Phase 1:
1. F01 should be analyzed first because rebalance delete marker and remote tiered behavior determines whether RustFS should move or skip these versions.
2. F02 can be implemented independently once the expected decommission overwrite semantics are confirmed.
3. F03, F04, and F05 can be designed together but should remain separate PRs unless a shared peer-failure helper is introduced.
For a long-running implementation task, use the phase plans as checkpoints:
1. Implement one fix block at a time.
2. Run the focused tests listed in that block.
3. Review the diff before moving to the next fix block.
4. Run the phase-level test matrix before considering the phase complete.
After the initial F01-F14 implementation pass, continue with `rebalance-decommission-followup-review-plan.md`. That follow-up plan is ordered by remaining risk and should be executed before treating the remediation as complete.
## Upstream Change Impact Notes
### 2026-06-17: `ed55857b refactor: move bucket operations contract (#3507)`
This upstream change moved `BucketOperations` from `crates/ecstore/src/store_api/traits.rs` into `crates/storage-api/src/bucket.rs` and re-exported it from `rustfs_storage_api`.
Impact on this remediation plan:
- No F01-F14 risk item is fixed by this upstream change.
- No planned fix block needs priority changes because of this upstream change.
- Implementation code that imports or bounds `BucketOperations` must now use `rustfs_storage_api::BucketOperations`.
- Generic implementations that need the ECStore error type should use an explicit associated error bound such as `BucketOperations<Error = crate::error::Error>`.
- The only touched planning-relevant files are import/boundary changes in paths such as `crates/ecstore/src/pools.rs`, `crates/ecstore/src/store.rs`, `crates/ecstore/src/set_disk.rs`, and `rustfs/src/admin/handlers/rebalance.rs`; the rebalance/decommission safety logic remains unchanged.
## Review Gates Before Implementation
Before any code patch starts for a fix block:
- Confirm the selected behavior in the corresponding plan.
- Identify the minimal set of files for that fix.
- Write or update failing tests first.
- Keep unrelated refactors out of scope.
- Run focused tests for the touched crate before broader checks.
## Verification Baseline
For any code PR generated from these plans:
- Run focused `cargo test` commands for touched crates and modules.
- Run `cargo fmt --all`.
- Run `cargo fmt --all --check`.
- Run `make pre-commit` before opening a PR when the implementation is ready.
- Clean generated build artifacts after build-based verification.
docs/superpowers/plans/rebalance-decommission-phase1-safety-plan.md
-381
View File
@@ -1,381 +0,0 @@
> **Archived implementation plan/tracker** — moved from `docs/architecture/` (2026-07).
> Kept for history; not maintained. File paths inside may reflect the pre-#3929
> module layout (e.g. `crates/ecstore/src/rebalance.rs` is now
> `crates/ecstore/src/store/rebalance.rs`; `set_disk.rs` is now `set_disk/`).
# Rebalance and Decommission Phase 1 Safety Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Fix the highest-risk rebalance/decommission safety defects so object-version semantics and distributed operation state cannot silently diverge.
**Architecture:** Phase 1 keeps the existing ECStore/Rebalance/PoolMeta architecture and introduces narrowly scoped behavior changes. The plan favors fail-closed semantics for unsafe migration, peer propagation, and startup conflicts. Each fix block should be implemented as a separate PR unless a shared helper is explicitly called out.
**Tech Stack:** Rust, Tokio, ECStore, Axum admin handlers, tonic peer RPC, MessagePack metadata, existing RustFS test modules.
---
## Scope
This plan covers:
- F01: rebalance delete marker and remote tiered version migration semantics.
- F02: decommission `DataMovementOverwriteErr` cleanup safety.
- F03: decommission `pool_meta` reload as a start barrier.
- F04: store startup recovery ordering for pool meta and rebalance.
- F05: rebalance distributed start/stop semantics and `stopping` visibility.
This plan does not cover multipart streaming, checksum preservation, lifecycle-expired cleanup, overwrite convergence, cleanup warning UX, audit fields, or metadata decoding hardening. Those belong to later phase plans.
## Shared Principles
- Treat unknown target equivalence as unsafe.
- Do not clean a source entry unless the target state is proven complete or the version is intentionally skipped by a documented policy.
- Do not report distributed admin success when required peers failed.
- Do not auto-run rebalance and decommission together after restart.
- Prefer small local helpers near existing logic over broad refactors.
## F01: Rebalance Delete Marker and Remote Tiered Version Safety
### Decision Needed
Remote tiered versions need a product decision before coding:
- **Recommended short-term behavior:** Match MinIO and skip remote tiered versions during rebalance. This avoids changing remote-tier metadata during a balancing operation and removes the immediate source-cleanup data-loss risk.
- **Later behavior:** Implement explicit cross-pool target metadata movement for remote tiered versions only after tests prove remote references, lifecycle state, and cleanup semantics.
Delete markers should not be skipped. They preserve versioned delete semantics and must be copied to the selected target pool before source cleanup.
### Files
- Modify: `crates/ecstore/src/rebalance.rs`
- Possibly modify: `crates/ecstore/src/data_movement.rs`
- Avoid unless necessary: `crates/ecstore/src/set_disk.rs`
- Test: existing rebalance unit tests in `crates/ecstore/src/rebalance.rs`
### Design
1. Add an ECStore-level helper for rebalance delete marker movement.
- Input: source pool index, bucket, object name, source `FileInfo`, version ID.
- Behavior: choose a target pool using the same placement rules as normal data movement, excluding source/rebalancing/decommissioned pools.
- Write the delete marker metadata to the target pool with the original version ID, mod time, delete marker flag, and replication state.
- Return success only when the target write succeeds or an equivalent target delete marker is confirmed.
2. Change the delete marker branch in `migrate_entry_version_with_retry_wait`.
- Do not call `SetDisks::delete_object_for_migration` for rebalance delete markers.
- Call the ECStore-level helper via the `transfer`/backend abstraction or split delete marker handling out of the `SetDisks` backend path.
- Do not set `moved = true` unless the target helper succeeds.
3. Change remote tiered branch.
- Short term: return `ignored = true`, `cleanup_ignored = false`, `moved = false` for remote tiered versions so they do not contribute to full source cleanup.
- If skipping a remote tiered version prevents source cleanup, record a clear status reason so operators understand why the entry remains.
4. Source cleanup must remain blocked if any version was skipped without cleanup permission.
### Implementation Tasks
- [ ] Write a failing test proving a rebalance delete marker currently does not require target-pool metadata before counting complete.
- [ ] Write a failing test proving remote tiered versions do not allow source cleanup under the short-term skip policy.
- [ ] Implement the delete marker target write helper.
- [ ] Replace the rebalance delete marker source-set branch with the target write helper.
- [ ] Replace the remote tiered branch with the skip-without-cleanup policy.
- [ ] Run focused rebalance tests.
### Focused Test Commands
```bash
cargo test -p rustfs-ecstore rebalance_delete_marker --lib
cargo test -p rustfs-ecstore remote_tier --lib
cargo test -p rustfs-ecstore rebalance_entry --lib
```
### Acceptance Criteria
- Rebalance does not clean a source entry when a delete marker target write fails.
- Rebalance preserves versioned delete behavior after source cleanup.
- Remote tiered versions are either safely skipped without source cleanup or explicitly moved with verified target metadata.
- Existing non-delete-marker rebalance behavior remains unchanged.
### Risks
- The existing `MigrationBackend` trait is source-set oriented. If adapting it becomes invasive, prefer an ECStore-specific path in `rebalance_entry` rather than widening the trait for one branch.
- Skipping remote tiered versions may leave more source entries behind. That is safer than deleting unverified metadata and can be improved in a later PR.
---
## F02: Decommission `DataMovementOverwriteErr` Cleanup Safety
### Decision
`DataMovementOverwriteErr` must not be cleanup-safe by default. It can count as complete only after explicit target equivalence verification.
### Files
- Modify: `crates/ecstore/src/pools.rs`
- Possibly modify: `crates/ecstore/src/data_movement.rs`
- Test: decommission tests in `crates/ecstore/src/pools.rs`
### Design
1. Update delete marker and remote tiered decommission error handling.
- Keep object-not-found and version-not-found handling as cleanup-safe only when existing semantics justify it.
- Remove `is_err_data_movement_overwrite(&err)` from branches that set `cleanup_ignored = true`.
2. Add a narrow equivalence helper only if an existing target version can be inspected cheaply.
- Compare bucket, object, version ID, delete marker state, ETag where applicable, mod time, and key metadata.
- If comparison cannot be done reliably in the current code path, treat overwrite as failure for Phase 1.
3. Preserve source entry on unsafe overwrite.
- Set `failure = true`.
- Include a clear error stage in logs/status.
- Do not increment the decommissioned count.
### Implementation Tasks
- [ ] Write a failing unit test for delete marker decommission where `DataMovementOverwriteErr` must not count complete.
- [ ] Write a failing unit test for remote tiered decommission where `DataMovementOverwriteErr` must not count complete.
- [ ] Remove `is_err_data_movement_overwrite` from cleanup-safe conditions in those branches.
- [ ] Add an explicit helper or comment documenting why overwrite is unsafe without equivalence.
- [ ] Run focused decommission tests.
### Focused Test Commands
```bash
cargo test -p rustfs-ecstore decommission --lib
cargo test -p rustfs-ecstore DataMovementOverwriteErr --lib
```
### Acceptance Criteria
- `DataMovementOverwriteErr` does not set `cleanup_ignored = true` by default.
- Source cleanup does not run when overwrite equivalence is unknown.
- Logs make unsafe overwrite distinguishable from not-found cleanup-safe cases.
### Risks
- Tightening this behavior may cause more decommission retries/failures in clusters that previously masked unsafe state. That is intended; status must make the failure actionable.
---
## F03: Decommission Pool Meta Reload Barrier
### Decision
A successful decommission start must mean all required peers have acknowledged the updated pool meta or the API must return an explicit failure/degraded result.
### Files
- Modify: `crates/ecstore/src/pools.rs`
- Modify: `crates/ecstore/src/notification_sys.rs`
- Modify: `rustfs/src/admin/handlers/pools.rs`
- Possibly modify: `crates/ecstore/src/rpc/peer_rest_client.rs`
- Possibly modify: `rustfs/src/storage/rpc/node_service.rs`
### Design
1. Change `start_decommission` propagation behavior.
- After `pool_meta.save`, call `reload_pool_meta`.
- If peer reload returns an aggregate error, return that error to the admin handler.
- Do not silently proceed.
2. Decide rollback behavior.
- Preferred minimal Phase 1: fail the API and leave persisted `pool_meta` in decommission state only if rollback is unsafe or unavailable; status must show reload failure.
- Stronger option: persist a reverted pool meta before returning failure. This needs careful lock and persistence review and should not be attempted without tests.
3. Ensure decommission workers do not spawn after reload failure.
- If the caller starts workers after `start_decommission` returns, returning `Err` is sufficient.
- If any background path can resume from persisted metadata after a failed reload, status must clearly show pending/degraded state.
### Implementation Tasks
- [ ] Write a failing test for `start_decommission` with peer reload failure.
- [ ] Change reload failure from `warn` to returned error.
- [ ] Update admin handler error mapping so clients do not receive success.
- [ ] Add status/log context for peer reload failure.
- [ ] Run focused pool/decommission tests.
### Focused Test Commands
```bash
cargo test -p rustfs-ecstore start_decommission --lib
cargo test -p rustfs-ecstore reload_pool_meta --lib
cargo test -p rustfs decommission --lib
```
### Acceptance Criteria
- Admin start decommission does not return success when peer reload fails.
- Decommission workers do not start after reload failure through the admin path.
- Failure output includes enough peer context for operators.
### Risks
- Persisted pool meta may already be updated before reload failure is detected. Avoid adding rollback until the exact persistence safety is reviewed. Fail-closed reporting is the minimum safe fix.
---
## F04: Store Init Recovery Ordering
### Decision
Store init must load and install pool meta before deciding whether rebalance can auto-start.
### Files
- Modify: `crates/ecstore/src/store/init.rs`
- Possibly modify: `crates/ecstore/src/rebalance.rs`
- Possibly modify: `crates/ecstore/src/pools.rs`
### Design
1. Reorder init sequence.
- Initialize boot time.
- Load pool meta.
- Validate and install pool meta.
- Resolve resumable decommission pools.
- Load rebalance meta.
- Start rebalance only if no decommission is active or resumable.
2. Define conflict behavior.
- If active decommission and started rebalance metadata coexist, decommission wins for Phase 1.
- Rebalance should not start.
- Record or expose a clear deferred/conflict reason if existing status structures can carry it without broad schema changes.
3. Preserve existing happy path.
- Clusters with only rebalance metadata should still auto-start rebalance.
- Clusters with only decommission metadata should still resume decommission after the existing delay.
### Implementation Tasks
- [ ] Write a failing init test for active decommission plus rebalance metadata.
- [ ] Move pool meta load/validate/install before `load_rebalance_meta` and `start_rebalance`.
- [ ] Add a helper such as `should_auto_start_rebalance_after_init`.
- [ ] Ensure decommission resume still uses the installed pool meta.
- [ ] Run focused init tests.
### Focused Test Commands
```bash
cargo test -p rustfs-ecstore init --lib
cargo test -p rustfs-ecstore rebalance_meta --lib
cargo test -p rustfs-ecstore decommission --lib
```
### Acceptance Criteria
- Rebalance does not start when active decommission is loaded from pool meta.
- Rebalance still starts when no decommission is active.
- Decommission resume behavior is unchanged except that it no longer races with an already-started rebalance.
### Risks
- Init ordering can affect existing startup failure behavior. Keep error handling and stage names explicit so operators can identify which stage failed.
---
## F05: Rebalance Distributed Start/Stop Semantics
### Decision
Admin start/stop must not report plain success for partial cluster state. If stop remains asynchronous, status must expose that distinction.
### Files
- Modify: `rustfs/src/admin/handlers/rebalance.rs`
- Modify: `rustfs/src/storage/rpc/node_service.rs`
- Modify: `crates/ecstore/src/notification_sys.rs`
- Modify: `crates/ecstore/src/rpc/peer_rest_client.rs`
- Possibly modify: `crates/ecstore/src/rebalance.rs`
### Design
1. Fix peer RPC stop first.
- `node_service::stop_rebalance` must return `success=false` or gRPC error when `store.stop_rebalance().await` fails.
- `notification_sys.stop_rebalance` must propagate aggregate peer failure instead of warning and returning `Ok`.
2. Fix start propagation.
- Avoid returning success from peer `load_rebalance_meta(start=true)` before local start validation has completed.
- If fully synchronous worker startup is too invasive, split semantics:
- metadata loaded;
- start accepted;
- worker running observable through status.
- Admin start should fail when any required peer fails load/start acceptance.
3. Define rollback/degraded semantics.
- Preferred Phase 1: return failure on propagation failure and attempt local stop if local rebalance was already started.
- If rollback fails, return a degraded error and expose status.
4. Add status language for stop.
- If cancellation is requested but workers may still be running, status should not claim fully stopped.
- Minimal approach: add a `stopping` or equivalent field if existing response types allow it.
### Implementation Tasks
- [ ] Write a failing test for peer stop returning success despite local stop error.
- [ ] Update `node_service::stop_rebalance` to return failure details.
- [ ] Write a failing test for `notification_sys.stop_rebalance` aggregate peer failure.
- [ ] Update `notification_sys.stop_rebalance` to return aggregate error.
- [ ] Write a failing admin start propagation test.
- [ ] Update admin start to fail or rollback on propagation failure.
- [ ] Add status distinction for requested stop versus completed stop if response schema permits.
- [ ] Run focused admin/RPC/rebalance tests.
### Focused Test Commands
```bash
cargo test -p rustfs-ecstore stop_rebalance --lib
cargo test -p rustfs-ecstore load_rebalance_meta --lib
cargo test -p rustfs rebalance --lib
```
### Acceptance Criteria
- Peer stop failure is visible to callers.
- Admin stop fails or reports degraded state on peer failure.
- Admin start fails or reports degraded state on peer load/start failure.
- Stop status does not mislead operators while workers are still winding down.
### Risks
- API response schema changes may affect clients. Prefer backward-compatible additions where possible, and keep error responses compatible with existing admin error handling.
---
## Phase 1 Test Matrix
Run these once the individual fix tests pass:
```bash
cargo test -p rustfs-ecstore rebalance --lib
cargo test -p rustfs-ecstore decommission --lib
cargo test -p rustfs-ecstore pools --lib
cargo test -p rustfs rebalance --lib
cargo test -p rustfs pools --lib
cargo fmt --all --check
```
Before opening a PR with code changes:
```bash
cargo fmt --all
cargo fmt --all --check
make pre-commit
```
After build-based verification, clean generated build artifacts to avoid unnecessary disk usage.
## Suggested PR Split
1. PR 1: F02 only. Smallest fail-closed fix.
2. PR 2: F04 only. Startup ordering and conflict tests.
3. PR 3: F03 only. Decommission peer reload barrier.
4. PR 4: F05 stop semantics first, then start semantics if review scope remains manageable.
5. PR 5: F01 delete marker fix and remote tiered skip policy.
F01 is highest risk, but it may need the most design review. F02 and F04 are good first implementation candidates because they reduce safety risk with narrower code changes.
## Open Questions Before Coding
1. For F01 remote tiered versions, should Phase 1 match MinIO and skip them during rebalance, or should RustFS implement cross-pool remote metadata movement now?
2. For F03 reload failure, should the implementation attempt to rollback persisted pool meta, or fail closed and expose degraded metadata state?
3. For F05 start propagation, is a backward-compatible degraded error response acceptable, or must the API stay strictly compatible with current success/error shapes?
docs/superpowers/plans/rebalance-decommission-phase2-data-movement-plan.md
-368
View File
@@ -1,368 +0,0 @@
> **Archived implementation plan/tracker** — moved from `docs/architecture/` (2026-07).
> Kept for history; not maintained. File paths inside may reflect the pre-#3929
> module layout (e.g. `crates/ecstore/src/rebalance.rs` is now
> `crates/ecstore/src/store/rebalance.rs`; `set_disk.rs` is now `set_disk/`).
# Rebalance and Decommission Phase 2 Data Movement Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Make shared data movement safe for large objects, metadata-preserving, and resilient to overwrite and cleanup races.
**Architecture:** Phase 2 keeps `crates/ecstore/src/data_movement.rs` as the shared migration core, but tightens its streaming, metadata, and equivalence behavior. Rebalance and decommission should share small helpers where their safety checks are identical, while keeping operation-specific policy decisions in `rebalance.rs` and `pools.rs`.
**Tech Stack:** Rust, Tokio async readers, ECStore object APIs, multipart APIs, lifecycle evaluator, filemeta metadata types, existing crate-local unit tests.
---
## Scope
This plan covers:
- F06: stream multipart data movement instead of buffering whole parts.
- F07: preserve and verify checksum, replication, version purge, and object-lock metadata.
- F08: align or explicitly define decommission lifecycle-expired cleanup behavior.
- F09: handle rebalance overwrite races with explicit target equivalence checks.
- F10: add final source cleanup verification before prefix deletion.
This plan assumes Phase 1 either has landed or its semantics are stable enough to build on.
## Shared Principles
- Streaming must keep memory bounded by buffer size, not part size.
- Metadata equivalence should be explicit and testable.
- A version is complete only when copied, intentionally skipped by policy, or proven equivalent on target.
- Source cleanup must be the last step and must verify the source version set has not changed.
- Fail closed when equivalence cannot be proven.
## F06: Stream Multipart Data Movement
### Decision
Multipart migration must not allocate a buffer equal to `part.size`. It should stream each part into `put_object_part`.
### Files
- Modify: `crates/ecstore/src/data_movement.rs`
- Possibly modify: `crates/ecstore/src/store/multipart.rs`
- Possibly modify: `crates/ecstore/src/store_api/readers.rs`
- Test: data movement tests in `crates/ecstore/src/data_movement.rs`
### Design
1. Replace per-part `Vec<u8>` allocation.
- Current behavior reads each part with `read_exact` into `chunk`.
- New behavior should create a bounded reader over the shared object stream for exactly `part.size` bytes.
2. Preserve part boundaries.
- Each `put_object_part` call must receive a reader that ends exactly at the part boundary.
- If a part stream ends early, return a staged `read_part` error and abort the multipart upload.
3. Preserve part index behavior.
- Continue decoding `part.index`.
- Preserve indexed reader behavior so erasure/indexed metadata remains compatible.
4. Preserve abort behavior.
- `abort_multipart_upload` must still run if any part upload or complete call fails before completion is marked.
### Implementation Tasks
- [ ] Add a failing data movement test with a fake large multipart stream that panics or fails if code tries to allocate/read the full part into memory.
- [ ] Add a test proving part boundary reads consume exactly the configured part sizes.
- [ ] Introduce a bounded async reader helper for a single multipart part.
- [ ] Replace `vec![0u8; part.size]` and `read_exact` in multipart migration with the bounded reader helper.
- [ ] Ensure part upload still wraps data in `PutObjReader` with correct size, actual size, and index.
- [ ] Keep existing abort-on-error behavior and add a focused test for failure during streamed part upload.
### Focused Test Commands
```bash
cargo test -p rustfs-ecstore data_movement --lib
cargo test -p rustfs-ecstore multipart --lib
```
### Acceptance Criteria
- Data movement memory is bounded and not proportional to multipart part size.
- Multipart part boundaries, sizes, ETags, and indexes remain correct.
- Failed streamed part upload aborts the temporary multipart upload.
### Risks
- Async reader composition can accidentally over-read into the next part. The part-boundary test is mandatory.
- If `put_object_part` requires a concrete reader type, introduce the smallest adapter needed rather than rewriting multipart internals.
---
## F07: Preserve and Verify Full Data Movement Metadata
### Decision
Checksum preservation is a confirmed fix. Replication, version purge, and object-lock behavior must be tested first; production changes should follow only where tests reveal loss or mismatch.
### Files
- Modify: `crates/ecstore/src/data_movement.rs`
- Possibly modify: `crates/ecstore/src/store_api/types.rs`
- Possibly modify: `crates/ecstore/src/set_disk.rs`
- Possibly modify: `crates/filemeta/src/fileinfo.rs`
- Possibly modify: `crates/filemeta/src/replication.rs`
- Test: data movement tests in `crates/ecstore/src/data_movement.rs`
### Design
1. Define metadata equivalence for data movement.
- Required fields: version ID, ETag, size, actual size, mod time, user metadata, storage class, checksum, multipart checksum, replication state, version purge state, object-lock mode/date, legal hold.
- Use a helper local to data movement tests first. Promote to production only if F09/F10 need it.
2. Preserve multipart checksum.
- When source part checksum exists, populate the corresponding `CompletePart` checksum fields.
- Preserve object-level multipart checksum metadata when complete metadata is written.
3. Validate replication state preservation.
- Current code copies `user_defined` and `set_disk` reconstructs `replication_state_internal` from metadata.
- Add tests to prove this works for replication status and version purge status.
- If tests fail, extend `ObjectOptions` or write path metadata handling narrowly.
4. Validate object lock preservation.
- Current code copies `user_defined`, which should carry object-lock headers.
- Add tests for retention mode/date and legal hold.
- If tests fail, fix the metadata copy path without changing normal user copy semantics.
### Implementation Tasks
- [ ] Add a metadata equivalence assertion helper in data movement tests.
- [ ] Add a failing multipart checksum preservation test.
- [ ] Populate `CompletePart` checksum fields from source part metadata.
- [ ] Add single-part checksum preservation test and fix object-level checksum metadata if needed.
- [ ] Add replication state and version purge state preservation tests.
- [ ] Add object-lock retention and legal hold preservation tests.
- [ ] Apply the minimal production fixes required by those tests.
### Focused Test Commands
```bash
cargo test -p rustfs-ecstore data_movement --lib
cargo test -p rustfs-ecstore checksum --lib
cargo test -p rustfs-ecstore replication --lib
cargo test -p rustfs-ecstore object_lock --lib
```
### Acceptance Criteria
- Migrated single-part and multipart objects preserve checksum behavior.
- Migrated multipart parts preserve per-part checksum metadata where RustFS stores it.
- Replication state and version purge state are equivalent after migration.
- Object-lock retention and legal hold remain unchanged after migration.
### Risks
- Some metadata may be derived rather than stored directly. Tests should compare externally observable object info, not only internal fields.
- Avoid broad `ObjectOptions` expansion unless current metadata copy cannot preserve a required field.
---
## F08: Lifecycle-Expired Version Cleanup Semantics
### Decision
RustFS should either align with MinIO by counting lifecycle-expired versions as decommission-complete, or explicitly document and expose retained expired source entries. The recommended behavior is to align with MinIO when lifecycle/object-lock/replication checks say the version is safe to expire.
### Files
- Modify: `crates/ecstore/src/pools.rs`
- Possibly modify: lifecycle evaluator under `crates/ecstore/src/bucket/lifecycle/`
- Test: decommission tests in `crates/ecstore/src/pools.rs`
### Design
1. Preserve existing lifecycle safety checks.
- Do not bypass object-lock or replication constraints.
- Keep `should_skip_lifecycle_for_data_movement` or equivalent evaluator as the source of truth.
2. Count safe expired versions as complete for source cleanup.
- Replace `expired == 0 && decommissioned == total_versions` with semantics that allow `decommissioned + expired == total_versions` when expired versions are safe.
3. Keep status honest.
- If a version is retained because expiration is not safe, status should show it as remaining, not completed.
### Implementation Tasks
- [ ] Write a failing test where one migrated version plus one lifecycle-expired version should allow source cleanup.
- [ ] Write a test where object-lock protected expired-looking version must not allow cleanup.
- [ ] Write a test where replication-pending version must not allow cleanup.
- [ ] Update source cleanup predicate to count safe expired versions.
- [ ] Update counters/status if existing fields distinguish expired from decommissioned.
### Focused Test Commands
```bash
cargo test -p rustfs-ecstore decommission --lib
cargo test -p rustfs-ecstore lifecycle --lib
cargo test -p rustfs-ecstore object_lock --lib
```
### Acceptance Criteria
- Source entry is cleaned when every version is migrated or safely lifecycle-expired.
- Protected versions still block cleanup.
- Decommission status remains understandable for migrated, expired, and blocked versions.
### Risks
- Counting expired versions as complete can hide lifecycle evaluator bugs. Tests must include protected negative cases.
---
## F09: Rebalance Overwrite Race Equivalence
### Decision
Unsafe `DataMovementOverwriteErr` remains a failure. Equivalent target overwrite can be counted complete only after explicit comparison.
### Files
- Modify: `crates/ecstore/src/data_movement.rs`
- Modify: `crates/ecstore/src/rebalance.rs`
- Possibly modify: `crates/ecstore/src/store/object.rs`
- Test: data movement and rebalance tests
### Design
1. Define target equivalence helper.
- Compare version ID, ETag, size, actual size, mod time, checksum, delete marker state, and selected user metadata.
- Reuse the test helper from F07 if appropriate, but production helper must be limited to fields needed for safety.
2. Apply helper only to overwrite race cases.
- If `DataMovementOverwriteErr` occurs and target pool differs from source pool, inspect target object/version.
- If equivalent, count migration as complete.
- If not equivalent or cannot inspect target, return failure.
3. Keep source-equals-target unsafe.
- Do not convert source-equals-target into success without independent target proof.
### Implementation Tasks
- [ ] Add a failing rebalance test where equivalent target version exists and overwrite should converge.
- [ ] Add a failing rebalance test where target version differs and overwrite must fail.
- [ ] Implement a minimal target equivalence helper.
- [ ] Wire the helper into data movement overwrite handling for rebalance.
- [ ] Keep decommission behavior aligned with F02.
### Focused Test Commands
```bash
cargo test -p rustfs-ecstore DataMovementOverwriteErr --lib
cargo test -p rustfs-ecstore rebalance --lib
cargo test -p rustfs-ecstore data_movement --lib
```
### Acceptance Criteria
- Equivalent overwrite does not block rebalance convergence.
- Non-equivalent overwrite does not clean source.
- Logs/status distinguish equivalent overwrite from unsafe overwrite.
### Risks
- Comparing too few fields can mark a corrupt target complete. Prefer strict comparison in Phase 2.
- Comparing too many volatile fields can prevent convergence. Use tests to calibrate.
---
## F10: Source Cleanup Preflight Verification
### Decision
Before deleting a source prefix, rebalance and decommission should verify the source version set still matches the migrated or intentionally skipped set.
### Files
- Modify: `crates/ecstore/src/rebalance.rs`
- Modify: `crates/ecstore/src/pools.rs`
- Possibly modify: `crates/ecstore/src/set_disk.rs`
- Test: rebalance and decommission cleanup tests
### Design
1. Capture source version identity at scan time.
- Version identity should include object name, version ID, delete marker flag, and enough metadata to detect a new or changed version.
2. Re-read before cleanup.
- Immediately before `delete_prefix`, read current source metadata.
- Compare current identity set to the expected cleanup set.
3. Defer on mismatch.
- Rebalance should defer/retry the entry with a clear last error.
- Decommission should fail the entry or retry according to existing decommission retry policy.
4. Keep not-found idempotent.
- If source entry is already gone, cleanup remains successful.
### Implementation Tasks
- [ ] Add helper to build a stable version identity set from `FileInfoVersions`.
- [ ] Add rebalance test where source metadata changes between migration and cleanup.
- [ ] Add decommission test where source metadata changes between migration and cleanup.
- [ ] Add cleanup preflight before rebalance source `delete_prefix`.
- [ ] Add cleanup preflight before decommission source `delete_prefix`.
- [ ] Return deferred/failure status with a clear reason on mismatch.
### Focused Test Commands
```bash
cargo test -p rustfs-ecstore rebalance_entry --lib
cargo test -p rustfs-ecstore decommission_entry --lib
cargo test -p rustfs-ecstore cleanup --lib
```
### Acceptance Criteria
- Cleanup does not delete a source entry if the version set changed after migration started.
- Already-deleted source remains idempotent.
- Mismatch errors are visible in status or last-error fields.
### Risks
- Extra metadata reads can add I/O to hot migration paths. Keep this as a cleanup-only check.
---
## Phase 2 Test Matrix
Run after individual fix tests pass:
```bash
cargo test -p rustfs-ecstore data_movement --lib
cargo test -p rustfs-ecstore multipart --lib
cargo test -p rustfs-ecstore rebalance --lib
cargo test -p rustfs-ecstore decommission --lib
cargo test -p rustfs-ecstore lifecycle --lib
cargo fmt --all --check
```
Before PR:
```bash
cargo fmt --all
cargo fmt --all --check
make pre-commit
```
Clean generated build artifacts after build-based verification.
## Suggested PR Split
1. PR 1: F06 streaming multipart migration.
2. PR 2: F07 checksum and metadata preservation tests/fixes.
3. PR 3: F08 lifecycle-expired cleanup semantics.
4. PR 4: F09 overwrite equivalence.
5. PR 5: F10 cleanup preflight verification.
F06 and F07 may combine only if checksum preservation requires the new streaming reader shape. F09 should wait for F07 if it depends on metadata equivalence helpers.
## Open Questions Before Coding
1. Which metadata fields are considered mandatory for equivalence in Phase 2: strict internal fields or externally observable fields only?
2. Should F08 fully match MinIO cleanup behavior, or should RustFS keep expired source entries but expose them as residual state?
3. Should F10 apply to both rebalance and decommission in the same PR, or should rebalance land first as the higher-risk cleanup path?
docs/superpowers/plans/rebalance-decommission-phase3-hardening-plan.md
-325
View File
@@ -1,325 +0,0 @@
> **Archived implementation plan/tracker** — moved from `docs/architecture/` (2026-07).
> Kept for history; not maintained. File paths inside may reflect the pre-#3929
> module layout (e.g. `crates/ecstore/src/rebalance.rs` is now
> `crates/ecstore/src/store/rebalance.rs`; `set_disk.rs` is now `set_disk/`).
# Rebalance and Decommission Phase 3 Hardening Plan
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
**Goal:** Improve operator visibility, auditability, metadata robustness, and documented compatibility after the Phase 1 and Phase 2 safety fixes.
**Architecture:** Phase 3 should not change core data movement semantics unless a hardening test exposes a safety bug. It adds bounded status data, structured logs, metadata validation tests, and an explicit decision around rebalance completion tolerance.
**Tech Stack:** Rust, tracing structured logs, admin status DTOs, serde/rmp metadata decoding, existing guardrail scripts, crate-local unit tests.
---
## Scope
This plan covers:
- F11: improve rebalance cleanup failure reporting.
- F12: add structured audit fields for rebalance/decommission operations.
- F13: harden persisted rebalance and pool metadata decoding.
- F14: decide and document rebalance completion threshold behavior.
This plan assumes the safety semantics from Phase 1 and the data movement behavior from Phase 2 are stable.
## Shared Principles
- Hardening must not hide safety failures.
- Status fields should be bounded to avoid unbounded metadata growth.
- Logs must be structured, searchable, and free of secrets.
- Metadata compatibility must be explicit: strict where possible, legacy-compatible where necessary.
## F11: Rebalance Cleanup Failure Reporting
### Decision
Rebalance cleanup failures should not be invisible completion details. Admin status should expose whether completion had cleanup warnings and provide bounded object-level detail.
### Files
- Modify: `crates/ecstore/src/rebalance.rs`
- Modify: `rustfs/src/admin/handlers/rebalance.rs`
- Possibly modify: admin response DTOs in the same module or related madmin types
- Test: rebalance status and metadata tests
### Design
1. Extend cleanup warning metadata.
- Keep existing count.
- Add a bounded list of recent cleanup failures, for example last 10 entries.
- Each entry should include bucket, object, message, and timestamp.
2. Keep metadata bounded.
- Do not store every cleanup failure unboundedly in `rebalance.bin`.
- Use a ring-buffer style helper or truncate older entries.
3. Make terminal state visible.
- If rebalance completes with cleanup warnings, status should show warning count and details.
- Avoid reporting a clean completion when source cleanup failed.
### Implementation Tasks
- [ ] Add a cleanup warning entry struct with bounded retention.
- [ ] Add metadata compatibility defaults for older `rebalance.bin` without the new list.
- [ ] Update `record_rebalance_cleanup_warning_in_meta` to append bounded entries and update count/last fields.
- [ ] Update admin status serialization to expose warning count and entries.
- [ ] Add tests for one warning, multiple warnings, and bounded truncation.
- [ ] Add legacy metadata decode test.
### Focused Test Commands
```bash
cargo test -p rustfs-ecstore cleanup_warning --lib
cargo test -p rustfs-ecstore rebalance_meta --lib
cargo test -p rustfs rebalance --lib
```
### Acceptance Criteria
- Multiple cleanup failures are visible through count and bounded details.
- Legacy rebalance metadata still decodes.
- Status distinguishes clean completion from completion with cleanup warnings.
### Risks
- Adding status fields may affect clients if response schemas are strict. Prefer additive fields with defaults.
---
## F12: Structured Audit Fields
### Decision
Rebalance and decommission admin operations should have consistent structured logs for success, rejection, propagation failure, and partial/degraded state.
### Files
- Modify: `rustfs/src/admin/handlers/rebalance.rs`
- Modify: `rustfs/src/admin/handlers/pools.rs`
- Modify: `crates/ecstore/src/notification_sys.rs`
- Modify: `crates/ecstore/src/rpc/peer_rest_client.rs`
- Possibly modify: `scripts/check_logging_guardrails.sh` or related guardrails if logging rules require updates
### Design
1. Define standard fields.
- `event`
- `component`
- `subsystem`
- `action`
- `result`
- `request_id` when available
- `actor` or masked access key when available
- `remote_addr` when available
- `pool_indices` or `rebalance_id`
- `peer`
- `error`
2. Apply to admin handlers.
- Start, stop, cancel, status-affecting operations.
- Authorization failures should remain handled by existing auth paths but should have enough context if already logged.
3. Apply to peer propagation.
- Peer success and failure should use stable event names.
- Partial failure must be warn/error, not info-only.
4. Avoid secret leakage.
- Never log raw credentials, signatures, tokens, or full auth headers.
### Implementation Tasks
- [ ] Inventory existing rebalance/decommission log events and field names.
- [ ] Define a small local helper or convention for masked actor/request fields if one already exists.
- [ ] Update rebalance admin start/stop/status logs.
- [ ] Update decommission start/cancel/status logs.
- [ ] Update notification peer propagation failure logs.
- [ ] Add log-capture tests for success, partial failure, and rejected operation where feasible.
- [ ] Run logging guardrail script if the repository has one.
### Focused Test Commands
```bash
cargo test -p rustfs rebalance --lib
cargo test -p rustfs pools --lib
cargo test -p rustfs-ecstore notification --lib
scripts/check_logging_guardrails.sh
```
### Acceptance Criteria
- Operators can answer who requested an operation, what resource it targeted, and which peers failed.
- Partial failures are searchable through stable structured fields.
- Logs contain no secrets.
### Risks
- Logging changes can be noisy. Keep high-volume per-object logs out of admin audit events.
---
## F13: Persisted Metadata Decode Hardening
### Decision
Persisted rebalance and pool metadata should reject or surface unknown/corrupt fields where compatibility allows. Legacy compatibility must be tested explicitly.
### Files
- Modify: `crates/ecstore/src/rebalance.rs`
- Modify: `crates/ecstore/src/pools.rs`
- Possibly add fixtures under an existing test fixture directory if one exists
- Test: metadata decode tests in `rebalance.rs` and `pools.rs`
### Design
1. Identify persisted structs.
- Rebalance metadata and stats structs.
- Pool metadata and decommission info structs.
2. Choose strictness per struct.
- Use strict unknown-field rejection where metadata is not expected to carry forward-compatible fields.
- For legacy-compatible structs, keep decode lenient but log or validate unknown/unsupported state where possible.
3. Add state validation.
- Reject conflicting terminal states such as complete plus canceled plus running.
- Reject impossible counters or invalid pool indices where existing validation can catch them.
4. Preserve legacy fixtures.
- Existing old metadata must still decode if compatibility is required.
### Implementation Tasks
- [ ] List all rebalance and pool metadata structs currently deserialized from persisted bytes.
- [ ] Add fixture tests for unknown fields, missing critical fields, and conflicting terminal states.
- [ ] Add strict serde attributes only where fixtures show compatibility is safe.
- [ ] Add post-decode validation helpers for state conflicts.
- [ ] Update load paths to surface validation errors with actionable messages.
### Focused Test Commands
```bash
cargo test -p rustfs-ecstore rebalance_meta --lib
cargo test -p rustfs-ecstore pool_meta --lib
cargo test -p rustfs-ecstore metadata --lib
```
### Acceptance Criteria
- Unknown/corrupt metadata does not silently become an apparently valid operation state.
- Legacy metadata fixtures continue to decode through documented paths.
- Conflicting terminal state is rejected or quarantined.
### Risks
- Strict decode can block startup if old metadata contains benign extra fields. Start with tests and post-decode validation before broad strict attributes.
---
## F14: Rebalance Completion Threshold Decision
### Decision
RustFS must explicitly choose whether to match MinIO's tolerance-based rebalance completion or keep strict completion. The recommended default is to match MinIO unless RustFS has a documented reason to move more data.
### Files
- Modify: `crates/ecstore/src/rebalance.rs`
- Possibly modify: admin docs if present
- Test: rebalance goal/completion tests in `rebalance.rs`
### Design Options
1. **Match MinIO tolerance.**
- Complete when pool free-space ratio is within the MinIO-like tolerance of the goal.
- Reduces movement and operational time.
- Best for compatibility.
2. **Keep strict behavior and document it.**
- No code behavior change.
- Add tests proving strict behavior is intentional.
- Status/docs should avoid implying MinIO-compatible tolerance.
3. **Configurable tolerance.**
- Most flexible but not recommended unless operators need it.
- Adds configuration and support burden.
### Recommended Plan
Start with option 1 if compatibility is the goal. If maintainers prefer strictness, implement option 2 with explicit tests and documentation.
### Implementation Tasks
- [ ] Add tests that capture current strict behavior around the completion goal.
- [ ] Add tests for MinIO-like tolerance behavior.
- [ ] Confirm maintainers choose MinIO-compatible or strict behavior.
- [ ] Implement the selected threshold behavior.
- [ ] Update status/docs comments so completion semantics are clear.
### Focused Test Commands
```bash
cargo test -p rustfs-ecstore rebalance_goal --lib
cargo test -p rustfs-ecstore check_if_rebalance_done --lib
cargo test -p rustfs-ecstore rebalance --lib
```
### Acceptance Criteria
- Completion threshold behavior is covered by deterministic tests.
- Operators can understand whether RustFS matches MinIO tolerance.
- Empty-queue completion path remains valid.
### Risks
- Changing threshold may alter operational expectations for existing users. Document the change if behavior changes.
---
## Phase 3 Test Matrix
Run after individual fix tests pass:
```bash
cargo test -p rustfs-ecstore rebalance_meta --lib
cargo test -p rustfs-ecstore pool_meta --lib
cargo test -p rustfs-ecstore rebalance --lib
cargo test -p rustfs rebalance --lib
cargo test -p rustfs pools --lib
cargo fmt --all --check
```
If logging guardrails are relevant:
```bash
scripts/check_logging_guardrails.sh
```
Before PR:
```bash
cargo fmt --all
cargo fmt --all --check
make pre-commit
```
Clean generated build artifacts after build-based verification.
## Suggested PR Split
1. PR 1: F11 cleanup warning status.
2. PR 2: F12 structured audit fields.
3. PR 3: F13 metadata decode hardening.
4. PR 4: F14 completion threshold decision and tests.
F11 may depend on Phase 2 cleanup semantics if F10 changes retry behavior. F12 can be done anytime after Phase 1 defines distributed failure semantics.
## Open Questions Before Coding
1. What is the maximum number of cleanup warning entries to keep in persisted rebalance metadata?
2. Which actor identity is safe and useful to log for admin requests?
3. Should metadata unknown fields fail startup, warn and continue, or quarantine only the affected operation?
4. Should RustFS match MinIO's rebalance completion tolerance by default?
docs/superpowers/plans/rebalance-decommission-post-remediation-review-plan.md
-137
View File
@@ -1,137 +0,0 @@
> **Archived implementation plan/tracker** — moved from `docs/architecture/` (2026-07).
> Kept for history; not maintained. File paths inside may reflect the pre-#3929
> module layout (e.g. `crates/ecstore/src/rebalance.rs` is now
> `crates/ecstore/src/store/rebalance.rs`; `set_disk.rs` is now `set_disk/`).
# Rebalance and Decommission Post-Remediation Review
> **Status:** Updated for branch `cxymds/rebalance-decommission-remediation`.
>
> **Scope:** Rebalance, decommission, shared data-movement helpers, source cleanup,
> delete-marker handling, lifecycle expiry, restart/cancel recovery, and CI coverage.
>
> **Conclusion:** The main code-level P1/P2 remediation set has been implemented on
> this branch. The remaining release gate is verification, not another known
> correctness rewrite: full CI must pass, `make pre-commit` must pass before the
> final PR state, and the data-movement e2e proof added to CI must complete in the
> GitHub runner environment.
## Review Corrections
The previous report needed two important corrections.
First, the R28 decommission permit issue is real but was overstated. The worker
permit was released before entry migration work, so `Workers` did not account for
entry lifetime and `wk.wait()` could not prove entry work had drained. However,
the current listing path awaits entry callbacks inline and bucket processing is
bounded, so the old "unbounded object movement" wording was not supported by the
code. The fix remains correct because permit lifetime now matches entry lifetime
and prevents future spawn-based regressions.
Second, SSE-C migration is related to raw data movement but has a distinct
failure mode. Before the raw migration path, SSE-C objects did not silently
corrupt through an empty `HeaderMap`; migration reads failed while resolving
SSE-C material because the customer SSE-C headers were missing. The raw internal
read path avoids requiring request customer headers for data movement.
## Implemented Remediation
| Area | Current Branch State | Representative Guards |
| --- | --- | --- |
| Raw migration reads | Rebalance and decommission use raw data-movement read options instead of normal transformed GET semantics. | `test_rebalance_object_migration_read_opts_are_raw_data_movement`, `test_decommission_object_migration_read_opts_are_raw_data_movement` |
| Raw write invariants | Data-movement writes preserve source version, ETag, tags, expires, part metadata, and checksums consistently. Single-part raw ETag validation no longer rejects preserved source ETags. | `test_data_movement_put_object_opts_preserves_version_and_etag`, `test_data_movement_opts_preserve_tags_and_expires`, `test_data_movement_single_part_raw_reader_does_not_validate_source_etag` |
| Terminal decommission recovery | Cancel/failed/complete paths cancel in-memory workers before terminal persistence, and terminal-save failure no longer leaves ghost cancelers. | decommission canceler unit tests in `crates/ecstore/src/pools.rs` |
| Rebalance operation identity | Rebalance merge/save/stop paths are id-aware and do not let stale operation state clobber a newer operation. | rebalance id-gate and merge tests |
| Start coordination | Rebalance and decommission starts cross-check persisted peer operation state before starting conflicting work. | start cross-check tests |
| Decommission worker accounting | Entry permits are held through entry completion. R28 is no longer a pending worker-accounting bug. | decommission worker permit tests |
| Rebalance rollback | Failed start rollback finalizes metadata instead of leaving `Started` plus `stopping=true` without a worker. | rollback tests |
| Decommission progress save | Periodic progress-save errors are best-effort; terminal saves remain strict. | progress-save tests |
| Queue semantics | Multi-pool decommission is queued, local-leader prefix scheduling is supported, completed restart is rejected, completed queue prefixes can advance, failed/canceled entries block automatic promotion, failed/canceled retry preserves bucket progress, and promoted queued pools are canceled if cancellation arrives before work starts. | queue/promotion/retry tests in `pools.rs` |
| Admin status and query hardening | Pool admin status exposes queued/progress state, dangerous pool/rebalance mutation queries reject unknown or ambiguous parameters, and pool status `by-id` parsing no longer falls back to pool 0. | `admin_pool_list_item`, `pools_handler_tests`, `rebalance_handler_tests` |
| Cancel/clear operations | Non-leader cancel intent is accepted and failed/canceled terminal decommission can be explicitly cleared. | remote cancel and clear tests |
| Resume equivalence | Delete-marker replication state, tiered-object metadata, multipart part numbers, tags, expires, and version counts are covered more strictly. | data-movement equivalence tests |
| Lifecycle expiry | Data movement no longer treats failed lifecycle expiry application as a successful skip. | `resolve_data_movement_lifecycle_expiry_result_rejects_apply_failure` |
| CI coverage | Existing RustFS e2e delete-marker migration proof is wired into the `e2e-tests` CI job with `--test-threads=1`, reusing the downloaded debug binary. | `.github/workflows/ci.yml` |
## Compatibility Decisions
The current RustFS decommission contract is no longer single-pool only. Multi-pool
requests are supported as queued operations on multi-pool deployments. The
current behavior is documented in
`docs/architecture/decommission-compatibility.md`.
Delete-marker behavior is intentionally characterized rather than guessed:
- a lone delete marker without replication is cleanup-only metadata and can be
skipped;
- delete markers with replication metadata remain eligible for movement;
- rebalance and decommission share the same predicate.
Lifecycle-expired versions are also explicit:
- decommission can count safely expired versions toward source cleanup only when
lifecycle expiry application succeeds;
- rebalance remains stricter and requires actual data-movement completion for
cleanup.
## Remaining Risks and Gates
There are no remaining known P1/P2 code changes in this remediation set, but the
branch is not final-release-ready until verification completes:
1. `make pre-commit` must pass before the PR is marked ready.
2. GitHub CI must pass, including the newly wired delete-marker migration e2e
proof.
3. Local full e2e verification was attempted but the machine ran out of disk
space while building `rustfs`; this is an environment blocker, not a test
assertion failure. The `target/` directory was cleaned afterward.
4. If product requires stronger end-to-end proof for encrypted and compressed
migration, add those scenarios to `crates/e2e_test` before release. Current
branch coverage is strongest at the internal option/invariant layer, with CI
e2e coverage focused on versioning/delete-marker semantics.
## Focused Verification Run During Implementation
Focused checks were run per task, including:
```bash
cargo test -p rustfs-ecstore data_movement_single_part_raw_reader --lib
cargo test -p rustfs-ecstore test_merge_rebalance_meta_preserves_stopping_stop_snapshot --lib
cargo test -p rustfs-ecstore test_merge_rebalance_pool_stats_clears_stopping_for_terminal_status --lib
cargo test -p rustfs-ecstore first_resumable_decommission_queue_indices --lib
cargo test -p rustfs-ecstore test_return_resumable_pools_skips_failed_decommission --lib
cargo test -p rustfs-ecstore resolve_data_movement_lifecycle_expiry_result --lib
cargo test -p rustfs-ecstore lifecycle_action_removes_data_movement_version --lib
cargo test -p rustfs-ecstore test_pool_meta_promoted_queued_decommission_can_be_canceled --lib
cargo test -p rustfs admin_pool_list_item --lib
cargo test -p rustfs pools_handler_tests --lib
cargo test -p rustfs rebalance_handler_tests --lib
cargo test -p rustfs admin_query_pool_status_by_id --lib
cargo fmt --all --check
```
The attempted local e2e command was:
```bash
cargo test -p e2e_test delete_marker_migration_semantics -- --nocapture
```
It failed because the local filesystem reached `No space left on device` while
building the RustFS debug binary. CI now runs the proof in the e2e job after the
debug binary artifact has been downloaded, and uses `--test-threads=1` to avoid
parallel test-server startup for that proof.
## Final PR Gate
Before marking the draft PR ready:
```bash
cargo fmt --all
cargo fmt --all --check
make pre-commit
```
If CI reports failures after the branch is pushed, inspect the failing job logs
instead of weakening the e2e gate. The delete-marker migration proof was added
because skipping it would leave the exact versioning regression class invisible
to CI.
docs/superpowers/plans/rebalance-decommission-remediation-plan.md
-551
View File
@@ -1,551 +0,0 @@
> **Archived implementation plan/tracker** — moved from `docs/architecture/` (2026-07).
> Kept for history; not maintained. File paths inside may reflect the pre-#3929
> module layout (e.g. `crates/ecstore/src/rebalance.rs` is now
> `crates/ecstore/src/store/rebalance.rs`; `set_disk.rs` is now `set_disk/`).
# Rebalance and Decommission Remediation Plan
> This document turns `docs/architecture/expert-review-analysis.md` into an actionable remediation backlog. It is intentionally split into independent fix blocks so each item can be analyzed, assigned, implemented, and verified separately.
## Scope
This plan covers the confirmed and calibrated issues from the rebalance/decommission expert review:
- Critical and high-risk data integrity issues.
- Distributed state propagation and recovery semantics.
- Data movement resource usage and metadata preservation.
- Operational visibility, auditability, and hardening gaps.
This file is not a code patch. Each block below should be expanded into a focused implementation plan before code changes begin.
## Priority Order
| Order | Fix ID | Source | Priority | Main Risk |
| --- | --- | --- | --- | --- |
| 1 | F01 | P1.1 | Critical | Rebalance may drop delete marker or remote tiered metadata |
| 2 | F02 | P1.5 | High | Decommission may treat unsafe overwrite as cleanup-safe |
| 3 | F03 | P1.4 | High | Retiring pool may remain writable on stale peers |
| 4 | F04 | P1.2 | High | Rebalance and decommission may both resume after restart |
| 5 | F05 | P1.3, A4 | High | Admin start/stop may report success for partial cluster state |
| 6 | F06 | P2.1 | High | Multipart migration can allocate whole parts in memory |
| 7 | F07 | P2.2, A3 | Medium | Migrated objects may lose checksum or other metadata semantics |
| 8 | F08 | P2.4 | Medium | Decommission leaves lifecycle-expired source entries behind |
| 9 | F09 | P2.3 | Medium | Rebalance overwrite races may fail instead of converging |
| 10 | F10 | A2 | Medium | Source cleanup lacks a final version-set guard |
| 11 | F11 | A1 | Medium | Rebalance cleanup failures are completion warnings only |
| 12 | F12 | P3.2 | Low | Admin operations are not sufficiently auditable |
| 13 | F13 | P3.3 | Low | Persisted metadata accepts unknown or inconsistent fields |
| 14 | F14 | P3.4 | Low | Rebalance completion threshold differs from MinIO |
## Phase 1: Data Integrity and Safety
### F01: Fix rebalance delete marker and remote tiered version migration
**Source:** P1.1
**Decision:** Confirmed fix required.
**Target priority:** Critical.
**Problem:** `rebalance_entry` marks delete markers and remote tiered versions as moved, but those branches operate through the source `SetDisks` path rather than a confirmed cross-pool target write. Source cleanup can then remove the only metadata that preserves tombstone or tiered-object semantics.
**Primary files:**
- `crates/ecstore/src/rebalance.rs`
- `crates/ecstore/src/data_movement.rs`
- `crates/ecstore/src/set_disk.rs`
- Tests near existing rebalance migration tests in `crates/ecstore/src/rebalance.rs`
**Preferred fix direction:**
- Route rebalance delete marker movement through an `ECStore`-level data movement path that chooses a non-source target pool and writes the exact version metadata to that target.
- For remote tiered versions, either:
- align with MinIO and skip tiered versions during rebalance, or
- implement a target-pool metadata move that preserves the remote tier pointer and lifecycle state.
- Do not count the version as rebalanced until the target metadata is confirmed.
**Acceptance criteria:**
- A versioned object with a delete marker remains deleted after rebalance source cleanup.
- A remote tiered version remains readable or correctly listed after rebalance source cleanup.
- Rebalance does not mark delete marker or tiered versions as complete when target metadata was not written.
**Required tests:**
- Multi-pool rebalance test for a versioned object with a delete marker.
- Multi-pool rebalance test for a remote tiered version.
- Failure test where target metadata write fails and source cleanup must not happen.
**Dependencies:** None. This is the first safety fix.
---
### F02: Stop treating `DataMovementOverwriteErr` as cleanup-safe in decommission
**Source:** P1.5
**Decision:** Confirmed fix required.
**Target priority:** High.
**Problem:** Decommission treats `DataMovementOverwriteErr` like object-not-found/version-not-found and sets `cleanup_ignored = true`. That error only means source and destination pool are the same; it does not prove an equivalent target version exists.
**Primary files:**
- `crates/ecstore/src/pools.rs`
- `crates/ecstore/src/data_movement.rs`
- `crates/ecstore/src/store/object.rs`
**Preferred fix direction:**
- Remove `DataMovementOverwriteErr` from cleanup-safe branches for delete marker and remote tiered decommission paths.
- If overwrite occurs because an equivalent target version already exists, perform an explicit equivalence check before counting the version complete.
- Otherwise return a migration failure and keep the source entry.
**Acceptance criteria:**
- `DataMovementOverwriteErr` does not increment the decommissioned count unless equivalence is proven.
- Source cleanup is blocked when target equivalence is unknown.
- Logs/status clearly state whether overwrite was equivalent-complete or unsafe.
**Required tests:**
- Delete marker decommission returns `DataMovementOverwriteErr` and does not clean source.
- Remote tiered decommission returns `DataMovementOverwriteErr` and does not clean source.
- Equivalent target version exists; overwrite can be counted complete only after metadata equality check passes.
**Dependencies:** F01 clarifies remote/tombstone movement semantics, but F02 can be implemented independently for decommission.
---
### F03: Make decommission `pool_meta` reload a cluster-wide start barrier
**Source:** P1.4
**Decision:** Confirmed fix required.
**Target priority:** High.
**Problem:** `start_decommission` saves pool meta and then calls peer reload. Reload failure is logged but the operation still returns success, leaving stale peers able to write into the retiring pool.
**Primary files:**
- `crates/ecstore/src/pools.rs`
- `crates/ecstore/src/notification_sys.rs`
- `crates/ecstore/src/rpc/peer_rest_client.rs`
- `rustfs/src/admin/handlers/pools.rs`
- `rustfs/src/storage/rpc/node_service.rs`
**Preferred fix direction:**
- Treat reload failure as a failed start unless the system has an explicit partial/degraded operation state.
- Return peer failure information to the admin handler.
- Do not spawn or resume decommission workers until required peers acknowledge the updated pool meta.
**Acceptance criteria:**
- Admin decommission start fails or returns a clearly degraded response when peer reload fails.
- No peer can select the retiring pool for new writes after decommission start is reported successful.
- Status exposes peer reload failure details.
**Required tests:**
- Unit test for `start_decommission` with mocked `reload_pool_meta` failure.
- RPC/admin test showing start does not return plain success on peer failure.
- Write-placement test verifying retiring pools are skipped only after successful cluster reload.
**Dependencies:** None.
---
### F04: Reorder store init recovery to restore pool meta before rebalance
**Source:** P1.2
**Decision:** Confirmed fix required.
**Target priority:** High.
**Problem:** Store init loads and starts rebalance before loading persisted pool meta. If unfinished decommission exists on disk, `start_rebalance` can miss it and both processes may resume.
**Primary files:**
- `crates/ecstore/src/store/init.rs`
- `crates/ecstore/src/rebalance.rs`
- `crates/ecstore/src/pools.rs`
**Preferred fix direction:**
- Load and install `PoolMeta` before attempting to start rebalance.
- After pool meta is loaded, explicitly decide:
- if decommission is active, do not auto-start rebalance;
- if rebalance metadata exists but conflicts with decommission, mark rebalance as blocked/deferred and expose it in status.
- Keep existing decommission resume delay, but ensure rebalance is not already running when decommission resumes.
**Acceptance criteria:**
- Restart with active decommission metadata and rebalance metadata does not run both workers.
- Startup status explains which operation was deferred and why.
- Existing startup without decommission still resumes rebalance normally.
**Required tests:**
- Init test with persisted active decommission plus started rebalance metadata.
- Init test with only rebalance metadata still resumes rebalance.
- Init test with corrupt or missing pool meta follows existing error policy.
**Dependencies:** None, but coordinate with F05 status semantics.
---
### F05: Make rebalance start/stop distributed semantics explicit and fail-safe
**Source:** P1.3, A4
**Decision:** Confirmed fix required.
**Target priority:** High.
**Problem:** Rebalance start can return success even when peer propagation fails or peer background start later fails. Stop can return success while peer stop failed or local stop errors were ignored. Stop is also asynchronous, but admin semantics do not clearly expose `stopping` versus `stopped`.
**Primary files:**
- `rustfs/src/admin/handlers/rebalance.rs`
- `rustfs/src/storage/rpc/node_service.rs`
- `crates/ecstore/src/notification_sys.rs`
- `crates/ecstore/src/rpc/peer_rest_client.rs`
- `crates/ecstore/src/rebalance.rs`
**Preferred fix direction:**
- For start:
- propagate rebalance metadata to peers before returning success;
- have peer RPC synchronously validate/start or return a structured failure;
- on partial failure, rollback local start or report a degraded state.
- For stop:
- never ignore `store.stop_rebalance()` errors;
- aggregate peer stop failures and return them to admin;
- expose `stop_requested`, `stopping`, and `stopped` in status if worker shutdown remains asynchronous.
**Acceptance criteria:**
- Admin start does not return plain success when any required peer fails to load/start.
- Admin stop does not return plain success when any required peer fails to stop.
- Status can distinguish a requested stop from all workers fully stopped.
**Required tests:**
- Peer `load_rebalance_meta(true)` failure.
- Peer background `start_rebalance` failure.
- Peer `stop_rebalance` local save failure.
- Stop requested while a worker is in a long migration operation.
**Dependencies:** Coordinate with F04 so startup and admin semantics use the same conflict model.
---
## Phase 2: Data Movement Correctness and Resource Control
### F06: Stream multipart data movement instead of buffering whole parts
**Source:** P2.1
**Decision:** Confirmed fix required.
**Target priority:** High.
**Problem:** Multipart migration allocates `Vec<u8>` for each part and reads the whole part into memory. Large parts plus concurrent workers can OOM the process.
**Primary files:**
- `crates/ecstore/src/data_movement.rs`
- `crates/ecstore/src/store/multipart.rs`
- `crates/ecstore/src/store/object.rs`
**Preferred fix direction:**
- Replace whole-part buffering with bounded streaming.
- Preserve the current part index and checksum/etag behavior through the streaming reader.
- Ensure multipart abort still runs on any failed part or complete call.
**Acceptance criteria:**
- Migrating a large multipart object does not allocate memory proportional to part size.
- Part ETag, size, actual size, and index handling remain correct.
- Failed migration aborts the temporary multipart upload.
**Required tests:**
- Fake large multipart reader proving memory is bounded.
- Multipart migration preserves part order and ETags.
- Failure during part upload aborts the multipart upload.
**Dependencies:** Coordinate with F07 for checksum preservation.
---
### F07: Preserve and verify full data movement metadata
**Source:** P2.2, A3
**Decision:** Confirmed fix required for checksum; additional metadata requires verification and likely fixes.
**Target priority:** Medium.
**Problem:** Data movement preserves basic metadata but does not fully prove checksum, multipart checksum, replication state, version purge status, or object-lock metadata equivalence after migration.
**Primary files:**
- `crates/ecstore/src/data_movement.rs`
- `crates/ecstore/src/store_api/types.rs`
- `crates/ecstore/src/set_disk.rs`
- `crates/filemeta/src/fileinfo.rs`
- `crates/filemeta/src/replication.rs`
**Preferred fix direction:**
- For multipart migration, populate `CompletePart` checksum fields when source part checksum exists.
- Preserve or reconstruct object-level checksum metadata.
- Add migration equivalence checks for:
- ETag;
- checksum and multipart checksum;
- replication status and version purge status;
- object lock retention mode/date and legal hold;
- version ID and mod time.
- Only change production metadata handling after tests show a real loss path.
**Acceptance criteria:**
- Migrated object metadata matches source for the fields above.
- `GetObjectAttributes` checksum behavior remains correct after migration.
- Object lock retention/legal hold remains unchanged across migration.
**Required tests:**
- Single-part object with checksum metadata.
- Multipart object with per-part checksum metadata.
- Object with replication state and version purge state.
- Object with governance/compliance retention and legal hold.
**Dependencies:** F06 should land first or in the same PR if checksum streaming needs shared reader changes.
---
### F08: Align decommission lifecycle-expired version cleanup semantics
**Source:** P2.4
**Decision:** Confirmed fix required or explicit design decision required.
**Target priority:** Medium.
**Problem:** Decommission skips lifecycle-expired versions but does not clean the source entry if any expired version exists. MinIO counts expired versions as completed and can clean the source entry.
**Primary files:**
- `crates/ecstore/src/pools.rs`
- Lifecycle evaluator code under `crates/ecstore/src/bucket/lifecycle/`
**Preferred fix direction:**
- Treat lifecycle-expired versions as completed for decommission source cleanup when they are not required to remain accessible.
- Keep lifecycle/object-lock/replication checks in place before treating a version as expired.
- If RustFS intentionally keeps expired source versions, expose that state in status and exclude the pool from being considered fully clean.
**Acceptance criteria:**
- Decommission can clean a source entry when all remaining versions are either migrated or lifecycle-expired.
- Object-lock or replication-protected versions are not wrongly treated as cleanup-safe.
- Status accurately reflects residual expired-source entries if they are intentionally retained.
**Required tests:**
- Bucket with one migrated version plus one lifecycle-expired version.
- Expired version protected by object lock must not be cleaned.
- Replication-pending version must not be treated as cleanup-safe.
**Dependencies:** Coordinate with F07 metadata tests for object lock and replication.
---
### F09: Handle rebalance overwrite races using explicit equivalence checks
**Source:** P2.3
**Decision:** Fix likely required; first confirm exact race cases with tests.
**Target priority:** Medium.
**Problem:** RustFS treats `DataMovementOverwriteErr` as non-transient during rebalance, while MinIO ignores acceptable overwrite races. A strict failure can prevent rebalance convergence when the target already has an equivalent version.
**Primary files:**
- `crates/ecstore/src/rebalance.rs`
- `crates/ecstore/src/data_movement.rs`
- `crates/ecstore/src/store/object.rs`
**Preferred fix direction:**
- Keep unsafe source-equals-target cases as failures.
- If the target version already exists, compare version ID, ETag, size, mod time, checksum, and key metadata.
- Count as complete only when equivalence passes.
- Add last-error/status text when overwrite is unsafe.
**Acceptance criteria:**
- Equivalent target version allows rebalance to continue.
- Non-equivalent target version fails and does not clean source.
- Behavior is documented as compatible with MinIO's acceptable overwrite race handling.
**Required tests:**
- Rebalance overwrite with equivalent target version.
- Rebalance overwrite with mismatched checksum or metadata.
- Rebalance overwrite where source and target pool are the same and no equivalent target exists.
**Dependencies:** F07 defines the metadata equivalence fields.
---
### F10: Add source cleanup preflight verification before deleting migrated entries
**Source:** A2
**Decision:** Fix recommended as defense-in-depth.
**Target priority:** Medium.
**Problem:** After versions are migrated, source cleanup deletes the source prefix based on the version list read before migration. A final verification would protect against state propagation failures, recovery races, or repair processes changing metadata before cleanup.
**Primary files:**
- `crates/ecstore/src/rebalance.rs`
- `crates/ecstore/src/pools.rs`
- `crates/ecstore/src/set_disk.rs`
**Preferred fix direction:**
- Before source cleanup, re-read source metadata or a generation marker.
- Verify the version set scheduled for deletion equals the version set that was migrated or intentionally ignored.
- If the version set changed, defer/retry the entry instead of deleting the prefix.
**Acceptance criteria:**
- Cleanup does not delete versions created or discovered after the migration scan.
- Changed source metadata causes a retry/defer state with a clear status error.
- The guard applies to rebalance and decommission where source prefix deletion is used.
**Required tests:**
- Simulate source metadata changing between migration and cleanup.
- Simulate peer state mismatch where a write lands on a rebalancing/decommissioning pool.
- Confirm unchanged metadata still allows cleanup.
**Dependencies:** Works best after F03 and F05 reduce state propagation failures.
---
## Phase 3: Cleanup Semantics, Observability, and Hardening
### F11: Improve rebalance cleanup failure handling and reporting
**Source:** A1
**Decision:** Fix recommended.
**Target priority:** Medium.
**Problem:** Rebalance source cleanup failure is converted to a warning and the task can still complete. Only a count and last warning are preserved.
**Primary files:**
- `crates/ecstore/src/rebalance.rs`
- `rustfs/src/admin/handlers/rebalance.rs`
- Rebalance status DTOs and serialization paths
**Preferred fix direction:**
- Preserve a bounded list of cleanup failures, not only the last one.
- Add a clear terminal or partial terminal state when cleanup warnings exist.
- Decide whether cleanup failure should:
- defer the entry for retry, or
- allow completion but mark the pool as `completed_with_cleanup_warnings`.
**Acceptance criteria:**
- Admin status reports cleanup warning count and representative object list.
- Operators can identify which objects need manual cleanup or retry.
- Rebalance terminal state is not misleading when source data remains.
**Required tests:**
- Multiple cleanup failures preserve count and bounded object details.
- Status response includes cleanup warning data.
- Completion state reflects cleanup warnings.
**Dependencies:** Coordinate with F10 if cleanup failures become retryable.
---
### F12: Add structured audit fields for rebalance and decommission operations
**Source:** P3.2
**Decision:** Fix recommended.
**Target priority:** Low.
**Problem:** Critical admin operations do not consistently log actor, remote address, request ID, pool indices, peer host, result, and partial failure state.
**Primary files:**
- `rustfs/src/admin/handlers/rebalance.rs`
- `rustfs/src/admin/handlers/pools.rs`
- `crates/ecstore/src/notification_sys.rs`
- `crates/ecstore/src/rpc/peer_rest_client.rs`
- Logging guardrail scripts if applicable
**Preferred fix direction:**
- Standardize structured fields for start, stop, cancel, status-affecting propagation, and peer RPC failures.
- Mask sensitive actor material.
- Use warn/error for partial failure rather than info-only logs.
**Acceptance criteria:**
- Start/stop/cancel logs include actor, request ID, pool/rebalance ID, peer, result, and error when present.
- Logs do not expose secrets.
- Partial failures are searchable by stable event fields.
**Required tests:**
- Log-capture tests for success, rejection, and partial failure.
- Guardrail check if logging conventions are enforced by script.
**Dependencies:** F03 and F05 define partial-failure semantics.
---
### F13: Harden persisted rebalance and pool metadata decoding
**Source:** P3.3
**Decision:** Fix recommended after compatibility review.
**Target priority:** Low.
**Problem:** Persisted rebalance and pool metadata can accept unknown fields silently. This may be acceptable for compatibility, but it weakens corruption and typo detection.
**Primary files:**
- `crates/ecstore/src/rebalance.rs`
- `crates/ecstore/src/pools.rs`
- Metadata fixture tests near existing metadata tests
**Preferred fix direction:**
- For formats that can be strict, add `deny_unknown_fields`.
- Where backward compatibility requires leniency, detect and warn on unknown fields during legacy decode.
- Add explicit validation for conflicting terminal states.
**Acceptance criteria:**
- Unknown fields either fail decode or produce a warning through a documented compatibility path.
- Missing critical fields fail safely.
- Conflicting states such as completed plus running are rejected.
**Required tests:**
- Unknown field fixture.
- Missing critical field fixture.
- Conflicting terminal state fixture.
- Legacy fixture proving backward compatibility.
**Dependencies:** None.
---
### F14: Decide and document rebalance completion threshold behavior
**Source:** P3.4
**Decision:** Design decision required.
**Target priority:** Low.
**Problem:** RustFS uses a stricter rebalance completion goal than MinIO's tolerance-based behavior. This may increase movement work and operational time.
**Primary files:**
- `crates/ecstore/src/rebalance.rs`
- Admin status documentation if present
- Tests around rebalance goal calculation
**Preferred fix direction:**
- Decide whether RustFS should:
- match MinIO tolerance;
- keep strict behavior and document it;
- make tolerance configurable with a conservative default.
- Avoid configurability unless a real operational requirement exists.
**Acceptance criteria:**
- Completion behavior is intentional and covered by tests.
- Status/ETA reporting does not imply MinIO-compatible tolerance if strict behavior remains.
- Large-pool behavior is covered by a deterministic unit test.
**Required tests:**
- Rebalance goal just below target.
- Rebalance goal within MinIO-like tolerance.
- Empty queue completion path remains valid.
**Dependencies:** None.
---
## Cross-Cutting Test Matrix
Each fix should add focused tests, but the following end-to-end scenarios should also be covered before enabling production use:
1. Rebalance a versioned object with normal versions, delete marker, and source cleanup.
2. Rebalance a remote tiered version or verify it is intentionally skipped.
3. Decommission a pool while peer reload fails.
4. Restart with both active decommission metadata and rebalance metadata.
5. Stop rebalance while a large multipart migration is in progress.
6. Migrate a large multipart object without memory proportional to part size.
7. Migrate objects with checksum, replication state, retention, and legal hold metadata.
8. Trigger source cleanup failure and verify admin status exposes it.
9. Trigger overwrite race with equivalent and non-equivalent target versions.
10. Decommission lifecycle-expired versions with object-lock and replication constraints.
## Suggested Execution Strategy
1. Start with F01 and F02 because they protect object version semantics.
2. Implement F03, F04, and F05 before broader rollout because they define distributed operation safety.
3. Implement F06 before running large-scale data movement tests.
4. Implement F07 through F11 as compatibility, convergence, and observability hardening.
5. Keep F12 through F14 as final hardening unless release criteria require them earlier.
Each fix should be developed in a separate PR unless two adjacent fixes share the same core implementation. In particular:
- F01 and F09 should not be merged together unless the equivalence helper is clearly isolated.
- F06 and F07 may be combined only if streaming part migration and checksum preservation share the same reader changes.
- F03 and F05 may share peer failure aggregation utilities, but their admin behavior should be tested separately.
docs/superpowers/plans/scheduler-baseline.md
-81
View File
@@ -1,81 +0,0 @@
> **Archived migration snapshot** — moved from `docs/architecture/` (2026-07)
> when the architecture-review ledger it fed closed out. Kept for history; not
> maintained.
# Scheduler Baseline Inventory
This inventory covers `G-011` for `rustfs/backlog#675`. It is a docs-only
snapshot of the current scheduling, backpressure, worker, scanner, heal, and
runtime-builder ownership. It does not define new behavior.
## Current Owners
| Surface | Current owner | Current responsibility | Migration boundary |
|---|---|---|---|
| `ConcurrencyManager` | `rustfs/src/storage/concurrency/manager.rs` | Owns the RustFS S3 read-path disk-read semaphore, I/O metrics, priority queue, storage media detection, access-pattern detection, and buffer strategy. | Keep request admission and I/O metrics behavior stable until a controller can consume the same state explicitly. |
| I/O scheduler core | `crates/io-core/src/scheduler.rs` | Owns the reusable buffer-size and priority algorithms consumed by the RustFS S3 read path (`rustfs/src/storage/concurrency/io_schedule.rs`). The former `SchedulerManager` facade in `rustfs-concurrency` was removed as zero-caller dead code (backlog#1025). | Treat `rustfs-io-core` as the reusable algorithm surface; the RustFS S3 read path owns its own scheduling wiring. |
| RustFS backpressure monitor | `rustfs/src/storage/backpressure.rs` | Tracks object-pipe watermark state used by RustFS storage backpressure tests and helpers, using the shared `PipeBackpressurePolicy` from `crates/concurrency/src/backpressure.rs`. The former `BackpressureManager`/`BackpressurePipe` facade in `rustfs-concurrency` was removed as zero-caller dead code (backlog#1025). | Preserve current state labels and watermark semantics; keep pipe sizing and watermark policy separate from object-read disk semaphore admission. |
| `Workers` | `crates/concurrency/src/workers.rs` | Provides cooperative worker-slot admission with `take`, `give`, and `wait`; current background workflows use it for bounded set workers. | Preserve blocking/wakeup semantics and over-release clamping. |
| Scanner cycle budget | `crates/scanner/src/scanner_budget.rs` | Cancels a child token when runtime, object-count, or directory-count budget is reached. | Preserve partial-cycle reason mapping and checkpoint accounting. |
| Heal admission | `crates/heal/src/heal/manager.rs`, `crates/heal/src/heal/channel.rs`, `rustfs_common::heal_channel` | Owns priority queue admission, duplicate merge/drop/full results, active-task tracking, retry admission, and channel responses. | Preserve low-priority scanner behavior and high-priority escalation gates. |
| Tokio runtime builder | `rustfs/src/server/runtime.rs` | Builds the multi-thread runtime from env/defaults, sets thread counts, stack, queue/event intervals, I/O event cap, thread name, and optional dial9 tracing. | Keep runtime defaults and env names stable when later startup phases move ownership. |
## Current Flow
```mermaid
flowchart TD
http["HTTP/S3 request"] --> app["app object usecase"]
app --> guard["ConcurrencyManager::track_request"]
app --> permit["disk-read semaphore permit"]
permit --> strategy["I/O queue status and buffer strategy"]
strategy --> ecstore["ECStore object/read path"]
ecstore --> setdisks["hashed set disks"]
scanner["scanner cycle"] --> budget["ScannerCycleBudget"]
budget --> folder["folder/object scan"]
folder --> healreq["heal channel request"]
healreq --> admission["HealManager admission queue"]
admission --> healworker["heal workers and retries"]
startup["startup entrypoint"] --> runtime["Tokio runtime builder"]
runtime --> services["background services"]
```
## Missing State For Later Work
`R-015` storage foundation:
- Needs a stable inventory of endpoint publication, local disk prewarm, lock
client setup, and per-set readiness state before any scheduler/controller
consumes storage topology.
- Must not infer set availability only from request-path I/O metrics.
`E-011` extension/runtime consumers:
- Need explicit ownership for runtime admission snapshots before extensions can
observe scheduler or backpressure state.
- Must not receive mutable handles to `ConcurrencyManager`, heal queues, or
scanner budget tokens.
`C-011` controller work:
- Needs desired/current/status snapshots for request admission, scanner budget,
and heal queue pressure before any controller can reconcile them.
- Must keep worker mutation explicit. Read-only status should report `None` or
no-op mutation until a reviewed worker lifecycle PR exists.
## Preservation Invariants
- Request reads must keep the same disk-read semaphore admission and active GET
accounting.
- I/O queue status and congestion metrics must remain derived from the same
permit counts.
- Scanner budget cancellation must keep its reason as runtime, objects, or
directories.
- Scanner inline-heal compatibility must continue to use asynchronous heal
admission.
- Heal duplicate admission must prefer merge semantics before full-queue
rejection.
- High-priority heal admission must still be able to displace lower-priority
queued work where the current manager allows it.
- Tokio runtime env names and fallback defaults must remain unchanged.
docs/superpowers/plans/site-replication-hardening-pr.md
-105
View File
@@ -1,105 +0,0 @@
> **Archived implementation plan/tracker** — moved from `docs/architecture/` (2026-07).
> Kept for history; not maintained. File paths inside may reflect the pre-#3929
> module layout (e.g. `crates/ecstore/src/rebalance.rs` is now
> `crates/ecstore/src/store/rebalance.rs`; `set_disk.rs` is now `set_disk/`).
## Related Issues
N/A
## Summary of Changes
This PR hardens the site replication control plane in small, reviewable commits. The work starts with guardrails identified during the MinIO comparison and multi-review pass: credential redaction, stricter diagnostic authorization, remove cleanup, and clearer status diagnostics.
### Implementation Log
1. `docs: plan site replication hardening`
- Purpose: establish the execution plan and PR draft before code changes.
- Reason: the site replication follow-up touches security, compatibility, operations, and tests, so reviewers need a durable record of each step and why it exists.
2. `fix: redact site replication target secrets`
- Purpose: prevent bucket replication target credentials from leaking through admin listing, bucket metadata export, and debug formatting.
- Reason: site replication stores the `site-replicator-0` secret in bucket targets today; keeping the internal storage format intact while redacting external surfaces reduces immediate blast radius without changing replication target loading.
3. `fix: require operation access for replication diagnostics`
- Purpose: require operation-level admin authorization for site replication diagnostic POST endpoints and clamp netperf duration.
- Reason: `devnull` and `netperf` read request bodies and exercise diagnostic work, so granting them through read-only site replication info permission was broader than intended.
4. `fix: clean site replication targets on remove`
- Purpose: remove bucket targets and `site-repl-*` replication rules that point at a removed deployment before completing pending remove.
- Reason: removing a site only from site replication state leaves bucket-level replication configuration behind, so object replication can continue attempting to reach a removed peer.
5. `feat: expose site replication status diagnostics`
- Purpose: add machine-readable peer fetch errors and pending operation progress to site replication status.
- Reason: operators previously saw only `Unknown` sync state or logs when peer metainfo fetches failed or remove/rotation was pending, which made automation and troubleshooting unnecessarily opaque.
6. `docs: expand site replication hardening scope`
- Purpose: keep the single-PR plan aligned with the expanded request to include MinIO compatibility, add preflight, bootstrap, lifecycle compatibility, retry, and repair work.
- Reason: the remaining work is larger than the first hardening pass, so reviewers need to see the intended sequence before more behavior changes land.
7. `fix: align site replication peer paths with minio`
- Purpose: send peer site-replication calls over MinIO-compatible admin paths and accept the MinIO-style peer join route.
- Reason: RustFS already accepts `/minio/admin` as an alias, so using MinIO wire paths improves mixed-cluster compatibility without removing RustFS route support.
8. `fix: validate site replication add topology`
- Purpose: preflight add requests with remote metainfo and IDP settings before creating service accounts, joining peers, or persisting state.
- Reason: MinIO rejects unsafe add topologies up front; RustFS now rejects duplicate deployment IDs, missing local deployment, IDP mismatch, multiple non-empty initial sites, and peers already configured with a different site-replication set.
9. `feat: bootstrap site replication metadata on add`
- Purpose: replay the local site replication snapshot to joined peers during add, before object backfill starts.
- Reason: existing hooks only replicate changes after site replication is enabled; existing policies, users, groups, bucket metadata, and buckets must be bootstrapped so a newly joined site does not start with an incomplete control-plane snapshot.
10. `fix: align lifecycle replication with minio semantics`
- Purpose: replicate bucket lifecycle metadata only when site replication has `replicate-ilm-expiry` enabled.
- Reason: MinIO keeps lifecycle expiry replication opt-in; RustFS should not replicate lifecycle metadata by default during live bucket metadata hooks or add-time bootstrap.
11. `feat: add site replication retry and repair MVP`
- Purpose: persist failed peer replication attempts as retry metadata, expose retry counts in status, and add an operation-level repair endpoint that replays the current local snapshot.
- Reason: transient peer failures were previously only visible through logs and required manual reconstruction; a durable, secret-safe queue plus repair replay gives operators a concrete recovery path without persisting sensitive request payloads.
12. `fix: satisfy site replication pre-pr checks`
- Purpose: remove a redundant MinIO admin-path branch and construct retry test fixtures directly.
- Reason: the full pre-PR gate enforces clippy warnings as errors; this keeps the final branch green without changing site replication behavior.
## Verification
Baseline started from `origin/main` at `758677da`, then was rebased onto the latest `origin/main` at `1d6a8259`.
- Passed: `cargo fmt --all --check`
- Passed: `cargo test -p rustfs-ecstore bucket_target --lib`
- Passed: `cargo test -p rustfs-madmin site_replication --lib`
- Passed: `cargo test -p rustfs route_policy --lib`
- Passed: `cargo test -p rustfs site_replication --lib`
- Passed: `cargo test -p rustfs route_policy --lib`
- Passed: `cargo test -p rustfs site_replication --lib`
- Passed: `cargo fmt --all`
- Passed: `cargo fmt --all --check`
- Passed: `cargo test -p rustfs site_replication --lib`
- Passed: `cargo fmt --all --check`
- Passed: `cargo test -p rustfs-madmin site_replication --lib`
- Passed: `cargo test -p rustfs route_policy --lib`
- Passed: `cargo test -p rustfs route_registration_test --lib`
- Passed: `cargo test -p rustfs site_replication --lib`
- Passed: `cargo fmt --all --check`
- Passed: `cargo test -p rustfs site_replication --lib`
- Passed: `cargo fmt --all --check`
- Passed: `cargo test -p rustfs site_replication --lib`
- Passed before the final main rebase: `make pre-pr`
- Not completed after rebasing onto `1d6a8259`: `make pre-pr` was interrupted during clippy at request time; formatting, unsafe-code, architecture, and logging guardrails had already passed in that run.
## Impact
The planned implementation is intended to reduce credential exposure, require stronger permission for diagnostic write-like endpoints, clean removed site replication targets, and make replication status more actionable. Compatibility-sensitive behavior is called out per commit in the implementation log.
The credential redaction step changes admin/export visibility of bucket target secrets. Stored target configuration is not migrated or reformatted by this PR step.
The diagnostic authorization step intentionally changes required permission for `POST /site-replication/devnull` and `POST /site-replication/netperf` from `SiteReplicationInfoAction` to `SiteReplicationOperationAction`.
The remove cleanup step preserves user-managed replication rules and non-replication targets, and only prunes targets/rules associated with removed site replication deployment IDs.
The status diagnostics step adds optional `PeerErrors` and `PendingOperation` fields to the status response. Existing clients can ignore them, while automation can use them to distinguish peer reachability/auth failures from content mismatch.
## Additional Notes
This draft PR includes bootstrap, durable retry, and repair work in one reviewable branch. It should remain draft until CI or a full local pre-PR run confirms the latest default branch baseline.
The expanded single-PR scope also includes MinIO wire-contract bootstrap validation, durable site-replication retry, full add-time IAM/bootstrap sync, lifecycle compatibility, and site-level repair.
The peer path compatibility step maps outbound peer requests to `/minio/admin/v3/site-replication/...`. Peer join uses the encrypted MinIO payload contract, while internal peer metadata/IAM/remove/edit requests remain plain JSON to match MinIO handlers.
The add preflight step performs remote reads before state mutation, so add fails earlier when a peer cannot report metainfo or IDP settings.
The bootstrap step replays IAM policies, built-in users with stored secret material, group membership/status, policy mappings, bucket creation, and bucket metadata through the existing peer replication handlers. It does not synthesize ordinary service-account secrets when the snapshot does not contain them; the site-replicator service account remains distributed through the join flow.
Lifecycle metadata is now skipped by default in site replication hooks and bootstrap snapshots unless the site replication peer state enables `replicate-ilm-expiry`.
The retry MVP intentionally stores peer/path/error/count metadata only. Repair regenerates payloads from the current local snapshot, avoiding persistent storage of user secrets, service-account secrets, or bucket target credentials in the retry queue.
docs/superpowers/plans/startup-timeline.md
-90
View File
@@ -1,90 +0,0 @@
> **Archived migration snapshot** — moved from `docs/architecture/` (2026-07)
> when the architecture-review ledger it fed closed out. Kept for history; not
> maintained.
# Startup Timeline Baseline
This document records the current binary startup order before runtime/lifecycle
migration work. It is a behavior-preservation baseline only; it does not define
new startup semantics.
## Scope
- Baseline commit: `ae9d25879d72bc8977f08e61062c022e2142483b`
- Entry points covered: `rustfs/src/main.rs::main`,
`rustfs/src/startup_entrypoint.rs::{run_process, async_main, run}`, and
startup lifecycle helpers
- Related migration task: `G-007`
- Out of scope for this baseline: embedded startup, admin route-action matrix,
and any runtime/lifecycle code movement
## Startup Stages
| Step | Source | Current action | Side effects | Fatal boundary | Ready stage |
|---|---|---|---|---|---|
| `BOOT-001` | `rustfs/src/startup_entrypoint.rs` | Apply external-prefix environment compatibility during async startup before command parsing. | Copies supported external env aliases into canonical `RUSTFS_*` process env keys and prints warnings or info to stderr. | Non-fatal; failure is logged to stderr and startup continues. | None |
| `BOOT-002` | `rustfs/src/main.rs` and `rustfs/src/startup_entrypoint.rs` | Call the startup entrypoint and build the Tokio runtime. | Installs runtime configuration and any runtime telemetry guard created by the runtime builder. | Fatal through `expect`; process exits if the runtime cannot be built. | None |
| `BOOT-003` | `rustfs/src/startup_entrypoint.rs` | Parse CLI command and dispatch non-server commands. | `info` and `tls` commands execute and return without server startup. | Command parse exits process with code 1; TLS command errors propagate. | None |
| `BOOT-004` | `rustfs/src/startup_preflight.rs` | Initialize config snapshot and license state. | Publishes config snapshot for later readers and initializes runtime license state. | License init is non-fallible in this path. | None |
| `BOOT-005` | `rustfs/src/startup_preflight.rs` | Initialize observability and store the global guard. | Initializes tracing/observability, stores the guard globally, and logs license/runtime telemetry status. | Fatal if observability init or guard publication fails. | None |
| `BOOT-006` | `rustfs/src/startup_runtime.rs`, `rustfs/src/startup_runtime_hooks.rs`, `rustfs/src/startup_tls_material.rs` | Log startup logo, initialize profiling, trusted proxies, rustls provider, and outbound TLS material. | Starts optional profiling tasks, trusted proxy config, default rustls provider, outbound TLS global state, TLS generation metric, and TLS metrics when enabled. | Profiling/proxy/provider setup is non-fatal; configured TLS material load is fatal on error. | None |
| `RUN-001` | `rustfs/src/startup_server.rs` | Enter startup run orchestration and create `GlobalReadiness`. | Allocates the readiness tracker shared with HTTP readiness gates. | Non-fatal. | Initial readiness state is not ready |
| `RUN-002` | `rustfs/src/startup_server.rs` | Parse and publish the configured region. | Updates ECStore global region when configured. | Fatal if the configured region is invalid. | None |
| `RUN-003` | `rustfs/src/startup_server.rs` | Resolve server address and warn on default credentials. | Computes server port/address and emits production credential warning when defaults are used. | Address parse is fatal; default credentials warning is non-fatal. | None |
| `RUN-004` | `rustfs/src/startup_server.rs` | Initialize global action credentials. | Publishes root/action credentials used by auth paths. | Fatal if global credentials cannot be initialized. | None |
| `RUN-005` | `rustfs/src/startup_server.rs` | Publish server port and address. | Updates global RustFS port and global address. | Non-fatal in this path. | None |
| `RUN-006` | `rustfs/src/startup_storage.rs` | Build endpoint pools and enforce unsupported filesystem policy. | Derives pool/set/disk layout from configured volumes and validates unsupported filesystem policy. | Fatal on endpoint build or unsupported filesystem policy error. | None |
| `RUN-007` | `rustfs/src/startup_storage.rs` | Publish endpoints and erasure type. | Updates global endpoints and erasure type. | Non-fatal in this path. | None |
| `RUN-008` | `rustfs/src/startup_storage.rs` | Initialize local disks, prewarm local disk id map, and initialize lock clients. | Opens local disk state, primes disk id lookup, and creates global lock clients. | Local disk init is fatal; prewarm and lock-client setup are non-fatal in this path. | None |
| `RUN-009` | `rustfs/src/startup_server.rs` | Initialize capacity management and service state manager. | Starts capacity management and moves service state to `Starting`. | Non-fatal in this path. | None |
| `RUN-010` | `rustfs/src/startup_server.rs` | Start S3 HTTP listener and optional console listener before storage is ready. | Starts HTTP servers with readiness gates; console listener starts only when enabled and configured. | Fatal if a configured listener cannot start. | Requests remain gated until full readiness except probe/admin/console/rpc/tonic/table-catalog exempt paths; see [`readiness-matrix.md`](../../architecture/readiness-matrix.md) |
| `RUN-011` | `rustfs/src/startup_storage.rs` | Create cancellation token and initialize `ECStore`. | Creates the runtime cancellation token and storage engine. | Fatal if `ECStore::new` fails. | None |
| `RUN-012` | `rustfs/src/startup_storage.rs` | Initialize ECStore config and global config system. | Initializes ECStore config, attempts server-config migration, then retries global config init up to 15 times. | Migration attempt is non-fatal in this path; global config init becomes fatal after retries. | Marks the `GlobalReadiness` `StorageReady` stage after global config init succeeds; later runtime readiness still rechecks storage, IAM, lock quorum, and gated peer health before `FullReady` |
| `RUN-013` | `rustfs/src/startup_storage.rs` and `rustfs/src/startup_services.rs` | Start replication and KMS systems. | Starts background replication pool, then initializes KMS from startup services. | Replication init is non-fatal in this path; KMS init is fatal on error. | `StorageReady` stage is already marked; dynamic runtime storage readiness is still checked before `FullReady`; KMS compatibility readiness remains feature-gated health behavior |
| `RUN-014` | `rustfs/src/startup_optional_runtime_sidecars.rs` and `rustfs/src/startup_protocols.rs` | Initialize optional protocol servers. | Starts FTP/FTPS/WebDAV/SFTP when feature-enabled and configured, collecting shutdown handles. | Feature-enabled protocol init is fatal on error; disabled protocols are non-fatal. | None |
| `RUN-015` | `rustfs/src/startup_services.rs`, `rustfs/src/startup_audit.rs`, and `rustfs/src/startup_deadlock.rs` | Initialize buffer profiling, event notifier, audit, and deadlock detector. | Starts buffer profile system, event notifier, audit system, and optional deadlock detector. | Audit startup failure is logged and non-fatal; the others are non-fatal in this path. | None |
| `RUN-016` | `rustfs/src/startup_bucket_metadata.rs` | List buckets and run bucket/replication/IAM metadata migrations. | Reads bucket names, migrates bucket metadata, initializes replication resync, migrates IAM config, and initializes bucket metadata system. | Bucket list and replication resync are fatal on error; metadata migration calls are non-fatal in this path. | Storage remains ready; IAM not yet ready |
| `RUN-017` | `rustfs/src/startup_iam.rs` | Bootstrap IAM inline or defer recovery. | Initializes IAM when possible; otherwise starts the deferred IAM recovery path through `startup_iam`. | Fatal only when `bootstrap_or_defer_iam_init` returns an unrecoverable error. | Inline success marks `IamReady`; deferred mode publishes `IamReady` later from the recovery task |
| `RUN-018` | `rustfs/src/startup_auth.rs` | Initialize Keystone and OIDC auth integrations. | Loads Keystone env config and initializes OIDC providers. | Keystone config parse is fatal; Keystone runtime init failure is non-fatal; OIDC init failure is non-fatal. | None |
| `RUN-019` | `rustfs/src/startup_notification.rs` | Add bucket notification config and initialize notification system. | Adds bucket notification configuration and publishes the global notification system. | Notification config add is non-fatal in this path; global notification init is fatal on error. | None |
| `RUN-020` | `rustfs/src/startup_background.rs` | Create AHM cancellation token and initialize heal manager when scanner or heal is enabled. | Creates AHM cancellation token and starts heal manager for heal/scanner workflows. | Heal manager init is fatal when enabled. | None |
| `RUN-021` | `rustfs/src/startup_observability.rs` | Print server info, init update check, allocator reclaim, metrics, memory observability, and auto-tuner. | Starts informational/update/memory/metrics background tasks when enabled. | Non-fatal in this path. | None |
| `RUN-022` | `rustfs/src/startup_lifecycle.rs` and `rustfs/src/startup_iam.rs` | Log successful startup and publish full readiness for inline IAM. | Logs version/address, checks runtime readiness, marks `FullReady`, and sets service state to `Ready` when IAM was ready inline. | Fatal if runtime readiness is not reached within the startup wait. | Marks `FullReady` only for inline IAM here |
| `RUN-023` | `rustfs/src/startup_lifecycle.rs` | Publish global init time and start data scanner when enabled. | Sets global init time and starts scanner after the successful-startup log. | Scanner start is non-fatal in this path. | Full readiness may already be published or may await deferred IAM recovery |
| `RUN-024` | `rustfs/src/startup_lifecycle.rs` | Wait for shutdown signal. | Blocks the main task until a shutdown signal is received. | Non-fatal. | Runtime remains in its current readiness state |
## Deferred IAM Readiness
| Step | Source | Current action | Side effects | Fatal boundary | Ready stage |
|---|---|---|---|---|---|
| `IAM-001` | `rustfs/src/startup_iam.rs:256` | Attempt `init_iam_sys` during bootstrap. | Initializes IAM against the ECStore object layer when possible. | Recoverable failures can enter deferred mode; unrecoverable errors propagate from bootstrap. | None |
| `IAM-002` | `rustfs/src/startup_iam.rs:73` | Spawn IAM recovery loop when bootstrap is deferred. | Retries IAM initialization with exponential backoff until shutdown or success. | Retry failures are logged; the service remains degraded. | None |
| `IAM-003` | `rustfs/src/startup_iam.rs:52` | Finalize IAM recovery after init succeeds. | Initializes `AppContext` if needed, marks `IamReady`, and calls runtime readiness publication. | Finalize failures are retried by the recovery loop. | Marks `IamReady`, then `FullReady` when runtime readiness succeeds |
## Readiness Gate
| Step | Source | Current action | Side effects | Fatal boundary | Ready stage |
|---|---|---|---|---|---|
| `READY-001` | `rustfs/src/server/readiness.rs:130` | Treat exact probe paths and admin/console/rpc/tonic/table-catalog prefixes as readiness-gate bypass paths. | Bypass paths continue to the inner service while the global readiness gate is not ready. | Non-fatal. | Does not change readiness stages |
| `READY-002` | `rustfs/src/server/readiness.rs:171` | Reject non-probe requests while `GlobalReadiness` is not ready. | Returns `503 Service Unavailable`, `Retry-After: 5`, `Content-Type: text/plain; charset=utf-8`, and `Cache-Control: no-store`. | Non-fatal. | Does not change readiness stages |
| `READY-003` | `rustfs/src/server/readiness.rs:202` | Wait for runtime storage, IAM, lock quorum, and gated peer-health readiness before publishing ready state. | Marks `FullReady` and updates `ServiceState` to `Ready` only when a state manager is provided. | Returns an error on timeout; inline startup treats that as fatal, while deferred IAM recovery retries finalization. | Marks `FullReady` |
## Shutdown Order
| Step | Source | Current action | Side effects | Fatal boundary | Ready stage |
|---|---|---|---|---|---|
| `STOP-001` | `rustfs/src/startup_shutdown.rs` | Cancel runtime token and move service state to `Stopping`. | Notifies cancellation-aware background tasks. | Non-fatal. | Service state moves to `Stopping`; readiness stages are not cleared here |
| `STOP-002` | `rustfs/src/startup_shutdown.rs` | Stop scanner/background services and AHM services according to enable flags. | Calls ECStore background shutdown and heal/scanner shutdown helpers. | Non-fatal in this path. | No readiness-stage change |
| `STOP-003` | `rustfs/src/startup_optional_runtime_sidecars.rs` | Plan optional runtime shutdown and log stopping state for FTP/FTPS/WebDAV/SFTP protocol servers. | Collects protocol shutdown handles. | Non-fatal in this path. | No readiness-stage change |
| `STOP-004` | `rustfs/src/startup_shutdown.rs`, `rustfs/src/startup_runtime_hooks.rs` | Stop event notifier, audit system, and profiling tasks. | Stops notifier and profiling tasks; audit stop failures are logged. | Non-fatal in this path. | No readiness-stage change |
| `STOP-005` | `rustfs/src/startup_shutdown.rs` and `rustfs/src/startup_optional_runtime_sidecars.rs` | Stop S3 and console HTTP servers, signal and wait for optional protocol shutdowns, then mark service state `Stopped`. | HTTP shutdown happens after notifier/audit/profiling shutdown in current order. | Join failures are logged by shutdown handles; this path does not return errors. | Service state moves to `Stopped`; readiness stages are not cleared here |
## Migration Rules
- Runtime/lifecycle PRs must map each moved startup line back to one of the
`BOOT-*`, `RUN-*`, `IAM-*`, `READY-*`, or `STOP-*` rows.
- A `pure-move` PR must keep the fatal boundary and ready-stage column unchanged.
- Any intentional change to this table is a separate `behavior-change` PR with
focused negative tests.
- Do not use this document to justify changing readiness, IAM recovery, HTTP
listener timing, lock quorum, or shutdown order in a docs-only PR.