mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-16 12:16:36 +00:00
e377149d3925853641fbadcb43130a9edaacf5ea
14621
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
e377149d39 |
mount: support mounting on Windows through WinFsp (#10536)
* mount: add the WinFsp filesystem adapter WinFsp speaks a path-based FUSE dialect; weed/mount implements the inode-based raw protocol the Linux kernel uses. This translates between them so Windows runs the same filesystem code as everywhere else rather than a second implementation: paths resolve to inodes one Lookup at a time, and the raw operations run unchanged underneath. Errno translation is spelled out rather than passed through. Go numbers Windows errnos as offsets from APPLICATION_ERROR, so the raw value would mean something unrelated by the time WinFsp read it. Hard links return ENOSYS since WinFsp has none, and byte-range locks stay with its kernel driver rather than the mount's lock table. Not reachable from the mount command yet. * mount: build the winfsp errno table with explicit precedence Platforms alias errnos differently: freebsd has no ENODATA and linux makes ENOATTR the same value as it. A map literal with colliding constant keys does not compile, so build the table and let the first entry win, keeping the general codes their own meaning. * mount: wire the winfsp adapter into the mount command RunMount was one function doing filer setup, mount-point preparation and serving. The setup is the same everywhere, so it moves to mount_common.go and each platform keeps only what differs. Windows differs mostly in the mount point: WinFsp wants a drive letter or a path that does not exist yet, so none of the unix preparation applies, and a bad one is worth rejecting up front because WinFsp reports failure as a bare false. Adds -windows.caseInsensitive for software that expects Windows naming rules. * ci: mount on windows and exercise it Builds weed.exe, installs WinFsp, starts a cluster, mounts S: and runs a test suite against it: round trips at several sizes, offset writes, rename, delete, nested directories, concurrent writers, and a directory wide enough to stand in for the case that prompted this. Nothing else here can run the Windows mount, so without this the adapter is only known to compile. * ci: build the windows mount without cgo The runner has MinGW, so cgo is on by default and cgofuse compiles its cgo variant, which needs WinFsp's headers. The nocgo variant loads the DLL at run time and is what the released weed.exe uses. * mount: make the winfsp path splitting portable and test it resolve and resolveParent had the splitting inline in a windows-tagged file, so the cases that matter most there — both separators, empty and dot components, the root having no parent to create in — could not be tested on any runner that builds this. * test: check the windows mount persists across a remount Reading a file back through the same live mount proves nothing about durability; the answer can come from the mount's own caches. Write the fixtures, confirm the filer serves them with the mount out of the path, then re-read after a teardown and remount. * test: cover the windows mount operations that had none Truncate, append, chtimes and the hard-link refusal were implemented but never exercised, and the errno table was only unit-tested for mapping, never end to end. Adds names that have to survive the UTF-16 boundary, rename over an existing target and across directories, and concurrent handles on one file rather than one file each. * ci: dial the filer over ipv4 and run the persistence phases localhost resolves to ::1 first on windows and the cluster binds ipv4 only, so the mount's grpc dial was refused while the http readiness probe passed by falling back to ipv4. * ci: pin the cluster to loopback and probe ports by connecting weed mini advertises the runner's LAN address and binds filer grpc there, so the mount's dial to 127.0.0.1:18888 was refused while http answered. The readiness probe also passed with nothing on 18888: Test-NetConnection reported success for a port that then refused a connection, so it now opens a socket instead. * ci: report listening ports before mounting The readiness probe connects to the filer grpc port and the mount is then refused on it, which cannot both be true; print the actual state. * ci: run the cluster, mount and tests in one step The runner tears down a step's process tree when its shell exits, so the cluster started in an earlier step was already gone: the readiness probe passed against a live filer, the step ended, and the mount then found nothing listening. A diagnostic step reported no weed.exe at all. Everything that needs those processes alive now shares a step. * mount: key windows file io on the handle, not the path Read and Write walked the path on every call to fill in a NodeId the raw filesystem never reads: both look the file up by handle. Under eight writers creating files in one directory the walk transiently missed and the write failed with ENOENT before reaching the filesystem at all. Same for flush, fsync and the release calls. O_EXCL now fails on an existing name instead of taking it over, and Symlink is refused: the entry is easy to create but WinFsp only follows it once the reparse point is wired up, so it read back as an empty file. * mount: translate cgofuse open flags for windows cgofuse reports MSVC's numbering and the raw filesystem tests Go's, so only the access mode and O_TRUNC lined up: O_EXCL arrived as O_APPEND and O_CREAT as nothing at all. Also report which handle a failed write was using, to tell a handle that was never issued from one released while still in use. * mount: report which step of a windows create failed A concurrent create fails with ENOENT and the path walk, the parent lookup and the create itself are indistinguishable from the caller. * ci: send weed logs to stderr on windows glog writes to its own files by default, so the mount's own error output never reached the redirected log. Its flags are global and have to come before the subcommand. * mount: resolve known paths from the inode table on windows Every create walked the parent chain with a filer lookup per component. With eight writers creating files in one directory that is hundreds of concurrent lookups of the same parent, and lookupEntry reports an authoritative ENOENT when the directory is cached, the entry is not in the cache and the inode table has no record — a window a concurrent refresh can open for a directory that plainly exists. A path the mount already tracks now resolves straight out of that table. * test: sync the windows persistence fixtures before closing The mount is killed rather than unmounted, so anything still queued for flush is legitimately lost and the test was measuring crash durability while calling it persistence. A 9MB file lost four chunks that way. * mount: keep the lookup refresh on the target path Resolving a tracked path straight from the inode table skipped Lookup, which is also what refreshes the entry: a truncate then read back the pre-truncate size. Only the parent chain takes the shortcut now, which is where the concurrent creates were racing anyway. * mount: log every windows resolve failure Open suppressed ENOENT and Getattr logged nothing, which hid the two callbacks that can report a missing file during a create. * mount: drop dot entries from windows directory listings readdir reports "." and ".." for the kernel, but Windows enumerates a directory without them and displays whatever it is handed, so a folder of 200 files listed 202. Go's ReadDir filters them, which is why only the PowerShell walk caught it. * mount: flush queued writes when windows mount is interrupted The signal handler exits the process the moment its hooks return, so the WaitForAsyncFlush after Serve never ran on ctrl-c and queued writes were dropped. * mount: let windows mount over an empty directory WinFsp turns a directory mount point into a reparse point, which NTFS allows on an empty directory and refuses on a populated one. The check rejected every existing directory, so the ordinary habit of creating the mount point first failed with a message saying it should not exist. CI now mounts over a pre-created directory and writes through it. * ci: run the windows mount check on any pull request It is the only thing that exercises the Windows mount, so restricting it to pull requests based on master skipped it for stacked ones. Replaces the branch name that was pushed to trigger it. * mount: do not log a missing windows entry as an error Windows probes for entries that do not exist as a matter of course, so ENOENT from getattr and open is an answer rather than a fault and would have filled the log. * mount: take the fast path for parent chains in every windows resolve Narrowing it to resolveParent left Getattr and Open re-walking the parent with a filer lookup per component, and those are what Windows calls before a create: eight writers in one directory still raced a meta cache refresh there. Only the final component needs the Lookup refresh. The pass that suggested otherwise came from a run five times slower than the failing ones, where the race had no room to appear. * mount: drop the windows path resolution shortcut Resolving from the inode table skipped the Lookup that refreshes an entry, and a truncate then read back its old size. Applying it only to the parent chain kept truncate correct but left concurrent creates failing, and applying it to the final component too inverted that. The two cannot both be satisfied this way, so this returns to looking up every component and leaves the concurrent create failure open. * mount: fall back to the open handle when a deferred entry is evicted A create that defers the filer write leaves the entry only in the local cache. Creating many files at once pushes the directory past the hot threshold and evicts it, taking that placeholder with it, so a lookup went to the filer, found nothing, and reported a file that plainly exists as missing. The handle still holding the unflushed entry is authoritative for it. Caught by concurrent creates over a Windows mount, which resolves a path on every call rather than relying on a kernel dentry cache. * mount: let cgofuse resolve to the version the module graph requires rclone already depends on cgofuse at a newer commit than the v1.6.0 pin, so readonly builds refused the go.mod until it matched what MVS picks. The interface and flag values the adapter uses are unchanged there. * mount: wait for a pending async flush before looking up on the filer Open, unlink and rename already wait, but a plain lookup went straight to the filer and read pre-close metadata: truncate a file, close it, and a path probe during the flush window reported the old size. The kernel attr cache hides this on linux; a front end that resolves paths on every operation hit it directly. * mount: reject a umask wider than the file mode it becomes ParseUint allowed 64 bits and the result is narrowed to os.FileMode, which is 32, so an out-of-range umask truncated silently instead of being reported as unparseable. * mount: address review findings on the windows mount WaitForAsyncFlush closed its channel unconditionally and shutdown reaches it from both the interrupt hook and the path that resumes after serving, so a ctrl-c could panic on a second close. The deferred-entry fallback read an open handle's entry without its lock, which is what the other two readers of that field take so FromPbEntry does not walk the chunk slice mid-append. The async-flush wait also sat ahead of the meta cache, making every stat of a recently closed file queue behind uploads; it belongs just before the filer is consulted. Windows entries were persisted as uid 0: the raw filesystem stores InHeader's owner and the adapter left it zero. They now carry the identity the mount was started with. The errno table used Linux numbering while cgofuse decodes MSVC's, so ENAMETOOLONG arrived as EDEADLK and five others were likewise wrong; a windows test pins each value to cgofuse's own constant. Also: break the filer handshake loop on success rather than always running ten rounds, accept a drive letter written S:\\, report a missing WinFsp instead of panicking, keep commas out of the volume label, and drop -windows.caseInsensitive, which told WinFsp the mount folds case while lookups stayed exact. * mount: return windows lookup references so the inode table stays bounded Every operation that hands back an EntryOut grants a reference the Linux kernel returns with FORGET. WinFsp has no FORGET, so the adapter took one per path component per call, plus one per child of every readdirplus, and never gave any back: inodeToPath grew for the life of the mount. Walking the 200k-file directory this exists for stranded 200k references. The adapter now plays the part the kernel plays. Each resolution releases what it took, and an open handle keeps the reference for its inode until Release, counted because the raw filesystem reuses one handle for repeated opens. Holding it is not optional: completeAsyncFlush skips the metadata flush when the saved path no longer maps to the inode, so releasing early would lose a close's metadata. Also stops persisting the display owner. -o uid=-1 makes WinFsp report the calling user whatever we say, but the value handed to the raw filesystem is written to the filer, and 4294967295 is what every other client would read. -windows.uid and -windows.gid set what is recorded. * mount: fix windows behaviours the reference implementations guard against WinFsp has no ro option — it discards the flag and leaves the volume writable — so -readOnly accepted writes and deletes. The refusal now happens in the operations themselves. Windows sends times around its own 1601 epoch, which arrive as a large negative second count; casting them through stored a year-1601 timestamp that every other client then read. Those are now left alone. rclone carries the same guard. Chown returned ENOSYS, and WinFsp passes a chown failure straight out of SetSecurity, so Explorer's Security tab and icacls failed for edits that were not about ownership. It now accepts and discards. Only create and mkdir presented a caller; the rest sent uid 0, which hasAccess treats as root, so deletes and renames skipped the permission check that creates got. Every operation presents the same identity now. A drive letter written S:\ reached WinFsp unnormalised, which recognises a drive only as exactly two characters and then failed as a directory path. A test also pins the open flag translation, since swapping O_EXCL and O_TRUNC would turn 'fail if it exists' into 'truncate it'. * mount: answer windows getattr and truncate from the open handle WinFsp keeps the path a handle was opened with and never updates it when the file is renamed, so resolving the path again fails on a handle that is still perfectly valid — the ordinary write-temp-then-rename save pattern. The handle already knows its inode, which also removes a full path walk from two operations WinFsp calls constantly. Readlink on the root now refuses. WinFsp probes there to decide whether the volume has symlinks and enables them unless it fails, and with them on it resolves a path a component at a time, each one reaching us as its own walk — all for a feature Symlink already refuses. * mount: require the windows mount directory not to exist WinFsp creates the directory itself with FILE_CREATE and removes it when the filesystem goes away, so an existing one — empty or not — fails with "mount point in use". Allowing an empty directory was wrong, and the CI check that appeared to prove otherwise was the vacuous one: listing a plain directory succeeds whether or not anything is mounted on it, so the step passed while the mount had failed and the writes went to local disk. That check now waits for the reparse point, which is what caught this. * mount: apply review comments on the windows mount -windows.uid and -windows.gid reached the adapter but not the filesystem parameters, which is what carries the owner written to the filer, so the flags changed nothing. Readdir re-resolved the path while Getattr and Truncate answer from the handle; a directory renamed during an enumeration then failed on the stale path WinFsp still holds. Utimens now honours UTIME_OMIT instead of writing whatever came with it. * mount: tag the unix-only lock tests away from windows The production lock files were tagged when the package was made to build on windows, but the tests that exercise them were not, so anything that compiles tests for windows still failed on syscall.F_WRLCK. * ci: vet the mount tests for each target too Only compiling the non-test build let an untagged test keep a per-OS syscall constant without anything noticing. |
||
|
|
4992ac1ca9 |
mount: keep the xattr flag constants off freebsd (#10552)
* mount: keep the xattr flag constants off freebsd x/sys/unix has no XATTR_CREATE or XATTR_REPLACE there, and weedfs_xattr.go is already tagged away from freebsd for that reason. Putting them in a !windows file dragged them back in, so master stopped building for freebsd. * ci: cross-compile freebsd and darwin too The windows-only check missed a freebsd break in the very file it was added to guard, because nothing else on a pull request compiles them. |
||
|
|
c191b2fe01 |
iceberg: let clients select their table bucket as the catalog warehouse (#10549)
* iceberg: accept bare bucket names and ARNs as the catalog warehouse Only s3://<bucket>/ was recognized. A warehouse spelled as a bare table bucket name or as the s3tables bucket ARN -- the two forms users reach for first, the latter being what AWS S3 Tables itself takes -- was silently dropped, so every call landed on the default "warehouse" bucket and failed with "table bucket warehouse not found". * iceberg: report a missing table bucket as 404, not 500 Pointing a client at a table bucket that does not exist -- which every client with no warehouse set does, since the default bucket "warehouse" rarely exists -- returned InternalServerError with a message naming a bucket the client never asked for. Answer 404 and say how to select one. * admin: show the warehouse in the PyIceberg example The example connected without one, so it always resolved to the default table bucket and every client that copied it failed on the first call. * test: pin bearer auth against a table bucket that exists The subtest called the catalog with no warehouse and accepted 500 as proof that auth had passed, since the default bucket does not exist. A missing table bucket now answers 404, which the test read as an auth failure. Give it a real table bucket so only 200 passes. * test: assert the missing-bucket guidance reaches the client The status and error type were checked but not the message, which is the part of the mapping that tells a user how to select a table bucket. * test: encode the warehouse query value The ARN case pasted raw colons and slashes into the query string. Go's parser tolerates them, so the test passed without modelling how a client actually sends the request. |
||
|
|
63a180ef75 |
telemetry: sync the server module with the client_golang bump (#10551)
The server module has its own go.mod and replaces the root module from ../.., so bumping prometheus/client_golang in the root leaves this one pinned below what the replacement needs and 'go build' refuses to run. |
||
|
|
88fd2d1be8 |
telemetry: stack volume servers per cluster, drop the total disk usage chart (#10550)
* telemetry: stack volume servers per cluster over time The fleet-wide server count says how many volume servers reported, but not who they belong to. Carry per-cluster counts in /api/cluster-sizes and draw them the same way as cluster sizes, sharing one cluster ranking so a cluster keeps its colour across both stacks. * telemetry: drop the total disk usage chart from the dashboard The stacked cluster sizes chart right below it has the same fleet total as its stack height, plus the per-cluster breakdown. /api/metrics still serves the aggregate for anyone graphing it elsewhere. |
||
|
|
cc2775d9f2 |
s3: register an identity's inline account instead of collapsing it into admin (#10548)
* s3: register an identity's inline account instead of collapsing it into admin Credential stores persist an account inline on the identity and never emit a top-level accounts list, so every user created through the IAM API or the admin UI with an email hit the "non exist account ID" branch and was given the shared admin account. Distinct users then presented the same owner id, so ownership checks could not tell them apart and each passed for the others' buckets. Treat an id missing from the account map as undeclared rather than invalid: register it, keeping an email another account already claimed. Both load paths now resolve the account through one helper. * s3: refresh an undeclared account from the identity that carries it The merge path starts from the live account cache, so an identity upserted with the same account id but a new email or display name kept the cached copy: the new address never reached the email index and the replaced one still resolved. Changing a user's email through the admin UI takes exactly that path. An account registered from an inline block is only described by the identity carrying it, so refresh it and move its email claim. Accounts from a top-level list and the predefined defaults are marked declared and stay authoritative. * s3: let an account reclaim an email once its holder moves away Two identities can carry the same email, and the second to load leaves the lookup with the first. Returning early when the incoming metadata matches the cached account meant the loser never re-ran the claim, so an address freed by the holder's update resolved to nobody until the loser itself changed. Re-index on the unchanged path, which is a no-op while another account still holds the address. |
||
|
|
846f9f7e7d |
build(deps): bump github.com/prometheus/client_golang from 1.24.0 to 1.24.1 (#10544)
build(deps): bump github.com/prometheus/client_golang Bumps [github.com/prometheus/client_golang](https://github.com/prometheus/client_golang) from 1.24.0 to 1.24.1. - [Release notes](https://github.com/prometheus/client_golang/releases) - [Changelog](https://github.com/prometheus/client_golang/blob/v1.24.1/CHANGELOG.md) - [Commits](https://github.com/prometheus/client_golang/compare/v1.24.0...v1.24.1) --- updated-dependencies: - dependency-name: github.com/prometheus/client_golang dependency-version: 1.24.1 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> |
||
|
|
196d4808a0 |
build(deps): bump github.com/aws/aws-sdk-go-v2/config from 1.32.25 to 1.32.33 (#10540)
build(deps): bump github.com/aws/aws-sdk-go-v2/config Bumps [github.com/aws/aws-sdk-go-v2/config](https://github.com/aws/aws-sdk-go-v2) from 1.32.25 to 1.32.33. - [Release notes](https://github.com/aws/aws-sdk-go-v2/releases) - [Commits](https://github.com/aws/aws-sdk-go-v2/compare/config/v1.32.25...config/v1.32.33) --- updated-dependencies: - dependency-name: github.com/aws/aws-sdk-go-v2/config dependency-version: 1.32.33 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> |
||
|
|
2a2ad05f1a |
build(deps): bump github.com/ydb-platform/ydb-go-sdk/v3 from 3.144.5 to 3.146.3 (#10538)
build(deps): bump github.com/ydb-platform/ydb-go-sdk/v3 Bumps [github.com/ydb-platform/ydb-go-sdk/v3](https://github.com/ydb-platform/ydb-go-sdk) from 3.144.5 to 3.146.3. - [Release notes](https://github.com/ydb-platform/ydb-go-sdk/releases) - [Changelog](https://github.com/ydb-platform/ydb-go-sdk/blob/master/CHANGELOG.md) - [Commits](https://github.com/ydb-platform/ydb-go-sdk/compare/v3.144.5...v3.146.3) --- updated-dependencies: - dependency-name: github.com/ydb-platform/ydb-go-sdk/v3 dependency-version: 3.146.3 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> |
||
|
|
b7be05fb41 |
build(deps): bump github/codeql-action from 4 to 4.37.4 (#10541)
Bumps [github/codeql-action](https://github.com/github/codeql-action) from 4 to 4.37.4. - [Release notes](https://github.com/github/codeql-action/releases) - [Changelog](https://github.com/github/codeql-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/github/codeql-action/compare/v4...v4.37.4) --- updated-dependencies: - dependency-name: github/codeql-action dependency-version: 4.37.4 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> |
||
|
|
2b2bbc625b |
build(deps): bump docker/login-action from 4.5.1 to 4.6.0 (#10539)
Bumps [docker/login-action](https://github.com/docker/login-action) from 4.5.1 to 4.6.0. - [Release notes](https://github.com/docker/login-action/releases) - [Commits](https://github.com/docker/login-action/compare/v4.5.1...v4.6.0) --- updated-dependencies: - dependency-name: docker/login-action dependency-version: 4.6.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> |
||
|
|
a9de90ae29 |
test: wait for every queued flush before deleting the log files (#10546)
TestSubscribeLoop_FlushProvenGapSkipsToRetained deleted the log files once the eviction watermark's own flush had landed, while the windows sealed after it were still queued. Those flushes then wrote their files back, and the subscriber served them from disk instead of taking the gap-skip path, so the windows whose files really were gone came out missing. Wait through the last sealed window instead. |
||
|
|
4f692bf9c3 |
mount: build the package on windows (#10535)
* mount: drop the unused go-fuse fs package dependency WFS embedded fs.Inode but never used any of its methods, and the only other reference was RENAME_EXCHANGE, a constant sitting next to three literals. Removing both drops fs and five internal packages from the mount build graph. * mount: build the package on windows Windows has no fcntl lock types, no O_ACCMODE and no x/sys/unix, so a handful of constants kept weed/mount pinned to unix even though the code using them is portable in-memory logic. Route them through per-OS shims and give setBlksize a windows no-op. The POSIX lock table now compiles on windows but stays unreachable: WinFsp resolves byte-range locks in its own kernel driver, so nothing will feed it there. go.mod points at a go-fuse branch commit and needs repinning to a release tag once that lands. * ci: cross-compile for windows Nothing caught the unix-only constants creeping into weed/mount until a release build failed. * mount: let readdir feed a sink instead of the kernel buffer doReadDirectory wrote directly into fuse.DirEntryList, which is the kernel's wire format. A front end that is not the kernel would have to pack entries only to parse them straight back out. Route it through DirEntrySink instead. ReadDir and ReadDirPlus pass the reply buffer, so nothing changes for the FUSE server. * mount: pin go-fuse v2.9.4 for the windows build |
||
|
|
529ffa5c86 |
mount: drop the unused go-fuse fs package dependency (#10534)
WFS embedded fs.Inode but never used any of its methods, and the only other reference was RENAME_EXCHANGE, a constant sitting next to three literals. Removing both drops fs and five internal packages from the mount build graph. |
||
|
|
813a4b0711 |
fix(filer): stop skipping recent unflushed events on metadata subscription gaps (#10501)
* fix(filer): don't skip unflushed events on metadata subscription gaps A subscriber that falls behind the in-memory log ring during a write burst could have its read position jumped past events that were evicted but not yet flushed to disk — silently lost for filer.backup/filer.sync/ mount subscribers. Route all three gap-skip sites through resolveDiskGapResume: only skip past windows older than a settled horizon (2*LogFlushInterval); recent gaps wait for the flush and re-read disk. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(filer): harden metadata gap-skip guard Address review findings on the settled-horizon guard: - local subscriptions: gate the skip on the buffer's flush watermark observed before the disk read (resolveLocalGapResume) — a disk miss is then proof the gap is empty, with no wall-clock assumptions - aggregated subscriptions: cap skips strictly below the horizon boundary (persisted reads exclude ts <= cursor) and pace capped advances, so the sliding horizon cannot cause disk-probe spinning - replace unbounded sync.Cond waits with a bounded select on the buffer's subscriber channel + retry timer + ctx cancellation, eliminating the lost-wakeup stall Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(filer): close remaining gap-skip loss paths from re-review - log_buffer: a sentinel-offset (time-based) read below the earliest in-memory entry silently started at earliest, skipping a window that may hold evicted-but-unflushed events. Track the ring's eviction watermark (lastEvictedTsNs) and keep the inclusive fast path only when nothing at/after the position was ever evicted; otherwise return ResumeFromDiskError so the subscription gap guard decides. - capped horizon advances stay on the disk-probe path (never expose a mid-gap position to the memory read) and keep pacing - gap jumps land just below earliest: positions are exclusive, so the earliest entry itself is still delivered - subscriber notification keys include clientId/epoch so a replacement stream never inherits a channel the old stream's cleanup closes Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(log_buffer): generalize the eviction-watermark read gate Third-review findings: - epoch/zero-time reads bypassed the eviction watermark: gate them the same way, so a SinceNs=0 subscriber cannot silently start at the earliest retained entry after an unflushed window was evicted - apply the watermark gate regardless of the cursor's batch offset (batch offsets carry no meaning for time-based reads); this also serves adjacent cursors (earliest == current+1) from memory instead of stalling them in the gap loop - LoopProcessLogData reader names include clientId/epoch, since they are registered as subscriber keys internally (same collision as the outer notification keys) - test: pin the gate (below/at watermark, epoch-after-eviction) and update the slow-consumer test to the sharper contract — complete in-memory history is served from memory; disk only once evicted Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(log_buffer): watermark equality is unsafe for inclusive sentinel cursors Sentinel (Offset <= 0) time-based cursors search from ts-1ns, i.e. they read inclusively of their own timestamp — and the evicted window may end exactly at that timestamp. Allow watermark equality only for exclusive (positive-offset) cursors; sentinel cursors must be strictly above it. Also shut down the test buffer and pin the equality cases. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(filer): wait on an empty aggregated buffer instead of falling through When a disk read finds nothing for a ResumeFromDiskError gap and the aggregated buffer has no readable entries (zero earliest time), resolveDiskGapResume declines to advance and control fell through to the in-memory read — which returns ResumeFromDiskError again immediately, spinning through the probe cycle without any wait. Fold the case into the existing recent-gap branch so it waits (notification, cancellation, or the retry interval) before re-probing, matching the local subscription path, which already waits unconditionally. * fix(filer): serve the exact eviction-boundary entry without another flush wait When the flush watermark has passed the earliest in-memory entry but that entry sits exactly one nanosecond above the cursor, the exclusive resume target collapses onto the cursor and resolveLocalGapResume declines to advance — while a sentinel cursor at the eviction watermark keeps deferring the in-memory read to disk. Progress then depends on the next flush cycle. Re-arm the cursor with a positive (exclusive) offset instead: ReadFromBuffer explicitly allows positive-offset cursors at the watermark, so the boundary entry is served from memory immediately. Also promote the aggregated path's horizon-capped gap skip to a warning: that skip may pass events a stalled flush lands later (the aggregated ring has no flush watermark to gate on), so operators should see when a flush stall outlasts the settled horizon. * fix(filer): resume a gap skip with an exclusive cursor The resume position landed one nanosecond below the earliest in-memory entry but kept the inclusive sentinel offset. When that timestamp is also the eviction watermark, the read gate answers ResumeFromDiskError for an inclusive cursor, the disk read finds nothing, and the resume target collapses onto the cursor, so neither helper can advance: the subscriber parks on timed waits forever. A skip is only taken once the gap is proven empty, so nothing remains to deliver at the resume timestamp. Resume exclusively instead, and let an inclusive cursor already sitting on the target count as progress. * fix(filer): gate aggregated gap skips on eviction, not wall clock Wall-clock age never proved persistence. The aggregated ring has no flush watermark because peers persist their own local logs, so a horizon of 2 * LogFlushInterval was standing in for one. While the disk stayed empty the horizon kept sliding forward and walked the cursor past evicted-but-unflushed events one window at a time - the volume-outage stall this change set exists to survive. The ring does carry a real proof: its eviction watermark. If nothing at or after the cursor was ever dropped, memory still holds every entry after it and the gap is provably empty. Below the watermark entries were dropped and only the producing peer's flush can supply them, so wait for that flush instead of advancing. The skip that remains is the one the ring can prove, which keeps the infinite-loop guard for a genuinely empty gap. * perf(filer): stop re-probing disk on every metadata append A subscriber parked on a gap woke on the log buffer's subscriber channel, which fires on every append. On the aggregated path each wake re-ran a full ReadPersistedLogBuffer - a ListDirectoryEntries plus a readahead goroutine - so one parked subscriber turned every cluster-wide metadata event into a store query, exactly while the cluster is already struggling with the flush stall that parked it. An append also cannot settle the gap: what this waits on is a peer persisting its own log, which nothing local signals. Wait on the timer alone there. The local buffer does signal its flush on that channel, so keep it, but drain the stale token first: otherwise the appends riding the same channel spin the wait at write rate. The retry interval covers the notification the drain discards. * fix(filer): surface a metadata subscriber parked on a gap Refusing to skip an unresolved gap trades silent loss for a silent stall, and a stall is no easier to diagnose: filer.sync and mount followers just stop advancing, with no error on either end. The only trace was a V(3) line nobody runs with. Warn on entry and once a minute after, and carry a subscribe_gap_stalled gauge for the duration, so a flush that never lands shows up as a stalled subscriber rather than a consumer that mysteriously went quiet. * refactor(filer): drop the metadata listener cond with no waiters left Both listenersCond.Wait() sites are gone, so listenersWaits never leaves zero, the guard in the filer's notify callback never fires, and the two Broadcast calls around client registration wake nobody. Remove the cond, its counter and its lock, and pass a nil notify func: the log buffer already skips a nil one, and the subscriber channels carry these wakeups now. * test(log_buffer): drive the eviction watermark through a real seal Both eviction tests wrote lastEvictedTsNs directly, leaving the one line that sets it uncovered: copyToFlushInternal has to read slot 0 before SealBuffer shifts it out, and moving that read one statement later still passed every test. Append past the ring instead, and check the watermark appears only on the seal that drops a window, matches that window's stop, and then advances. The buffer under test never flushes, matching the aggregated meta ring where the watermark is the only emptiness proof. * refactor(filer): build the subscriber reader name once per stream It was rebuilt on every loop iteration from values that cannot change for the life of the stream. Hoist it next to the notification key it mirrors. * docs(filer): tighten the gap-handling comments Several ran to eight lines restating the same reasoning at each site. Keep the non-obvious why, drop the retelling. * fix(filer): mark a confirmed disk position as an exclusive cursor A disk read returns the timestamp of the last entry it handed to the subscriber, but the cursor built from it stayed inclusive. Land that cursor exactly on the eviction watermark and the read gate sends it back to disk for an entry the disk just delivered; the re-read finds nothing, neither emptiness proof holds for an inclusive cursor there, and the subscriber parks - until some later flush, or forever while one stays stalled. Everything after the watermark was sitting in memory the whole time. Carry the offset instead, so it also covers the ring evicting onto the cursor after the read rather than before it. The constant is no longer gap-specific, so it is now named for what it asserts. * perf(filer): re-park a gap wait woken by an ordinary append Draining one stale token did not bound anything: the subscriber channel carries an append per metadata write, so under continuous writes the next one satisfies the wait immediately. During the write burst these stalls come from, each parked subscriber ran the recovery loop at write rate rather than the intended two-second cadence. Only a flush can settle a gap, so check the flush watermark on wake-up and go back to waiting if an append is all that arrived. The retry timer is created once, so re-parking does not extend the interval. * fix(filer): keep a disk-derived cursor inclusive Marking every disk position exclusive assumed the entry at that timestamp was the only one there. On the aggregated stream it is not: disk can deliver one filer's persisted event at T while another filer's event at the same T is still unflushed in the aggregate ring. The exclusive cursor skipped it, and since the persisted reader also excludes ts <= its start, nothing would ever bring it back. Take the weaker guarantee instead. The reason the exclusive cursor was introduced - an inclusive one landing on the eviction watermark parks forever - is better answered in the resolvers: at the watermark memory holds nothing (retained windows start strictly after it) and the persisted reader cannot return that entry at any later time either, so refusing the gap buys nothing and never ends. Skip it whatever the cursor's inclusivity. That proof does not depend on a flush, so the local resolver takes it as a second, independent disjunct alongside its flush watermark. * perf(filer): wake a parked gap wait on flushes only Re-parking on an append bounded the work but not the wake-ups: the subscriber channel carries one per metadata write, so a parked subscriber still took a scheduling round-trip per write. Worse, draining it kept the channel empty, so every writer's non-blocking notify succeeded instead of falling through - the burst paid for the wake-ups too. Give the log buffer a flush-only subscriber list, notified from loopFlush, and park on that. The append channel now fills once and stays full, which is exactly the state the non-blocking send is designed for. * fix(filer): stop the gap-stall gauge from leaking a series per connection clientName is req.ClientName + "@" + peer address, so it carries the client's ephemeral source port and changes on every reconnect. Labelling the gauge with it minted a new series per connection, and clearing it only ever Set(0), so nothing was ever released - a client in a reconnect loop grows the filer's metric map and /metrics payload without bound. Key it on the stable client-supplied name, as the neighbouring subscribe gauge already does, and delete the series on teardown. Two logging fixes ride along, both in the same reporter: the resume warning was unpaced while the park warning throttles to one a minute, so a burst that parks and resumes every couple of seconds warned on every cycle; and clear() doubled as the teardown path, announcing "resumed after 14m0s parked" for a client that actually gave up and disconnected still behind. * fix(filer): give a parked gap wait the exits the read loop has A park never re-enters the read loop, so every exit that loop relies on stopped working while a subscriber was parked. It kept scanning the filer store every 2s for a client that a higher-epoch reconnect had already superseded, until the TCP connection finally died - hours, on a half-open one. It never reached the only code that honors UntilNs, so bounded callers like `weed shell fs.verify` and `filer.meta.tail -until=` hung instead of exiting. And a notification channel closed out from under it turned the bounded wait into a spin, since a receive on a closed channel returns instantly. Check all three where the subscriber actually waits. Bound the wait too: waiting is productive while the window is still queued for flush, but a peer that never returns, or whose filer store this filer cannot read at all, makes it permanent - and a subscriber that silently stops delivering is no better than one that silently skips. Fail the stream after that instead of hanging, loudly enough to say which gap and for how long. * fix(log_buffer): keep the eviction gate out of the shared read path The gate belonged to the filer's subscribe loops but was installed in ReadFromBuffer, which the message queue shares and which has no gap handling of its own. Three MQ paths broke on it: GetUnflushedMessages asks for everything in memory past the flush watermark and got ResumeFromDiskError instead, so SQL results silently dropped unflushed rows; a RESET_TO_EARLIEST consumer's epoch cursor was sent to disk, and the MQ disk reader resets an empty read back to epoch, so the whole partition replayed on every pass; and a disk cursor landing on the watermark carried offset -2, which the gate refused and the disk reader's ts <= start filter also skips, so neither side could ever serve it. The rewrite also dropped the old Offset <= 0 requirement, letting a stale positive-offset cursor jump to memory over on-disk history, and refused any negative-timestamp cursor even with nothing evicted. Restore the read path exactly as it was and keep only lastEvictedTsNs, which is the useful primitive. The filer loops now consult the watermark themselves before reading memory, which is where the gap handling that makes the refusal actionable already lives - and they check it every pass, not just when the disk came up empty, since a disk read can leave the cursor short of the watermark too. * fix(filer): read the log file whose window spans past its own name A log file is named for the start of the window it holds, but a window runs up to a flush interval longer, so "12-30" can hold entries through 12:31:58. File selection compared the cursor's minute against that name and skipped anything sorting earlier, so a subscriber resuming at 12:31:10 never saw the rest of that file: the read reported nothing on disk while the entries sat in it. That was survivable when a miss only meant "wait and retry", but the gap resolvers now read a miss as proof the range is empty and move the cursor past it, which turns those entries into silent loss. Start the file scan a flush interval early; entries are still filtered against the exact cursor, so this only opens one more file and never re-delivers. * fix(filer): resume a gap with a cursor the memory read will serve Reverting the eviction gate put ReadFromBuffer back to refusing every positive-offset cursor below the in-memory window, but the resolvers still handed back one - earliest-1 marked exclusive. So the resume bounced straight to ResumeFromDiskError, the disk had nothing, and the resolver saw its own cursor as no progress and parked: a subscriber stalled, and after the new bound failed outright, with the whole gap sitting in the ring the entire time. The exclusive marking only existed to dodge a park the gate itself caused, and the gate is gone. Resume at earliest with the sentinel offset, which is the position master used and which case 2.1 reads inclusively. That makes the resume unconditionally ahead of the cursor, so the progress check it needed goes away with it. * fix(filer): stop dropping a log file whose window outruns its name Listing the earlier file was not enough: the iterator then decided whether to read it by comparing the *following* file's name against the cursor, which treats that name as an upper bound on this file's contents. It is not one. A file is named for the start of the window it holds, minute-truncated, and the window runs up to a flush interval longer, so "12-30" can hold an event at 12:31:20 while "12-31" sits right after it - and a cursor at 12:31:10 skipped straight past the event. Bound the decision on the file itself: skip it only when its name plus the minute truncation plus a flush interval still lands at or before the cursor. That also covers the last file in the queue, which the old check never skipped because it had no successor to compare against. * fix(filer): stop a chunk-ref read from rewinding the subscriber CollectLogFileRefs reports the minute-level name of the last file it shipped, and the caller assigns that straight to the read position. Since the scan now reaches back a flush interval to catch a spanning file, a request at 12:31:10 that picks up the 12-30 ref moved the cursor to 12:30:00 - so the memory read that followed replayed events older than the client's own SinceNs and re-sent what the chunk reader had already been handed. The same rewind was reachable before, within a minute, whenever the last file's name sorted behind the request. Clamp the reported position to the one that was asked for. It still under-advances by design, since the server never reads the entries it ships refs for and cannot know where they end. * fix(filer): resume below earliest so a one-entry window is not skipped Moving the resume onto earliest itself was wrong for the smallest window there is. A sealed window holding a single entry has startTime == stopTime, and the sealed-buffer lookup only enters a window whose stopTime is strictly after the cursor, so a cursor sitting exactly on earliest walks past it and its sole event is never delivered. Low-volume metadata windows are routinely one entry, which is precisely when losing it is hardest to notice. Go back to one nanosecond below, which takes the startTime.After branch and returns the whole window, and keep the sentinel offset the memory read requires. The earlier test used an active multi-entry buffer, where both cursors happen to work; the new one seals single-entry windows and compares which entry comes back. * fix(filer): count a persisted read as progress only when it moves A chunk-ref read reports the minute-level name of the last file it shipped, now clamped so it never rewinds, so it comes back non-zero even when it names the position the subscriber already held. The loops read non-zero as progress: they cleared the stall timer, then found the cursor still short of the eviction watermark and parked again. Every retry re-shipped the same refs and reset the timer, so the bound that is supposed to end an unrecoverable stall was never reached - and a chunk-capable client buffers those refs waiting for an event that never comes, so it just accumulates duplicates. Require the reported position to be strictly ahead of the cursor. * fix(filer): count the evicted ranges the aggregated stream cannot prove The eviction watermark belongs to the merged ring, but the disk it gets checked against is the union of every peer's own log and each peer flushes on its own schedule. A read that lifts the cursor from below the watermark to above it may have done so entirely on a peer that is already ahead, while a lagging peer still holds unflushed events inside the range just crossed; when it flushes them they sit behind the cursor and are never delivered. One aggregate maximum is not proof that every peer persisted the range. Nothing available locally separates that from the ordinary case where every peer had in fact persisted it: the aggregator tracks peers by address while log files carry a random per-filer id, so "has this peer flushed through T" cannot be answered here at all. Deciding it needs the source filer's own flush watermark carried on the subscribe stream, which is a wire change this does not make. Count and log the crossing so the window is at least measurable instead of invisible. * fix(filer): send log file refs through the pipelined sender sendLogFileRefs wrote on the raw gRPC stream while pipelinedSender's goroutine concurrently calls Send for queued memory events - two senders on one stream, which gRPC forbids. The window used to open once per fall-behind; the gap retry loop now reopens it every pass. Routing refs through the sender also restores ordering: refs used to overtake up to 1024 queued events, and the client treats any non-ref message as the signal to process buffered refs, so overtaken refs were applied against the wrong position. The reason refs bypassed the sender was the batcher: their TsNs of 0 reads as far behind, and the client recognizes refs by the top-level field alone - a refs envelope would drop its Events tail, and refs inside Events would be applied as an empty event. Teach the batcher instead: refs messages always go solo, and one drained mid-batch is sent solo right after that batch. * fix(filer): count parked subscribers instead of flagging them by name The per-client gauge series could not work. Its label was rebuilt from the peer address at first, which leaks a series per reconnect; keyed on the client-supplied name instead, it collides - every mount registers as "mount" - so one stream's teardown deleted a parked sibling's live series, and the sibling never re-created it because its own park state said the gauge was already set. Either way the alert this gauge exists to drive goes dark. A count needs no identity: Inc on park, Dec on resume or teardown, scope as the only label. Client details stay in the logs. Also start warning only once a stall has outlived the warn interval. park() warned immediately on every first park, so catch-up churn that parks and resumes every couple of seconds logged a warning pair per cycle - exactly the flood the pacing was supposed to prevent, burying the long-stall warnings that matter. * fix(filer): one gap resolver, and re-arm an unservable adjacent cursor The two resolvers were the same function - the aggregated one is the local one with a flush watermark of zero, since its ring never flushes - duplicated down to the comment justifying the resume target. A fix applied to one and not the other is how the two streams drift; merge them. The merge carries the one behavioral fix both copies needed. Timestamp collision bumps make adjacent entries exactly 1ns apart, so an entry ending an evicted window leaves the cursor exactly one below earliest with a positive batch offset. The resume target then equals the cursor and both copies refused it as no progress - but that cursor cannot be served (ReadFromBuffer refuses positive offsets below the window) while the sentinel resume at the same timestamp is, and both deliver exactly the entries after it. Refusing parked a subscriber whose data was entirely in memory: until the next flush locally, and through a 15-minute stall failure on the aggregated path. Advance on equal target when the held cursor is exclusive; a sentinel there is already served, so it still refuses. * fix(filer): a bounded subscription parked exactly on UntilNs is finished The park's UntilNs check was strict while the bound is inclusive and cursors are exclusive: a disk read whose last entry sits exactly on UntilNs leaves the cursor there with everything up to the bound already delivered. If the next range was an unprovable gap, the completed subscription parked anyway and eventually failed - fs.verify hanging and then erroring on a healthy cluster. * fix(filer): re-derive the subscribe loops as one state machine The loops had grown three generations of gap handling - a post-disk-read branch, a post-memory-read branch, and an every-pass guard bolted on in front of the memory read - each consulting state the others mutated. The worst interaction wedged the aggregated stream permanently: diskExhausted compared the disk result against the cursor that result had just updated, and paired it with a ResumeFromDiskError latch that only a resolver advance cleared, so one fall-behind sent every later pass into the gap block and LoopProcessLogData never ran again. Mounts kept a healthy-looking stream and applied nothing. Both loops now run the same derived sequence. One disk pass; progress is the pre-update cursor against the result, freshly each pass. One gap decision before the memory read: a cursor the ring evicted past either keeps draining the disk (it just advanced), resolves forward (the gap is proven empty), or parks - and a cursor memory refused with nothing evicted after it re-arms onto the retained window. The stale-latch branch is gone: the error is only consulted on a pass whose own disk read came up empty. All four parks go through one parkOnGap helper, so the exits live in one place. The eviction watermark is read after the disk read and the same value feeds both the guard and the unproven-crossing report, which previously compared against a snapshot taken before a potentially minutes-long backlog read and missed evictions landing during it. The next-day jump now clears the stall reporter - it used to leave a stale park epoch that could kill the next brief park instantly at the 15-minute bound - and counts its own watermark crossing. Chunk-ref reads get two rules the old loops lacked. Refs are sent once per position: retries re-sent identical batches every two seconds into a client that only drains them on a non-ref message, growing an unbounded pending list. And when refs cannot advance the cursor - they report file start minutes, which sit below the content the client was actually given, pinning the cursor under the watermark forever during bursts - the pass falls through to entry reads, which move the cursor by real timestamps and double as the client's drain signal. * fix(filer): give up on an unprovable gap instead of failing the stream Failing after maxGapStall assumed the client could do something better, but every consumer just reconnects at the same SinceNs and hits the same wall, so an unprovable gap - a dead peer, or a peer whose filer store this filer cannot read at all - turned into a permanent 15-minute fail/reconnect loop delivering nothing. Master handled the same state by skipping instantly and silently. Take the middle: wait the full bound, then abandon the gap and resume at the eviction watermark, where everything retained starts strictly after, so the loss is exactly the range that could not be proven. The skip shares the unproven-crossing counter and logs at error level - loss is bounded, recorded, and the stream keeps working. A stall with nothing evicted past the cursor loses nothing by waiting, so it restarts the clock and keeps parking rather than skipping. * test(filer): pin the file-skip bound through the production predicate The spanning-file test asserted against its own copy of the arithmetic, so a regression in the iterator - restoring the next-file-name comparison or dropping the flush-interval term - would keep CI green while re-introducing the silent loss the fix closed. Extract the bound into logFileMayContainAfter, call it from the iterator, and point the test at it; breaking the production expression now fails the test. * test(log_buffer): pin the flush-subscriber contract The registry the filer's gap parks wait on had no test at all. Cover the observable contract: an append never wakes a flush subscriber, a flush does with the watermark already stored, unregistering closes the channel so an abandoned waiter unblocks, and double or unknown unregisters are harmless. The store-before-notify ordering in loopFlush is what makes the parks' wake-up re-check sound, and it is not black-box testable - reordering leaves a same-goroutine window of nanoseconds that hundreds of tight round-trips never catch. Mark it load-bearing at the site instead; a reorder now at least has to argue with the comment it deletes. * fix(filer): a -1 SinceNs is a position, not the refs-gate sentinel The once-per-position refs gate used -1 as "never sent", but a client may legally subscribe with SinceNs=-1, whose cursor timestamp is exactly -1: the very first pass then believed refs were already sent there and fell back to streaming the whole persisted history entry by entry - the bootstrap load chunks mode exists to avoid. Use MinInt64, which no cursor can carry. * refactor(filer): drop the aggregator's listener cond with no waiters left Same shape as the FilerServer cond already removed: nothing increments ListenersWaits and nothing ever calls Wait, so the three Broadcasts wake nobody and the notify callback's guard is always false. Aggregated subscribers wake through the buffer's subscriber channels now. * fix(filer): make the gap metrics say what they count The crossing counter's help text described only the aggregated peer case, but give-ups increment it for local stalls too - a wedged local flush - which sends an operator chasing peer replication when the problem is the local store. Label it by scope and say both. The stalled gauge counted every park, including waits with nothing evicted and nothing at risk, while its help text promised evicted-but-unpersisted events; describe it as what it is, a count of subscribers parked on a gap. * test(filer): make the flush and stall tests assert what they claim The flush-subscriber rounds were vacuous: the probe entry sat above the round timestamps, so every round entry was collision-bumped and the stored watermark exceeded the local value each assertion compared against - the same bump mistake this test suite already made once. Put the probe below the rounds and guard each round against bumping, so a vacuous setup fails instead of passing. The stall-outcome test wrote the reporter's park epoch directly, bypassing the gauge Inc that gaveUp() later Decs - leaving the shared process gauge at -1 for every test that runs after it. Park through the real path, age the park by hand, release what the test holds, and assert the gauge lands back where it started. * fix(filer): finish the stream checks before marking it parked parkOnGap stamped the reporter before waitOnGap ran its instant done exits, so a bounded subscription completing inside a gap state was marked parked for the microsecond before done fired - a phantom gauge blip and a false "disconnected still behind" warning on every healthy completion. Fold waitOnGap into parkOnGap so the done exits run first and the park mark only ever covers a stream that actually waits. While the park owns its timer, back the retry off as the stall ages - 2s probes growing toward one a minute - since every retry re-reads the persisted log, and probing the store each 2s for 15 minutes per parked subscriber during the very outage that parked them makes the bad time worse. The subtest still named for the old fail-the-stream stall behavior goes with the merge. * fix(log_buffer): gate filer cursors against eviction under the read lock The subscribe loops checked the eviction watermark and then read memory, but a seal can land between the two: the read then served a sentinel cursor from the earliest retained window, silently skipping the window just evicted - the loss class this PR exists to make loud, surviving as a race. The only place the check is atomic with the serve decision is inside ReadFromBuffer, under the lock seals take to evict. Rather than put the policy back into the shared read path - which broke four message-queue readers last time - add a new sentinel offset that opts into it: EvictionGatedOffset reads inclusively exactly like -2, except below the watermark it is refused to disk. The filer loops stamp it on every cursor they hand the memory read; a refusal lands in the same gap machinery the loop-side check feeds, so the race collapses into the handled path. MQ cursors never carry it and keep master behavior byte-for-byte. * fix(filer): gate the aggregated gap on received, not bumped, timestamps The aggregated ring rewrites an out-of-order arrival to its head plus a nanosecond, so after any bump-heavy interval - a peer history replay following a restart is enough - its eviction watermark lives above every timestamp that exists on any peer's disk. Comparing a disk cursor against it parked subscribers that had in fact drained every peer's log: a 15-minute delivery freeze ending in a give-up skip and a false loss alarm, on a healthy cluster where master resumed instantly. Track a second watermark in the received timestamp space - the highest pre-bump timestamp among evicted entries - and gate the aggregated loop on that. Disk cursors and received timestamps are the same space, so the comparison means what it says: at or past it, every evicted entry's original was at or below the cursor, and everything flushed of them was already delivered. The bumped watermark keeps guarding the in-ring read gate, whose cursors live in ring space. The local buffer is untouched: it flushes its own bumped timestamps, so there the two spaces are one. * fix(filer): ship each log chunk once and stop echoing ref'd files inline Chunk mode duplicated data through two doors. Consecutive ref collections overlap by design - the scan backs off a flush interval to catch a spanning file, and a filer appends chunks to its newest file - so the same file was shipped again on every pass that re-listed it: the client re-downloaded its chunks, and a duplicated file mid-batch rewinds timestamps inside the client's per-filer merge, which reads each stream as sorted - transiently resurrecting deleted entries during catch-up. Track per subscription how many chunks of each file were shipped and send only the unsent suffix; state prunes with the scan window, so it holds a few files per filer. The second door was the entry fallback: when refs cannot advance the minute-named cursor, the pass streamed the ref'd file's tail inline, and the client applies inline events unfiltered - the same tail it already applied from chunks. Entry passes for chunk clients now advance the cursor without delivering; everything they skip is covered by the refs already sent or the deltas the next collection ships. * refactor(filer): one gap decision shared by both subscribe loops The post-disk gap tree - guard, drain, resolve, two parks, the re-arm - existed twice, differing only in buffer, watermark space, flush getter, park channels, and reason strings. Four rounds of review fixes have shown the copies drift the moment one is edited alone. gapPass now carries the five differences and the tree lives once; the loops shrink to a three-way switch between reading memory, restarting the pass, and ending the stream. * docs(filer): trim the gap-machinery comments to the why Several blocks had grown to ten-plus lines restating what the tests already pin or retelling one rationale at multiple sites. Keep the non-obvious why - the load-bearing flush ordering, the two timestamp spaces, the refusal-at-equality argument - in a few lines each. * fix(log_buffer): credit an entry's received timestamp to its own window The received-ts capture ran before the rollover check, so an append that sealed the previous window stamped its timestamp onto that window and then lost it in the reset of the new one. The eviction watermark this feeds broke both ways: the sealed window's value was inflated by an entry it does not contain - parking aggregated subscribers on gaps that were drained - and the entry's real window was deflated, proving gaps empty that still held its event on some peer's unflushed path. Credit the timestamp only after the entry lands, when its window is known. * fix(filer): rebase a shipped chunk suffix to logical offset zero A grown file's delta kept the chunks' original file offsets, but the client's chunk reader starts at logical zero and a list opening higher reads as instant EOF - a successfully empty replay, and since chunk clients no longer receive disk entries inline, the appended events were silently dropped. Clone the suffix chunks with offsets rebased to zero; the cut is record-aligned because each append is one uploaded chunk of whole entries, so the suffix decodes as a file of its own. * fix(filer): finish every chunk refs batch with a transition the client acts on Both chunk consumers buffer refs until a non-ref message arrives, so a source with historical logs and a quiet ring - a mount reconnecting after a filer restart is the common case - shipped its backlog and then went silent: the client sat on the refs until the next metadata mutation anywhere in the cluster. The disk step now ends every batch with the empty-notification marker the client already treats as a resume-cursor advance. The same step closes the inline replay: the cursor used to stay at the last file's minute name, so the memory read re-delivered the retained tail of a file the client had just read via chunks - T1..Tn applied twice. The advance-only entry read now runs on every chunk pass, moving the cursor to the true disk content end before memory is consulted. Ordering inside the pass is load-bearing: the entry read can outrun the shipped refs by a chunk appended between collection and read, and the transition timestamp becomes the client's refs filter - stamping it past unshipped content would silently drop that chunk's events on the next delta. The pass therefore re-ships the delta after the entry read, so the transition never exceeds shipped content. Bump-displaced aggregated entries can still arrive inline above the cursor with originals below it; that duplication is bounded and stays within the documented at-least-once residual. * fix(filer): prune ref state at the minute the scan actually stops at The collector compares file names at minute granularity while the prune used the exact-nanosecond scan bound, so for a cursor at 12:31:20 the 12-30 file was still collected but its sent state was already deleted - the next pass reshipped the whole file, re-creating the duplicate-refs class the state exists to prevent. Truncate the bound to the minute the file names live in. * fix(filer): derive the chunk cursor from the shipped refs themselves The advance-only entry read left the three positions that must agree in each other's blind spots. Its snapshot could trail the second delta's, so a chunk appended between them shipped events newer than the cursor and the memory pass sent them again. And it made the filer decode the tail range on every pass, serialized ahead of the client's own reads by the transition marker - re-introducing a slice of the replay work chunk mode exists to offload. Compute the cursor from the shipped set instead: the final entry timestamp of each filer's last shipped chunk, decoded once through the shared chunk cache. Refs coverage, transition marker, and memory start are then the same number by construction - nothing is decoded twice, nothing is dropped, and the per-pass server cost falls to one cached chunk decode per filer. The second delta and the once-per-position refs gate existed to patch the entry read's snapshot races, so both go with it; the range read survives only as a fallback for legacy chunks that do not decode standalone. * fix(filer): keep the chunk-cursor probe inside the shipped snapshot Three holes in the tail probe, all variations of stepping outside what was shipped. A permanently missing chunk failed the stream before the transition marker, so the client discarded its pending refs and reconnected to the same failure forever - blocking all later metadata behind one dead volume, where every other replay path (including the client's own reader) skips such chunks; the probe now walks back to the last readable chunk, and a filer with nothing readable simply contributes no cursor. The legacy fallback re-listed the logs after the refs were collected, so a concurrent append could push the range end over an unshipped chunk and the marker past events the client never received; it now streams the shipped chunk list itself, so no snapshot other than the shipped one is ever consulted. And a file selected before UntilNs can hold entries past it, which the client filters while still adopting the marker as its checkpoint - a later bounded request then skipped them; the marker is clamped to the bound. * fix(filer): make the cursor probe an exact mirror of the client's reader The probe answered from the server's view of the chunks; the marker's correctness depends on the client's. Its backward walk found the last readable chunk, but the client reads forward and stops at the first unreadable one, never resuming within a file - for readable, missing, readable the marker claimed the suffix the client never applied, losing those entries permanently. Keeping only each filer's final file ref discarded the progress of earlier readable files when that file was wholly missing, rewinding the marker to the start cursor. And a torn trailing size prefix - what a crashed writer leaves - failed the probe where the client reads a clean end, blocking the marker forever on data the client accepts. The probe is now shaped like the reader it answers for: per file the readable prefix, per filer the newest file with content, and no condition escapes as an error - understating the marker only re-ships, overstating loses events, and a probe failure must never block the transition the client is waiting on. Each rule is pinned by a test that fails against the previous shape. * fix(filer): judge chunk readability at the volumes, not the decode cache Two ways the probe's answer could drift from what the client experiences. A chunk this server decoded earlier stays warm in the shared cache after its volume dies, so the probe sailed past a chunk the direct-reading client stops at - marker beyond the unread suffix, entries lost. Every chunk now passes a volume lookup before the cache is consulted; the lookup rides the master client's in-memory map, so the probe stays cheap. And a probe stop was treated as harmless understatement, but the delta had already marked the whole ref sent: a transient server-side failure left the cursor stranded behind shipped content for the life of the connection, parking aggregated streams below the watermark for data the client already holds. The pass now rolls back the sent state of every ref above the file that answered the probe, so unreached refs re-ship and re-probe until the cursor gets there. Re-shipped entries at or below the client's checkpoint are filtered client-side, and batches are marker-separated, so a re-shipped file cannot rewind a merge mid-batch. * test(filer): end-to-end subscribe-loop harness and wire-contract tests Every escaped bug across this change's review rounds lived in an interaction the unit tests could not see: the loop state machine, the disk/memory handoff, or the server/client contract. The harness runs the real SubscribeLocalMetadata loop against a real leveldb-backed filer, faking only the volume layer behind the existing test hooks, and asserts the delivered stream itself. Eight scenarios, each pinning a class this change was reviewed for: the headline evicted-unflushed gap parks and then delivers in full; a ring that evicted nothing serves memory promptly; a backlog-to-live handoff with 1ms-adjacent timestamps across every boundary delivers exactly once; a flush-proven gap over vacuumed log files skips to the retained ring including a single-entry window; a bounded subscription terminates at its bound; a permanently wedged flush ends in the give-up skip with the stream still alive; and chunk mode is checked against the real client code - pb.ReadLogFileRefs applied to the shipped refs must cover everything the transition marker claims, with and without a dead volume in the middle. Validated by re-introducing three fixed bugs: the missing eviction guard delivers during the unproven gap, a 2ms cursor error at the handoff drops exactly one event, and resuming at rather than below the earliest retained window loses a single-entry window's sole event - each caught by the scenario built for it. The gap timing knobs become vars so parks run at test speed, a small filer hook swaps the volume-touching read functions, and a sender test pins the refs wire rules the client depends on: never batched, never an envelope, everything in order. * fix(filer): re-ship a partially read answering file, pin the probe's limits The sent-state rollback stopped at files newer than the one that answered the probe. When the answering file itself was only prefix-readable - a dead or transient chunk mid-file - its unread suffix stayed marked sent, and the next append advanced the cursor past it for good. The probe now reports whether the answering file was read through to its end, and a prefix-limited answer re-ships that file too; a torn tail counts as complete, since the client's read ends there as well. The rollback rules live in one predicate with a table test - files below a complete answer stay sent, because the client has moved past them and re-shipping cannot rewind its filter. Two test honesty fixes ride along. The loop harness derived its timestamp base from time.Now() per call, so expectations recomputed across a second boundary drifted by exactly one second; the base is now fixed per harness. And the probe's liveness boundary is pinned as a test instead of a comment: a volume lookup cannot see a dead needle or a stale location inside a resolvable volume, so a warm cache can answer past a chunk the client fails on - accepted because metadata log chunks die volume-at-a-time and the alternative is a real read per probe, which is what the probe exists to avoid. The test states the boundary so changing it is a decision, not an accident. --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
|
e696c2585e |
s3 remote: honor s3.support_tagging in UpdateFileMetadata (#10532)
* s3 remote: honor s3.support_tagging in UpdateFileMetadata The write path already skips tagging when the remote is configured without tagging support, but the metadata-update path sent PutObjectTagging or DeleteObjectTagging unconditionally. On remotes that reject tagging requests, any metadata update failed -- including updates with no tags at all, which land on the DeleteObjectTagging branch. * s3 remote: drop the never-read supportTagging field Every maker set it, nothing read it: the tagging decision is made from conf.S3SupportTagging. Keeping a field that looks like the switch but is not invites exactly the inconsistency the previous commit fixed. |
||
|
|
0d7173a029 |
remote storage: actually delete objects when a directory is removed (#10531)
* remote storage: actually delete objects when a directory is removed On object-store backends RemoveDirectory returned nil without doing anything, so a directory delete synced to the remote as a successful no-op and the objects under that prefix stayed there forever. Nothing surfaced the divergence: the sync logged rmdir, advanced its offset, and the local namespace looked clean. Deleting a bucket-level directory on a filer store that can drop a whole bucket emits no per-child delete events at all, so the single rmdir event was the only chance to clean up the remote. Each backend now lists the prefix and deletes what it finds: S3 in DeleteObjects batches of one listing page, GCS and Azure per object. The prefix always ends with a slash so a sibling like dir2 survives deleting dir, and errors propagate so a failed delete is retried instead of silently skipped. A directory that maps to the bucket root is left alone: wiping every object in the bucket from one namespace event is too destructive, and bucket removal already has its own path. * gcs remote: wrap the per-object delete error The listing error in the same function already wraps, so the delete error should stay inspectable with errors.Is as well. * s3 remote: name the empty-listing test for what it checks The prefix in that test is a normal directory; what is empty is the listing. The bucket-root guard has its own test. * s3 remote: report the scope of a failed delete batch A DeleteObjects response can carry per-key errors for up to a thousand keys. Surfacing only the first hid how much of the batch failed, and surfacing all of them would build an unbounded error string, so report the count with the first failure as the sample. |
||
|
|
c21d92b70a |
test: wait for async write-budget release after pipeline shutdown (#10530)
Shutdown drops the sealed-chunk map references, but an in-flight uploader goroutine holds the final reference and releases its budget slot only after reacquiring chunksLock. Asserting Used()==0 immediately after Shutdown races those releases on slow runners. Poll with a bounded deadline instead. |
||
|
|
de00091765 |
test: random needles always carry at least one byte (#10523)
A zero-data needle lands in .dat as a size-0 record, byte-identical to a delete marker, so scans that walk .dat count it as deleted. Once in 1024 writes newRandomNeedle produced one, and the idx-head repair then skipped a row TestRepairIdxHeadTombstones_ReadOnlyVolume expected back. |
||
|
|
fa9f471f56 |
EC scrubbing: list shards for needles failing scrubs in the result output. (#10510)
This allows to pinpoint failures to a subset of shards, which can then be bisected and potentially reconstructed. |
||
|
|
c2701955c7 |
s3: cover three untested STS paths (#10521)
* s3: test the GetCallerIdentity handler The handler had no test, only XML marshalling, so nothing pinned that a caller presenting session credentials is reported as the assumed role rather than the user who minted the session. * s3: drive AssumeRoleWithWebIdentity over HTTP with a real OIDC token Coverage reached the OIDC path either at the IAMManager service layer or through the Authorization: Bearer shortcut. Nothing exercised the public STS entry point an AWS SDK actually calls, which is where parameter parsing, the IAMManager dispatch and the XML response shape live. * s3: test that every STS route emits an audit entry STS responses go out through WriteXMLResponse, which never calls PostLog, so track() is the only thing that logs them. A route registered outside it would mint credentials with no audit trail and nothing would notice. STS has three routing layers, so a new action is easy to attach to the wrong one. * s3: make the STS tests assert what they claim to cover The audit routing test ran against an uninitialized STS service, so every case answered 503 and a non-404 status was the only evidence the request had reached STS at all - the POST-body case could have been served by the dispatcher's IAM branch and still passed. Back it with a real STS service and assert the STS response namespace, which IAM and S3 responses do not carry. The session policy case checked that the policy travelled in the token rather than that it restricted anything; assert the narrowed bucket is allowed and another bucket is refused. Give the forged-token case the same claim set a valid token gets, so it cannot pass for want of a claim, and cover both rejection paths: a key we do not publish, and a key id absent from the JWKS. |
||
|
|
ef6a706c0e |
master: count only writable volumes as crowded when deciding growth (#10522)
The crowded map keeps volumes that later became unwritable, so their state survives transient writability flips without flapping. But volumes packed to capacity (fs.mergeVolumes) or turned read-only stay above the crowded threshold and get re-marked on every heartbeat, so the raw map size can permanently exceed the writable count. ShouldGrowVolumes then returns true forever, every assign-path grow request passes the gate, and the periodic grow loop fires too, creating volumes without bound -- worse with -volumePreallocate. Count crowded as the intersection with writables instead: growth checks and the layout gauges only see crowded volumes that can still take writes. |
||
|
|
1ce106e69d |
s3: audit the assumed-role principal and the STS caller (#10519)
* s3: log the requester's principal ARN in the audit entry An STS session authenticates as an opaque session subject, so requester alone gave an operator no way back to the assumed role or the session name. Record the principal ARN next to the identity name and emit it as requester_arn. * s3: record the caller identity in the STS handlers AssumeRole, GetFederationToken and GetCallerIdentity verify the caller themselves and are not wrapped by the auth middleware that records the identity, so every audit entry for minting a session had an empty requester. * s3: resolve the audit principal ARN the way policy evaluation does A JWT-authenticated identity carries no PrincipalArn — the auth layer hands the principal over in a request header — so reading the field directly left requester_arn empty for OIDC callers. buildPrincipalARN is the resolver the policy path already uses: header first, then the identity's own ARN, then a synthesized user ARN for legacy identities that have none. |
||
|
|
fa432f9a6a |
s3: keep an admin's role session scoped to the role (#10520)
* s3: keep an admin's role session scoped to the role AssumeRole copied the caller's admin standing into the minted session as the is_admin claim, which short-circuits base policy evaluation. An admin assuming a scoped-down role therefore kept full access and the role's attached policies, explicit denies included, were never evaluated. Only a session the caller assumed for itself carries the claim now — a legacy static admin has no IAM policies for such a session to inherit. * s3: name the caller when it assumes a session for itself An identity that carries no principal ARN left the self-assumed session with an empty role name in its assumed-role ARN. callerPrincipalArn synthesizes the canonical user ARN for that case. |
||
|
|
82c67b5896 |
test: cover listings spanning a run of retracted keys (#10517)
* test: cover listings spanning a run of retracted keys A listing drops entries whose current version is a delete marker. When a run of consecutive entries drops out, the page being filled can come back empty, and an empty page is easily mistaken for the end of the listing — everything after the run then never appears and the caller is told those objects do not exist. Backup repositories produce exactly this shape: a batch of keys under one prefix is retracted while writing continues under the next. Covers a retracted run before live keys and between live keys, walked with page sizes smaller than the run so at least one page is filled entirely from entries that get dropped, plus the version view of the same namespace where every version and every delete marker must still be reported. * test: sweep every page size in both walks and paginate the version listing |
||
|
|
d8d29c4ede |
s3: carry storage class in the cached listing metadata (#10516)
A listing on a versioned bucket is served from metadata cached on the .versions directory entry so the whole listing is a single scan. The cache carried size, mtime, ETag, owner and the delete-marker flag but not the storage class, so newListEntry found none and fell back to STANDARD. The result was that HEAD and the listings disagreed about the same object: HEAD reported the class the object was stored with, while ListObjectsV2 and ListObjectVersions reported STANDARD for every object. Clients that filter or tier on storage class act on the listing. Caches the class alongside the other listing fields, clears it with them, and copies it in the routed RECOMPUTE_LATEST path so both finalize paths agree. |
||
|
|
910fa1ff37 |
test: compare ListObjects and ListObjectVersions over the same namespace (#10515)
* test: compare ListObjects and ListObjectVersions over the same namespace The two listings walk the same tree through separate code paths, so a client navigating by versioned listings can see a different namespace than one navigating by plain listings, and concludes keys are missing that are plainly there. Testing each path on its own never catches that; only comparing them does, and nothing compared them. Asserts both report identical current keys and identical common prefixes across a backup-shaped tree: nested prefixes, a prefix naming an object exactly, a key that is simultaneously an object and the parent of other keys, a partial key fragment, and a prefix matching nothing. The version view is reduced to what a plain listing reports — latest versions that are not delete markers — so the comparison is like for like. * test: guard against truncated pages and cover the delete-marker path |
||
|
|
fce4da5c9c |
test: pin verb parity on lock-arbitration keys through acquire and release (#10514)
* test: pin verb parity on lock-arbitration keys through acquire and release Backup clients arbitrate repository ownership by writing and retracting small keys under a fixed prefix and re-probing them, each probe using a different verb. They trust those verbs to agree; a key reported present by one and absent by another makes the client either spin or declare the repository corrupt, and neither shows up as an error on the storage side because each individual answer is locally correct. The keys are written and immediately deleted by version id, which is the cycle that empties a version container, so parity is asserted on both sides of the delete and across repeated re-acquire cycles where residue accumulates. Reports which verbs disagreed rather than just failing. * test: run the reacquire cycle on every lock key, and drain probe bodies |
||
|
|
33a974b4c5 |
test: pin that an unusable version id is refused, never resolved (#10513)
* test: pin that an unusable version id is refused, never resolved A version id containing a path separator, or "." / "..", can never name a stored version. Resolving one to the null or latest version instead would let a caller destroy a live version by asking for one that does not exist, on a bucket configured for immutability. The guard exists today and holds on every verb; it had no test. Pins two properties: such a request is refused with a client error rather than a 5xx (a 5xx invites endless retries of something that can never succeed), and the version that does exist survives every refused request. * test: require exactly 400 for an unusable version id |
||
|
|
2961448a36 |
test: cover delete idempotency on versioned object-locked buckets (#10512)
* test: cover delete idempotency on versioned object-locked buckets Backup clients probe and retract lock keys continuously, so they routinely delete keys and versions that are already gone, and they batch those deletes alongside keys that do exist. S3 makes all of that succeed; returning an error turns ordinary lock arbitration into a job failure. The behaviour is correct today but had no coverage, and it runs through the object-lock retention check, which is the most likely place for a missing object to start being reported as an error. Covers: deleting a key that never existed, deleting a well-formed version id that names nothing (twice, and without disturbing the version that does exist), and a batch whose middle key is missing — every requested key must come back under its own name rather than silently taking another row's slot. * test: verify the deletes actually took effect, not just that they returned |
||
|
|
f582c8451b |
s3: report a peer that went away as ClientDisconnected, not IncompleteBody (#10511)
* s3: report a peer that went away as ClientDisconnected, not IncompleteBody A streaming PUT whose body ends early is always reported as IncompleteBody (400). That collapses two cases with opposite causes: the peer vanished mid-upload, and the peer sent fewer bytes than it promised while still connected. The first points at the network path, the second at the client, and once merged they cannot be told apart from the logs. Split out ClientDisconnected (499) and select it when the request context shows the peer is gone. The upload itself keeps running on a background context so chunks still finish, which means cancellation races the read error; a missed signal degrades to IncompleteBody exactly as before. * s3: note what request-context cancellation is taken to mean |
||
|
|
0720955ea7 |
helm: pass extraEnvironmentVars and security.toml to the bucket-creation hook (#10477)
* helm: give the bucket hook the credentials weed shell needs The post-install bucket hook pipes s3.configure into `weed shell`. Since #9442 the filer only serves its IAM gRPC service to callers presenting an admin-signed JWT, and since #9536 `weed shell` mints that token itself - but only if it can find the filer signing key. The hook job sees neither a security.toml nor the WEED_* environment overrides: its env list is hardcoded, and it is the only workload in the chart without a security.toml mount. So with jwtSigning.filerWrite=true the hook logs error: failed to get user anonymous: rpc error: code = Unauthenticated desc = missing authorization metadata and `weed shell` exits 0 regardless, which leaves the Job green while anonymousRead is never applied. Render the merged extraEnvironmentVars into the job's env - keeping non-string values as valueFrom, so keys held in a Secret stay a reference - and mount security.toml the way every other workload does. The hardcoded WEED_CLUSTER_* entries go away because those values are part of the global extraEnvironmentVars defaults and would otherwise render twice. Signed-off-by: Sebastian Preisner <preisner@puzzle-itc.de> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * helm ci: report a missing bucket hook Job instead of crashing Signed-off-by: Sebastian Preisner <preisner@puzzle-itc.de> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * helm ci: assert where the bucket hook mounts security.toml A mount under the wrong path leaves weed shell without the key just as surely as no mount at all, so check the target, not only the name. Signed-off-by: Sebastian Preisner <preisner@puzzle-itc.de> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * helm: keep the bucket hook's cluster endpoints chart-computed Dropping the hardcoded WEED_CLUSTER_* left the readiness waits dereferencing $WEED_CLUSTER_SW_*, which exist only while the values keep the default alias. Rename the cluster, or clear extraEnvironmentVars, and the hook polls "http://" for five minutes and then fails the release. Derive the env names from the alias and render the addresses from the chart, as all-in-one already does. * helm ci: assert the bucket hook follows a renamed cluster alias Renames the cluster and drops the default addresses, then checks that every env name the readiness waits dereference is set, and that the address is the chart's rather than the one left in the values. * helm ci: check the hook's endpoints and security.toml source The bucket hook tests took two shortcuts a wrong render slips through. The alias test only rejected the stale master address the values kept, so any other wrong address passed and the filer address was never compared at all. And the security.toml test matched the volume by name, which a same-named emptyDir or a foreign ConfigMap satisfies while `weed shell` still has no signing key. Compare both cluster addresses against the Services the chart renders, in the default and the renamed-alias case, and assert the volume is backed by the chart's security-config ConfigMap. Checked by pointing the volume at another ConfigMap and the filer env at a wrong but non-empty address: both now fail, both passed before. Signed-off-by: Sebastian Preisner <preisner@puzzle-itc.de> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Sebastian Preisner <preisner@puzzle-itc.de> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
|
7b8188fc41 |
wdclient: age vid map entries by generation instead of chaining snapshots (#10506)
* wdclient: age vid map entries by generation instead of chaining snapshots The vid map kept its history as a linked list of past snapshots, trimmed in place by storing nil into a node's cache pointer. That cost up to six full copies of the volume-location map, a recursive walk taking a different lock per level, and deletes that had to cascade through every generation. It also had to special-case explicitly-empty entries, or fallback would resurrect locations a newer snapshot had cleared. Keep one map instead, and stamp each entry with the generation it was learned in. resetVidMap bumps the generation and drops entries that were not relearned within the retained window, which is the same retention the chain provided: an entry survives DefaultVidMapCacheSize resets. The first write of a generation replaces an entry rather than merging into it, so a volume that moved answers with where it is now — the property a fresh map per reset used to give for free. Entries are copy-on-write, so locations handed to a caller are no longer shifted underneath it by a concurrent delete. The map is never swapped now, so the client-side lock and its stable / current accessors go away with it. * wdclient: make vid map entries immutable and drop them once emptied Review follow-up. Updating an entry in place left the copy-on-write guarantee resting on callers never holding the entry pointer; install a new entry instead, so the rule is simply that a stored entry never changes. Deleting a volume's last location now drops the entry rather than keeping an empty one, which a client that never resets would otherwise hold for every volume it ever saw deleted. Lookups already treat an empty entry as a miss, so nothing observable changes. * wdclient: let the newest generation decide between regular and EC locations GetLocations checked the regular map first whatever its generation, so a volume that was EC encoded kept answering with the regular copies the previous master knew until they expired — for as long as the retained window, since nothing relearns a copy that no longer exists. The snapshot chain did not have this problem: the newest map was consulted first and only a volume it knew nothing about fell through to older ones. Restore that by comparing generations, with regular copies winning a tie, since a tie means one generation reported both. |
||
|
|
ae4839e005 |
mount: keep a sealed chunk alive until its own upload finishes (#10504)
Sealing a logic chunk index that already held a sealed chunk dropped the old chunk's only reference and freed its page chunk. That chunk's upload may not have started reading it yet — Execute() returns as soon as the job is handed to a goroutine — so mem.Free could hand a live 2 MiB mem chunk back to the slot pool, the next NewMemChunk would overwrite it, and the in-flight upload shipped whatever bytes were there. Under fio randwrite the volume server rejected those needles with "Content-MD5 did not match md5 of file data" and the FUSE write failed with EIO. Give the sealed chunk a second reference for its upload, dropped only by the upload itself, and let the upload unindex itself only while it still owns the index — the unconditional delete could evict a newer sealed chunk and hide its dirty pages from readers. |
||
|
|
7fb36025b3 |
wdclient: read the vid map cache link before the live map (#10505)
resetVidMap trims the cache chain by storing nil into a node's cache pointer once it ages past vidMapCacheSize. A lookup that missed in its own map and only then loaded that pointer could find the link already severed, reporting "not found" for a volume that stayed resolvable the whole time. Load the link first, while it is still guaranteed live. A published vidMap's cache pointer only ever goes from its ancestor to nil, so reading it earlier can never yield staler history. |
||
|
|
3514925581 |
filer: let a nested path rule turn worm off (#10503)
* filer: let a nested path rule turn worm off mergePathConf ORs the booleans, so worm set on a bucket could never be lifted on a directory under it, while every string field is overridden by the more specific rule. Make worm tri-state instead: unset inherits, set wins. readOnly, fsync and disableChunkDeletion keep the OR, so a nested rule still cannot escape a lock the bucket set. Configurations written before this carry an explicit "worm": false on every rule, because they are marshalled with EmitUnpopulated. Reading those back as an override would quietly drop worm from nested paths, so filer.conf is now stamped with a version and the flag is dropped to unset when the version predates it. * filer: copy the worm value out of the matched rule mergePathConf aliased the pointer into the merged result, so a caller that wrote through it would reach into the stored rule. |
||
|
|
4dc1b70b2f |
test: pin that a .vif replication outranks the superblock (#10499)
* test: pin that a .vif replication outranks the superblock Store.ConfigureVolume rewrites the .vif and never the replica-placement byte in the .dat, so that byte keeps whatever the volume was created with for good. readSuperBlock reads it and then overrides it from the .vif, which is what makes a replication change take effect and survive a remount. Invert that and every replication change silently reverts on the next mount, while the .vif on disk still records what the operator asked for -- a durability setting quietly going back to its old value, with nothing to indicate it. Worth pinning rather than reading off the code, because the field beside it resolves the other way: version takes the superblock over the .vif. Two fields, one function, opposite precedence, each a line to invert wrongly. Covers the empty case too, since a .vif that declares no replication has to leave the superblock standing or a volume whose replication was never configured would be forced to whatever the zero value parses as. * test: drop the unreachable nil check on MaybeLoadVolumeInfo It initialises the returned pointer before the existence check and every return is naked, so it never yields nil. Guarding against it implied a contract the callee does not have. |
||
|
|
3e74db1609 |
fix(master): bump seaweedfs/raft to v1.2.0 for the snapshot race (#10498)
The master could die with a nil pointer dereference inside raft.(*server).TakeSnapshot. The leader and follower loops ask for a snapshot on every iteration and callers can ask at any time, so several goroutines built one at once, all writing to the one pendingSnapshot field. Whichever finished first saved it and set the field to nil, and the rest either dereferenced nil while attaching peers and state, or stored nil as the current snapshot, leaving the master with no snapshot to send to a lagging peer. v1.2.0 holds a mutex for the length of a snapshot and keeps the one being built in a local. It also stops a snapshot recovery that cannot finish from being reported as done, writes snapshots to one side and renames them into place, loads the snapshot covering the most of the log rather than the last one in filename order, and takes the lock that guards the peer map on both sides, where a snapshot cloning the peers alongside a membership change was a concurrent map iteration and write. |
||
|
|
63d5140485 |
s3: allow copying an object onto itself in a versioned bucket (#10497)
* s3: allow copying an object onto itself in a versioned bucket The copy writes a new version instead of overwriting in place, which is how an earlier version is restored. Buckets with versioning off or suspended keep rejecting a self-copy that changes nothing. * s3: cover the suspended-versioning self-copy rejection Suspended versioning overwrites the null version in place, so a self-copy that changes nothing stays rejected. Pin that alongside the never-versioned case. |
||
|
|
e7a678fa72 |
s3: keep the list marker exclusive for versioned objects (#10496)
* s3: keep the list marker exclusive for versioned objects A versioned object lives in a "<key>.versions" directory, so the entry name never matched the marker and start-after/marker returned the marker key itself. * s3: match the list marker against the raw entry name too A backend that echoes the marker it was given returns the ".versions" directory name, which no longer matched once the comparison used the object name alone. Cover both, and unit test each half. |
||
|
|
ccf5dc34e9 |
test: stop comparing two JWTs minted a second apart (#10495)
TestProxyReadDropsCallerJwtQueryParam mints a read token up front and requires the token the volume server would evaluate to equal it byte for byte. The expiry claim has one-second resolution -- GenJwtForVolumeServer sets it from jwt.NewNumericDate(time.Now().Add(...)) -- so two mints on either side of a tick produce different strings for the same authority and the same file, and the assertion fails for a reason the test is not about. It surfaces on the 32-bit job, where the runner is slow enough that the HEAD subtest (the second one, after a full proxy round trip) lands in a later second than the mint at the top of the test. Confirmed directly: minting the same file id with the same key either side of a boundary yields different tokens. Assert what the test is actually about instead -- that the credential decodes against the read key and authorizes this file id -- which holds whatever second it is minted in, and is a closer statement of the property than string equality. |
||
|
|
09da5f634f |
helm: optional NetworkPolicy per component (#10479)
* helm: optional NetworkPolicy per component In a namespace with a default-deny policy the chart cannot be installed: the components never reach each other, and the post-install bucket hook waits on the master and filer until it gives up. networkPolicy.enabled renders one policy per component, selecting its pods by the standard app.kubernetes.io labels and admitting the other pods of the release on the ports that component listens on. The port lists come from the same values as the containerPorts, and CI asserts the two agree. Restricting egress is a second opt-in with extraEgress for the filer store and notification sinks, which the chart cannot know about. Closes #10421 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * helm: say "changed" instead of "retuned" in the policy comments codespell reads "retuned" as a misspelling of "returned" and fails the spelling job. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * helm: refuse empty port and DNS peer lists instead of widening In a NetworkPolicy an empty ports list means every port and a missing peer selector means every pod, so `kubeApiServer.ports: []` silently opened the API server CIDRs on all ports, and nulling a DNS selector rendered `podSelector: null`, which is every pod in kube-system. Both now fail the render, and the DNS rule emits only the selectors that are set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * helm: gate the bucket hook Job and its policy on one helper Both were deriving the same condition from the same values, kept in step by a comment. seaweedfs.bucketHookEnabled makes it one definition, so adding an S3 mode cannot leave the Job running without its policy - which under default-deny means the hook hangs. CI pins the pairing across the eleven modes that decide it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * helm: say that an egress default-deny needs both toggles networkPolicy.enabled on its own only covers a default-deny that restricts ingress. Where Egress is in its policyTypes as well, which is the usual baseline, the components still cannot resolve DNS and egress.enabled is required too. Both values and the README said the first half of that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * helm: quote the label values the policies emit The same int-coercion the hook templates were fixed for, in the file this PR adds: unquoted, a release named 123 renders app.kubernetes.io/instance as a YAML integer in the metadata, the podSelector and both peer selectors, and the API server rejects the object - a policy that silently never applies. CI now renders the chart as release "123" and fails if any policy label comes out as a non-string. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * helm: gate the resize hook policy on the same lookup as its Job The policy rendered whenever the hook was enabled, which is the default, so every release carrying networkPolicy got one - as a pre-install hook that Helm never collects, left in the namespace after uninstall. It also made egress.kubeApiServer.cidrs mandatory for every release, since the policy claims the API server, for a Job that only runs on an upgrade that grows a PVC. Move the command computation the Job is gated on into a helper and read it from both. * helm: drop the API server rule from the admin policy No seaweedfs binary talks to the Kubernetes API - there is no client-go in go.mod - so the rule granted admin an egress path it never uses, and forced anyone running admin with egress on to name an API server address for it. The pod-RW ClusterRole the comment was reasoning from is a leftover from a migration and is not read by any component. * helm: reject a networkPolicy.components key that names no component The component names are not guessable - objectstorage-provisioner, seaweedfs-all-in-one, volume-<name> - and a typo silently dropped the rules it was carrying. Check against every component the chart can produce, not the enabled ones, so a values file shared across releases can still hold overrides for a component this one leaves off. * helm: name the all-in-one policy after the workload componentName prefixes the release fullname onto the suffix it is given, and the component label already starts with seaweedfs-, so the policy came out as <release>-seaweedfs-seaweedfs-all-in-one. * helm ci: assert the denied probe failed rather than that it did not succeed kubectl run's "pod/x created" was captured alongside the pod log, so an empty log would still not match exit=0 and the denial would pass without anything having been tested. * helm: document what turning the network policies on costs Three things the values did not say: a Prometheus outside the release stops scraping and nothing reports it, the resize hook's policy is a hook resource that uninstall leaves behind, and the DNS selectors are wrong on OpenShift. * helm: spell out the managed distributions codespell reads as a typo codespell has AKS in its dictionary as a misspelling of ASK, so the DNS selector note failed the spelling job. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * helm ci: read the probe verdict from the marker line, not the whole log The default-deny step decides both probes from the pod log, but kubectl logs returns stderr as well, and busybox wget reports "download timed out" there even under -q. The denied probe therefore produced two lines beginning with wget:, which matches neither exit=0 nor exit=*, so a correct denial landed in the arm meant for a probe that produced no result at all and failed the job. The earlier form hid this behind a catch-all that treated anything without exit=0 as a denial; tightening that assertion made the stray line fatal without narrowing the input it reads. Pick the marker line out instead. The trailing || true is required: the step runs under bash -e, so a grep that matches nothing would abort it rather than reach the arm that reports an empty result, which is the case that assertion exists to catch. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Sebastian Preisner <preisner@puzzle-itc.de> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
|
78ed665557 |
webdav: answer PROPFIND child stats from the listing (#10492)
* webdav: answer PROPFIND child stats from the listing golang.org/x/net/webdav discards the FileInfo that Readdir returned and stats every child again, five times over, so a PROPFIND on a directory costs five sequential filer lookups per entry: 15006 lookups and 1.4s for 3000 subdirectories, 24s for 60000. Windows Explorer times out well before that. Hold the entries a listing already fetched for the lifetime of the request and serve those stats from them - 6 lookups and 0.03s for the same 3000 entries. WebDavFile.Stat has to stop dropping its request context for the held entries to be reachable. * webdav: keep the request context on the lookups a listing drives stat, Readdir and Seek reached the filer on context.Background(), so a client that walked away left the listing streaming and the lookups running. Seek also missed the entries the listing had already fetched. Write and cleanup paths keep their own context - a cancelled request must not abandon a flush half done. |
||
|
|
a1a3ac5b82 |
helm: label the hook Job pods so selectors can reach them (#10478)
* helm: label the hook Job pods so selectors can reach them The bucket hook pod carries only managed-by/instance and the volume resize hook pod carries no labels at all, so nothing keyed on the standard app.kubernetes.io set can address either of them. Give both the same name/chart/managed-by/instance/component labels the other workloads use, with the component naming each hook. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * helm ci: assert the whole label set on the Job and its pod The block checked three keys and only looked at the pod's component, so a missing chart/managed-by label, a wrong component on the Job, or a Job and pod that disagree would all have passed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * helm ci: compare the hook's release labels against a real workload Presence alone let a wrong value through. The name/instance/chart values now have to match a workload that already renders them, which also keeps the chart version out of the test. managed-by stays a presence check: the chart puts it on workload metadata but not on pod templates. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * helm: quote the resize hook's managed-by and instance labels Matches the bucket hook, and keeps a numeric release name a string instead of an int the API server rejects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Sebastian Preisner <preisner@puzzle-itc.de> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> |
||
|
|
01937cfad1 |
telemetry: build the over-time charts from confirmed clusters (#10489)
Short-lived clusters report once under a fresh raft topology id and never again, so CI runs and demo stacks each become their own cluster. Held forward for the whole active window they pile up, and the volume server line climbs every day while capacity stays flat. Sum the fleet series over confirmed clusters only, the same set the version and OS charts already use. |
||
|
|
46ceb253b0 |
telemetry: report anonymous cluster stats by default (#10488)
The reports are what tell us which versions and cluster sizes are actually in use, and almost nobody flips the flag on, so the numbers we have are close to useless. Default it on for master, server and mini, and say in the flag help and the startup log how to turn it off. Nothing new is collected: still an in-memory cluster id that changes on restart, version, os, server counts, volume count and disk bytes, sent once a day by the leader master only. |
||
|
|
c2183566b6 |
ec.encode: name the shard ids an aborted deletion found (#10486)
Counting by node lost the per-node ShardsInfo, and with it the shard ids the old summary printed -- the message an operator gets when the pre-delete check refuses now says only how many shards each node holds. That is the wrong half. A set holding shards 0-9 and one holding 4-13 are both "10 shards", and only the ids say whether what survived can rebuild the volume, or which node to go looking at. Keep the count and list the ids beside it. |
||
|
|
a4692005e9 |
ci: harden the fusermount3 repair (#10485)
* ci: move the fusermount3 repair into a composite action Three copies of the same block were already drifting apart, and the target comes from PATH: only ever add setuid root to a root-owned, non-symlink binary under the system bin paths, and say why otherwise. * test: say that the process exited in the wait errors "process exit status 1 before ... accepted connections" is missing its verb. Also mark the SIGTERM return discarded - it fails with os.ErrProcessDone exactly when the select below already handles it. * ci: prefer the distro fusermount3 over escalating a shadow copy The shadowing /usr/local/bin/fusermount3 is not root-owned either, so setting its setuid bit would have handed root to a binary the runner user owns - the repair now symlinks the distro one earlier in PATH and touches nothing, keeping the in-place chmod for a root-owned binary with no distro alternative. A setuid bit only grants root when root owns the file, so accept an existing one only then. * ci: run the FUSE workflows when the shared action changes Their paths filters listed each workflow file but not the composite action all three now call. |
||
|
|
c4798979d8 |
ec.encode: count shards wherever they landed before deleting the source (#10483)
generateEcShards writes shards beside the source volume, so encoding a volume that lives on a non-default medium puts them on that medium while -diskType still says hdd. The pre-delete check counted only the -diskType bucket, so it saw a complete set as zero shards, called it unrecoverable and aborted -- leaving the volume as both a .dat and a full shard set, which every later reader then disagrees about. Count by node across disks, as waitForEcShardsToRegister in the same file already does. The spread check is unaffected: it locates shards through collectEcShardBitsByNode and only uses diskType to find free slots. |
||
|
|
167c114dae |
ci: fix FUSE mounts against the new runner image (#10484)
* ci: restore the setuid bit on a shadowed fusermount3 Newer ubuntu-22.04 runner images carry a source-built fusermount3 in /usr/local/bin that shadows the distro one in PATH and is not setuid root. go-fuse looks the helper up through PATH, so every unprivileged mount fails with "mount failed: Operation not permitted". * test: fail a fuse test as soon as its mount process dies A mount that cannot mount at all exits within a second, but the harness still waited out the 30s readiness timeout and then reported "mount point not ready within timeout", leaving the real cause buried in the log tail. Watch the child processes and report their exit instead. * mount: report a failed mount without a goroutine dump A mount failure is an environment problem - no /dev/fuse, fusermount not setuid, stale mount point - and the all-goroutine stack dump Fatalf adds buries the one line that says so. |