mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-26 08:18:18 +00:00
a80699b6dd6c4317caf2acd14e6f8a7d495341fe
1949 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
a80699b6dd |
feat: add an opt-in NATS JetStream publish path for the notify and audit targets (#4634)
feat(targets): add an opt-in NATS JetStream publish path for the notify and audit targets The NATS notify and audit targets publish through NATS Core, which returns before the server has durably accepted the message. A broker restart or a connection drop between the publish and the flush loses the event, even though the send queue has already cleared it, and no acknowledgement gates that clear. An opt-in JetStream publish path clears a queued event only after the server returns a durable PublishAck, so delivery is at-least-once across a broker restart or a reconnect. It applies to both the notify and audit NATS targets, is off by default, and is byte-identical to the NATS Core path when disabled. The path includes durable store-and-forward, a stable dedup id sent as the Nats-Msg-Id header so a replayed event is collapsed by the stream duplicate window, pre-flight stream validation, and a bounded failed-events store for terminally-failed and retry-exhausted events. Three configuration keys per target select it: JETSTREAM_ENABLE, JETSTREAM_STREAM_NAME, and JETSTREAM_ACK_TIMEOUT_SECS, under the RUSTFS_NOTIFY_NATS_ and RUSTFS_AUDIT_NATS_ prefixes. The on-disk batch filename separator changes from colon to underscore so batch names are valid on Windows filesystems, with transparent read-back of files written under the previous separator. The migration affects the shared queue store for every target type and lands with this feature because the store gains its first Windows-exercised paths here. Co-authored-by: houseme <housemecn@gmail.com> |
||
|
|
25f81f812c |
feat(site-replication): support custom TLS peers (#4802)
* feat(madmin): add site replication TLS settings * feat(site-replication): support custom TLS peers * test(site-replication): remove redundant clones * test(site-replication): avoid needless resolver collection |
||
|
|
e9a0200a72 | fix(ecstore): hedge slow shard reads in lockstep GET to cut the large-object first-byte tail (#4799) | ||
|
|
27a7cc739e |
fix(targets): keep pulsar target online after restart without TLS (#4798)
The pulsar config validation rejected any non-`pulsar+ssl` broker whenever `tls_allow_insecure` was set or `tls_hostname_verification` was disabled. Both toggles are inert on a plaintext `pulsar://` broker — the Pulsar client only applies them for `pulsar+ssl` — so treating a non-default value as fatal is wrong. This surfaced as issue #4796: the console persists `tls_hostname_verification` as `false` (the checkbox defaults to unchecked while the server default is `on`). At creation time the value is not present in the unmerged request and falls back to the safe default, so the connectivity check passes and the target comes online. On restart the persisted config is merged and validated, the `false` is read back, and the target is rejected — permanently offline even though Pulsar is reachable. Relax both the loader-side (`validate_pulsar_broker_config`) and runtime-side (`PulsarArgs::validate`) checks so only a `tls_ca` bundle — real TLS trust material that is never defaulted to a non-empty value — requires a `pulsar+ssl` scheme. The inert toggles no longer fail the target. This also heals configs already persisted with the bad value. Add regression coverage for both paths. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
0ac7f0d0cf | chore: refresh erasure codec and rust toolchains (#4795) | ||
|
|
7ece747eab | fix(ecstore): suppress missing rollback rename warnings (#4792) | ||
|
|
6096bb189d | fix(ecstore): demote reliable_rename NotFound WARN to debug (#4789) | ||
|
|
535d672b1f | fix(admin): report heal runtime state (#4786) | ||
|
|
2e85709634 |
chore: refresh workspace lockfile (#4785)
* chore: refresh workspace lockfile * chore: bump uuid to 1.23.5 * chore: bump pollster and path-absolutize |
||
|
|
1553dc3f62 |
Address P2 follow-ups from the 2026-07-10..12 merged-PR review (backlog#1210-1220) (#4783)
* fix(obs): open cleaner compression source with O_NOFOLLOW The compressor opened the source log via File::open, which follows a symlink at the final path component. Between the scanner selecting a regular file and this open, an attacker with write access to the log directory could swap the entry for a symlink (TOCTOU) pointing at, say, /etc/shadow, whose contents would then be copied into an archive. Open the source with O_NOFOLLOW on Unix so such a swap fails with ELOOP; the temp/archive path already refused symlinks, this closes the source side. Refs rustfs/backlog#1210 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(obs): recompress instead of trusting leftover cleaner archives archive_header_ok only checked the first 2-4 magic bytes before treating an existing .gz/.zst as a completed prior result and letting the caller delete the source log. A file with valid magic but a truncated or forged body passes that check, so an attacker with write access to the log directory (or a crashed prior run) could plant such a stub and make the cleaner delete the real log without ever producing a usable archive — silent audit-data loss. Chosen fix: stop trusting cross-process leftovers entirely and always recompress the source in this pass, rather than fully decoding every leftover to validate it. Full-decode validation would add real CPU cost and decode-bug surface for a rare crash-recovery case; the existing atomic create_new+rename already overwrites whatever sits at the archive path (a planted symlink is replaced, never followed) with a freshly written, fsync'd archive, so a partial/forged leftover can never gate source deletion. This is the lowest-regression option. Refs rustfs/backlog#1211 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(object-data-cache): cap memory-gate reservation at cache growth headroom The memory gate subtracts `admitted_since_refresh` from the snapshot's available bytes so a burst arriving faster than the 5 s refresh cannot over-allocate. That counter is GROSS: it only rolls over on the refresh and never rolls back when a fill is later evicted, cancelled, or loses the invalidation race. Under sustained high-throughput churn (net footprint flat and far below `max_capacity`) the raw counter balloons past the memory the cache actually holds, so `effective_available` collapses and the gate reports false memory pressure — skipping the hottest fills with SkippedMemoryPressure until the next 5 s refresh. This only lowers hit rate; it never returns wrong data and self-heals each refresh. Fix direction 1 (minimal regression): cap the reservation deduction at the cache's own growth headroom (`max_capacity - weighted_size()`) instead of letting the unbounded gross counter shrink the system-available budget. The cache can never hold more than `max_capacity`, so a burst adds at most that headroom of real memory before moka evicts to stay bounded (net-zero churn beyond that point) — capping the deduction there keeps the reservation honest without treating gross churn as growth. Chosen over net-accounting (direction 2, releasing bytes on every failure/cancel/eviction path) because that only plugs the leak on failed fills and would not address the core defect: churn of *successful* insert/evict fills over the 5 s window. It also touches only the gate plus one call site rather than every failure path in moka_backend. The cap only ever raises `effective_available`, so real memory pressure (a low snapshot at refresh) still suppresses fills; when the cache is at capacity the headroom is 0 and the deduction vanishes, correctly reflecting net-zero churn. `MokaBackend` now stores `max_capacity` and passes the live headroom into `allows_fill`. Adds targeted gate tests: gross churn far above headroom no longer falsely suppresses, yet the reservation still bounds a burst while the cache can genuinely grow. Refs rustfs/backlog#1212 Co-Authored-By: heihutu <heihutu@gmail.com> * test(ecstore): assert native O_DIRECT path runs in uring read test uring_preserves_o_direct_for_eligible_reads only compared bytes through LocalDisk::read_file_mmap_copy. On a filesystem that rejects O_DIRECT the read silently degrades to the buffered StdBackend fallback and the byte check still passes, so the test could go green without the native read_at_direct path ever executing -- a vacuous pass. Add a per-disk native_direct_reads counter on UringBackend, incremented only when pread_uring_direct completes, and rebuild the test to drive a real UringBackend's pread_bytes and assert the counter is non-zero (every eligible read went through the native tier). When io_uring or O_DIRECT is unavailable on the host filesystem (restricted CI runners, tmpfs), the test skips loudly via eprintln instead of asserting a tautology, while still checking byte-correctness on whatever tier served the read. The counter also gives a gray release a positive signal that the O_DIRECT tier is serving reads, not just a fallback count. Refs rustfs/backlog#1213 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): warn + count read-time EINVAL on native O_DIRECT reads classify_direct_read_error is only reached from the read side: the O_DIRECT open in pread_uring_direct already succeeded (an open-time refusal is handled earlier as DirectOpenError::ODirectRefused). So an EINVAL/EOPNOTSUPP arriving here is a read-time error on an fd the kernel accepted for O_DIRECT -- far more likely an alignment bug in the aligned read path than an unsupported filesystem. The old code latched the disk's native path off with only a once-per-disk debug trace, hiding a potential correctness regression behind a silent buffered-read downgrade. Diagnostics only: the fallback behaviour is unchanged (the native path is still latched off and the caller still reads via StdBackend). This adds a rustfs_io_uring_direct_read_einval_total counter and promotes the once-per-disk trace from debug to warn so an operator can see an alignment regression instead of an unexplained latency/CPU shift. Refs rustfs/backlog#1214 Co-Authored-By: heihutu <heihutu@gmail.com> * docs(ecstore): document data-blocks-first default and its tail-latency cost DEFAULT_RUSTFS_GET_DATA_BLOCKS_FIRST_READER_SETUP is true and must stay true: deferred-parity is the deliberate, already-rolled-out full-object GET default from backlog#1159/#923. Flipping it back to false in code would silently revert that rollout for every deployment that has not set the env var, so this commit only documents -- no behaviour change. The added notes explain what data-blocks-first does (schedule data shards up front, engage parity lazily on a missing/corrupt data shard), the known trade-off (parity is engaged late, so a slow-but-not-dead data drive raises GET p99 because the faster parity shards are not raced against it until a data shard is declared missing), and the operational rollback switch (RUSTFS_GET_DATA_BLOCKS_FIRST_READER_SETUP=false), which is intentionally an env override rather than a code default change. No metric was added: the low-risk observability hook for "slow data drive engaged deferred parity" would live at the deferred-stripe engage point, which is out of this file's scope; this change stays documentation-only to avoid touching the hot GET path. Refs rustfs/backlog#1215 Co-Authored-By: heihutu <heihutu@gmail.com> * docs(ecstore): document wide-directory walk stall hazard and tuning list_dir enumerates a whole directory in one os::read_dir call (count = -1), and the walk caller bounds that entire enumeration with the per-read stall budget (default 5s) as if it were a single read. For a wide, flat prefix -- one directory holding millions of immediate children -- a single readdir can exceed the budget on a healthy disk, trip DiskError::Timeout, and surface as a ListObjects 500 quorum failure though the drive is fine (a #2999 sub-class). This is documented, not rewritten: turning the one-shot readdir into a streaming/batched enumeration that refreshes the stall deadline between chunks is an architecture-level change with high regression surface (ordering, the count contract, quorum merge) and belongs in a separate follow-up. The supported mitigation today is operational, so the comments point wide-directory deployments at RUSTFS_DRIVE_WALKDIR_STALL_TIMEOUT_SECS and the high-latency drive-timeout profile, which widen the budget with no code change. Notes were added at list_dir, the scan_dir call site, and get_drive_walkdir_stall_timeout. No behaviour change. Refs rustfs/backlog#1216 Co-Authored-By: heihutu <heihutu@gmail.com> * docs(ecstore): document consumer-peek vs producer-stall coupling In list_path_raw the consumer's peek_timeout is drawn from the same source and same value (walkdir_stall_timeout, default 5s) as the producer-side walk stall budget, but the two measure different things: the producer stall bounds a single drive read, while the consumer peek bounds the gap between two ADJACENT entries arriving from a reader. Because they share a value, the consumer cannot wait meaningfully longer for the next entry than the producer is allowed to spend producing one. Walking a region dense with non-listable internal items can make a HEALTHY drive miss the budget between visible entries; the consumer then declares it stalled and detaches it, dropping a good drive from the merge and capping the "large prefix succeeds" guarantee. Documented, not decoupled: giving the consumer peek an independent, strictly-larger budget would cut these false detaches but equally delays detaching a genuinely dead drive and shifts listing tail-latency semantics, so it wants soak data before changing the default. The comment records the invariant any such follow-up must keep -- consumer peek >= producer stall, never stricter -- so it can never fail a drive before the producer would. No behaviour change. Refs rustfs/backlog#1217 Co-Authored-By: heihutu <heihutu@gmail.com> * fix(io-metrics): add time-based trigger for low-IOPS latency percentiles Percentiles were recomputed only every 128 IOs and seeded to 0, so a low-traffic deployment exported p95/p99 = 0/stale for a long time after startup. Add a 10s wall-clock trigger alongside the count throttle so the first recompute can fire before 128 samples accrue. Hot-path per-op mean update is unchanged. Refs rustfs/backlog#1218 Co-Authored-By: heihutu <heihutu@gmail.com> * test(e2e): cover codec-streaming parity under fault injection and NoSuchKey The codec-streaming compat A/B previously ran only against a healthy 4-disk EC set with successful full GETs: the DiskFaultHarness was constructed but never faulted, the error path was untested, and the range assertion silently compared legacy-vs-legacy (ranges always fall back to the duplex path), overstating what it proved. Add two genuinely-failable scenarios reusing the existing harness and fixtures: - Parity reconstruction A/B: take one data disk offline and re-run the full object matrix on both phases while the EC 2+2 set rebuilds each large object from the surviving shards. Assert codec == legacy byte-for-byte (sha256) and header-for-header, and assert the codec phase served the reconstructed objects with zero duplex-pipe fallback (the reader gate is drive-health-independent, so the codec fast path is really exercised through reconstruction). - NoSuchKey negative path: compare the HTTP status + S3 error code of a missing-key GET across the legacy and codec phases and require them to be identical (404/NoSuchKey), guarding against the codec env perturbing the error path. Also clarify the range-phase comment so it is not misread as codec-range correctness coverage: both sides are served by the same legacy range path, so the assertion only proves ranges keep working and keep falling back to legacy with the gates open. Verified: cargo check/--no-run pass and the test passes locally (1 passed; dup_codec=0 confirms the codec path ran). Refs rustfs/backlog#1219 Co-Authored-By: heihutu <heihutu@gmail.com> * ci(ecstore): exercise native O_DIRECT read path on an ext4 loopback The uring-integration leg ran on the runner's default TMPDIR, which may sit on tmpfs/overlayfs where open(O_DIRECT) fails and the native read_at_direct path silently latches off to the aligned StdBackend fallback. Mount a dedicated ext4 loopback and point TMPDIR at it so the real io_uring dep (bumped git->0.1.0->0.2.0->0.2.1) and the native O_DIRECT read path are actually covered rather than validated only by signature diffing. Refs rustfs/backlog#1220 Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
63b4568f85 |
fix(ecstore): reclaim orphan data dirs on the healthy heal path (#4781)
PR #4356 wired `reclaim_orphan_data_dirs` only into `heal_object`'s post-heal tail, which runs after the `disks_to_heal_count == 0` early return. That early return is exactly the state of the objects the sweep targets: a valid `xl.meta` with all shards present plus a leaked pre-#3510 data dir needs no shard healing, so a healthy heal returned before reclaim and swept nothing. On a healthy deployment (single node, no degraded disks) the reclaim was therefore dead code — an admin heal walked the objects, "healed" them, and reclaimed no leaked space. Run the best-effort reclaim on the `disks_to_heal_count == 0` path as well, gated on `!opts.dry_run`. The shared match+log block is factored into `reclaim_orphan_data_dirs_best_effort` so both exits behave identically. A reclaim failure still never fails the heal. Adds an end-to-end regression: put a healthy non-inline object, plant an unreferenced UUID data dir under it on every disk that holds the object, then drive `heal_object`. A dry-run heal must leave the stray in place; a real heal must reclaim it while preserving the live data dirs, `xl.meta`, and object contents. The test fails against the pre-fix control flow. Refs #3231, #3191, #4356. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
418f505a81 | fix(ecstore): allow concurrent directory scans (#4778) | ||
|
|
3b139e5267 |
fix(obs/cleaner): harden log cleaner durability, symlink safety, and retention (audit OLC-01..14) (#4776)
* fix(obs/cleaner): fsync archive and log dir before deleting source logs OLC-01: the compression path flushed the BufWriter but never synced the archive data or the parent directory before renaming, and the source was then unlinked with no durability barrier. A crash after rename but before the page cache reached disk could leave a truncated/zero-length archive while the source was already gone — permanent log/audit data loss. Because the archive is always renamed to a brand-new name (guaranteed by the existing exists() guard), ext4 auto_da_alloc does not mask this. Hand the underlying File back from the writer closure, sync_all() it before rename, fsync the parent directory, and fsync the log directory after the unlinks so a delete cannot be reordered ahead of the archive it justified. Guard the temp file with an RAII cleanup so an early return or panic cannot leak a *.tmp orphan. Ref: rustfs/backlog#1194 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * fix(obs/cleaner): create compression temp file with O_EXCL and O_NOFOLLOW OLC-03: the temp archive was opened with File::create at a predictable `<source>.gz.tmp` path with no O_EXCL/O_NOFOLLOW, so an actor with write access to the log directory could pre-plant that path as a symlink and have the compressor follow it — truncating and overwriting an arbitrary external file, then chmod-ing it to the source log's mode. This mirrors the symlink refusal already enforced on the deletion path (secure_delete). Route temp creation through create_tmp_archive(), which uses create_new (O_CREAT|O_EXCL) to refuse a pre-existing entry and, on Unix, O_NOFOLLOW to refuse a symlink at the final path component. Ref: rustfs/backlog#1196 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * fix(obs/cleaner): create compression temp file with restrictive mode OLC-06: File::create left the temp archive world-readable (0644 & ~umask) for the entire duration of compressing a large log, exposing the full plaintext of a possibly-0600 audit log on shared hosts until the mode was copied only after the write completed. Pass the source mode into create_tmp_archive and open the temp file with it (default 0600) so it is restrictive from creation; the post-write chmod still tightens/matches the source mode exactly. Ref: rustfs/backlog#1199 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * fix(obs/cleaner): validate existing archive before skipping recompression OLC-02: the idempotency guard used Path::exists() (which follows symlinks) and trusted whatever it found, then the caller deleted the source. A planted `<archive>.gz` symlink, or a zero-length/truncated archive left by a crashed run (OLC-01), would green-light deleting the source with no valid backup — data loss / log destruction. Replace exists() with symlink_metadata (no follow) and only treat the entry as a completed prior result when it is a regular, non-empty file whose header matches the codec magic (gzip 1f 8b / zstd 28 b5 2f fd). Anything else falls through to recompression, whose atomic create_new+rename replaces the bad entry (a symlink is replaced, never followed or deleted through). Ref: rustfs/backlog#1195 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * fix(obs/cleaner): stop dry-run overstating reclaimed bytes for compression OLC-08: in dry-run, compress_with_writer returned output_bytes = 0, so projected_freed_bytes = input and delete_files reported the full input as freed. A real run keeps the archive on disk (freed = input - archive), so dry-run overstated reclaim by the whole archive footprint. Estimate the archive with a deliberately conservative ratio so the projection never exceeds what a real run reclaims. Ref: rustfs/backlog#1201 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * fix(obs/cleaner): make freed-byte accounting resilient; document steal metric OLC-12: input/output byte sizes were read via metadata().unwrap_or(0), which silently reports 0 on failure and skews freed-byte metrics (input - 0 = full input, overstating reclaim). Use the copy() byte count as the authoritative input size and, when the archive metadata read fails, conservatively assume no savings instead of 0. Also document that the steal_success_rate counts only victim steals (batch = one success), so it reads as a relative rebalancing signal, not absolute task acquisition. Ref: rustfs/backlog#1205 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * fix(obs/cleaner): preserve level-0 semantics, allow zstd 22, log effective levels OLC-09: build() and the codec calls clamped gzip/zstd levels to [1,9]/[1,21], silently rewriting gzip level 0 (store) and zstd level 0 (codec default) to 1 and blocking the legal zstd maximum of 22. Clamp to [0,9]/[0,22] so those meanings survive, and echo the effective (post-clamp) levels in the startup log via new effective_gzip_level()/effective_zstd_level() getters so the log matches what actually runs. Ref: rustfs/backlog#1202 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * fix(obs/cleaner): bound compressed archives by byte cap; warn on retention=0 OLC-04: archive expiry was gated on compressed_file_retention_days > 0, so retention=0 disabled it entirely while compression kept producing archives, and max_total_size_bytes only ever bounded uncompressed logs — unbounded disk growth. Replace select_expired_compressed with select_archives_to_delete, which applies age expiry (when retention is on) and, regardless of retention, trims the oldest archives until the set fits under max_total_size_bytes. Also warn at startup when compression is on with retention=0 so the "keep forever" semantics are not mistaken for "delete immediately". Ref: rustfs/backlog#1197 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * fix(obs/cleaner): warn on invalid exclude glob instead of dropping silently OLC-05: build() dropped unparseable exclude globs via filter_map(...ok()), so a typo (or a literal comma splitting a char-class in the config string) turned "protect this file" into "delete this file" with no signal. Log a warning per rejected pattern with the raw string and parse error so the misconfiguration is visible. Ref: rustfs/backlog#1198 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * perf(obs/cleaner): backoff idle workers, cap worker count, lower small-host floor OLC-11: the work-stealing loop re-spun on Steal::Retry with no yield and used yield_now on the empty path, burning CPU during redistribution windows; worker_count had no upper bound so a mis-set parallel_workers over a directory of thousands of logs could spawn thousands of threads; and default_parallel_workers forced >=4 workers even on 1-2 vCPU hosts. Use crossbeam_utils::Backoff (spin->yield, reset on work) on the idle paths, clamp worker_count to MAX_PARALLEL_COMPRESS_WORKERS, and lower the default floor to 1. Ref: rustfs/backlog#1204 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * fix(obs/cleaner): warn when active-file guard is disabled by empty filename OLC-13 (defense-in-depth): the scanner protects the live log purely by exact filename equality against active_filename. An empty active_filename silently disables that protection, so a non-empty file_pattern could make the live log a deletion candidate via the public builder. Warn in build() when that unsafe combination is configured. The audit's "never delete the newest match" structural guard is intentionally not implemented: it would conflict with the legitimate keep_files=0 semantics (purge all rotated logs). The naming contract is instead locked by regression tests (OLC-14). Ref: rustfs/backlog#1206 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * fix(obs/cleaner): warn on unknown algorithm/match_mode, echo match_mode OLC-07: from_config_str silently fell back to defaults for unrecognized compression algorithm and match mode (any non-"prefix" value became Suffix), hiding operator typos like "prefixx" that could make the cleaner match no rotated logs. Warn on a non-empty unrecognized value in both parsers, and echo the resolved match_mode in the startup log alongside the algorithm. Ref: rustfs/backlog#1200 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * fix(obs/cleaner): derive orphan .tmp suffixes and exempt them from min age OLC-10: orphan `*.gz.tmp`/`*.zst.tmp` cleanup was gated by min_file_age_seconds (default 3600), so crash-left orphans lingered up to an hour, and the tmp suffix list was hardcoded rather than derived from compressed_suffixes() — a new codec would leave `*.<ext>.tmp` orphans the scanner never recognizes. Derive the temp suffix from CompressionAlgorithm::compressed_suffixes(), and gate orphan removal on a small fixed grace window instead of min_file_age (orphans are never live-written after the rename that would promote them). Ref: rustfs/backlog#1203 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * test(obs/cleaner): cover symlink, archive expiry, idempotency, and edge cases OLC-14: add regression tests for the previously-untested safety/correctness branches — symlink rejection (external target never deleted), archive age expiry vs fresh retention, archive byte-cap trim with retention disabled, gz/zst classification, max_single_file_size selection, min_age protecting a fresh non-empty log, active-file exclusion when the active name also matches the pattern, invalid exclude glob not aborting build, dry-run + compression creating no archive, gzip round-trip validity, and the idempotent-archive branch trusting a valid prior archive. Ref: rustfs/backlog#1207 (audit rustfs/backlog#1193) Co-Authored-By: heihutu <heihutu@gmail.com> * style(obs/cleaner): apply rustfmt and collapse nested if (clippy) Formatting-only cleanup over the audit fix series: rustfmt normalization of the multi-line expressions introduced in compress.rs/core.rs, plus collapsing the delete_files directory-fsync into a single let-chain to satisfy clippy::collapsible_if. No behavior change. Co-Authored-By: heihutu <heihutu@gmail.com> * refactor(obs/cleaner): collapse redundant source stats in compression path The per-issue fixes to compress_with_writer accumulated three metadata() syscalls on the source in the real compression path: an input_bytes read that was immediately shadowed by the copied byte count (a dead read), a source_mode read (OLC-06), and the pre-OLC-06 post-write chmod re-reading the same mode. Collapse to a single fd-based read — move the dry-run input_bytes read into the dry-run branch, read source_mode from the already-open fd (no path stat, no TOCTOU), and reuse it for the post-write chmod. Behavior is unchanged (same inode's mode, written for input size); 3 source stats -> 1. Co-Authored-By: heihutu <heihutu@gmail.com> * chore(obs/cleaner): fix typo flagged by CI (mis-set -> misconfigured) Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
13bdca6762 |
build(toolchain): switch Rust channel to stable (#4775)
* Change Rust toolchain channel to stable Signed-off-by: houseme <housemecn@gmail.com> * style: apply clippy --fix and cargo fix lint suggestions Run `cargo clippy --fix --all-targets --all-features` and `cargo fix --lib --all-targets` across the workspace, then resolve the remaining warnings by hand: - collapse needless borrows in `format!` args, prefer `?` over explicit early returns, and use `.values()` / `.flatten()` iterator adapters - rewrite the `Md5` scan loop via `manual_flatten` and re-indent the `select!` macro body (rustfmt skips macro interiors) - annotate the intentional dead-code `Md5` inherent methods (constructed only by the test factory) with `#[allow(dead_code)]` Behavior is unchanged. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Signed-off-by: houseme <housemecn@gmail.com> Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
d8e69a3adf |
fix(logging): enforce single-writer sinks and bound tracing (#4765)
* fix(obs): prevent rolling log stdout aliasing * fix(ecstore): bound hot-path tracing payloads * test(logging): guard service and disk log invariants * fix(obs): silence useless_conversion on st_dev for Linux clippy rustix's Stat.st_dev is u64 on Linux/glibc, making u64::try_from a no-op that trips clippy::useless_conversion under -D warnings. The conversion is still needed on macOS/BSD where st_dev is a signed dev_t, so suppress the lint on that line rather than dropping the portable fallible conversion. * fix(logging): keep stdout sink validation portable (#4769) * fix(obs): keep stdout device conversion portable * docs(logging): update single-writer plan status --------- Co-authored-by: overtrue <anzhengchao@gmail.com> Co-authored-by: houseme <housemecn@gmail.com> |
||
|
|
2ddafb4ed9 |
test(ecstore): bound file sync probe waits (#4767)
Co-authored-by: Zhengchao An <anzhengchao@gmail.com> |
||
|
|
4c9431704c | fix(ecstore): cancel orphaned listing walks (#4773) | ||
|
|
e3533a4611 |
test(replication): cover version deletion convergence (#4764)
test(replication): cover version delete convergence |
||
|
|
5a4cf1d4b5 |
fix: repair flaky moka clear drain loop from #4759 (#4763)
fix(cache): drain pending removals until entry_count reaches zero in clear() The previous clear() implementation used a single run_pending_tasks() call after invalidate_all(), which was insufficient under concurrent fill pressure — moka processes invalidations lazily in batches, so entries can linger after a single maintenance pass. Replace the fixed single call with a drain loop (up to 256 rounds) that calls run_pending_tasks() and yields between iterations until entry_count() reaches zero. This ensures the concurrency storm test (moka_backend_concurrency_storm_leaves_no_leaked_state) passes reliably. The earlier fix (#4759) added a pre-invalidate_all drain and an 8-pass loop but reordered operations in a way that introduced a new race. This commit keeps the original invalidate_all-first ordering and only adds the drain loop after the initial run_pending_tasks() call. |
||
|
|
631d93092e |
docs(obs): align OtelConfig doc defaults with actual constants (#4760)
Six doc-comments on `OtelConfig` fields stated defaults that disagreed with the constants the runtime actually applies in `extract_otel_config_from_env`. Correct them to match: - profiling_export_enabled: false -> true (DEFAULT_OBS_PROFILING_EXPORT_ENABLED) - sample_ratio: 0.1 -> 1.0 (SAMPLE_RATIO) - meter_interval: 15 -> 30 (METER_INTERVAL) - logger_level: info -> error (DEFAULT_LOG_LEVEL) - log_rotation_time: daily -> hourly (DEFAULT_LOG_ROTATION_TIME), and document the full minutely/hourly/daily set plus the daily fallback - log_cleanup_interval_seconds: 21600 -> 1800 (DEFAULT_OBS_LOG_CLEANUP_INTERVAL_SECONDS) Doc-comment only; no behavior change. |
||
|
|
3832f1d270 | fix(cache): drain pending removals during clear (#4759) | ||
|
|
b540c7e2d0 | test(ecstore): cover list marker key stripping (#4757) | ||
|
|
a5765274fc |
fix: auto-repair test_rename_data_shares_file_sync_limit hang on macOS (#4758)
fix(test): use canonicalized disk root for file_sync_probe in rename_data test On macOS, tempfile::tempdir returns /var/folders/... while LocalDisk resolves the root to /private/var/folders/... via dunce::canonicalize. The file_sync_probe::enter() path check uses starts_with(), so passing the non-canonical tempdir path caused the probe to never activate, making wait_for_active() hang indefinitely. Use disk.root (already canonicalized) for the probe instead. |
||
|
|
676f2276b4 |
fix(replication): refresh targets after site endpoint edits (#4756)
* fix(replication): refresh targets after site endpoint edits * fix(replication): serialize site bucket lifecycle |
||
|
|
af831bde4b | fix(cache): drain entries before clear returns (#4751) | ||
|
|
5088a6cde4 |
perf(obs): trim per-collection-cycle waste in report_metrics (#4748)
`report_metrics` runs on every metrics collection cycle and did three things it did not need to, for every metric, every cycle: - interned `metric.name`/`metric.help` through a `Mutex<HashMap>` even when the `Cow` was already `Borrowed(&'static str)` (the common case for statically named metrics); - re-ran `describe_*!` (which re-locks the recorder's metadata map) although the metadata never changes; - allocated a fresh `Vec<(String, String)>`, cloning every label key and value, even though `metric.labels` is already `[(&'static str, Cow<'static, str>)]`. Now: names/help resolve to `&'static str` without touching the intern cache when already borrowed; each metric is described once (tracked in a `HashSet`); and the recorder is fed `&metric.labels` directly, removing the per-cycle label clone. No metric names, help, label keys/values, or emitted values change. Verified by building rustfs-obs and running the report unit tests (the `metrics` macro accepts the borrowed label slice directly). Addresses rustfs/backlog#1185 (P3, report path). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
6886dca7d1 |
feat(ecstore): make GET codec-streaming a single rollout switch (backlog#1183) (#4752)
backlog#1183, staged rollout step. Simplify enabling the zero-duplex GET
codec-streaming fast path to a single switch — RUSTFS_GET_CODEC_STREAMING_ROLLOUT
(default "off") — now that body/header parity is proven (parity e2e net + bench
A/B on backlog#1183).
- Add a clean production rollout token "on" (aliases "full"/"production"); the
legacy "internal"/"benchmark" tokens remain accepted.
- Flip DEFAULT_RUSTFS_GET_CODEC_STREAMING_ENABLE and the two ..._COMPAT_CONFIRMED
defaults to true. They are retained as emergency kill-switches (set any to
false to force the fast path off) but no longer gate enablement — the rollout
switch does.
No production behavior change: with no env set the rollout switch defaults to
"off", so GET stays on the legacy duplex path exactly as before. Flipping the
hard default to on is deferred to a follow-up after a production soak.
Note (intentional semantics change): with the rollout switch opted in
("on"/"internal"/"benchmark"), codec streaming now activates without also
setting the two ..._COMPAT_CONFIRMED vars — compatibility is confirmed, so those
confirmations are baked in.
Co-authored-by: heihutu <heihutu@gmail.com>
|
||
|
|
028ba6a675 |
perf(ecstore): parallelize multipart shard syncing (#4734)
Bound large shard-directory syncs per disk and process while preserving small-directory and durability behavior. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
|
633c131cef |
perf(io-metrics): cache handles for hot label-less metric emitters (#4750)
The `metrics` macros re-run the recorder's `register_*` on every emission — a `RwLock` read, a name-key hash, and an `Arc` clone — even for a metric that never varies its key. For the hot, label-less recorders on the per-IO path (`record_data_transfer`, `record_io_latency`, `record_io_latency_p95/p99`, `record_io_queue_congestion`) that lookup is pure overhead once observability is on. Add `counter_increment_cached!` / `gauge_set_cached!` / `histogram_record_cached!` that resolve the handle once via `LazyLock` in production and reuse it. Under `cfg(test)` they re-resolve on every call, because the `metrics` crate resolves against a thread-local recorder that `with_local_recorder` swaps per test — a process-global cached handle would bind to whichever recorder was active first and break test capture. The macros only wrap FIXED (label-less) keys, and the `metrics_enabled()` gate still short-circuits before any emission when disabled. Verified: the only callers of these functions are the collector (io-metrics' own cfg(test) tests, which re-resolve) and production code; no cross-crate test captures them. rustfs-io-metrics builds on both cfg paths, 147 unit + 4 doctests pass, clippy clean. Addresses rustfs/backlog#1185 (P3, per-emission handle caching). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
89557a7ffe |
perf(ecstore): cache io_uring fallback root label (#4747)
`record_uring_fallback` is called at multiple sites in the io_uring read path whenever a read falls back to `StdBackend`. It formatted `self.root.display().to_string()` on every call, heap-allocating a `String` from a `Path` that never changes after construction — pure per-read waste when io_uring is degraded. Cache the label once in `UringBackend::try_new` as a `String` field (`root_label`) and clone it per emission. The metric name and the `"root"` label value are unchanged; only the redundant `Path` formatting is removed. The clone is a single alloc of an already-short string. Refs: rustfs/backlog#1185 Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
5282c71f86 |
perf(io-metrics): make internode peer-health read-mostly with an RwLock (#4746)
`cluster_peer_should_bypass` is called before every internode RPC when the offline-bypass feature is enabled (remote disk `get_client`, remote locker), and it took a single process-global `Mutex<HashMap>` even for the overwhelmingly common case of an online or unknown peer, which is read-only. That serialized all concurrent internode client acquisitions on one lock. Switch `CLUSTER_PEER_HEALTH` to a `RwLock`: - the hot check takes a shared read lock and returns immediately for unknown or online peers, so concurrent RPCs no longer serialize; - only an offline peer (rare) drops to the write lock to record a re-probe, with a re-fetch/re-check because the state can flip back online between releasing the read lock and taking the write lock; - the write paths (dial reachable/unreachable) and the read-only `cluster_peer_is_offline` move to `write()`/`read()` accordingly. Behavior is unchanged on every path (online/unknown -> not bypassed; offline -> bypass with one re-probe per interval); `Instant::now()` now runs only on the offline path. Poison recovery is preserved. All internode unit tests pass. Addresses rustfs/backlog#1185 (P2). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
53bd4bb300 |
test(e2e): pin GET codec-streaming body/header parity vs legacy duplex (backlog#1183) (#4745)
backlog#1183 tracks flipping the default GET data path from the legacy tokio::io::duplex double-copy to the zero-duplex codec-streaming fast path. That flip is gated behind RUSTFS_GET_CODEC_STREAMING_BODY_COMPAT_CONFIRMED / ..._HEADER_COMPAT_CONFIRMED, which need empirical evidence the two paths are byte- and header-identical. Add an e2e regression net that runs the same object matrix twice against the same on-disk EC shards, changing only the codec-streaming env gates: phase A (default) takes the legacy duplex path, phase B (gates opened) takes codec streaming. It asserts byte-for-byte (sha256) and header-for-header equality across inline / small / multi-block (1.5M/3M/5M+) / multipart objects, plus a ranged GET (which falls back to legacy by gate design). Path confirmation is not assumed: the legacy path logs "Created duplex pipe ..." per full GET, so the test counts that marker per phase and asserts the codec phase created zero duplex pipes, proving the fast path actually ran rather than silently falling back to the path it is compared against. To capture the child server's logs for that assertion, add an optional RustFSTestEnvironment.capture_log_path (default None = inherit stdio, backward compatible) that redirects the spawned server's stdout+stderr to a file. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
763f246f8a |
test(ecstore): add shared MockWarmBackend test utility for lifecycle and tier tests (#4716)
* test(ecstore): extract shared MockWarmBackend into a test-util feature (backlog#1148 ilm-6) The tier/lifecycle integration tests carried two byte-for-byte copies of an in-memory WarmBackend mock — one in crates/scanner/tests and one in rustfs/src/app — plus duplicated register_mock_tier and polling helpers. Both implemented the same ecstore WarmBackend trait. Consolidate them into ecstore behind a new `test-util` feature, exposed via the `rustfs_ecstore::api::tier::test_util` facade: - MockWarmBackend: in-memory WarmBackend with an operation log (for ordering assertions such as "local delete precedes remote remove") and fault injection (FaultConfig): unreachable, HTTP 5xx, credential rejection, injected latency, plus external_remove to simulate an out-of-band remote deletion. - register_mock_tier / register_mock_tier_backend: register the mock into any TierConfigMgr handle (the global manager used by scanner tests or a per-instance one used by the app tests). - xl.meta transition assertion helpers: read_transition_meta, assert_transition_meta_consistent (cross-shard consistency of the status/tier/remote-key/remote-version-id tuple plus free-version count), and free_version_count. - polling helpers: wait_for_remote_absence, wait_for_object_count, wait_for_free_version_absence. Both existing copies now consume this single definition; `rg 'struct MockWarmBackend'` collapses to one. The feature is enabled only from [dev-dependencies], so it never links into the production binary (resolver 3). Designed for downstream ilm-8 (restore lifecycle) and ilm-11 (tier fault injection matrix). Coordinates with #4706 (ilm-2), which adds op-logging to the scanner mock — that op-logging is now part of this shared surface, so #4706 should rebase onto it. Refs rustfs/backlog#1148 (ilm-6), rustfs/backlog#1155. * test(ecstore): fix shared MockWarmBackend usage after main merge - Access stored objects via MockWarmBackend::contains() instead of the now private inner objects map (fixes E0609 after the shared test-util refactor). - Drop dead ReadCloser/ReaderImpl/DiskAPI imports and the unused transition_api test re-exports the mock extraction left behind. - Reword the scanner/rustfs test-util dependency comments so they no longer embed the literal rustfs_ecstore:: path that trips the ECStore architecture-migration guard. |
||
|
|
ce7d3119b2 |
perf(io-metrics): throttle collector percentile sort off the per-IO path (#4744)
`MetricsCollector::record_io_operation` recomputed P50/P95/P99 on every disk IO by taking a read lock, collecting up to `max_latency_samples` (1000) into a `Vec<u128>` and fully sorting it — an O(n log n) alloc+sort per operation on the GET read path (gated by stage metrics). Both consumers sample only periodically: the autotuner reads `avg_io_latency_us` on its tuning tick, and the P95/P99 values are never read internally — they only feed OTEL export. Nothing needs per-IO freshness. Split the two costs: - The window mean (stored in `avg_io_latency_us`, read by the autotuner) is now maintained in O(1) via a running sum kept in step with the window's push/pop, and refreshed on every op — no sort, no warm-up regression. - The P95/P99 sort is throttled to once per `PERCENTILE_RECOMPUTE_INTERVAL` (128) operations. A bounded lag is invisible to the periodic autotuner tick and OTEL export while the per-op sort cost is amortised ~128x away. Semantics are otherwise unchanged: same sliding window, same percentile indices, same mean value. Tests updated — the percentile test forces a recompute to exercise the math directly, and a new test asserts the mean tracks every op while the percentiles only recompute at the interval boundary. Addresses rustfs/backlog#1185 (P1). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
96e5699568 |
test(ilm): self-managed lifecycle expiry e2e + backdated-mtime helper (backlog#1148 ilm-3) (#4710)
Convert the two reliant lifecycle expiry tests from #[ignore] + a hardcoded localhost:9000 server to self-managed RustFSTestEnvironment tests (random port, isolated temp dir) and wire them into the PR e2e-smoke subset. Time control is chosen per test and documented in-module: - test_lifecycle_expiry_backdated_mtime: mod_time back-dating via the internal source-replication backdoor (new put_object_with_backdated_mtime helper) so a Days=1 rule is already due, with RUSTFS_ILM_PROCESS_TIME=1 shrinking the rounding boundary. Asserts the backdated matching-prefix object expires and a non-matching-prefix object with the same backdating survives (isolates the prefix filter). Proves the backdoor does not trip the replication gate. - test_lifecycle_versioned_current_version_expiry_creates_delete_marker: RUSTFS_ILM_DEBUG_DAY_SECS=2 (ilm-5) accelerates the day length; asserts a latest delete marker is created, the original data version is retained and still readable by version id. - test_lifecycle_zero_day_expiry: Days=0 immediate expiry; matching object deleted, non-matching survives. All three use RUSTFS_SCANNER_CYCLE=1 so the scanner runs every second; tests poll for the terminal state instead of sleeping fixed wall-clock. Ignore count for reliant/lifecycle.rs goes 2 -> 0. CI wiring: added a distinct 'test(/^reliant::lifecycle::/)' clause to the existing profile.e2e-smoke default-filter in .config/nextest.toml (the single sanctioned e2e wiring mechanism per crates/e2e_test/README.md; no new e2e job). Refs rustfs/backlog#1148 (ilm-3), rustfs/backlog#1155 |
||
|
|
264b2dd480 |
perf(metrics): drop needless per-emission work on hot metric paths (#4743)
* perf(metrics): drop needless per-emission work on hot metric paths Audit of the metrics hot paths surfaced four low-risk wins where emission did work it did not need to: - `record_file_cache_reclaim_success/error` (disk/local.rs) called `.to_string()` on `kind` (already `&'static str`) and on the `"ok"`/`"err"` literals, heap- allocating up to four `String`s per page-cache reclaim window — which runs per read-stream reclaim. The `metrics` macros accept `&'static str` label values directly, so pass them as-is. - `record_read_repair_dedup` (set_disk/core/io_primitives.rs) likewise `.to_string()`-ed an already-`&'static str` `reason`. - `SetDisks::get_object_reader` (set_disk/ops/object.rs) captured `Instant::now()` and emitted the `rustfs.lock.acquire.*` counter and histogram unconditionally on every GET, right beside an already-gated stage timer. Gate them behind `get_stage_metrics_enabled()` too, so an inactive observability config pays no per-GET clock read or recorder lookups. - The per-response-body-chunk counter in server/http.rs re-ran the `counter!` registry lookup on every chunk (a streamed GET emits many). Resolve the label-less handle once into a `LazyLock<metrics::Counter>`; the global recorder is installed at startup before any response streams, so the cached handle binds to the final recorder. No metric names or label values change. The only behavior change is that the `rustfs.lock.acquire.*` GET-path metrics now follow the GET stage-metrics flag, consistent with the neighbouring stage timings. Co-Authored-By: heihutu <heihutu@gmail.com> * perf(metrics): gate page-cache reclaim metrics behind metrics_enabled() `record_file_cache_reclaim_success/error` run per read-stream reclaim window on large-object reads and emitted unconditionally. When general metrics are disabled the `counter!`/`histogram!` macros still construct three metric keys per call for nothing. Skip the emission behind `rustfs_io_metrics::metrics_enabled()`, matching how the io-metrics free functions self-gate. The serial reclaim-metrics test now enables the flag (save/restore) alongside the existing stage gate. Left ungated deliberately: `record_read_repair_dedup` (rare read-repair path, and its non-serial test would need a global-flag toggle), and the HTTP body-chunk counter (its cached handle already makes the disabled case a no-op increment). Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
e742a540a4 |
test(cache): guard the body-cache eligibility gate against deny-list regressions (#1146) (#4742)
`full_object_plaintext_len` decides whether a body-cache hook hit may serve bytes in place of the erasure read. It is a fail-closed allow-list: it excludes every read whose `ReadPlan::build` applies some other transform (ranged/part, raw/data-movement, restore, encrypted, remote) with an early `return None`, then returns a `Some(..)` length only for the whole-plaintext cases. A newly added `ReadPlan` branch that nobody teaches this gate about falls through to `None` and safely bypasses the cache. Flip it to a deny-list and the same new branch silently serves bytes in the wrong representation — the backlog#1108 / #1109 / #1146 class of bug. The existing unit and e2e tests only cover the branches that exist today. This adds `scripts/check_body_cache_whitelist.sh`, a structural guard wired into pre-commit / pre-pr / dev-check and CI, that asserts every exclusion predicate and a `return None` still precede the first `Some(..)`. Reordering a predicate, dropping one, moving the positive return ahead of the gate, or renaming/removing the function all fail; wording, formatting, and adding a new exclusion in the same gate do not. Mutation-tested against all four regression shapes. This machine-enforces the structural invariant that backlog#1146 was kept open to guard by hand. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
1ab66e124c |
test(ecstore): pin the remaining io_uring fd-cache invariants (#1180) (#4739)
Close the three test-completeness items in rustfs/backlog#1180 that the earlier hardening (rustfs/rustfs#4726, #4729) left unpinned; the other two of the five (sharded cancel routing, bailout-handle error) already landed in rustfs/uring. - rename_data end-to-end invalidation: drive the real `LocalDisk::rename_data` commit (non-inline part, so `invalidate_part_paths` is non-empty) and assert the destination part descriptor is dropped — not merely `rename_file`/`delete`. Both production `rename_data` call sites share this invalidation, so a "fix one copy, miss the other" regression is now caught. The test is non-vacuous: it seeds the cache, removes the on-disk data dir out of band (the cached fd keeps the old inode alive and clears the path for the directory rename `rename_data` performs), and asserts a read still returns the OLD bytes before the commit — which fails outright if the cache is off, so it cannot pass without a live cache. - FD_CACHE_TTL backstop: an injected short TTL proves the cache self-evicts a descriptor with no explicit invalidation; a static check pins the 5s value. - zero-length read bounds parity on the cache-HIT path: a `length == 0` read past EOF must be rejected identically to the miss path and StdBackend, pinning the #1173 fix against regression. Refactors `FdCache::new` to delegate to a private `with_ttl(ttl)` helper so the TTL backstop can be exercised with a short TTL instead of a multi-second wait. Verified: `cargo test -p rustfs-ecstore` on Linux with real io_uring (seccomp=unconfined, RLIMIT_NOFILE raised, RUSTFS_URING_TESTS_MUST_RUN=1 so a degraded skip fails rather than passing vacuously). Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
793b2a06e2 | perf(ecstore): snapshot per-IO env config on local disk read/write paths (#4736) | ||
|
|
9d64d71bd7 | fix(ecstore): reduce GET reader setup shard fanout (#4735) | ||
|
|
a8eed1c44d |
perf(io-metrics): add a global switch to gate general metric emission (#4733)
The io-metrics crate already had per-stage GET/PUT switches, but the request headers/summaries and ~40 general recorders (I/O scheduler, bytes-pool, zero-copy, bandwidth, system-resource, error/timeout/retry) emitted unconditionally, paying their label allocations and arithmetic even when no metric recorder is installed. Add `METRICS_ENABLED: AtomicBool` (default false) with `set_metrics_enabled()` / `metrics_enabled()`, isomorphic to the existing stage switches, and gate: - GET headers/summaries via `get_stage_metrics_enabled()`: record_get_object_request_start / _request_started / _request_result / _timeout / _completion / _total_duration_with_path, plus legacy record_get_object. - PUT headers via `put_stage_metrics_enabled()`: record_put_object_request_start / _request_result, plus legacy record_put_object. - 41 general recorders via `metrics_enabled()`. Startup enables all three switches together under `observability_metric_enabled()` (startup_observability.rs), with a passthrough in startup_runtime_sources.rs. The global recorder is itself only installed when observability metric export is on, so gating off is pure cost savings when export is disabled and a no-op when it is enabled. Deliberately NOT gated (would break function, not just observation): the EC encode in-flight and GET buffered-bytes accounting guards (add/remove_ec_encode_inflight_bytes, track_get_object_buffered_bytes) whose values are read back by current_*; and the stateful sibling modules (lock/deadlock/backpressure/autotuner/adaptive_ttl/collector/internode). The console realtime metrics endpoint (/admin/v3/metrics) samples system state and internode metrics directly, so it does not depend on the gated recorders. Adds a toggle test exercising both the enabled and disabled paths, serialized on the existing METRICS_FLAG_LOCK to stay robust under nextest's shared-process model. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
c79366d42c |
refactor(ecstore): tidy io_uring read backend (docs + fd-cache handle) (#4732)
Two behavior-preserving cleanups to the io_uring local read backend that accumulated as later work layered onto it: - Reunite the `UringBackend` struct doc comment. The backlog#1145 fd-cache constants were inserted into the middle of the struct's doc block, so its opening sentences were orphaned onto `ENV_RUSTFS_IO_URING_FD_CACHE` and the struct itself was documented by a sentence fragment starting mid-clause with "through rustfs-uring's...". Move the opening back onto the struct and give the constant its own one-line doc. - Fold the fd-cache handle and its lookup key into a single `Option<(&FdCache, FdKey)>` in `pread_uring`, so the get and the insert_if_fresh sites stop re-deriving `self.fd_cache.as_ref()` and re-matching presence. Semantics are identical, including the backlog#1176 generation guard (`gen_at_open` snapshot before open, insert only when the generation is unchanged). No functional change. The borrow/move pattern was validated against a host-compilable reduction (the cfg(linux) path cannot be built from macOS); CI compiles the Linux backend. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
4f29dcdcec |
chore(deps): bump rustfs-uring to 0.2.1 (#4731)
chore(deps): bump rustfs-uring to 0.2.1 from crates.io rustfs-uring 0.2.1 routes the driver's runtime diagnostics through `tracing` with structured fields instead of `eprintln!` (rustfs/uring#13). No public API change — UringDriver::probe_and_start_sharded, read_at, and read_at_direct keep their signatures — and the read path / cancel-safety ownership model are untouched, so this is a drop-in patch bump of the version requirement plus the lockfile entry. 0.2.1 adds a `tracing` dependency; it is recorded in the rustfs-uring lock entry. tracing is already in the workspace graph, so nothing new is pulled in. The Cargo.lock change is scoped to the rustfs-uring package only; unrelated lockfile drift is left for its own change. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
ffca98cdbf |
fix(ecstore): harden io_uring integration (#4726)
* fix(ecstore): close the fd-cache open-then-insert race with a generation guard (rustfs/backlog#1176) pread_uring's miss path opened a descriptor on the blocking pool and only then inserted it into the moka cache. moka's invalidations cover only entries present at call time, so a heal/delete commit that invalidated between the open and the insert could not stop the just-opened stale inode from being cached afterwards — serving the pre-heal/pre-delete inode for up to the TTL and defeating the heal. Add an invalidation generation to FdCache, bumped by invalidate_exact and invalidate_under before they touch moka. The read path snapshots the generation before opening and inserts via insert_if_fresh, which refuses the insert if the generation moved during the open and, with a post-insert re-check, removes the entry if an invalidation raced the insert itself. Reads that never miss are unaffected. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): invalidate the fd cache on the primary object-delete paths (rustfs/backlog#1175) The fd-cache invalidation contract was only wired into DiskAPI::delete, rename_file and rename_data, but object deletion almost never goes through LocalDisk::delete — DeleteObject(s) reach delete_version, delete_versions -> delete_versions_internal, and delete_paths, all of which remove a version's data dir (move_to_trash / rename_all staging) with no invalidation. A cached io_uring descriptor kept the deleted part.N inode readable for up to the TTL, so a GET in that window could still return deleted data. Invalidate every cached fd under the removed data dir at each site: in delete_version and delete_versions_internal the data_dir uuid and object path are in hand (invalidate_cached_fds_under(volume, "{path}/{uuid}")); delete_paths invalidates under each removed path. A later rollback that restores a data dir just causes the next read to re-open it. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): close remaining fd-cache invalidation gaps (rustfs/backlog#1177) Three residual paths could keep serving a stale descriptor: - delete_volume removed the whole bucket tree (remove_dir_all/remove_dir) with no invalidation, and the cache-hit read path skips the volume-access check, so a cached fd kept a removed object readable. Add invalidate_cached_fds_for_volume (a per-volume moka predicate) and call it after the bucket is removed. - A retired LocalDisk instance (renew_disk on reconnect builds a fresh one) kept its populated cache alive while still referenced by in-flight ops, so invalidations through the new instance never reached it. close() now clears the backend's cache via clear_cached_fds. - rename_data's post-commit rollback (a commit-metadata fsync failure under strict durability) restored the old data dir without dropping fds cached during the committed window; the streaming branch now invalidates the dst part fds on those rollback paths. The inline branch's rollback runs inside spawn_blocking and is left to the TTL backstop. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): narrow the io_uring latch classes to match StdBackend (rustfs/backlog#1171) The runtime degradation classification reused the probe-time restriction errnos, which the driver's C7 contract explicitly warns against, so a single per-file error could latch a whole disk off io_uring: - is_io_uring_unsupported no longer includes EACCES: at read time on an already-open fd it is per-file (an LSM hooks security_file_permission on every read) and StdBackend hits the same denial, so falling back masks nothing and a full-disk latch would be wrong. ENOSYS and EPERM (seccomp/LSM applied after startup) remain. EOPNOTSUPP is now classified per-path by the caller. - pread_uring_direct's read-error arm now mirrors StdBackend: an O_DIRECT-shape error (EINVAL/EOPNOTSUPP) latches only direct_uring.supported so eligible reads take StdBackend's aligned path, instead of over-latching the whole io_uring backend or never latching a read-side EINVAL at all. - try_new only negative-caches genuine restriction-class probe failures in URING_UNSUPPORTED_DISKS; an unexpected (possibly transient) probe failure now falls back without latching, so the next reconnect re-probes. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(ecstore): log when a disk latches io_uring off at runtime (rustfs/backlog#1172) A probe-gated gray release was flying blind: the permanent per-disk `active` latch flipped with no log and no metric, so the only message operators ever saw was the startup "io_uring read backend enabled" line — which stayed true on dashboards even after the very first read latched the disk back to StdBackend forever. Add latch_active_off, which flips the latch with `swap` and logs the true->false transition exactly once at warn with a dedicated event constant, disk root, and errno. Both the buffered and O_DIRECT read paths use it. A fallback/latch metric counter and periodic export of the driver StatsSnapshot (cq_overflow, cancel_already) remain as follow-ups that need rustfs_io_metrics plumbing. Co-Authored-By: heihutu <heihutu@gmail.com> * chore(audit): correct the stale rustfs-uring license-allow rationale (rustfs/backlog#1181) The dependency-review allow said rustfs-uring is "pulled as a git dependency", but ecstore now pins it from crates.io. Update the rationale and scope the allow to the exact pinned version (pkg:cargo/rustfs-uring@0.1.0) so a future version bump forces a conscious re-review of the license/provenance claim instead of being waved through on an outdated justification. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): offload io_uring driver teardown off the tokio worker (rustfs/backlog#1170) UringBackend held Arc<UringDriver> and had no Drop, so when the last LocalDisk reference dropped in async context (disk reconnect via renew_disk, or shutdown), UringDriver's own Drop ran on that thread — sending Shutdown and joining each shard thread, which can block up to the bounded-drain timeout (5s) on a hung / D-state disk, stalling a tokio worker. Wrap the driver in ManuallyDrop (deref is transparent, so read call sites are unchanged) and add a Drop that takes the Arc and, when a runtime is present, drops it on a blocking thread so the potentially-blocking join never runs on a runtime worker. Off-runtime it drops inline. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): restore StdBackend read parity on the uring paths (rustfs/backlog#1173) Two byte-for-byte parity breaks against StdBackend on the io_uring read paths: - A zero-length read on an fd-cache hit returned Ok(empty) without any bounds check, while StdBackend and the uring miss path return FileCorrupt for an offset past EOF. Fstat the cached descriptor on the length==0 path and match. - reclaim_read_range fadvise(DONTNEED)'d the raw unaligned [offset, offset+len) range, but fadvise only drops fully-covered pages, so the head partial page stayed resident — whereas StdBackend's mmap path reclaims the page-aligned superset. Bitrot shards' 32-byte block headers keep offsets off page boundaries, so this diverged on the common case. Page-align the reclaim window to match the mmap path exactly. (The third parity item from the audit — a failed reclaim fadvise failing the read — is already parity: StdBackend's mmap path propagates the same fadvise error with `?`, so no change is needed.) Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): bound worst-case in-flight memory by chunking huge uring reads (rustfs/backlog#1174) The driver's backpressure permits count operations, not bytes, and it zero-fills a full-size buffer per op, so a single unbounded read could pin ~length bytes per permit (128 permits x shards x up to ~2 GiB). ecstore passes a whole part's shard range as one pread_bytes with no upstream chunking. On the buffered path, split reads larger than URING_MAX_OP_LEN (128 MiB) into sequential chunks, awaited one at a time, so worst-case in-flight memory is bounded by permits x URING_MAX_OP_LEN per shard. The threshold is high enough that ordinary shard reads keep the single-op, zero-copy fast path unchanged. The O_DIRECT path (opt-in, alignment-constrained) is left for a follow-up. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): gate the io_uring fd cache on RLIMIT_NOFILE headroom (rustfs/backlog#1178) The fd cache holds up to FD_CACHE_CAPACITY (512) descriptors per disk, but try_new cannot know the disk count and nothing checked the process fd budget. On a bare-metal / non-systemd run with the common 1024 soft RLIMIT_NOFILE, two disks would already exhaust fds with EMFILE surfacing on reads and probes. Check the soft limit at try_new: enable the cache only with ample headroom (>= 16384), otherwise log a warning once and fall back to open-per-read. The packaged systemd unit sets 1,048,576, so tuned deployments are unaffected. Co-Authored-By: heihutu <heihutu@gmail.com> * test(ecstore): make io_uring test skips visible and gate non-vacuity (rustfs/backlog#1179) The ecstore io_uring tests degrade to a silent pass when io_uring is unavailable (bare `return`s or plain eprintlns), so a CI leg on a restricted runner never exercises the real UringBackend/FdCache/latch paths yet still goes green — an integration regression could merge unseen. Add uring_test_skip: it emits a grep-able `SKIP <name>` line and, when RUSTFS_URING_TESTS_MUST_RUN is set (a CI leg that guarantees io_uring, e.g. a seccomp=unconfined container), panics instead of skipping. Route the silent-skip sites through it. Wiring a dedicated CI leg that sets that env on a capable runner is tracked in the issue; this provides the enforcement mechanism. Co-Authored-By: heihutu <heihutu@gmail.com> * test(ecstore): cover delete_paths fd-cache invalidation (rustfs/backlog#1180) Add an end-to-end test that seeds the descriptor cache with a read, removes the part via disk.delete_paths (one of the primary object-delete entry points that does not go through LocalDisk::delete), and asserts the next read no longer returns the removed inode — pinning the invalidation added in #1175. The sharded cancel-routing half of #1180 is covered in the rustfs-uring PR. Co-Authored-By: heihutu <heihutu@gmail.com> * io_uring audit follow-ups: O_DIRECT chunking, inline invalidation, metrics, CI leg (backlog#1160) (#4729) * fix(ecstore): chunk large O_DIRECT reads too, bounding in-flight memory (rustfs/backlog#1174) The buffered read path already splits reads above URING_MAX_OP_LEN into sequential chunks; do the same for the O_DIRECT path, which was left for a follow-up. read_at_direct aligns each chunk's sub-range internally, and chunk sizes are a multiple of URING_MAX_OP_LEN so a boundary re-read is at most one block. Extract classify_direct_read_error so the single-op and chunked paths share one copy of the EINVAL/EOPNOTSUPP-vs-subsystem latch classification rather than duplicating it. Co-Authored-By: heihutu <heihutu@gmail.com> * fix(ecstore): invalidate cached fds on the inline rename_data rollback (rustfs/backlog#1177) The streaming rename_data branch invalidates cached part fds on its post-commit rollback paths, but the inline branch runs its commit and rollback inside a single spawn_blocking closure where the async invalidate cannot be called, so it was left to the TTL backstop. Capture the closure's result instead of `??`-propagating it: on error (a commit-metadata fsync failure under strict durability rolls the committed rename back), invalidate the dst part paths at the async level before returning. Inline objects keep their data in xl.meta rather than separate part inodes, so this is largely defensive, but it removes the caveat and keeps the two branches consistent. Co-Authored-By: heihutu <heihutu@gmail.com> * feat(ecstore): export io_uring latch/fallback and driver stats metrics (rustfs/backlog#1172) Complete the gray-release observability. Beyond the warn log added earlier, emit metrics so a dashboard can answer "how much traffic is on io_uring vs falling back, and is any disk degrading": - rustfs_io_uring_latch_off_total — a disk latching io_uring off at runtime. - rustfs_io_uring_read_fallback_total — each io_uring -> StdBackend read fallback (latched-off short-circuit, O_DIRECT error, buffered error). - a low-frequency per-disk exporter of the driver StatsSnapshot as gauges (in_flight, cq_overflow, cancel_already), spawned in try_new. It holds only a Weak reference so it never keeps the driver alive, and drops any temporary strong reference on the blocking pool so a last-reference UringDriver::Drop join never runs on an async worker (rustfs/backlog#1170). submit_errors is deliberately not exported yet: it is a field added in the unreleased rustfs-uring 0.2.0, and ecstore still pins 0.1.0. It lands once the dependency is bumped (rustfs/backlog#1181). Co-Authored-By: heihutu <heihutu@gmail.com> * ci: add a real-io_uring integration leg on ubuntu-latest (rustfs/backlog#1179) The existing self-hosted sm-standard runners cannot guarantee io_uring is available (a container seccomp filter can block io_uring_setup), so the ecstore uring tests degrade to a silent skip and never exercise the real UringBackend/FdCache/latch paths in CI. Add a job on GitHub-hosted ubuntu-latest, which runs a recent kernel with no container seccomp filter, running the uring-named ecstore tests with RUSTFS_IO_URING_READ_ENABLE=true and RUSTFS_URING_TESTS_MUST_RUN=1 — the non-vacuity gate makes the leg fail rather than skip if io_uring is unavailable, so an integration regression can no longer merge green behind a vacuous pass. Co-Authored-By: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> --------- Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
ab60244d7d |
chore(deps): bump rustfs-uring to 0.2.0 (#4730)
chore(deps): bump rustfs-uring to 0.2.0 from crates.io rustfs-uring 0.2.0 is published on crates.io. It carries the read-driver hardening from the backlog#1160 adversarial audit (cancel-safety and graceful-degradation fixes on the Linux io_uring read path). The public API ecstore consumes is unchanged — UringDriver::probe_and_start_sharded, read_at, read_at_direct all keep their 0.1.0 signatures — so this is a drop-in bump of the version requirement plus the lockfile entry. The Cargo.lock change is intentionally scoped to the rustfs-uring package only; unrelated lockfile drift is left for its own change. Co-authored-by: heihutu <heihutu@gmail.com> |
||
|
|
67a88f3feb | test(replication): cover multipart fanout integrity (#4727) | ||
|
|
bec5227018 |
test(e2e): add negative presigned URL SigV4 coverage (#4714)
test(e2e): negative presigned-URL SigV4 suite (backlog#1151 sec-2) Add crates/e2e_test/src/presigned_negative_test.rs, the query-string SigV4 sibling of the header-SigV4 suite (sec-1, #4708). Presigned URLs were only exercised on the happy path, so nothing pinned our end-to-end enforcement of expiry or query-signature verification. Against a live single-node RustFSTestEnvironment (random port, isolated temp dir), the suite asserts HTTP status + S3 error <Code> for: - expired presigned GET -> 403 AccessDenied (Request has expired) - tampered X-Amz-Signature -> 403 SignatureDoesNotMatch - wrong secret key -> 403 SignatureDoesNotMatch - tampered target key (swapped after signing) -> 403 SignatureDoesNotMatch - tampered presigned PUT -> 403 SignatureDoesNotMatch + object not stored - positive controls: valid presigned GET (body match) and PUT (HEAD verify) Expiry is controlled without real waiting via the AWS SDK presigner's start_time (sign as of one hour ago with a 60s window). s3s checks expiry before the signature, so the expired case surfaces AccessDenied. Wired into the e2e-smoke nextest profile (fast, single-node, no external deps) so it runs on every PR per the crates/e2e_test/README.md admission criteria. Refs rustfs/backlog#1151 (sec-2), rustfs/backlog#1155. |
||
|
|
d6e3aa9140 |
test(admin-auth): add unit and e2e coverage for the admin authorization gate (#4717)
test(admin-auth): unit + e2e coverage for the admin authorization gate Adds the first tests for the central admin authorization gate `rustfs/src/admin/auth.rs`, which previously had zero coverage (backlog#1151 sec-4, master plan #1155). Unit tests (rustfs/src/admin/auth.rs): - Refactor the two `validate_admin_request*` entry points to share a single generic decision core `evaluate_admin_actions<S: Store>` / `check_admin_request_auth<S: Store>` (removes the duplicated action-loop and makes the gate testable without a running cluster). - Cover: owner/root credential allowed; authenticated non-admin credential denied with AccessDenied; missing credential denied; and the multi-action loop grants on any permitted action, denies when none pass. Backed by an in-memory empty IamSys so the owner path short-circuits to allow and every other principal resolves to deny. E2E (crates/e2e_test/src/admin_auth_test.rs, raw SigV4 over HTTP, no awscurl dependency): - non_admin_credential_denied_on_admin_api: a limited IAM user gets 403 AccessDenied on GET /rustfs/admin/v3/info while root succeeds. - root_credential_rotation_takes_effect: restart with rotated --access-key/--secret-key; new credential works and the stale one is rejected, on both the admin plane and the S3 data plane. - default_credentials_emit_startup_warning: booting with the default rustfsadmin credentials emits the default-credentials warning. Refs rustfs/backlog#1151 (sec-4), #1155. |
||
|
|
2ebe8e561b |
fix(replication): allow loopback replication targets under an explicit test opt-in (#4725)
* fix(replication): allow loopback replication targets under an explicit test opt-in
Commit
|
||
|
|
f63af3df63 |
chore: retire completed-migration scaffolding, wire orphaned boundary check (#4719)
The ecstore/global-state migrations are done (backlog#815, #939, #1052 all closed). Review of every migration-era test/gate measure found three things actually retirable or broken — everything else is a live anti-regression guard and stays. Remove: - scripts/check_metrics_migration_refs.sh — guards a migration that finished: rustfs_metrics:: has zero hits, the metrics crate no longer exists, and the script was never wired into CI or make (only reference was one line in config-model-boundary-adr.md, also removed). - crates/obs init_metrics_collectors — the "backward-compatible alias kept during migration" the removed script was guarding. Zero callers; pure delegate to init_metrics_runtime. Archive (docs/superpowers/plans/, continuing the 2026-07 convention, with the standard archived banner): - startup-timeline.md, scheduler-baseline.md, profiling-numa-capability-inventory.md, kms-development-defaults-inventory.md — one-shot snapshots whose only consumer is the already-archived migration-progress ledger (their same-dir links there start resolving again after the move); zero script pins; fed the closed backlog#660/#665 architecture-review ledger. Fixed the one outbound link (startup-timeline -> readiness-matrix) that the move would have broken — check_doc_paths.sh deliberately does not scan plans/, so nothing else would have caught it. Wire (found orphaned by the same review): - scripts/check_extension_schema_boundaries.sh guards a live contract crate but was never invoked anywhere. Add lint-fmt.mak target, include in pre-commit/pre-pr/dev-check, add ci.yml Quick Checks step (job already installs ripgrep), sync the CONTRIBUTING.md enumerated list, and harden the script against a silently-passing rg probe when src/ is missing. Keep (verified live, documented so the next cleanup pass does not repeat this analysis): - scripts/check_architecture_migration_rules.sh — added a header stating it is a permanent boundary guard, not retirable migration scaffolding; 'migration' in the name is historical. - check_migration_gate_count.sh + floor, delete-marker e2e proof, all pinned docs, compat-cleanup-register sync, remaining inventories (referenced by live docs). Verification: all 7 guard scripts pass, actionlint clean, cargo check --workspace (excl e2e) clean, cargo fmt --check clean. Adversarially reviewed by two independent skeptic passes; their 7 findings (alias left behind, broken outbound link, missing banners, wrong backlog attribution, CONTRIBUTING drift, rg exit-2 hole, missing header rationale) are all folded in. |