rename_all_missing_source_still_warns and rename_all_real_failure_still_warns
assert that `warn_reliable_rename_failure` emitted its WARN, but that is a
single production callsite shared with tests that call rename_all *without*
installing a subscriber — rename_all_missing_source_returns_file_not_found,
two tests above, is one of them.
tracing caches each callsite's Interest process-globally and the first thread
to reach a callsite fixes that value; while at most one dispatcher is
registered, tracing-core derives it from the registering thread's own
subscriber, and registration is once-only. When the subscriber-less sibling
wins, the callsite is cached as Interest::never() and the WARN never fires,
so the assertion sees empty output:
ordinary missing-source failures must keep the WARN, got:
Reproduced at 3/25 with `disk::os::tests::rename_all_missing_source` (both
tests), against 0/20 for the victim alone. Fixed by pinning callsite interest
inside warn_capture(), so every current and future user of that helper is
covered rather than just the two tests that happen to fail today.
pin_callsite_interest_for_test() moves from cluster::rpc::background_monitor
to a new crate-level test_tracing module: it is domain-neutral and now has
consumers in two unrelated subsystems, and disk::os should not have to reach
into a cluster::rpc test helper.
Verified: repro filter 0/30 (was 3/25); disk::os:: 0/12; cluster::rpc:: 0/12
and its poisoner pair 0/15, confirming the moved helper still holds.
Follow-up to #5438. Closes the last item in #5439.
Only replication-target writes took the bucket transaction lock. Every other
config write (policy, tagging, lifecycle, versioning, ...) went straight to
the process-local metadata-system guard, which serializes nothing across
nodes.
Each config write is a read-modify-write of one whole BucketMetadata blob:
load the blob, replace one field, save the blob back. The namespace locks
inside read_config/save_config are taken and released separately, so they do
not span that cycle. Two nodes updating different config files of the same
bucket therefore both load the same blob, each set their own field, and the
later save drops the other's -- with both clients already told 2xx. This is
not last-writer-wins on one document; an orthogonal config silently vanishes.
Route update(), delete() and update_config_with() through
acquire_config_write_guards(), which takes the cluster-wide transaction lock
first and the metadata-system write guard second. That order is load-bearing:
taking the process-local guard first would park every local reader and writer
of every bucket behind a lock whose holder may be another node, turning
remote contention into a local stall.
The lock is per bucket rather than per config file, since a per-file key
would let exactly the offending pair run concurrently. Rename the helper to
acquire_bucket_metadata_transaction_lock to match, but deliberately keep the
lock resource string as "bucket-targets/{bucket}/transaction.lock": the key
is what nodes agree on, so renaming it would leave a mixed-version cluster
with two disjoint keys and stop old and new nodes from excluding each other
on the very writes that are serialized today.
update_config_with() already narrowed its staleness window to a single load
and save, but its exclusion was explicitly process-local; it is now
cluster-wide, so its doc comment no longer disclaims cross-node races.
Also make update_and_parse load through self.api instead of the ambient store
handle, so the read and the write of one read-modify-write cannot resolve to
different instances.
The new tests drive two BucketMetadataSys instances over one ECStore -- the
in-process stand-in for two nodes, since they share no RwLock and can only be
serialized by the namespace lock. Verified the lost-update test has teeth by
removing the lock and confirming it fails on round 0, with the tagging config
clobbered to empty by the concurrent policy write.
Restore persisted bucket notification rules after the notification target runtime converges so restarted nodes rebuild their local rule engine without requiring an unchanged PUT bucket notification request.
Fixes#5428
Co-authored-by: heihutu <heihutu@gmail.com>
peer_rest_recovery_probe_logs_keep_request_id_span_context failed ~10% of
`cargo test -p rustfs-ecstore --lib -- cluster::rpc::` runs with
left: "request-span", right: "recovery-monitor". The recovery-monitor
info_span! was evaluating to Span::none(), so the probe's log line landed
under the caller's span.
tracing caches each callsite's Interest in process-global state, and the
first thread to reach a callsite fixes that value. While at most one
dispatcher is registered, tracing-core takes a fast path that derives the
interest from the registering thread's own subscriber, and registration is
once-only (CAS). Under libtest a sibling test reaches recovery_monitor_span
via mark_offline_and_spawn_recovery from a thread with no subscriber, so
the interest is derived from NoSubscriber and cached as Interest::never()
for the whole process.
Add pin_callsite_interest_for_test(): registering a second, inert
dispatcher rebuilds every registered callsite's interest against the live
dispatcher set (repairing a poisoned value) and keeps tracing-core off the
single-dispatcher fast path (preventing new ones). This also covers the
production marked_suspect / recovery_monitor_started event callsites that
remote_disk_network_error_starts_recovery_monitor_with_request_context
asserts on.
rename_data_response_accepts_legacy_json_without_decode_error is a
separate root cause: it snapshots the process-global internode metrics and
asserts the decode-error counter did not move, which siblings that record
decode errors (or reset the counters) invalidate. Put the 11 tests that
observe those counters in one #[serial(internode_metrics)] group.
Both races are impossible under nextest, which runs each test in its own
process, so neither test belongs in the ecstore-serial-flaky test-group
(that serializes across process boundaries) nor in the ci-profile
quarantine (they never redden CI).
Verified: cluster::rpc:: subset 0/30 failures under libtest (was 3/30);
target test paired with its poisoner 0/30 (was 4/20); 5/5 clean under
nextest at 179/179.
The Test and Lint lane intermittently stalls until the inner 75m timeout
kills the cargo process group (issue #5394). The evidence collected since
#5402 shows the stall can begin in the build phase before any test runs
(run 30449339653: last output at minute 4 of the nextest step, then 71
silent minutes until SIGTERM), but the post-mortem pgrep always reports
nothing because GNU timeout has already terminated the whole process
group by the time it runs.
Add a background sampler to the nextest step that appends system and
process snapshots (loadavg, PSI, memory, disk, top-RSS processes,
cargo/rustc/linker/build-script processes, D-state processes) to the
existing test-and-lint artifact every 60 seconds, and record kernel
OOM/kill events in the post-mortem diagnostics. The last samples before
a timeout identify the wedged process or the resource pressure that
caused the stall. This only instruments the failure mode where the
runner survives; jobs whose runner disappears entirely still upload no
artifacts and need runner-pool-side logs.
Refs #5394
* fix: keep hotpath gate csv paths clean
Redirect the enhanced bench driver's verbose output away from command substitution so run_hotpath_warp_ab.sh records only the candidate cell path before passing baseline_compare.csv files to the gate.
Co-Authored-By: heihutu <heihutu@gmail.com>
* perf(utils): reduce highway hash key setup
Cache the parsed HighwayHash keys as compile-time constants and add a criterion benchmark for the PUT hash hotpath.
Co-Authored-By: heihutu <heihutu@gmail.com>
---------
Co-authored-by: heihutu <heihutu@gmail.com>
The LoadBucketMetadata peer-notification handler loaded bucket metadata
with the fabricating loader (ConfigNotFound -> BucketMetadata::new) and
unconditionally cached the result. On a transient read-quorum dip during
a reload notification, a peer cached an authoritative "no Object Lock"
default for a lock-enabled bucket, disabling the batch-delete retention
gate (object_lock_delete_check_required) on that node until the next
refresh, and wiping its bucket-target/durability sync state.
Production changes:
- New BucketMetadataSys::reload_from_store (metadata_sys::
reload_bucket_metadata): the peer reload path uses the presence-aware
loader and installs only metadata actually read from persisted
storage. A load miss returns an error (surfaced to the notifying peer
as success=false) and leaves the cache untouched; deletion still
propagates only through the dedicated DeleteBucketMetadata
notification.
- The reload runs under the outer metadata-sys write guard, load
included, mirroring update(): every other cache installer holds that
lock, so a reload snapshot can never land after - and roll back - a
newer concurrent install (the stale-load lost-update from the
review), and the install-plus-registry-sync sequence stays atomic
against concurrent removes and reloads (previously only the set call
was write-guarded, with the load outside any lock).
- The peer-visible miss error is a fixed string: the notifying peer
substring-matches error text against network-failure needles
(is_network_like_error), so interpolating a bucket name (e.g. a legal
bucket literally named "unavailable") could mark a healthy peer
offline.
- get_config's lazy insert routes through set(), picking up the
negative-cache invalidation.
An earlier draft instead guarded set() with a per-config updated_at
freshness comparison. Adversarial validation rejected it (three roles
independently): update_config stamps with the handling node's wall
clock, so within the skew the cluster already tolerates (+/-300s RPC
auth window) a config rewritten with an earlier stamp - e.g. revoking a
public-read policy through a second node, or any same-field rewrite
after an NTP step-back - would be skipped by every peer forever,
silently pinning the revoked permissive config with no re-convergence
path (the 15-minute refresh also routed through the guard). Race
staleness is second-scale while skew is minute-scale, so no tolerance
bound can separate them; the write-guard serialization closes the same
race without clocks and preserves the refresh loop's unconditional
converge-to-disk property, which is the cluster's self-healing
mechanism.
Startup audit (BucketMetadataSys::init): concurrent_load's
insert-if-vacant still installs a fabricated default when a transient
miss hits at boot - indistinguishable from a legacy bucket without a
metadata file at this layer - bounded by the next successful persisted
load. Making the object-lock gate fail closed on such entries is filed
as a follow-up, alongside the bare "unavailable" needle in
is_network_like_error and the Swift cache-only metadata writes.
Tests:
- bucket::metadata_sys::tests::
peer_reload_never_caches_fabricated_defaults_as_authoritative:
miss installs nothing / miss keeps the existing entry intact
(asserting the dedicated non-persisted error) / persisted reload
converges the cache over a stale entry.
- node_service::tests::
test_load_bucket_metadata_failure_skips_scanner_maintenance:
a failed reload reports failure and does not advance scanner
maintenance activity (previously recorded even on a miss).
- The handler success path stays uncovered at the RPC layer (needs an
isolated global object layer, like the pre-existing ignored test);
the composition is pinned at the sys level instead.
Verification:
- cargo fmt --check and cargo clippy --lib --tests clean on
rustfs-ecstore and rustfs.
- Targeted suites green; full cargo test -p rustfs-ecstore --lib:
3198/3200 with two parallelism-sensitive lock-test flakes from the
known baseline (pass in isolation; a different pair flakes per run).
- Adversarial validation (high-risk tier, all seven roles as
independent parallel reviewers) run per AGENTS.md; all findings
fixed or rebutted with evidence, three out-of-scope findings filed
as follow-up tasks.
Co-authored-by: houseme <housemecn@gmail.com>
Zhengchao An