mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-10-06 14:31:57 +02:00
d7a02567e38dba8eeeaa0f309db48afc0bfa17be
1059
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Chris Lu
|
eafe79ebff |
filer: skip UpdateEntry when inline content is unchanged (#11580)
* filer: skip UpdateEntry when inline content is unchanged SaveInsideFiler rewrites config files (IAM identities, filer.conf, remote mappings, policies) unconditionally. Each no-op UpdateEntry is a metadata event the local meta log persists to /topics/.system/log, which appends a chunk to a volume. A client that rewrites identical config on a timer, e.g. the seaweedfs-operator 5-minute resync calling UpdateUser with unchanged actions, keeps .dat/.idx files growing on an otherwise idle cluster and prevents HDD spindown (seaweedfs/seaweedfs#11571). Skip the UpdateEntry when the stored inline content is byte-identical, so unchanged writes produce no metadata event and no volume writes. * filer: test that identical SaveInsideFiler writes skip UpdateEntry * filer: require stamped Md5 before skipping identical writes An entry holding identical content but no Md5 (written before hashing, or by a tool that cleared it) would never get the stamp that IF_ETAG_MATCH conditional writes key off. Skip only when both the stored content and its Md5 match, so one write still lands to repair the stamp. |
||
|
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> |
||
|
yi111andYi-111-a
|
3e679e925e |
filer: keep generated inodes inside the positive signed 64-bit range (#11567)
AsInode derives inodes from HashStringToLong, which is uniform over int64, so roughly half of the derived values land above math.MaxInt64 once they are converted to uint64. The Elasticsearch store indexes Entry.Attr.Inode as a signed long, so those values are rejected with HTTP 400 and the metadata entry is never written, which the filer then retries forever. Fold the sign bit off in one place, util.NormalizeInode, and route both derivation sites through it: FullPath.AsInode (path plus creation time) and the hard-link branch in ensureEntryInode (HardLinkId hash). Masking keeps the other 63 hash bits, so distinct paths still get distinct inodes, and it applies identically to the FUSE mount, which derives the same value. Co-authored-by: Yi-111-a <34116709+0-xiaosu@users.noreply.github.com> |
||
|
|
1445960f8c |
filer: persist pending chunk deletions across restarts (durable deletion ledger) (#11550)
* fix(filer): persist pending chunk deletions across restarts The in-memory FileIdDeletionQueue and DeletionRetryQueue lose every queued-but-unconfirmed deletion when the filer process restarts. Because deletions only enter the pipeline through that queue, a crash between enqueue and the volume confirming the delete leaks the chunk permanently: nothing remembers it. In a multi-filer deployment this was observed as growing collections of orphaned chunks after filer restarts, and — via meta-replay from a peer that still had the entry — orphans being "resurrected" as live references on the recovered filer. This implements the "periodic snapshot with recovery on startup" option noted in the existing DeletionRetryQueue TODO, using the store's KV layer (no new iterator API required across the 15+ store backends): - queueDeletions() is the single entry point that keeps the hot in-memory queue and the durable ledger in sync. - Only terminal outcomes (success / not-found / permanent) remove an id from the ledger; retryable failures keep it, which is the point. - A timer and Shutdown() snapshot the pending set to a single KV key. - On startup, reloadDeletionLedger() re-queues recovered ids after a grace window so the initial peer meta-aggregation settles first. This avoids a new hazard: purging a chunk that a lagging peer is about to re-reference as live data (stale replay turns a stale read into a dangling read otherwise). - Volume deletes are idempotent (not-found == success), so re-deleting after a crash never double-frees. - Kill switch via viper: filer.deleteQueue.persist=false opts out entirely (reload also refuses to recover so a stale ledger never comes back). Tunables: filer.deleteQueue.persistInterval, .recoveryGrace. Adds unit tests covering snapshot+recover, retry-keeps-entry, disabled switch, and zero-value Filer safety (run green under -race). Co-Authored-By: Athena 🏛️ <hermes-agent@local> (custom / Qwen3.8-Flash-Next-ROCmFP4) * filer: harden the deletion ledger - Scope the ledger key by filer address so filers sharing one store do not overwrite each other's pending sets; ledgers written under the old unscoped key are claimed once on startup. - Serialize snapshots on deletionSnapshotLock so an in-flight timer snapshot cannot overwrite a newer shutdown snapshot, and wake the snapshotter on every queue/forget so a queued id persists within milliseconds instead of a full interval. - Merge recovered ids into the pending set immediately on reload; only the queue push waits out the grace window, so an early snapshot rewrites the recovered ids rather than dropping them. - A failed or unparseable ledger read blocks persistence for the run instead of letting snapshots overwrite the unread ledger. - Split the ledger into part keys when it exceeds one 64KB value so stores with a size cap (FoundationDB) do not strand the backlog. - GetReadyItems reports retry-exhausted ids so they are forgotten in the ledger instead of replaying after every restart. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: close the remaining deletion-ledger durability gaps - A manifest referencing a missing part is corruption: surface a wrapped error and block persistence instead of treating the ledger as absent. - Multipart snapshots write generation-scoped part keys and publish the manifest last, so a crash never mixes old and new part contents. - Orphaned parts are tracked in a persisted .stale sidecar and retried. - Legacy/index ledgers are republished under the scoped key before the old keys are removed. - A ledger index lets a filer restart under a new address claim the ledger its previous incarnation left behind. - Expired and permanently-failed retry items only forget the ledger epoch they recorded, so they cannot erase a re-queued id. - A failed startup read no longer disables persistence: every snapshot retries the reload until the store reads again. * filer: tighten ledger claiming, index updates, and retry epochs - touchLedgerIndex verifies its write and retries so a concurrent filer's merge cannot silently drop this key from the index. - Foreign-ledger claims abort on any unreadable source instead of leaving it stranded once the new scoped key exists. - A source that republished during the claim is left in place and its newer ids merge into the claimant's pending set. - AddOrUpdate no longer overwrites the ledger epoch of an in-flight retry item, so its expiry or permanent outcome cannot forget a record that was re-queued after the attempt began. - The recovery grace wait exits on shutdown instead of re-queueing after the filer has stopped. * filer: requeue surviving records, persist claim deltas, guard index writes - A dropped retry item (expired or permanent) whose ledger record was re-enqueued now pushes the id back through the hot queue instead of leaving it pending with nothing scheduled. - Ids merged from a claim source that republished mid-claim are rewritten under our ledger immediately, so they are durable even if the claimant crashes before the next snapshot. - touchLedgerIndex aborts when the index read fails for a real error; only ErrKvNotFound means the index is empty, so a transient failure can no longer wipe peer entries with a one-key write. --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
|
8d97284d0a |
filer: option to store system metadata logs in their own collection (#11551)
* feat(filer): option to store system metadata logs in their own collection The filer's internal /topics/.system/log chunks are assigned to the filer's default collection (-collection). In a multi-filer deployment that default is often empty, so every restart flap, full-sync, or event-buffered flush grows the default collection with system chunks that are indistinguishable from user data in collection.list. This is a large part of what makes the default collection balloon and confuses orphan analysis. This keeps the internal log in a dedicated collection when the operator asks for one, without changing where user data goes: - New optional override, filer.options.metaLog.collection (and .replication), read in NewFiler so both `weed filer` and `weed server -filer` honour it. Default "" => exactly today's behaviour (log follows the filer default), fully backward compatible. - Resolution is a small helper: override first, then the filer default, then a storage rule matched on the log path. Kept separate from the user write path so the internal log targets itself. - bucketCollection() is hardened the same way it already protects the filer's default collection: a bucket that happens to resolve to the redirected meta-log collection must not drop it on delete, because it backs internal log volumes. - Scaffold filer.toml documents the new knobs under [filer.options]. Related to the persisted deletion ledger branch (fix/persist-deletion-queue): together they cut the two sources of post-flap junk in the default collection — that PR stops orphaned user-chunk leak on filer crash, this one stops the internal log from living in default at all. They are independent: no file overlap, no functional dependency; either can merge first. They are paired only in the narrative of cleaning up default. Adds unit tests for the collection/replication resolution chain, the viper keys, and the bucket-delete guard (run green under -race). Co-Authored-By: Athena 🏛️ <hermes-agent@local> (custom / Qwen3.8-Flash-Next-ROCmFP4) * filer: collect bucket chunks when its collection survives the delete bucketCollection returning "" preserves the collection, but the bucket path still skipped per-entry chunk collection and could skip listing the children entirely, so a bucket sharing the meta-log (or any preserved) collection left its object chunks orphaned with no entry pointing at them. Only the wholesale drop of a deleted collection skips those now. Note in filer.toml that the meta-log target should stay stable: chunks written under an older collection are not migrated. * filer: exercise the metaLog override wiring through NewFiler The viper test only echoed back the keys it set, so a wrong key in NewFiler would still pass. It now asserts the fields NewFiler fills from those keys. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: tighten comments around the metaLog collection override Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
757917f564 |
filer: evict remote-cached objects under storage pressure (#11515)
* filer: identify remote-mounted entries safe to drop under disk pressure ListEvictableRemoteEntries walks every mounted directory directly on the filer store (no lazy remote listing) and returns entries that hold local chunks fully synchronized with remote, ordered oldest-cached first. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: evict remote-cached chunks oldest-first and vacuum the garbage uncacheRemoteEntry applies the same transition remote.uncache does - cleared chunks plus a reset LastLocalSyncTsNs under the entry path lock - and evictRemoteCachedEntries serializes passes over all mounts until a byte target is met. Aged victims are preferred; a second pass accepts any synchronized cached entry when aged ones cannot cover the request, since a failed read is worse than a dropped hot object. Cleared chunks only become disk space after compaction, so reclaimRemoteCacheSpace pairs each pass with a rate-limited VacuumVolume call that also picks up orphaned partial fills. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: trigger remote cache eviction under storage pressure A periodic check (30s) reads disk usage from master topology and evicts remote-mounted cached chunks once any disk crosses -filer.remoteCacheEvictThreshold (default 0.9; 0 disables), with a vacuum pass to reclaim the tombstoned needles. The cold-read cache path also kicks the same reclaim when a fill fails on exhausted volumes - the request still falls back to streaming from the remote, but the cache stops being permanently wedged full. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: flush deletion queue before remote cache vacuum Vacuum ran immediately after eviction while evicted file IDs still sat in the asynchronous deletion queue, so compaction saw no garbage and the cache stayed wedged. Flush the queue synchronously first and shorten the vacuum cooldown so sustained pressure does not wait five minutes between reclaim passes. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: cover remote cache eviction under capacity pressure Unit tests pin the eligibility filter and oldest-first ordering; the integration test runs a constrained two-node setup that saturates the cache, verifies the oldest synced entry is evicted and vacuumed, and that a later read re-caches it. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: coalesce remote cache reclaim passes A failed cache fill used to queue behind any in-flight eviction, stacking full mount traversals during a write-failure storm. Skip the pass when one is already running; the caller falls back to streaming from remote regardless. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: stop the remote cache janitor on shutdown The eviction ticker kept running after Shutdown closed the metadata store and could traverse a closed store. Give the janitor a context cancelled from Shutdown and propagate it into its master RPCs and traversals. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: vacuum only tombstoned volumes and retry deferred passes VacuumVolume with no volume id swept every collection, compacting volumes unrelated to the cache fill that failed. Now the reclaim path collects the vids of file ids actually flushed from the deletion queue and compacts only those. Vids that land inside the vacuum cooldown stay in a pending set the janitor retries on each tick, so chunks evicted just after a sweep are not stranded until the next pressure event. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: count only pressured disks when evicting remote cache The janitor measured the largest excess on one disk but let bytes on healthy disks satisfy the reclaim target. Split the topology disk view per physical disk and count only chunk bytes whose volumes sit on an over-threshold disk; entries contributing nothing there are skipped. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: compare remote cache sync time at nanosecond precision Second-precision mtime comparisons let a local write in the same second as the last sync still qualify as evictable, discarding unsynced changes. Compare LastLocalSyncTsNs against full-precision mtime (mtime_ns round-trips through the entry codec), and apply the same fix to remote.uncache's inline check. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: invalidate remote sync stamp on local content change A local overwrite that keeps the remote entry's LastLocalSyncTsNs looks evictable even though the remote copy no longer matches, and some write paths stamp mtime at second precision so a timestamp comparison cannot catch it. UpdateEntry now clears the stamp when chunks change without a fresh stamp, leaving replicated updates authoritative. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: bound remote cache master rpcs and vacuum all evicted garbage VolumeList and VacuumVolume now run under a 30s context so a stalled master cannot wedge the eviction janitor. The targeted vacuum drops the garbage threshold so volumes with under 10% deleted bytes still compact. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: tolerate straggler fills in remote cache eviction test Detached fills from the concurrent wave keep racing the final checks: live chunks legitimately fill both volumes, and a re-cached object can be evicted again before its commit is observed. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: start remote cache eviction loop after filer init The janitor's first tick dereferences fs.filer; starting the goroutine before NewFiler assigns it could panic when startup exceeds an interval. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: keep remote cache vacuum intent across retries Evicted entries now record their chunk volumes for vacuum directly, so the intent survives whoever consumes the shared deletion queue first. A pending volume keeps several vacuum attempts so tombstones that land late are still compacted, and the janitor retries pending volumes under the reclaim mutex instead of flushing unrelated deletes every tick. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: bound each remote cache vacuum request independently A shared 30s deadline across pending volumes let one slow compaction cancel the rest. Each VacuumVolume now gets its own context, and pending volumes keep more attempts since the master reports request acceptance rather than compaction. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: tighten remote cache reclamation bound Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: treat chunk timestamp changes as content changes chunksEqual now also compares ModifiedTsNs so an update that rewrites a chunk record still invalidates the remote sync stamp. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: run remote cache queue flush under the reclaim context BatchDelete for flushed file ids now uses the caller's context instead of context.Background(), so a reclaim pass bounded by shutdown or timeout stops its deletes too. Other callers keep their existing behavior. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: scope remote cache vacuum to evicted volumes The flush no longer feeds the shared deletion queue's ids into the pending set — only evicted chunks' volumes are tracked, so ordinary deletions no longer pick up repeated vacuum attempts. The flush also runs under a shutdown-immune bounded context and is skipped when no volume is pending. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: retry remote cache vacuums even after unmount Pending volumes were only retried while a remote mount existed; removing the last mount skipped every later pass and left evicted bytes allocated. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: reclaim partial cache fills that run out of capacity A fill that fails midway queues its written chunks for deletion, but when no entries remain evictable the reclaim pass found no pending volumes and skipped the flush and vacuum entirely, leaving the partial garbage to the slow periodic vacuum while the disk stayed full. Mark the failed fill's chunk volumes pending so the pass tombstones and compacts them even when nothing was evicted. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
9b3b12c607 |
filer: pin-aware reader cache eviction and stream release (#11503)
* filer: synchronize stream pins and release them on transitions Guard chunkStream.cacher with the ReaderCache lock everywhere: mount sections share one ChunkReadAt across concurrent reads, and unsynchronized release could double-unpin. Reads served from the chunk cache now detach the stream's pin instead of retaining the previous chunk. Eviction prefers unpinned downloaders so a pinned buffer is not dropped mid-stream. A new ReleaseStream lets callers drop their pin without destroying the shared cache; S3 and WebDAV readers use it. lastChunkFid becomes atomic since concurrent mount reads can update it. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: keep eviction bounded when every downloader is pinned Both eviction paths still fall back to a pinned victim when no unpinned one exists, so abandoned stream pins cannot bypass the downloader limit or stall the memory budget. Budget eviction also rechecks the pin under the ReaderCache lock at removal time: a stream that pinned the selected victim in between keeps it mapped and the selection retries. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: restore budget bookkeeping when a victim gets pinned mid-eviction removeUnpinned losing the pin race left the victim out of the idle list while still holding its reservation, making it unevictable even as the pinned fallback. Push it back when the reservation is still live. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Tobias Gurtzick
|
3abdef3202 |
filer: keep shared chunk buffers pinned while another stream reads them (#11502)
The ReaderCache is shared by all streams of a process (every S3 GET, for instance), but a ChunkReadAt released chunks as if it owned them: - moving on to the next chunk called UnCache on the previous one, destroying the buffer even when other streams were still inside it; - since #11384 a buffer is dropped once any reader has consumed it to the end and no read call is in flight. Streams copy out in slices (256 KiB in the S3 gateway), so between two calls a slower stream is not attached and loses the buffer to a faster one. Either way the slower stream refetches the whole chunk from the volume servers. With many clients downloading the same popular object at once, each chunk is fetched over and over; in production we saw the S3 gateway pull ~10 Gbit/s from volume servers while serving ~1 Gbit/s to clients. A ChunkReadAt now pins the chunk it is positioned in. The pin is taken and released only under the ReaderCache lock, since concurrent ReadAt calls on one ChunkReadAt (as in mount) share it. It is released when the stream reads the chunk to its end, moves to another chunk (including one served from the chunk cache), or falls back to random reads. A buffer is dropped once no stream pins it and no read is in progress, if it was consumed or its last stream left it; a read still in flight when the stream leaves drops it on detach, as UnCache did via destroy. Eviction by slot limit and memory budget is unchanged. lastChunkFid is now guarded as well: concurrent ReadAt calls raced on it. Tests: two ChunkReadAt instances streaming one object in interleaved slices fetch each chunk exactly once (2-3 times before); leaving a chunk for a chunk-cache hit or while another read is in flight releases it; concurrent ReadAt calls on one ChunkReadAt leave no pins behind under -race. |
||
|
yi111andChris Lu
|
4fec65d949 |
filer: demote client-cancelled directory listing log from error (#11495) (#11496)
* filer: demote client-cancelled directory listing log from error (#11495) * filer: quote path in canceled listing log --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> |
||
|
Chris LuandDevin
|
f31a026b2a |
master,filer: fix lock ring poisoning after leader change (#11453)
* cluster: never broadcast an empty lock ring An empty member list is never a usable ring state, but a delayed RemoveServer on a former leader can fire after the new leader already broadcast the recovered ring. That late broadcast carries a newer wall-clock version, so clients accept the empty ring and permanently reject the good one. Skip the broadcast entirely when the member list is empty, keeping the last non-empty snapshot for reconnecting clients. * cluster: periodically rebroadcast the lock ring Ring updates are purely event-driven, so one lost or poisoned update is permanent until the next membership change — with a single filer that may never come. Re-arm a per-group timer after every broadcast so the current leader keeps re-sending the ring; clients reject nothing newer than their last accepted version, so a re-sent snapshot always heals a stale view. * filer,s3api: reset the lock ring on master change Ring versions are per-master monotonic — each master stamps wall-clock nanoseconds — so a late high-version update accepted from a former leader makes the new leader's snapshot look stale forever. Detect a leader change across the reconnect gap (currentMaster is cleared between attempts, so remember the last served master) and reset the ring to bootstrap state so the new leader's view always applies. * cluster: fail lock acquisition when no lock server exists retryUntilLocked loops forever, so a filer reporting an empty lock ring wedges every append write indefinitely. Bound only the "no lock server found" case — ordinary contention is still waited out since the holder releases eventually. The constructors now return nil on failure: the filer append path and S3 object writes fail fast, while mounts degrade to their existing lockless mode. * cluster: reset only the ring version on master change Ring versions are per-master monotonic, so a version gate reset is all a leader change needs. Clearing the whole ring made every filer its own write owner until the next update and dropped the prior-owner window for keys the new leader remaps; the last ring now keeps routing until the new leader's snapshot transitions off it. * cluster: skip redundant ring installs and defer rebroadcasts An unchanged member list now only bumps the accepted version instead of installing a snapshot: periodic rebroadcasts no longer fire the topology-change callback or restart the prior-owner window. And a rebroadcast that lands inside a membership stabilization window yields to the pending timer rather than publishing an intermediate ring. * cluster,mount: bound lock unavailability, fail ops that cannot lock Only 'lock already owned' contention retries without bound now; every other failure — no lock server, or a dead ring member refusing connections — shares the same unavailability budget, so a ring naming departed filers can no longer hang a lock forever. Mount open-write, create, and rename fail with EAGAIN when the required lock cannot be acquired instead of proceeding without cross-mount serialization. * cluster: check pending stabilization inside the broadcast critical section rebroadcast released the mutex between the pending-timer check and nextBroadcastUpdate, so a membership change arriving in the gap could arm a stabilization timer while the rebroadcast emitted an intermediate ring. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * mount: acquire path locks before mutating create/rename state Create took the DLM lock only after the filer create, so a lock failure returned EAGAIN with an eagerly persisted file left behind. Rename marked source handles renamed before acquiring locks, so a failed acquisition left them suppressing old-path flushes for a rename that never happened. Both now take the locks first; the create's lock is released again if the entry race loses to another creator and AcquireHandle takes over. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * mount: keep the old-path lock when rename lock migration fails The migration stopped the handle's lock before acquiring the replacement, so a nil result left the handle writing with no lock at all. Acquiring the new-path lock first means failure keeps the existing lock instead of reporting success with serialization dropped. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * mount: skip new-path rename lock when a handle already holds it A target file open for write on this mount already carries a lock on newPath; the lock manager does not grant a second lock to the same owner, so the rename would wait on itself until the handle closed. Also avoid locking twice when old and new paths coincide. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * mount: hand the rename's target lock to the migrating handle The rename holds a lock on newPath for its duration, so the response migration's fresh acquisition waited on that same lock until the handle released — under fhLockTable, blocking the handle's own close. Adopt the rename's lock directly; nested move responses still acquire their own. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * mount: move the replaced target's lock to the renamed handle When the target path was already locked by an open handle on this mount, the migrated source handle kept only its stale old-path lock — the target's close would then release the last lock on the new path while the renamed handle was still open. Adopt the replaced handle's lock instead. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * mount: stop the handle lock inside the fh lock on release ReleaseHandle stopped fh.dlmLock before taking the fhLockTable slot, so a rename migration holding that slot could still observe and adopt a lock that was already stopping. Stopping under the fh lock makes the transfer serialize against the release. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * mount: claim the replaced target's lock for the renamed handle When the target path is already locked by an open handle on this mount, adopting it at migration time keeps the renamed path protected after that handle closes, without waiting on a lock this mount already holds. If the handle was released mid-migration the claimed lock is stopped, and a fresh acquire covers the case where it was already gone. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * mount: claim the target handle's lock before the rename runs Skipping the new-path lock when a handle already holds it let that handle's close release the lock mid-rename, leaving the path unguarded until the response migrated it. Take over the lock at check time and hold it for the rename's duration: the response adopts it for the migrating handle, or it returns to the target handle / is released on failure. The target handle lookup also falls back to the entry's stored inode for a forgotten path mapping. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * mount: read handle locks only under the fh lock during rename The loose dlmLock reads raced ReleaseHandle, which now mutates the lock inside the handle lock; check and claim it under the same hold. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
2f6c237238 |
filer: keep lazy remote reads from resurrecting deleted paths (#11452)
* filer: keep lazy remote reads from resurrecting deleted paths Under a remote mount with filer.remote.sync as write-back, a path that was deleted or renamed away could come back as a chunkless remote-only entry: between the local delete and the daemon's remote delete, a store miss made maybeLazyFetchFromRemote trust a bucket that was behind the filer. The ghost then outlived the remote object -- HEAD answered 200, GET failed, and nothing cleaned it up. The filer now tombstones paths it deletes under a remote mount, learned both synchronously from its own delete path and from peer metadata events. The lazy fetch and the lazy listing skip a tombstoned path until the path is written again, until the mount's persisted write-back sync offset has passed the delete event (the remote delete has landed), or until a generous TTL covers a mount without a daemon. Fixes #11440 Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: cover recursive remote deletes with an ancestor tombstone A recursive delete now records the directory tombstone before walking children, so a partial traversal or a store that drops the subtree without listing it still leaves every descendant covered. Directory tombstones also subsume older descendant entries on add, descendant adds covered by a standing ancestor are skipped, and an existing tombstone can be refreshed even at capacity. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: scope remote tombstones to the deleted object's generation A remote object whose own mtime postdates the local delete is a new generation, not the one the tombstone hides, so a recreated directory can surface remote writes made after its delete while old-generation objects stay hidden. Lazy fetch now stats the remote object before deciding, listings pass each child's remote mtime, and a sync offset releases a tombstone once it reaches the delete's own timestamp. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: rebuild remote deletion tombstones after restart In-memory tombstones are lost on restart while remote write-back offsets persist, so a filer boot replays the persisted metadata log from the oldest mount offset and folds deletes back into the tombstone set through the same event handler. Lazy remote reads hold off while the replay runs so a pending delete cannot resurrect in the gap. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: release remote tombstones only after their delete event lands The write-back offset orders against event timestamps, but the synchronous delete path recorded tombstones with the local clock before its event was emitted — a later unrelated event could already have pushed the mount's watermark past that guess, releasing the tombstone before the daemon applied the delete. Tombstones recorded ahead of their event are now marked pending and can only be lifted by the event confirming them or by TTL; event-stamped tombstones release through the offset as before. The remote-mtime generation bypass is dropped: remote and filer clocks are independent, and a pending remote delete removes whatever object sits at the path, so a "newer" remote object would only resurrect as a phantom. Tombstoned lookups now skip the remote stat entirely. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: drop dir tombstone when recursive delete fails before listing The ancestor tombstone is recorded before the child listing; if that listing fails nothing was deleted, and the leftover tombstone would hide still-existing remote children for the whole TTL. Tombstones for children already deleted stay, since their remote deletes are still owed. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: block lazy remote reads on startup tombstone rebuild The rebuild gate is now a done-channel set synchronously before the replay goroutine starts, so no lazy read can slip through in between. Reads wait on it with context cancellation instead of returning an empty miss that makes remote-only objects look deleted. The replay start is floored at now-TTL: mounts without a recorded write-back offset previously replayed the whole persisted history, and events older than the TTL would only build already-expired tombstones. The gate check now runs after the mount lookup so replaying the meta log's own directory listings does not deadlock on the gate, and the replay retries with backoff until it succeeds instead of failing open. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: mark restamped tombstone pending until its delete event lands When a local delete raises an existing tombstone's timestamp, the new value is only a local clock guess ahead of that delete's event. Leaving the tombstone un-pending lets a write-back offset release it before the event is actually consumed, reopening the resurrection window. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: bound tombstone replay to the tombstone TTL Persisted-log replay retried forever, keeping lazy remote reads gated indefinitely when the log cannot be read. Cap retries at the tombstone TTL measured from replay start: past that point every tombstone would have expired anyway, so opening the gate loses no protection. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: re-check deletion tombstone before persisting lazy fetch A delete landing while StatFile is in flight passed the earlier tombstone check but still persisted the fetched entry, resurrecting a path whose remote delete is pending. Re-check right before CreateEntry. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: retract a lazily persisted entry when a delete raced the insert The pre-insert tombstone check still leaves a window between the check and the store insert. Since deletes always record the tombstone before removing the entry, a tombstone visible right after a successful insert means the delete already ran: delete the entry back out so the tombstoned path stays deleted. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: note why the replay deadline can safely open the gate Deletes made after startup are captured by the live delete and event paths, so a stalled replay can only be missing pre-restart deletes, all of which are past the tombstone TTL by the deadline. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: retract only the entry a lazy remote read materialized Deleting by path after a raced delete could remove a legitimate rewrite that replaced the fetched entry. Verify the stored entry still matches the remote object (or the just-created directory shape) before deleting, and apply the same post-insert check to lazy listing children. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer: require full-entry equality before retracting a lazy entry Remote-only matching still removed a write that had updated the fetched entry, e.g. appended chunks. Compare the persisted entry against what this read materialized; any change means a real update owns the path. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
b750853c42 |
shell: refuse s3.bucket.create on an existing bucket (#11455)
* shell: refuse s3.bucket.create on an existing bucket CreateEntry without o_excl replaces the bucket entry, dropping every extended attribute: lifecycle configuration, owner, versioning and the irreversible Object Lock flag. Send o_excl so a re-run fails with 'bucket already exists' instead of silently resetting the bucket. * filer: fail exclusive creates when the lookup itself fails CreateEntry discards FindEntry errors, so an o_excl create hitting a transient store failure would take the insert path and upsert over the entry it was meant to preserve. Propagate the lookup error when o_excl is set; non-exclusive creates keep their existing semantics. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * shell: test s3.bucket.create requests an exclusive create Exercises the command end to end through a fake filer gRPC server and asserts the OExcl flag reaches the wire along with the already-exists error path. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * shell: synchronize captured requests and assert the exact bucket error The fake filer records CreateEntry requests on the gRPC server goroutine, so reads need the same mutex; the test also now checks for the exact "bucket my-bucket already exists" message rather than any error that mentions existence. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris Lu
|
11791fad6a |
filer: resolve the collection a bucket delete drops (#11439)
* filer: resolve the collection a bucket delete drops A bucket delete dropped the collection named after the bucket, which assumes bucket name is collection name. With a collection rule the write path honors, deleting the bucket either orphaned its collection or, when a bucket was named after a shared collection, removed volumes other buckets still write to. Resolve the collection through the same rule chain the write path uses and drop it only when no other bucket resolves there too. A listing failure keeps the collection, the safe side of an unknown. * filer: prove collection exclusivity across all paths before dropping it The sibling-bucket scan missed every non-bucket writer: a broad rule like '/' or '/buckets/', a rule under a surviving bucket, or a rule on an unrelated path can route into the same collection. Check every storage rule's prefix instead, and mirror the grouped gateway's explicit <group>_<bucket> collection, which otherwise resolves a rule-named collection the bucket never wrote to. * s3: let the filer own the collection decision on bucket delete Both entry points deleted a name-derived collection around the filer's own resolved delete, bypassing its exclusivity check and wiping sibling data. The filer now resolves the collection a bucket actually used, including the grouped form. * filer: keep a collection the default write route also uses Rule-less writes outside buckets land in the filer's default collection, so a bucket resolving there shares it with them. |
||
|
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. |
||
|
Chris Lu
|
bdc37a1e86 |
mount/shell: bucket allow-empty-folders toggle, mount keeps explicit false (#11370)
* mount: keep a deliberate bucket allow-empty-folders setting * shell: s3.bucket.allowEmptyFolders toggles the empty folder cleaner * shell: guard allow-empty-folders toggle with expected extended attrs * filer: drop cached empty-folder policy on bucket entry update * mount: guard allow-empty-folders write with expected extended attrs * filer: skip caching a stale cleanup policy read across an update * mount, shell: snapshot the full extended attributes for update preconditions * filer: fail closed and invalidate on all bucket entry events for cleanup policy * filer: key the cleanup policy generation by bucket * filer: requeue cleanup when the bucket policy cannot be loaded * filer: expire idle cleanup policy generations * filer: skip requeueing cleanup after the cleaner stops * filer: bound cleanup retries on repeated policy failures * filer: cover cleanup requeue on repeated policy failures * filer: keep cleanup policy generations while reads are in flight * filer: exercise the cleanup queue lifecycle in the retry-cap test |
||
|
Chris Lu
|
3ebc05930d |
s3: separate Object Lock configuration permission (#11361)
* s3: separate object lock configuration permission * test: synchronize manifest cancellation setup |
||
|
David Christopher
|
a859f0a019 |
filer: preserve accepted metadata log records on shutdown (#11359)
fix: flush metadata log before closing filer store Serialize sealed-batch handoffs with shutdown, reject late appends, and wait for log-buffer workers before closing the filer metadata store. Cover queued writes, interval and explicit flushes, late-write rejection, and pending persistence with shutdown tests. |
||
|
dependabot[bot]andChris Lu
|
01433e801d |
build(deps): bump github.com/redis/go-redis/v9 from 9.21.0 to 9.22.0 (#11306)
* build(deps): bump github.com/redis/go-redis/v9 from 9.21.0 to 9.22.0 Bumps [github.com/redis/go-redis/v9](https://github.com/redis/go-redis) from 9.21.0 to 9.22.0. - [Release notes](https://github.com/redis/go-redis/releases) - [Changelog](https://github.com/redis/go-redis/blob/master/RELEASE-NOTES.md) - [Commits](https://github.com/redis/go-redis/compare/v9.21.0...v9.22.0) --- updated-dependencies: - dependency-name: github.com/redis/go-redis/v9 dependency-version: 9.22.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * test(redis_conf): track go-redis 9.22.0 default read timeout of 5s go-redis 9.22.0 raised the default ReadTimeout from 3s to 5s (part of the cross-SDK configuration alignment). Update TestUnsetKeepsGoRedisDefaults to expect the new default so the bump in #11306 stops failing CI. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Bruce ZouandChris Lu
|
ea179963c0 |
filer: clean up manifest resolve error propagation and add webdav tes… (#11297)
filer: clean up manifest resolve error propagation and add webdav test (#78) Drop GitHub issue references from comments and trim verbose comments. Replace the viewFromChunksOrErr helper with the existing NonOverlappingVisibleIntervals + ViewFromVisibleIntervals at the stream call sites, and add a WebDavFile.Read regression test for the manifest resolution failure path. Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> |
||
|
Chris Lu
|
38c14d3c13 |
filer: apply SSRF guard to the lazy-remote fetch/list/delete paths (#11294)
* filer: add guarded remote-storage client builder hook for lazy fetch The lazy-remote fetch path (maybeLazyFetchFromRemote) resolved its remote-storage client through the unguarded shared cache, bypassing the SSRF chokepoint (BuildGuardedRemoteStorageClient) that the CVE-2026-73080 remediation wired into the volume, filer stream and s3 stream dial paths. Add a RemoteStorageClientBuilder hook on Filer plus conf-only lookups on FilerRemoteStorage, and route the lazy fetch through the builder when set (endpoint deny-list + DNS-rebinding-safe dialer), falling back to the shared cache otherwise. The filer server wires the builder in a follow-up. * filer: route lazy directory listing through the guarded remote client maybeLazyListFromRemote shared the unguarded client resolution of the fetch path, so a caller-supplied remote endpoint was dialed without the SSRF deny-list or rebinding-safe dialer. Resolve the conf and build the client through buildRemoteStorageClient so the same guard covers listing. * filer: route lazy remote delete through the guarded remote client maybeDeleteFromRemote issued outbound DELETE/RemoveDirectory requests through the unguarded client, giving a write-side SSRF to a caller-chosen endpoint. Resolve the conf and build the client through buildRemoteStorageClient so the endpoint deny-list and rebinding-safe dialer apply to the delete path as well. * filer server: wire the guarded remote client builder into the filer Set Filer.BuildGuardedRemoteClient to BuildGuardedRemoteStorageClient and forward AllowUntrustedRemoteEndpoints so the lazy-remote fetch, list and delete paths apply the same SSRF endpoint checks as the volume and streaming read paths. * filer: test lazy fetch honors the guarded remote client builder Add a regression test that sets BuildGuardedRemoteClient to a rejecting builder and asserts maybeLazyFetchFromRemote returns no entry without reaching the remote, covering the SSRF guard wired in the prior commits. * filer: skip remote client for local-only lazy deletes maybeDeleteFromRemote resolved and validated the mount's remote client before checking entry.Remote, so a local-only file (no Remote entry) under a mount whose endpoint the guard rejects failed to delete: the guard error aborted the metadata deletion, leaving a file that needs no remote operation undeletable. Move the local-only check ahead of client construction so only remote-backed files and directories pay the guard. * filer: build the guarded remote client inside the lazy singleflight The lazy fetch and list paths built the guarded client before their singleflight blocks, so concurrent requests for the same key each allocated a fresh SDK client and HTTP transport even though only one remote operation ran. Move client construction inside the singleflight so the deduplicated operation builds it once, matching the per-request guard semantics of the sibling streaming paths without the duplicate transport churn. * filer: test guarded rejection for the lazy list and delete paths Add regression tests that set BuildGuardedRemoteClient to a rejecting builder and assert the lazy list does not reach the remote, a remote-backed file delete is blocked, and a local-only file under a rejected mount still deletes (covering the local-only fix). * filer: decouple lazy guarded-client build from the first caller's context Building the guarded client inside the singleflight made concurrent fetches share the first caller's context. If that caller canceled while endpoint DNS validation was running, the builder returned an error and published a not-found result to other callers whose contexts were still valid. Build with context.WithoutCancel so the guard's DNS validation is not tied to any single caller's cancellation, matching the list path's existing decoupling for the remote operation itself. * filer: reject remote-storage confs that dial blocked endpoints at load The filer's lazy-fetch / lazy-list / remote-delete paths resolve remote storage clients by name from FilerRemoteStorage.storageNameToConf and dial them via remote_storage.GetRemoteStorage, which bypasses the SSRF deny-list the volume server (BuildGuardedRemoteStorageClient) and the filer's own direct-read path apply. A RemoteConf planted under /etc/remote with a loopback / private / IMDS S3 endpoint is reloaded into storageNameToConf on the next metadata-change event and then dialed on the next cache miss — server-side request forgery from the filer. Apply the volume server's SSRF deny-list at conf load time, the single chokepoint that populates storageNameToConf: - Add RemoteStorageConfValidator, injected into FilerRemoteStorage by the filer server (the filer package cannot import the server package). A conf that fails validation is dropped from storageNameToConf, so the name-based client resolution on the lazy paths returns "not found" instead of dialing the blocked endpoint. - Add ValidateRemoteConfForLoad in weed_server, which mirrors BuildGuardedRemoteStorageClient's gcs credential + endpoint checks (validateRemoteEndpoint via guardedRemoteClient) without building a client. allowUntrusted skips the check, mirroring the volume server opt-out (-filer.allowUntrustedRemoteEndpoints). - The filer server injects the validator at construction. A conf whose type dials a fixed provider host (no caller-supplied endpoint) passes; only caller-influenced endpoints are denied. * filer: skip DNS resolution in the load-time SSRF validator ValidateRemoteConfForLoad resolved hostnames during /etc/remote reload, so a transient DNS failure (2s timeout) dropped the conf from the fresh map that replaces the live map, disabling a working mount until the next metadata event. The build-time guard (BuildGuardedRemoteStorageClient) already re-resolves and re-validates the endpoint at dial time with the rebinding-safe dialer, so DNS at load is redundant for security. Split the static checks (scheme, IMDS hostnames, IP-literal blocked addresses, gcs credentials) into validateRemoteEndpointForLoad, which does no DNS. Hostname endpoints pass at load and are caught at dial if they resolve to a blocked address. This preserves fail-fast for statically-blocked confs (loopback IPs, IMDS hostnames) without letting transient DNS failures disable mounts. * filer: accept empty S3 endpoints in the guarded remote client builder guardedRemoteClient returned ok=true with an empty endpoint for a standard AWS S3 config (no custom S3Endpoint), so BuildGuardedRemoteStorageClient and ValidateRemoteConfForLoad rejected it with "remote endpoint is empty" — breaking standard AWS S3 mounts on the lazy paths and the sibling streaming read paths that already use the guarded builder. An empty endpoint is not caller-supplied: the AWS SDK derives the regional endpoint from the region, so there is nothing for the SSRF guard to validate. Return ok=false for empty S3-compatible endpoints so the builder falls through to the shared unguarded cache, matching the historical behavior for standard AWS S3. |
||
|
|
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> |
||
|
Bruce Zou
|
eb6a7e93ca |
Fix mount eio on manifest resolve failure (#11287)
* mount: fail reads with error when chunk manifest resolution fails When SetChunks fails to resolve a chunk manifest (e.g. the volume is on a remote tier with reads disabled), the sections map stays empty and readDataAtSequential/readDataAtParallel zero-fill every missing section as if it were a sparse hole. Reads then return all-zero data with no error, so a plain cp of a large manifest-based file silently produces a completely zero-filled file. Remember the resolve error in ChunkGroup (guarded by sectionsLock) and return it from ReadDataAt. A later successful SetChunks clears it. Fixes the mount path of #11286. * filer: propagate manifest resolve errors in streaming read paths ViewFromChunks discards the chunk manifest resolve error returned by NonOverlappingVisibleIntervals. On failure the chunk views come back empty, and the streaming paths zero-fill the entire requested range, serving HTTP 200 / WebDAV 200 responses whose body is all zeros. Propagate the error in PrepareStreamContentWithThrottler, PrepareStreamContentWithPrefetch and the WebDAV read path so these requests fail with 500 instead. Fixes the filer HTTP and WebDAV paths of #11286. * mount: fail lseek with EIO when chunk manifest resolution fails SearchChunks still consulted the stale section map after SetChunks recorded a manifest resolution failure, so SEEK_DATA/SEEK_HOLE would describe the unresolved regions as sparse holes or return ENXIO. Return the recorded error from SearchChunks and map it to EIO in Lseek. Also add regression tests for the stream preparation error paths. Addresses review feedback on #11287. |
||
|
Konstantin AdzerandChris Lu
|
42b0ca7850 |
s3 sink: report the source read error the SDK hides (#11277)
* s3 sink: report the source read error the SDK hides filer.backup stops for good on an event whose chunks are gone from the volume servers: the uploader reads the body, the read fails with the volume's 404, and the AWS SDK returns "ContentLength=N with Body length 0" without the cause. isIgnorable404 would skip such an event, but it never sees the 404, so the event is retried forever and the checkpoint never advances. ChunkStreamReader keeps its first source failure and the s3 sink returns it when the upload fails. * s3 sink: trim verbose comments on source error propagation --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
|
2cd6c36c54 |
filer: end local-only metadata subscriptions when remote peers appear (#11251)
* filer: end local-only metadata subscriptions when remote peers appear SubscribeMetadata delegates to SubscribeLocalMetadata whenever the MetaAggregator knows no remote peers at stream setup. Peer discovery is asynchronous with the gRPC server accepting streams: the master announces filers after Filer.Init, via ListExistingPeerUpdates and OnPeerUpdate. A subscriber that connects inside that window is pinned to a filer-local stream for its whole life, silently missing every other filer's writes. For filer.remote.sync in a multi-filer cluster this means the remote tier permanently stops receiving writes served by other filers (#11247). End the delegated local stream when the first remote peer appears, so the client reconnects into the aggregated stream. The end surfaces as an error, not a clean EOF: RetryUntil-driven followers (mount meta cache, s3api IAM) treat a clean end as following finished and stop reconnecting. The arrival channel is armed under the same lock as the peer check in RemotePeerArrivedChan, so a peer learned in between sends the stream straight to the aggregated path instead of parking on a channel that would never fire. A standalone filer is unaffected: no peer ever appears, the channel never fires, and the local stream serves indefinitely. Fixes #11247 * filer: interrupt disk replay on peer arrival, trim comments Check upgradeOnRemotePeer inside eachLogEntryFn and chunkDiskPass so a peer arriving during a backlog replay stops the stream before the cursor advances past older remote events. Wrap errAggregationUpgrade with StopReadingError so LoopProcessLogData does not log it. Remove issue references from comments and trim verbose commentary. * filer: check upgrade signal between ref batches Pass upgradeOnRemotePeer to sendRefsBatched so a peer arriving while refs are shipped to a slow client is detected between batches, not only after the full batch completes. * filer: interrupt gap park on peer arrival Pass upgradeOnRemotePeer through gapPass to parkOnGap so a peer arriving during a gap park ends the stream immediately instead of waiting for the retry timer (up to one minute). --------- Co-authored-by: Tyagiquamar <Tyagiquamar@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Lu
|
723f473f02 |
filer: widen metadata-subscription readahead buffers (#11237)
The metadata-subscription readahead channels were sized for a low-throughput era and now bottleneck replay catch-up: - ReadPersistedLogBuffer's readaheadSize was 1024 entries: the background visitor fills the channel, then blocks on the consumer's gRPC Send, so volume-server I/O for the next log file never overlaps with delivery of the current one. Each disk pass takes longer, and the subscribe loop re-lists log files (ListDirectoryEntries on the filer store) more often to drain the same backlog. Raised to 8192 so the reader stays ahead of the consumer through a full log file's worth of entries. - readFilersMerged's logEntryChannelSize was 512 entries per filer stream: the same serialization on the client side, where weed mount (chunk mode) reads persisted log chunks directly from volume servers. A small channel means the producer stalls on the merge consumer's processEventFn, and the next log file's chunks are never fetched ahead. Raised to 4096 so volume I/O overlaps with event delivery. The wider buffers keep the producer goroutines reading through a full log file while the consumer is still processing the previous one, turning serial read→process→read into pipelined read∥process. This cuts the per-pass wall time that drives filer store listings and volume-server round-trips, reducing filer workload under backlog catch-up (e.g. CSI deployments where ~200 mounts reconnect on filer restart). |
||
|
Chris Lu
|
cb9fcd39d2 |
filer/postgres: create filemeta table on startup via createTable config (#11229)
* filer/postgres: create default filemeta table on startup The postgres filer store hardcoded CreateTableSqlTemplate to empty and never created the filemeta table, unlike postgres2/mysql2/sqlite which all create it during Initialize. Users had to create the table manually or the filer would crash loop with "relation filemeta does not exist". Read the createTable config option (same as postgres2), default to DefaultCreateTableQuery when unset, and execute CREATE TABLE IF NOT EXISTS on the default table after the connection pool is established. SupportBucketTable stays false so per-bucket table creation remains a no-op; only the shared filemeta table is created, via a direct ExecContext since AbstractSqlStore.CreateTable short-circuits without bucket support. * filer/postgres: accept boolean createTable = true/false viper reads a TOML boolean as the string "true"/"false" via GetString, so createTable = true was being used as a SQL template and failed. Add ResolveCreateTableQuery to normalize the value: true and empty select the default template, false disables table creation, anything else is a custom template. Both postgres and postgres2 now use it, and both skip the CREATE TABLE call when the resolved template is empty. * scaffold: document createTable option for postgres filer store Replace the commented-out CREATE TABLE SQL in the [postgres] scaffold with a createTable config hint, matching the [postgres2] section. Users no longer need to manually create the filemeta table before starting the filer. * filer/postgres: make createTable opt-in for postgres, keep postgres2 default The previous commit defaulted postgres to create the filemeta table even when createTable was unset, which could break existing deployments whose DB user lacks CREATE TABLE privileges. ResolveCreateTableQuery now returns empty for an unset value so postgres only creates the table when createTable is explicitly true or a custom template — preserving the prior no-DDL behaviour for existing configurations. postgres2 keeps its existing always-create default: it defaults an empty resolved value to DefaultCreateTableQuery, and only skips when createTable is explicitly false. * filer/postgres2: simplify createTable handling, document all modes Drop the false opt-out from postgres2 — it only skipped the default table while per-bucket CreateTable still ran, leaving restricted DB roles broken on bucket access. postgres2 now accepts true the same way (defaulting to DefaultCreateTableQuery) and keeps its existing always-create behaviour for every other value, matching the original semantics. The scaffold comment now documents true/false/custom for the postgres section so users know false (or unset) is the backward-compatible default. * filer/postgres2: normalize false via ResolveCreateTableQuery postgres2 only handled "" and "true", leaving createTable = false as the literal string "false" which CreateTable then executed as invalid SQL. Route it through ResolveCreateTableQuery (which maps false to empty) and default the empty result to DefaultCreateTableQuery, so false is treated the same as unset for the bucket-aware store. * filer/postgres2: honor createTable = false for default table postgres2 treated false the same as unset and always created the default filemeta table, failing startup for restricted DB roles that explicitly opted out. Track the original false value before ResolveCreateTableQuery collapses it to empty, and skip the default CreateTable call when set. Per-bucket table creation is unaffected — it is a runtime requirement of the bucket-aware store. Users who need to suppress all DDL should use the postgres (non-bucket) store with createTable unset. * filer/postgres2: disable bucket tables when createTable = false Setting SupportBucketTable = false when createTable is explicitly false makes AbstractSqlStore.CreateTable a no-op (it already returns nil when SupportBucketTable is false), so neither the default filemeta table nor per-bucket tables are created. The template stays empty and no DDL runs, honouring the opt-out for restricted DB roles. All data routes to the pre-provisioned filemeta table, matching the postgres (non-bucket) store. * filer: suppress DDL without disabling bucket routing Setting SupportBucketTable = false when createTable = false also disabled per-bucket routing, hiding objects in pre-provisioned per-bucket tables. Keep SupportBucketTable true and instead skip the CREATE TABLE execution when the resolved template is empty. GetSqlCreateTable now returns empty for both postgres and mysql SQL generators when CreateTableSqlTemplate is empty, and AbstractSqlStore.CreateTable skips the ExecContext call when the SQL is empty. This preserves bucket routing while suppressing all DDL for users who explicitly set createTable = false and pre-provision their tables. * filer: add SkipDDL to suppress CREATE and DROP without disabling routing createTable = false with SupportBucketTable = true preserved bucket routing but deleteTable still executed DROP TABLE on bucket deletion, dropping externally managed tables. CanDropWholeBucket also returned true, so the S3 layer tried whole-table drops instead of row-by-row deletes. Add a SkipDDL flag to AbstractSqlStore, independent of SupportBucketTable. CreateTable and deleteTable both skip when SkipDDL is set, and CanDropWholeBucket returns false so bucket deletion falls back to row-by-row metadata deletes. postgres2 sets SkipDDL when createTable is explicitly false — bucket routing is preserved, no DDL runs. * filer: fall back to row-by-row delete when CanDropWholeBucket is false DeleteFolderChildren took the whole-table drop path whenever the path was a bucket root, even when SkipDDL made deleteTable a no-op. The no-op returned nil, the caller returned early, and rows inserted after the recursive enumeration survived the bucket deletion. Gate the whole-table drop on CanDropWholeBucket so the row-by-row DeleteFolderChildren SQL runs when SkipDDL is set, removing all metadata without issuing DROP TABLE. |
||
|
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. |
||
|
ssshr-66
|
c6b330be2b |
[Mount] Add ChunkGroup seeking tests and fix boundary handling (#11223)
* fix issue 11221 * reply ai comments |
||
|
ssshr-66andChris Lu
|
361fd6b263 |
[Filer] Parallelize Chunk Manifest Resolution to Reduce Large File Read Latency (#11215)
* fix issue-11214 * fix(filer): cancel sibling manifest reads on failure * fix(filer): scope manifest cancellation to read batch and propagate context to encrypted reads Address PR review comments on #11215: - Scope cancellation to each parallel read batch instead of the resolver-wide context, so a later manifest failure does not cancel recursive work for an earlier successful manifest (CodeRabbit #3952446554). - Propagate the resolver context through GetAuthenticatedWithContext so encrypted sibling reads observe cancellation and stop promptly when another manifest fails (Greptile #3952422343). - Use net.ListenConfig.Listen with an explicit context in the test fixture to satisfy the noctx linter (CodeRabbit #3952111696). - Add regression tests for encrypted sibling cancellation and for preserving earlier manifest children on later failure. * fix(filer): return known manifest errors without blocking on earlier children Address Greptile review comment on #11215: After all parallel reads complete, pre-scan slots for the first real (non-internal-cancel) error before recursing into earlier manifests' children. If a later manifest already failed, return its error promptly with data chunks already in hand, instead of blocking on recursive network reads of earlier manifests' children. Updated the regression test to verify the error returns within 1 second when an earlier manifest's child has a 2-second delay, and that the child is never loaded. * fix(filer): filter partial child chunks by requested range on error path Address CodeRabbit review comment on #11215: The pre-scan error path appended non-manifest child chunks from earlier manifests without applying the [startOffset, stopOffset) overlap check used for top-level chunks. A child outside the requested range could be returned in dataChunks alongside the later manifest's error. Apply the same range predicate before appending. Add regression test with an out-of-range child chunk. * fix(filer): buffer job channel and abort submission on batch cancellation Address Greptile review comment on #11215: - Use a buffered job channel (capacity 128) so submission does not block when all workers are busy. This ensures a promptly-failing manifest is always queued and can cancel stalled sibling reads once a worker picks it up, instead of blocking the caller on the unbuffered channel send. - Add batchCtx.Done() to the submit select so submission aborts promptly when the batch is already cancelled by a sibling failure. - Add regression test with 5 manifests (4 stalled + 1 failing) verifying the failing job is queued and picked up after a stalled worker is freed. * fix(filer): avoid double WaitGroup decrement on batch cancellation in submit Address Devin review comment on #11215: When batchCtx.Done() fired in submit, it called job.done.Done() and returned false. The caller in resolve also called reads.Done() on the same WaitGroup, causing a double decrement that would panic with a negative counter. Fix: submit sets the result error but does not decrement the WaitGroup. The caller always owns the decrement and skips overwriting the result when submit already set it. * fix(filer): overflow execution when job queue buffer is full Address Greptile follow-up review comment on #11215: With a 128-entry buffer, if more than 132 in-range manifests (4 workers + 128 buffer) stall at one level, a promptly-failing manifest beyond the buffer cannot be submitted and cannot cancel the stalled reads. Fix: when the buffer is full, run the job directly in a goroutine instead of blocking on the channel send. This only triggers for >132 manifests at one level (exceedingly rare), so the bounded concurrency guarantee (4 workers) holds for all normal workloads. Extracted executeJob method shared by both workers and overflow goroutines. * fix(filer): bound overflow execution with a semaphore Address Devin review comment on #11215: The unbounded overflow goroutines could create thousands of concurrent reads for large files, defeating the four-worker resource bound. Fix: add a semaphore (capacity = maxChunkManifestResolveWorkers) that overflow goroutines must acquire before doing the read. While waiting for the semaphore, they also watch batchCtx and r.ctx so they exit promptly on cancellation. Total concurrency is now bounded to 2 * workers (4 workers + 4 overflow) in the degenerate case. * refactor(http): add ctx to GetAuthenticated signature instead of new function Reuse the existing GetAuthenticated name by adding ctx as the first parameter, matching the pattern of ReadUrl, ReadUrlAsStream, and RetriedFetchChunkData. Removes the GetAuthenticatedWithContext wrapper. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Lu
|
8a68337256 |
filer: pack SSE chunks into manifests (#11175)
* filer: pack SSE chunks into manifests * s3: resolve encrypted manifests before reads * s3: scope encrypted manifest resolution to ranges |
||
|
Eliah RusinandChris Lu
|
cfa8afec92 |
filer: guard FoundationDB 100KB value limit and pack earlier (#11161)
* filer: guard the FoundationDB value size limit, not the transaction limit An entry's whole chunk list is one FoundationDB value, and FDB caps a value at 100,000 bytes while a transaction may reach 10MB. UpdateEntry checked the transaction limit, so every entry between the two limits passed the guard and was rejected by FDB itself with error 2103 (Value length exceeds limit). The failure surfaced inside the store rather than at the guard, so the S3 layer dropped the connection and clients saw a network fault instead of an error. Check the value limit in UpdateEntry and KvPut instead, after gzip and before the transaction, with an error that names the limit it hit. The removed transaction-size constant guarded nothing else: DeleteFolderChildren batches by entry count. Refs #11158 * filer: fold at 500 chunks in the foundationdb build Manifest packing is what keeps a large file's entry small, but it only ran once a flat chunk list reached 10000 chunks. A FoundationDB value stops at 100,000 bytes and an entry's whole chunk list is one value, which at ~100 bytes per chunk record is about 1000 chunks -- so on FDB the write always failed before packing could help: a 3.3 GiB PutObject at the default -maxMB=4 was already past the limit. FoundationDB support is its own build (`go build -tags foundationdb`, shipped as its own image), so the batch is a build-time choice and needs no negotiation at run time. The tagged build folds at 500, every other build keeps 10000 and is untouched. 500 is not arbitrary: a single fold level leaves (chunks/batch) manifest pointers plus up to (batch-1) unfolded chunks in the entry, so the reachable chunk count is highest when the two terms are near equal. For a 100,000-byte budget that optimum is 500, which holds an entry inside the limit up to ~250,000 chunks -- ~1 TB at -maxMB=4, against ~4 GB before. Larger files need nested packing, which no batch size substitutes for. One binary serves every role in that image, so the filer and each client that folds -- S3, mount, WebDAV, weed shell, filer.copy -- agree on the batch by construction. A binary built with the tag but pointed at another store folds earlier than that store requires, costing one manifest blob per 500 chunks and one read to resolve it. Fixes #11158 * filer: fold with rollback inside MaybeManifestize, not beside it A fold that fails midway has already uploaded manifest blobs for its earlier batches, and returns only the data chunks -- dropping the manifests it had separated out of the caller's list. Both were wrong in ways that mattered: - AppendToEntry assigned that shortened list straight to entry.Chunks and created the entry, so an append to an already-folded file whose fold failed lost every previously folded chunk. weed mount had the same shape. - cleanupChunks logged the error as "not good, but should be ok" and then returned it through a named result, failing the whole CreateEntry or UpdateEntry, while the blobs it had written stayed behind referenced by nothing. The S3 path was alone in handling this, through a private helper beside MaybeManifestize. A second entry point next to the one everything else calls just means the wrong one gets used, so the behaviour moves inside MaybeManifestize: on failure it returns inputChunks as it received them, and hands the blobs it saved to a deleteChunks callback. The filer, S3 and filer.copy pass their existing deleters -- filer.copy already cleans up this way after a failed upload -- and mount, WebDAV and weed shell pass nil, which reports the blobs rather than collecting them, as before. Each caller keeps its own error policy: the filer HTTP PUT path and filer.copy still fail the request, the rest still continue with the flat list, which is a correct entry. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Lu
|
a0b1272cc3 |
filer: authorize the chunk proxy and the root listing like the rest of the filer port (#11152)
* filer: require a read token for the root listing maybeCheckJwtAuthorization waved through every GET/HEAD on "/", so a filer with jwt.filer_signing.read.key set still served its root directory listing -- entry names, sizes and chunks[].file_id -- to a caller holding no token at all, and served the same listing to a token restricted by allowed_prefixes. The exemption was added for health checks before the filer had /healthz and /readyz. Both are registered on the default and read-only muxes ahead of the "/" handler and answer without a token, so drop it. Point the mTLS harness at /healthz, which is what it was probing for. * filer: keep the jwt query parameter out of a proxied chunk request The proxy stripped "jwt" from the forwarded query on reads only, on the grounds that a writer's own credential travels there. It does not: an uploader carries its AssignVolume token in the Authorization header, and the query parameter on this path holds a filer credential. Strip it for every method. A volume server has no business seeing a filer token, and because security.GetJwt reads the query before the header, relaying one would hide the writer's own token behind it. * filer: dispatch the chunk proxy after the JWT gate The ?proxyChunkId= branch returned before maybeCheckJwtAuthorization ran, so GET, PUT, POST and DELETE against any needle in the cluster were reachable on the filer's HTTP port with no filer credential, on a filer where every other request answered 401. An anonymous caller read a stored object, replaced its bytes, or deleted the needle, which the master's next vacuum makes permanent. #10434 stopped the filer from minting a volume write token for that caller, which closes the write half only where the volume server has a jwt.signing.key of its own -- not the shipped default, and not what scaffold/security.toml recommends for a filer deployment. The read half stayed open in every configuration, because the filer mints the read token itself. Move the dispatch below the gate. A file id carries no path, so a token restricted by allowed_prefixes cannot be scoped against one and is refused here; every consumer of this endpoint holds an unrestricted token. * filer: mint the volume credential for a proxied write too The proxy minted a volume token on reads and forwarded whatever the caller sent on writes. #10434 made it that way because the branch ran ahead of the JWT gate, so a token minted here would have been signed for an unauthenticated caller; the branch now runs behind the gate, and the credential the caller presents there is a filer one, which a volume server cannot validate and has no business seeing. Mint at the access level the request needs, and drop the caller's Authorization when there is no key to mint from. A proxied uploader then needs only the filer credential, instead of holding one for each hop with a single header to put them in. * mount, mq, filer.sync: send the filer credential for a proxied chunk Every in-tree consumer of ?proxyChunkId= reached the filer anonymously: mount and the broker put the AssignVolume token in the Authorization header, which is a volume credential, and filer.sync sent nothing at all. That was enough only while the branch ran ahead of the filer's JWT gate. Build the URL through one helper, and pick the credential from the URL it returns: a chunk proxied through a filer is a request to the filer, which authorizes it and attaches the volume credential itself, so the token there is a filer one at the access level the request needs. * filer: honor -exposeDirectoryData The flag was declared on all three commands that start a filer and read by none of them: FilerOption.ExposeDirectoryData was only ever assigned from filer.expose_directory_metadata in security.toml, so -exposeDirectoryData=false silently left the listing exposed. Only the TOML key had any effect. Plumb the flag through and let either switch turn the listing off. * filer: count a proxied chunk request once Moving the dispatch below the gate put it after the deferred request observation, so every proxied chunk now landed in FilerRequestHistogram twice, once under its HTTP method and once under chunkProxy. Name the deferred one after the proxy instead, the way the unsupported-method branch already does, which also gives the endpoint the status codes FilerRequestCounter records. |
||
|
Chris Lu
|
8112f2733a |
filer: batch exact lookup RPC, authoritative volume lookup, VolumeDelete status codes (#11122)
* storage: make DeleteVolume errors inspectable with errors.Is An absent volume wraps ErrVolumeNotFound and an only-empty refusal now wraps ErrVolumeNotEmpty with %w instead of %v, so callers no longer have to match on the message. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * volume server: return NotFound and FailedPrecondition from VolumeDelete An absent volume maps to codes.NotFound and a non-empty volume under only_empty to codes.FailedPrecondition, so a caller retiring a volume can treat NotFound as already done. The store message is kept in the status description because the EC empty-replica sweep still matches on it. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * wdclient: add LookupVolumeIdsAuthoritative Bypasses the vid map and asks the provider directly, for callers where a stale positive location is unsafe. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * filer: add LookupDirectoryEntries batch lookup RPC Up to 4096 exact-path lookups in one call, resolved concurrently with results in request order, plus one deduplicated location lookup for every volume the returned entries reference and per-fid read tokens when the filer signs reads. unavailable_volume_is_miss lets cache-style callers take an entry whose volume has no live location as a miss, resolved against the master rather than the filer's location cache. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * filer: test that an expired file entry is deleted on read Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * filer: test that AssignVolume and CreateEntry resolve the same TTL rule Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * master: refuse partial lookups while warming up LookupVolume returned Unavailable during warm-up only when every requested volume was missing. A batch mixing a reported volume with one whose server has not reconnected yet came back as a partial answer with a per-volume not-found, which a caller treating the master as authoritative reads as gone. Any not-found during warm-up is now Unavailable, which callers already retry. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * filer: build batch test requests instead of copying a proto message Copying a generated message copies its internal mutex, which go vet's copylocks check rejects. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * filer: match ErrNotFound with errors.Is and state the miss rule's contract A wrapped not-found from the store would otherwise be reported as an error rather than a miss. The comments now say why a nil location map is the only sign of an unanswered lookup: the provider returns nil when it got no answer and a populated map, with unserved volumes reported as errors, when the master did answer. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * volume server: map absent and non-empty VolumeDelete errors in the Rust server Matches the Go server: an absent volume is NotFound and an only_empty refusal is FailedPrecondition instead of Internal, with the messages the EC empty-replica sweep matches on. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm * filer: test that a malformed entry keeps its error outside cache mode Same test file as the enterprise tree, so the next sync sees one version. Claude-Session: https://claude.ai/code/session_01T4MEV3ETqFFKN46Uu2ZrUm |
||
|
Chris Lu
|
fe98520358 |
read: try a replica that stopped answering last, and relearn its volume's locations (#11130)
* http: try a volume server that failed to answer last A cached location list is shuffled on every read, so once a replica dies half the reads keep dialing it first and pay a connect failure or timeout before the healthy replica answers. Remember, per host, when a request got no answer at all and order such hosts last for the next half minute. Once that passes, one read probes the host in its usual place while the others keep it last until the probe settles, so a black-holed server costs one stalled read per interval instead of one per read. Nothing is ever skipped: a host that failed is still tried when the others fail too. Any response, including an error status, counts as reachable. Claude-Session: https://claude.ai/code/session_011NYXuzGttwrTMsfLYvmQFs * filer: refresh a chunk's locations after one of them fails A mount's location cache is only relearned when every cached location fails. When one replica dies and the other still answers, every read succeeds and the dead replica stays in the cache, and in the shuffled order it keeps being dialed first long after the master has dropped it. When a read fails on one location and a later one answers, call the refresh hook so the cached entry is dropped and looked up again. The read that already paid for the failure returns its data; the reads after it start from the locations the master knows now. Claude-Session: https://claude.ai/code/session_011NYXuzGttwrTMsfLYvmQFs * http: claim the probe for every expired host, and try it first The claim was only checked for the first url, so with two replicas whose marks expired together the second was probed by every read at once. Claim each expired host on its own and put the reads that won a claim ahead of the reachable hosts, so a probe is always a real attempt and a lost claim always means the host is tried last. Claude-Session: https://claude.ai/code/session_011NYXuzGttwrTMsfLYvmQFs * filer: refresh a chunk's locations in the streaming read path too The streaming loop had no refresh hook, so a manifest or streamed chunk that failed on one cached location and was served by another kept the stale entry until every location failed. Give it the same hook as the buffered loop, built by one refreshUrls function shared by the reader cache and the stream callers. Claude-Session: https://claude.ai/code/session_011NYXuzGttwrTMsfLYvmQFs * http: probe at most one expired host per read Claiming every expired host in one ordering left all but the first claim without an attempt, since a read stops at its first answer, and a host that had come back waited another interval for nothing. Claim only the first expired host a read sees and leave the rest last and unclaimed, so each following read probes one of them. Claude-Session: https://claude.ai/code/session_011NYXuzGttwrTMsfLYvmQFs * test: start the live server before releasing the dead server's port Closing the dead server first let the live server come up on the same port, in which case the dead location answers and the partial failure under test never happens. Claude-Session: https://claude.ai/code/session_011NYXuzGttwrTMsfLYvmQFs |
||
|
Chris Lu
|
9f6efc8b53 |
filer: a listing over a hard link no longer deadlocks a bounded SQL pool (#11118)
* filer: give the SQL stores' key-value reads their own connections A listing holds the connection its rows are on for the whole iteration, and FilerStoreWrapper calls maybeReadHardLink -> KvGet from inside that iteration, so a hard-linked entry needs a second connection while the first is still busy. Out of one bounded pool that is a deadlock: the listings fill the pool and then wait for a connection none of them will release, and the wrapper's context.WithoutCancel leaves the waiters without a deadline, so the filer stays wedged rather than erroring. The sqlite store shows it at its sharpest -- it allows a single connection, so one listing over one hard-linked entry never returns. On postgres with connection_max_open = 50, 60 concurrent listings over hard-linked entries made no progress at all. Key-value reads now run on their own pool, carved out of connection_max_open rather than added to it, so the operator's cap still bounds what the store opens against the database. An unbounded pool keeps a single pool: nothing can wait there. sqlite's single connection becomes two, one per pool, and its writes get a busy timeout so a write that meets the reader waits instead of failing. Claude-Session: https://claude.ai/code/session_018DWwctzD4T2DmnczPRM47t * sqlite: keep both pools on one database, whatever the dbFile spells A dbFile that already carries URI options got a second "?" appended, which the driver reads as part of the preceding option value, and a bare :memory: is private to each connection, so the key-value pool would open its own empty database and every key-value operation would fail on a missing filemeta. Claude-Session: https://claude.ai/code/session_018DWwctzD4T2DmnczPRM47t * sqlite: assert the busy timeout on the in-memory DSN too Claude-Session: https://claude.ai/code/session_018DWwctzD4T2DmnczPRM47t |
||
|
Carlos LeyvaandChris Lu
|
241541c026 |
filer: SQL store pool defaults that survive a concurrent walk (#11110)
* filer: SQL store pool defaults survive concurrent walks (idle == open == 50, lifetime 300s) The code defaults for the four SQL stores were connection_max_idle=2 with NO default for connection_max_open (unlimited) or lifetime, while the scaffold filer.toml documents 10/50/300 -- so an env-configured or minimal-toml filer got the worst possible pool. Under a concurrent listing burst (s3.lifecycle.run-shard walks 16 shards in parallel) every operation released above the 2 idle slots closes its TCP connection, so the walk opens a fresh connection per operation until the filer exhausts its ephemeral ports: list /buckets/... : failed to connect ... dial tcp ...:5432: connect: cannot assign requested address Measured on a production filer: 0 -> 28k TIME_WAIT with only ~1.3k concurrent, and in the minimal docker-compose reproduction (2000-dir bucket, port range narrowed to 400): the whole range in TIME_WAIT with only ~12 ESTABLISHED. Default all three knobs, with idle == open so released connections are kept and reused: idle connections only accumulate up to the actual peak concurrency and connection_max_lifetime_seconds recycles them, so a quiet deployment holds nothing extra. An explicit 0 still disables the caps as before. The scaffold's connection_max_idle moves 10 -> 50 to match. With this change the same reproduction completes all 16 shards with the default configuration (TIME_WAIT peak 19 vs the whole port range). * filer: trim the SQL pool default comments One line of the non-obvious why is enough; the rest narrated the code. Claude-Session: https://claude.ai/code/session_018DWwctzD4T2DmnczPRM47t * filer: leave the SQL stores' connection_max_open unset A listing holds its connection for the whole row iteration while its callback runs another query -- FilerStoreWrapper.maybeReadHardLink does a KvGet per hard-linked entry -- so every concurrent listing needs two connections from the same pool. With a default cap, listings past the cap wedge: 60 concurrent listings over hard-linked entries made no progress at all against a 50 connection pool, and the wrapper's context.WithoutCancel leaves the waiters without a deadline. The idle pool is what fixes the connection churn: idle 50 with an unbounded max_open holds the same 14 postgres sessions across a 16-way listing burst that opened 455 with idle 2. Claude-Session: https://claude.ai/code/session_018DWwctzD4T2DmnczPRM47t --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris Luandbruce-zzz
|
0f05957bc4 |
filer: self-heal chunk manifest reads when volume locations go stale (#11107)
* filer: self-heal fetchWholeChunk on stale volume locations Upstream #10156/#10800 wired cache invalidation into the buffer-based read paths, but manifest resolution still goes through fetchWholeChunk, which returns the raw error on failure. When cached volume locations are stale (volume tiered to remote storage, server rolled), resolving a large multipart file fails permanently even though other locations are healthy. Thread the ChunkGroup's cacheInvalidator through ResolveChunkManifest / ResolveOneChunkManifest / fetchWholeChunk, and on failure invalidate, re-lookup and retry once via the existing retryFetchWithFreshLocations helper. The streaming bytesBuffer is reset before the retry so partial bytes from the failed attempt cannot corrupt the manifest proto.Unmarshal. Non-mount callers pass nil and keep their semantics. * filer: move the manifest self-heal tests in with the other manifest tests Also make the stale server stream a prefix and then abort mid-body, which is what actually leaves partial bytes in the buffer: an HTTP error status returns before ReadUrlAsStream ever calls the writer, so a 500 never exercised the Reset the tests claimed to cover. Claude-Session: https://claude.ai/code/session_01FK3oGC5ZVeJYvNBWgb9JUD * filer: keep the cached volume locations when a manifest read is cancelled A cancelled or timed-out read says nothing about where the volume lives, so dropping the location and going back to the master only costs the next reader a round trip. PrepareStreamContentWithThrottler already guards its self-heal this way. The guard also goes inside retryFetchWithFreshLocations, since the caller can be cancelled between its own check and the invalidation, and that covers the reader cache and prefetch paths too. fetchWholeChunk returns the context error rather than the stream failure it provoked, and ResolveOneChunkManifest wraps with %w so errors.Is still sees it. That matters even where no invalidator is passed: volume.fsck resolves manifests with nil and tells its own abort from a corrupt manifest that way, so the cancellation check sits ahead of the nil-invalidator return. Claude-Session: https://claude.ai/code/session_01FK3oGC5ZVeJYvNBWgb9JUD * filer: self-heal manifest reads on the filer and s3 paths too Every caller that already holds the location cache backing its lookup function can hand it over: the filer's read, copy and deletion paths and the log cache have the MasterClient right there, and s3api has the FilerClient. MinusChunks takes one for the same reason, since the deletion path resolves manifests through it. Only the shell tools and the replication sinks, whose lookup functions cache privately with nothing to invalidate, keep passing nil. Claude-Session: https://claude.ai/code/session_01FK3oGC5ZVeJYvNBWgb9JUD --------- Co-authored-by: bruce-zzz <bruce.zou@hhy-data.com> |
||
|
|
7620e96171 |
expose whether a volume replica is backed by remote storage, and prefer local replicas (#11105)
* expose whether a volume replica is backed by remote storage
Volume locations returned by lookups do not indicate whether a replica
has been tiered to remote storage. Readers cannot distinguish a local
replica from a remote-backed one, so they may hit a remote-backed
replica first even when a local replica is available.
Add DataInRemote to the lookup location message, populate it from the
master's volume info, and carry it through the wdclient vid map so
clients can prefer local replicas when resolving chunk locations.
* wdclient: prefer local volume replicas over remote-tier replicas on lookup
LookupFileIdWithFallback (and the publicUrl variant in FilerClient)
didn't honor the DataInRemote flag when shuffling URLs, so the
DataInRemote patch only took effect in LookupVolumeServerUrl. Apply
the same ReorderToFront(localUrls) to sameDcUrls/otherDcUrls so
non-remote replicas stay at the front, matching the existing vidMap
convention.
* wdclient: propagate DataInRemote across tier transitions on existing replicas
When a volume is tiered to remote storage or a remote-backed replica is
restored locally, the cached DataInRemote on the same volume-server URL
stayed at its old value because two pieces of state never updated:
* master_grpc_server.go only split newVolumes and (already-tracked) volumes
into NewVids vs RemoteVids. ChangedVolumes went straight to NewVids, so
the broadcast announced the re-classified volume as a fresh arrival and
the client had no way to tell whether its existing cache was stale.
* vid_map.addLocationToMap early-returned when an entry already had the
same URL. A tier transition reports the same URL with DataInRemote
flipped, so the cached entry stayed at the old classification.
Wire both sides together: ChangedVolumes now go through the same IsRemote
split as newVolumes, and addLocationToMap replaces the existing entry in
place when the URL matches but DataInRemote has changed. The server
reference key only depends on URL/grpc port, so the refcount does not
move across the flip.
Adds vid_map_remote_transition_test.go covering the local->remote and
remote->local paths so the in-place update and the cache-key stability
are pinned by tests.
* wdclient: prefer local replicas across data-center boundaries
The previous local-first ordering hoisted local URLs to the front of each
data-center bucket separately, then concatenated same-DC before other-DC.
That meant a same-DC remote replica could still be tried before an
other-DC local replica even though the local one would answer cheaply.
Reorder once across the full candidate list: concatenate same-DC and
other-DC first, then ReorderToFront pulls every local replica to the very
front while preserving the DC preference inside each tier. Apply the same
ordering in all four lookup paths so the cached vidMap, the
LookupFileIdWithFallback provider path, FilerClient.GetLookupFileIdFunction
(PublicUrl-preferred variant), and the deprecated filer.LookupFn all agree:
- weed/wdclient/vid_map.go (LookupVolumeServerUrl)
- weed/wdclient/vidmap_client.go (LookupFileIdWithFallback)
- weed/wdclient/filer_client.go (LookupFileId)
- weed/filer/reader_at.go (LookupFn)
Strengthen the existing local-first tests: vidmap_client_localfirst_test
now asserts both endpoints are present (not just the local one is first),
and slice_test asserts an exact match instead of accepting two orderings.
Add TestLookupFileIdWithFallbackGlobalLocalFirst to pin the cross-DC
ordering invariant: any local replica (same or other DC) precedes every
remote-tier replica; within each tier DC1 precedes DC2.
Add docstrings to ToVolumeLocations, ReorderToFront, LookupVolumeServerUrl,
LookupFileId, GetVidLocations, GetLocations, LookupFileIdWithFallback, and
updateVidMap so the touched lookup paths are described in one place.
* topology: broadcast tier transitions on existing replicas
When a volume replica is tiered to remote storage or restored locally, the
wdclient's cached DataInRemote went stale: every connected client kept
preferring a remote-backed replica over a freshly restored local one, or
demoted a freshly tiered remote replica. The fix in commit
|
||
|
Chris Lu
|
86761cc7d5 |
filer: keep empty folders that are s3tables catalog entries (#11102)
* s3tables: build the catalog attribute keys from one shared prefix Every attribute the catalog stores on a bucket, namespace, table or view entry is spelled out with the same literal prefix. Name it once in s3_constants so code outside the package can recognize a catalog entry without repeating the string. Claude-Session: https://claude.ai/code/session_01GfZsc4cyNB2yr6KYLRv9q1 * filer: keep empty folders that are s3tables catalog entries A namespace, table or view is a directory whose extended attributes are the catalog record. Its files can live elsewhere - a rename moves only the catalog pointer and leaves the data at the old path, and a view has no files at all - so an empty one is still a live entry. Drop a table, then rename another table onto that name: the drop queues the old table's folders, the rename recreates the name path, and two minutes later the cleaner deletes it and cascades into the namespace, losing a table the catalog still lists. Claude-Session: https://claude.ai/code/session_01GfZsc4cyNB2yr6KYLRv9q1 * filer: drop a queued cleanup when the folder is created again A cleanup is queued against the folder that was found empty. If that folder is deleted and a new one takes its name, the queue entry outlives the folder it was about and the next pass deletes the replacement. A drop followed by a rename onto the dropped name does exactly this: the name path comes back as a live catalog entry two minutes before the queue is read. Claude-Session: https://claude.ai/code/session_01GfZsc4cyNB2yr6KYLRv9q1 |
||
|
Chris Lu
|
ba5b14b457 |
master, filer, s3api: bound the collection deletes that strand a caller (#11026)
* master: bound each volume server DeleteCollection, and finish the fan-out A collection delete fanned out to every volume server holding it with context.Background(), so a server that accepted the connection and then went quiet held the whole delete open with nothing to end it. Each RPC is bounded now, on the same budget allocateVolumeTimeout gives the other master-to-volume-server admin RPC. The volume server runs the delete to completion regardless of the request context, so giving up costs the confirmation and not the deletion. The walk itself is the caller's, not a per-server one: - It outlives the caller. A cancelled request must not abandon a destructive fan-out part-done, with volumes left behind and no request still running to come back for them. - It no longer stops at the first server that refuses, which left the collection on every server after it in the list. The first failure is still what is reported, and the collection stays in the topology so a later delete comes back for the rest. - It sends one RPC per server rather than one per replica. ListVolumeServers reports a node once for every replica it holds, while DeleteCollection removes the whole collection from the server it reaches, so a collection with thousands of volumes repeated the same whole-collection delete thousands of times over. Both passes run too. Returning after a failed normal pass left the collection's EC shards in place with nothing left to retry them. Claude-Session: https://claude.ai/code/session_01EnB1fbryyKc2LetRZxQPTP * master: delete the EC shards behind /col/delete too The HTTP handler carried its own copy of the volume-server walk and only ever ran the normal pass, so a collection deleted through it kept its EC shards. It shares the gRPC path now, which also gets it the bounded RPCs and the one-per-server fan-out. Claude-Session: https://claude.ai/code/session_01EnB1fbryyKc2LetRZxQPTP * filer: bound the collection delete a bucket delete leaves behind Deleting a bucket entry deletes its collection afterwards, deliberately detached from the request so a client that hangs up cannot strand the bucket's volumes. Detached meant unbounded, though: with the master down or mid-election the wait for a leader has nothing to end it, so the handler parks, and the client retrying behind it parks another. It keeps outliving the request and now carries a deadline of its own. The budget bounds the wait, not the work: the master keeps deleting on its own fan-out once asked, so giving up costs the confirmation. Claude-Session: https://claude.ai/code/session_01EnB1fbryyKc2LetRZxQPTP * s3api: bound the collection RPCs a bucket creation and deletion issue Neither carried a deadline, so a transient failure anywhere down the chain held the S3 request open until the client gave up on it. Both budgets are taken outside the filer failover walk, so one budget covers the whole walk rather than granting each filer a fresh one. The walk itself stops when that budget is spent, and stops without blaming anyone: the caller's own expiry is not evidence against the filer that was answering, and the next filer has no time left to answer in either. Recorded as a filer failure, a slow master upstream would flag every filer in the walk, and the three failures that open the circuit take unrelated object reads down with them. Claude-Session: https://claude.ai/code/session_01EnB1fbryyKc2LetRZxQPTP * s3api: a failed collection listing no longer fails a bucket creation PutBucket lists collections to notice a leftover one it is about to reuse. The result feeds a warning and nothing else -- s3a.exists is what decides whether the bucket already exists -- yet a transient failure of that listing returned 500 and refused the creation. It is advisory now, so a failure is logged and the creation continues, exactly as it does when the listing returns false. Claude-Session: https://claude.ai/code/session_01EnB1fbryyKc2LetRZxQPTP |
||
|
Chris Lu
|
af6f69740c |
Read metadata log chunks the way the mount reads every other chunk (#11018)
* Replay metadata log chunks the way the mount reads every other chunk The subscription's log-chunk replay built its own lookup, which always resolves volume server addresses. A mount started with -volumeServerAccess=filerProxy cannot reach those, so every fresh subscription failed on the previous minute's persisted segment and resubscribed a second later, forever. Take the lookup from the caller instead; the mount hands over the one it uses for file reads, which also keeps publicUrl and the bounded location cache in play. Claude-Session: https://claude.ai/code/session_01NGqrxYj7cHUpSrL249n3Z6 * Keep a log chunk read failure off the filer connection A metadata subscriber reads persisted log chunks over HTTP from volume servers and hands whatever went wrong back as the subscription's error. "connection refused" from a volume server then matched the transport patterns that decide a gRPC channel is dead, so every failed replay closed the shared filer ClientConn and cancelled the assign and upload RPCs riding on it with "the client connection is closing". Mark those read failures so they are judged for what they are. Claude-Session: https://claude.ai/code/session_01NGqrxYj7cHUpSrL249n3Z6 |
||
|
Chris Lu
|
60893c5ef3 |
Classify a filer error before a user-controlled path is wrapped into it (#11004)
* util, pb: classify a filer error by the status the server sent DoSeaweedListWithSnapshot wrapped a failed ListEntries with %v, dropping the gRPC status, so IsTransientError fell back to matching substrings against a message that now held the caller's path. Keep the status with %w and let it decide, reading the server's own text rather than the wrapper's. Claude-Session: https://claude.ai/code/session_01BjDWtZsCoZY6x4pdDmGWxU * s3: keep the bucket and prefix out of the list retry decision A bucket named transport, or a prefix under logs/unavailable/, made a PermissionDenied listing look transient and got it retried; a key holding the not-found sentence suppressed a retry that should have run. Both checks now read the filer's status, and only fall back to the text when there is none. Claude-Session: https://claude.ai/code/session_01BjDWtZsCoZY6x4pdDmGWxU * filer, s3: classify a delete failure before the path is wrapped into it The filer put the non-empty-folder marker behind its own "delete directory %s" wrapper and the gateway matched it as a substring, so a key named after the marker turned a real delete failure into the demote-the-marker no-op and the request answered 204. Keep the marker leading the message that crosses the wire, turn it back into a sentinel where the response is read, and match that. Claude-Session: https://claude.ai/code/session_01BjDWtZsCoZY6x4pdDmGWxU |
||
|
Chris Lu
|
902a12fd6f |
wdclient: bound the wait for a master leader by the caller's context (#11002)
* wdclient: bound the wait for a master leader by the caller's context WithClient waited on GetMaster with context.Background(), so a caller that arrived while no master leader was known parked in a 200ms poll loop until one appeared, whatever deadline it had already set on the RPC. Each retry above it then left another goroutine in the same wait. Take the context in WithClient and WithClientCustomGetMaster and hand it to GetMaster, and stop the retry loop once it is done. The dial keeps context.Background(): fn brings its own RPC context, so a cancellation seen here cannot be attributed to the shared connection. Call sites pass whatever they hold: the request context in the filer's CollectionList, DeleteCollection and Statistics handlers and in the credential store's propagation, the operation context in the shell's s3.bucket.delete and the kafka gateway's broker and filer discovery, and context.Background() where there is none - the shell commands, the admin dashboard wrapper, and the exclusive locker's initial lease. The locker's release keeps its own uncancelled context so a slow unlock cannot turn into a ghost lock. Claude-Session: https://claude.ai/code/session_01BjDWtZsCoZY6x4pdDmGWxU * wdclient: test that WithClient gives up with the caller's context Claude-Session: https://claude.ai/code/session_01BjDWtZsCoZY6x4pdDmGWxU * wdclient: cut the master retry backoff short when the caller gives up util.Retry sleeps unconditionally between attempts, so a transient error arriving just before the caller's deadline still cost it a full backoff step. Use the context-aware util.RetryWithBackoff, the same helper the volume lookup in this file already uses. Two call sites went with it: the shell's lock-holder lookup builds its three second bound before WithClient so it also covers finding the leader, as its comment already promised, and the filer's post-delete collection cleanup goes back to an uncancelled context - the entry is already gone, so a caller that hung up must not leave the collection behind. Claude-Session: https://claude.ai/code/session_01BjDWtZsCoZY6x4pdDmGWxU * wdclient: test that a cancel during backoff ends the retry Claude-Session: https://claude.ai/code/session_01BjDWtZsCoZY6x4pdDmGWxU |
||
|
Chris Lu
|
eb3bbfeb1f |
filer: apply the path's storage rule TTL on every write path (#10963)
* filer: cover the storage rule TTL on the object transaction write path An object written through ObjectTransaction used to land with ttlSec 0 even under an fs.configure TTL rule, while the same object written through CreateEntry got the rule's TTL. Guard the shared stamping so the two paths cannot drift apart again. * filer: apply the path's storage rule to an appended entry AppendToEntry resolved the storage option from the path - so its chunks land on a TTL volume under an fs.configure TTL rule - but never stamped the rule's TTL on the entry it creates, leaving an entry that outlives its data. Route it through applyStorageDefaultsToEntry, which now feeds the entry's own TTL into the option so the placement an existing entry's appended chunks get is unchanged. * filer: apply the path's storage rule to a completed TUS upload The PATCH path resolves the storage option from the target, so a TUS upload into an fs.configure TTL prefix writes its chunks to a TTL volume, but completion built the final entry with ttlSec 0 - the entry outlived the data it pointed at. Stamp it through applyStorageDefaultsToEntry, which also subsumes the hand-rolled read-only check and supplies the rule's name-length limit. * filer: apply the destination's storage option TTL to a copied entry The copy handler re-uploads the source's chunks under the destination's storage option, so a copy into an fs.configure TTL prefix already lands its data on a TTL volume. The entry, though, carried the source's ttlSec - 0 for a source outside the prefix, or the source's own TTL where the two rules differ - so it never expired with the data it pointed at. Take the TTL from the same option the chunks were placed with, after the data-only copy has restored the destination's metadata. |
||
|
Chris Lu
|
44115c1051 |
filer: stop TUS uploads from turning into garbage (#10945)
* filer: store TUS sub-chunks through the regular chunk writer A TUS sub-chunk was written with one assigned file id, retried up to three times against that same id, and abandoned on failure: an attempt that had landed on some replicas left a needle no session record and no entry ever references, unreclaimable by vacuum. dataToChunkWithSSE, which the regular write path uses per chunk, assigns a fresh file id per attempt and hands back the file ids of failed attempts, which are now freed the way the regular write path frees them. * filer: retry a chunk write on a fresh volume when the server 5xxs The filer's chunk writer assigns a fresh file id per attempt but only retried transient network errors, so a volume filling up and turning read-only mid-write failed the whole request even though the very next assignment would have landed elsewhere. Every other write client already routes this through ShouldReassignUpload; the filer's own write path now does the same, for regular uploads and TUS sub-chunks alike. * filer: export the chunk deletion queue The filer test harness in weed/server builds filer.Filer as a struct literal, so any code path reaching DeleteChunks dereferenced a nil queue. Exported like the neighboring DeletionRetryQueue so the harness can arm it. * filer: complete a TUS upload whose chunk records overlap A PATCH retried while its predecessor was still storing a sub-chunk - a proxy timeout with an immediate retry is enough - records the same range twice. HEAD computes Upload-Offset as the covered watermark and reported the upload fully received, but completion demanded exactly adjacent records and failed every attempt: the client concluded success from offset == length, no entry was created, and the session eventually expired, turning the entire upload into deleted needles for the vacuum to chew through. Completion now validates gapless coverage with the same watermark HEAD uses. A record extending coverage joins the entry - the read path resolves partial overlaps by ModifiedTsNs, and the raced copies carry identical bytes - while a fully covered duplicate is freed once the entry lands. * filer: allow one mutating TUS request per session at a time Nothing stopped two PATCHes from writing the same range concurrently: both loaded the same offset, both passed the conflict check, and both recorded their sub-chunks. A client whose request timed out in a proxy retries immediately while the server side is still storing the buffered sub-chunk, which is exactly that race. A session now accepts one PATCH or DELETE at a time, the way tusd locks uploads; a concurrent one is refused with 423 Locked, which TUS clients retry, and HEAD keeps answering so progress polling is unaffected. The chunk state is loaded under the claim, so a retried PATCH sees every record its predecessor left and conflicts cleanly instead of duplicating data. * test: cover a TUS PATCH raced by its own retry Stalls a PATCH mid-body over a raw connection, retries the same range while it is in flight, and expects the retry refused with 423 Locked; the upload then resumes from the reported offset and the final content must be intact. * filer: never free a TUS duplicate the entry still references Coverage is computed from ranges, so a record fully covered by another is treated as a duplicate no matter which needle it names. A malformed record naming a file id the entry keeps would have had that needle freed right after the entry landed - the corruption this change set exists to stop. The duplicates are now freed in one batch, skipping any file id the entry references; their records go with the session directory. * test: bound the raw TUS connection reads http.ReadResponse on the stalled PATCH's connection blocked until the whole go test timeout if the filer never answered. * filer: free the needles of chunk write attempts a retry replaced A volume server stores the needle locally and only then fans out to the replicas, so a replication failure 5xxs with the data already written. Each attempt assigns its own file id, so once a later attempt lands elsewhere nothing references the earlier ones: the caller only sees the chunk that succeeded, and the failed ids were dropped. They are now freed the way the caller frees them when the whole write fails. Retrying on a 5xx makes this reachable on every read-only or full volume, which is exactly the condition that filled the reporter's volumes. |
||
|
MaratKarimovandMarat Karimov
|
a3afe4460b |
tarantool: fix upsert data corruption and missing context propagation (#10926)
Co-authored-by: Marat Karimov <karimov_m@inbox.ru> |
||
|
Chris Lu
|
863fec6c3f |
S3: let a key that is a prefix of other keys be an object (#10912)
* filer: keep the sentinel when CreateEntry reports an update failure CreateEntry flattened the error UpdateEntry wraps, so errors.Is stopped matching and ErrExistingIsDirectory and ErrExistingIsFile never reached the S3 mapper, which answered a retryable 500 instead. * s3: let a key that is a prefix of other keys be an object S3 keys are flat, so "a/b" and "a/b/c" are independent objects that coexist in either write order. The filer stores a key as a path, so one of them has to live on the directory the other is nested under. Writing the nested key first refused the prefix key outright. Writing it second promoted the file to a directory, which kept its data but lost the key: an empty object left nothing to recognise it by and disappeared, and one with data listed under a trailing slash it never had. Mark the directory that carries such a key, and write the object onto it when the path is already a directory. The mark makes an empty prefix object visible to listings and readable by GET and HEAD, keeps the empty folder cleaner off it, and lists it under the key it was written with. Deleting the key strips the mark back off along with the data. * filer: keep a TTL off a directory that stands for an object An expired entry is deleted a row at a time, so expiring a directory removes it and leaves everything under it unreachable. Promoting a file to a directory carried its TTL across, and a promoted file is exactly the one that has keys nested under it. Drop the TTL on promotion, and leave one an older build wrote alone. The lifecycle worker still expires the object, through the delete that leaves the directory behind. * s3: delete the null version of a key other keys are nested under The routed delete cannot remove an entry that other keys live under, and answered a retryable 500 rather than falling back to the lock path the unversioned delete already falls back to. That path then looked the entry up under the bucket with the whole key as its name, so the demote wrote it back one directory too high and failed as not found. Fall back on any non-precondition error, and split the key before deleting it. Trailing-slash directory markers with children reach the same delete. * filer: keep the sentinel when MkFile and Mkdir report a create failure Same flattening one layer out: every mkFile caller lost the sentinel, so a CopyObject onto a key that other keys are nested under answered a retryable 500 where a PutObject of the same key answers 409. * s3: copy and rename a key that other keys are nested under Such a key is stored on the directory those keys live in, and copy and rename both refused it: the source lookup maps every directory entry to NoSuchKey, so a key a plain GET serves could not be copied or moved, and the destination side refused it as a directory conflict. The source is read through a view of the entry as the object it names. The destination is written the way a PutObject of that key writes it. A rename at either end copies the object's own data across and strips it off the source key rather than going through AtomicRenameEntry, which moves a directory by moving everything under it - the nested keys are not part of what is being renamed. |
||
|
|
df93d01c06 |
admin: add bucket lifecycle rule editing (#10860)
* admin: add bucket lifecycle rule editing * address greptile's comments * more small fixes * coderabbit's comments * more comment fixes * more fixes * more * maybe last * last ? * 14850 * 14851 * filer: stamp the content MD5 on every SaveInsideFiler write An entry's ETag falls back to Attributes.Md5, so conditional writers key IF_ETAG_MATCH off it. SaveInsideFiler carried the looked-up attributes forward without refreshing the hash, leaving it describing whatever the previous writer stored: a later conditional write matched the stale hash and overwrote content that had already changed. * s3api: give the bucket lifecycle constants and the write route key one definition each The extended-attribute keys, the XML size cap and the object-write ring key prefix were each spelled out in two places, so the admin dashboard's copies could drift from the gateway's. Move them to the packages both sides already import and alias them where the short local name reads better. * admin: patch the bucket entry's lifecycle keys instead of rewriting the entry The save read the bucket entry, edited its extended map and wrote the whole entry back, guarded by IF_UNMODIFIED_SINCE. Nothing that writes a bucket entry advances its mtime - not the S3 gateway's patchBucketEntry, not SetBucketOwner, not SetBucketQuota - so the guard never fired and the stale snapshot reverted whatever else had changed since the lookup. Send the PATCH_EXTENDED mutation the S3 gateway already uses for these keys: the filer re-reads and merges under the bucket path lock, so only the two lifecycle keys move. That removes the reason for the mtime snapshot, the verification retry loop and the compensating restore of the cleared day-TTL rules, which the migration now logs instead. * s3api: run the delete-lifecycle day-TTL migration through the shared helper DeleteBucketLifecycleHandler kept its own copy of the read-strip-write sequence the put handler now shares, including a missing return that let a ToText failure persist a truncated filer.conf and write a second response. It also wrote the whole file back unconditionally, reverting any concurrent edit; the shared helper writes conditionally. * admin: answer 404 when a lifecycle request names a bucket that does not exist Every SetBucketLifecycle failure came back as 500, including the lookup miss for an unknown bucket, so a client or monitor read a caller error as a server fault and retried it. * s3api: emit lifecycle XML a client would recognize Two changes to what MarshalCanonical writes, both visible through GetBucketLifecycleConfiguration, which replays the stored bytes verbatim: stamp the S3 namespace on the root, and put a size range under <And>. A <Filter> carries one predicate, so two size bounds side by side is a shape AWS does not document. Parsing still accepts either. * admin: fix the lifecycle editor's handling of stored status, deletes and empty saves Four things the editor got wrong: A stored <Status> the S3 API never validated, say 'enabled', left both radio buttons unchecked, so reading the form threw on a null querySelector result and Save did nothing. Collapse anything but an exact 'Enabled' to 'Disabled', which is what the engine already does with it. Deleting a rule re-rendered an open edit form from the snapshot taken when editing began, discarding what had been typed; every other transition folds the form in first. The Transition warning only matched a bare <Transition>, missing the form with attributes, self-closed or namespace-prefixed. Saving an emptied rule list clears the configuration through a path with no prompt, next to a Delete-all-rules button that asks. Also collapses the three divergent copies of formatBytes on this page to one. * filer: stop the day-TTL migration from deleting an operator's path rule The migration removed every rule under the bucket's path that carried a day TTL in the bucket's collection. The add path it is retiring used AddLocationConf, which merged its TTL onto whatever already sat at the prefix, so a rule can hold operator settings the lifecycle path never wrote - a disk type, WORM retention, a read-only flag, a placement pin. Deleting the whole rule to retire its TTL took those with it, leaving objects under that prefix on defaults nobody asked for. Delete only rules shaped like ones the add path created from scratch; anything else keeps its settings and loses just the TTL. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> |
||
|
Chris Lu
|
0c95137528 |
filer: stop aggregated metadata subscribers from spinning on a peer watermark hold (#10863)
* fix(filer): stop logging a held aggregated read as an error An aggregated subscriber may not read past the peers' low-watermark, and it stops at the first entry beyond it by returning a sentinel from the read callback. LoopProcessLogData logs every callback error, so on a cluster that keeps writing - where there is almost always an entry newer than the watermark - every read wrote an ERROR line naming the entry it stopped at, thousands per minute per filer. Mark the stop as control flow: an error wrapping StopReadingError is handed back to the caller unlogged, and the held-read sentinel wraps it. * fix(filer): release an aggregated watermark hold on peer progress A held read waited on the aggregated buffer's data channel, which the next write signalled - but a write cannot release a hold, only a peer reporting further progress can. On a cluster that keeps writing the loop therefore re-ran a whole pass per arriving event, log file listing and all, and held again on the same entry every time. Signal held readers from the meta aggregator instead, whenever a low-watermark rises: a peer reporting, or one dropped past its removal grace. The retry interval stays as the backstop for what no watermark covers. Count the holds so a parked subscriber stays visible. * fix(filer): floor how often an aggregated watermark hold releases Peers advance their delivery watermark on every event they stream, so releasing a hold on every advance is the same pass-per-event storm as releasing on every write, just without the log lines - and each pass lists a day of log files. Floor the release at 20ms. Advances inside the floor collapse into one release, which then delivers everything they covered. * fix(filer): pace a peer's delivery claim by what its subscribers hold at A filer's local metadata stream carries an idle heartbeat to its peer aggregators, and each peer turns it into that filer's delivery low-watermark. Aggregated subscribers hold at the minimum across peers, so a filer quiet enough to fall back on the heartbeat parked every subscriber in the cluster up to a keepalive interval - 5 seconds - behind live writes. With nine filers, most of them quiet at any moment, the minimum sat there permanently. Pace that heartbeat at 200ms once the filer has peers. It stays a keepalive, at the keepalive interval, for a filer with none. * fix(filer): wake each aggregated hold on its own watermark A persisted-log read is held by what the peers have flushed, an in-memory read by what they have delivered, but both parked on one channel closed whenever either minimum rose. Peers advance their delivery watermark on every event they stream, so a flush-held reader woke at the coalescing floor to re-list a day of log files and park again on the same entry - the storm this set out to fix, in the one place asymmetric peer progress still reached. Signal the two separately and park each read on the one that bounds it. |
||
|
|
930603eb74 |
S3: optionally serve remote-mounted objects from remote when the local read fails (#10837)
* feat(s3): serve from remote on local read failure When a locally-cached chunk of a remote-mounted object becomes unreadable (volume server down/restarting, or an evicted needle 404ing under retry-backoff), fall back to serving the object from its mounted remote instead of erroring. A bounded pre-flight probe makes a stuck volume trip the timeout rather than stalling the request. Gated by -localReadFallbackToRemote (default off) with -localReadFallbackTimeout (2s default), so existing deployments are unaffected until they opt in. * fix(s3): register local-read-fallback flags for mini/server/filer The mini, server and filer launchers build S3Options directly and only populate the flag pointers they register. Without registering the two new flags there, startS3Server dereferenced nil pointers and crashed at boot, failing every integration suite that runs `weed mini`. * fix(s3): treat a zero-byte probe read as unreadable A read that returns no byte -- whether it reports io.EOF or no error at all -- means the offset is not locally readable, so the probe must fall back to the remote rather than proceeding to stream a truncated response. Only a returned byte (including the object's final byte with a trailing io.EOF) counts as readable. * s3: finish a mid-stream local read failure from the remote mount The pre-flight probe only proves the byte at the requested offset readable. A multi-chunk object can still lose a later chunk after the 200/206 and its Content-Length are committed, which truncated the body with no fallback. Resume from the mounted remote at the byte the local copy stopped at, so the response still carries the declared length. A short local read that surfaces as a clean EOF is treated the same way instead of silently truncating. * s3: fall back to the remote mount without a CLI switch Serving a remote-mounted object from its authoritative remote is what the read should have done all along -- the alternative is a 500 on an object the cluster can still reach -- so make it the behavior instead of two new flags, with the probe bounded by a constant. * s3: trim the comments on the fallback path * filer: report only the contiguous prefix when a parallel chunk read fails The parallel branch of doReadAt fans the chunk reads straight into their own windows of the output buffer, then sums every task's bytesRead. A middle chunk failing while a later one succeeds therefore returned a length covering a hole the reader never filled, handing the caller zeros in the middle of otherwise valid data. * s3: only splice the remote onto a local prefix while it is the cached generation Eligibility establishes a size match, not byte identity: a remote key overwritten with same-size content between the cache fill and the fallback would have finished the response with bytes from a second generation, under the first one's ETag. Stat the remote before resuming and keep the local error when it no longer matches -- a truncated body is a visible failure, a spliced one is not. --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |