* fix(site-replication): route every state RMW through the locked transaction
P1-15 PR2 (rustfs/backlog#1796, batch B2 of rustfs/backlog#1675), the
follow-up promised by rustfs/rustfs#5882.
PR1 left ~26 read-modify-write call sites on
config/site-replication/state.json in the pre-transaction shape: a
process-local mutex around load / mutate / save, each IO taking its own
object lock. Nothing held a distributed lock across the whole sequence, so
two nodes of one site still lost each other's updates, and the transitional
mutex kept the old shape available to copy.
Every remaining RMW now runs inside update_site_replication_state;
read-only sites use load_site_replication_state, whose object read comes
with the object-level read lock. SITE_REPLICATION_STATE_LOCK and its owner
helper are gone, together with their architecture-guard allowlist entry and
inventory row.
The multi-stage flows (add / edit / peer join / peer edit / remove / rotate)
keep their updated_at and pending-id CAS, but the CAS now runs inside the
transaction that writes, against the state that transaction loaded. Peer
probes, IAM work and fan-outs run between transactions and hold no lock at
all — the add no longer blocks every writer of the site across its peer join
round trips, and it re-checks the precondition right after the capability
probes so the common race is rejected before any IAM write or remote join.
When the add's commit CAS still fails, the error says the peers may already
be joined and that re-running the add reconverges. The add adopts only the
fields it computed (exhaustive destructure — adding a state field is a
compile error until classified); fields owned by writers that do not bump
updated_at keep their freshly loaded values.
Ordering of peer-edit deliveries now rests on the generation fence landed in
PR1 rather than on a guard that could never order two nodes: the add's
finalize fan-out carries the generation allocated in its commit. An accepted
peer join PRESERVES the applied-generation high-water marks — join fan-outs
are routine (adds and rotations both deliver SRPeerJoin to existing peers),
so wiping them would let stalled older edits land after any join; the
unilateral-removal rejoin misfence that a wipe would have patched is
pre-existing since the fence landed and needs an epoch in the fence instead.
The rotation handler now takes the lifecycle guard: the background
service-account reconciler runs its repair under a lifecycle try-acquire,
and its pending-rotation precheck is only sound if a rotation cannot start
mid-repair — an exclusion the removed process mutex used to provide as a
side effect.
update_site_replication_state_when_changed adds persist-or-skip so ack
markers and pending-clearing paths stop rewriting the object on a miss —
load-bearing, because the shared persist helper clears the whole object for
a ≤1-peer pending-free state — and save_site_replication_state is now
cfg(test): the pre-P1-15 shape can no longer be written in production code.
No on-disk format change.
Verification: cargo nextest run -p rustfs -E
'test(/admin::handlers::site_replication::/)' (181 passed); site-replication
dual/three-node e2e (13 passed); cargo clippy -p rustfs --all-targets -D
warnings; make pre-commit. Mutation checks: dropping the state-object lock
from the boundary reds the separate-node concurrency tests; flipping a
persist-or-skip miss to a persist reds
test_missed_pending_clear_must_not_rewrite_the_state_object. Reviewed by
three independent adversarial passes (correctness/concurrency,
security/compatibility, simplicity/test-coverage); their confirmed findings
are folded in.
* fix(site-replication): serialize peer-join admission around its IAM write
Review follow-up (overtrue): two joins accepted by the same node could
interleave as "A checks a stale snapshot and pauses reading its body, B
applies secret B and commits, A resumes, overwrites IAM with secret A, and
A's commit is refused as superseded" — the persisted state advertised B's
contract while IAM only accepted A's secret, failing every peer
control-plane call. The pre-P1-15 process mutex serialized same-node joins
end to end; removing it dropped that exclusion.
admit_peer_join now runs the staleness check, the IAM upsert and the state
commit under the lifecycle guard, with the authoritative pre-check taken
against a load under that guard BEFORE IAM changes anything. The closing
transaction still re-checks staleness: the guard is process-local (exactly
as far as the old mutex reached) and the state-object lock arbitrates joins
accepted by different nodes. The body is fully read before the guard so a
stalling sender cannot block add/remove/rotate/reconciler.
The IAM step is injected, and the gated-body regression test reproduces the
review's ordering: join A is held mid-IAM while a newer join B arrives; B
must wait at the guard, and both IAM order and the final persisted state end
on B. Mutation-verified: removing the lifecycle guard from admit_peer_join
turns the test red.
Verification: cargo nextest run -p rustfs -E
'test(/admin::handlers::site_replication::/)' (182 passed);
site-replication dual/three-node e2e (13 passed); cargo clippy -p rustfs
--all-targets -D warnings; make pre-commit.
* fix(site-replication): fence peer-join admission across nodes
Review follow-up (overtrue, round 2): the lifecycle guard only serializes
joins within one process. Node A could pass the staleness check for an
older T1, node B write secret B to IAM and commit a newer T2, and node A
then overwrite IAM with secret A while its own state commit is refused as
superseded — state advertising T2's contract while IAM only accepts A's
secret.
The admission (staleness check -> IAM upsert -> state commit) now also runs
under a distributed join-admission lock, a namespace-lock key with no
backing object, following the repair execution lock's pattern — including
its nesting of config-object locks (admission -> state), and delegating
crash safety to the lock subsystem's lease expiry instead of a hand-rolled
TTL. The staleness check runs against a load taken inside the lock, before
IAM changes anything, so a superseded join exits without touching IAM. The
closing transaction keeps its re-check for defence in depth and for
old-version nodes that do not take the admission lock during a rolling
upgrade (that mixed-version window keeps today's behavior and closes when
the upgrade completes).
admit_peer_join_across_nodes is the admission minus the process-local
lifecycle guard — exactly what a second node runs — and the new
separate-nodes regression test drives it directly with join A gated
mid-IAM: join B must wait at the distributed lock, and both the IAM write
order and the final persisted state end on B. Mutation-verified: removing
the admission lock turns the test red while the same-node test (which
drives the full admit_peer_join) stays green.
Verification: cargo nextest run -p rustfs -E
'test(/admin::handlers::site_replication::/)' (183 passed);
site-replication dual/three-node e2e (13 passed); cargo clippy -p rustfs
--all-targets -D warnings; make pre-commit.
* refactor(obs): make dial9 telemetry opt-in and actually record events
The dial9 Tokio-runtime profiler was disabled by default, yet every build
paid for it, and enabling it produced trace files with no events in them.
Recorded empty traces
---------------------
`build_traced_runtime` called `TracedRuntime::builder()...build(..)`, but dial9
only starts recording in `build_and_start*`. `build` still returns a live guard
whose `is_enabled()` reports true, and still creates and seals segment files —
they just contain a header and no events. It also skipped `with_trace_path`, so
the background worker driving the segment pipeline was never spawned.
Measured on the new smoke example: 310 bytes of bare segment header, against
5640 bytes for the same workload once recording actually starts.
Switch to `with_trace_path(..).build_and_start(..)`.
Cost was unconditional
----------------------
`--cfg tokio_unstable` was a global `[build] rustflags` entry and `rustfs-obs`
depended on `dial9-tokio-telemetry` unconditionally, so all builds depended on
Tokio's non-semver API. Worse, an environment `RUSTFLAGS` replaces (never
appends to) the config-file value, so any caller exporting their own RUSTFLAGS
silently dropped the flag — the long comment in build.yml was a scar from that.
dial9 is now an opt-in feature (`dial9`, plus `dial9-s3` and `dial9-taskdump`),
the global rustflag is gone, and `crates/obs/build.rs` fails the compile if the
feature is on without the flag. Telemetry builds go through `make build-profiling`.
Metrics that could not lie
--------------------------
`rustfs_dial9_{events_total,bytes_written_total,rotations_total,cpu_overhead_percent}`
were hard-coded to zero — a Counter pinned at 0 reads as "nothing happened".
Removed. `rustfs_dial9_enabled` was sourced from the environment, so it read 1
even when the traced runtime failed and the process fell back to a standard
runtime; it is replaced by `rustfs_dial9_supported` (compile-time),
`rustfs_dial9_configured` (intent) and `rustfs_dial9_active_sessions` (reality).
No `writer_healthy` gauge is exported: dial9's `RotatingWriter` can enter its
`Finished` state and stop writing, but exposes no way to observe that, so the
gauge could only ever be hard-coded to 1. Documented as a known gap instead.
Final events were lost
----------------------
The `TelemetryGuard` lived in a `static OnceLock`, which is never dropped, so
buffered events were never flushed at exit. `build_tokio_runtime` now returns
the guard and `run_process` drops it before any exit path.
Also
----
- `disk_usage_bytes` was a `read_dir` + per-file `stat` on the metrics
collection path. It is now sampled by a background task into an atomic.
- `SAMPLING_RATE`/`S3_BUCKET`/`S3_PREFIX` were parsed, warned about, and
discarded. S3 upload is now wired to dial9's `with_s3_uploader` behind
`dial9-s3`; `SAMPLING_RATE` has no upstream equivalent and is removed.
- Wire `with_task_dumps` (async backtraces of stalled tasks), configurable via
`RUSTFS_RUNTIME_DIAL9_TASK_DUMP_{ENABLED,IDLE_THRESHOLD_MS}`.
- Split `telemetry/dial9.rs` into `config`/`state`/`enabled`/`disabled`; the
stub keeps the public API identical so callers need no `#[cfg]`.
- Drop four print-only examples and the manual test bin that exercised the
removed `init_session` scaffolding.
Verified: cargo check/clippy/test across default, `dial9`, and `dial9-s3`;
build.rs correctly rejects `dial9` without `--cfg tokio_unstable`;
`make pre-commit` passes.
Co-Authored-By: heihutu <heihutu@gmail.com>
* docs(obs): document dial9 as an on-demand profiler
scripts/run.sh advertised a `SAMPLING_RATE` knob that was never passed to dial9,
and claimed "CPU overhead < 5% (with sampling rate 1.0)" and "lower values reduce
CPU overhead" on the strength of it. The knob is gone; the guidance built on it
had to go too.
Replace it with what is actually true: dial9 needs a `make build-profiling`
binary, its disk budget evicts oldest-first (so a high poll rate can overwrite
the incident you are chasing), and it cannot be toggled without a restart.
Add docs/operations/dial9-runtime-profiling.md covering the build variants, an
investigation walkthrough, the configuration table, how to read the three
supported/configured/active_sessions gauges against each other, and the upstream
gap that makes writer death only indirectly observable.
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(obs): add a dial9 smoke example that proves events are recorded
The bug this guards against is invisible to every existing signal: with
`build` instead of `build_and_start`, dial9 creates the trace file, seals
segments, and reports `TelemetryGuard::is_enabled() == true` — it simply
records no events. Only the segment's byte count tells the two apart.
Measured on this workload: 5640 bytes when recording, 310 bytes (a bare
segment header) when not. The example asserts >= 2048 bytes, and was verified
to fail with the `build` call restored.
Also correct the comment on the `is_enabled` check in `finish_traced_runtime`.
It claimed to catch "recording silently off"; it does not. It only rejects the
inert guard a lenient config yields after a build failure. Recording is
guaranteed by `build_and_start`, not by that check.
Co-Authored-By: heihutu <heihutu@gmail.com>
* test(rustfs): accept Unsupported runtime telemetry capability
A binary built without the `dial9` feature now reports the runtime-telemetry
capability as `Unsupported` rather than `Disabled`. The distinction matters to
operators: `Disabled` implies the capability can be switched on by setting an
environment variable, which is not true here — telemetry needs a rebuild.
Widen the assertion and pin the new semantics: when `dial9::is_supported()` is
false, the state must be exactly `Unsupported`.
Co-Authored-By: heihutu <heihutu@gmail.com>
* fix(obs): drop the dial9-s3 feature, its TLS stack is vulnerable
CI's Dependency Review and `cargo deny` both reject the branch: dial9's
`worker-s3` feature depends on aws-sdk-s3-transfer-manager 0.1.3, which pins
aws-smithy-http-client onto hyper-rustls 0.24 and rustls-webpki 0.101.7. That
webpki carries RUSTSEC-2026-0098, -0099 and -0104.
0.1.3 is the latest release of the transfer manager, and 1.2.0 the latest of the
smithy client, so there is nothing to upgrade to. Cargo's feature unification can
add features but cannot drop a transitive dependency, so it cannot be worked
around from here either — the rest of the workspace already resolves to the safe
rustls-webpki 0.103 / hyper-rustls 0.27.
Remove the `dial9-s3` feature and the `with_s3_uploader` wiring. The two S3
environment variables stay parsed and warned about, now naming the real reason
rather than a missing build feature. Trace segments are collected from the output
directory instead. Tracked as D9-14 in rustfs/backlog#1157.
With this, Cargo.lock is byte-identical to main: the PR no longer touches the
dependency graph at all.
Also correct the `dial9-taskdump` documentation. It claimed the feature "compiles
to a no-op elsewhere"; in fact `tokio/taskdump` raises a `compile_error!` on any
target other than linux/{aarch64,x86,x86_64}. Verified by trying to build it on
macOS, which is how the claim was found to be wrong.
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
唐小鸭