mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-09-29 19:25:35 +00:00
0f3ba98e11ff2f9bb6e8f38301c4f5937a31b3eb
9957
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  
|
0f3ba98e11 |
volume: make volume.scrub report a live needle whose stored id is damaged (#11468)
* storage: scrub live needles' stored id against the index key scrubVolumeData only compared the needle's stored id for tombstones, so header damage on a live needle — where the data CRC cannot see it — passed every scrub mode while reads of that needle kept failing or serving the wrong key's data. Compare the id for every indexed needle. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * rust volume: scrub live needles' stored id against the index key (parity) Mirror the Go scrub fix: compare the stored needle id with the index key for live needles too, not only for deleted ones. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * rust volume: cover damaged live needle id in scrub test The tombstone test proved the index-key check fires for deleted entries; add the live-needle mirror of Go's TestScrubVolumeDataChecksLiveNeedleId so a regression in the live path is caught in Rust too. 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> |
||
|
|
80a26020d7 |
util: serialize all ViperProxy access so startup cannot hit concurrent map read/write (#11470)
* util: serialize every ViperProxy method; stop promoting unlocked viper calls ViperProxy embedded *viper.Viper, so only the five declared methods took the mutex while every promoted call — GetStringMap in backend.LoadConfiguration was the reported crash — touched viper's maps unsynchronized. `weed server` starts the volume server (SetDefault writer) and the master (GetStringMap reader) back to back, and a race build reports the pair on a plain start. The wrapped viper is now a named field: a method must be declared here to exist on the proxy, so unsynchronized access fails at compile time rather than at runtime. Every promoted use in the tree (GetStringMap, GetUint32, GetFloat64, GetDuration, IsSet, AllKeys, Set) gets a locked wrapper; NewViperProxy replaces struct literals for local vipers. GetStringMap deep-copies its result — viper hands back the internal subtree, so iterating it after the lock is released would race the next writer. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * util: take the shared lock while LoadConfiguration merges a config file viper.MergeInConfig rewrites the same maps the proxy serializes; without the lock a merge can race a concurrent SetDefault or reader exactly like the reported startup crash. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * util: deep-copy slice elements in the GetStringMap snapshot A slice of maps inside the returned subtree still shared the inner maps — copy elements recursively so nothing the caller mutates is viper's internal state. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * util: add the missing AutomaticEnv wrapper used by tests sse_reader_test reaches it through GetViper(); without the wrapper the call no longer exists once the viper field stopped being embedded. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * util: return a fresh slice from GetStringSlice A stored []string comes back uncast from viper — the backing array is shared internal state like the GetStringMap subtree, so copy it while holding the lock. 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> |
||
|
5389f61cef |
volume server: do not finish a GET when the needle CRC mismatches (#11464)
* volume server: do not finish a GET when the needle CRC mismatches A streamed full-needle read compared the CRC only after every page had been written. Once the response buffer flushed, the client already had a completed 200 and the corrupt bytes. Hold the last page until the checksum matches, and if an earlier page has already been flushed, abort the connection instead of calling http.Error. Fixes #11459 * volume server: abort partial-content bodies on write error too The non-Range path drops the unflushed tail and aborts on a mid-body error; the single-range and multi-range paths still flushed it after WriteHeader(206) was committed, delivering corrupt bytes as a complete body. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume server: assert the started 200 is aborted in the write-error test The test previously returned on any request error, so it passed without verifying the abort. It now asserts the client got the committed 200 headers and then a failed body read. Also trims comments. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
|
8ad2f29e3e |
shell: let volume.deleteEmpty drop volumes with no live needles (#11437)
* shell: let volume.deleteEmpty drop volumes with no live needles The candidate check only accepted a .dat at superblock size, so a volume whose every needle was deleted still had to be vacuumed first — minutes of compaction to rewrite bytes that were all garbage anyway. FileCount counts every indexed entry and DeleteCount every entry made garbage by overwrite or delete, so FileCount <= DeleteCount means nothing live remains and the volume can be unlinked directly. The quietFor guard is unchanged. * volume server: add only_garbage VolumeDelete guard VolumeDelete(only_empty) refuses every volume that ever held data, so a volume whose needles are all deleted could only be removed after a vacuum rewrote it. The new only_garbage flag deletes only when the byte counters show nothing live: DeletedSize covering all of ContentSize, the same all-garbage state vacuum measures. Byte counters are used because the file/delete counts drift on index reload. * rust volume: mirror only_garbage VolumeDelete guard Same check as the Go server: a volume deletes under only_garbage when its deleted bytes cover all content bytes. The grpc handler rejects before the store drops the volume from its map, since destroy errors after removal would still unmount it. * volume delete: let either enabled check pass, keep onlyEmpty on the wire An upgraded shell sending only_garbage to a pre-upgrade server would be read as an unconditional delete (field ignored, only_empty false). The request now keeps only_empty set so old servers check emptiness and refuse, while new servers delete when either check passes. * volume.deleteEmpty: skip remote-backed and protected read-only volumes A remote-tiered replica shares its cloud object with the other replicas, so keepRemoteData=false on one delete removes data they still reference. Protected read-only volumes are quarantined or under maintenance, which is exactly when a replica should not be dropped. * volume delete: validate guarded copies across disks before deleting * volume delete: hold copy locks across guarded validate-and-delete CheckVolumeDeletable released each copy's locks before Destroy ran, so a write landing on a later copy between the two passes refused its destroy after earlier copies were already removed. Pin every copy's dataFileAccessLock (and its location's volumesLock) across validation and removal so a refused delete leaves all copies intact. * volume delete: send deleted-volume notices after releasing locks A blocking send on a full DeletedVolumesChan under volumesLock can stall the heartbeat loop that drains it while it waits on the same locks. Collect the notices under the lock span and send after release. * pb: restore generated-file cosmetics to match the repo's protoc version 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> |
||
 Ilia DemianenkoandGitHub
|
c58bd0dfd3 |
s3: honor assignment fsync in UploadWithRetry (#11449)
* fix: honor assignment fsync in UploadWithRetry * Tests feedback |
||
  
|
975cec9228 |
s3api: exclude marker part in listObjectParts pagination (#11463)
* s3api: exclude marker part in listObjectParts pagination Signed-off-by: Tyagiquamar <mohdquamartyagi@gmail.com> * s3api: guard listObjectParts marker boundary and enhance pagination test Signed-off-by: Tyagiquamar <mohdquamartyagi@gmail.com> * s3api: fold in review feedback from the parallel #11462 fix Same core fix; this adds the explanatory comment, tightens the overflow guard to math.MaxInt64, makes the fake filer sort entries like a real listing, and adds the marker-exclusivity assertions alongside the pagination walk. Co-authored-by: yi111 <yi111@users.noreply.github.com> --------- Signed-off-by: Tyagiquamar <mohdquamartyagi@gmail.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> Co-authored-by: yi111 <yi111@users.noreply.github.com> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
5b79f51e3c |
filer.remote.sync: stamp entries with IF_CHUNKS_EQUAL so a stale write-back cannot delete live chunks (#11435)
* filer.remote.sync: stamp entries with IF_CHUNKS_EQUAL so a stale write-back cannot delete live chunks updateLocalEntry records the RemoteEntry stamp after an upload by writing the event's entry back with UpdateEntry. The filer deletes every stored chunk absent from an updated entry, so when the file was rewritten while its upload was in flight (or the event is a replay), the stale snapshot deletes the rewrite's chunks: the entry then points at the new fid with no needle behind it, and the rewrite's own upload fails and is skipped as superseded. The stamp write now carries WriteCondition IF_CHUNKS_EQUAL over the event's chunk fids, evaluated by the filer under the path lock. A refused stamp means the filer moved past this event; the superseding event follows in the log and stamps the current entry, so the refusal is logged and skipped like a superseded upload. Reproduction: weed server -filer plus a weed server -s3 remote, remote.mount, filer.remote.sync; hold the remote (docker pause) so one upload stays in flight, rewrite the file through the filer, unpause. Before: the entry's chunk is 404 on every volume server. After: the stale stamp is refused, the rewrite's chunk stays live and reads back after a vacuum. * filer.remote.sync: stamp entries with IF_ENTRY_EQUAL so stale inline content or metadata cannot be restored The IF_CHUNKS_EQUAL guard compared only the chunk fid multiset, so a rewrite that touched inline content or metadata alone still compared equal and the stale snapshot overwrote the live entry. The new clause compares the whole stored entry against the event's entry under the same path lock. * filer: route conditional UpdateEntry to the entry's owner filer Two filers locking the same path locally could still pass a stale condition on the non-owner while the owner's entry had moved on. When a condition or expected_extended precondition is set, forward the request to the entry's owner the same way conditional CreateEntry does, with is_moved bounding the hop. * filer: compare IF_ENTRY_EQUAL against the normalized expected entry FindEntry grows FileSize to the chunk extent, so a raw event entry with FileSize still zero failed the condition on an unchanged file and the stamp was skipped, letting a replay upload the object again. * filer.remote.sync: classify refused stamps by gRPC status only A FailedPrecondition substring in an unrelated error would have been swallowed as a skipped stamp; status.FromError already unwraps. * remote sync: keep the event entry intact for IF_ENTRY_EQUAL --------- Co-authored-by: Chris Lu <chrislusf@users.noreply.github.com> |
||
|
Chris LuandGitHub
|
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 LuandGitHub
|
56d2f05ccd |
topology: wake the vacuum dispatcher when a worker frees quota (#11436)
* topology: wake the vacuum dispatcher when a worker frees quota The dispatch loop slept a fixed 10s whenever every pending volume was waiting for a per-server quota slot, so a sweep took volumes x 10s regardless of how fast the compactions were. Workers now signal on a buffered channel after crediting quota; the dispatcher waits on it with the 10s sleep kept only as a timeout. * master: add -vacuumIntervalSeconds to tune the automatic sweep interval The 14-minute base interval was a literal inside the refresh loop while every neighbouring vacuum knob was already a flag. Defaults to 840s, unchanged. * topology: keep the 14 minute floor on the vacuum interval A zero-valued MasterOption or a negative -vacuumIntervalSeconds left the sweep sleeping only its jitter, so treat non-positive intervals as the previous default. |
||
|
Chris LuandGitHub
|
8c1be63c92 |
ecbalancer: let a non-overflow parity shard leave a data-bearing rack (#11438)
The parity pass only queued shards past the per-type cap, so a single parity shard sharing a rack with data was never a move candidate even when an empty data-free rack existed (2+1 over 3 DCs settled 2/1/0). Non-overflow candidates now move too, but only to a rack without data; overflow shards keep the existing data-rack fallback. |
||
|
Chris LuandGitHub
|
b3a8701989 |
lance: authenticate the catalog with Bearer tokens and x-api-key (#11431)
* lance: accept OAuth2 bearer tokens for catalog auth Lance and LanceDB clients can only send OAuth2 / Bearer / API-Key headers on catalog calls, never SigV4, so behind an auth-enabled S3 gateway every namespace request failed with 403 Access Denied. Mirror the Iceberg catalog's OAuth2 support: POST /oauth/token accepts an S3 access key / secret key as client_id / client_secret, validates them against IAM, and returns a signed JWT. The Auth middleware accepts that token as a Bearer credential before falling through to SigV4. Closes #11430 * lance: accept x-api-key header carrying an S3 credential The Lance namespace spec's third auth scheme maps api_key onto the x-api-key header. Accept "access_key:secret_key" there and validate it against IAM, so clients that only hold static headers can authenticate without minting a token first. * lance: answer invalid_client with the Basic challenge RFC 6749 5.2 requires a 401 from the token endpoint to carry WWW-Authenticate matching the scheme the client used, so it knows how to retry. * lance: cap the token endpoint request body /oauth/token is unauthenticated, so ParseForm needs the same size bound decodeBody applies to every other catalog request. * lance: keep query strings out of request logs /oauth/token rejects a client_secret sent in the query, but the logging middleware and the catch-all wrote RequestURI to the log before that rejection ran. Log the path alone so a mis-sent secret never reaches the log. * lance: log the escaped path, not the decoded one URL.Path decodes percent escapes, so a request like /%0aFORGED could split log lines. EscapedPath keeps the encoding while still dropping the query string. |
||
|
b9ad62fc16 |
[Volume] Keep DAT and index state consistent after async batch Sync failure (#11425)
* fix 11400 * persist failed-recovery quarantine and harden rollback - record the unavailable state in a .unavailable marker, fsync it, and re-arm it on load so a restart cannot serve an unverified pair - quarantine the volume so heartbeats stop advertising it - block MarkVolumeWritable while unavailable, rechecked under noWriteLock - fail every request of a failed batch, not only the succeeded ones - restore the needle map and truncate .dat on inline fsync rollback failure - add truncateIndex for the sorted-file needle map - mirror the fail-closed semantics in the Rust volume server * volume: erase rolled-back mappings instead of leaving tombstones A rolled-back batch or failed inline write used Delete() to undo a needle that did not exist beforehand, leaving a tombstoned map entry whose stale offset makes the next write to that needle fail reading a header that no longer exists. Add removeMapping/restoreMapping to the mappers so recovery erases entries that were absent before the batch and reinstates the exact prior offset/size for ones that were, including tombstones. The index row still goes through Delete so a replay forgets the needle. * volume: gate bulk readers on unavailable and fsync the marker's dir - fsync_dir(&self.dir) synced the volume dir's parent, not the dir holding .unavailable; pass the marker path so the create survives a host crash - export UnavailableError and check it in ReadAllNeedles, VolumeTailSender, VolumeIncrementalCopy, and IncrementalBackup so replica-sync paths cannot stream or append data from an unverified .dat/.idx pair; mirror on the Rust side via read_dat_slice, read_all_needles, dat_scan_plan, and the incremental-copy handler * volume: drop issue references from comments near touched code * volume: stop active scans when the volume becomes unavailable The stream entry-point checks ran once per RPC, so a volume quarantined by a failed recovery mid-scan kept serving data. Recheck availability per needle/chunk on the detached read paths: tail scan and heartbeat, read-all, incremental copy, incremental backup writes, and the Rust StreamingBody chunk reads. Rust incremental copy also rejects a quarantined volume before sync_to_disk touches the backend. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris LuandGitHub
|
e2608edda4 |
volume: fix 5-byte index offset corruption in makeupDiff (#11411)
* volume: encode all offset bytes when makeupDiff replays a write makeupDiff patched only bytes 8:12 of the index entry, so under the 5BytesOffset build the fifth byte kept the old offset's high bits and the replayed needle's index pointed 32 GiB-aligned ranges away from its body. A later vacuum then dropped the entry as unreadable. Rebuild the entry with needle_map.ToBytes, the same encoder the tombstone branch just below uses. * volume: test makeupDiff replay across a 32 GiB offset boundary Sparse-file test: truncate the .dat to 64 GiB after one write, compact, write a second needle, commit, and assert the index offset matches the .compacted size and the needle stays readable through a second vacuum. Only runs under -tags=5BytesOffset. |
||
|
Chris Lu
|
6848cdf9e1 |
s3: close seaweedfs-quota policy-confusion gap (#11409)
* s3: count seaweedfs-quota as an operation subresource PUT /bucket?policy&seaweedfs-quota was not rejected by hasAmbiguousSubresource because operationSubresources omitted the seaweedfs-quota key. The router then picks the policy route (registered first) while the IAM action resolver may resolve the request to s3:PutBucketQuota, letting a quota-only identity write a bucket policy. Reject the combination before routing, matching the fix for policy&tagging (#10987). * s3: resolve seaweedfs-quota after other bucket subresources The quota routes are registered last among the bucket subresource routes, but the action resolver found seaweedfs-quota inside the unordered bucketQueryActions map, so a request carrying it alongside another selector could be authorized as the quota operation while the router served the earlier-registered handler. Resolve it explicitly at the end so the resolver agrees with the router, mirroring how list-type is handled. * s3: count resolver subresources in the ambiguity guard hasAmbiguousSubresource only counted operationSubresources, so adding a query parameter to the action resolver without updating that list reopened the authorize-one-serve-another gap. Count bucketQueryActions keys as operation selectors too, and add a test that walks the registered routes and fails on any query key that is neither an operation subresource nor a known modifier. |
||
 
|
ca62d4297b |
volume: load the .ecj deletion journal in chunks, and repair a torn tail (#11408)
* volume: load the .ecj deletion journal in chunks, and repair a torn tail Two independent defects in the EC deletion journal's load path. 1. The loader issued one NEEDLE_ID_SIZE-byte positional read per entry. That is fine for a healthy journal -- kilobytes -- and pathological for a large one. A `.ecj` is semantically a SET of deleted needle ids but is written as an append-only log that nothing dedupes, and several paths append a peer's ENTIRE journal onto the local one (VolumeEcShardsCopy with copy_ecj_file, EC index recovery, and ec_decode's deliberate cross-holder merge), so a volume whose shards are repeatedly balanced between two servers grows the file without bound. Observed in production: 1.51 TB and 1.30 TB on the two holders of one 10+4 volume containing ~100 distinct ids. At that size the per-entry loop is ~188e9 syscalls, run synchronously while holding the deleted_needles write lock and before the HTTP port opens. The process sits at 100% of one core with a small RSS -- the set stays tiny because the ids repeat -- reading at a few MiB/s because 8-byte reads defeat readahead, logs nothing after "Adding storage location", and ignores SIGTERM. The master then unregisters every volume it holds and reads of them fail. 4.46 and 4.47 are both affected. Read in 1 MiB chunks and build into a local set, merging once at the end so the write lock is not held for the whole scan. Measured on a 256 MiB journal of 100 distinct ids: 33,554,500 syscalls -> 257, identical resulting set. 2. A torn tail silently corrupted later deletes. The journal handle is in append mode, so writes land at the physical end regardless of alignment. A trailing partial record therefore pushed every later append out of alignment: the loader skipped the partial bytes, but the next mount decoded them together with the leading bytes of the following entry, producing one garbage id and dropping the delete that came after the tear -- after acknowledging it. Truncate to a whole number of records at mount, before anything can append. The repair uses its own read+write (non-append) handle: on Windows, append(true) requests FILE_APPEND_DATA without FILE_WRITE_DATA (and .write(true) is subsumed by .append(true)), so SetEndOfFile through the journal handle fails with ERROR_ACCESS_DENIED. The same trap exists in journal_delete's recovery path, which calls set_len on the append handle to roll back a partial write whose sync failed. It is error-handled rather than fatal, so on Windows that rollback silently does not happen. Untouched here; worth a separate fix. Bounding the journal's growth needs compaction, which is deliberately not in this change: replacing the file under a store that can hold several EcVolume instances for one volume id requires coordinating with the other holders, and that belongs at the store layer. Sent separately. Tests: a journal spanning several read chunks loads every entry; a trailing partial record is ignored rather than panicking; a torn tail is truncated at mount and a delete taken afterwards survives a remount. * volume: roll back a failed .ecj append through a dedicated write handle The append handle lacks FILE_WRITE_DATA on Windows, so the set_len rollback after a failed sync silently did nothing and the journal could drift one record past deleted_needles. Same trap as the torn-tail repair in this file; fix it the same way. Also format the new tests. * volume: mirror chunked .ecj load and torn-tail repair in Go --------- Co-authored-by: chrislusf <chrislusf@users.noreply.github.com> Co-authored-by: Devin <devin@cognition.ai> |
||
 Eliah RusinGitHubDevin <158243242+devin-ai-integration[bot]@users.noreply.github.com>Claude Fable 5.1Chris LuDevin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
6d676eda67 |
rust volume: typed errors for store compaction so gRPC can answer NotFound (#11355)
* rust volume: typed errors for store compaction so gRPC can answer NotFound The vacuum entry points on `Store` returned `Result<_, String>`, so the gRPC layer had nothing to branch on and answered `Status::internal` for every failure. A vacuum loop that races a volume being moved or deleted saw the same code as a disk going bad, and `weed shell` could only tell the two apart by matching on the message text. `VolumeError` gains `VolumeNotFound(VolumeId)` — the existing `NotFound` is needle-level and carries no payload — and `InsufficientSpace`, and `compact_volume`, `commit_compact_volume`, `cleanup_compact_volume` and `delete_collection` return it. `impl From<VolumeError> for tonic::Status` in `server/mod.rs` maps not-found to `not_found`, read-only to `failed_precondition`, insufficient space to `resource_exhausted`, already-exists to `already_exists`, and everything else to `internal`; the four RPCs prefix their own context with `status_with_context`, so a message reads "commit compact volume 7: volume id 7 is not found". The store-side "during compact" / "during commit compact" / "during cleaning up" suffixes are gone, and the free-space message drops the volume id the prefix already supplies. `check_compact_volume` had no callers — `VacuumVolumeCheck` computes the garbage level from its own `find_volume` — and is deleted. `compact_volume` folded the size estimate into its first lookup, dropping the `unwrap()` re-lookup that only existed to dodge a borrow. `ascending_visit` on `CompactNeedleMap`, `RedbNeedleMap`, `SortedFileNeedleMap` and the `NeedleMap` dispatch is now generic over the visitor's error type, like `CompactMap::ascending_visit` already was. The three signatures that can fail on their own bound `E: From<String>` to carry those failures; the in-memory walk in `iter_entries` names `Infallible`, which says in the type what its comment used to say in prose. No Go shell command matches on the old error text: the strings exist only in weed/storage/store_vacuum.go. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * volume server: trim comments and answer the same codes from Go - vacuum_volume_check reports VolumeError::VolumeNotFound like the other vacuum RPCs instead of its own "not found volume id" wording - drop doc comments that restate what the code says - Go volume server wraps ErrVolumeNotFound/ErrInsufficientSpace from store_vacuum.go so VacuumVolumeCheck/Compact/Commit/Cleanup and DeleteCollection answer NotFound/ResourceExhausted, matching the Rust volume server; volumeDeleteStatusError generalized to volumeStatusError Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume server: prefix operation context on vacuum errors Lower-level errors forwarded by CompactVolume, CommitCompactVolume, CommitCleanupVolume and DeleteCollection carry no volume id or operation name. Wrap with %w so the status mapping still sees the sentinel chain, matching the context the Rust server's status_with_context adds. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume server: map NotEmpty to FailedPrecondition, share mapper in VolumeDelete Go's volumeStatusError maps ErrVolumeNotEmpty to FailedPrecondition; the Rust Status conversion was missing it and volume_delete kept a hand-rolled match. Route it through status_with_context like the vacuum handlers. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.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 LuandChris Lu
|
26fc90187e |
s3: accept x-amz-checksum-mode from the query string, case-insensitively
Presigned HeadObject/GetObject requests hoist x-amz-checksum-mode into the signed query string, so a strict header-only check would withhold stored checksums on presigned reads that AWS honors. |
||
|
Chris LuandChris Lu
|
ef463fe1af |
s3: read complete-request checksum values from headers or query
Presigned CompleteMultipartUpload requests hoist x-amz-checksum-type and the full-object checksum header into the signed query string, so a header-only lookup would skip BadDigest validation for them. |
||
|
Chris LuandChris Lu
|
17ad5a1419 |
s3: accept FULL_OBJECT checksums without per-part checksums at complete
COMPOSITE uploads must still carry every part checksum in the complete request, but FULL_OBJECT uploads may instead supply the whole-object checksum in an x-amz-checksum-* request header. Compare that header against the computed object checksum and return BadDigest on mismatch, matching AWS. |
||
|
Chris LuandChris Lu
|
b03419ee92 |
s3: reject UploadPart checksum algorithms conflicting with the upload
An UploadPart that explicitly selects a different checksum algorithm than the one declared at CreateMultipartUpload would store a checksum CompleteMultipartUpload could never accept. Reject the conflict up front with InvalidRequest, matching AWS. |
||
|
Chris LuandChris Lu
|
f849b7c823 |
s3: validate per-part checksums in CompleteMultipartUpload
Parse the Checksum* elements of each completed part and enforce what AWS does for uploads created with x-amz-checksum-algorithm: every part must carry a checksum in the complete request (InvalidRequest when missing, BadDigest when it differs from the stored part checksum), and an x-amz-checksum-type header must match the upload resolved checksum type (BadDigest). Add the issue-11401 reproduction as a regression test. |
||
|
Chris LuandChris Lu
|
f15b980976 |
s3: UploadPart inherits the checksum algorithm of its multipart upload
AWS computes a checksum for every part of an upload created with x-amz-checksum-algorithm, even when the part request carries no checksum headers. Mirror that: when the part request specifies no algorithm, apply the one stored on the upload entry so the part entry keeps a checksum CompleteMultipartUpload can fold into the object checksum. |
||
 hsdfatandGitHub
|
110b485bae |
fix(volume): stop ScanVolumeFileFrom at a header it cannot advance past (#11398)
fix(volume): stop scans at a header they cannot advance past A corrupt .dat header with a very negative size gives a record length (NeedleHeaderSize + NeedleBodyLength) of zero or less: v3 sizes -43..-36 and v2 sizes -35..-28 give exactly zero, and smaller sizes give a negative length. ScanVolumeFileFrom advanced by that length, so it re-read the same header forever or stepped back into the record before it. weed fix, weed export, weed compact, incremental weed backup and the tail sender behind volume.move and volume.merge could hang on such a volume, and weed compact could also finish with a .cpx that had dropped every needle after the header. Return an error wrapping needle.ErrorCorrupted instead. The check runs after the visitor has seen the record, so the rebuild scanner still stops quietly with io.EOF. Smaller negative sizes whose record length is positive are still stepped over, preserving the salvage behavior compaction relies on. Mirror the guard into the Rust volume scans: DatScanPlan::scan and read_all_needles fail on a non-positive record length, as does scan_dat_head, so a corrupt header cannot stall a tail pass or leave the repair scan walking stale offsets. |
||
|
|
06dda12e4b |
fix(volume): validate sizes in ReadNeedleBlob and WriteNeedleBlob (#11399)
* fix(volume): reject negative sizes in ReadNeedleBlob and WriteNeedleBlob A ReadNeedleBlob RPC with a size of -44 or below (-36 on v2 volumes) panics in makeslice inside needle.ReadNeedleBlob. The volume gRPC server has no recovery interceptor, so one request kills the process. Smaller negative sizes return bytes that are not a record. WriteNeedleBlob accepted a negative size whenever the blob header carried the same value: it appended the blob to .dat and indexed the needle with that size, which reads as deleted. Reject size < 0 in both Volume methods. Size 0 still passes, since delete records carry it. The Rust volume server got the same storage guards in #11345. * fix(volume): reject needle blobs whose length does not match their size WriteNeedleBlob appends the blob as is. A blob that is not the length its size implies leaves .dat off the 8-byte grid, and every later ordinary write to the volume is indexed at a truncated offset and reads back as EOF. A blob off by 8 bytes keeps the grid but leaves bytes that a .dat scan reads as the next record. The in-tree callers already send exact lengths. The one case this newly refuses is a copy between volumes of different needle versions, and that case already writes a broken record: a v3 record lands on a v2 volume with 8 extra bytes, and a v2 record on a v3 volume either fails the timestamp check or lands 8 bytes short. This is separate from the negative-size guards, whose Rust counterpart is #11345. The Rust server does not check the length yet. * fix(volume): guard the blob buffer allocation in needle.ReadNeedleBlob Volume.ReadNeedleBlob rejected negative sizes, but needle.ReadNeedleBlob still sized its buffer from the size and is called directly by vacuum and other paths. Reject a deletion marker before make() there too, and use size.IsDeleted() in the volume-level checks. * fix(volume): mirror the blob length check in the rust volume server write_needle_blob_and_index checked the size against the blob header but appended the blob verbatim, so a blob that is not the length its size implies still leaves .dat off the record grid. Match the Go check. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
|
a93a1ab2eb |
fix(volume): return an error instead of 201 when a write lands on no volume (#11397)
* fix(volume): return an error instead of 201 when a write lands on no volume ReplicatedWrite only writes locally when this server holds the volume. For a volume id no server holds, the master lookup returns no locations, so the write went nowhere and the upload still got 201 Created. The same happened for a type=replicate write to a server without the volume, so the primary, or the S3 chunk fan-out, counted a replica that was never written. A server without the volume still forwards the write to the replicas the master lists. When there is nothing to forward to, fail with "volume N not found on host:port". PostHandler returns that as 500, the status the Rust volume server already returns here, and uploaders re-assign on 5xx. Fixes #6609 * volume: reuse Store.HasVolume, drop issue ref from test comment --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
|
cd1e738422 |
[Volume] Scrub local deletion tombstones during FULL scrub (#11396)
* fix 11388 * fix(volume): scrub validates local deletion tombstones TombstoneFileSize (-1) is an .idx-only sentinel; the physical record it points at carries a zero-sized body. Normalize deleted index sizes to 0 via onDiskSize before computing disk usage and calling ReadData, so corrupted or truncated tombstone records are detected instead of skipped. Offset-zero entries (remote logical deletes, no .dat record) remain skipped, and the physical needle id is checked against the index key. Mirror the behavior in the Rust volume server. * fix(volume): scrub preserves physical size of deleted non-tombstone entries Size.Raw()/raw() already encodes the index-to-disk mapping: tombstone (-1) -> 0, other negative sizes -> their absolute value (the offset then points at the original record, per the ReadDeleted path). Use it instead of mapping every deleted size to 0. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris LuandGitHub
|
f6a3286b32 |
fix(volume): derive needle body tail bound from the version layout (#11395)
* fix(volume): derive needle body tail bound from the version layout The size guard in ReadNeedleBodyBytes computed the tail length as checksum, plus timestamp only for Version3. Forks and future on-disk formats whose tail carries more fields would silently under-check and still panic in readNeedleTail on a truncated body. Derive the tail from NeedleBodyLength minus data and padding so the bound stays exact for every version. Iterate IsSupportedVersion in the new tests instead of hardcoding v1-v3 so downstream formats get covered automatically, and skip versions the build cannot write rather than failing on them. * test: skip needle write only on the unsupported-version error A blanket skip would hide a real writer regression. Skip the version subtest only when the writer reports the version is not supported in this build (the error text differs between builds), and fail on any other write error. |
||
|
Chris LuandGitHub
|
01bb3b3053 |
s3api: add Snowflake s3compat API integration tests (#11394)
* s3api: add Snowflake s3compat API integration tests Run the upstream snowflakedb/snowflake-s3compat-api-test-suite against a local SeaweedFS server in CI. test/s3/snowflake/run.sh starts weed server with S3 (-s3.autoCreateBucket=false so missing-bucket PUTs return NoSuchBucket), prepares the fixtures the suite needs (versioned bucket, deny-all-policy bucket, >1000-object prefix), clones the suite, patches it to path-style addressing, and runs mvn -Dtest=S3CompatApiTest. The suite also exposed that GetBucketLocation returned 404 NoSuchBucket for a malformed bucket name; validate the name first and return 400 InvalidBucketName like AWS. * test: harden snowflake s3compat runner per review - Pin the upstream suite to a tested commit (SUITE_REV) instead of the moving default branch - Bind the test server to loopback only - Require the AccessDenied error code when verifying the denied bucket - Fix README so go install runs in a subshell - checkout with persist-credentials: false - Make the concurrency group unique per PR, and widen path filters to the storage/operation/wdclient/cluster/pb packages the S3 stack uses * test: advertise loopback ip for snowflake test server -ip.bind 127.0.0.1 alone left the volume server advertising the host's primary address, so chunk uploads were refused. Also set -ip 127.0.0.1 and disable the Iceberg/Lance listeners so the harness is loopback-only and does not collide with other local services. |
||
|
hsdfatandGitHub
|
5769057af3 |
fix(volume): return an error instead of panicking on a corrupt needle size (#11393)
ReadNeedleBodyBytes sliced the needle body with the size from the needle header without checking it. A corrupted .dat header carrying size -1 still gets a positive body length (16 bytes on v3), so vacuum compaction read that body and panicked with "slice bounds out of range [:-1]". Writers never put a negative size in a .dat header: a delete appends a size-0 record, and TombstoneFileSize only lives in the .idx. Reject a size that is negative or leaves no room for the checksum/timestamp tail with an error wrapping ErrorCorrupted. ScanVolumeFileFrom already logs body read errors and moves on, so compaction now skips the record like any other corrupt needle. Fixes #6763 |
||
|
Chris LuandGitHub
|
37bf1cd91d |
volume: validate copy/tail source addresses before dialing (#11390)
* pb: stop exiting the process on malformed server addresses ServerToGrpcAddress and GrpcAddressToServerAddress called glog.Fatalf when hostAndPort could not parse the port, which os.Exit(255)ed the whole process. A caller-supplied copy or tail source address reached this path synchronously in the serving goroutine, so one anonymous VolumeCopy with a non-numeric port terminated the volume server. Log the parse error and return the input unchanged instead: the dial or request that consumes the address then fails as an ordinary error. * volume: validate copy and tail source addresses before dialing VolumeCopy, VolumeEcShardsCopy and VolumeTailReceiver dial a caller-supplied source address (SourceDataNode / SourceVolumeServer) with no endpoint validation, so an anonymous caller could aim the volume server at loopback, link-local (cloud metadata) or other unintended destinations and read dial behavior back as a connectivity oracle. Apply the same peer-target deny list FetchAndWriteNeedle uses for replica targets: the source must be a bare host:port whose host is not loopback, link-local or unspecified; cluster peers stay reachable on private networks, and -volume.allowUntrustedRemoteEndpoints opts out. The loopback-using copy tests set the flag to keep exercising the copy path in process. * rust volume: validate copy and tail source addresses before dialing Mirror the Go guard on the Rust volume server: volume_copy, volume_ec_shards_copy and volume_tail_receiver dial a caller-supplied source address, so run it through validate_replica_target first (bare host:port; no loopback, link-local or unspecified hosts; private peers stay allowed). --volume.allowUntrustedRemoteEndpoints opts out; the test fixture and the Rust test-cluster launcher set it so loopback sources in tests keep working. * volume: pin validated copy/tail source addresses at dial time validateReplicaTarget resolves the source hostname once, but the gRPC client resolved it again at connect, leaving a DNS-rebinding window for hostname sources. The copy and tail source dials now run through the same guardedDialerPolicy the remote-storage path uses, so every resolved address is re-checked against the replica deny list (private peers allowed) immediately before the TCP connect. guardedDialerPolicy also moves to util.OutboundDialContext so the guarded path keeps the -ip.bind source binding the default gRPC dialer had. The Rust volume server mirrors this with connect_guarded, a tonic connector that resolves, re-checks each address, and connects to the first passing IP; handlers use it whenever the untrusted-endpoint opt-out is off. A handler-level test now exercises the enabled validation branches for all three source-taking RPCs. * pb: return empty server address for malformed grpc addresses GrpcAddressToServerAddress used to return the unparseable input on a hostAndPort failure, so a malformed raft address (e.g. "host:abc") flowed into admin dashboard master maps unchanged. Return an empty string instead, skip empty conversions at the two raft-cluster merge sites, and drop the now-stale comment about the fatal exit the earlier commit removed. * test: opt erasure-coding loopback clusters out of the remote endpoint guard The erasure-coding suites drive VolumeEcShardsCopy / VolumeCopy between volume servers bound to 127.0.0.1, which the copy/tail source guard now rejects by default. Pass -volume.allowUntrustedRemoteEndpoints to the test volume launches, matching what the volume_server framework harnesses already do. * admin: only claim fallback master leadership on an empty raft response A nonempty RaftListClusterServers response whose entries were all rejected left masterMap empty, so the fallback marked the reachable current master as leader the same way a genuinely empty (non-raft) response does. Track whether the successful response returned zero servers and only promote the fallback master then. |
||
|
Chris LuandGitHub
|
a6d72bc272 |
s3api: delete orphaned chunks only when the entry is confirmed absent (#11389)
* s3api: test for chunks deleted under an entry the filer committed Issue #11387: the filer can report a create failure after inserting the entry (e.g. a parent-directory creation failing post-insert). The error arrives in the response rather than as a transport status, so it maps to a definitive error and putToFiler deletes the chunks of the live entry. * s3api: confirmCreateLanded also reports a confirmed-absent entry The verification a failed create runs can answer both directions: the entry matching the uploaded chunks proves the write landed, and an authoritative not-found proves the uploaded chunks are orphaned. Return both outcomes so the cleanup path can gate on the fact rather than the error class. An empty upload can never prove a landing, so a zero-chunk entry match no longer upgrades the outcome. * s3api: delete orphaned chunks only when the entry is confirmed absent A failed create no longer skips verification based on the error class: the filer can fail after inserting the entry (issue #11387) and a partially-applied routed transaction can leave it behind too, both surfacing as definitive errors. Every failed create now resolves the entry's fate, and the uploaded chunks are deleted only when the entry is confirmed absent; anything unverifiable keeps them for vacuum. * s3api: confirm absence on every filer the create could have committed on A lock-path create fails over across filers, so the entry can live on a replica the routed owner has not caught up to; one not-found does not prove absence. The confirmation now queries the owner, the prior owner, and the failover set, declaring absent only when none of them has the entry. * s3api: bound the reconciliation lookups confirmCreateLanded runs The lookups ran on context.Background() under the object write lock, so a connected filer that never replies could stall the write path. One timeout now covers the whole enumeration; an expired budget fails the remaining lookups as uncertain, which keeps the chunks. |
||
|
Chris LuandGitHub
|
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 LuandGitHub
|
87ee3b63a2 |
s3: abort completed multipart uploads metadata-only (#11385)
* s3: abort a completed upload's leftover directory metadata-only A .uploads/<id> directory can outlive the object it completed into when the commit's metadata-only removal failed or the gateway died in between; the restored part entries then share chunks with the published object. AbortMultipartUpload deleted the directory recursively, chunks and all, so aborting such a leftover destroyed a committed object (#11382). Run the same check s3.clean.uploads gained in #11375 before deleting: when the object entry or a version file under <key>.versions carries the upload id, remove .uploads/<id> metadata-only and answer the abort; when the lookup cannot decide, refuse with InternalError rather than risk live chunks. * s3: apply the completed-upload check to lifecycle MPU abort lifecycleAbortMPU ran the same destructive recursive delete on .uploads/<id>. Reuse uploadCompleted so a leftover whose object entry or version file carries the upload id is removed metadata-only, and an undecidable lookup retries later instead of freeing live chunks. * s3: serialize abort's upload-dir delete with the object's commit The completed check alone leaves a race: abort can read completed=false, then an in-flight completion publishes the object over the same part chunks before the recursive delete frees them. Run the check and delete inside the object write lock, which non-routed completions hold for their whole finalize. With an owner, send the data delete as an ObjectTransaction on the object's lock key — a routed commit then either loses its upload-exists precondition after our delete or has already stamped the object, which the transaction's IF_EXTENDED_NOT_EQUAL condition detects and falls back to a metadata-only remove. lifecycleAbortMPU shares removeUploadDir so both callers get the same ordering. * s3: check for an empty object before resolving its write owner * s3: check completion at the abort's resolved object key An upload record missing ExtMultipartObjectKey skipped the completed check entirely even though the request's Key names the object. |
||
|
Chris LuandGitHub
|
0ca1c19821 |
s3api: unify auth error handling across s3tables, iceberg and lance (#11381)
* s3api: fail closed when S3 Tables signature verification fails * s3api: avoid nil Account dereference in S3 Tables auth log * iceberg: return auth error instead of falling back to DefaultAllow * lance: return auth error instead of falling back to DefaultAllow * s3api: stop trusting client-supplied s3-account-id The header is set by the server after successful authentication; scrub inbound values alongside the other internal headers, and apply the same admin guard to the header fallback branch of getAccountID that the identity branch already has. * test: cover table-catalog auth wrappers and principal resolution * test: configure anonymous identity where catalog clients do not sign * s3api: scrub s3-account-id after signature verification |
||
|
Chris LuandGitHub
|
f40687b34e |
s3: tighten STS session token handling (#11383)
* s3api: test that a session token must not reveal its credential * sts: derive secret access key with HMAC keyed on the signing key * s3api: stop accepting STS session tokens as bearer credentials * security: reject STS session tokens on filer and admin gRPC auth * test: sign s3/iam framework requests with the session credential * s3api: exercise the real auth pipeline in the end-to-end harness |
||
|
Chris LuandGitHub
|
15520f601f |
s3: commit multipart upload and remove .uploads atomically; purge completed uploads metadata-only (#11375)
* s3: commit versioned multipart upload in one transaction CompleteMultipartUpload wrote the version file, flipped the .versions pointer, then removed .uploads/<id> metadata-only as a best-effort post-commit step. A filer error or gateway crash in that window left the upload directory referencing the same chunks as the published object, and the next s3.clean.uploads run purged it with data -- corrupting a committed object. Put the version file, remove the upload directory metadata-only (its chunks are the object's chunks), and recompute the latest pointer in one ObjectTransaction under the object's per-path lock on the owner filer. The mutation order keeps every partial state safe: the chunks stay referenced at all times, and a published object never coexists with the upload directory the cleaner would purge. Unused part entries are freed before the transaction, since the metadata-only directory delete would otherwise leak their chunks. * s3: remove upload directory inside the multipart object PUT The same committed-object/stranded-upload window existed on the suspended and non-versioned paths: writeMultipartObject committed the object, then a best-effort rm dropped .uploads/<id>. Ride the metadata-only removal on the routed PUT itself so the two land in one transaction; the unrouted mkFile fallback keeps post-commit cleanup. * shell: purge completed uploads metadata-only in s3.clean.uploads A leftover .uploads/<id> can outlive a committed object when the completion's metadata-only delete fails or the gateway dies in between; its part entries then share chunks with the live object, and a recursive purge frees them out from under it. Before purging a stale upload, check whether it completed: the object entry or any version file under <key>.versions carrying the upload id. If so, delete with skipChunkDeletion. If the lookup fails, skip the upload for this run rather than risk live chunks. * s3: abort multipart completion when unused part cleanup fails Deleting the upload directory metadata-only erases the only metadata pointing at part entries whose deletion failed, orphaning their chunks. Propagate the error so the completion fails while the upload directory still exists and the request remains retriable. * s3: require the upload directory to exist at multipart commit A delete that does not take the object lock (abort, lifecycle, s3.clean.uploads) can remove .uploads/<id> and its chunks between the prepare step and the commit transaction. The commit now carries an IF_EXISTS precondition on the upload directory so the race fails the request with NoSuchUpload instead of publishing an object over freed chunks. * s3: keep the version file when the upload directory is gone The finalize transaction has no rollback, so a failure at the latest-pointer recompute leaves the version written and .uploads/<id> removed. Deleting the version then destroys the only remaining record of the upload, making a retried CompleteMultipartUpload return NoSuchUpload while the version's chunks leak. Roll back only while the upload directory survives; otherwise keep the version, which a retry resolves through SeaweedFSUploadId and the version reconciler promotes. * s3: keep manifests when a routed object write partially commits For non-versioned and suspended completions the object PUT precedes the upload-directory DELETE, so an error can mean the object entry exists while the response reports failure. Freeing this attempt's manifest chunks then destroys the committed object. Keep them when the object entry survived, and after a failed null-marker finalize which always follows a committed write. * s3: skip the keep-version path on precondition failure A rejected precondition means no mutation ran, so there is no version file to preserve and this attempt's manifests are orphans the error cleanup should free. * s3: keep manifests when the object-existence check itself fails A transient lookup error previously read as absent, letting the error cleanup free manifest chunks a committed object still references. * s3: keep the upload directory when post-commit part cleanup fails Removing it metadata-only after a failed entry delete erases the only reference to the leftover chunks. Leave the directory so the entries keep their chunk references for s3.clean.uploads or manual recovery. * pb: fix filer list entry counting on 32-bit int(limit) wraps to -1 on 386 when limit is math.MaxUint32, so the beyond-limit check discarded every streamed entry. Compare in uint64 instead; the semantics are unchanged on 64-bit platforms. * shell: resolve trailing-slash object keys in s3.clean.uploads Completion stores a key ending in / inside the directory it names (<bucket>/dir/dir), but FullPath+DirAndName on the normalized key looked one level too high. Deriving dir and name with path.Dir and path.Base mirrors getEntryNameAndDir so the completed-upload check finds the entry instead of purging its chunks. * s3: heal a suspended completion hidden behind a delete marker Removing .uploads/<id> inside the commit transaction means a failed finalizeSuspendedNullWrite leaves nothing to retry against: the object entry is committed but the marker still makes the key read as deleted, and a retried CompleteMultipartUpload can only report NoSuchUpload. When the upload directory is gone, check the regular path for an entry carrying the upload id and re-run the marker finalize, so the retry both succeeds and repairs the key. Only suspended buckets can hold this state; anything newer owns the key. * s3: report store errors when resuming a committed multipart object |
||
|
Chris LuandGitHub
|
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 LuandGitHub
|
ce1e0dc30a |
s3api: don't delete chunks when CreateEntry outcome is ambiguous (#11376)
* s3api: map ambiguous filer transport errors to retryable 503 Canceled, DeadlineExceeded and Unavailable can be returned after the filer applied the write, so the outcome is ambiguous. Reporting them as a 4xx tells the client not to retry; report ServiceUnavailable instead. * s3api: verify entry existence before deleting orphaned chunks A failed CreateEntry can still have landed on the filer when the error is a transport failure, and entryCreated=false would tombstone chunks a live entry references, leaving a dangling pointer that survives only because reads pass readDeleted=true until vacuum reclaims the needle. Before deleting, look the entry up: if it is stored with the same chunks, the write succeeded; if the lookup cannot be answered, keep the chunks for vacuum to reclaim; only a confirmed absence still cleans up. * s3api: regression tests for ambiguous CreateEntry outcomes Covers the three post-create-failure cases in putToFiler: the entry landed despite the error (treat as success, keep chunks), the entry is confirmed absent (delete orphans), and the outcome is unverifiable (keep chunks, return error). * volume: count reads served from deleted needles A readDeleted read succeeding on a tombstoned needle is the signal that metadata still points at deleted data. Count it under a readDeletedNeedle handler label in both the Go and Rust volume servers so the condition is visible before vacuum turns it into a 404. * s3api: never delete chunks on an ambiguous create error Review feedback on the first fix showed verification could still go wrong in both directions: a stale or lagged lookup could report not-found for a committed entry, a prefix object stores its chunks on a directory entry, and filer-side manifestization rewrites the top-level chunk ids the comparison relied on. Rework the rule so the outcome classes are asymmetric: - A transport-level error (anything filerErrorToS3Error maps to a retryable 503) is ambiguous and never deletes chunks; the lookup can only upgrade the write to success. - Any other error is a definitive filer refusal and still cleans up. confirmCreateLanded asks the write owner first, resolves the stored entry through chunk manifests, requires an exact match of the uploaded file ids, and on success runs the finalize callback the failed create skipped (under the object write lock, with the same rmObject undo the create path uses). Zero-chunk writes stay ambiguous since they cannot be told apart by chunks. * s3api: cover definitive refusals and stale entries in put tests The confirmed-failure case now uses a definitive refusal so it still exercises orphan cleanup, and a new case keeps chunks when the stored entry belongs to an older object rather than this PUT. * volume: count deleted-needle reads once per request Streamed Go reads ran the deleted check in readNeedle and again in readNeedleDataInto, and non-streamed Rust reads in stream_info and the full-read fallback, double-counting one request. Count at the single entry probe each implementation takes per GET: readNeedle in Go, read_needle_stream_info in Rust. * s3api: run recovered-write rollback under the object lock Two follow-ups from review: ResolveChunkManifest returns traversed manifest blobs in its manifestChunks output, so requiring it empty rejected every manifestized landing; and the rmObject undo ran after the object write lock was released, so a concurrent newer write could be deleted between finalize failure and rollback. Compare only the resolved data chunks and keep the undo inside the lock. * s3api: verify, finalize and roll back recovered creates in one lock A lookup done before the object write lock let a concurrent PUT replace the entry between the chunk comparison and the finalize/rollback section, so a failed afterCreate could rmObject a newer write. Run the owner lookup, manifest resolution, chunk comparison, afterCreate and the conditional undo inside a single withObjectWriteLock section. |
||
|
Chris LuandGitHub
|
4ec564469a |
s3tables: hide inaccessible catalog resources (#11365)
* s3tables: hide inaccessible table buckets * s3tables: hide inaccessible namespaces * s3tables: hide inaccessible tables * s3tables: hide inaccessible resources in rename and namespace delete RenameTable/RenameView denied on the source now report the same not-found as a missing source, and the destination name conflict is checked only after destination authorization so a denied caller cannot distinguish an existing destination namespace or name from a missing one. DeleteNamespace denials use the same formatted message as a missing namespace. |
||
|
Chris LuandGitHub
|
66f1754896 |
s3: enforce dedicated Object Lock actions (#11362)
s3: enforce dedicated object lock actions |
||
|
Chris LuandGitHub
|
994e1f7d64 |
admin: replace Font Awesome with MIT-licensed icons (#11364)
admin: replace Font Awesome with MIT icons |
||
 David ChristopherandGitHub
|
1a285c1334 |
filer: join shutdown paths before closing metadata store (#11363)
Serve can return when its listener closes while HTTP requests are still draining. The main path could then close the metadata store before those requests finish. Make signal, context, and Serve-exit paths join one shutdown sequence. Drain gRPC and HTTP concurrently with 15-second default limits, then close the store. Test both completion orders. |
||
|
Chris LuandGitHub
|
3ebc05930d |
s3: separate Object Lock configuration permission (#11361)
* s3: separate object lock configuration permission * test: synchronize manifest cancellation setup |
||
|
Chris LuandGitHub
|
0c7beec697 | server: add filer-specific disableHttp flag (#11360) | ||
|
David ChristopherandGitHub
|
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. |
||
|
71f8128d75 |
shell: fs.verify -pruneEntries deletes entries whose needles are lost (#11338)
* shell: fs.verify -pruneEntries deletes entries whose needles are lost
* shell: harden fs.verify -pruneEntries guards; VolumeNeedleStatus returns NotFound for absent needles
* shell: resolve chunk manifests in fs.verify metadata path; require confirmed deletion before counting prunes
* shell: anchor fs.verify legacy missing-needle error matching
* shell: keep fs.verify metadata scan alive on manifest resolution failures
* shell: classify EC missing needles and keep manifest failures unverified
VolumeNeedleStatus now canonicalizes erasure_coding.NotFoundError to
codes.NotFound, so absent needles in EC volumes reach the prune path
through the same stable contract as regular volumes. The client-side
isNeedleMissingError keeps recognizing the legacy wrapped EC shape
("locate in local ec volume: ... needle not found") for mixed-version
clusters.
A chunk manifest that fails to resolve is now an entry-level
verification failure even when the raw top-level chunks are healthy:
the file is not fully readable without the manifest. Raw chunks are
still verified on a resolution failure so a missing top-level manifest
needle is classified and can be pruned. The per-entry logic is
extracted into resolveAndVerify for testability.
* shell: trim fs.verify prune comments
---------
Co-authored-by: Chris Lu <chris.lu@gmail.com>
|
||
 
|
1f037e48f9 |
s3: a list marker that sorts before the prefix excludes nothing (#11322)
* s3: a list marker that sorts before the prefix excludes nothing ListObjects `marker` and ListObjectsV2 `start-after` are a plain key cutoff: list the keys that sort after it. A marker that sorts before the prefix and is not under it therefore excludes no key carrying the prefix, and the listing must equal the one with no marker at all. normalizePrefixMarker treated every marker that does not start with the prefix as "something wrong" and the listing came back empty. Clients send this shape routinely: docker/distribution's S3 storage driver walks prefix "<root>/<path>/" with start-after "<root>" (its rootdirectory), so on SeaweedFS a registry walk saw an empty bucket. zot read that as "no repositories": /v2/_catalog was empty, GC/scrub/retention never saw a repo, and on restart its storage parse deleted every repository's metadata as "no longer in storage". listFilerEntries now lists as if no marker were given when the marker sorts before the prefix; the response still echoes the marker the client sent. A marker that sorts after the prefix's subtree is left alone: it may legitimately sit inside a partial-name prefix's match set, which normalizePrefixMarker already handles, and otherwise correctly lists nothing. Reproduce on 4.44 and 4.47: curl -s "$S/zot?list-type=2&prefix=zot/zot/&start-after=zot/zot/" # all keys curl -s "$S/zot?list-type=2&prefix=zot/zot/&start-after=zot" # KeyCount 0 curl -s "$S/zot?list-type=2&prefix=zot/zot/&start-after=a" # KeyCount 0 * s3: keep the prefix's own key excluded by a marker that names it Fold the before-prefix marker rule into normalizePrefixMarker, which now also derives prefixEndsOnDelimiter from the effective marker instead of each cursor rebuilding the expression. A marker equal to the prefix is no longer trimmed to a subtree cutoff: start-after "a/b/" with prefix "a/b/" excludes only the "a/b/" key, so the walk starts inside that directory and its children still list. Adds a listing-level test that walks the whole path for both start-after shapes a registry sends, and covers the new normalization cases. * s3: leading slashes do not hide a marker that names the prefix * s3: echo the V1 marker the client sent, not the walk's cutoff * s3: filter only the walk's cutoff from the V1 page, not the echoed marker * s3: skip the key an exclusive marker names as it streams --------- Co-authored-by: Zuse <be9c90a8-c104-4be2-b7a4-9f92eb833ac8@forge.local> Co-authored-by: Chris Lu <chris.lu@gmail.com> |