5 Commits

Include renames

Author	SHA1	Message	Date
Chris Lu	6a594da739	filer: free orphaned hard-link chunks and remote objects in delete sweeps (#11809) ... * filer: free shared chunks when a folder delete drops the last hard link doBatchDeleteFolderMetaAndData skipped chunk collection for every hard-linked child, relying on maybeDeleteHardLinks to retire the shared inode record. When the delete removes the last link the record is gone and nobody else will ever free those chunks. After the link callback runs, check whether the record survived; if it did not, queue the entry's chunks for deletion. * filer: free orphaned hard-link chunks in the replayed delete sweep The subtree sweep on a replayed directory delete skipped every hard-linked child, so a link that was the inode's last owner leaked its shared chunks. Check the inode record after the store delete; when it is gone, queue the child's chunks like any other swept entry. * filer: mirror remote-object cleanup in the replayed delete sweep Children a lagging peer alone still holds under a replayed directory delete were dropped with plain store deletes: their remote objects leaked and lazy remote fetches could resurrect them. The sweep now records event-stamped tombstones and deletes swept non-directory children through maybeDeleteFromRemoteIfUnchanged, which carries the same stat guard the write-back daemon's processDeleteEvent applies plus a modification-time check — a delayed replay cannot erase an object another writer replaced, even a same-size one. A failed remote delete fails the replay so the event is retried instead of orphaning the object. The directory's own tombstone comes from the event pipeline at the event timestamp and covers the subtree. Metadata-only deletes and cross-cluster events keep the origin's semantics. * filerstore: return real errors from KvGet on MongoDB, ArangoDB, Elastic These stores mapped every lookup failure to ErrKvNotFound, so a database error looked identical to a missing key: hard-link records could appear gone while links still owned their chunks, and path timestamp reads could treat committed state as absent. Return the underlying error and keep ErrKvNotFound for genuinely missing keys. * filer: bind remote-object deletes to the observed object version A guarded stat-then-delete still lets a replacement landing between the two requests go down with the deleted object. Add an optional conditional-delete operation to remote storage clients — S3 If-Match on the live ETag, GCS generation match, Azure IfMatch — and use it everywhere a delete is guarded by a stored stamp: the write-back daemon's delete, rename-source, and pending-rename paths, and the replayed-delete sweep. Backends without it keep the guarded stat-then-delete. * filer: restamp the remote entry after a metadata-only remote update Changing Content-Encoding rewrites the object in place on S3 and advances its LastModified, while the recorded stamp keeps the old mtime. A version-checked delete would then mistake the object for a replacement and keep it forever. Re-stat after UpdateFileMetadata so the saved stamp describes the live object. * filer: mirror the remote-delete ownership checks in the replayed sweep The sweep deleted swept children's remote objects on the listing's snapshot alone. Two gaps: a remote-only entry that took the path since — one a commit record may not cover, such as a lazy fetch materialized from the remote — reads the object as its only copy, and an entry removed while its rename copy was pending leaves the object at the source key orphaned. The per-child delete now runs under the child's entry lock with the commit-time check repeated, keeps a remote-only entry that holds the path, and releases the pending rename's source under the same ownership and stamp conditions the write-back daemon's releasePendingSource applies. * remote.sync: restamp only the object the metadata update touched The live stat after UpdateFileMetadata was saved unconditionally, so a replacement written between the update and the stat stamped the entry with an object it never described — and a later delete would then pass the freshness guard on the replacement's data. The restamp now requires the live object to still match the stored stamp on size and ETag; a different object keeps the old stamp, which the delete guard reads as replaced and keeps. * filer: skip unbudgeted reader-cache test on 32-bit TestReaderCacheUnboundedWithoutBudget proves the reader cache has no implicit memory budget by keeping 96 concurrent 4 MiB downloads in flight (>256 MiB). On linux/386 the weed/filer test binary already runs near the 4 GiB address-space limit, so the burst reliably crashes it with "out of memory" mid-suite. * filer: bail out of a swept child lock instead of waiting on it Replay already holds the event's path locks when the sweep reaches a descendant. A local rename can hold that descendant while waiting on one of the replayed paths, so blocking on the child lock deadlocks both. The sweep now tries the lock and reports ErrLockContended — transient, so the replication retry replays the event once the local write settles. * remote: report the stamp a metadata update produced A metadata write can rewrite the object it touches — S3 changes Content-Encoding through an in-place copy, which can mint a new ETag — so restamping from a fresh stat can reject the very object the update produced, or describe a replacement that landed in between. UpdateFileMetadata now returns the RemoteEntry of the object it wrote (S3 from the copy result, GCS from the update attributes, Azure from a post-update stat); nil means the object was untouched and callers keep the stored stamp. * remote: restamp Azure from the metadata write's own etag	2026-10-18 12:23:30 -07:00
Chris LuandDevin	6c07a5fdd0	s3: keep small ranged GETs on range reads, no whole-chunk downloads (#11577) ... * filer: keep a ranged read in random mode through its contiguous tail A far ReadAt on a fresh ReaderPattern left the sequential counter at -1, so the next buffer of the same ranged request landed on the frontier and flipped the verdict straight back to sequential — readChunkSliceAt then paid a whole-chunk fetch for the remainder of the range. Drop the counter to -ModeChangeLimit when random mode is entered so the verdict needs sustained sequential evidence to undo, matching the hysteresis an established sequential stream already gets. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: pin small ranged GETs to range reads A ranged GET whose first read lands within SeqTolerance of offset 0 is judged sequential immediately, and even a far-starting range could flip back mid-request; either way readChunkSliceAt downloads each covered chunk in full, multiplying disk reads for small ranged reads (measured ~7x). Pin random mode for ranged requests no larger than SeqTolerance so all of the request's buffer reads stay range fetches. Larger ranges keep the dynamic pattern, where whole-chunk fetches amortize. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: fetch only the part of a chunk the view covers Replaces the PinRandomMode size heuristic with a per-chunk coverage rule. ViewFromVisibleIntervals already clips chunk views to the request window, so a view that is not IsFullChunk() is one the request only partially needs; fetch it as a range regardless of the detected read pattern. This closes the holes a request-size pin left open: ranges larger than SeqTolerance no longer revert to whole-chunk downloads once their buffers look sequential, and ranges that fully cover a chunk keep the shared whole-chunk path instead of fetching 256KiB slices piecemeal. Prefetch (MaybeCache) skips clipped views so it cannot amplify a range read either. PinRandomMode is dropped: no caller needs it once coverage drives the fetch choice. Range fetches route through fetchChunkDataFn so tests observe them the same way as whole-chunk downloads. * filer: keep ciphered chunks on the whole-chunk path A range fetch cannot save bytes for a ciphered chunk: readEncryptedUrl always downloads and decrypts the whole blob before slicing. Sending partial views of ciphered chunks through fetchChunkRange would repeat the full download per buffer, so they keep the shared whole-chunk path where one download serves every buffer. Prefetch stays enabled for them for the same reason. * filer: keep compressed chunks on the whole-chunk path Like ciphered chunks, a range request on a compressed chunk makes the volume server read and decompress the whole needle, so range-per-buffer would repeat the full backend read for each 256KiB window. Route them through the shared whole-chunk path via ChunkView.CanRangeFetch. * filer: fall back to range fetch when a chunk exceeds the reader budget A ciphered or compressed chunk larger than readerCacheSizeMB can never be read through the whole-chunk path — the budget rejects the buffer — so its partial views must still range-fetch or the GET fails outright. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>	2026-10-03 15:01:04 +08:00
Chris Lu	c72eda50a8	s3: drop implicit reader cache budget that throttled S3 GETs (#11384) ... * fix(filer): leave reader cache unbounded without an explicit budget NewReaderCache silently installed a 256MiB ReaderCacheBudget when the caller passed none. Only weed mount opts into a budget; every other caller (S3 gateway, WebDAV, query engine, mq logstore) inherited the cap. Under ~90 concurrent S3 GETs of medium objects, prefetch wants far more than 64 chunk buffers, so reserve() serialized chunk fetches, clients timed out and retried, and the retry re-downloaded chunks the cancelled request had already fetched. A nil budget now means unbounded, restoring the pre-4.47 behavior for callers that never asked for a memory cap; reserve/complete/release are nil-safe. The mount path is unchanged and still enforces -readerCacheSizeMB. Fixes #11380 * feat(s3): expose -s3.readerCacheSizeMB reader buffer budget Operators who want the S3 gateway read path memory-bounded can now opt in: -s3.readerCacheSizeMB on weed filer/server/mini and -readerCacheSizeMB on standalone weed s3, matching the mount flag. The default 0 keeps the unbounded pre-4.47 behavior; a positive value installs a shared ReaderCacheBudget across in-flight and retained chunk buffers for all S3 GETs. * fix(filer): validate chunk size before consulting the reader budget A nil budget returned early and skipped the negative chunkSize check, letting a corrupted size reach mem.Allocate and panic. Also drop the command-specific flag prefix from the S3 validation error since standalone weed s3 exposes the option as -readerCacheSizeMB. * filer: drop chunk buffers once fully consumed ReaderCache retained every completed chunk buffer in the downloaders map until the slot limit evicted it, so buffers lingered after all readers finished with them. Track attached readers on each SingleChunkCacher and remove the cacher when the last reader consumes the buffer to its end. In-flight download deduplication and the prefetch handoff are unchanged: a buffer always survives until fully read, partial reads keep it available, and an attached reader pins a consumed buffer until it detaches. Repeat reads now go through the chunk cache where enabled, or refetch. * filer: drop consumed buffers on last detach, rechecked under cache lock Two review findings on the drop-on-consume change: - Removal only fired when the detaching reader itself reached the chunk end. If the end-reaching reader finished first and the last remaining reader did a partial read or cancelled, the consumed buffer and its budget reservation lingered until eviction. Track a persistent consumed flag instead, so any end-reaching read marks the buffer and the last detach drops it. - remove() checked only map identity, so a reader attaching between the reader count hitting zero and removal could attach to a cacher that was then deleted underneath it. removeConsumed() re-checks identity, readers == 0, and consumed under the ReaderCache lock; a raced attach keeps the cacher and its own detach retries the removal.	2026-09-18 01:02:32 -07:00
3 people	8db41d0217	[Mount] Cache Chunk Manifest Resolution for Repeated File Opens (#11266) ... * cache resolved chunk manifests for Mount * Address PR review: per-mount cache, singleflight, reuse ResolveOneChunkManifest - Own the manifest cache per WFS mount instead of a process-global variable, so manifests from one filer backend are never served to another (Devin/CodeRabbit major bug). - Coalesce concurrent cold misses via singleflight so only one fetch runs during a cold burst (Greptile P2). - Copy cached data after releasing the mutex so a large copy does not block concurrent hits, inserts, and evictions (CodeRabbit nitpick). - Reuse the existing ResolveOneChunkManifest function name instead of introducing a new resolveOneChunkManifest wrapper. - Validate (unmarshal) manifest bytes before caching so malformed manifests do not poison the cache. - Add TestChunkGroupManifestResolutionCoalescesColdMisses covering the singleflight cold-miss path. * Address round 2 review: coalesced-miss cancellation, test overlap - Use singleflight.DoChan in fetchOrLoad and select on ctx.Done() so a caller whose context is canceled while waiting for an in-flight fetch returns ctx.Err() promptly instead of blocking for the leader's result (Devin BUG). - Add TestResolveOneChunkManifestCanceledWaiterReturnsDuringCoalescedMiss covering the canceled-waiter path. - Delay the cold-miss fixture response so the leader's fetch is still in flight when concurrent opens join the singleflight, making the one-fetch assertions reliable (CodeRabbit Minor). * Address review: keep ResolveOneChunkManifest four-argument Restore the exported ResolveOneChunkManifest to its original four-argument signature so external callers keep compiling. Move the cache-aware resolution into an unexported resolveOneChunkManifest helper that accepts the per-mount ChunkManifestCache. The exported function delegates to the helper with a nil cache, preserving the historical uncached behavior for every non-Mount caller. The Mount path (ChunkGroup.SetChunks) now calls the unexported helper with the mount-owned cache. Tests and benchmarks that exercise the cache path call the unexported helper directly. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com>	2026-09-12 20:06:25 -07:00
Chris Lu	4a1d65939f	fix(mount): bound reader cache memory across open files (#11220) ... * fix(filer): bound retained reader cache buffers by bytes * test(filer): keep in-flight downloads during cache trimming * feat(mem): expose pooled allocation capacity for byte reservations * fix(mount): share a configurable reader buffer budget across files * fix(filer): release failed prefetch slots and memory reservations * feat(mount): expose a soft Go runtime memory limit * docs(filer): restore shared-download rationale in startCaching The one-line comment replacing the original context.Background() explanation was too thin for readChunkAt to cross-reference shared resource semantics. Restore a concise note on why request cancellation must not abort a download shared by concurrent readers. * test(filer): loosen reader cache test deadlines to 5s Three tests used 1-second deadlines that can flake on CI under load: TestReaderCacheBudgetInFlight, TestReaderCacheEvictionDoesNotHoldCacheLock, and TestReaderCacheFailedPrefetchReleasesBudget. Increase to 5 seconds. * test(filer): cover re-read after reader cache eviction Add TestReaderCacheReReadAfterEviction: reads chunk 'a', reads chunk 'b' (evicting 'a' via budget pressure), then re-reads 'a' and asserts a fresh download returns correct data. Verifies the core correctness property that eviction never exposes missing or stale data to readers.	2026-09-08 10:51:28 -07:00