15113 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
3 people	4f9bbd51cb	rust volume: stop glibc retaining freed EC buffers as unreturnable heap (#11255) ... * rust volume: stop glibc retaining freed EC buffers as unreturnable heap A Rust volume server doing EC work accumulates hundreds of MB of resident anonymous memory that it never gives back, and under a hard cgroup MemoryMax that ends in an OOM kill while most of the resident set is free-but-unreturned. It is not a leak. glibc serves allocations >= M_MMAP_THRESHOLD with mmap and munmaps them on free, but the threshold is ADAPTIVE: freeing an mmap'd block raises it toward that block's size, up to 32 MiB. EC reconstruction and needle reassembly allocate large short-lived buffers, so the first few train the threshold upward and every later buffer is carved from the heap instead. Heap pages only return to the OS from the top of the arena, so they stay resident for the life of the process -- reusable, but anonymous, and anonymous pages cannot be reclaimed under pressure the way page cache can. The retained footprint is exactly the headroom a burst of maintenance work needs. Measured on a 17-node cluster (EC 10+4, --index=redb), one node, two identical `ec.scrub -mode full` rounds over 10912 EC files each, same unit restarted with and without a pinned threshold: baseline round 1 round 2 60s idle default (adaptive) 10 MB 84 MB 88 MB 88 MB pinned threshold 10 MB 13 MB 14 MB 14 MB 78 MB retained versus 4 MB for identical work. On heavier mixed scrub workloads the same effect reached ~600 MB per volume server against a 3 GiB cap, and restarting the process was the only way to release it. Calling mallopt(M_MMAP_THRESHOLD, ...) sets the threshold and disables the dynamic adjustment. Pin it to glibc's own default rather than inventing a value: the goal is to stop the adaptation, not to second-guess the default. MALLOC_MMAP_THRESHOLD_ still wins if an operator sets it, glibc-only, and a failed mallopt is logged rather than fatal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MFr2v4BUqrXdgj4LEUAwVF * Address PR review: validate env overrides, honour GLIBC_TUNABLES, fix non-glibc test compile Three review-bot findings on seaweed-volume/src/malloc_tuning.rs: 1. (CodeRabbit) The test used cfg!(...), which keeps both branches in compilation. On non-glibc targets DEFAULT_MMAP_THRESHOLD is undefined, so the test failed to compile. Split into #[cfg]-gated tests so each branch only references items defined for that target. 2. (Greptile) MALLOC_MMAP_THRESHOLD_ was checked by presence only. An empty or non-numeric value makes glibc ignore the override while we still skipped mallopt, leaving the adaptive threshold enabled -- exactly the behaviour this module exists to prevent. Now we defer only when the value is non-empty and parses as an integer; otherwise we fall through to pinning. 3. (Codex) The modern GLIBC_TUNABLES=glibc.malloc.mmap_threshold=... tunable was missed, so mallopt could overwrite an operator's explicit tunable. Now we detect that tunable (with the same validation) and defer to it. The override check moved into the glibc-gated inner function, so off glibc pin_mmap_threshold() always reports NotApplicable regardless of any allocator env vars that happen to be set. The startup log for DeferredToEnv is reworded to cover both override sources. Added tests for the override parsers and the off-glibc no-op. * Address round-2 review: match glibc's actual override parsing Three follow-up review-bot findings after the first round of fixes, all rooted in our validation not matching how glibc actually parses the overrides: 1. (Greptile, P1) parse::<i64>() accepted negative values like "-1" and returned DeferredToEnv, but glibc's threshold is unsigned and rejects negatives — so we skipped mallopt while glibc also ignored the override, leaving the adaptive threshold enabled. Now we reject negatives and zero. 2. (Devin, BUG) glibc parses thresholds as unsigned (strtoul for tunables, atoi for the legacy var). Values above i64::MAX are valid for glibc but were rejected by parse::<i64>(), so we pinned 128 KiB over the operator's explicit setting. Now we parse as u64, accepting the full unsigned range. 3. (CodeRabbit, Major) Two issues in usable_glibc_tunable_threshold: a. A malformed sibling entry (e.g. glibc.malloc.check=2=2:...) makes glibc reject the entire GLIBC_TUNABLES string, but our per-entry scan still returned true for the valid-looking mmap_threshold entry. Now we validate every entry (exactly one '=') before accepting any. b. Hex values (0x20000) are accepted by glibc's strtoul but were rejected by parse::<i64>(). Now parse_strtoul_threshold handles 0x-prefixed hex. MALLOC_MMAP_THRESHOLD_ stays decimal-only (atoi), matching glibc. Added regression tests for negatives, zero, >i64::MAX, hex tunables, and malformed mixed GLIBC_TUNABLES entries. Verified: clippy clean and tests pass on macOS (non-glibc); glibc-gated code type-checks for x86_64-unknown-linux-gnu. * Address round-3 review: match glibc's actual override parsing Three follow-up review-bot findings (Greptile P1, Devin BUG, CodeRabbit Major) all on the same issue: the round-2 fix rejected negative and zero override values, but glibc actually accepts them. Verified against the glibc source (malloc/malloc.c, malloc/arena.c, elf/dl-tunables.c, elf/dl-misc.c): - do_set_mmap_threshold(size_t value) does NO clamping — it just sets mp_.mmap_threshold = value and mp_.no_dyn_threshold = 1. - MALLOC_MMAP_THRESHOLD_: glibc calls atoi(value) then mallopt, which always sets the threshold and disables dynamic adjustment — even for empty, negative, or non-numeric values (atoi returns 0). So ANY presence of the variable means the operator's override is in effect. Reverted to presence-only check for the legacy variable. The round-1 Greptile comment claiming glibc "cannot apply the override" for empty/malformed values was incorrect. - GLIBC_TUNABLES: glibc parses values with _dl_strtoul (elf/dl-misc.c), which accepts decimal, 0x hex, 0 octal, an optional sign (negatives wrap to unsigned long), and requires the entire value consumed (tunable_parse_num checks endptr == strval + len). Replaced parse_strtoul_threshold with dl_strtoul_consumes_all that replicates _dl_strtoul's parsing and checks full consumption. Now accepts -1 (wraps to SIZE_MAX), 0, 0x20000, 010 (octal), and values above i64::MAX. The duplicate-= validation for GLIBC_TUNABLES (from round 1) is kept — glibc's parse_tunables_string returns -1 if any entry's value contains a duplicate =, rejecting the entire string. Added dl_strtoul_consumes_all tests covering decimal, hex, octal, negative, zero, empty, whitespace, trailing garbage, and sign-only inputs. Updated usable_glibc_tunable_threshold tests to accept negative, zero, and empty values. Verified: clippy clean and tests pass on macOS (non-glibc); glibc-gated code type-checks and clippy clean for x86_64-unknown-linux-gnu. * Address round-4 review: add overflow detection, fix sign-only test assertions Two Greptile P1 findings: 1. Overflowing tunables bypass threshold pinning: dl_strtoul_consumes_all consumed every digit and returned true for values like 18446744073709551616 (u64::MAX + 1), but glibc's _dl_strtoul stops at the overflowing digit (sets endptr there, returns UINT64_MAX), so tunable_parse_num rejects the value (endptr != strval + len). Added overflow detection matching glibc's cutoff/cutlim logic — on overflow, the parser stops and returns false. 2. Sign-only parser assertions fail: the test asserted !dl_strtoul_consumes_all("-") and !dl_strtoul_consumes_all("+"), but _dl_strtoul skips the sign, finds no digit, sets endptr to the position after the sign (== end of string), and returns 0. tunable_parse_num sees endptr == strval + len → true. So glibc accepts sign-only strings as value 0. Fixed the test assertions to expect true. Also fixed "0x" with no hex digits: _dl_strtoul parses "0" as octal, then stops at "x" (not an octal digit), so endptr != end of string → rejected. The base-detection now requires a hex digit after "0x" before switching to hex; otherwise "0" is parsed as octal and "x" stops the parser. Added overflow regression tests: 18446744073709551616 (u64::MAX + 1), 99999999999999999999 (20 nines), 0x10000000000000000 (2^64). Verified: clippy clean and tests pass on macOS (non-glibc); glibc-gated code type-checks and clippy clean for x86_64-unknown-linux-gnu. * Address round-5 review: accept bare 0x prefix, remove unused helper Two review-bot findings (Devin BUG + CodeRabbit Major) on the same issue: the round-4 fix required a hex digit after "0x" before switching to hex base, but glibc's _dl_strtoul unconditionally advances past "0x"/"0X" when the first char is '0' and the next is 'x'/'X' — even if no hex digit follows. In that case the digit loop breaks immediately, endptr reaches the end, and the value is 0. tunable_parse_num accepts it. Removed the is_digit_in_base lookahead from the base-detection condition and the now-unused is_digit_in_base helper. Updated the test assertions for "0x" and "0X" to expect true (accepted as value 0). The Greptile P1 overflow comment is invalid: glibc's _dl_strtoul rejects 18446744073709551616 (u64::MAX + 1) — on overflow it sets endptr to the overflowing digit (not end of string) and returns UINT64_MAX, so tunable_parse_num sees endptr != strval + len and rejects. My implementation correctly returns false for this value, matching glibc. Verified: clippy clean and tests pass on macOS (non-glibc); glibc-gated code type-checks and clippy clean for x86_64-unknown-linux-gnu. * Address round-6 review: rewrite tunable parser to match glibc exactly Two Greptile P1 comments (3975151906, 3975151911) both invalid, but investigation revealed a real bug in the split(':')-based parser: Bug: usable_glibc_tunable_threshold used split(':') which loses the distinction between an entry terminated by ':' (glibc skips it) and one terminated by '\0' with no '=' (glibc rejects the entire string). Examples: - "glibc.malloc.mmap_threshold=262144:glibc.cpu.x" (no '=' at end): glibc rejects entire string, old code accepted it. - "glibc.malloc.mmap_threshold=262144:" (trailing ':'): glibc rejects entire string, old code accepted it. Fix: replaced split(':') with a character-by-character parser matching glibc's parse_tunables_string exactly. The parser tracks position in the original string and correctly handles all three terminators ('=', ':', '\0') for both name and value scanning. Comment 3975151906 (near-maximum values): Invalid. Verified against _dl_strtoul: for 18446744073709551615 (u64::MAX), cutoff = u64::MAX/10, cutlim = u64::MAX%10 = 5. After 19 digits result == cutoff. 20th digit 5: overflow check (digval > cutlim) is 5 > 5 = false → no overflow. glibc accepts u64::MAX. Added regression test asserting it's accepted. Comment 3975151911 (later malformed entry): Invalid. Verified against parse_tunables (elf/dl-tunables.c): when parse_tunables_string returns -1, parse_tunables prints a warning and returns immediately without applying ANY tunable — including ones already parsed into the array. Added regression test for "threshold=262144:check=2=2" (threshold before malformed sibling) asserting it's rejected. Added regression tests: u64::MAX accepted, threshold-before-malformed rejected, no-'=' at end rejected, trailing ':' rejected, leading ':' accepted. Verified: clippy clean and tests pass on macOS; glibc-gated code type-checks and clippy clean for x86_64-unknown-linux-gnu. * Fix CI: correct hex trailing-garbage test assertion The test asserted !dl_strtoul_consumes_all("0x20000abc"), but in hex mode a-f are valid digits — "0x20000abc" is a valid hex number (0x20000abc = 536874044), not trailing garbage. _dl_strtoul consumes the entire string and tunable_parse_num accepts it. The assertion failed on Linux CI where the glibc-gated test actually runs. Replaced with "0x20000g" — 'g' is not a hex digit, so _dl_strtoul stops at 'g' and tunable_parse_num rejects the value. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-09-10 09:29:31 -07:00
ssshr-66andChris Lu	e919bec9d1	fix(volume): Harden Volume Copy Validation and Failure Handling (#11252) ... * fix(volume): harden volume copy validation * fix(volume): use stream context for ReadVolumeFileStatus in VolumeCopy ReadVolumeFileStatus ran on context.Background() while the adjacent VolumeStatus call used stream.Context(), an inconsistency left over from the context revert in #11252. Use stream.Context() consistently so the source status check is cancelled with the VolumeCopy stream. * fix(volume): reserve destination before deleting existing replica FindFreeLocation now runs before DeleteVolume so a full target fails without destroying the existing replica. Previously, when the initial VolumeStatus check failed (advisory) but ReadVolumeFileStatus succeeded, the existing replica was deleted before a destination was reserved, risking data loss if no location had enough free space. Add a regression test verifying the existing replica survives when the destination is full and the initial status check fails. * fix(volume): count replaced replica slot in FindFreeLocation FindFreeLocation now accepts the volume being replaced so its slot is treated as available. Without this, a location at its MaxVolumeCount limit could not replace its sole replica even though deleting it would free the slot. VolumeCopy passes the volume ID so destination selection succeeds before the existing replica is deleted. Add TestVolumeCopyReplacesReplicaAtSlotLimit covering a single-slot location that must replace its only replica. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-09-09 23:33:05 -07:00
3 people	2cd6c36c54	filer: end local-only metadata subscriptions when remote peers appear (#11251) ... * filer: end local-only metadata subscriptions when remote peers appear SubscribeMetadata delegates to SubscribeLocalMetadata whenever the MetaAggregator knows no remote peers at stream setup. Peer discovery is asynchronous with the gRPC server accepting streams: the master announces filers after Filer.Init, via ListExistingPeerUpdates and OnPeerUpdate. A subscriber that connects inside that window is pinned to a filer-local stream for its whole life, silently missing every other filer's writes. For filer.remote.sync in a multi-filer cluster this means the remote tier permanently stops receiving writes served by other filers (#11247). End the delegated local stream when the first remote peer appears, so the client reconnects into the aggregated stream. The end surfaces as an error, not a clean EOF: RetryUntil-driven followers (mount meta cache, s3api IAM) treat a clean end as following finished and stop reconnecting. The arrival channel is armed under the same lock as the peer check in RemotePeerArrivedChan, so a peer learned in between sends the stream straight to the aggregated path instead of parking on a channel that would never fire. A standalone filer is unaffected: no peer ever appears, the channel never fires, and the local stream serves indefinitely. Fixes #11247 * filer: interrupt disk replay on peer arrival, trim comments Check upgradeOnRemotePeer inside eachLogEntryFn and chunkDiskPass so a peer arriving during a backlog replay stops the stream before the cursor advances past older remote events. Wrap errAggregationUpgrade with StopReadingError so LoopProcessLogData does not log it. Remove issue references from comments and trim verbose commentary. * filer: check upgrade signal between ref batches Pass upgradeOnRemotePeer to sendRefsBatched so a peer arriving while refs are shipped to a slow client is detected between batches, not only after the full batch completes. * filer: interrupt gap park on peer arrival Pass upgradeOnRemotePeer through gapPass to parkOnGap so a peer arriving during a gap park ends the stream immediately instead of waiting for the retry timer (up to one minute). --------- Co-authored-by: Tyagiquamar <Tyagiquamar@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-09-09 20:16:50 -07:00
Chris Lu	7fa2f75f30	s3: bucket-policy Allow must not override an identity explicit Deny (#11256) ... * s3: add isActionExplicitlyDeniedByApplicablePolicies helper Add a helper that reports whether any applicable identity-side policy (attached IAM policies, enabled-group policies, or the IAM-integration session policy) explicitly denies an action. It reuses the existing evaluateAttachedIAMPolicies, resolveS3AuthTarget, buildPrincipalARN, and isActionExplicitlyDeniedByIAM helpers, and fails closed on evaluation errors. A nil identity has no identity-side policy plane, so the helper returns false to keep the bucket policy authoritative for anonymous access. No behavior change yet; the next commits apply it to the two bucket-policy Allow short-circuits. * s3: enforce identity explicit Deny before bucket-policy Allow authRequestWithAuthType short-circuits on a matching bucket-policy Allow and skips VerifyActionPermission, so an explicit Deny in an authenticated identity attached, group, or session policy is bypassed. A non-admin principal with s3:PutBucketPolicy can install a bucket-policy Allow for itself and read an object its identity policy explicitly denies. Before honoring a bucket-policy Allow, check the applicable identity-side policies for a matching explicit Deny via the new isActionExplicitlyDeniedByApplicablePolicies helper, and fail closed. The cross-account behavior is preserved: a bucket Allow still supplies the Allow an identity policy omits (implicit denial), and a nil identity keeps the bucket policy authoritative for anonymous access. Regression tests cover the explicit-Deny override, the implicit-deny Allow preservation, and the unmatched-key fall-through control. * s3: enforce identity explicit Deny in secondary object-key auth authorizeObjectKeyAction authorizes keys the request URL does not name (CopySource, DeleteObjects body keys, POST Object form keys) and shares the same bucket-policy Allow short-circuit as the primary path, so an explicit Deny in the identity, group, or session policy is bypassed the same way when a bucket policy allows the secondary key. Apply isActionExplicitlyDeniedByApplicablePolicies before accepting the bucket-policy Allow, mirroring the primary path. A regression test covers AuthorizeCopySource for both the explicit-Deny override and the implicit-deny Allow preservation.	2026-09-09 20:15:43 -07:00
github-actions[bot]	5061a16b12	docs: regenerate star history chart	2026-09-10 02:39:04 +00:00
Chris Lu	3c9a4bbdda	rust: prevent phantom volumes + validate collection hint in mount_volume_by_id (#11254) ... * rust: prevent phantom volumes + validate collection hint in mount_volume_by_id The collection-hint path (and the find_volume_file_base fallback) called create_volume on any matching .vif/.idx sidecar. create_volume -> Volume::new -> load(create_dat_if_missing=true) writes an empty .dat and registers a phantom normal volume, which can shadow a real EC volume whose .ecx lives on a sibling disk. This reintroduces the phantom-volume bug the codebase explicitly guards against in load_existing_volumes. Apply the same guard load_existing_volumes uses to both paths: only mount when a real .dat is present or the .vif references a remote-tiered file; otherwise skip the candidate (no phantom). Also reject path-bearing collection hints ('/', '\\', '..') so the shortcut cannot route .dat creation outside the storage directory, falling back to the safe scan. Adds 3 regression tests; all 336 storage:: tests pass. Addresses Devin + Greptile review comments on PR #11249. * rust: address review — .note guard, multi-candidate scan, foo..bar hint Address the four review comments on #11254: 1. Greptile (P1): contains("..") rejected valid collections like "foo..bar". Replaced with collection != ".." — volume_file_name joins with "_" so a ".." inside a name is part of the filename, not a parent reference. Only the exact ".." name is rejected. Added a test that "foo..bar" mounts. 2. Devin #0001 (bug): mount_volume_by_id did not check the .note marker, so an interrupted VolumeCopy could mount as a live (truncated) volume. Added a .note check before create_volume in both the collection-hint path and the fallback — a candidate with .note is skipped (matches load_existing_volumes). Added a test covering both paths. 3. Devin #0002 + CodeRabbit (major): find_volume_file_base returned only the first matching candidate, so a lone sidecar on disk 0 hid a real .dat on disk 1 (the split-disk EC layout the phantom guard protects against). Added find_volume_file_bases (plural) that collects all candidates; the fallback now iterates every candidate and mounts the first with a real .dat or remote .vif. find_volume_file_base delegates to it for configure_volume. Added a two-disk test: sidecar on disk 0, real .dat on disk 1 — mount succeeds from disk 1. All 339 storage:: tests pass (6 mount_volume_by_id tests). * rust: continue past create_volume failure in mount_volume_by_id Address Devin review comment on #11254: when create_volume fails on an earlier candidate (e.g. an unreadable .dat), mount_volume_by_id returned the error immediately instead of trying later candidates. A valid volume on another disk remained unmounted. Both the collection-hint loop and the find_volume_file_bases fallback now remember the last error and continue scanning. A successful mount returns immediately; if no candidate succeeds, the last error (or NotFound) is returned. Matches DiskLocation::open_volumes and Go Store.mountVolume. Added test_mount_volume_by_id_continues_past_open_failure (chmod 000 .dat on disk 0, real volume on disk 1, mounts from disk 1). All 340 storage:: tests pass.	2026-09-09 14:54:09 -07:00
Feng Shao	13bf056a15	Mount req with collection (#11249) ... * volume mount req support specify collection * rust mirror change	2026-09-09 12:55:47 -07:00
Nguyễn Đăng Minh Lực	c968084b34	iceberg: fix OAuth token expiry handling (401 + token-exchange + configurable TTL) (#11242) ... * iceberg: return 401 for invalid or expired Bearer tokens BUG-0001: when the OAuth JWT expired, Server.Auth fell through to the S3 SigV4 authenticator, which rejects the "Authorization: Bearer" scheme with NotImplemented — a 501. Iceberg clients (Java OAuth2Manager, pyiceberg) only refresh tokens on 401, so they retried the dead token forever: RisingWave sinks stalled and Doris catalog queries failed every token TTL (1h) until the client process was restarted. A request carrying a Bearer header is an Iceberg REST client: answer 401 (+ WWW-Authenticate: Bearer, RFC 6750) when the token fails, and only fall through to the S3 authenticator when no Bearer header is present. * iceberg: make OAuth token TTL configurable via ICEBERG_OAUTH_TOKEN_EXPIRY BUG-0001 follow-up: production evidence shows Iceberg Java 1.10.x clients (RisingWave connector node, Doris FE) never re-fetch tokens on 401 — the sink stalled again on token expiry even with the 501→401 fix, and no POST /v1/oauth/tokens appeared in server logs across dozens of retries. 401 is necessary but not sufficient for these clients. The TTL was hardcoded to 3600 with no knob. Read the expiry (seconds) from ICEBERG_OAUTH_TOKEN_EXPIRY, defaulting to 3600, so deployments can issue longer-lived tokens (e.g. 86400) to survive client restart cycles. * iceberg: support OAuth token exchange (RFC 8693) for client refresh Decompiling the Iceberg Java 1.10.1 client bundled with Doris FE showed the missing half of BUG-0001: OAuth2Manager refreshes via token-exchange (AuthConfig.exchangeEnabled defaults to true — the client_credentials re-fetch branch only runs with exchange disabled), so a server that only accepts client_credentials leaves Iceberg clients unable to ever refresh their token, regardless of 401 correctness. Accept grant_type=urn:ietf:params:oauth:grant-type:token-exchange on POST /v1/oauth/tokens: verify the subject_token signature against the issuing credential, allow exchange within a recovery grace window (max(2*TTL, 1h), capped 24h) so clients holding tokens that expired while the grant was unsupported recover without a restart, and mint a fresh access token with the configured TTL. * iceberg: harden OAuth token exchange and Bearer matching per review - match the Bearer scheme case-insensitively (RFC 7235), like authenticateBearer already does - accept optional client authentication on the token-exchange grant (Basic or form credentials, bound to the subject token's client); expired subject tokens now require it. Iceberg Java's proactive refresh sends Bearer-only headers, so the grant cannot require it - reject subject tokens without an exp claim, and re-check the issuer on the verified claims - unauthenticated exchange cannot extend the lifetime past the subject token's own expiry (no chain-refresh from a leaked token) - return 400 invalid_grant per RFC 6749 §5.2 (was 401) - include issued_token_type on exchange responses (RFC 8693) - clamp ICEBERG_OAUTH_TOKEN_EXPIRY to 365d so Duration math cannot overflow into already-expired tokens * iceberg: give authenticated token exchanges a fresh full TTL The remaining-lifetime cap only guards unauthenticated (Bearer-only) exchanges; an authenticated client renewing a live token must get the full configured TTL, matching client_credentials. * iceberg: reject token exchange when no lifetime remains A Bearer-only exchange with under a second of subject lifetime would mint a token with expires_in: 0. Reject with invalid_grant instead. * iceberg: pin near-expiry test token to the next second boundary jwt/v5 serializes exp at one-second precision, so a 300 ms offset can round into the current second and route the test through the expired branch instead of the ttlSeconds<=0 guard. Mint the subject with the next whole-second expiry: live at exchange time, deterministically under a second of remaining lifetime. * iceberg: drop internal ticket reference from comments * iceberg: clamp oversized OAuth TTLs on 32-bit platforms strconv.Atoi on an int-sized value fails with ErrRange on 386, so an oversized ICEBERG_OAUTH_TOKEN_EXPIRY silently fell back to the default instead of clamping. Parse in 64-bit space and clamp, then narrow. * iceberg: make OAuth TTL narrowing explicit * iceberg: disable legacy OAuth in PyIceberg integration tests	2026-09-09 10:54:39 -07:00
3 people	516e251f9e	rust volume: move the crate to edition 2024 (#11244) ... * rust volume: move the crate to edition 2024 Edition 2024 turns three things in this crate into hard errors, and changes drop order in a further 34 places without changing compilation. The compiler errors are fixed here; the silent changes were audited against `RUSTFLAGS='-W rust-2024-compatibility' cargo check --all-targets` output captured before the flip, since edition 2024 stops reporting them. `std::env::set_var`/`remove_var` are unsafe as of 2024 because they race with concurrent readers. All six call sites are safe by construction rather than by assertion, and the SAFETY comments say why: the build script runs single-threaded before anything else in the process, and every test reaching the `config.rs` helpers holds `process_state_lock()` for the duration. The two `ref` bindings in handlers.rs sit in patterns that already borrow implicitly, so removing the modifier leaves both bindings at `&String`. On the 34 drop-order sites: no lock guard's scope is extended anywhere, and `volume.rs` has none. Most are moved-from `Option`/`Result` husks — `if let Some(v) = map.remove(&k)`, `while let Some(m) = stream.next().await` — where the value is moved into the binding and the temporary has nothing left to drop; where closing order actually matters these paths already call `v.close()`, `ec_vol.destroy()` or `drop(writer)` explicitly. Two sites get strictly better ordering: the metrics read guard in `run_metrics_push_loop` shrinks to the end of its initializer block (it never crossed an `.await` either way), and an EC test now closes the volume's descriptors before the `TempDir` removes the directory. No `rust-version` is declared. Edition 2024 needs rustc 1.85, but that is not the binding constraint — the dependency tree already requires 1.91.1 through the `aws-sdk-s3`/`aws-smithy-*` family, so `cargo +1.85 check` fails on the deps regardless. CI builds on `dtolnay/rust-toolchain@stable`. `vendor/reed-solomon-erasure` is a separate package and keeps edition 2021. Cargo.lock is unchanged despite edition 2024 implying resolver 3. Verified: `cargo test` 551 passed / 0 failed, `cargo test --no-default-features` 550 passed / 0 failed (the two feature sets produce an identical migration site list), `cargo build --release` clean. No automated test covers shutdown ordering, so the channel and runtime sites in `main.rs`, `write_queue.rs` and `grpc_server.rs` were read individually. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018nty5Rj7ssMQdFxHHjZgDC * rust volume: address edition-2024 review feedback Three fixes from review of the edition bump. Serialize the two environment-reading tests. The SAFETY comments on the `env::set_var`/`remove_var` helpers claim every test touching the environment holds `process_state_lock()`, but `test_resolve_config_defaults_dir_to_platform_temp_dir` and `test_resolve_config_index_accepts_redb_and_leveldb_aliases` called `resolve_config` — which reads HOME/USERPROFILE, SEAWEED_WRITE_QUEUE and the WEED_* set — without taking it. `set_var` is unsafe precisely because a concurrent *reader* is UB, not only a concurrent writer, so the comment was overclaiming. An audit of the module found exactly these two; every other environment-touching test already held the lock. The race predates edition 2024, which only made the requirement explicit. Declare `rust-version = "1.91.1"`. The edition needs 1.85, but that was never the binding constraint: `cargo +1.90 check --all-targets` fails on the `aws-sdk-s3`/`aws-smithy-*` family, and 1.91.1 checks clean. Declaring the verified floor turns a wall of per-dependency errors into one clear message. Cargo.lock is unchanged despite this making the resolver MSRV-aware. Update the README, which advertised "Rust 1.75+ (2021 edition)". 1.75 was already stale before this branch — the tree has needed 1.91 for a while. Verified: `cargo test` 551 passed / 0 failed, `cargo test --no-default-features` 550 passed / 0 failed, `cargo build --release` clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018nty5Rj7ssMQdFxHHjZgDC * rust volume: state the exact MSRV patch release in the README The README said "Rust 1.91+", which reads as 1.91.0 and is wrong by one patch release: `cargo +1.91.0 check --all-targets` fails on the aws-sdk-s3 family, `cargo +1.91.1` passes. Say 1.91.1+, matching `rust-version` in Cargo.toml, and call out that the patch component is load-bearing so nobody installs 1.91.0 and hits the same wall. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018nty5Rj7ssMQdFxHHjZgDC * rust worker: move the workspace to edition 2024 Moves the seaweed-worker workspace (core, lance, sort) from edition 2021 to 2024, the same migration seaweed-volume just got in this branch. Edition 2024 turns exactly one thing in this workspace into a hard error. The baseline came from RUSTFLAGS='-W rust-2024-compatibility' cargo check --all-targets, run before the flip; unlike seaweed-volume's 34 silent + 8 hard sites, the worker reports only the one hard site and no tail_expr_drop_order or if_let_rescope sites at all. The worker is a much smaller crate and none of its expressions hold a guard or temporary whose drop order the edition changes, so there is nothing to audit on the silent side. Fixed (1 site): std::env::set_var is unsafe as of 2024 because it races with concurrent readers. The single call is in crates/core/build.rs, which sets PROTOC from protoc_bin_vendored the way seaweed-volume's build script does. A build script's main runs single-threaded before anything else in the process, so no other thread can be reading the environment concurrently; the SAFETY comment says so. There are no config.rs-style test helpers here -- the worker's tests do not mutate the environment -- so unlike the volume crate there are no process_state_lock() callers to audit. No redundant ref bindings to clean up: a grep for ref across the three crates finds none. MSRV: rust-version = "1.94.1", verified rather than inferred. Edition 2024 only needs 1.85, but the dependency tree needs more: lance's aws feature pulls in a newer cut of the same aws-sdk-*/aws-smithy-* family that sets seaweed-volume's 1.91.1 floor, and that newer cut requires 1.94.1. cargo +1.94.0 check --all-targets fails on that family; cargo +1.94.1 check --all-targets is clean. The worker's floor is therefore higher than the volume's, and moves with lance and the AWS SDK rather than with the edition. CI builds on dtolnay/rust-toolchain@stable, so nothing changes there. The edition is set once in [workspace.package] and inherited by each member via edition.workspace = true; rust-version is added the same way. The workspace keeps its explicit resolver = "2" -- edition 2024 would default to resolver 3, but the pin is deliberate and Cargo.lock is unchanged by this commit either way. The README gains a "Requires Rust 1.94.1+ (2024 edition)" line in its Building section, matching the one seaweed-volume's README now carries, and calling out that the patch release is load-bearing (1.94.0 does not build) so nobody installs 1.94.0 and hits the same wall. Verification: * cargo check --all-targets -- clean, zero warnings (default toolchain 1.97) * cargo +1.94.1 check --all-targets -- clean * cargo +1.94.0 check --all-targets -- fails on the AWS SDK, as claimed * cargo test --all-targets -- 40 passed, 0 failed (core 13, sort 11, lance lib 3, lance bin 2, compaction 6, lifecycle 1, sort integration 4) * Cargo.lock unchanged Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-09-09 10:54:06 -07:00
Chris Lu	01fc31cb71	fix(filer): use path.Split instead of filepath.Split for filer paths (#11246) ... FullPath.DirAndName() and FullPath.Name() used filepath.Split, which is OS-dependent: on Windows it treats backslash as a path separator, corrupting filer paths that contain literal backslashes. Filer paths always use "/" as the separator, so switch to path.Split and path.Join which only split on "/" regardless of the host OS. This fixes the backslash case from #11243 where a file saved as /test/special\reverseslash4.jpg was stored with a corrupted path on Windows filer builds. The #, ?, and % cases from the same issue are client-side URL-encoding problems (the server never receives the raw characters), but once the client properly percent-encodes them the server now handles the decoded path correctly on all platforms.	2026-09-09 10:43:16 -07:00
ssshr-66	966692fa23	[Volume] Validate record counts after volume copy (#11238) ... * validate Volume Copy record counts * Delete s3api_object_versioning_bench_test.go * reply ai comments	2026-09-09 02:18:18 -07:00
github-actions[bot]	168b9c39f8	docs: regenerate star history chart	2026-09-09 05:10:20 +00:00
Chris Lu	f1f6886d0e	fix: rebase on latest master before pushing star history chart (#11240) ... * fix: rebase on latest master before pushing star history chart The daily star history workflow git push was rejected with a non-fast-forward error because new commits landed on master between the checkout and the push. Fetch full history (fetch-depth: 0) and rebase the generated commit on top of the latest remote branch before pushing so the workflow no longer fails when master has moved. * fix: retry rebase-and-push to handle concurrent master updates Address review feedback: a one-shot rebase still races if master advances between the rebase and the push. Match the bounded retry loop used by java_release.yml — push first, and on rejection rebase and retry up to five times before failing. * fix: serialize runs and ensure every rebase is followed by a push Address review feedback: - Devin (line 54-55): the old loop rebased after the 5th failed push but never pushed the rebased commit. Restructure so each rebase (attempts 2-5) is followed by a push attempt, with a clear 5-attempt cap. - Greptile: overlapping runs could conflict on the SVG during rebase. Add a concurrency group (cancel-in-progress: true, matching the repo convention) so only one chart regeneration runs at a time. * fix: scope concurrency by ref and guard rebase against transient failures Address review feedback: - Devin (line 16): the global star-history concurrency group let a manual run on another branch cancel an in-flight daily master update. Scope the group by github.ref so only same-branch runs cancel each other. - Greptile (line 58): git pull --rebase runs under the fail-fast shell, so a transient fetch error or conflict aborted the whole step before remaining attempts ran. Guard the rebase so a failure aborts the in-progress rebase and continues to the next attempt instead.	2026-09-08 21:43:24 -07:00
Chris Lu	2ffa696809	fix(volume): handle faulty storage media (Go + Rust) (#11233) ... * fix(volume): track EC shard read errors and unmount on faulty media Extract the volume EIO tracker into a reusable IoErrorTracker and add the same tracking to EcVolume. Sustained EIO on .ecx lookups or .ecd shard reads now unmounts the EC volume in the heartbeat (without deleting files) so the master re-replicates from healthy peers, mirroring the existing volume replica quarantine. Closes #11227 (EC shard unmount). * rust(volume): mirror EC shard read error tracking and unmount Add EIO tracking to the Rust EcVolume mirroring Go: a streak counter with IO_ERROR_TOLERANCE, a sticky quarantine flag, and unmount (not file deletion) in the heartbeat so the master re-replicates from healthy peers. * feat(metrics): expose storage IO error counter and quarantine gauge Add a storage_io_error_total counter incremented on every EIO recorded by the volume or EC shard tracker, and an io_quarantine gauge labelled by kind (volume/ec_shard) reflecting the count of replicas suppressed in the heartbeat. Mirrored in Go and Rust. * feat(healthz): report 503 when local replicas are IO-quarantined Add Store.HasIoQuarantine (Go) / Store::has_io_quarantine (Rust) and have /healthz return 503 when any local volume or EC shard is quarantined due to sustained storage-media EIO, so a load balancer can drain a server whose underlying media is faulty. Mirrored in Go and Rust. * fix(volume): keep quarantined EC volumes in memory and reset EIO on success Address review feedback: instead of unloading quarantined EC volumes (which discards the quarantine state healthz needs), keep them in memory and just skip them from heartbeat reporting, mirroring the regular volume quarantine. Also clear the EIO streak on successful .ecx reads in Rust so a transient error does not accumulate, and add an ec_shard label to the io_quarantine gauge in both Go and Rust. * fix(volume): exclude quarantined EC shards from heartbeat and add Rust volume tolerance Address review feedback: - Filter quarantined EC volumes from CollectErasureCodingHeartbeat (Go) and collect_ec_shard_delta_messages / collect_live_ec_shards (Rust) so the master stops advertising faulty shards and re-replicates from healthy peers. - Add consecutive EIO count and sticky quarantine to the Rust regular Volume, mirroring Go IoErrorTracker: a single EIO no longer deletes the replica; the heartbeat quarantines after the tolerance threshold and keeps the volume in memory. - Use the quarantine flag (not last_io_error) in has_io_quarantine so /healthz reflects sustained, not transient, failures. * fix(volume): make Rust quarantined volumes read-only and wire recovery Address Devin review: - Set no_write_or_delete on Rust volumes when quarantined in the heartbeat, so cached or direct clients cannot mutate a faulty replica after the master removes it (mirrors Go). - Wire reset_io_error_state into Volume::set_writable so an operator making a volume writable again clears the sticky quarantine and the volume re-enters heartbeat rotation. * fix(volume): clear EC quarantine on shard re-mount for operator recovery Address Greptile review: re-mounting EC shards (Go loadEcShardWithIdxDir / Rust mount_ec_shards_with_idx_dir) now calls ResetIoErrorState on the existing EcVolume, giving operators a documented recovery path that clears the sticky quarantine and returns the EC volume to heartbeat rotation. Mirrored in Go and Rust. * fix(volume): do not clear EC quarantine on routine shard mounts Address review feedback: clearing the EC IO quarantine on every mount (including duplicate, retry, sibling-shard, and reconciliation mounts) is too aggressive and can re-advertise known-bad shards before the storage media has been validated. Remove the automatic reset from the mount path; quarantine clears naturally on restart or full unmount when a fresh EcVolume is created with clean state. * test(volume): update Rust IO error test for quarantine semantics The heartbeat now quarantines a volume with sustained EIO (keeps it mounted, makes it read-only, omits it from heartbeat) instead of deleting it. Update test_collect_heartbeat_deletes_io_error_volume to assert the volume stays in the store with no_write_or_delete set, and update set_last_io_error_for_test to set the consecutive error count at the tolerance threshold so the test reflects a sustained error. * fix(volume): reset EIO streak after full write and match Windows media errors Move the success-side EIO reset from append_needle (after write_all only) to the end of do_write_request, after flush_dat/flush_idx complete, so a successful write_all followed by a failed fsync no longer resets the counter before the EIO is recorded. Repeated fsync EIOs now accumulate toward the quarantine threshold as intended. Recognize Windows storage-media failure codes ERROR_CRC (23) and ERROR_IO_DEVICE (1117) in addition to Unix EIO (errno 5), so quarantined heartbeat behavior is preserved on Windows. Mirrors the change in both Go and Rust volume servers. * fix(volume): preserve checkpoint EIO and clear streak on successful delete maybe_checkpoint_index now returns whether the checkpoint succeeded; the success-side EIO reset in do_write_request and do_delete_request only fires when it did, so a checkpoint media failure is no longer erased by the unconditional reset that followed it. do_delete_request also gains the success reset that was lost when append_needle stopped clearing the streak, so a successful delete still clears an earlier failure streak. is_storage_io_error now uses libc::EIO on Unix instead of a hard-coded 5, and the ECX binary-search read path gains a Windows fallback (seek + read_exact) so the buffer is no longer zeroed on non-Unix targets.	2026-09-08 21:42:56 -07:00
Chris Lu	0ce5ca42ea	helm: supply admin auth in CI renders that enable admin (#11239) ... PR #11236 added a render-time guard that fails the chart when admin.ip is non-loopback (default 0.0.0.0) and admin auth is not configured, since weed admin 4.46 refuses to bind a non-loopback address without authentication. Several pre-existing helm_ci.yml test cases enable admin.enabled=true as part of "everything on" renders without a password, so helm template now exits non-zero and the Verify template rendering step fails. Add admin.secret.adminPassword to the four render calls that turn on admin without auth (IAM gRPC opt-in, NetworkPolicy EVERYTHING, egress without kubeApiServer.cidrs, and the license ALL_ON dict), using the same key ci/admin-values.yaml already uses.	2026-09-08 20:51:53 -07:00
3 people	9b12d13934	volume server: release the store lock before scrubbing EC volumes (#11235) ... * volume server: release the store lock before scrubbing EC volumes `ec.scrub` makes a Rust volume server stop serving for the duration of the scrub, and then kills its own gRPC connection: error: rpc error: code = Unavailable desc = keepalive ping failed to receive ACK within timeout Measured on a 4.46 cluster (17 Rust volume servers on one host, ~520 volumes and 53 EC volumes, --index=redb, EC 10+4). It reproduces against a SINGLE node in 30-70s, in checksum, index and local modes, at -maxParallelization 1. ## Cause The CHECKSUM arm of scrub_ec_volume reads every byte of every local shard while holding the caller's store.read() guard: let store = self.state.store.read().unwrap(); let ecv = store.find_ec_volume(vid)...?; let (blocks, broken, errs) = ecv.checksum_scrub(); // GBs of I/O, lock held VolumeServerState::store is a std::sync::RwLock, which is write-preferring. The periodic heartbeat's collect_heartbeat_with_snapshot takes store.write() and blocks; once that writer is pending, every later store.read() queues behind it. Every HTTP handler takes store.read(), so the node serves nothing, stops heart-beating, and cannot answer the scrub RPC's own keepalive - the scrub kills the connection it is running on. The INDEX and LOCAL arms have the same shape, and the node-wide scrub_volume loop is worse: it held ONE guard across every volume on the node. ## Evidence offcputime, off-CPU stacks >1s in a 30s window during a scrub: futex_wait seaweed_volume::server::heartbeat::collect_heartbeat_with_snapshot - tokio-rt-worker 27967020 <- 27.97s blocked, of a 30s window A single HTTP /status request issued 12s into a scrub, with 180s of patience, was accepted and queued for 120 seconds, then served once the scrub released. Thread states throughout: 1 D + 48 S. One thread working, 48 idle - not executor starvation and no thread pileup, which is what a single lock holder looks like. Memory was tested and ruled out as the cause: the same scrub was run at MemoryMax 3G, 8G and unlimited. With no limit there is no reclaim at all, page cache grows freely to 22 GB, and the node still goes unresponsive at t+30s. anon stays flat at 48-86 MB in every run. ## Fix checksum_scrub, scrub_index and scrub_local gain plan types - EcChecksumScrubPlan, EcIndexScrubPlan and EcLocalScrubPlan - snapshotted from the volume under a brief guard. The handler builds a plan, drops the guard, and runs the scan in spawn_blocking, off the async workers, since it is synchronous CPU + file I/O either way. A plan captures DESCRIPTORS, not paths. Resolving a path again after the guard is dropped would let a writer that legitimately unlinks the files - the heartbeat's delete_expired_ec_volumes, which reaches EcVolume::destroy(), or volume_ec_shards_delete - surface an intentional removal as "scrub read error: No such file or directory" and put the volume in broken_volume_ids. A descriptor outlives the name. For the shards it duplicates the handle the mounted EcVolumeShard already holds (try_clone_file), which is what Go does: ChecksumScrub reads through shard.ReadAt (weed/storage/erasure_coding/ec_volume_scrub.go:71), never through a path. That also inherits open_volume_file's O_NOATIME and drops a dead branch - the old code built {base}.ec{id}.v{gen} for a non-zero generation, a name nothing in this tree writes. dup shares the kernel offset, so shard reads stay positional; the .ecx gets a fresh open instead, since check_index_file seeks. FULL/READS is unchanged here: it already released the guard across the index walk, and still re-takes it per needle in store_ec::scrub_snapshot_under_lock for that needle's local shard intervals - short holds, many of them. scrub_volume now takes the read guard PER VOLUME instead of across the whole loop, so the heartbeat can land between volumes. Its per-volume work still runs under the guard; Volume needs an equivalent plan to fix that properly, left as a follow-up and noted in the code. ## A failed scrub task must not take the whole RPC down Moving the scans into spawn_blocking changed where a panic lands. It no longer unwinds inside the handler's own future; it comes back as a JoinError at the .await, and all four join points sat behind a `?`. So one bad volume out of six hundred returned Err from the entire handler: the broken_volume_ids, broken_shard_infos and details already gathered for the other 599 were dropped, and emit_scrub_metrics - the only writer of SCRUB_LAST_TIME_SECONDS, SCRUB_VOLUME_FAILURES and SCRUB_SHARD_FAILURES - was never reached, so the staleness alert kept firing while real corruption went unreported. And there is a reachable panic behind it. EcLocalScrubPlan::run() sized its reassembly buffer with Vec::with_capacity(get_actual_size(size, version) as usize) which for any negative size that is not the -1 tombstone skipped above is a capacity-overflow abort. Mode 3 (LOCAL) is the default of `weed shell ec.scrub`, and a scrub is what you point at an index you already suspect, so an arbitrary i32 in a .ecx size field is in-scope input. The buffer is Rust-only - Go appends to a nil slice and has no capacity hint here. Guard on `want <= 0` and fall through with an empty buffer: locate_data returns no intervals for a non-positive size, read stays 0, and the existing `read != want` error reports the row exactly as Go does. Each join point now records the failure against its own volume and continues. A panic is evidence about the volume and counts as broken; a non-panic JoinError is not - spawn_blocking only reports one when the runtime is going down, the volume was never scanned, and counting it would put a false corruption into SCRUB_VOLUME_FAILURES. total_volumes moves before the join in modes 1, 3 and 4 (2|5 already counted there) so a failed join cannot silently shrink it. Mode 2|5's verify_ec_shards join is the one that must not `continue`: the needle walk above has already produced findings for that volume. The tombstone guard stays is_tombstone() on purpose. ScrubLocal in ec_volume_scrub.go:228 skips only IsTombstone(), while the distributed walk in store_ec.go:516 skips all IsDeleted() - the asymmetry is Go's, and both Rust walks mirror their own counterpart. ## Both servers: a node-wide scrub skips a volume that vanished mid-run Releasing the lock makes the volume set legitimately mutable during a scrub, so a node-wide run can reach a volume that has since been unmounted. That is not a scrub failure. A node-wide run now logs and skips it; an explicitly requested volume id still returns NotFound. The Go server is changed the same way, so both implementations answer the same shell command identically. mark_broken_volumes_readonly tolerates the same teardown one step later, instead of throwing away the whole scrub report. ## Test test_scrub_plans_are_self_contained_and_match_direct_call drops the EcVolume and runs both plans on another thread, asserting the results match the direct calls. A plan that borrowed from EcVolume could do neither, so the test stops compiling if the snapshot regresses to a borrow. test_scrub_plans_survive_files_removed_after_snapshot unlinks every shard and the .ecx after the plans are built, then asserts the results still equal the direct call. Against a path-resolving version it fails with all 14 shards reported as "No such file or directory". test_local_scrub_plan_reports_negative_size_ecx_row rewrites a .ecx row's size to -1000 and runs the local plan on another thread, so the join is the assertion - that thread is the spawn_blocking whose panic used to fail the RPC. Without the capacity guard it fails with "capacity overflow"; with it, the row is reported. The Go tests cover both halves of the vanished-volume rule for volumes and EC volumes. 517 lib tests pass, plus 34 across the other targets (`cargo test`). `go test ./weed/server -run Scrub` passes. ## Known remaining, not fixed here `ec.scrub -volumeId=N` is still fanned out to every node, and a node that holds no shard of N returns NotFound, so the shell command errors even when the nodes that do hold shards scrub cleanly. That is a shell-side fan-out question rather than a volume-server one, and both servers keep the existing behaviour for an explicitly requested id. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DvHoW85w6SNKNBPvrqLMmK * scrub: discard checksum block count from total_files; capture .ecx fd for FULL walk Two review fixes: 1. CHECKSUM arm: plan.run() returns blocks scanned, not a file count. Go discards it (_, shardInfos, serrs = v.ChecksumScrub()) so TotalFiles stays a needle/file count. The Rust arm was adding it to total_files, inflating the count. Discard it to match Go. 2. FULL/READS (scrub_ec_volume_distributed): the needle walk reopened the .ecx by PATH after the store guard was released, so a concurrent teardown that unlinks or replaces the .ecx (heartbeat delete_expired_ec_volumes, volume_ec_shards_delete) could surface an intentional removal as a scrub error or mix index generations within one scrub. Capture a second .ecx descriptor under the guard (the index plan handle is consumed by its own structural walk, and both seek) and read through it instead -- the same descriptor-outlives-name invariant the checksum plan shard handles use. * scrub: bind FULL/READS walk to one encode generation Address Devin review: after capturing the .ecx descriptor under the guard, scrub_snapshot_under_lock still re-resolves the volume by id per needle, so a teardown-and-remount of the same vid between two rows would apply the captured .ecx offsets to a replacement volume's shards -- falsely reporting corruption. Capture the volume's encode_ts_ns (encode-run identity) in Phase A and pass it to scrub_snapshot_under_lock. If the mounted volume's encode_ts_ns no longer matches, abort the walk like a mid-scan unmount instead of mixing generations within one scrub. * scrub: run FULL/READS index scan in the blocking pool Address CodeRabbit review (5147767192): index_plan.run() reads the whole .ecx synchronously, so running it on the async executor worker could block unrelated RPC work handled on the same executor. Move it into spawn_blocking, matching the treatment the CHECKSUM/LOCAL arms already give their plans. A join failure (panic/cancellation) is reported as a seed error so the per-volume findings below are not silently dropped. * scrub: move ecx walk to blocking pool, classify join errors, guard encode_ts_ns==0 Three CodeRabbit review fixes (5148034447): 1. Move the FULL/READS needle walk (walk_index_file over the captured ecx descriptor) into spawn_blocking. It reads the full .ecx synchronously and was still running on the async executor worker, the same blocker the index_plan.run() fix in the previous commit addressed. 2. Preserve JoinError classification in both spawn_blocking join points in scrub_ec_volume_distributed. A panic is evidence about the volume and counts as broken; a cancellation only happens at runtime shutdown, the volume was never scanned, and returning it as an error would put a false corruption into broken_volume_ids (the FULL/READS arm marks the volume broken on any non-empty errs). Panics return an error; cancellations return clean. 3. Do not treat encode_ts_ns == 0 as a verified generation match. The .vif assigns 0 when it carries no encode-run identity (legacy/pre-feature volumes), so 0 == 0 would accept a teardown-and-remount and apply the old .ecx offsets to the replacement volume's shards. Only enforce the generation check when the captured identity is non-zero; when it is zero, fall back to the pre-check behavior (no generation binding) rather than aborting a scrub that was already running without the guard. --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-09-08 19:12:30 -07:00
Chris Lu	723f473f02	filer: widen metadata-subscription readahead buffers (#11237) ... The metadata-subscription readahead channels were sized for a low-throughput era and now bottleneck replay catch-up: - ReadPersistedLogBuffer's readaheadSize was 1024 entries: the background visitor fills the channel, then blocks on the consumer's gRPC Send, so volume-server I/O for the next log file never overlaps with delivery of the current one. Each disk pass takes longer, and the subscribe loop re-lists log files (ListDirectoryEntries on the filer store) more often to drain the same backlog. Raised to 8192 so the reader stays ahead of the consumer through a full log file's worth of entries. - readFilersMerged's logEntryChannelSize was 512 entries per filer stream: the same serialization on the client side, where weed mount (chunk mode) reads persisted log chunks directly from volume servers. A small channel means the producer stalls on the merge consumer's processEventFn, and the next log file's chunks are never fetched ahead. Raised to 4096 so volume I/O overlaps with event delivery. The wider buffers keep the producer goroutines reading through a full log file while the consumer is still processing the previous one, turning serial read→process→read into pipelined read∥process. This cuts the per-pass wall time that drives filer store listings and volume-server round-trips, reducing filer workload under backlog catch-up (e.g. CSI deployments where ~200 mounts reconnect on filer restart).	2026-09-08 16:58:09 -07:00
Alessandro ZanattaandChris Lu	5f77a0b67e	admin: allow insecurely binding to any IP if -allowInsecureNoAuth is set (#11228) ... * admin: allow insecurely binding to any IP if -allowInsecureNoAuth is set * admin: rename -allowInsecureNoAuth to -allowInsecureBind The new flag name is shorter and clearer: it describes what is being allowed (an insecure bind to a non-loopback address) without the redundant "NoAuth" suffix. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-09-08 16:57:44 -07:00
Chris Lu	fd4fa72289	helm: pass -ip to admin so StatefulSet becomes Ready on 4.46 (#11236) ... * helm: pass -ip to admin so StatefulSet becomes Ready on 4.46 Since weed admin 4.46 changed its default listen address from all interfaces to loopback (127.0.0.1), the admin StatefulSet template never passed -ip, so the admin server bound to loopback only. The chart's httpGet readiness/liveness probes dial the pod IP, not loopback, so the probes never succeeded and the admin StatefulSet stayed 0/1 forever — breaking upgrades with helm --wait or GitOps controllers. Add an admin.ip value (default "0.0.0.0", restoring the pre-4.46 behaviour) and render it as -ip. A non-loopback bind requires authentication, so fail at render time when admin.ip is non-loopback and neither admin.secret.adminPassword nor admin.secret.existingSecret is set, instead of letting the pod crash-loop. Document the value and add a chart-testing CI values file. Bumps chart to 4.46.1. Fixes #11234 * helm: address review feedback on admin bind validation Align the chart's loopback classification with weed admin's isLoopbackIp (net.ParseIP + IsLoopback): the whole 127.0.0.0/8 range and ::1 are loopback; localhost and wildcard addresses are non-loopback, matching the binary. Previously the exact-string check rejected valid loopback addresses like 127.0.0.2 while permitting localhost (which the binary treats as non-loopback). Recognize WEED_ADMIN_PASSWORD supplied via admin.extraEnvironmentVars / admin.secretExtraEnvironmentVars as authentication, since weed admin picks it up through viper's AutomaticEnv. Previously such deployments were wrongly rejected at render time. Remove [https.admin] mTLS from the validation message and docs: the chart only generates [grpc.admin] (gRPC mTLS), not [https.admin] (HTTP mTLS), so mentioning it as an alternative was misleading. Document that the -ip flag requires SeaweedFS 4.46 or newer, so pinning admin.imageOverride to an older image is not supported with this chart. Extracted the loopback and auth checks into reusable helpers (seaweedfs.admin.isLoopbackIp, seaweedfs.admin.authEnabled) following the existing seaweedfs.filer.mysqlEnabled pattern. * helm: tighten loopback classification to reject malformed 127.x addresses Use regexMatch instead of hasPrefix for the IPv4 loopback check so malformed values like "127.not-an-ip" are not accepted as loopback (net.ParseIP returns nil for them, so weed admin treats them as non-loopback). Also recognize the expanded IPv6 loopback form "0:0:0:0:0:0:0:1" in addition to "::1", matching net.ParseIP behavior for the two common representations.	2026-09-08 16:18:27 -07:00
Chris Lu	cb9fcd39d2	filer/postgres: create filemeta table on startup via createTable config (#11229) ... * filer/postgres: create default filemeta table on startup The postgres filer store hardcoded CreateTableSqlTemplate to empty and never created the filemeta table, unlike postgres2/mysql2/sqlite which all create it during Initialize. Users had to create the table manually or the filer would crash loop with "relation filemeta does not exist". Read the createTable config option (same as postgres2), default to DefaultCreateTableQuery when unset, and execute CREATE TABLE IF NOT EXISTS on the default table after the connection pool is established. SupportBucketTable stays false so per-bucket table creation remains a no-op; only the shared filemeta table is created, via a direct ExecContext since AbstractSqlStore.CreateTable short-circuits without bucket support. * filer/postgres: accept boolean createTable = true/false viper reads a TOML boolean as the string "true"/"false" via GetString, so createTable = true was being used as a SQL template and failed. Add ResolveCreateTableQuery to normalize the value: true and empty select the default template, false disables table creation, anything else is a custom template. Both postgres and postgres2 now use it, and both skip the CREATE TABLE call when the resolved template is empty. * scaffold: document createTable option for postgres filer store Replace the commented-out CREATE TABLE SQL in the [postgres] scaffold with a createTable config hint, matching the [postgres2] section. Users no longer need to manually create the filemeta table before starting the filer. * filer/postgres: make createTable opt-in for postgres, keep postgres2 default The previous commit defaulted postgres to create the filemeta table even when createTable was unset, which could break existing deployments whose DB user lacks CREATE TABLE privileges. ResolveCreateTableQuery now returns empty for an unset value so postgres only creates the table when createTable is explicitly true or a custom template — preserving the prior no-DDL behaviour for existing configurations. postgres2 keeps its existing always-create default: it defaults an empty resolved value to DefaultCreateTableQuery, and only skips when createTable is explicitly false. * filer/postgres2: simplify createTable handling, document all modes Drop the false opt-out from postgres2 — it only skipped the default table while per-bucket CreateTable still ran, leaving restricted DB roles broken on bucket access. postgres2 now accepts true the same way (defaulting to DefaultCreateTableQuery) and keeps its existing always-create behaviour for every other value, matching the original semantics. The scaffold comment now documents true/false/custom for the postgres section so users know false (or unset) is the backward-compatible default. * filer/postgres2: normalize false via ResolveCreateTableQuery postgres2 only handled "" and "true", leaving createTable = false as the literal string "false" which CreateTable then executed as invalid SQL. Route it through ResolveCreateTableQuery (which maps false to empty) and default the empty result to DefaultCreateTableQuery, so false is treated the same as unset for the bucket-aware store. * filer/postgres2: honor createTable = false for default table postgres2 treated false the same as unset and always created the default filemeta table, failing startup for restricted DB roles that explicitly opted out. Track the original false value before ResolveCreateTableQuery collapses it to empty, and skip the default CreateTable call when set. Per-bucket table creation is unaffected — it is a runtime requirement of the bucket-aware store. Users who need to suppress all DDL should use the postgres (non-bucket) store with createTable unset. * filer/postgres2: disable bucket tables when createTable = false Setting SupportBucketTable = false when createTable is explicitly false makes AbstractSqlStore.CreateTable a no-op (it already returns nil when SupportBucketTable is false), so neither the default filemeta table nor per-bucket tables are created. The template stays empty and no DDL runs, honouring the opt-out for restricted DB roles. All data routes to the pre-provisioned filemeta table, matching the postgres (non-bucket) store. * filer: suppress DDL without disabling bucket routing Setting SupportBucketTable = false when createTable = false also disabled per-bucket routing, hiding objects in pre-provisioned per-bucket tables. Keep SupportBucketTable true and instead skip the CREATE TABLE execution when the resolved template is empty. GetSqlCreateTable now returns empty for both postgres and mysql SQL generators when CreateTableSqlTemplate is empty, and AbstractSqlStore.CreateTable skips the ExecContext call when the SQL is empty. This preserves bucket routing while suppressing all DDL for users who explicitly set createTable = false and pre-provision their tables. * filer: add SkipDDL to suppress CREATE and DROP without disabling routing createTable = false with SupportBucketTable = true preserved bucket routing but deleteTable still executed DROP TABLE on bucket deletion, dropping externally managed tables. CanDropWholeBucket also returned true, so the S3 layer tried whole-table drops instead of row-by-row deletes. Add a SkipDDL flag to AbstractSqlStore, independent of SupportBucketTable. CreateTable and deleteTable both skip when SkipDDL is set, and CanDropWholeBucket returns false so bucket deletion falls back to row-by-row metadata deletes. postgres2 sets SkipDDL when createTable is explicitly false — bucket routing is preserved, no DDL runs. * filer: fall back to row-by-row delete when CanDropWholeBucket is false DeleteFolderChildren took the whole-table drop path whenever the path was a bucket root, even when SkipDDL made deleteTable a no-op. The no-op returned nil, the caller returned early, and rows inserted after the recursive enumeration survived the bucket deletion. Gate the whole-table drop on CanDropWholeBucket so the row-by-row DeleteFolderChildren SQL runs when SkipDDL is set, removing all metadata without issuing DROP TABLE.	2026-09-08 16:17:02 -07:00
Chris Lu	8782749f26	admin: reject IAM policy deletion while still attached to a user/group (#11230) ... * admin: add IsPolicyAttached helper to detect user/group attachments Introduces AdminServer.IsPolicyAttached, which lists the users and groups that still have a managed policy attached, reusing the existing credential manager ListUsers / ListAttachedUserPolicies / ListGroups / GetGroup methods. This is the building block for rejecting policy deletion while a policy is still referenced, so deleted policy names stop lingering in a user attached policy names list (issue #11225). * admin: reject IAM policy deletion while still attached Guards AdminServer.DeletePolicy with the new IsPolicyAttached check and returns the typed ErrPolicyStillAttached error when the policy is still referenced by a user or group. This matches AWS IAM and the existing IAM API handler behavior, fixing the stale reference where a deleted policy name kept showing up in a user attached policy names list (issue #11225). * admin: return 409 Conflict when deleting an attached IAM policy The admin UI DeletePolicy handler now maps ErrPolicyStillAttached to HTTP 409 Conflict instead of 500, so the dashboard can surface the attachment conflict to the user rather than reporting a generic server error. * admin: skip vanished groups when checking policy attachments IsPolicyAttached now treats a group that disappears between ListGroups and GetGroup (credential.ErrGroupNotFound) as no longer attached instead of failing the whole deletion with HTTP 500, matching the IAM API handler which skips vanished groups. * test: assert policy state after deletion paths Strengthen GetPolicy assertions in the policy deletion tests to check the returned policy is non-nil after a rejected deletion and nil after a successful one, not just that no lookup error occurred (GetPolicy returns nil, nil when a policy is absent).	2026-09-08 16:16:01 -07:00
Chris Lu	b88156fe6b	fix(s3api): evaluate aws:SourceIp from the direct TCP peer, not forwarded headers (#11231) ... * fix(s3api): use direct peer IP for aws:SourceIp in bucket policy engine extractSourceIP in the bucket-policy engine trusted X-Forwarded-For and X-Real-Ip whenever the TCP peer looked private (loopback/RFC1918/link-local), with no configurable trusted-proxy allowlist. In containerized deployments the gateway peer is almost always private, so any caller reaching it directly or from a co-located workload could spoof aws:SourceIp and bypass IpAddress/NotIpAddress bucket-policy restrictions. Always return the direct peer address (r.RemoteAddr), matching AWS S3 semantics. Remove the now-unused isPrivateIP helper and header-trust branch. Update TestExtractConditionValuesFromRequestSourceIPPrecedence to assert the peer IP is used regardless of forwarding headers, and add regression tests TestExtractSourceIP_IgnoresForwardedHeaders and TestExtractSourceIP_EnforcesIPRestrictionPolicy. * fix(s3api): use direct peer IP for aws:SourceIp in IAM role/session policies The IAM middleware's extractSourceIP trusted X-Forwarded-For and X-Real-IP whenever the TCP peer looked private (loopback/RFC1918/link-local), with no configurable trusted-proxy allowlist. In containerized deployments the gateway peer is almost always private, so any caller reaching it directly or from a co-located workload could spoof aws:SourceIp and bypass IpAddress/NotIpAddress conditions on role and session policies (IsPrincipalActionExplicitlyDenied). Always return the direct peer address (r.RemoteAddr), matching AWS S3 semantics. Remove the now-unused isPrivateIP helper, privateNetworks table, and its init(). Update TestRequestContextExtraction and TestIPBasedPolicyEnforcement to assert the peer IP is enforced regardless of forwarding headers, and add regression test TestUserInlinePolicySourceIpCondition_IgnoresForwardedHeaders.	2026-09-08 15:09:31 -07:00
Chris Lu	557fffa350	iam: preserve native Admin when IAM policies are attached (#11226) (#11232) ... * iam: expose tri-state result from attached policy evaluation evaluateIAMPolicies returned a bool that collapsed explicit Deny and no-match into a single false, so the authorization path could not tell "policies forbid this" from "policies say nothing". Introduce evaluateAttachedIAMPolicies returning Allow/Deny/NoMatch and keep evaluateIAMPolicies as a bool projection for existing callers. This is preparation for unioning native permissions with attached policies while preserving deny-always-wins. * iam: preserve native Admin when IAM policies are attached Attaching an IAM policy routed authorization exclusively to the attached policies, dropping the identity native permissions. A user with native Admin lost all access after attaching a non-granting policy, and stayed locked out if that policy was deleted without being detached first (#11226). Treat a native bare Admin grant as a permission floor that survives attached policies: when the attached policies do not explicitly allow, fall back to isAdmin() on the attached-policy path, and on the IAM integration path allow unless an attached policy explicitly denies. Explicit Deny still wins on both paths. Only bare Admin is consulted because inline policies flatten lossily into Actions (dropping conditions), so scoped actions are not unambiguously native and must keep flowing through the policy engine. * iam: regression tests for native Admin surviving attached policies Reproduces issue #11226: - TestNativeAdminSurvivesAttachedPolicy: a user with native Admin keeps Write access after attaching a policy that does not grant it. - TestNativeAdminSurvivesDeletedPolicy: the same user keeps Write access after the attached policy is deleted without being detached. - TestAttachedPolicyExplicitDenyOverridesNativeAdmin: an explicit Deny in an attached policy still constrains a native admin (deny-always-wins). * iam: apply native Admin floor before IAM principal validation The native Admin floor in authorizeWithIAM ran after the auth-path switch, which denies when no session principal or PrincipalArn is present. An Admin identity without a PrincipalArn (no session token) was therefore denied before the floor executed. Move the floor ahead of the switch and derive the principal for its explicit-deny check with buildPrincipalARN, which already handles identities without a PrincipalArn. Adds a regression case for an Admin identity with an empty PrincipalArn. Addresses CodeRabbit review feedback on PR #11232.	2026-09-08 14:39:43 -07:00
Chris Lu	4a1d65939f	fix(mount): bound reader cache memory across open files (#11220) ... * fix(filer): bound retained reader cache buffers by bytes * test(filer): keep in-flight downloads during cache trimming * feat(mem): expose pooled allocation capacity for byte reservations * fix(mount): share a configurable reader buffer budget across files * fix(filer): release failed prefetch slots and memory reservations * feat(mount): expose a soft Go runtime memory limit * docs(filer): restore shared-download rationale in startCaching The one-line comment replacing the original context.Background() explanation was too thin for readChunkAt to cross-reference shared resource semantics. Restore a concise note on why request cancellation must not abort a download shared by concurrent readers. * test(filer): loosen reader cache test deadlines to 5s Three tests used 1-second deadlines that can flake on CI under load: TestReaderCacheBudgetInFlight, TestReaderCacheEvictionDoesNotHoldCacheLock, and TestReaderCacheFailedPrefetchReleasesBudget. Increase to 5 seconds. * test(filer): cover re-read after reader cache eviction Add TestReaderCacheReReadAfterEviction: reads chunk 'a', reads chunk 'b' (evicting 'a' via budget pressure), then re-reads 'a' and asserts a fresh download returns correct data. Verifies the core correctness property that eviction never exposes missing or stale data to readers.	2026-09-08 10:51:28 -07:00
ssshr-66	c6b330be2b	[Mount] Add ChunkGroup seeking tests and fix boundary handling (#11223) ... * fix issue 11221 * reply ai comments	2026-09-08 10:50:41 -07:00
Chris LuandCopilot	213f4c5d5c	release: wait for Go proxy propagation (#11219) ... Allow normal post-tag proxy propagation before dispatching downstream releases, while preserving the check that prevents them from pinning the previous commit. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>	2026-09-07 21:38:49 -07:00
github-actions[bot]	d997fba157	4.46 4.46	2026-09-08 03:45:21 +00:00
Chris Lu	c0a7dbb2bb	iam: bind CreateServiceAccount ParentUser to the caller (#11218) ... * iam: bind CreateServiceAccount target to caller in AuthorizeIamAction A non-admin holding iam:CreateServiceAccount could pass an arbitrary ParentUser and mint a service account for any identity, inheriting that identity permissions. Add a self-target category so a granted non-admin may only target their own identity; admins remain unrestricted. * iam: authorize CreateServiceAccount against its ParentUser target AuthIamManagement passed UserName as the authorization target for every action, so CreateServiceAccount was authorized with an empty target and the self-target binding never saw the caller-supplied ParentUser. Pass ParentUser for that action so the binding takes effect on the live path. * iam: test CreateServiceAccount binds target to caller Regression test: a non-admin holding iam:CreateServiceAccount may target itself but is denied targeting another identity; admins remain unrestricted. * iam: authorize CreateServiceAccount against ParentUser on the S3 port UnifiedPostHandler passed UserName as the authorization target for every IAM action, so CreateServiceAccount was authorized with an empty target on the S3-port route and the self-target binding never saw the caller ParentUser. Extract iamTargetUserName (ParentUser for CreateServiceAccount, UserName otherwise) and use it from both IAM dispatch surfaces so the binding applies on the live S3-port path as well as the standalone iam server. * iam: test CreateServiceAccount ParentUser binding on the S3 port End-to-end regression test through UnifiedPostHandler: a non-admin holding iam:CreateServiceAccount is denied (403) when targeting another identity and passes authorization when targeting itself.	2026-09-07 20:25:03 -07:00
ssshr-66andChris Lu	361fd6b263	[Filer] Parallelize Chunk Manifest Resolution to Reduce Large File Read Latency (#11215) ... * fix issue-11214 * fix(filer): cancel sibling manifest reads on failure * fix(filer): scope manifest cancellation to read batch and propagate context to encrypted reads Address PR review comments on #11215: - Scope cancellation to each parallel read batch instead of the resolver-wide context, so a later manifest failure does not cancel recursive work for an earlier successful manifest (CodeRabbit #3952446554). - Propagate the resolver context through GetAuthenticatedWithContext so encrypted sibling reads observe cancellation and stop promptly when another manifest fails (Greptile #3952422343). - Use net.ListenConfig.Listen with an explicit context in the test fixture to satisfy the noctx linter (CodeRabbit #3952111696). - Add regression tests for encrypted sibling cancellation and for preserving earlier manifest children on later failure. * fix(filer): return known manifest errors without blocking on earlier children Address Greptile review comment on #11215: After all parallel reads complete, pre-scan slots for the first real (non-internal-cancel) error before recursing into earlier manifests' children. If a later manifest already failed, return its error promptly with data chunks already in hand, instead of blocking on recursive network reads of earlier manifests' children. Updated the regression test to verify the error returns within 1 second when an earlier manifest's child has a 2-second delay, and that the child is never loaded. * fix(filer): filter partial child chunks by requested range on error path Address CodeRabbit review comment on #11215: The pre-scan error path appended non-manifest child chunks from earlier manifests without applying the [startOffset, stopOffset) overlap check used for top-level chunks. A child outside the requested range could be returned in dataChunks alongside the later manifest's error. Apply the same range predicate before appending. Add regression test with an out-of-range child chunk. * fix(filer): buffer job channel and abort submission on batch cancellation Address Greptile review comment on #11215: - Use a buffered job channel (capacity 128) so submission does not block when all workers are busy. This ensures a promptly-failing manifest is always queued and can cancel stalled sibling reads once a worker picks it up, instead of blocking the caller on the unbuffered channel send. - Add batchCtx.Done() to the submit select so submission aborts promptly when the batch is already cancelled by a sibling failure. - Add regression test with 5 manifests (4 stalled + 1 failing) verifying the failing job is queued and picked up after a stalled worker is freed. * fix(filer): avoid double WaitGroup decrement on batch cancellation in submit Address Devin review comment on #11215: When batchCtx.Done() fired in submit, it called job.done.Done() and returned false. The caller in resolve also called reads.Done() on the same WaitGroup, causing a double decrement that would panic with a negative counter. Fix: submit sets the result error but does not decrement the WaitGroup. The caller always owns the decrement and skips overwriting the result when submit already set it. * fix(filer): overflow execution when job queue buffer is full Address Greptile follow-up review comment on #11215: With a 128-entry buffer, if more than 132 in-range manifests (4 workers + 128 buffer) stall at one level, a promptly-failing manifest beyond the buffer cannot be submitted and cannot cancel the stalled reads. Fix: when the buffer is full, run the job directly in a goroutine instead of blocking on the channel send. This only triggers for >132 manifests at one level (exceedingly rare), so the bounded concurrency guarantee (4 workers) holds for all normal workloads. Extracted executeJob method shared by both workers and overflow goroutines. * fix(filer): bound overflow execution with a semaphore Address Devin review comment on #11215: The unbounded overflow goroutines could create thousands of concurrent reads for large files, defeating the four-worker resource bound. Fix: add a semaphore (capacity = maxChunkManifestResolveWorkers) that overflow goroutines must acquire before doing the read. While waiting for the semaphore, they also watch batchCtx and r.ctx so they exit promptly on cancellation. Total concurrency is now bounded to 2 * workers (4 workers + 4 overflow) in the degenerate case. * refactor(http): add ctx to GetAuthenticated signature instead of new function Reuse the existing GetAuthenticated name by adding ctx as the first parameter, matching the pattern of ReadUrl, ReadUrlAsStream, and RetriedFetchChunkData. Removes the GetAuthenticatedWithContext wrapper. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-09-07 18:44:47 -07:00
Chris Lu	75ec5ec193	admin: allow setting volume read-only and read/write modes (#11217) ... * admin: support setting volume read-only and read/write modes * admin: address PR review on volume access-mode persistence Reject trailing JSON values in the SetVolumeReadOnly handler so requests like {"read_only":true}{} no longer pass validation, and add a trailing-value case to the invalid-request test. Propagate .vif persistence failures through the access-mode chain. PersistReadOnly now returns the SaveVolumeInfo error and rolls back the in-memory volumeInfo on failure; Store.MarkVolumeReadonly and Store.MarkVolumeWritable propagate that error and roll back their noWrite flags, so the API reports failure instead of success while restart would revert the mode. * admin: make .vif persistence atomic and preserve error chain SaveVolumeInfo now writes to a .vif.tmp file, syncs it, renames it over the target, and fsyncs the directory. A write/sync/close failure leaves the existing .vif intact, so the PersistReadOnly in-memory rollback matches the durable state instead of diverging from a partially written file that restart would apply. Switch the error wrappers in PersistReadOnly, MarkVolumeReadonly, and MarkVolumeWritable from %v to %w so callers can use errors.Is and errors.As to classify persistence failures. * admin: treat post-rename dir fsync failure as a warning After os.Rename commits the new .vif, the on-disk file already holds the requested mode. A directory fsync failure only risks losing the rename across a crash; returning an error here would make PersistReadOnly roll back in-memory state while the durable file keeps the new mode, splitting the replica. Log the failure as a warning instead, matching the best-effort nature of FsyncDir (already skipped on Windows). * admin: distinguish post-rename durability failures and use unique temp files SaveVolumeInfo now uses os.CreateTemp for the staging file, preventing concurrent saves for the same volume from colliding on a shared .tmp path. A directory fsync failure after os.Rename returns a NotCrashDurableError instead of being silently swallowed. The rename already committed the new metadata to disk, so PersistReadOnly, MarkVolumeReadonly, and MarkVolumeWritable skip the in-memory rollback for this error type (keeping state aligned with the durable file) while still propagating the failure to the API. Pre-commit failures continue to roll back as before. * admin: continue post-commit work after NotCrashDurableError MarkVolumeWritable now clears the EIO quarantine and the gRPC handlers (makeVolumeReadonly step 3, makeVolumeWritable master notification) proceed with their post-commit work when SaveVolumeInfo returns a NotCrashDurableError, instead of aborting and leaving the volume unavailable or the master unaware of the mode change. The durability warning is still propagated to the API caller. Pre-commit failures continue to abort early as before. * admin: handle NotCrashDurableError in tier and EC callers VolumeTierMoveDatFromRemote and VolumeEcShardsGenerate now check for NotCrashDurableError from SaveVolumeInfo. When the rename has already committed the new .vif, they continue with their post-commit work (backend switch, remote deletion, keeping generated EC shards) instead of aborting and leaving the on-disk metadata inconsistent with the file layout. The durability warning is logged for the operator.	2026-09-07 18:40:37 -07:00
github-actions[bot]	53a18ecadd	docs: regenerate star history chart	2026-09-08 00:54:44 +00:00
dependabot[bot]andChris Lu	2d4b730a2f	build(deps): bump github.com/twmb/avro from 1.7.2 to 1.8.0 (#11210) ... * build(deps): bump github.com/twmb/avro from 1.7.2 to 1.8.0 Bumps [github.com/twmb/avro](https://github.com/twmb/avro) from 1.7.2 to 1.8.0. - [Commits](https://github.com/twmb/avro/compare/v1.7.2...v1.8.0) --- updated-dependencies: - dependency-name: github.com/twmb/avro dependency-version: 1.8.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * iceberg: adapt to twmb/avro v1.8.0 and iceberg-go defensive copies avro v1.8.0 changes Schema.Root() to return *SchemaNode, which breaks iceberg-go v0.6.0's internal avro_schemas.go. The fix (apache/iceberg-go#1843) is only on iceberg-go's main branch, unreleased, so bump iceberg-go to that commit (c210509) alongside the avro bump. That iceberg-go revision also changes two behaviors seaweedfs worked around: - It now infers a manifest list's format version from the embedded writer schema, so a list missing the "format-version" header entry (DuckDB's shape is read as v2, not v1. ReadManifestList's header patching is now a redundant safety net; tests updated to expect v2. - It returns defensive copies from DataFile.Partition(), so the ReadManifest shim's in-place partition normalization was silently discarded. Rebuild the entry through NewDataFileBuilder when any partition value is normalized, copying every other DataFile field so manifest round-trips are preserved. - It converts day-transform partitions to iceberg.Date on read (applyDayTransformDates), so the day-partition cases the shim and tests guarded now convert without help; tests updated to expect iceberg.Date from the raw read. EOF ) * iceberg: accept assert-ref-snapshot-id without snapshot-id iceberg-go's new nullableInt64 parser rejects an assert-ref-snapshot-id requirement whose "snapshot-id" field is absent from the JSON, even though the Iceberg REST spec makes it optional (null means the ref must not already exist). v0.6.0 used a plain *int64, so absent was nil and accepted. ClickHouse sends the requirement without snapshot-id when asserting a branch does not yet exist, so its writes fail with "missing required field \"snapshot-id\"". normalizeRequirements splices an explicit null into any assert-ref-snapshot-id requirement missing the field before handing the JSON to iceberg-go's parser, restoring the v0.6.0 behavior across both iceberg-go versions. * iceberg: fix v1 block_size_in_bytes default in rebuilt manifest entries rebuildManifestEntry set block_size_in_bytes to 0, but the v1 manifest schema requires the default of 64 MiB ("Always write default in v1"). The original value is not exposed on the DataFile interface, so use the spec default. Also clarify the fallback comment to note that empty (zero-record / zero-byte) files also trigger it, not just a nil spec. Add a round-trip test that writes a rebuilt entry as v1 and verifies block_size_in_bytes is 64 MiB via Avro decoding. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-09-07 15:58:24 -07:00
3 people	005012edcf	rust volume: quick-repair redb on durable checkpoints (#11203) ... * rust volume: insert redb rebuilds in needle-id order Unlink the .rdb before create (create does not truncate). Collapse last-write-wins, then insert live keys sorted so 4.2.0 packs leaves. * rust volume: rebuild redb from a BTreeMap and clear leftover keys Peak rebuild memory is one ordered map instead of HashMap + Vec + stable-sort scratch. Unlink stays best-effort: if it fails, retain clears the leftover table before sorted insert. Compute idx metrics before the write so a read error does not unlink a committed .rdb. * rust volume: drop the extra redb read transaction on put/delete put uses insert()'s previous value. delete gets then inserts the tombstone in the same write transaction. Truncate the .idx row on any failed redb write after the append. * rust volume: unpack redb blobs through packed_to_needle_value save_to_idx, ascending_visit, and collect_entries used the same length-check copy as get. Route them through the helper so a wrong-length value is absent everywhere, not a panic. * rust volume: quick-repair redb on durable checkpoints set_quick_repair(true) on the durable checkpoint transaction so an OOM-killed volume server opens without a full-file repair scan. * rust volume: reopen redb from .idx on non-poisoned commit error redb 4.2.0 can make a Durability::None commit visible before returning Err(CommitError::Storage(..)). In that state the database refuses further write transactions, so truncating the .idx row (the old behavior) would leave a redb-only put or tombstone that the stored idx_size makes the reload skip. Distinguish CommitError::TransactionPoisoned (txn rolled back, db still usable -- truncate the orphan .idx row as before) from other commit errors (change may be visible, db refuses writes -- keep the .idx row, close the database, and reopen from .idx to repair redb's internal state). db becomes Option<Database> so reopen_from_idx can drop the old file lock before load_from_idx opens the same path. rdb_path, version, and cache_bytes are stored so the reopen uses the same configuration. * rust volume: truncate .idx row when redb is closed in put put appends the .idx entry before acquiring the write transaction. When db_or_err() fails (db is None after a failed reopen), the ? returned without calling truncate_idx_to_offset, so a write reported failed remained in the authoritative .idx and was replayed on restart. Handle db_or_err() explicitly and truncate the orphan .idx row before returning the error, matching the existing handling for begin_write, open_table, and insert failures. --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-09-07 13:50:49 -07:00
Chris Lu	5d5ea18287	topology: fix fatal concurrent map read/write on VolumeLayout.crowded (#11216) ... SetVolumeCrowded mutated the crowded map under accessLock.RLock(), while GetWritableVolumeCount reads the same map under RLock() on the Assign hot path. Two concurrent RLock holders with one writing and one reading the map triggers a fatal "concurrent map read and map write" that kills the master process (unrecoverable, bypasses recover). Take the write lock in SetVolumeCrowded instead. This event path is a low-frequency single consumer driven by the crowded-volume event loop, and every other mutation of crowded already holds Lock(); setVolumeCrowded takes no nested locks, so there is no deadlock path. The hot readers (GetWritableVolumeCount, CloneWritableVolumes) keep using RLock. Adds a -race regression test that fails (race detected) on the old RLock and passes with the write lock. Fixes #11211	2026-09-07 13:47:22 -07:00
dependabot[bot]	34936c610d	build(deps): bump helm/kind-action from 1.14.0 to 1.15.0 (#11212) ... Bumps [helm/kind-action](https://github.com/helm/kind-action) from 1.14.0 to 1.15.0. - [Release notes](https://github.com/helm/kind-action/releases) - [Commits](https://github.com/helm/kind-action/compare/v1.14.0...v1.15.0) --- updated-dependencies: - dependency-name: helm/kind-action dependency-version: 1.15.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-09-07 13:27:09 -07:00
dependabot[bot]	5e3af51d91	build(deps): bump docker/setup-qemu-action from 4.2.0 to 4.3.0 (#11213) ... Bumps [docker/setup-qemu-action](https://github.com/docker/setup-qemu-action) from 4.2.0 to 4.3.0. - [Release notes](https://github.com/docker/setup-qemu-action/releases) - [Commits](https://github.com/docker/setup-qemu-action/compare/v4.2.0...v4.3.0) --- updated-dependencies: - dependency-name: docker/setup-qemu-action dependency-version: 4.3.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-09-07 13:27:01 -07:00
Junker der ProvinzandJunker der Provinz	a4885b7975	log_buffer: stop closing a notification channel another reader still holds (#11177) ... * fix(log_buffer): stop closing a notification channel another reader still holds - #10810 The report blames the polling loop for the busy spin, but that loop is not what burns the core. LogBuffer keeps one notification channel per subscriberID, and UnregisterSubscriber closes it. Two registrations that share a subscriberID share that channel, which happens whenever a client opens a second stream or an old stream has not yet noticed it was replaced, so the first unregister closes a channel the other reader is parked on. A closed channel makes every receive in awaitNotificationOrTimeoutFor return instantly, and that reader then spins at full speed for the rest of its life. Subscriptions are now reference counted. Registering an existing subscriberID hands back the same channel and raises the count; the channel is only closed when the last holder unregisters. * test: fail if the surviving reader stops instead of keeps reading Review caught that the iteration count alone proves nothing: had LoopProcessLogData returned when the duplicate reader unregistered, the counter would sit at 0 and the assertion would pass without a reader ever having been there to spin. Check the reader is still running before trusting its low count. --------- Co-authored-by: Junker der Provinz <jdp@braethoria.com>	2026-09-07 13:20:27 -07:00
dependabot[bot]	e0f9e02761	build(deps): bump github.com/prometheus/client_model from 0.6.2 to 0.6.3 (#11208) ... Bumps [github.com/prometheus/client_model](https://github.com/prometheus/client_model) from 0.6.2 to 0.6.3. - [Release notes](https://github.com/prometheus/client_model/releases) - [Commits](https://github.com/prometheus/client_model/compare/v0.6.2...v0.6.3) --- updated-dependencies: - dependency-name: github.com/prometheus/client_model dependency-version: 0.6.3 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-09-07 13:19:23 -07:00
dependabot[bot]	3f8cc380fb	build(deps): bump cloud.google.com/go/storage from 1.64.0 to 1.67.0 (#11206) ... Bumps [cloud.google.com/go/storage](https://github.com/googleapis/google-cloud-go) from 1.64.0 to 1.67.0. - [Release notes](https://github.com/googleapis/google-cloud-go/releases) - [Changelog](https://github.com/googleapis/google-cloud-go/blob/main/CHANGES.md) - [Commits](https://github.com/googleapis/google-cloud-go/compare/compute/v1.64.0...compute/v1.67.0) --- updated-dependencies: - dependency-name: cloud.google.com/go/storage dependency-version: 1.67.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-09-07 13:19:12 -07:00
dependabot[bot]	3eba8ebb9b	build(deps): bump github.com/go-ldap/ldap/v3 from 3.4.13 to 3.4.14 (#11209) ... Bumps [github.com/go-ldap/ldap/v3](https://github.com/go-ldap/ldap) from 3.4.13 to 3.4.14. - [Release notes](https://github.com/go-ldap/ldap/releases) - [Commits](https://github.com/go-ldap/ldap/compare/v3.4.13...v3.4.14) --- updated-dependencies: - dependency-name: github.com/go-ldap/ldap/v3 dependency-version: 3.4.14 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-09-07 13:19:00 -07:00
Eliah Rusin	ed5b342f0c	rust volume: optional redb insert_before bulk load (#11205) ... * rust volume: quick-repair redb on durable checkpoints set_quick_repair(true) on the durable checkpoint transaction so an OOM-killed volume server opens without a full-file repair scan. * rust volume: optional redb insert_before bulk load Behind redb-experimental-cursor (default off). Production binary stays on sorted insert(). CI unit tests run both feature settings. * rust volume: exercise insert_before across leaf splits Replace the 5-key cfg clone with a 4000-key reverse-order rebuild so CursorMut::insert_before hits page splits. CI runs the feature only on storage::needle_map unit tests.	2026-09-07 13:15:47 -07:00
dependabot[bot]	99b84eeb88	build(deps): bump github.com/getsentry/sentry-go from 0.48.0 to 0.49.0 (#11207) ... Bumps [github.com/getsentry/sentry-go](https://github.com/getsentry/sentry-go) from 0.48.0 to 0.49.0. - [Release notes](https://github.com/getsentry/sentry-go/releases) - [Changelog](https://github.com/getsentry/sentry-go/blob/master/CHANGELOG.md) - [Commits](https://github.com/getsentry/sentry-go/compare/v0.48.0...v0.49.0) --- updated-dependencies: - dependency-name: github.com/getsentry/sentry-go dependency-version: 0.49.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-09-07 09:37:41 -07:00
Eliah RusinandChris Lu	b6690cfbc8	rust volume: drop the extra redb read transaction on put/delete (#11204) ... * rust volume: insert redb rebuilds in needle-id order Unlink the .rdb before create (create does not truncate). Collapse last-write-wins, then insert live keys sorted so 4.2.0 packs leaves. * rust volume: rebuild redb from a BTreeMap and clear leftover keys Peak rebuild memory is one ordered map instead of HashMap + Vec + stable-sort scratch. Unlink stays best-effort: if it fails, retain clears the leftover table before sorted insert. Compute idx metrics before the write so a read error does not unlink a committed .rdb. * rust volume: drop the extra redb read transaction on put/delete put uses insert()'s previous value. delete gets then inserts the tombstone in the same write transaction. Truncate the .idx row on any failed redb write after the append. * rust volume: unpack redb blobs through packed_to_needle_value save_to_idx, ascending_visit, and collect_entries used the same length-check copy as get. Route them through the helper so a wrong-length value is absent everywhere, not a panic. --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>	2026-09-07 09:31:00 -07:00
Eliah Rusin	94b10c006d	rust volume: insert redb rebuilds in needle-id order (#11202) ... * rust volume: insert redb rebuilds in needle-id order Unlink the .rdb before create (create does not truncate). Collapse last-write-wins, then insert live keys sorted so 4.2.0 packs leaves. * rust volume: rebuild redb from a BTreeMap and clear leftover keys Peak rebuild memory is one ordered map instead of HashMap + Vec + stable-sort scratch. Unlink stays best-effort: if it fails, retain clears the leftover table before sorted insert. Compute idx metrics before the write so a read error does not unlink a committed .rdb.	2026-09-07 09:29:12 -07:00
Chris Lu	15e4da65f7	volume: avoid read-only replica write targets (#11195) ... * master: carry replica read-only state in volume lookups * volume: refresh writable replica targets * volume: preserve read-only replicas for deletes * master: propagate read-only delete capability * volume: target delete-capable replicas * volume: honor configured HTTPS for replica deletes * volume: reject insecure delete authorization forwarding * master: broadcast delete capability changes * volume: align Rust replica routing * http: protect credentialed replica redirects * master: preserve digest compatibility for delete capability * volume: propagate read-only state in short heartbeats * volume: report changed short volume state * http: guard TLS client redirects * master: announce mounted volume read-only state * volume: replace changed identity deltas * master: replace incremental volume layouts in order * master: keep moved volume lookup available * volume: announce read-only mounts	2026-09-07 09:23:56 -07:00
Tim Olowandtimolow	331c6c3642	shell: volume.check.disk — actionable verdict for diverged vacuumed replicas (#11197) ... * shell: volume.check.disk — actionable verdict for diverged vacuumed replicas When -resurrectMissingNeedles is gated off because both replicas have been vacuumed (compaction revision > 0) — the normal state of any production cluster — check.disk previously stopped at 'cannot prove they are missing writes vs vacuumed deletes' and did nothing, leaving a diverged replica with no repair path. volume.fix.replication does not catch it either: it only acts when the replica COUNT is below the expected replication, never when two replicas are both present but hold different live data. Classify the divergence instead of dead-ending: liveDivergence() counts live (non-deleted) needles present on one replica's index but entirely absent from the other, in both directions. Tombstones are excluded, so vacuum asymmetry (a compacted replica that dropped deleted entries) is not mistaken for divergence. reportDivergenceVerdict() turns the count into an operator action: - one-sided (one replica has all the live data, the other has no unique live needles) -> print the exact safe repair: volume.copy -source <complete> -target <lagging> -volumeId <id> Re-copying the complete replica is safe precisely because the lagging side holds no unique live data; VolumeCopy's verify-before-destroy gate independently confirms the source holds the volume before deleting the target. - two-sided (split-brain, both sides have unique live data) -> warn and do NOT emit an auto repair; point to volume.fsck -findMissingChunksInFiler to confirm the 'missing' needles are orphans before converging. Report-only: no data is modified and the resurrection safety gate is untouched. This is what lets a 13-volume diverged cluster be diagnosed and repaired in minutes instead of by hand-diffing every index. Observed motivating case: home SeaweedFS 4.45 cluster, 13 010 cross-rack volumes diverged after failed replicate writes (ReplicatedWrite MaxAttempts=1 fire-and-forget), 11 one-sided + 2 two-sided, all repaired via volume.copy. * shell: volume.check.disk — address review nits on divergence verdict - Use pb.NewServerAddressFromDataNode (dialable ip:port, Address with Id fallback) for the advertised volume.copy -source/-target instead of the logical node Id, which may not be dialable. - Make the one-sided verdict tombstone-aware: when the lagging replica has been vacuumed, absent live needles may be valid deletions whose tombstones were dropped, so a whole-volume re-copy would resurrect them. The command is only advertised as safe when the lagging side is proven never-vacuumed (compaction revision 0 read under -resurrectMissingNeedles); otherwise a caveat is printed pointing at fsck/needle-level repair. - Fix reversed copy direction when the source replica is the lagging one (must copy complete -> lagging in both cases). - Test: real tombstone (negative size) with the correct 0/0 expectation and || assertion; verdict test now covers dialable address, corrected direction, and caveat on/off. * shell: volume.check.disk — per-replica revision knowledge, no copy command for vacuumed lagging side - Track srcRevKnown/tgtRevKnown separately: in unidirectional mode the target revision IS read, so a proven never-vacuumed target no longer gets a false resurrection warning (regression: bidi=false, target rev 0). - A one-sided verdict now only emits the volume.copy command when the lagging side is proven never-vacuumed; when it is vacuumed (or unproven) the verdict refuses to print the destructive command and points at fsck/needle-level repair instead — an appended caveat next to a ready-to- paste copy command was still inviting the resurrection. - deletionCaveat now returns the boolean safety decision. * shell: volume.check.disk — preserve gRPC port, proven two-sided is not split-brain - Emit the raw ServerAddress string (host:port.grpcPort) instead of String()/ToHttpAddress(), which drops the custom gRPC port and would make the suggested volume.copy dial the default port and fail. - Two-sided divergence with both replicas proven never-vacuumed under -resurrectMissingNeedles is mutually missed writes, not split-brain: recommend re-running with -apply (in-place resurrection both directions) instead of the split-brain no-auto-repair warning. - Tests: case F (proven two-sided -> -apply, no split-brain warning), case G (custom gRPC ports preserved in emitted addresses). --------- Co-authored-by: timolow <tim@timolow.dev>	2026-09-06 23:25:28 -07:00
Chris Lu	3225d2b0ce	rust worker: install rustls CryptoProvider to fix TLS panic (#11194) (#11196) ... * rust worker: add install_default_crypto_provider helper lance's aws backend pulls aws-lc-rs and reqwest's rustls-tls pulls ring, so rustls 0.23 cannot auto-select a CryptoProvider and tonic's client TLS panics on first use. Add install_default_crypto_provider, pinning the default to aws-lc-rs, mirroring the Rust volume server's helper of the same name. Includes a regression test that builds a TLS channel and panics without the install in this crate, where both providers link. * rust worker: install the crypto provider at startup Call install_default_crypto_provider before any TLS use, the way the Rust volume server does in its main. Without this a worker started with --tls-ca/--tls-cert/--tls-key panics on the first admin dial (#11194).	2026-09-06 22:05:40 -07:00
github-actions[bot]	8537d5bc08	docs: regenerate star history chart	2026-09-07 01:07:12 +00:00
Chris Lu	70a26cb5d2	s3: gate IAM-cache gRPC RPCs behind admin Bearer auth (#11190) ... * s3: gate IAM-cache gRPC RPCs behind admin Bearer auth The SeaweedS3IamCacheServer registered on the S3 gateway's internal gRPC port (default 0.0.0.0:18333) accepted PutIdentity/RemoveIdentity/PutPolicy/ DeletePolicy/GetPolicy/ListPolicies/PutGroup/RemoveGroup with no per-RPC authentication. An unauthenticated network peer could call PutIdentity with Actions:[Admin] and write straight into the live accessKeyIdent map that the SigV4 path reads, bypassing S3 authentication entirely. Mirror the filer's IamGrpcServer.checkAdminAuth: require a Bearer token signed with jwt.filer_signing.key (read from the existing s3a.filerGuard) at the top of every IAM-cache RPC. With no key configured the check is a no-op, matching the rest of SeaweedFS's gRPC surface. * credential: attach admin Bearer token to S3 IAM-cache propagation The filer's PropagatingCredentialStore fans IAM mutations out to peer S3 servers over the SeaweedS3IamCache gRPC service. Now that the S3 handlers require a Bearer token signed with jwt.filer_signing.key, attach one to the outgoing propagation context (mirroring shell/iamAdminAuthContext). With no key configured it is a no-op, so deployments that run without the signing key keep working. * credential: mint IAM-cache admin token after master discovery propagateChange attached the admin Bearer token before ListClusterNodes, so master-client retries could run down the (default 10s) token lifetime before the peer S3 fan-out began, leaving peers to reject an expired token and IAM caches stale. Move withIamCacheAdminAuth to after discovery succeeds, immediately before the propagation timeout is derived. * credential: cap IAM-cache propagation timeout below JWT lifetime The propagation fan-out used a fixed 10s timeout. If an operator configures jwt.filer_signing.expires_after_seconds below 10, the admin token can expire while slower S3 peers are still being contacted, leaving their IAM caches stale. Derive the propagation deadline as min(10s, tokenTTL) so it never outlives the token. withIamCacheAdminAuth now returns the token's lifetime (0 = no expiry) for this purpose.	2026-09-06 12:21:51 -07:00
Eliah RusinandClaude Opus 5	ade4bdf9e6	rust volume: stop a tier move whose caller has gone (#11192) ... Both tier-move handlers run in a detached tokio::spawn and report progress through a closure that returns (), with the send result discarded. Nothing observes the caller leaving, so an abandoned move uploads or downloads the whole .dat anyway and then commits the transition. Go aborts both. Its progress callback returns `stream.Send`'s error, which surfaces out of the reader in s3_upload.go:99 and the writer in s3_download.go:84 and fails the transfer, so the volume info is never rewritten. The Rust port dropped that by typing the callback as FnMut(i64, f32) with no result. Give the callback Go's signature -- FnMut(i64, f32) -> Result<(), String> -- and abort when the caller's channel is closed. Checked on every part rather than only where progress is reported, since the report is rate-limited to one a second and would miss a caller that left in between. A merely full channel is a slow reader, not a departed one, so only TrySendError::Closed counts as cancellation. Two consequences of aborting mid-transfer that the old code never had to handle: - upload_file now aborts the multipart upload when the transfer fails. An abandoned multipart upload does not show up in an ordinary object listing but still accrues storage charges until a lifecycle rule reaps it, and cancellation makes that a routine path rather than a rare one. - The tier-down handler removes the partial .dat. download_file pre-allocates the destination to the object's full size, so an aborted download leaves a file of the right length and the wrong content -- and this handler refuses to run at all when a local .dat exists, so leaving one wedges every retry on "already on local disk" and a restart would load the sparse file as the volume's data. There is deliberately no check between a finished transfer and the bookkeeping that follows. Once the object is in S3, or the .dat is on disk, that bookkeeping is what makes the state consistent; stopping there would leave an object paid for and referenced by nothing, or a complete local .dat the volume still calls remote. Go does not gate there either -- its callback only runs during the transfer. Claude-Session: https://claude.ai/code/session_0122W3eqt6gmLUMxmRoZdPAb Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>	2026-09-06 12:18:47 -07:00