mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-07-25 17:42:54 +00:00
4.39
375
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  
|
bdcc3154ed |
refactor: centralize genUploadUrl in UploadOption (#10164)
* refactor: centralize genUploadUrl in UploadOption Replace inline genFileUrlFn closures with operation.GenUploadUrl field: - Add GenUploadUrl func(host, fileId) string to UploadOption struct - Add GenUploadUrlProxy(filerAddress string) utility function - Remove genFileUrlFn parameter from UploadWithRetry signature - Update all callers: mount, gateway, mq, filer_copy, filer_sync This matches the weed mount -filerProxy pattern exactly, factorizing the URL generation logic across all consumers. Co-Authored-By: Athena 🏛️ <hermes-agent@local> (custom / Qwen3.6-35B-A3B-UD-Q4_K_XL.gguf) * docker release: run all platform jobs in one wave, cache rocksdb compile Drop max-parallel so the 13 per-platform builds run together instead of two waves of 8 (rocksdb was queuing behind the cap and starting ~8 min late). Keep cache-to mode=max for rocksdb: its RocksDB static_lib compile is sha-independent, so it caches across releases and stops being the ~16-min long-pole that gates the merge fan-in. go-build variants stay mode=min. Co-Authored-By: Athena 🏛️ <hermes-agent@local> (custom / Qwen3.6-35B-A3B-UD-Q4_K_XL.gguf) * refactor: centralize genUploadUrl in UploadOption Replace inline genFileUrlFn closures with operation.GenUploadUrl field: - Add GenUploadUrl func(host, fileId) string to UploadOption struct - Add GenUploadUrlProxy(filerAddress string) utility function - Remove genFileUrlFn parameter from UploadWithRetry signature - Update all callers: mount, gateway, mq, filer_copy, filer_sync This matches the weed mount -filerProxy pattern exactly, factorizing the URL generation logic across all consumers. Co-Authored-By: Athena 🏛️ <hermes-agent@local> (custom / Qwen3.6-35B-A3B-UD-Q4_K_XL.gguf) * Remove accidental ROCmFPX submodule reference * gofmt chunk upload option block * Preserve broker cipher and re-read proxy filer per upload attempt Chunk uploads must keep the configured Cipher, and both the mount and broker current filer can change on failover, so build the proxy upload URL inside the closure instead of capturing the address once. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
 Chris LuandGitHub
|
f475d60fcf |
mount: move directory cache state to a side map to shrink InodeEntry (152 to 32 bytes) (#10114)
mount: move directory cache state to a side map to shrink InodeEntry The mount keeps an InodeEntry alive for every inode the kernel references. On a mount that is almost entirely regular files, each entry carried the full directory readdir-cache bookkeeping (four time.Time fields plus counters), bloating it to 152 bytes whether or not the inode was a directory. Move that state into a dirState held in a side map keyed by inode, and drop the isDirectory bool: an inode is a directory iff it has a dirState. InodeEntry is now just paths + nlookup at 32 bytes, landing in a smaller Go allocator size class; on a mount with tens of millions of cached file inodes that is several GB less resident heap. As a side effect the readdir-cache scan helpers iterate only directories instead of every inode. |
||
|
Chris LuandGitHub
|
0f1ec8983d |
mount: don't fail close() on a benign FUSE interrupt (#10102)
A FUSE interrupt is not a process kill. Go's async preemption (SIGURG) makes a close() under load emit an interrupt on nearly every flush, so deriving the metadata-flush context from the FUSE cancel channel turned healthy concurrent close()s into EIO: the interrupt cancelled the in-flight CreateEntry, which surfaced as "input/output error". Bound the flush with a deadline instead. A healthy CreateEntry finishes in well under a second, so the deadline only fires against a genuinely stuck filer -- still keeping close() from hanging forever -- while benign preemption no longer aborts a good flush. |
||
|
Chris LuandGitHub
|
95427b5573 |
security: add BearerPrefix constant for Authorization headers (#10101)
Introduce security.BearerPrefix ("Bearer ", RFC 6750) and use it
everywhere an "Authorization: Bearer <token>" header is constructed,
replacing the scattered "BEARER "/"Bearer " string literals. SeaweedFS
matches the scheme case-insensitively when parsing (security.GetJwt), so
behavior is unchanged; this removes the magic string and settles the
casing on the standard form. The parser's upper-case comparison stays as
is on purpose.
|
||
|
5456f9d695 |
mount: confirm an empty directory rebuild before caching it (#10092)
A directory rebuild wiped the cached children, listed the filer once, and published the directory authoritatively cached over whatever came back. A transient empty listing -- a momentary list-stream glitch that ends as a clean EOF with no entries -- then stranded a populated directory cached over an empty store, hiding every file in it until some unrelated event happened to rebuild it: stat returns ENOENT and readdir returns nothing though the files are safe on the filer, and nothing re-triggers a build. Re-read the directory when the listing comes back empty before trusting it. The first re-read is immediate, since the likely transient clears on a fresh stream; later attempts space out. A genuinely empty directory still lists empty every time and caches as before, so only empty listings pay the extra read. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
Chris LuandGitHub
|
5112da98a2 |
mount: skip redundant permission checks under default_permissions (#10089)
With default_permissions (the mount default) the kernel enforces unix permission bits from the getattr/lookup attributes before it ever calls Open, Create, or Mknod. The mount was re-checking permissions in AcquireHandle and createRegularFile anyway, which duplicated the kernel's work and kept the supplementary-group lookup on the per-file hot path. Gate only the mode-bit access check on default_permissions being off, so a non-root copy does no permission work on open/create. createRegularFile still loads the parent to validate it exists, since the create RPC skips the filer-side parent check. With default_permissions off the mount remains the sole enforcer, so the full check still runs. |
||
|
Chris LuandGitHub
|
ef109fe9e1 |
mount: don't hang close() when a writer is killed during flush (#10090)
* operation: bound AssignVolume with a deadline AssignVolume ran on context.Background(), so when the filer is overwhelmed the RPC could block indefinitely and wedge every caller holding the connection. Give it a 30s deadline so a stuck assign fails and the caller's retry/error path runs instead of hanging forever. * mount: abort flush when the FUSE request is interrupted On close(), a killed process blocks in fuse_flush waiting for the mount to answer. doFlush ran its metadata CreateEntry on context.Background() and ignored the kernel interrupt channel, so against an overwhelmed filer the flush never completed and the process stayed in uninterruptible sleep -- making the pod un-killable. Derive a context from the FUSE cancel channel in Flush/Fsync and thread it through doFlush -> flushMetadataToFiler -> streamCreateEntry; the retry loop stops as soon as the context is cancelled. Release and the pre-rename flush keep a non-cancellable context since they must finish regardless. * operation: harden the AssignVolume timeout test Make the test double's signal send non-blocking and bound the receive with a timeout so a regression can't wedge the test instead of failing it. |
||
 
|
1e2412e502 |
fix: enforce XATTR_REPLACE semantics in setxattr (#10059)
* 修复weedfs_xattr.go 中 XATTR_REPLACE 语义缺失 * mount: fix XATTR_CREATE/XATTR_REPLACE flag semantics in setxattr XATTR_CREATE fell through into the XATTR_REPLACE branch: creating a new attribute hit the empty-oldData guard and returned ENODATA instead of creating it, while creating over an existing attribute silently succeeded without the EEXIST that setxattr(2) requires. Drop the fallthrough chain so CREATE returns EEXIST when the attribute already exists, REPLACE returns ENODATA when it is missing, and otherwise the value is written. Test existence via the map lookup so an attribute with an empty value is still treated as present. --------- Co-authored-by: 王郁文 <wangyuwen@cmict.chinamobile.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
20e4614fc6 |
feat(mount): attach Content-MD5 to chunk uploads (#10016)
* mount: attach Content-MD5 to chunk uploads
Mount writes never set UploadOption.Md5, so FileChunk.ETag stays empty
and filer.ETag() degenerates to md5("")-N: same-size files compare
equal regardless of content, defeating metadata-level verification
like filer.sync.verify.
Compute each chunk's MD5 and send it as Content-MD5. The volume server
verifies it on ingest (rejecting in-flight corruption) and echoes it
back, persisting FileChunk.ETag like filer/S3 writes already do.
The dirty-page flush paths already pass a *util.BytesReader whose
backing slice is the whole chunk, so the digest is taken in place with
no extra read, copy, or allocation (UploadWithRetry unwraps it the same
way downstream). Only the rarer plain-reader callers (e.g. manifest
chunks) fall back to io.ReadAll. Skipped under -cipher, where only the
ciphertext reaches the server.
The digest encoding (std-base64 of the raw md5) is the contract the
volume server verifies against, so it is factored into contentMD5Base64
and covered by a unit test.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* mount: compute chunk Content-MD5 in the uploader, not the caller
Move the WantMd5 hashing into UploadWithRetry, where the chunk is already
buffered for the retry path, so saveDataAsChunk stops type-switching the
reader and re-reading plain readers. One materialization point, and the
cipher exclusion lives next to the hash instead of at every call site.
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Chris Lu <chris.lu@gmail.com>
|
||
|
|
18868e5204 |
fix(mount): run entry invalidations off the meta-cache apply loop (#10002)
* fix(mount): run entry invalidations off the meta-cache apply loop The apply loop ran invalidateFunc inline, which acquires the open file handle's lock in fhLockTable. Meanwhile flushMetadataToFiler holds that same fh lock and then waits on the apply loop (applyLocalMetadataEvent). When both target the same open file concurrently, the loop blocks on the fh lock while the lock holder blocks on the loop: an ABBA deadlock that backs up every later readdir/flush and hangs the mount. Fix: dispatch entry invalidations to a dedicated FIFO worker goroutine so the apply loop never blocks on locks held by goroutines waiting on it. Adds a regression test reproducing the interleaving. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * perf(mount): update invalidate counter once per batch Run the batch's invalidateFunc calls without re-taking invalidateMu per item, then bump invalidateProcessed and broadcast once after the loop. WaitForEntryInvalidations only needs the count to reach its target and a batch always completes together, so the per-item lock + broadcast was wasted work. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * mount: extract the invalidate worker into util.AsyncBatchWorker The apply loop's off-thread entry-invalidation queue was a one-off mutex + cond + slice + counters living inside MetaCache. Pull it out as a generic unbounded FIFO worker so the deadlock-avoidance contract (never block the producer, drain on shutdown, wait-for-quiesce) lives in one place. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris LuandGitHub
|
0a45c4d097 |
mount: cache supplementary group IDs for non-root access performance (#10008)
* mount: cache supplementary group IDs to improve non-root access performance * mount: clear supplementary group cache between tests and add cache verification test * mount: add docstrings and benchmarks for supplementary group cache * mount: add performance test demonstrating cache effectiveness * mount: add TTL-based cache expiry for supplementary group IDs (5-minute refresh) |
||
|
Chris LuandGitHub
|
7c32e651f7 |
mount: fix deadlock reading an uncached remote-mounted file (#9995)
* mount: apply cached remote entry without blocking the read Reading an uncached remote-mounted file hung forever. The read holds the file-handle shared lock across the on-demand download, then synchronously waits on the metadata apply loop to apply the cached entry. The filer's update event for that same object reaches the apply loop first and runs invalidateFunc, which wants the file-handle exclusive lock — held in shared mode by the still-running read. The loop blocks on the read; the read blocks on the loop. Enqueue the apply without waiting so the read never blocks on the apply loop while holding the lock. The handle is already updated via SetEntry, and the filer subscription delivers the same event regardless. * mount: regression test for uncached remote read deadlock Drives the real Read path through downloadRemoteEntry with a stub filer that broadcasts the matching invalidate event before returning, reproducing the lock-ordering deadlock. Fails (times out) without the fix. |
||
|
Chris LuandGitHub
|
1826a5d222 |
fix(mount): pin rebuild entries by their own inode, not inodeToPath (#9993)
isLocalOnlyEntry resolved the pinned-child check through inodeToPath. A kernel Forget drops the path→inode mapping once the lookup count reaches zero, but an async writeback flush — and the file handle, still in fhMap during the drain — is keyed by inode and outlives that mapping. Between Release dispatching the async flush and the flush reaching the filer, a Forget could unpin an in-flight create, so a concurrent directory rebuild would wipe it and the file would ENOENT until the flush lands and the cache refreshes. Key the check off the inode the store entry already carries (createFile stamps it into the placeholder), so the pin no longer depends on a mapping Forget can remove. |
||
|
Chris LuandGitHub
|
93f91c96ca |
fix(mount): keep a deferred local create from vanishing when its dir is rebuilt (#9991)
wfs.Create defers the filer create to Flush and inserts a local-only placeholder into the metaCache (dirtyMetadata=true), so a just-created file exists locally before the filer has it. When the parent directory falls out of cache (hot-dir read-through, idle evict) and is rebuilt, EnsureVisited wipes the store and refills from a filer listing that does not yet include the un-flushed create, then markCachedFn publishes the directory authoritatively cached without it. lookupEntry then returns an authoritative ENOENT and ReadDir returns nothing — the file disappears from the mount although the client created it. Under concurrent read+write churn on one directory this is the ConcurrentReadWrite flake. Preserve children the mount flags as local-only (open dirty handle or pending async flush — the signal lookupEntry already trusts) across the rebuild's wipe instead of blind-deleting them. Unpinned stale children are still dropped so a rebuild cannot resurrect a deleted entry. |
||
|
Chris LuandGitHub
|
8d388acc0e |
mount: tolerance-window write pattern detection for concurrent writeback (#9984)
* mount: tolerance-window write pattern detection for concurrent writeback WriterPattern decides whether each dirty chunk is buffered in RAM (NewMemChunk) or spilled to an on-disk swap file (NewSwapFileChunk), so a sequential stream misclassified as random is pushed through disk-backed swap. The old detector compared each write's start to the previous write's exact stop offset (atomic.Swap of lastWriteStopOffset, then lastOffset == offset). That is brittle under concurrent FUSE writeback: MonitorWriteAt runs before the per-handle write lock, so parallel Write upcalls interleave the swap, and writeback flushes dirty pages slightly out of offset order. A genuinely sequential stream then repeatedly reads as random and spills to swap. Replace the exact match with the same frontier + tolerance approach used on the read side: track a never-regressing max frontier (offset+size) via a CAS loop and treat a write whose start is within SeqTolerance (8 MiB) of that frontier as sequential. The existing +/-ModeChangeLimit hysteresis is kept, so a single outlier write can't flip the mode. Adds page_writer_pattern_test.go covering sequentiality, hysteresis, reorder tolerance, the inclusive tolerance boundary, frontier non-regression (the CAS invariant), and recovery back to sequential mode. * mount: read write frontier inside the CAS loop Capture the frontier from the CAS loop's own load rather than a separate up-front snapshot, so the sequentiality diff is judged against the freshest pre-image even if a concurrent writeback upcall advances the frontier while we loop. Also drops the now-redundant load. Behavior is unchanged for the single-threaded tests; addresses review feedback on #9984. |
||
|
Chris LuandGitHub
|
048f9ece2d |
Fix filer metadata-replay OOM under mount reconnect storms (#9901)
* fix(filer): propagate multi-filer metadata log read errors A genuine (non not-found) read error in one filer's log stream was logged and skipped, then the merged cursor advanced past the gap, silently dropping that file's events. Abort the whole replay so the subscriber re-reads from the unchanged position; chunk-not-found still skips. * perf(mount): read persisted metadata log chunks directly from volume servers Set LogFileReaderFn so the filer returns log file references and the mount reads the chunk data itself, instead of the filer reading, decoding, and streaming every persisted entry. Keeps a reconnect storm of many mounts from concentrating hundreds of concurrent log replays in filer memory. * perf(filer): pre-size chunk stream reader buffer to view size The chunk size is known up front, so grow the buffer once instead of letting bytes.Buffer double as the streamed pieces arrive (which transiently overshoots to ~2x per reader). * fix(filer): bound concurrent persisted-log replays Each server-side replay holds an open chunk reader per source filer plus a readahead buffer, so a reconnect storm of clients that predate the metadata-chunks offload multiplies into many GB. Gate replays with a semaphore; abort the acquire when the subscriber's stream is gone so cancelled clients do not pile up parked goroutines. |
||
|
Chris LuandGitHub
|
b5a952bcb1 |
fix(mount): don't strand a directory cached-but-empty when an eviction races a rebuild (#9791)
* fix(mount): don't strand a directory cached-but-empty when an off-loop wipe races a rebuild Idle eviction, kernel Forget, and the copy-range fallback cleared a directory's cached entries directly, off the metaCache apply loop, after resetting the cached flag in inodeToPath as a separate step. A concurrent rebuild could publish a fresh listing (markCachedFn) in between, so the late DeleteFolderChildren left the directory flagged cached over an empty store. lookupEntry then returns an authoritative ENOENT and ReadDir returns nothing, so every file in the directory disappears from the mount although it is still present on the filer. Route those wipes through a new apply-loop step that resets the flag and wipes the store together, serialized with a build's markCachedFn, and skips a directory while it is building. * fix(mount): route the meta-event retry cleanup through the apply-loop purge The subscription-retry callback wiped the mount root's cached children directly off the apply loop and reset the cache flags as a separate step — the same pattern that can leave a concurrently-rebuilding root cached-but-empty. Invalidate all flags (safe on its own, it never deletes entries) then purge the root's children through the apply loop. |
||
|
Chris LuandGitHub
|
2386fa550a |
grpc: don't tear down the shared master connection on a caller's own timeout (#9775)
A Canceled/DeadlineExceeded from the caller's per-request context was treated like a dead channel: it closed the shared cached ClientConn and cancelled every other in-flight RPC on it with "the client connection is closing". Under a burst of concurrent chunk assigns (e.g. a large S3 multipart upload) one slow assign hitting its 10s attempt timeout could poison the connection for all the rest, cascading into a flood of 500s. Thread the caller's context into shouldInvalidateConnection and only invalidate on Canceled/DeadlineExceeded while that context is still live, which isolates the genuine stale-channel signal (a peer restart behind a k8s Service VIP). To carry the context, add a ctx parameter to the existing WithGrpcClient, WithMasterClient, and WithMasterServerClient; the master assign and volume-lookup paths pass their per-attempt context and every other caller passes context.Background(). |
||
|
Chris LuandGitHub
|
f8caaa4464 |
mount,filer: re-assert POSIX locks via keepalive (ownership migration + restart) (#9668)
* mount: renew POSIX lock leases via keepalive The mount tracks the inode keys it holds locks on and a background loop renews its session lease (KEEP_ALIVE) with each key's owner filer every 5s, within the filer's 15s TTL. A live mount is never reaped; a dead one stops renewing and owners reclaim its locks. Tracking is a superset: holds are added on grant and dropped only on owner release, so a still held lock is never under-renewed. * mount,filer: re-assert held POSIX locks via keepalive The owner filer holds POSIX advisory locks as in-memory soft state, so a key's owner change (ring rebalance) or an owner restart lost or stranded them: the new or restarted owner was blind to existing holders and would double-grant. Make the keepalive carry the mount's held lock ranges per key. The mount mirrors its own granted locks (posixOwn), and each tick re-asserts them to the key's current owner, which rebuilds that session's locks from the assertion — self -healing after a takeover or restart. The owner arbitrates re-asserted locks against other sessions so it never double-grants; a lock that lost a migration race is reported, not forced. A bare keepalive (no ranges) still just renews. |
||
|
Chris LuandGitHub
|
3976264391 |
mount: keep the posix-lock hint until the release RPC succeeds (#9670)
routedReleasePosixOwner dropped the local owner hint before sending RELEASE_POSIX_OWNER, so a transient RPC failure left the lock held on the owner filer with no local record to retry from — stranded until session-lease reaping. Drop the hint only after a successful release; on failure keep it so a later flush retries, with lease reaping as the backstop. |
||
|
Chris LuandGitHub
|
3481f13f54 |
mount: route POSIX advisory locks to the owner filer under -dlm (#9669)
With -dlm, GetLk/SetLk/SetLkw and the flush/release cleanup paths go to the inode's owner filer via the PosixLock RPC instead of the local table, so flock/fcntl are honored across mounts. Advisory locking rides the same switch as whole-file write coordination — and is therefore off under writeback cache, which implies single-writer. Keys are the inode identity (HardLinkId else path); SetLkw is client-side polling with the FUSE cancel channel (no server wait queue); a per-mount session id namespaces owners; a local hint avoids a release RPC on every close. Background unlock/release RPCs are bounded so a stuck filer can't hang close(). |
||
|
Chris LuandGitHub
|
68cae26c0b |
mount: fix SetAttr/GetAttr crash from concurrent chunk append under writebackCache (#9667)
* mount: hold the entry lock while reading chunk size in GetAttr/SetAttr Async upload workers append chunks to an open handle's shared entry under the LockedEntry lock (FileHandle.AddChunks), but GetAttr and SetAttr computed FileSize by iterating entry.Chunks without taking it. A concurrent append that reallocated the backing array tore the slice read and crashed in filer.TotalSize. Surfaces with -writebackCache, where handles stay open and flush asynchronously while metadata ops keep arriving. Take the LockedEntry lock for those reads (and SetAttr's truncate rewrite). * mount: re-read entry under the lock in GetAttr/SetAttr If SetEntry swapped the handle's entry pointer between maybeReadEntry and the lock acquisition, the old pointer is orphaned. Re-read fh.entry.Entry under the lock so SetAttr mutates the live entry instead of losing the update, and GetAttr reports the current one. * mount: cover the truncate path in TestAttrChunkRace Alternate SetAttr between mtime-only and a shrinking size so the test also exercises the entry.Chunks rewrite under fh.entry.Lock, not just the read-side size walk. * mount: snapshot chunks under the entry lock on the read path readFromChunks holds fh.entryLock (excludes SetAttr) but not the LockedEntry lock the async uploader appends under, so IsInRemoteOnly, the FileSize fallback, and the RDMA/peer chunk walks read entry.Chunks while AddChunks reallocated it — the same torn-slice crash as GetAttr/SetAttr. Snapshot size, inline content, and the chunk list under a brief LockedEntry RLock, then hand the snapshot to the RDMA/peer helpers instead of holding the lock across network I/O. The captured slice stays valid: append never mutates the old backing array, and truncate is excluded by the fh.entryLock. |
||
|
Chris LuandGitHub
|
a4415c39aa |
fix(mount): keep periodic metadata flush from dropping concurrent chunk uploads (#9574)
* fix(mount): keep periodic metadata flush from dropping concurrent chunk uploads The periodic flush snapshotted entry.Chunks, then ran CompactFileChunks and MaybeManifestize (the manifest upload is a network round trip) before reassigning entry.Chunks. Async uploaders append freshly uploaded chunks during that window, and the reassignment overwrote them: the data stayed on the volumes but the file lost those chunk references, leaving zero-filled holes on read. Large sequential writes such as cat of two 15 GiB files hit several flush cycles and ended up corrupted. Snapshot the chunk list under the entry lock with a length marker, do the slow compaction and manifestization on the snapshot, then splice the processed prefix back in front of whatever chunks arrived after the snapshot. * mount: drop redundant slice copies in the flush splice processedPrefix is freshly built and the tail sub-slice is consumed immediately under the entry lock, so append straight onto processedPrefix instead of allocating two throwaway copies. |
||
|
Chris LuandGitHub
|
4d04609bb8 |
fix(mount): don't release file handles from FUSE Forget (#9529)
fix(mount): don't release file handles from Forget Forget(nodeid, nlookup) only decrements the kernel inode lookup count. File handle lifecycle belongs to FUSE Open/Release. Driving the FH refcount from Forget coupled two unrelated counters and could tear down a still-live handle if Forget ever raced ahead of Release. Drop the ReleaseByInode call (and the now-unused method). |
||
|
Chris LuandGitHub
|
6d12ebeefe |
fix(mount): fall through to filer when cached dir misses a tracked inode (#9436)
lookupEntry returned ENOENT whenever the metaCache had the parent marked
cached but the child entry was absent. That's only correct when the
kernel has no record of the path either — when inodeToPath still maps
it, the three layers disagree (#9139). Triggers in practice under bursts
of concurrent metadata ops and after delete/rename events from another
mount drop the local entry without clearing the inode mapping; the test
flake fixed in
|
||
|
Chris LuandGitHub
|
194dce27bf |
fix(mount): preserve user-set mtime through async/periodic flush (#9363) (#9370)
* fix(mount): preserve user-set mtime through async/periodic flush (#9363) flushMetadataToFiler and flushFileMetadata both stamped time.Now() onto the entry before sending it to the filer, clobbering any mtime SetAttr had stored from utimes()/touch -m -d. The reproducer hit this ~1s after touch because the writebackCache deferred close from the prior write ran flushMetadataToFiler after the user's utimes call. Flush has no business inventing timestamps. Move the write-time stamp into Write (where it always belonged for POSIX correctness) and let flush persist whatever Write or SetAttr already put on the entry. * test(mount): tighten mtime regression test, drop tautological one - userMtime now has non-zero nanoseconds, so the *Ns assertions catch a regression that would zero the field. - Add CtimeNs assertion (was missing). - Drop TestWriteStampsEntryMtime: it duplicated the implementation it was supposed to test, so a regression in Write would not have failed it. Driving the real Write path needs a full PageWriter, which is out of scope for this fix; TestFlushFileMetadataPreservesUserMtime is the meaningful regression for #9363. |
||
|
Chris LuandGitHub
|
e96190d128 |
fix(mount): skip pressure-eviction of gappy page chunks (#9330) (#9334)
* fix(mount): skip pressure-eviction of gappy page chunks (#9330) A page chunk whose written-interval list has an internal hole was being sealed under buffer-pressure eviction, then SaveContent would emit one volume chunk per maximal adjacent run with no chunk covering the hole; reads then silently zero-fill the gap (filer/stream.go:177-186). On a sequential cp through FUSE, that bakes in-flight 4 KiB writes into split volume chunks and leaves chunk-sized blocks of zeros on the destination. Filter the pressure-driven sealers (SaveDataAt's over-limit path, EvictOneWritableChunk, ProactiveFlush) to only seal chunks whose written intervals form one unbroken run. The flush-on-close path (FlushAll) is unchanged: at close every gap is by definition a sparse-file write that the app legitimately never made. * fix(mount): also gate IsContiguouslyWritten on leading zero-offset Tighten IsContiguouslyWritten to also reject empty lists and lists whose first interval does not start at offset 0. The internal-gap and leading-gap cases are symmetric for pressure-driven sealing: both put in-flight FUSE writeback for the missing range at risk of being baked into split volume chunks. The flush-on-close path is still unfiltered (sparse writes are sealed legitimately at FlushAll). Also align EvictOneWritableChunk's bestBytes initialization with SaveDataAt (start at 0) so an empty chunk is never picked, matching the new semantic. Addresses gemini-code-assist review on PR #9334. * fix(mount): preserve cap-pressure liveness in EvictOneWritableChunk The previous version of this fix had EvictOneWritableChunk return false whenever every dirty chunk was gappy. That broke the accountant's Reserve loop: cond.Wait only wakes on Release, Release only fires on upload completion, and refusing to seal anything means no upload starts — the writer hangs at the -writeBufferSizeMB cap forever. Two-pass selection: prefer the fullest gap-free chunk (issue #9330: this is what protects sequential cp from racing FUSE writeback), fall back to the oldest non-empty writer when nothing is gap-free. Oldest-first maximizes the chance that FUSE writeback for the gap range has already settled. The actual sealing path is unchanged — SaveContent still emits one volume chunk per maximal adjacent run; pages that arrive after the seal land in a fresh MemChunk for the same logicChunkIndex and are sealed in turn, so coverage is reconstructed at read time by readResolvedChunks. Sequential cp at default settings always hits the strict pass (writes arrive contiguous-from-0 within their logicChunkIndex), so the bug-fix behavior is preserved; the fallback only runs under genuinely sparse workloads or under FUSE writeback so backed up that no chunk has settled, where forced progress is preferable to a hung mount. * test(mount): pin ProactiveFlush gap-skip behavior (#9330) Sibling regression test for the ProactiveFlush guard added in this series: same 3-chunk setup as TestEvictOneWritableChunk_SkipsGappyChunks (internal gap, leading gap, contiguous). Verifies ProactiveFlush picks the contiguous chunk when staleness criteria are otherwise satisfied, returns false when only gappy chunks remain (no liveness fallback like EvictOneWritableChunk has — failing here is just a missed optimization), and that filling the holes lets the chunks auto-seal via maybeMoveToSealed. * style(mount): trim verbose comments on #9330 fix |
||
|
Chris LuandGitHub
|
31e5e0dee2 |
fix(mount): keep async flush when LockOwner has no POSIX locks (#9300)
FlushIn.LockOwner is populated by the kernel for any fd that may have participated in locking, not only when locks were actually taken. The previous Flush logic treated any non-zero LockOwner as a closing lock holder and forced a synchronous flush, which silently disabled the writebackCache async-flush path (introduced in #8727) for most ordinary close() calls. Consult the POSIX lock table before forcing sync: only owners that currently hold a non-flock byte-range lock need the synchronous path to coordinate with blocked SetLkw waiters. Other closes go async as intended. |
||
|
Chris LuandGitHub
|
7a461ffc2f |
fix(mount): copy xattr value bytes to avoid FUSE buffer aliasing (#9278)
fix(mount): copy xattr value bytes to avoid FUSE buffer aliasing (#9275) SetXAttr stored the caller-supplied `data` slice directly into entry.Extended. That slice aliases go-fuse's per-request input buffer, which is returned to a pool the moment the handler returns. When a file is open during setxattr (the open-fh path defers persistence to flush), the next FUSE request recycles the buffer and silently overwrites the stored xattr bytes; flushMetadataToFiler then ships the corrupted bytes to the filer. `cp -a` reproduces this because it issues a setxattr while holding an open fh, then continues to issue follow-up FUSE ops that reuse the same buffer. The path-based setxattr (e.g. setfattr without an open fh) saves synchronously inside the same handler, so the bytes were marshalled before the buffer could be reused — that is why the source file in the report looked fine and only the cp -a destination was garbage. Defensively copy the bytes when storing them, and add a unit test that mutates the caller buffer after SetXAttr returns to lock in the invariant. |
||
|
Chris Lu
|
6cbcdf488c |
chore(mount,fuse-test): diagnostics for FUSE ConcurrentReadWrite ENOENT flake
PR #9230 attempt 1 hit an intermittent TestConcurrentFileOperations/ConcurrentReadWrite failure where stat returned ENOENT for a path all writers had just succeeded against, and the captured mount.log carried no signal about which layer dropped the entry because the relevant lookup logged at V(4). Two diagnostic-only changes (no behavior change on the happy path): - weed/mount/weedfs.go: in lookupEntry, when filer GetEntry returns ErrNotFound for a path whose inode is still tracked locally with no in-flight create or flush, log Warningf with inode + dirtyHandle + pendingFlush + localCache + dirCached. This surfaces layer-by-layer state at the moment of the suspicious ENOENT. - test/fuse_integration/framework_test.go: on AssertFileExists failure, dump five 100ms-spaced stat retries, a parent ReadDir, and a direct O_RDONLY open before failing. Triangulates kernel dentry caching vs mount lookup vs filer state. |
||
|
Chris LuandGitHub
|
da2e90aefd |
fix(mount): sanitize non-UTF-8 filenames; keep marshal errors per-request (#9207)
* fix(mount): sanitize non-UTF-8 filenames; keep marshal errors per-request (#9139) A single file with invalid-UTF-8 bytes in its name (e.g. a GNOME Trash "partial" like \x10\x98=\\\x8a\x7f.trashinfo.9a51454f.partial) made every FUSE-initiated filer RPC fail with: rpc error: code = Internal desc = grpc: error while marshaling: string field contains invalid UTF-8 and then produced an avalanche of "connection is closing" errors on unrelated LookupEntry / ReadDirAll / UpdateEntry calls, causing the volume-server QPS dips reported in #9139. Root cause is twofold: 1. Proto3 `string` fields require valid UTF-8, but the FUSE kernel passes raw name bytes. Create/Mknod/Mkdir/Unlink/Rmdir/Rename/Lookup/Link/ Symlink all forwarded those bytes directly into CreateEntryRequest.Name, DeleteEntryRequest.Name, StreamRenameEntryRequest.{Old,New}Name and Entry.Name. saveDataAsChunk also copied the FullPath into AssignVolumeRequest.Path unchecked. 2. When the marshal failed, shouldInvalidateConnection treated the resulting codes.Internal as a connection problem and dropped the shared cached ClientConn — canceling every other in-flight RPC on it. Fix: - Add sanitizeFuseName (strings.ToValidUTF8 with '?' replacement, matching util.FullPath.DirAndName) and make checkName return the sanitized name. Apply at every FUSE entry point that passes a name to the filer RPC, including Unlink/Rmdir (which did not previously call checkName) and both oldName/newName in Rename. Add a backstop scrub for AssignVolumeRequest.Path so async flush paths cannot reintroduce invalid bytes from a pre-sanitization cached FullPath. - In weed/pb.shouldInvalidateConnection, detect client-side marshal errors via the gRPC library's "error while marshaling" prefix and return false: the connection is healthy, only the request is bad. Refs: https://github.com/seaweedfs/seaweedfs/issues/9139#issuecomment-4301184231 * fix(mount,util): use '_' for invalid-UTF-8 replacement (URL-safe) Sanitized filenames flow downstream into HTTP URLs (volume-server uploads, filer HTTP API, S3/WebDAV gateways). '?' is the URL query-string delimiter and would split the path the first time the name lands in one, so swap every invalid-UTF-8 replacement to '_'. This covers the two pre-existing sites in weed/util/fullpath.go as well, keeping all paths sanitized the same way. * refactor(pb): detect client-side marshal errors via errors.As, not substring Replace the raw `strings.Contains(err.Error(), ...)` check with a type-based carve-out: use errors.As against the `GRPCStatus() *Status` interface to pull the original Status out of any fmt.Errorf("...: %w") wrapping, then match the library-owned "grpc:" prefix on that Status's Message. Why not errors.Is against a proto-level sentinel: gRPC's encode() collapses the inner proto error with "%v" (stringification) before wrapping it in a Status, so the original error type does not survive into the caller. The Status itself is the structural signal that does survive. Why not status.FromError: when the caller wraps the Status error with fmt.Errorf("...: %w", ...), status.FromError rewrites Status.Message with the full err.Error() of the outermost wrapper, which defeats a prefix check on the library-owned message. errors.As gives us the original Status whose Message is still verbatim from the gRPC library. A new test asserts that a plain errors.New("grpc: error while marshaling: …") — i.e. the same text attached to something that is NOT a gRPC status — does not short-circuit invalidation, so we never silently keep a cached connection alive based on a coincidental substring match. * refactor(util): centralize UTF-8 sanitization; add FullPath.Sanitized Addresses review feedback on PR #9207. Nitpick: every invalid-UTF-8 replacement across the codebase (DirAndName, Name, mount.sanitizeFuseName, the weedfs_write.go backstop) now goes through a single util.SanitizeUTF8Name helper, so the replacement char ('_' — URL-safe) is chosen in one place. Outside-diff: three proto fields took raw FullPath strings that could break marshaling if an entry ever carried invalid UTF-8 (CreateEntryRequest.Directory in Mkdir, DeleteEntryRequest.Directory in Unlink, AssignVolumeRequest.Path in command_fs_merge_volumes). The reviewer's suggested fix — using DirAndName() — would have silently changed Directory from parent to grandparent, because DirAndName sanitizes only the trailing component. Added FullPath.Sanitized(), which scrubs every component, and applied it at the three sites. Exposure is narrow in practice (FUSE-boundary sanitization and the gRPC-side isClientSideMarshalError carve-out already cover the #9139 cascade), but the defense-in-depth is cheap and consistent with the existing AssignVolume backstop. New tests in weed/util/fullpath_test.go document: - SanitizeUTF8Name: valid UTF-8 passes through unchanged; invalid bytes become '_' (not '?', which is URL-special). - FullPath.Sanitized: scrubs bytes in any component, not just the last. - FullPath.DirAndName: dir remains raw on purpose — callers needing a clean full path must use Sanitized(). The test pins this behavior so it is not accidentally "fixed" in a way that changes the (dir, name) semantics callers depend on. |
||
|
Chris Lu
|
b94ad82472 |
fix(test): stabilize ConcurrentLockContention; warn on coherence drift
TestPosixFileLocking/ConcurrentLockContention failed in CI (run 24857323067) with ENOENT when re-opening the file after all 8 workers had successfully written and closed. The 20s openWithRetry budget was exhausted, pointing at a real but unproven metaCache/parent-cache coherence issue in the mount under bursts of concurrent Release. Test: hold the initial fd open for the whole subtest; use it for the post-workers Sync() and the verification read. Workers still exercise the concurrent-flock invariant and per-record write correctness; the re-open path is no longer load-bearing. On Eventually failure, dump ReadDir of the parent, Stat, and a fresh O_RDONLY open so a future recurrence has state to debug from. Drop the darwin-only ENOENT t.Skip branches that hid this same flake. Mount: in weedfs.lookupEntry, when returning ENOENT from the "parent cached but child missing" branch, log at Warningf instead of V(4) when the kernel is still tracking this path's inode. That combination is the smoking-gun signal for cache drift and is rare enough in normal use not to spam the log. |
||
|
Chris LuandGitHub
|
06ccd0e9fc |
fix(mount): flush dirty handles on Release when kernel skipped Flush (#9165)
* fix(mount): flush dirty handles on Release when kernel skipped Flush The FUSE protocol allows the kernel to send Release without a preceding Flush; file handles that reach Release with dirtyMetadata=true (notably deferred creates that never saw any write) would then have their pending filer CreateEntry dropped on the floor, leaving the mount and filer out of sync. Detect dirty handles in Release and call doFlush before tearing the handle down. Skip the fallback when an async flush is already pending so we don't double-submit. Flock-unlock Releases stay on the synchronous path so close()-time serialization is preserved. Adds TestReleaseFlushesDirtyCreateIfFlushWasSkipped covering the create-without-flush path. * address review: drop racy dirty-flag peek, let doFlush self-gate fh.dirtyMetadata / fh.asyncFlushPending are written from the periodic metadata flusher and async flush worker under fhLockTable, so the unsynchronized read in Release was a data race per the reviewer. Just call doFlush unconditionally on every Release; it already fast- paths the clean case (dirtyPages.FlushData early-returns when hasWrites is false, and the dirty-metadata branch short-circuits), so the extra call after a normal Flush is cheap while the no-Flush-before-Release path still recovers a deferred create. |
||
|
Chris LuandGitHub
|
f1d5f31a93 |
fix(mount): retry saveEntry on transient filer errors; stop mismapping Canceled to EIO (#9141)
* fix(mount): retry saveEntry on transient filer errors, stop mismapping Canceled to EIO When the mount's gRPC connection to the filer flaps (e.g. a transient restart or network blip), every in-flight setattr/utimes/chmod/xattr/ rename-driven saveEntry returns "code = Canceled desc = grpc: the client connection is closing" at the same instant. Two bugs in saveEntry then turned each of those into a hard EIO for the user: 1. The error was wrapped with fmt.Errorf(... %v ...) before being passed to grpcErrorToFuseStatus. %v stringifies the status, so status.FromError could no longer unwrap the gRPC code and the Canceled→ETIMEDOUT branch in the classifier never fired; every Canceled error fell through to the default EIO. 2. saveEntry issued a single streamUpdateEntry call with no retry, unlike doFlush which already wraps its CreateEntry in retryMetadataFlush. One stream flap therefore propagated straight to the FUSE caller instead of being ridden out across the 4-attempt / ~7s backoff window. Wrap the UpdateEntry call in retryMetadataFlush (matching doFlush and completeAsyncFlush) and switch the wrap verb to %w so the classifier can still see the gRPC code. This recovers transient closes silently and, if retries are exhausted, returns ETIMEDOUT instead of EIO. Reported by rclone users in #9139 where a large concurrent copy (hundreds of .partial uploads per filer flap) surfaced as walls of EIOs because each .partial rename's post-setattr hit saveEntry at the worst possible moment. * mount: skip saveEntry retries on permanent filer errors Address gemini-code-assist review on #9141: blindly retrying every UpdateEntry failure with exponential backoff means interactive FUSE ops like chmod/utimes/xattr can hang for ~7s before surfacing clearly permanent errors (NotFound, PermissionDenied, InvalidArgument, etc.). Introduce retryMetadataFlushIf, a variant of retryMetadataFlush that accepts a shouldRetry predicate, and an isRetryableFilerError classifier that short-circuits on a conservative whitelist of terminal gRPC codes. Transient errors (Canceled / Unavailable / DeadlineExceeded / ResourceExhausted / Internal) and non-gRPC errors still retry, so the original fix for #9139 (rclone EIO burst during filer connection flaps) is preserved. |
||
|
Chris LuandGitHub
|
a8ba9d106e |
peer chunk sharing 7/8: tryPeerRead read-path hook (#9136)
* mount: batched announcer + pooled peer conns for mount-to-mount RPCs * peer_announcer.go: non-blocking EnqueueAnnounce + ticker flush that groups fids by HRW owner, fans out one ChunkAnnounce per owner in parallel. announcedAt is pruned at 2× TTL so it stays bounded. * peer_dialer.go: PeerConnPool caches one grpc.ClientConn per peer address; the announcer and (next PR) the fetcher share it so steady-state owner RPCs skip the handshake cost entirely. Bounded at 4096 cached entries; shutdown conns are transparently replaced. * WFS starts both alongside the gRPC server; stops them on unmount. * mount: wire tryPeerRead via FetchChunk streaming gRPC Replaces the HTTP GET byte-transfer path with a gRPC server-stream FetchChunk call. Same fall-through semantics: any failure drops through to entryChunkGroup.ReadDataAt, so reads never slow below status quo. * peer_fetcher.go: tryPeerRead resolves the offset to a leaf chunk (flattening manifests), asks the HRW owner for holders via ChunkLookup, then opens FetchChunk on each holder in LRU order (PR #5) until one succeeds. Assembled bytes are verified against FileChunk.ETag end-to-end — the peer is still treated as untrusted. Reuses the shared PeerConnPool from PR #6 for all outbound gRPC. * peer_grpc.go: expose SelfAddr() so the fetcher can avoid dialing itself on a self-owned fid. * filehandle_read.go: tryPeerRead slot between tryRDMARead and entryChunkGroup.ReadDataAt. Gated by option.PeerEnabled and the presence of peerGrpcServer (the single identity test). Read ordering with the feature enabled is now: local cache -> RDMA sidecar -> peer mount (gRPC stream) -> volume server One port, one identity, one connection pool — no more HTTP bytecast. * test(fuse_p2p): end-to-end CI test for peer chunk sharing Adds a FUSE-backed integration test that proves mount B can satisfy a read from mount A's chunk cache instead of the volume tier. Layout (modelled on test/fuse_dlm): test/fuse_p2p/framework_test.go — cluster harness (1 master, 1 volume, 1 filer, N mounts, all with -peer.enable) test/fuse_p2p/peer_chunk_sharing_test.go — writer-reader scenario The test (TestPeerChunkSharing_ReadersPullFromPeerCache): 1. Starts 3 mounts. Three is the sweet spot: with 2 mounts, HRW owner of a chunk is self ~50 % of the time (peer path short-circuits); with 3+ it drops to ≤ 1/3, so a multi-chunk file almost certainly exercises the remote-owner fan-out. 2. Mount 0 writes a ~8 MiB file, then reads it back through its own FUSE to warm its chunk cache. 3. Waits for seed convergence (one full MountList refresh) plus an announcer flush cycle, so chunk-holder entries have reached each HRW owner. 4. Mount 1 reads the same file. 5. Verifies byte-for-byte equality AND greps mount 1's log for "peer read successful" — content matching alone is not proof (the volume fallback would also succeed), so the log marker is what distinguishes p2p from fallback. Workflow .github/workflows/fuse-p2p-integration.yml triggers on any change to mount/filer peer code, the p2p protos, or the test itself. Failure artifacts (server + mount logs) are uploaded for 3 days. Mounts run with -v=4 so the tryPeerRead success/failure glog messages land in the log file the test greps. |
||
|
Chris LuandGitHub
|
73f10fa528 |
peer chunk sharing 6/8: announce queue + batched flush (#9135)
mount: batched announcer + pooled peer conns for mount-to-mount RPCs * peer_announcer.go: non-blocking EnqueueAnnounce + ticker flush that groups fids by HRW owner, fans out one ChunkAnnounce per owner in parallel. announcedAt is pruned at 2× TTL so it stays bounded. * peer_dialer.go: PeerConnPool caches one grpc.ClientConn per peer address; the announcer and (next PR) the fetcher share it so steady-state owner RPCs skip the handshake cost entirely. Bounded at 4096 cached entries; shutdown conns are transparently replaced. * WFS starts both alongside the gRPC server; stops them on unmount. |
||
|
Chris LuandGitHub
|
fe9ca35bbd |
peer chunk sharing 5/8: mount chunk-directory shard (#9134)
mount: tier-2 chunk directory + FetchChunk streaming on one gRPC port Collapses the old two-port design (HTTP peer-serve + separate gRPC directory) into a single gRPC service that handles every mount-to- mount exchange: ChunkAnnounce, ChunkLookup, and the new FetchChunk byte stream. * peer_directory.go: fid -> holders shard, HRW-gated; returns holders in LRU order; capacity-bounded; Sweep handles eviction under write-lock while Lookup runs under RLock (hot path is concurrent). * peer_grpc.go: single MountPeer gRPC server implementing all three RPCs. FetchChunk frames bytes at 1 MiB per Send so the default 4 MiB message cap does not constrain chunk size; cache miss returns gRPC NOT_FOUND so clients distinguish miss from transport error. Reuses pb.NewGrpcServer for consistent keepalive + msg-size tuning. * peer_bytepool.go: sync.Pool wrapper around *[]byte that the server uses to avoid a fresh 8 MiB allocation per FetchChunk call. * WFS wiring starts the gRPC server on option.PeerListen (the single peer port) using the advertise address resolved in PR #3 as the HRW identity. A background sweeper evicts expired directory entries every 60 s. |
||
|
Chris LuandGitHub
|
8a6348d3e9 |
peer chunk sharing 4/8: mount registrar + HRW owner selection (#9133)
* proto: define MountRegister/MountList and MountPeer service Adds the wire types for peer chunk sharing between weed mount clients: * filer.proto: MountRegister / MountList RPCs so each mount can heartbeat its peer-serve address into a filer-hosted registry, and refresh the list of peers. Tiny payload; the filer stores only O(fleet_size) state. * mount_peer.proto (new): ChunkAnnounce / ChunkLookup RPCs for the mount-to-mount chunk directory. Each fid's directory entry lives on an HRW-assigned mount; announces and lookups route to that mount. No behavior yet — later PRs wire the RPCs into the filer and mount. See design-weed-mount-peer-chunk-sharing.md for the full design. * filer: add mount-server registry behind -peer.registry.enable Implements tier 1 of the peer chunk sharing design: an in-memory registry of live weed mount servers, keyed by peer address, refreshed by MountRegister heartbeats and served by MountList. * weed/filer/peer_registry.go: thread-safe map with TTL eviction; lazy sweep on List plus a background sweeper goroutine for bounded memory. * weed/server/filer_grpc_server_peer.go: MountRegister / MountList RPC handlers. When -peer.registry.enable is false (the default), both RPCs are silent no-ops so probing older filers is harmless. * -peer.registry.enable flag on weed filer; FilerOption.PeerRegistryEnabled wires it through. Phase 1 is single-filer (no cross-filer replication of the registry); mounts that fail over to another filer will re-register on the next heartbeat, so the registry self-heals within one TTL cycle. Part of the peer-chunk-sharing design; no behavior change at runtime until a later PR enables the flag on both filer and mount. * filer: nil-safe peerRegistryEnable + registry hardening Addresses review feedback on PR #9131. * Fix: nil pointer deref in the mini cluster. FilerOptions instances constructed outside weed/command/filer.go (e.g. miniFilerOptions in mini.go) do not populate peerRegistryEnable, so dereferencing the pointer panics at Filer startup. Use the same `nil && deref` idiom already used for distributedLock / writebackCache. * Hardening (gemini review): registry now enforces three invariants: - empty peer_addr is silently rejected (no client-controlled sentinel mass-inserts) - TTL is capped at 1 hour so a runaway client cannot pin entries - new-entry count is capped at 10000 to bound memory; renewals of existing entries are always honored, so a full registry still heartbeats its existing members correctly Covered by new unit tests. * filer: rename -peer.registry.enable flag to -mount.p2p Per review feedback: the old name "peer.registry.enable" leaked the implementation ("registry") into the CLI surface. "mount.p2p" is shorter and describes what it actually controls — whether this filer participates in mount-to-mount peer chunk sharing. Flag renames (all three keep default=true, idle cost is near-zero): -peer.registry.enable -> -mount.p2p (weed filer) -filer.peer.registry.enable -> -filer.mount.p2p (weed mini, weed server) Internal variable names (mountPeerRegistryEnable, MountPeerRegistry) keep their longer form — they describe the component, not the knob. * filer: MountList returns DataCenter + List uses RLock Two review follow-ups on the mount peer registry: * weed/server/filer_grpc_server_mount_peer.go: MountList was dropping the DataCenter on the wire. The whole point of carrying DC separately from Rack is letting the mount-side fetcher re-rank peers by the two-level locality hierarchy (same-rack > same-DC > cross-DC); without DC in the response every remote peer collapsed to "unknown locality." * weed/filer/mount_peer_registry.go: List() was taking a write lock so it could lazy-delete expired entries inline. But MountList is a read-heavy RPC hit on every mount's 30 s refresh loop, and Sweep is already wired as the sole reclamation path (same pattern as the mount-side PeerDirectory). Switch List to RLock + filter, let Sweep do the map mutation, so concurrent MountList callers don't serialize on each other. Test updated to reflect the new contract (List no longer mutates the map; Sweep is what drops expired entries). * mount: add peer chunk sharing options + advertise address resolver First cut at the peer chunk sharing wiring on the mount side. No functional behavior yet — this PR just introduces the option fields, the -peer.* flags, and the helper that resolves a reachable host:port from them. The server implementation arrives in PR #5 (gRPC service) and the fetcher in PR #7. * ResolvePeerAdvertiseAddr: an explicit -peer.advertise wins; else we use -peer.listen's bind host if specific; else util.DetectedHostAddress combined with the port. This is what gets registered with the filer and announced to peers, so wildcard binds no longer result in unreachable identities like "[::]:18080". * Option fields: PeerEnabled, PeerListen, PeerAdvertise, PeerRack. One port handles both directory RPCs and streaming chunk fetches (see PR #1 FetchChunk proto), so there is no second -peer.grpc.* flag — the old HTTP byte-transfer path is gone. * New flags on weed mount: -peer.enable, -peer.listen (default :18080), -peer.advertise (default auto), -peer.rack. * mount: register with filer and maintain HRW seed view Adds the mount-side tier-1 client. On startup the mount calls MountRegister with its advertise address (PR #3) and keeps both the filer entry and the local seed view fresh via background tickers (30 s register / 30 s list, 90 s filer TTL). * peer_hrw.go: pure rendezvous-hashing helper picking a single owner per fid via top-1 HRW. Adding or removing one seed moves only ~1/N fids. * peer_registrar.go: heartbeat + list poller. Seeds() returns the slice directly (no per-call copy) since listOnce atomically swaps; background RPCs bind their context to Stop() so unmount doesn't hang on a slow filer. * WFS wiring uses ResolvePeerAdvertiseAddr from PR #3 for the identity registered with the filer. No HTTP server, no second port — one reachable address represents the mount. * mount: broadcast MountRegister/MountList to every filer Previously the registrar called through wfs.WithFilerClient, which only reaches whichever filer the WFS filer-client session happens to be on. That meant two mounts pointing at different filers would never see each other: the filer mount registries are in-memory and per-filer (no filer-to-filer sync), so each mount's MountList only returned peers that had also registered through the same filer. This commit makes the registrar multi-filer aware: * NewPeerRegistrar now takes the full FilerAddresses slice and a per-filer dial function. The old single-filer peerFilerClient interface is gone. * registerOnce fans a MountRegister RPC out to every filer in parallel. Succeeds if at least one filer accepted — an unreachable filer is tolerated, logged, and retried on the next heartbeat. * listOnce polls every filer's MountList in parallel and merges the responses by peer_addr, keeping the newest LastSeenNs on duplicates. Mounts talking to different filers therefore converge once every filer has been polled once. The merged-list property is what lets a fleet of mounts spread across multiple filers still form a single HRW seed view. Each filer only ever sees the subset of mounts that heartbeat through it, but the registrar reconstructs the union client-side. New unit tests guard both properties: - RegisterBroadcastsToAllFilers: one registerOnce hits all N filers. - ListMergesAcrossFilers: mount-a on filer-1 and mount-b on filer-2 both appear in the merged seed set. - ListMergeKeepsNewestLastSeen: the same mount reported by two filers collapses to one entry with the freshest timestamp. |
||
|
Chris LuandGitHub
|
af1e571297 |
peer chunk sharing 3/8: mount peer-serve HTTP endpoint (#9132)
* proto: define MountRegister/MountList and MountPeer service Adds the wire types for peer chunk sharing between weed mount clients: * filer.proto: MountRegister / MountList RPCs so each mount can heartbeat its peer-serve address into a filer-hosted registry, and refresh the list of peers. Tiny payload; the filer stores only O(fleet_size) state. * mount_peer.proto (new): ChunkAnnounce / ChunkLookup RPCs for the mount-to-mount chunk directory. Each fid's directory entry lives on an HRW-assigned mount; announces and lookups route to that mount. No behavior yet — later PRs wire the RPCs into the filer and mount. See design-weed-mount-peer-chunk-sharing.md for the full design. * filer: add mount-server registry behind -peer.registry.enable Implements tier 1 of the peer chunk sharing design: an in-memory registry of live weed mount servers, keyed by peer address, refreshed by MountRegister heartbeats and served by MountList. * weed/filer/peer_registry.go: thread-safe map with TTL eviction; lazy sweep on List plus a background sweeper goroutine for bounded memory. * weed/server/filer_grpc_server_peer.go: MountRegister / MountList RPC handlers. When -peer.registry.enable is false (the default), both RPCs are silent no-ops so probing older filers is harmless. * -peer.registry.enable flag on weed filer; FilerOption.PeerRegistryEnabled wires it through. Phase 1 is single-filer (no cross-filer replication of the registry); mounts that fail over to another filer will re-register on the next heartbeat, so the registry self-heals within one TTL cycle. Part of the peer-chunk-sharing design; no behavior change at runtime until a later PR enables the flag on both filer and mount. * filer: nil-safe peerRegistryEnable + registry hardening Addresses review feedback on PR #9131. * Fix: nil pointer deref in the mini cluster. FilerOptions instances constructed outside weed/command/filer.go (e.g. miniFilerOptions in mini.go) do not populate peerRegistryEnable, so dereferencing the pointer panics at Filer startup. Use the same `nil && deref` idiom already used for distributedLock / writebackCache. * Hardening (gemini review): registry now enforces three invariants: - empty peer_addr is silently rejected (no client-controlled sentinel mass-inserts) - TTL is capped at 1 hour so a runaway client cannot pin entries - new-entry count is capped at 10000 to bound memory; renewals of existing entries are always honored, so a full registry still heartbeats its existing members correctly Covered by new unit tests. * filer: rename -peer.registry.enable flag to -mount.p2p Per review feedback: the old name "peer.registry.enable" leaked the implementation ("registry") into the CLI surface. "mount.p2p" is shorter and describes what it actually controls — whether this filer participates in mount-to-mount peer chunk sharing. Flag renames (all three keep default=true, idle cost is near-zero): -peer.registry.enable -> -mount.p2p (weed filer) -filer.peer.registry.enable -> -filer.mount.p2p (weed mini, weed server) Internal variable names (mountPeerRegistryEnable, MountPeerRegistry) keep their longer form — they describe the component, not the knob. * filer: MountList returns DataCenter + List uses RLock Two review follow-ups on the mount peer registry: * weed/server/filer_grpc_server_mount_peer.go: MountList was dropping the DataCenter on the wire. The whole point of carrying DC separately from Rack is letting the mount-side fetcher re-rank peers by the two-level locality hierarchy (same-rack > same-DC > cross-DC); without DC in the response every remote peer collapsed to "unknown locality." * weed/filer/mount_peer_registry.go: List() was taking a write lock so it could lazy-delete expired entries inline. But MountList is a read-heavy RPC hit on every mount's 30 s refresh loop, and Sweep is already wired as the sole reclamation path (same pattern as the mount-side PeerDirectory). Switch List to RLock + filter, let Sweep do the map mutation, so concurrent MountList callers don't serialize on each other. Test updated to reflect the new contract (List no longer mutates the map; Sweep is what drops expired entries). * mount: add peer chunk sharing options + advertise address resolver First cut at the peer chunk sharing wiring on the mount side. No functional behavior yet — this PR just introduces the option fields, the -peer.* flags, and the helper that resolves a reachable host:port from them. The server implementation arrives in PR #5 (gRPC service) and the fetcher in PR #7. * ResolvePeerAdvertiseAddr: an explicit -peer.advertise wins; else we use -peer.listen's bind host if specific; else util.DetectedHostAddress combined with the port. This is what gets registered with the filer and announced to peers, so wildcard binds no longer result in unreachable identities like "[::]:18080". * Option fields: PeerEnabled, PeerListen, PeerAdvertise, PeerRack. One port handles both directory RPCs and streaming chunk fetches (see PR #1 FetchChunk proto), so there is no second -peer.grpc.* flag — the old HTTP byte-transfer path is gone. * New flags on weed mount: -peer.enable, -peer.listen (default :18080), -peer.advertise (default auto), -peer.rack. |
||
|
Chris LuandGitHub
|
1c130c2d47 |
fix(mount): close inodeLocks cleanup race that let two flock holders coexist (#9128)
* fix(mount): close inodeLocks cleanup race that allowed two flock holders
PosixLockTable.getOrCreateInodeLocks released plt.mu before the caller
acquired il.mu. A concurrent maybeCleanupInode could delete the map
entry in that window; the first caller would then insert its lock into
the orphaned inodeLocks while a later caller created a fresh entry in
the map, so findConflict never observed the orphaned lock and two
owners could simultaneously believe they held the same exclusive flock.
This matches the flaky CI failure seen in
TestPosixFileLocking/ConcurrentLockContention:
Error: Should be empty, but was [worker N: flock overlap detected with 2 holders]
Mark removed inodeLocks as dead under plt.mu+il.mu, and have SetLk /
SetLkw recheck the flag after locking il.mu, refetching the live entry
from the map when orphaned. Also delete the map entry only if it still
points to this il, so a racing recreate is not clobbered.
Adds TestConcurrentFlockChurnPreservesMutualExclusion: 16 goroutines x
500 flock/unflock iterations on one inode. Reliably reports 500+
overlaps per run before the fix; clean across 100 race-enabled runs
after.
* fix(mount): extend dead-flag contract to GetLk and self-heal primitives
Address review feedback on the initial cleanup-race fix:
1. GetLk had the same stale-pointer bug as SetLk. A caller could grab
an inodeLocks pointer, have cleanup orphan it and a replacement il
receive a conflicting lock, then answer F_UNLCK off the empty dead
pointer. Add the same dead recheck + refetch loop.
2. getOrCreateInodeLocks and getInodeLocks now treat a dead map entry
as defective: the former replaces it with a fresh inodeLocks, the
latter drops it and returns nil. Production cannot reach that state
(maybeCleanupInode atomically deletes under plt.mu when it sets
dead), but the hardening guarantees the SetLk / SetLkw / GetLk
retry loops always make progress even if a future refactor reorders
those operations, and it lets the white-box tests set up a stale
dead entry without spinning.
3. Strengthen the regression suite:
- TestSetLkRetriesPastDeadInodeLocks: deterministic white-box test
that installs a dead il in the map and asserts SetLk routes the
new lock into a fresh il (not the orphan), that GetLk reports the
resulting conflict, and that a different-owner acquire is rejected
with EAGAIN.
- TestGetInodeLocksEvictsDeadEntry: verifies both map-read primitives
drop or replace dead entries.
- TestConcurrentFlockChurnPreservesMutualExclusion: replace the
timing-fragile Add(1)-and-check counter with a Swap+CAS detector.
Each worker claims a slot after SetLk OK and releases it before
UN, flagging both an observed predecessor and a lost CAS on
release. Against a reverted fix the detector fires 1000+ times per
run; with the fix clean across 100 race-enabled iterations.
* test(mount): fail fast on unexpected SetLk statuses in churn loop
The stress test blindly spun on any non-OK SetLk status and discarded
the unlock return. If SetLk ever returns something other than OK or
EAGAIN (e.g. after a future refactor introduces a new error), the
acquire loop would spin forever and an unlock failure would be
silently swallowed.
Capture the acquire status, retry only on the expected EAGAIN, and
assert unlock returns OK. Use t.Errorf + return (not t.Fatalf) because
the checks run on worker goroutines where FailNow is unsafe. The
Swap+CAS overlap detector is unchanged.
|
||
|
Chris LuandGitHub
|
00a2e22478 |
fix(mount): remove fid pool to stop master over-allocating volumes (#9111)
* fix(mount): remove fid pool to stop master over-allocating volumes
The writeback-cache fid pool pre-allocated file IDs with
ExpectedDataSize = ChunkSizeLimit (typically 8+ MB). The master's
PickForWrite charges count * expectedDataSize against the volume's
effectiveSize, so a full pool refill could charge hundreds of MB
against a single volume before any bytes were actually written.
That tripped RecordAssign's hard-limit path and eagerly removed
volumes from writable, causing the master to grow new volumes
even when the real data being written was tiny.
Drop the pool entirely. Every chunk upload goes through
UploadWithRetry -> AssignVolume with no ExpectedDataSize hint,
letting the master fall back to the 1 MB default estimate. The
mount->filer grpc connection is already cached in pb.WithGrpcClient
(non-streaming mode), so per-chunk AssignVolume is a unary RPC
over an existing HTTP/2 stream, not a full dial. Path-based
filer.conf storage rules now apply to mount chunk assigns again,
which the pool had to skip.
Also remove the now-unused operation.UploadWithAssignFunc and its
AssignFunc type.
* fix(upload): populate ExpectedDataSize from actual chunk bytes
UploadWithRetry already buffers the full chunk into `data` before
calling AssignVolume, so the real size is known. Previously the
assign request went out with ExpectedDataSize=0, making the master
fall back to the 1 MB DefaultNeedleSizeEstimate per fid — same
over-reservation symptom the pool had, just smaller per call.
Stamp ExpectedDataSize = len(data) before the assign RPC when the
caller hasn't already set it. This covers mount chunk uploads,
filer_copy, filersink, mq/logstore, broker_write, gateway_upload,
and nfs — all the UploadWithRetry paths.
* fix(assign): pass real ExpectedDataSize at every assign call site
After removing the mount fid pool, per-chunk AssignVolume calls went
out with ExpectedDataSize=0, making the master fall back to its 1 MB
DefaultNeedleSizeEstimate. That's still an over-estimate for small
writes. Thread the real payload size through every remaining assign
site so RecordAssign charges effectiveSize accurately and stops
prematurely marking volumes full.
- filer: assignNewFileInfo now takes expectedDataSize and stamps it
on both primary and alternate VolumeAssignRequests. Callers pass:
- SSE data-to-chunk: len(data)
- copy manifest save: len(data)
- streamCopyChunk: srcChunk.Size
- TUS sub-chunk: bytes read
- saveAsChunk (autochunk/manifestize): 0 (small, size unknown
until the reader is drained; master uses 1 MB default)
- filer gRPC remote fetch-and-write: ExpectedDataSize = chunkSize
after the adaptive chunkSize is computed.
- ChunkedUploadOption.AssignFunc gains an expectedDataSize parameter;
upload_chunked.go passes the buffered dataSize at the call site.
S3 PUT assignFunc stamps it on the AssignVolumeRequest.
- S3 copy: assignNewVolume / prepareChunkCopy take expectedDataSize;
all seven call sites pass the source chunk's Size.
- operation.SubmitFiles / FilePart.Upload: derive per-fid size from
FileSize (average for batched requests, real per-chunk size for
sequential chunk assigns).
- benchmark: pass fileSize.
- filer append-to-file: pass len(data).
* fix(assign): thread size through SaveDataAsChunkFunctionType
The saveAsChunk path (autochunk, filer_copy, webdav, mount) ran
AssignVolume before the reader was drained, so it had to pass
ExpectedDataSize=0 and fall back to the master's 1 MB default.
Add an expectedDataSize parameter to SaveDataAsChunkFunctionType.
- mergeIntoManifest already has the serialized manifest bytes, so
it passes uint64(len(data)) directly.
- Mount's saveDataAsChunk ignores the parameter because it uses
UploadWithRetry, which already stamps len(data) on the assign
after reading the payload.
- webdav and filer_copy saveDataAsChunk follow the same UploadWithRetry
path and also ignore the hint.
- Filer's saveAsChunk (used for manifestize) plumbs the value to
assignNewFileInfo so manifest-chunk assigns get a real size.
Callers of saveFunc-as-value (weedfs_file_sync, dirty_pages_chunked)
pass the chunk size they're about to upload.
|
||
|
Chris LuandGitHub
|
7916e61c08 |
fix(mount): avoid self-notify deadlock in Link and CopyFileRange handlers (#9110)
The Link and CopyFileRange FUSE request handlers were calling fuseServer.InodeNotify (and EntryNotify for copy) synchronously while the kernel was still waiting for the request's reply on the same /dev/fuse fd. Notifications share that fd, so the syscall.Write can block indefinitely when the kernel hasn't drained its queue yet, hanging the entire mount. A goroutine dump from a stuck mount showed the Link handler blocked in syscall.Write inside InodeNotify while the server's read loop kept waiting for new requests. Drop the synchronous notifies. The local meta cache is still updated inline, so subsequent filesystem ops see the fresh state; the kernel's attr/dentry caches re-fetch once their TTL expires. |
||
|
Chris LuandGitHub
|
9896eade51 |
feat(mount): set FOPEN_KEEP_CACHE on re-open of unchanged files (#9097)
* feat(mount): set FOPEN_KEEP_CACHE when file mtime is unchanged On re-open of an unmodified file, signal the kernel to preserve its existing page cache. This eliminates redundant volume server reads for workloads that repeatedly open-read-close the same files (build systems, config readers, etc.). * fix(mount): use guarded type assertion for openMtimeCache load Use the two-value form of type assertion when loading from sync.Map to prevent potential panics if a non-int64 value is ever stored. * fix(mount): skip redundant mtime store and invalidate on truncation - Avoid redundant sync.Map Store when cached mtime already matches the current mtime, reducing contention on the hot open path. - Invalidate openMtimeCache in SetAttr when file size changes (truncation), preventing stale kernel page cache after ftruncate. * fix(mount): use nanosecond mtime precision and bounded cache for FOPEN_KEEP_CACHE - Compare both Mtime (seconds) and MtimeNs (nanoseconds) to detect sub-second modifications common in automated workloads. - Replace unbounded sync.Map with a bounded map + mutex (8192 entries, random eviction when full), following the existing atimeMap pattern. - Extract applyKeepCacheFlag and invalidateOpenMtimeCache methods for clarity and testability. - Add tests for nanosecond precision and cache eviction. * fix(mount): invalidate mtime cache in truncateEntry for O_TRUNC consistency Add invalidateOpenMtimeCache call to truncateEntry so the Create path with O_TRUNC follows the same explicit invalidation pattern as SetAttr and Write. |
||
|
Chris LuandGitHub
|
216b52c13a |
perf(mount): add graduated write backpressure (#9099)
* perf(mount): add graduated write backpressure before buffer cap Introduce soft (80%) and hard (95%) throttling thresholds in WriteBufferAccountant. When write buffer usage approaches the cap, Reserve() inserts brief sleeps to slow writers gradually rather than blocking them completely at the cap. This smooths out write latency under sustained load. * fix(mount): address review feedback for graduated backpressure - Use projected usage (used + n) for threshold checks so a single reservation crossing a threshold is throttled immediately. - Widen no-throttle test timing tolerance to softThrottleDelay to avoid CI flakes from scheduling jitter. - Replace fragile timing upper-bound in recovery test with counter-based invariant (hardThrottleCount stays at 1). * docs(mount): clarify single-shot throttle design in WriteBufferAccountant Add a comment explaining why graduated throttling runs once per Reserve call rather than inside the blocking loop: once at the cap, the evictor + cond.Wait mechanism frees actual capacity, which time-based sleeps cannot do. |
||
|
Chris LuandGitHub
|
213b6c3107 |
fix(mount): count manifest sizes in merge condition to prevent accumulation (#9101)
* fix(mount): count manifest sizes in merge condition to prevent manifest accumulation shouldMergeChunks only counted non-manifest chunk sizes toward the bloat threshold, so overlapping manifests accumulated undetected across flush cycles. During sustained random writes (e.g. fio), each metadata flush compacts non-manifest chunks and may group them into a new manifest via MaybeManifestize, while carrying forward all existing manifests. Since the merge condition only checked non-manifest totals against 2x file size, the growing pile of redundant manifests never triggered a merge. In a real workload: 4 GB file, 25 manifests each covering ~4 GB (107 GB manifest data on volume servers), but shouldMergeChunks saw only 4.2 GB of non-manifest chunks vs the 8.6 GB threshold -- no merge. Fix: include manifest coverage sizes in totalChunkSize. This correctly detects the 111 GB total vs 8.6 GB threshold and triggers the merge, which re-reads the file as clean chunks and lets the filer's MinusChunks (which resolves manifests) garbage-collect all redundant sub-chunks. * test(mount,filer): add manifest chunk correctness tests Filer-level tests (filechunk_manifest_test.go): - TestManifestRoundTripPreservesChunks: create -> serialize -> deserialize preserves fileId, offset, size, and timestamp for all sub-chunks - TestCompactResolvedOverlappingManifests: two manifests covering the same range, older sub-chunks correctly identified as garbage after compaction - TestDoMinusChunksWithResolvedManifests: old manifest sub-chunks detected as garbage when compared against new clean chunks (mirrors filer cleanup) - TestManifestizeSmallBatchWithRemainder: batch=3 with 7 chunks produces 2 manifests + 1 remainder, all data recoverable after resolve - TestCompactMultipleOverlappingManifestGenerations: 5 generations of full-file manifests, compaction keeps only the newest generation - TestManifestBloatDetection: with N overlapping manifests, merge triggers at N >= 2 (stored > 2x file size) Mount-level test (weedfs_file_sync_test.go): - TestFlushCycleManifestAccumulation: simulates the flushMetadataToFiler pipeline over multiple cycles with a small manifest batch (5 chunks). Verifies merge triggers at cycle 2 when accumulated manifests exceed the 2x threshold. Uses SeparateManifestChunks + CompactFileChunks + shouldMergeChunks to mirror the real code path. |
||
|
Chris LuandGitHub
|
f9df187928 |
fix(mount): reduce chunk fragmentation from random writes (#9096)
* fix(mount): reduce chunk fragmentation from random writes Random writes via FUSE mount create many small partially-overlapping chunks that bloat storage and slow reads. Two complementary fixes: 1. Fill gaps at seal time: when a partial writable chunk is sealed for upload, read existing file data into the unwritten regions so SaveContent uploads one complete chunk instead of many fragments. No overhead for sequential writes (IsComplete short-circuits). 2. Merge on fsync: after CompactFileChunks, if total non-manifest chunk data exceeds 2x the logical file size, re-read and re-upload the entire file as properly-sized chunks. The filer's cleanupChunks garbage-collects all superseded chunks. * revert FillGaps: reading from volume servers during write path is too expensive * test(mount): add tests for chunk merge condition Extract shouldMergeChunks as a testable function and add tests covering: - empty file, single chunk, non-overlapping chunks (no merge) - exactly 2x boundary (no merge) vs just over 2x (merge) - manifest chunks extend file size but don't count toward stored total - input slices are not mutated (safe append) - CompactFileChunks + condition: fully superseded, staggered overlaps, 75% overlap pattern, many 4K writes at 1K step - randomized bloat detection (50 iterations) - visible content preserved after compaction (100 iterations) * fix(mount): cap merge buffer at file size for small files Avoids allocating a full ChunkSizeLimit buffer when the file being merged is smaller. Addresses PR review feedback. |
||
|
Chris LuandGitHub
|
d7865909ba |
feat(mount): proactive flush of idle writable chunks (#9094)
* feat(mount): proactive flush of idle writable chunks Add a background goroutine that periodically scans writable chunks across all open file handles and seals those that are idle and unlikely to receive further writes, submitting them for async upload. A writable chunk is proactively flushed when it has been idle for 500ms AND meets one of: nearly full (>=90%), behind the sequential write frontier by 2+ chunks, or stale for 5+ seconds. Flushing only happens when the upload pipeline has spare capacity (< half of concurrent writer slots in use). This prevents partial chunks from accumulating until fsync/close, which is particularly beneficial for bursty or small-file workloads where chunks may never reach IsComplete(). Also fixes a latent bug in ActivityScore where MarkRead/MarkWrite used value receivers, silently discarding all mutations. * refactor(mount): reuse WriterPattern instead of duplicating sequential detection Remove the isSequential atomic from UploadPipeline. The proactive flusher now reads IsSequentialMode() from the existing WriterPattern on PageWriter and passes it as a parameter to ProactiveFlush. This avoids duplicating the sequential/random detection that WriterPattern already maintains. * fix(mount): address PR review feedback - Make ActivityScore thread-safe using atomics (CAS loop for score updates, atomic load/swap for timestamp). Previously MarkRead was called under RLock while MarkWrite held a write lock, creating a data race on the shared fields. - Fix ProactiveFlush half-capacity guard: use multiplication (uploaderCount*2 >= max) instead of floor division (max/2) which misbehaves for odd or small concurrentWriterMax values. * fix(mount): review fixes for proactive flush - Fix TOCTOU race in lastWriteChunkIndex update: use CAS loop so concurrent writers cannot regress the frontier. - Remove unused UploaderCount() getter. - Reuse the caller-provided tsNs instead of calling time.Now() again in WriteDataAt for lastWriteTsNs, eliminating a redundant syscall per write. |
||
|
Chris LuandGitHub
|
bdebd63f7c |
fix(mount): evict writable chunks when writeBufferSizeMB cap is reached (#9091)
* fix(mount): evict writable chunks when writeBufferSizeMB cap is reached Reported on #8777: fio 4k randwrite with --nrfiles=1000 on a weed mount hangs indefinitely after ~1 second of activity, volume server QPS drops to zero, and the test never makes progress beyond a few hundred writes. A scaled-down local repro (4 jobs x 250 nrfiles x 40 MiB with -chunkSizeLimitMB=32 -writeBufferSizeMB=10240) reproduced the hang exactly: fio stuck in fio_state=SETTING_UP, mount idle, all writer goroutines parked in page_writer.(*WriteBufferAccountant).Reserve with n=0x2000000 (32 MiB). Root cause: UploadPipeline.SaveDataAt reserves a full chunkSize slot against the global WriteBufferAccountant the first time it allocates a writable chunk for a given file. The reservation is released by SealedChunk.FreeReference only after the chunk's async upload completes, and chunks only move from writable to sealed when they fill or when the file is flushed/closed. For 4k random writes with small per-file data and iodepth holding files open, writable chunks never fill and never close, so their slots are pinned forever. Once openFiles * chunkSize exceeds writeBufferSizeMB, every new writer blocks in Reserve's cond.Wait with no possible waker, a hard deadlock. The user-visible symptoms (volume QPS=0, waited 30min, still running) are exactly that state. In the reported config, 8 jobs * 1000 nrfiles * 32 MiB = 256 GB of reservations against a 10 GB cap. Fix: add a SetEvictor hook on WriteBufferAccountant. When Reserve would otherwise block on the cond, it single-flights an evictor callback that walks wfs.fhMap, picks the first open file handle with a writable chunk, and force-seals its fullest writable chunk via a new UploadPipeline.EvictOneWritableChunk. Force-sealing submits the chunk for async upload on the existing uploader pool; the upload's SealedChunk.FreeReference releases the global slot, broadcasts on the accountant cond, and unblocks the waiter. The fullest-first heuristic matches the existing over-limit path in SaveDataAt and keeps us from thrashing on repeatedly re-sealing half-empty chunks. The original #9066 motivation (cap global write-pipeline growth so swap can't be filled while uploads stall) is preserved: Reserve still blocks when the cap is actually exhausted by in-flight uploads, and a failed evictor (no writable chunks to seal) falls through to cond.Wait exactly as before. Verified end-to-end in a privileged debian container running a full master + volume + filer + weed mount stack. Against the reporter's config shrunk to the same memory footprint (4 jobs x 250 nrfiles x 40 MiB, -chunkSizeLimitMB=32 -writeBufferSizeMB=10240): - baseline binary (master): fio stalls after ~320 writes, killed by timeout at 400s with io=1280 KiB and bw=2383 B/s. - patched binary: fio completes in 1.9 s at 82.8 MiB/s, all 40,000 writes issued, 156 MiB written, rc=0. page_writer unit tests still pass. Touches: WriteBufferAccountant (SetEvictor + single-flight in Reserve), UploadPipeline.EvictOneWritableChunk, DirtyPages interface + the two existing implementers (ChunkedDirtyPages, PageWriter), and a new weedfs_write_buffer_evict.go holding the WFS.evictOneWritableChunk callback that walks fhMap. * fix(mount): re-check write-budget cap after evict and make eviction panic-safe Addresses two review concerns on #9091: 1. CodeRabbit flagged that the original Reserve loop only considered the break condition when the evictor reported success: if evicted { if a.used+n <= a.cap || a.used == 0 { break } } a.cond.Wait() A concurrent Release firing during the evictor's execution window (we drop a.mu around the callback so uploader goroutines can drain) would land its Broadcast on an empty waiter set and then be missed, because we unconditionally call cond.Wait() on the same iteration. In practice an in-flight async upload eventually fires another broadcast so this would delay rather than permanently hang — but the logic is wrong either way and should re-check the cap after every evictor round. 2. Gemini flagged that setting `a.evicting = true` and dropping `a.mu` without a deferred unwind leaves the accountant in a broken state if the evictor panics: the flag stays set forever and no Reserve caller can ever run another eviction, even though the mutex gets unlocked by the normal stack unwind. Pull the evictor call into a tiny helper runEvictorLocked whose defer re-acquires a.mu and clears a.evicting regardless of panic status, then broadcasts so other blocked Reservers re-evaluate. Reserve checks the cap condition immediately after the helper returns; only falls through to cond.Wait() when the budget is still exhausted. No behavior change on the happy path; the fix is in the error and race corners. * doc(mount): clarify eviction-scan cost on the blocked-Reserve path Gemini review on #9091 flagged that the fhMap scan inside evictOneWritableChunk is O(open files). Document why the cost is bounded to the Reserve-blocked path and paid at most once per chunkSize drained, rather than on the write hot path, and record the reason we do not preempt with an LRU. |
||
|
Chris LuandGitHub
|
08d9193fe1 |
[nfs] Add NFS (#9067)
* add filer inode foundation for nfs
* nfs command skeleton
* add filer inode index foundation for nfs
* make nfs inode index hardlink aware
* add nfs filehandle and inode lookup plumbing
* add read-only nfs frontend foundation
* add nfs namespace mutation support
* add chunk-backed nfs write path
* add nfs protocol integration tests
* add stale handle nfs coverage
* complete nfs hardlink and failover coverage
* add nfs export access controls
* add nfs metadata cache invalidation
* fix nfs chunk read lookup routing
* fix nfs review findings and rename regression
* address pr 9067 review comments
- filer_inode: fail fast if the snowflake sequencer cannot start, and let
operators override the 10-bit node id via SEAWEEDFS_FILER_SNOWFLAKE_ID
to avoid multi-filer collisions
- filer_inode: drop the redundant retry loop in nextInode
- filerstore_wrapper: treat inode-index writes/removals as best-effort so
a primary store success no longer surfaces as an operation failure
- filer_grpc_server_rename: defer overwritten-target chunk deletion until
after CommitTransaction so a rolled-back rename does not strand live
metadata pointing at freshly deleted chunks
- command/nfs: default ip.bind to loopback and require an explicit
filer.path, so the experimental server does not expose the entire
filer namespace on first run
- nfs integration_test: document why LinkArgs matches go-nfs's on-the-wire
layout rather than RFC 1813 LINK3args
* mount: pre-allocate inode in Mkdir and Symlink
Mkdir and Symlink used to send filer_pb.CreateEntryRequest with
Attributes.Inode = 0. After PR 9067, the filer's CreateEntry now assigns
its own inode in that case, so the filer-side entry ends up with a
different inode than the one the mount allocates via inodeToPath.Lookup
and returns to the kernel. Once applyLocalMetadataEvent stores the
filer's entry in the meta cache, subsequent GetAttr calls read the
cached entry and hit the setAttrByPbEntry override at line 197 of
weedfs_attr.go, returning the filer-assigned inode instead of the
mount's local one. pjdfstest tests/rename/00.t (subtests 81/87/91)
caught this — it lstat'd a freshly-created directory/symlink, renamed
it, lstat'd again, and saw a different inode the second time.
createRegularFile already pre-allocates via inodeToPath.AllocateInode
and stamps it into the create request. Do the same thing in Mkdir and
Symlink so both sides agree on the object identity from the very first
request, and so GetAttr's cache path returns the same value as Mkdir /
Symlink's initial response.
* sequence: mask snowflake node id on int→uint32 conversion
CodeQL flagged the unchecked uint32(snowflakeId) cast in
NewSnowflakeSequencer as a potential truncation bug when snowflakeId is
sourced from user input (e.g. via SEAWEEDFS_FILER_SNOWFLAKE_ID). Mask
to the 10 bits the snowflake library actually uses so any caller-
supplied int is safely clamped into range.
* add test/nfs integration suite
Boots a real SeaweedFS cluster (master + volume + filer) plus the
experimental `weed nfs` frontend as subprocesses and drives it through
the NFSv3 wire protocol via go-nfs-client, mirroring the layout of
test/sftp. The tests run without a kernel NFS mount, privileged ports,
or any platform-specific tooling.
Coverage includes read/write round-trip, mkdir/rmdir, nested
directories, rename content preservation, overwrite + explicit
truncate, 3 MiB binary file, all-byte binary and empty files, symlink
round-trip, ReadDirPlus listing, missing-path remove, FSInfo sanity,
sequential appends, and readdir-after-remove.
Framework notes:
- Picks ephemeral ports with net.Listen("127.0.0.1:0") and passes
-port.grpc explicitly so the default port+10000 convention cannot
overflow uint16 on macOS.
- Pre-creates the /nfs_export directory via the filer HTTP API before
starting the NFS server — the NFS server's ensureIndexedEntry check
requires the export root to exist with a real entry, which filer.Root
does not satisfy when the export path is "/".
- Reuses the same rpc.Client for mount and target so go-nfs-client does
not try to re-dial via portmapper (which concatenates ":111" onto the
address).
* ci: add NFS integration test workflow
Mirror test/sftp's workflow for the new test/nfs suite so PRs that touch
the NFS server, the inode filer plumbing it depends on, or the test
harness itself run the 14 NFSv3-over-RPC integration tests on Ubuntu
22.04 via `make test`.
* nfs: use append for buffer growth in Write and Truncate
The previous make+copy pattern reallocated the full buffer on every
extending write or truncate, giving O(N^2) behaviour for sequential
write loops. Switching to `append(f.content, make([]byte, delta)...)`
lets Go's amortized growth strategy absorb the repeated extensions.
Called out by gemini-code-assist on PR 9067.
* filer: honor caller cancellation in collectInodeIndexEntries
Dropping the WithoutCancel wrapper lets DeleteFolderChildren bail out of
the inode-index scan if the client disconnects mid-walk. The cleanup is
already treated as best-effort by the caller (it logs on error and
continues), so a cancelled walk just means the partial index rebuild is
skipped — the same failure mode as any other index write error.
Flagged as a DoS concern by gemini-code-assist on PR 9067.
* nfs: skip filer read on open when O_TRUNC is set
openFile used to unconditionally loadWritableContent for every writable
open and then discard the buffer if O_TRUNC was set. For large files
that is a pointless 64 MiB round-trip. Reorder the branches so we only
fetch existing content when the caller intends to keep it, and mark the
file dirty right away so the subsequent Close still issues the
truncating write. Called out by gemini-code-assist on PR 9067.
* nfs: allow Seek on O_APPEND files and document buffered write cap
Two related cleanups on filesystem.go:
- POSIX only restricts Write on an O_APPEND fd, not lseek. The existing
Seek error ("append-only file descriptors may only seek to EOF")
prevented read-and-write workloads that legitimately reposition the
read cursor. Write already snaps the offset to EOF before persisting
(see seaweedFile Write), so Seek can unconditionally accept any
offset. Update the unit test that was asserting the old behaviour.
- Add a doc comment on maxBufferedWriteSize explaining that it is a
per-file ceiling, the memory footprint it implies, and that the real
fix for larger whole-file rewrites is streaming / multi-chunk support.
Both changes flagged by gemini-code-assist on PR 9067.
* nfs: guard offset before casting to int in Write
CodeQL flagged `int(f.offset) + len(p)` inside the Write growth path as
a potential overflow on architectures where `int` is 32-bit. The
existing check only bounded the post-cast value, which is too late.
Clamp f.offset against maxBufferedWriteSize before the cast and also
reject negative/overflowed endOffset results. Both branches fall
through to billy.ErrNotSupported, the same behaviour the caller gets
today for any out-of-range buffered write.
* nfs: compute Write endOffset in int64 to satisfy CodeQL
The previous guard bounded f.offset but left len(p) unchecked, so
CodeQL still flagged `int(f.offset) + len(p)` as a possible int-width
overflow path. Bound len(p) against maxBufferedWriteSize first, do the
addition in int64, and only cast down after the total has been clamped
against the buffer ceiling. Behaviour is unchanged: any out-of-range
write still returns billy.ErrNotSupported.
* ci: drop emojis from nfs-tests workflow summary
Plain-text step summary per user preference — no decorative glyphs in
the NFS CI output or checklist.
* nfs: annotate remaining DEV_PLAN TODOs with status
Three of the unchecked items are genuine follow-up PRs rather than
missing work in this one, and one was actually already done:
- Reuse chunk cache and mutation stream helpers without FUSE deps:
checked off — the NFS server imports weed/filer.ReaderCache and
weed/util/chunk_cache directly with no weed/mount or go-fuse imports.
- Extract shared read/write helpers from mount/WebDAV/SFTP: annotated
as deferred to a separate refactor PR (touches four packages).
- Expand direct data-path writes beyond the 64 MiB buffered fallback:
annotated as deferred — requires a streaming WRITE path.
- Shared lock state + lock tests: annotated as blocked upstream on
go-nfs's missing NLM/NFSv4 lock state RPCs, matching the existing
"Current Blockers" note.
* test/nfs: share port+readiness helpers with test/testutil
Drop the per-suite mustPickFreePort and waitForService re-implementations
in favor of testutil.MustAllocatePorts (atomic batch allocation; no
close-then-hope race) and testutil.WaitForPort / SeaweedMiniStartupTimeout.
Pull testutil in via a local replace directive so this standalone
seaweedfs-nfs-tests module can import the in-repo package without a
separate release.
Subprocess startup is still master + volume + filer + nfs — no switch to
weed mini yet, since mini does not know about the nfs frontend.
* nfs: stream writes to volume servers instead of buffering the whole file
Before this change the NFS write path held the full contents of every
writable open in memory:
- OpenFile(write) called loadWritableContent which read the existing
file into seaweedFile.content up to maxBufferedWriteSize (64 MiB)
- each Write() extended content in-place
- Close() uploaded the whole buffer as a single chunk via
persistContent + AssignVolume
The 64 MiB ceiling made large NFS writes return NFS3ERR_NOTSUPP, and
even below the cap every Write paid a whole-file-in-memory cost. This
PR rewrites the write path to match how `weed filer` and the S3 gateway
persist data:
- openFile(write) no longer loads the existing content at all; it
only issues an UpdateEntry when O_TRUNC is set *and* the file is
non-empty (so a fresh create+trunc is still zero-RPC)
- Write() streams the caller's bytes straight to a volume server via
one AssignVolume + one chunk upload, then atomically appends the
resulting chunk to the filer entry through mutateEntry. Any
previously inlined entry.Content is migrated to a chunk in the same
update so the chunk list becomes the authoritative representation.
- Truncate() becomes a direct mutateEntry (drop chunks past the new
size, clip inline content, update FileSize) instead of resizing an
in-memory buffer.
- Close() is a no-op because everything was flushed inline.
The small-file fast path that the filer HTTP handler uses is preserved:
if the post-write size still fits in maxInlineWriteSize (4 MiB) and
the file has no existing chunks, we rewrite entry.Content directly and
skip the volume-server round-trip. This keeps single-shot tiny writes
(echo, small edits) cheap while completely removing the 64 MiB cap on
larger files. Read() now always reads through the chunk reader instead
of a local byte slice, so reads inside the same session see the freshly
appended data.
Drops the unused seaweedFile.content / dirty fields, the
maxBufferedWriteSize constant, and the loadWritableContent helper.
Updates TestSeaweedFileSystemSupportsNamespaceMutations expectations
to match the new "no extra O_TRUNC UpdateEntry on an empty file"
behavior (still 3 updates: Write + Chmod + Truncate).
* filer: extract shared gateway upload helper for NFS and WebDAV
Three filer-backed gateways (NFS, WebDAV, and mount) each had a local
saveDataAsChunk that wrapped operation.NewUploader().UploadWithRetry
with near-identical bodies: build AssignVolumeRequest, build
UploadOption, build genFileUrlFn with optional filerProxy rewriting,
call UploadWithRetry, validate the result, and call ToPbFileChunk.
Pull that body into filer.SaveGatewayDataAsChunk with a
GatewayChunkUploadRequest struct so both NFS and WebDAV can delegate
to one implementation.
- NFS's saveDataAsChunk is now a thin adapter that assembles the
GatewayChunkUploadRequest from server options and calls the helper.
The chunkUploader interface keeps working for test injection because
the new GatewayChunkUploader interface is structurally identical.
- WebDAV's saveDataAsChunk is similarly a thin adapter — it drops the
local operation.NewUploader call plus the AssignVolume/UploadOption
scaffolding.
- mount is intentionally left alone. mount's saveDataAsChunk has two
features that do not fit the shared helper (a pre-allocated file-id
pool used to skip AssignVolume entirely, and a chunkCache
write-through at offset 0 so future reads hit the mount's local
cache), both of which are mount-specific.
Marks the Phase 2 "extract shared read/write helpers from mount,
WebDAV, and SFTP" DEV_PLAN item as done. The filer-level chunk read
path (NonOverlappingVisibleIntervals + ViewFromVisibleIntervals +
NewChunkReaderAtFromClient) was already shared.
* nfs: remove DESIGN.md and DEV_PLAN.md
The planning documents have served their purpose — all phase 1 and
phase 2 items are landed, phase 3 streaming writes are landed, phase 2
shared helpers are extracted, and the two remaining phase 4 items
(shared lock state + lock tests) are blocked upstream on
github.com/willscott/go-nfs which exposes no NLM or NFSv4 lock state
RPCs. The running decision log no longer reflects current code and
would just drift. The NFS wiki page
(https://github.com/seaweedfs/seaweedfs/wiki/NFS-Server) now carries
the overview, configuration surface, architecture notes, and known
limitations; the source is the source of truth for the rest.
|
||
|
Chris LuandGitHub
|
7a7f220224 |
feat(mount): cap write buffer with -writeBufferSizeMB (#9066)
* feat(mount): cap write buffer with -writeBufferSizeMB Without a bound on the per-mount write pipeline, sustained upload failures (e.g. volume server returning "Volume Size Exceeded" while the master hasn't yet rotated assignments) let sealed chunks pile up across open file handles until the swap directory — by default os.TempDir() — fills the disk. Reported on 4.19 filling /tmp to 1.8 TB during a large rclone sync. Add a global WriteBufferAccountant shared across every UploadPipeline in a mount. Creating a new page chunk (memory or swap) first reserves ChunkSize bytes; when the cap is reached the writer blocks until an uploader finishes and releases, turning swap overflow into natural FUSE-level backpressure instead of unbounded disk growth. The new -writeBufferSizeMB flag (also accepted via fuse.conf) defaults to 0 = unlimited, preserving current behavior. Reserve drops chunksLock while blocking so uploader goroutines — which take chunksLock on completion before calling Release — cannot deadlock, and an oversized reservation on an empty accountant succeeds to avoid single-handle starvation. * fix(mount): plug write-budget leaks in pipeline Shutdown Review on #9066 caught two accounting bugs on the Destroy() path: 1. Writable-chunk leak (high). SaveDataAt() reserves ChunkSize before inserting into writableChunks, but Shutdown() only iterated sealedChunks. Truncate / metadata-invalidation flows call Destroy() (via ResetDirtyPages) without flushing first, so any dirty but unsealed chunks would permanently shrink the global write budget. Shutdown now frees and releases writable chunks too. 2. Double release with racing uploader (medium). Shutdown called accountant.Release directly after FreeReference, while the async uploader goroutine did the same on normal completion — under a Destroy-before-flush race this could underflow the accountant and let later writes exceed the configured cap. Move accounting into SealedChunk.FreeReference itself: the refcount-zero transition is exactly-once by construction, so any number of FreeReference calls release the slot precisely once. Add regression tests for the writable-leak and the FreeReference idempotency guarantee. * test(mount): remove sleep-based race in accountant blocking test Address review nits on #9066: - Replace time.Sleep(50ms) proxy for "goroutine entered Reserve" with a started channel the goroutine closes immediately before calling Reserve. Reserve cannot make progress until Release is called, so landed is guaranteed false after the handshake — no arbitrary wait. - Short-circuit WriteBufferAccountant.Used() in unlimited mode for consistency with Reserve/Release, avoiding a mutex round-trip. * test(mount): add end-to-end write-buffer cap integration test Exercises the full write-budget plumbing with a small cap (4 chunks of 64 KiB = 256 KiB) shared across three UploadPipelines fed by six concurrent writers. A gated saveFn models the "volume server rejecting uploads" condition from the original report: no sealed chunk can drain until the test opens the gate. A background sampler records the peak value of accountant.Used() throughout the run. The test asserts: - writers fill the budget and then block on Reserve (Used() stays at the cap while stalled) - Used() never exceeds the configured cap even under concurrent pressure from multiple pipelines - after the gate opens, writers drain to zero - peak observed Used() matches the cap (262144 bytes in this run) While wiring this up, the race detector surfaced a pre-existing data race on UploadPipeline.uploaderCount: the two glog.V(4) lines around the atomic Add sites read the field non-atomically. Capture the new value from AddInt32 and log that instead — one-liner each, no behavioral change. * test(fuse): end-to-end integration test for -writeBufferSizeMB Exercise the new write-buffer cap against a real weed mount so CI (fuse-integration.yml) covers the FUSE→upload-pipeline→filer path, not just the in-package unit tests. Uses a 4 MiB cap with 2 MiB chunks so every subtest's total write demand is multiples of the budget and Reserve/Release must drive forward progress for writes to complete. Subtests: - ConcurrentLargeWrites: six parallel 6 MiB files (36 MiB total, ~18 chunk allocations) through the same mount, verifies every byte round-trips. - SingleFileExceedingCap: one 20 MiB file (10 chunks) through a single handle, catching any self-deadlock when the pipeline's own earlier chunks already fill the global budget. - DoesNotDeadlockAfterPressure: final small write with a 30s timeout, catching budget-slot leaks that would otherwise hang subsequent writes on a still-full accountant. Ran locally on Darwin with macfuse against a real weed mini + mount: === RUN TestWriteBufferCap --- PASS: TestWriteBufferCap (1.82s) * test(fuse): loosen write-buffer cap e2e test + fail-fast on hang On Linux CI the previous configuration (-writeBufferSizeMB=4, -concurrentWriters=4 against a 20 MiB single-handle write) deterministically hung the "Run FUSE Integration Tests" step to the 45-minute workflow timeout, while on macOS / macfuse the same test completes in ~2 seconds (see run 24386197483). The Linux hang shows up after TestWriteBufferCap/ConcurrentLargeWrites completes cleanly, then TestWriteBufferCap/SingleFileExceedingCap starts and never emits its PASS line. Change: - Loosen the cap to 16 MiB (8 × 2 MiB chunk slots) and drop the custom -concurrentWriters override. The subtests still drive demand well above the cap (32 MiB concurrent, 12 MiB single-handle), so Reserve/Release is still on every chunk-allocation path; the cap just gives the pipeline enough headroom that interactions with the per-file writableChunkLimit and the go-fuse MaxWrite batching don't wedge a single-handle writer on a slow runner. - Wrap every os.WriteFile in a writeWithTimeout helper that dumps every live goroutine on timeout. If this ever re-regresses, CI surfaces the actual stuck goroutines instead of a 45-minute walltime. - Also guard the concurrent-writer goroutines with the same timeout + stack dump. The in-package unit test TestWriteBufferCap_SharedAcrossPipelines remains the deterministic, controlled verification of the blocking Reserve/Release path — this e2e test is now a smoke test for correctness and absence of deadlocks through a real FUSE mount, which is all it should be. * fix: address PR #9066 review — idempotent FreeReference, subtest watchdog, larger single-handle test FreeReference on SealedChunk now early-returns when referenceCounter is already <= 0. The existing == 0 body guard already made side effects idempotent, but the counter itself would still decrement into the negatives on a double-call — ugly and a latent landmine for any future caller that does math on the counter. Make double-call a strict no-op. test(fuse): per-subtest watchdog + larger single-handle test - Add runSubtestWithWatchdog and wrap every TestWriteBufferCap subtest with a 3-minute deadline. Individual writes were already timeout-wrapped but the readback loops and surrounding bookkeeping were not, leaving a gap where a subtest body could still hang. On watchdog fire, every live goroutine is dumped so CI surfaces the wedge instead of a 45-minute walltime. - Bump testLargeFileUnderCap from 12 MiB → 20 MiB (10 chunks) to exceed the 16 MiB cap (8 slots) again and actually exercise Reserve/Release backpressure on a single file handle. The earlier e2e hang was under much tighter params (-writeBufferSizeMB=4, -concurrentWriters=4, writable limit 4); with the current loosened config the pressure is gentle and the goroutine-dump-on-timeout safety net is in place if it ever regresses. Declined: adding an observable peak-Used() assertion to the e2e test. The mount runs as a subprocess so its in-process WriteBufferAccountant state isn't reachable from the test without adding a metrics/RPC surface. The deterministic peak-vs-cap verification already lives in the in-package unit test TestWriteBufferCap_SharedAcrossPipelines. Recorded this rationale inline in TestWriteBufferCap's doc comment. * test(fuse): capture mount pprof goroutine dump on write-timeout The previous run (24388549058) hung on LargeFileUnderCap and the test-side dumpAllGoroutines only showed the test process — the test's syscall.Write is blocked in the kernel waiting for FUSE to respond, which tells us nothing about where the MOUNT is stuck. The mount runs as a subprocess so its in-process stacks aren't reachable from the test. Enable the mount's pprof endpoint via -debug=true -debug.port=<free>, allocate the port from the test, and on write-timeout fetch /debug/pprof/goroutine?debug=2 from the mount process and log it. This gives CI the only view that can actually diagnose a write-buffer backpressure deadlock (writer goroutines blocked on Reserve, uploader goroutines stalled on something, etc). Kept fileSize at 20 MiB so the Linux CI run will still hit the hang (if it's genuinely there) and produce an actionable mount-side dump; the alternative — silently shrinking the test below the cap — would lose the regression signal entirely. * review: constructor-inject accountant + subtest watchdog body on main Two PR-#9066 review fixes: 1. NewUploadPipeline now takes the WriteBufferAccountant as a constructor parameter; SetWriteBufferAccountant is removed. In practice the previous setter was only called once during newMemoryChunkPages, before any goroutine could touch the pipeline, so there was no actual race — but constructor injection makes the "accountant is fixed at construction time" invariant explicit and eliminates the possibility of a future caller mutating it mid-flight. All three call sites (real + two tests) updated; the legacy TestUploadPipeline passes a nil accountant, preserving backward-compatible unlimited-mode behavior. 2. runSubtestWithWatchdog now runs body on the subtest main goroutine and starts a watcher goroutine that only calls goroutine-safe t methods (t.Log, t.Logf, t.Errorf). The previous version ran body on a spawned goroutine, which meant any require.* or writeWithTimeout t.Fatalf inside body was being called from a non-test goroutine — explicitly disallowed by Go's testing docs. The watcher no longer interrupts body (it can't), so body must return on its own — which it does via writeWithTimeout's internal 90s timeout firing t.Fatalf on (now) the main goroutine. The watchdog still provides the critical diagnostic: on timeout it dumps both test-side and mount-side (via pprof) goroutine stacks and marks the test failed via t.Errorf. * fix(mount): IsComplete must detect coverage across adjacent intervals Linux FUSE caps per-op writes at FUSE_MAX_PAGES_PER_REQ (typically 1 MiB on x86_64) regardless of go-fuse's requested MaxWrite, so a 2 MiB chunk filled by a sequential writer arrives as two adjacent 1 MiB write ops. addInterval in ChunkWrittenIntervalList does not merge adjacent intervals, so the resulting list has two elements {[0,1M], [1M,2M]} — fully covered, but list.size()==2. IsComplete previously returned `list.size() == 1 && list.head.next.isComplete(chunkSize)`, which required a single interval covering [0, chunkSize). Under that rule, chunks filled by adjacent writes never reach IsComplete==true, so maybeMoveToSealed never fires, and the chunks sit in writableChunks until FlushAll/close. SaveContent handles the adjacency correctly via its inline merge loop, so uploads work once they're triggered — but IsComplete is the gate that triggers them. This was a latent bug: without the write-buffer cap, the overflow path kicks in at writableChunkLimit (default 128) and force-seals chunks, hiding the leak. #9066's -writeBufferSizeMB adds a tighter global cap, and with 8 slots / 20 MiB test, the budget trips long before overflow. The writer blocks in Reserve, waiting for a slot that never frees because no uploader ever ran — observed in the CI run 24390596623 mount pprof dump: goroutine 1 stuck in WriteBufferAccountant.Reserve → cond.Wait, zero uploader goroutines anywhere in the 89-goroutine dump. Walk the (sorted) interval list tracking the furthest covered offset; return true if coverage reaches chunkSize with no gaps. This correctly handles adjacent intervals, overlapping intervals, and out-of-order inserts. Added TestIsComplete_AdjacentIntervals covering single-write, two adjacent halves (both orderings), eight adjacent eighths, gaps, missing edges, and overlaps. * test(fuse): route mount glog to stderr + dump mount on any write error Run 24392087737 (with the IsComplete fix) no longer hangs on Linux — huge progress. Now TestWriteBufferCap/LargeFileUnderCap fails with 'close(...write_buffer_cap_large.bin): input/output error', meaning a chunk upload failed and pages.lastErr propagated via FlushData to close(). But the mount log in the CI artifact is empty because weed mount's glog defaults to /tmp/weed.* files, which the CI upload step never sees, so we can't tell WHICH upload failed or WHY. Add -logtostderr=true -v=2 to MountOptions so glog output goes to the mount process's stderr, which the framework's startProcess redirects into f.logDir/mount.log, which the framework's DumpLogs then prints to the test output on failure. The -v=2 floor enables saveDataAsChunk upload errors (currently logged at V(0)) plus the medium-level write_pipeline/upload traces without drowning the log in V(4) noise. Also dump MOUNT goroutines on any writeWithTimeout error (not just timeout). The IsComplete fix means we now get explicit errors instead of silent hangs, and the goroutine dump at the error moment shows in-flight upload state (pending sealed chunks, retry loops, etc) that a post-failure log alone can't capture. |