mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-11-02 18:22:40 +01:00
bcb3940b378ea4f7e548ad8cd5774bc32c4ae0fe
10417
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Chris LuandClaude Opus 5.5
|
5a60ce04e3 |
vector: stop re-parsing a filter's JSONPath for every scanned row (#12302)
* vector: parse a filter's JSONPath once per batch instead of per row
Lance's json_extract and json_exists parse their JSONPath argument again
for every row they see. Every translated S3 Vectors filter goes through
them, so a flat scan paid about 1 µs per row before the predicate
compared anything, and a 10%-selective {"category": {"$eq": "c3"}} ran
about 50% slower than the same query unfiltered.
The path is always a literal, so the scanner now parses the filter as
before and swaps in copies of the two functions that parse it once per
batch. They call the same jsonb selector and serializer, so results and
errors do not change. Calls whose path is not a literal keep Lance's
functions. The filter SQL is unchanged, so the gateway, existing tables
and the distributed workers (which run query::ann) need no change.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK
* s3vectors: read the field once per extractor in $in and $nin
$in and $nin rendered one IN per member, each with its own json_extract,
so an N-member list parsed the row's metadata N times. Members that share
an extractor (strings and booleans, or numbers) now go into one IN list,
so a list reads the field at most twice. Matching is unchanged: each
group still only sees its own JSON type and still coalesces NULL before
$nin negates it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK
* s3vectors: rename the $in literal map so codespell passes
Codespell reads "lits" as a misspelling of "list".
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
Chris LuandClaude Opus 5.5
|
a1a208506e |
volume: serve a ranged GET without reading the whole needle (#12301)
* volume: serve a ranged GET without reading the whole needle A ranged GET read every needle up to 1MB in full, checksum and all, then copied out the requested bytes. Read the metadata and stream only the range instead, as the Go server already does above 1MB and the Rust server does at any size. A small range of a 790KB needle goes from ~340us to ~160us. The metadata read stays non-terminal: the range read that follows reports the io outcome, so a failing disk still builds its error streak. HEAD, compressed needles and chunk manifests keep their current reads. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * volume: read the whole needle for a range that may need an image transform Without a URL extension, the stored name can still make a ranged GET with width, height or crop parameters an image transform, which needs the payload. Keep the full read there instead of streaming the range. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * volume: stream a range from the record its metadata came from The streamed read looked the needle up again, so a replacement written after the metadata read sent new bytes under the old length, checksum and headers. Read the record the metadata read resolved, as the Rust server does, and after a compaction refuse a record whose size changed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * rust volume: read the whole needle for a range that may need an image transform A ranged GET with no URL extension but width, height or crop parameters took the metadata-first path, so a stored image name never got its transform. Read a small needle whole there, as the Go server does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * volume: keep later parts of a multi-range reply on a moved record When a compaction moved the record before one part of a multi-range reply, that part re-resolved it, but the next part went back to the offset the metadata read saw. Remember the re-resolved offset too. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * volume: check a compaction-moved record is the one the metadata came from After a compaction, a range re-resolved the needle and accepted any record of the same size, so a replacement written in between could send its bytes under the old headers. Compare the moved record's checksum and append time with the metadata already read, and fail otherwise. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * volume: accept a re-encoded checksum and count read errors on a moved record Compaction rewrites a pre-3.09 checksum in the current encoding, so the moved-record check now accepts either. A failed read of the moved record's metadata now feeds the io-error streak and surfaces as that error instead of not-found. The pinned record is a fresh value rather than a write through the index's pointer. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * volume: keep a moved record's wrapped offset when a range follows it With 4-byte offsets, a record past the 32GB wrap is read at offset plus MaxPossibleVolumeSize. The compaction refresh dropped that, so it checked and streamed the wrong location. Retry the wrapped offset in the moved record check, as the metadata read does, and apply it to the stream. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Chris LuandClaude Opus 5.5
|
4be191173a |
vector: keep ListVectors pages and segments flat as an index grows; read only keys in GetVectors (#12300)
* vector: scan only the fragments a ListVectors page covers A page pushed its offset into the scan, but the scan still loaded every fragment first, reading each one's deletion file: about 105 S3 GETs per page at 50k rows against 25 at 10k. The page's fragments now come from the live row counts in the manifest, so a page reads only its own. Tokens and row order are unchanged; a manifest without the counts keeps the old scan. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * s3vectors: list a ListVectors segment in the engine The gateway paged through the whole index for every segment and kept one row in segmentCount, so a segment cost segmentCount engine pages and 16 segments listed slower than one. The engine now takes segment_count and segment_index: it reads keys across the stride, keeps the segment's rows and takes their data and metadata alone, one call per page. A segment is still the rows whose position modulo segmentCount is segmentIndex, and its token still the position to resume from. An engine that predates the fields does not set segmented, and the gateway then pages through it as before. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * vector: read only the key column across the table in GetVectors The key lookup scanned every column. On S3, lance reads a column during the scan when a row of it is under 1000 bytes, so a 128-dimension vector was read for every row of the table to return 100 of them, and data and metadata were read even when the request did not ask for them. The scan now projects the requested columns and materializes everything but the key late, for the matching rows only. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * s3vectors: start the legacy segment fallback from the page already read With an engine that ignores segment_count, the gateway dropped the page it had just read and fetched it again. The fallback now consumes that page first, so a one-row segment page still costs one engine call. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * vector: check metadata on segmented ListVectors pages The segment test listed data only. Its rows now carry metadata on two in three keys, and every segmented row, read through take_rows, must equal the same row in a plain listing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * vector: lease the version while a segment takes its data A segmented page with data or metadata reads keys first and then takes the other columns from the same version, so maintenance could reclaim its files in between. That path now holds a lease like GetVectors. A keys-only page is one scan and stays unleased, as plain pages are. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * vector: leave GetVectors materialization to lance Forcing every column late made a 100-key GetVectors with data slower at 10k x 128 in the perf cluster: 54.7 ms against 46.3 ms. The take issued a read per row, where lance's heuristic reads a narrow vector column in one pass per fragment. The projection keeps the gain on its own: with neither data nor metadata requested, the call went from 38.6 ms to 25.3 ms and from 150 to 89 S3 GETs. At 50k x 384 the vector is already late under the heuristic, and a full lookup is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Chris LuandClaude Opus 5.5
|
ab355b43b9 |
s3tables: keep the vector-namespace fence on a declare of an existing index (#12293)
s3tables: keep the fence on a declare of an existing index Deciding the existing-index exception before the handler ran left a window: an index deleted between that lookup and the handler's own check would be recreated, unfenced, by a caller without s3vectors:CreateIndex. Drop the up-front exception, so every CreateTable runs fenced and a fresh create is refused at its first write. The engine's declare still passes because the idempotent path writes nothing; for a build whose idempotent path re-applies state already in place, the fence answers an UpdateEntry that leaves the entry exactly as stored without writing. Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Chris LuandClaude Opus 5.5
|
e2826c48bd |
s3tables: make the vector namespace of a vector bucket read-only (#12292)
* s3tables: make the vector namespace of a vector bucket read-only S3 Vectors authorizes index operations against the vector bucket policy, but an index is also a catalog table in the bucket's "vector" namespace, and the S3 Tables API only checks s3tables grants. A caller the vector policy denies DeleteIndex could still delete the index, and its data, with DeleteTable. Untrusted writes that name the vector namespace of a LANCE bucket now run behind a fence that passes filer reads and refuses filer writes. The operation's own checks still answer everything short of a write, so a caller without the grant gets its usual denial and learns nothing about the bucket, and a CreateTable that finds the index already there, which is what the vector engine's declare does, still succeeds. A refused write answers 403 AccessDenied. Trusted callers (the gateway, the shell) are unaffected, and so is a namespace named vector in an Iceberg bucket. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * lance: refuse register and deregister in a vector bucket's vector namespace Every catalog write the Lance namespace makes goes through the S3 Tables handler and its vector namespace fence, but register over an existing table at the same location and deregister only add or remove a marker file in the dataset directory. Check the vector namespace before those writes, after the table read that already gates them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3tables: refuse deferred creates in a vector bucket's vector namespace Manager.AuthorizeCreateTable gates the Iceberg create paths that write metadata into the table bucket before the catalog registers the table. Those writes never pass the handler's fence, so the gate applies the same read-only rule once the caller's own create permission is confirmed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3tables: let a declare of an existing index past the vector namespace fence The vector engine declares every index the gateway has just created, and that CreateTable only reaches its idempotent path. Upstream that path writes nothing, so the fence never trips, but a build whose idempotent path repairs table state would refuse the engine. Decide the exception up front instead: a CreateTable naming a finished index in the vector namespace runs unfenced, while a new name or an unfinished create stays fenced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Chris LuandClaude Opus 5.5
|
e118ba7127 |
vector: keep S3 Vectors requests off the master-discovery lock (#12291)
* vector: throttle worker discovery from lookup end cli() set nextResolve before resolve() ran, so a lookup that takes as long as the throttle interval (an unreachable first master costs the full 5s dial timeout) left the next lookup already due when it returned. Discovery then ran back to back and, since it holds d.mu, every vector request queued behind it. resolve() now sets nextResolve when the lookup lands, using the short recheck window only while no client is cached. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * vector: resolve workers outside the discovery lock resolve() held d.mu across the master lookup, so while it waited out an unreachable master every vector request blocked on cli(), even with a healthy worker already cached. The lookup now runs in a goroutine without d.mu, and d.resolving keeps it to one at a time. Callers with a cached client use it while the lookup is in flight; callers without one wait for that lookup and share its result. Throttling, the sticky worker and reset(c) are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * vector: keep the cached worker connection when a redial fails resolve() closed d.conn before dialing the replacement worker. A synchronous dial error then left d.client bound to a closed connection. Requests got codes.Canceled, which never triggers reset, and later refreshes saw the unchanged d.worker and never redialed. The old connection is now closed only after the new dial succeeds. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Chris LuandClaude Opus 5.5
|
9a09506be6 |
s3vectors: bound ListVectors parameters and lowercase distanceMetric (#12290)
* s3vectors: bound ListVectors maxResults and segment parameters segmentCount had no upper bound, so segmentCount=1000000, segmentIndex=999999 made the segment emulation scan the whole index to return nothing. Apply the AWS ListVectors windows before authorization and any engine call: maxResults 1..1000, segmentCount 1..16, segmentIndex in [0, segmentCount), and segmentCount and segmentIndex given together. The fields become pointers so an explicit 0 is rejected while an omitted one still takes the default. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3vectors: store and return distanceMetric in lowercase CreateIndex stored distanceMetric exactly as sent, so an index created with "COSINE" came back as "COSINE" from GetIndex and QueryVectors. AWS returns the lowercase enum values. Lowercase the metric when the index is created, and again when the record is read so indexes already stored in mixed case render the same. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Chris LuandClaude Opus 5.5
|
21fc46692c |
s3vectors: authorize CreateVectorBucket as a vector action, close DeleteIndex race, fold policy service case (#12287)
* s3vectors: authorize CreateVectorBucket as an s3vectors action CreateVectorBucket went through the catalog's CreateTableBucket check, so an identity granted s3vectors:* was denied the create while s3tables:* was allowed it. Check s3vectors:CreateVectorBucket in the gateway, the same way the catalog checks a bucket create, and create the record on the internal manager under the caller's name so the caller stays the owner. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3vectors: delete only the index record DeleteIndex checked DeleteIndex read the record, checked it is a Lance index, then deleted by name. A table that replaced the index in between was deleted too. Send the checked record's version token, so a replaced record fails the delete with a 409 ConflictException. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3tables: reject s3vectors rules in any letter case validatePolicyDocument kept s3vectors actions and resources out of the shared policy slot with case-sensitive checks, so "S3Vectors:GetIndex" was stored as a rule that never takes effect. IAM service prefixes are case-insensitive; fold case in both checks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Anton Rozanov
|
e7f5c6873a |
ec.balance: spread concurrent shard moves over target nodes (#12283)
* ec.balance: spread concurrent shard moves over target nodes The planner gives every freshly encoded volume the same target order, so running each volume's moves in plan order sends all concurrent copies to the same node while the other nodes idle. Pick, among the volumes not already moving, the pending move whose target receives the fewest moves right now. One volume still never has two moves at once, and moves of one shard keep their plan order. * ec.balance: let a move into a node wait for the move of its volume leaving it The planner counts the slot an outgoing shard frees as room for a later incoming shard of the same volume, and the receiver refuses a shard it has no slot for. Make the spread tests deterministic: hold the first moves until all have started, and check that errors of several volumes are all returned. * ec.balance: pin that the spread moves run concurrently and in plan order without parallelism |
||
|
Chris LuandClaude Opus 5.5
|
3a8d756bdd |
s3: drop retired volume servers from the gateway location cache (#12281)
* wdclient: publish applied volume-location deletes to registered listeners The master already tells each connected client which volumes a server stopped hosting; only the MasterClient's own vidMap applied it. Expose the same filtered deletions (moves between a server's disks excluded) through OnVolumeLocationDelete so clients keeping a second location cache can drop the retired server too. * wdclient: drop retired volume servers from the FilerClient location cache The S3 gateway's FilerClient keeps its own vidMap, filled by filer lookups, and only forgot a retired volume server after a read against it failed (a ~15s header-timeout stall when the host still resolves). When built with a MasterClient, subscribe to its applied deletions so the cache forgets the server as soon as the master reports it gone. * wdclient: drop retired EC servers from the regular location map too Filer lookups cache EC locations alongside regular ones in vid2Locations, so an EC removal has to clear the retired server there as well as in the EC map for the delete to reach what reads actually use. * wdclient: tombstone pushed deletions against in-flight provider fills A filer lookup already running when a delete lands can return the retired location afterwards and re-cache it. Keep a short tombstone on each deleted (vid, url) pair so fills neither cache nor return it while the lookup source is still settling. * wdclient: clear the FilerClient location cache on master reconnect Removals the master pushed while the gateway link was down never arrive as deletes, so on reconnect the lookup-fed cache is dropped outright rather than kept generation-wise like the stream-fed one, and relearned through filer lookups as before. * wdclient: invoke push hooks outside their registration locks A callback that registers or replaces a hook would wait on the read lock the invocation already holds and stall the master stream. Copy the hook under the lock and call it after the lock is released. * wdclient: report pushed adds so confirmed re-adds clear the suppression A server coming back at the same URL inside the tombstone window stayed hidden from provider fills. updateVidMap now carries the additions it applied through the same hook (renamed OnVolumeLocationChange), and the registered cache untombstones them without eagerly caching them. * wdclient: amortize tombstone expiry over a delete batch A server leaving takes every volume it hosted with it, and scanning all recentDeletes per removed vid squared the batch. Prune lazily on reads and at most once per half TTL on writes. * wdclient: let every location cache subscribe to pushed volume changes The setters were single-slot, so a second FilerClient built on the same MasterClient would have silently replaced the first one's subscription. Registration now returns an unhook func, delivered to all listeners, and FilerClient.Close releases its own. * wdclient: hold the deletion fence until the master re-confirms a location * wdclient: check the deletion fence and the cache add under one lock * wdclient: drop provider answers that crossed a cache reset * wdclient: bound the deletion fence by evicting its oldest mark * wdclient: refresh a repeated removal instead of evicting another mark * wdclient: evict the oldest deletion-fence mark in constant time * wdclient: fence a server only once it serves the volume in no form ec.encode registers the shards before the regular copy is deleted from the same server, and ec.decode mounts the regular copy before the shards come off it. Both arrive add-first in separate heartbeats, so the later removal tombstoned a (vid, url) pair the server still serves and dropped it from the FilerClient map; the filer's answer naming that server was then fenced out and the volume read as not found until the gateway reconnected. Tombstone and report a removal only when neither map still names the url for that volume. * wdclient: serve a provider answer that crossed a cache reset The answer is as fresh as the provider's view; only the cache it would land in has been re-told. Dropping it failed every read whose lookup was in flight during a master reconnect with 'volume not found'. Return it and leave the new view to be filled by the next lookup. * wdclient: test the moved volume whose old server keeps proxying * wdclient: stop caching inside the master lookup provider The provider wrote every answered location through addLocation, which clears the deletion fence: a lookup still naming a server a pushed delete had just retired untombstoned and re-cached it. Fallback lookups already gate caching through fillLocation, and authoritative lookups bypass the cache by contract, so the provider no longer writes the map at all. * wdclient: count only master-pushed locations when suppressing a removal hasLocation could not tell a confirmed regular copy from a lookup fill: the master answers an EC volume with its shard servers, so the regular map holds shard locations that arrived by lookup, and entries an earlier generation learned linger past a reconnect. An EC removal whose url still showed either was suppressed, leaving the retired server served. The vid map now records the generation each regular location was pushed at, drops an unconfirmed regular entry alongside the shard that fed it, and answers hasServingLocation from pushed marks and current-generation entries only. * wdclient: test removals against lookup-cached and stale locations * wdclient: fence removals only in the gateway cache The fence and the pushed-location marks ran inside every MasterClient, including the filer's, whose map is also filled by lookups and cleared by InvalidateCache. A removal reported while the server still served the volume (e.g. a decoded copy whose mark a failed read had wiped) left the pair fenced until the master pushed it again, with reads failing as volume not found meanwhile. Only the S3 gateway's FilerClient now tombstones, from the removals its push-only MasterClient reports. That client decides a removal from the current generation of both its maps after the whole message is applied, so pushedLocations goes away, and the master lookup provider caches as before. The hooks follow the existing single-slot SetOn...Fn pattern and are wired next to SetOnPeerUpdateFn. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * wdclient: let a removal fence expire after a minute The master drops a broadcast when a client's channel is full, so the re-add that would lift a fence can be lost while the gateway stays connected. A mark held until that re-add then hid a live server, and a volume whose only copy it held read as not found until the next master reconnect. Bounding marks by a minute still outlasts a lookup in flight when the removal landed; expiring early only falls back to learning by a failed read, as before this change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Chris LuandClaude Opus 5.5
|
fdbaba52f9 |
s3 sink: forward user metadata to the destination object (#12286)
* s3 sink: forward user metadata to the destination object filer.backup and filer.sync to an S3 sink copied the object body and tags but dropped the X-Amz-Meta- attributes the S3 gateway stores with each object, so the destination object came back without its user metadata. Send them as the upload's metadata, keyed by the lowercased suffix. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3 sink: send mtime as metadata without writing it into the entry The sink stored X-Amz-Meta-Mtime into the replicated entry's Extended map, modifying the caller's entry, and since tags were limited to X-Amz-Tagging- keys it was never sent anywhere. Add it to the upload metadata instead, keeping an mtime the source object already carries. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3 sink: RFC 2047 encode metadata values that are not printable ASCII The S3 gateway keeps user metadata values as raw bytes. Forwarded as is, a UTF-8 value fails SigV4 on AWS and a control character makes the HTTP client reject the request, and filer.backup retries such an object forever. Encode those values as RFC 2047 encoded words, which S3 decodes on write. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3 sink: upload without user metadata past the S3 2 KB limit The S3 gateway does not cap user metadata, but S3 rejects more than 2 KB of it, and filer.backup would retry that object forever. Upload such an object without its metadata and log a warning, as the sink did before metadata was forwarded. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3 sink: skip metadata keys that are not valid header names Filer gRPC clients can store any Extended key. An X-Amz-Meta- key with a space or control character makes the HTTP client reject the whole upload, which filer.backup retries forever. Leave such keys out with a warning. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3 sink: pick the same value for metadata keys that differ only in case An entry holding both X-Amz-Meta-Project and the legacy x-amz-meta-project maps both to project, and map order decided which value was uploaded. Keep the lowest key, so the canonical form wins over the legacy lowercase one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * s3 sink: use a test value codespell accepts Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> |
||
|
Chris Lu
|
1c84f98cdf |
master: deleting a read-only volume no longer drifts the active count (#12285)
* master: undo the stored record when a delete report arrives Deleting a volume decremented the active count from the delete report's ReadOnly flag, which the volume server never sets, so a volume the master had already stopped counting as writable was uncounted a second time. Every volume.move does this to its source, and the node, rack, DC and topology counts drifted negative; the full-list path already used the stored record. Undo exactly what was counted for the record held. * volume: say whether a deleted volume was read-only Explicit deletes, unmounts, collection deletes and the departures named in a delta heartbeat all left ReadOnly unset, so an older master undid the writable count for a volume it was no longer counting. Carry the flags, as the Rust server already does. |
||
|
Chris Lu
|
8d3cb2ed2d |
filer: resume a rejoining peer's meta subscription at its last position (#12279)
* filer: resume a rejoining peer's meta subscription at its last position OnPeerUpdate starts every peer add at the update's arrival time. For filers sharing one store there is no persisted offset to correct it, so a peer that drops out and rejoins (a graceful restart) resubscribes from "now" and the events it logged while its stream was down never reach the aggregated buffer - filer.backup and other meta subscribers silently skip them. Track each peer's delivered-through position (the same timestamp domain as the stream's events and idle-heartbeat claims) and rewind the start of a rejoining subscription to it. First-seen peers keep the given start, and non-shared peers still resume from their persisted offset. * filer: keep the re-add resume point behind a replay-floored event The session's replay floor already pins the persisted offset and the reconnect resume point so an event that outlived its retry budget is redelivered on resubscribe; cap peerResumeTsNs to the same position on session exit so the new re-add resume cannot skip past it either. * filer: stop test aggregators' log-buffer workers * filer: cover peer rejoin resubscribing at the resume position * filer: drop the unreachable replay-floor cap on the resume point Only a peer with its own store can floor an event, and that peer's subscribe start always comes from its persisted offset, never from the re-add resume point. The cap also ran in the old session's defer, after the re-add had already read the value. * filer: say why a re-add may resume from an idle heartbeat The heartbeat comments promised a reconnect never resumes from one, while the re-add resume point does. Spell out the difference and the fence that makes the local-stream claim safe to resume from. * filer: forget a removed peer's resume point after an hour The map held an entry for every address ever seen; with pod IPs that is one per restart. The removal mark now outlives the watermark grace and takes the resume point with it once the retention passes. * filer: advance the resume point only for tracked peers Same rule as the watermarks: a straggler from a dropped session must not recreate the entry. * filer: set the test aggregator's subscribe loop instead of a nil fallback |
||
|
Nicholas Oh
|
f5989a21fb | s3: honor assignment fsync in streaming copies (#12277) | ||
|
|
a94edebf00 |
fix(s3tables): keep the data under a deleted namespace (#12276)
* fix(s3tables): keep the data under a deleted namespace A rename leaves the source table directory holding data the renamed table still points at, and DeleteNamespace removed it recursively. The delete now keeps the directory, minus its catalog record, when files remain under it; CreateNamespace can take it back. * fix(s3tables): keep hidden-directory data, report name takeover as 409 - DeleteNamespace also looks for files under hidden directories, since a table location may sit under one. - CreateNamespace answers 409 when a file takes the name during the take-back of a kept directory. * s3tables: a vanished leftover holds nothing to keep A bare directory removed between the namespace listing and the walk made dirHoldsFiles answer its ListEntries NotFound, and the delete reported that miss as a 404 or 500 on a live namespace. Skip the gone directory instead: whatever it held is already gone, so it cannot keep the namespace. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3tables: a delete's gone entry is a conflict, not a success The filer answers a precondition delete on an entry that is already gone with FailedPrecondition carrying the not-found text, which DoRemoveRequest reads as a completed delete -- DeleteNamespace answered 200 for a delete that never ran. deleteDirectoryIfMatches now calls DeleteEntry directly, keeping the FailedPrecondition-to-conflict mapping and refusing a response-level error. keepAsLeftover likewise reports a directory removed since the scan as ErrConcurrentUpdate, so the handler answers 409 rather than 500. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Jay HemnaniandChris Lu
|
ecd716e0e2 |
s3: allow POST Object uploads when auth is off (#12273)
* s3: allow POST Object uploads when auth is off With no identities configured every other handler skips authentication, but PostPolicyBucketHandler still verified the form signature, so a browser form upload failed with InvalidAccessKeyId (or a malformed credential error without signature fields). Skip the signature check when auth is off; the policy conditions are still enforced. * s3: update AuthPostPolicy comment for the auth-off path --------- Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris LuandDevin
|
05584c541c |
s3: take list markers as raw keys under encoding-type=url (#12275)
* s3: take ListObjects marker/start-after as raw keys Under encoding-type=url AWS url-encodes the response fields but treats request markers as raw keys: SDKs decode Marker/NextMarker/StartAfter and send the decoded key back. Unescaping the marker server-side read a literal "%XX" key as the key it encodes — StartAfter=a%2Bb resumed after "a+b", skipping it, and a paginator whose marker decoded to an earlier key got the same NextMarker twice and failed. Take the request value as-is; only the server's own base64: continuation token still decodes. The marker-echo dedup now compares contents in the response wire encoding and NextMarker in its raw form, since a raw "%XX" marker equals the escaped emission of a different key. * s3: take ListMultipartUploads key-marker as a raw key Same raw-marker rule as ListObjects: under encoding-type=url the emitted key markers are url-escaped keys, and a client decodes them before sending the marker back, so the request value is already the raw key. Keeping the token decode only for the non-url path, where the response could not carry the raw key and falls back to the base64: token form. * s3: cover ListMultipartUploads url key-marker at the handler The selectMultipartUploads tests bypass the request marker decoding, so a regression that unescapes an encoding-type=url key-marker would pass them. Drive a listing through ListMultipartUploadsHandler with a literal "a%2Bb" marker and assert it resumes after that exact key. 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> |
||
|
Jay Hemnani
|
26783fbcb3 |
s3: keep an unset CORS MaxAgeSeconds unset (#12272)
A rule without MaxAgeSeconds was stored as 0, so GetBucketCors returned MaxAgeSeconds 0 and preflights sent Access-Control-Max-Age: 0. Store -1 for unset, which corsRuleFromProto already reads back as nil. |
||
|
hsdfat
|
9df2125bb0 |
fix(s3): reject non-integer max-parts, part-number-marker and max-uploads (#12270)
getObjectResources and getBucketMultipartResources dropped the strconv error, so a value that does not parse was read as 0. ListParts with max-parts=abc then reached the filer with limit 0, which the filer reads as its default listing limit, and returned every part with <MaxParts>0</MaxParts>; part-number-marker=abc restarted the listing from part 1; and ListMultipartUploads with max-uploads=abc answered an empty, untruncated list even with uploads in progress. Return the existing InvalidArgument errors (ErrInvalidMaxParts, ErrInvalidPartNumberMarker, ErrInvalidMaxUploads) instead, the way max-keys is validated. max-parts and max-uploads are parsed as 32-bit integers to match the documented 0..2147483647 range. |
||
|
|
6cf6ce81d7 |
fix(volume): reject a negative suffix length in Range headers (#12269)
* fix(volume): reject a negative suffix length in Range headers
parseRange accepted "bytes=--5" as a suffix range of -5 bytes and
returned {start: size+5, length: -5}. The volume server then answered
206 with an empty body and Content-Range "bytes 15-9/10", and the
filer's inline-content reader sliced entry.Content[15:10] and panicked.
Restore the check Go's net/http parseRange has: a suffix-length must be
a non-negative integer (RFC 7233 2.1), so a leading '-' or a negative
value is an invalid range. Apply the same check to the Rust volume
server's parse_range_header.
* test: negative-suffix manifest read is now rejected with 416
The parser change makes a negative suffix an invalid-range error, so
the duplicate-range case in this test can no longer reach the "empty
200" path. Assert the new rejection and that nothing is read.
* volume: treat a zero-length suffix range as unsatisfiable
"bytes=-0" resolved to {start: size, length: 0}, and the range handler
then answered 206 with "Content-Range: bytes 10-9/10", a value clients
must not recombine (and a first-byte-pos RFC 9110 cannot express).
Classify the zero-length suffix with the existing no-overlap case —
its resolved first-byte-pos sits at the content size — so the request
gets a 416 "bytes */size" unless another range in the set overlaps.
Applies to the Go and Rust volume servers alike.
---------
Co-authored-by: chrislusf <chrislusf@gmail.com>
Co-authored-by: user.email <chrislusf@users.noreply.github.com>
|
||
|
Chris LuandDevin
|
97e1758bcc |
s3: auto-create bucket on POST Object uploads to a missing bucket (#12266)
* s3: auto-create bucket on POST Object uploads to a missing bucket PutObject and CreateMultipartUpload lazily create the bucket, but a signed POST Object upload answered NoSuchBucket. Mirror the PUT path: when the versioning-state lookup reports the bucket missing, run the shared handleAutoCreateBucket gate (bucket-name check, AutoCreateBucket option, admin identity) and then re-read the versioning state. The POST handler already records the verified signer identity in the request context, so the admin check and bucket-owner attribution work unchanged. * test: assert the POST auto-create upload lands, not just non-404 The missing-bucket POST test passed whenever the handler created the bucket, even if the upload then failed. Drive the full path through the existing putToFiler fakes — assigned volume accepts the chunk, object entry and payload land — and assert the 201 success response. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: record verified post-policy signer's account id The POST policy path authenticates in the handler, after the internal account header was already scrubbed, so auto-created buckets and the uploaded objects stored no canonical owner. * test: assert POST auto-create records bucket owner, keep writes applied in denied case --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
b70c6dc6ed |
s3: let bucket-scoped Admin grants auto-create matching buckets (#12268)
* s3: let bucket-scoped Admin grants auto-create matching buckets autoCreateBucket required a global Admin identity, so an identity scoped to a prefix like "Admin:outline*" could explicitly CreateBucket but was denied when an upload auto-created the same bucket. Gate on CanDo(ACTION_ADMIN, bucket) — the same check explicit CreateBucket enforces — so a prefix-scoped key lazily creates only matching buckets. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3: run auto-create bucket admin through the policy-aware path A scoped Admin identity with an attached IAM policy that denied s3:CreateBucket could still auto-create a matching bucket, because userCanAdminBucket only consulted legacy Actions. Evaluate the same VerifyActionPermission explicit CreateBucket gets, over a synthesized PUT /bucket request so the attached-policy check resolves to s3:CreateBucket on the bucket ARN regardless of the upload's shape. * s3: record verified identity as owner of auto-created buckets An admin authenticated outside the middleware (IAM integration, post policy signature) left no identity in the request context, so an auto-created bucket stored no owner and never reached the owner index. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
|
bed0d7401d |
volume: after losing the leader, dial the leader the other masters name (#12259)
* volume: after losing the leader, dial the leader the other masters name When the heartbeat stream to the leader breaks and that master stays unreachable (partitioned or powered off, so dials hang rather than being refused), the volume server could take close to a minute to register with the new leader, and the new leader answered assigns with "no volume server capacity registered yet" in the meantime. Two things in the retry path added up: - refreshLeaderFromPeers asked the seed masters in order with up to 5s each, inside a budget of one pulse (5s by default). With the lost master listed first, the whole budget went to it and the live peers that already knew the new leader were never asked. - Even when the refresh found the new leader, the loop moved on to the next seed master. When the lost leader had been reached through a redirect, the next seed can be that same master, so the loop dialed it again and waited out the 20s gRPC connect timeout. Ask the lost master last, and when the refresh names a leader other than the lost master, dial it next. Assisted-by: Claude <noreply@anthropic.com> * rust volume: dial the leader the other masters name after losing one Mirror the Go heartbeat change: refresh_leader_from_peers asks the lost master last and returns the leader it found, and the loop dials that leader next instead of a seed that can be the unreachable master itself. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * rust volume: let a named leader consume the next seed slot Restarting the seed scan on every redirect let an earlier seed that keeps reporting the dead leader starve every seed after it: the loop bounced between the redirect target and the next seed forever. Go consumes the next range value for the redirect instead, so a pass always reaches every seed. Drop the breaks so new_leader overrides the next slot the same way. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume: try a named leader on top of the seed scan A reported leader used to replace the next seed slot: with seeds A,B and A persistently naming a dead leader X, X consumed B's slot on every pass so B was never dialed. The hint is now a one-shot attempt made before the seed it would have displaced. Its own result stays pending when a seed attempt names nothing, so a live follower's redirect chain is followed while every seed is still dialed on every pass. Mirrors between the Go and Rust volume servers. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume: follow a fresh leader hint before the pending seed A named leader whose stream breaks can leave the peer refresh naming its replacement; the loop then dialed the pending seed first and paid a dead master's connect timeout ahead of the fresh name. Fresh hints are now followed right away, bounded by the seed count and stopping once a name repeats, so a peer that keeps naming the same dead leader still cannot starve the seed scan. Go and Rust mirrored. * volume: dial each named leader once per pass (Go + Rust) A name that stays pending after the hint chain — the cap was hit, or the refresh named one already dialed — was retried before every later seed in the pass: each seed reset the hint budget, so a dead leader still cost a sleep and a connect timeout per seed. Track the names dialed in a pass, seeds included, and skip a hint that repeats one. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume: let a seed's named leader replace a pending hint (Go + Rust) The seed dial runs after the hint chain, so the leader it names is the freshest answer the pass has — but it was dropped whenever a stale hint was still pending, and the next seed then chased the older name first. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume: keep an untried leader hint over the seed's stale answer (Go + Rust) A seed that keeps naming a stale leader spends the per-pass hint budget on it; its own answer then overwrote the live name the hint chain had just reached, so that leader was never dialed. A pending hint now survives the seed dial until it has been tried. --------- Co-authored-by: user.email <chrislusf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
e98adfd3bb |
security: fail closed on gRPC TLS errors, verify dialed server identity (#12263)
* security: fail closed on misconfigured gRPC TLS LoadServerTLS/LoadClientTLS returned plaintext credentials on every error path once TLS was configured: a typo in tls.min_version or tls.cipher_suites, cert and key in different directories, or cert/key set without grpc.ca all left the component serving or dialing plaintext. A partially configured TLS setup now stops the process at startup instead; leaving cert/key unset still means no TLS, as before. The loaders also stat cert/key/ca files up front since the pemfile providers read lazily and would otherwise only fail at handshake time. Mirrored in the Rust volume server: partial grpc.volume cert/key, a missing grpc.ca, or a rustls build failure now abort startup rather than warn-and-continue on plaintext. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * security: optional server-identity allowlist for gRPC clients The mTLS client accepts any certificate the configured CA signs, so a per-workload CA lets one component stand in for any other: a filer identity can answer as the master. The server side already screens callers via allowed_commonNames and allowed_wildcard_domain; nothing screened the server. LoadClientTLS now wires AdditionalPeerVerification when the caller configures a server allowlist: grpc.<component>.allowed_server_commonNames (falling back to a shared grpc.allowed_server_commonNames) matched against the peer cert's subject CN, DNS SANs, or URI SANs such as a SPIFFE id, plus a grpc.allowed_server_wildcard_domain suffix match. Leaving them unset keeps the previous behavior. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * security: URI SANs are exact-match only in the server identity allowlist A URI identity whose path happens to end in the wildcard domain would otherwise pass the DNS suffix check. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * security: tighten gRPC TLS fail-closed and server-identity paths - URI SANs match the server-identity allowlist exactly; the wildcard domain suffix check applies only to CN and DNS SANs. - A partially set client_cert/client_key pair is an error when the cert/key fallback is empty, instead of dialing plaintext. - Cert/key/ca files are loaded and parsed at startup (LoadX509KeyPair and CA PEM validation) since pemfile providers read lazily. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * rust volume: error on a partial grpc client_cert/client_key pair Mirror the Go loadClientTLS path: when client_cert or client_key is set without its mate and the cert/key fallback is empty, fail instead of dialing plaintext. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * security: wildcard domain match requires a valid hostname A CA-signed identity like ".example.com" satisfied the bare suffix check for wildcard ".example.com" despite not being a hostname. Require a real subdomain: non-empty label before the suffix, a '.' boundary when the configured wildcard has no leading dot, and DNS-name syntax. A leading "*." label still matches so wildcard certs keep working. Applied to both the Go server/client authenticators and the Rust volume's client-cert verifier. * security: ignore a trailing root dot in wildcard domain matching * tls: reject a partial dedicated client cert/key pair instead of falling back When exactly one of component.client_cert/client_key is set, warn-and- fallback silently dialed with the serving identity. Fail closed like the other TLS misconfigurations in this change; mirrored in the Rust loader. Regression tests cover a partial pair with a valid fallback. * rust volume: harden partial client cert pair test with real pem files * security: verify dialed server identity per target kind on the filer The filer reused one grpc.filer-verifying dial option for master, volume, filer and s3 targets, so a broad allowlist accepted the wrong component at the wrong port and a narrow one rejected legitimate peers. Outbound calls now pick grpc.<target>.allowed_server_commonNames (falling back to the client component's and the global list) while still presenting the grpc.filer client identity. * security: scope per-target server allowlists under the client component The filer read grpc.volume.allowed_server_commonNames when dialing a volume, but that key already denotes the volume's own outbound allowlist (e.g. the master identity). A deployment restricting volume->master calls would make the filer reject real volume servers. Per-target policy now lives under the client component: grpc.filer.volume_* falls back to grpc.filer.* then grpc.*. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Nguyễn Lộc Phúcanduser.email
|
7d8b72c2b4 |
fix(s3tables): answer 404 for a table name that was renamed away (#12258)
* s3tables: answer 404 for a table name that was renamed away A rename leaves the source directory in place to hold the data under it, with its catalog attributes cleared. UpdateTable, DeleteTable, the table policy calls and the table maintenance calls read the metadata attribute of that directory and reported its absence as an internal error, so they answered 500, and GetTablePolicy answered NoSuchPolicy. An absent metadata attribute now reads as a missing entry. tableMetadata reports it as both filer_pb.ErrNotFound and the attribute error, so callers that test for either keep working. The policy and maintenance handlers read the attribute through the new readTableMetadataAttr, which reports filer_pb.ErrNotFound, so each call answers NoSuchTable. * s3tables: a view record without its metadata reads absent too * s3tables: refuse a catalog write that lands on a metadata-less record --------- Co-authored-by: user.email <chrislusf@users.noreply.github.com> |
||
|
Chris LuandDevin
|
816f9356b4 |
volume.copy: reject only on file-count drift, warn on deleted-count (#12253)
* volume.copy: reject only on file-count drift, warn on deleted-count Go warned and kept the copy on any count mismatch. The file count is exact on both sides of a stable source, so a mismatch means a genuinely bad copy and should reject; only the deleted count is a rebuild-time estimate that legitimately drifts. Match the Rust check_copy_counts, which warns solely on the counter that can drift. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume.copy: stage the copy and validate before dropping the old replica The destination replica was deleted before the copy even started, so a copy that failed its checks — now including the file-count validation — left the server with no replica at all. The copy now writes under a copying/ staging dir the volume loader never scans, the file-size and count checks run on the staged .idx (LoadCompactNeedleMap gives the same counters the mount would), and only then is the existing replica deleted and the staged files renamed into place. A rejected or failed copy leaves the previous replica untouched, and the .note marker is no longer needed since partial staged files can never be loaded. * volume.copy: remove only the files a failed copy moved into place * volume.copy: keep the .note in-progress marker through the renames A crash between the staged-file renames left a loadable volume without its .vif (writable where the source was read-only) or a partial set the load scan could not find. Write the .note before the renames and land the .vif first so any partial state is discovered and swept, then drop the note before the mount. * volume.copy: serialize copies of the same volume Two copies of the same volume write the same staged copying/ file names and can pass the existing-volume check before the first one mounts. Fail a concurrent second copy as retryable, matching the Rust volume server's volume_copies_in_flight. * volume.copy: restore the replaced replica when the swap fails Once the staged copy validates, the outgoing replica's files are hard-linked into copying/ before DeleteVolume unlinks them. If a later rename or the mount then fails, the preserved files are renamed back and remounted, so a failed swap no longer leaves the node with no replica. The links are removed on success. * volume.copy: clear stale needle-map files around the swap A leftover .ldb/.sdx/.rdb at the destination base is trusted over the fresh .idx the mount loads, and a mount that fails leaves the backends it built. Clear them before the staged files land and again when a failed copy cleans up, gated on no live replica owning them. Also keep <base>.vif on cleanup while an .ecx index shares the basename. * volume.copy: skip the swap when the caller has gone A caller that left during the copy cannot use the result; bail before the swap deletes the replica it would replace. * volume.copy: discard a mounted copy when the result send fails A mounted copy was already announced to the master, so a later failure deletes it through the store to withdraw the announcement before the files go away, rather than unlinking files out from under a registered replica. * volume.copy: keep a vif-less copy discoverable on a split index dir With -dir.idx the staged .idx lands outside the data dir, so a copy whose source has no .vif leaves nothing the volume scan can find and the mount fails after the replica it replaced was already deleted. An empty .vif marks the basename; the loader treats it as absent and writes a real one on mount. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume.copy: preserve the replaced replica on its own disk * volume.copy: restore the replaced replica's .vif past a kept incoming one * test: cover the rollback link/restore helpers * volume.copy: preserve the mounted replica's own files for swap rollback The swap backup linked files at the copy's target paths, which are wrong when the replica sits on another disk or keeps its .idx beside the .dat; a failed swap then restored nothing. Link the mounted volume's real paths and require every present file to link before the delete, since a partial backup cannot roll back. Re-check the store at swap time so a replica that mounted mid-copy is replaced rather than overwritten, and drop a .kept incoming .vif that occupies the path the replica's own .vif restores to. * volume.copy: clear a stale .idx a mount would prefer over the copy A load switches dirIdx to the data dir when an .idx sits beside the .dat, so a leftover <data>.idx shadows the fresh index the copy lands in the idx dir, and a surviving <idx>.idx fills in when the source had no index at all. Clearing only the derived backends still let a stale needle map reach the mount. * volume.copy: leave the shared copying/ dir for racing copies Copies of different volumes serialize per volume id but share the disk's copying/ dir; a finishing copy could rmdir it between another copy's MkdirAll and its first staged write, failing that copy with ENOENT. * volume.copy: fail replica preservation on stat errors other than not-found * volume.copy: refuse a mount while that volume is being copied A copy claims the volume id, then waits on the source with the replacement staged. A mount in that window loads the replica whose files the swap then clears; refuse it with Unavailable, retryable once the copy settles. Mirrors the same refusal on the Rust volume server. * volume.copy: refuse configure while that volume is being copied Configure unmounts and remounts; during a copy that already claimed the volume id the remount loads files the swap then unlinks. Refuse with Unavailable, retryable once the copy settles. Mirrors the Rust volume server's same refusal. * volume.copy: leave a still-standing replica's index on a rejected copy * volume.copy: preserve an unmounted replica's files for swap rollback * volume.copy: claim the in-flight id in mount and configure, not just check it * volume.copy: keep an EC-shared .vif when displacing an unmounted replica * volume.copy: restore replaced files over a surviving partial restoreReplacedVolumeFiles deleted a preserved link whenever the base name was still taken, on the assumption a landed file there belonged to the copy. When a moved-file removal fails, that left a surviving partial paired with the restored set's other files — mixed generations in one replica. A leftover at the basename now comes off first so the backup goes back, and the link is kept whenever it cannot go back. * volume.copy: link a repeated replaced path once so rollback cannot delete it * volume.copy: refuse allocation and staged adoption under the copy claim AllocateVolume and adoptStagedVolume still registered a volume while a copy of it held the destination: NewVolume loads the index before registration, and the copy then truncates or replaces files the live replica already opened, leaving it mounted on a stale index. Both paths now claim the volume id like VolumeMount and VolumeConfigure, so registration either wins the claim before the copy starts or sees it and refuses with Unavailable. * volume.copy: serialize reload loads with the copy claim A SIGHUP reload ran loadExistingVolume outside the copy's claim: its .note branch could unlink a copy's files mid-write, and its NewVolume+SetVolume could register a replica between the copy's recheck and its swap, leaving a live volume on a stale index. loadExistingVolume now consults a per-location hook the server installs: a vid whose claim is held is skipped for the next pass; otherwise the load takes the claim and its release drops it after registration, so whichever side claims the id first owns the files for the operation. * volume.copy: let a claim holder's own mount past the load guard VolumeMount, VolumeConfigure, VolumeCopy, and adoptStagedVolume all hold the per-volume claim while calling store.MountVolume, which reaches loadExistingVolume where the load guard saw the caller's own claim and skipped the disk: every mount failed with 'volume not found on disk'. Plumb claimHeld through MountVolume -> LoadVolume -> loadExistingVolume via the new MountVolumeClaimed entry point so an owner's own load proceeds while loads without the claim are still skipped. * volume.copy: keep the strict count check when only deletes drift Deletes append tombstone rows without moving the file count, so a copy whose source only deleted mid-flight still has an exactly comparable file count. Gate the strict check on file-count stability alone; the deleted-count drift is already a warning inside checkCopyCounts. * volume.copy: abort the swap when stale index files cannot be removed A surviving .ldb/.sdx/.idx is trusted over the copied .idx at mount, so a failed removal must stop the copy before the staged files land, not be swallowed. Mirrors the Rust side's remove_partial_files abort. * volume.copy: clear staged leftovers before starting a copy The staging names are fixed per volume, so a crashed copy can leave files under copying/; a retry whose new source lacks an optional file (.vif, or .dat on a remote volume) would rename the previous run's bytes into place. Clear them up front and refuse the copy when they cannot be removed. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
cc04c805a4 |
volume.copy: drop the needle-map index a failed copy leaves behind (#12252)
* volume.copy: drop the needle-map index a failed copy leaves behind A VolumeCopy that mounts and then fails validation is only unloaded, not deleted, so a leveldb (.ldb) or sorted-file (.sdx) index built by the mount outlives the copy. The next copy of the same volume to this server then trusts the stale index (generateLevelDbFile replays only past its watermark), which is wrong once the source has compacted in between. Mirrors the Rust server's remove_partial_files. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume.copy: clear a stale index before a copy writes A crashed copy, or one whose cleanup failed, leaves a stale .ldb/.sdx/.rdb next to the .idx that this copy's mount would trust (generateLevelDbFile replays only past the stored watermark, and a same-size .idx keeps every un-replayed offset). Abort on a removal error rather than risk a mount reusing a poisoned index. Mirrors the Rust server's pre-write remove_partial_files call. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume.copy: keep a mounted replica when the final response send fails Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume.copy: discard a mounted replica the caller abandoned A send failure after MountVolume means the caller is gone, but the replica was already announced to the master. Deleting its files under the live mount breaks the loaded index; leaving it mounted keeps an orphan the master routes to. Mirror the Rust copy cleanup: track the mount, and on error delete through the store (unmount + withdraw the announcement + remove files, keeping remote data), and check the stream context before mounting. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume.copy: serialize copies of a volume on one server A second VolumeCopy of the same volume could pass the existing-volume check before the first copy mounts, then clear or overwrite the files the first copy still owns. Mirror ecDecodesInFlight: the claim covers the whole copy, and a duplicate fails fast so the caller can retry. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume.copy: remove only files the copy owns, and every stale index Three cleanup gaps around which files a copy may remove: - The failure cleanup ran on any error, so an abort before the first write (a failed stale-index clear or a failed .note write) could delete a previous replica's files the copy had not touched. File removal is now gated on the copy having started writing. - A mount that finds an .idx beside the .dat builds the needle-map index under the data base, so leftovers there poison the next copy the same way the idx-dir ones do. Clear and clean up .idx/.ldb/ .sdx/.rdb under both bases when the dirs differ. - A regular volume and an EC volume for the same id share <base>.vif. Keep it while the EC index is on the disk, as the destroy path does. - When the discard of a mounted copy is refused (e.g. it is compacting) the volume stays registered; unlinking its files would strand a live replica. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume.copy: do not unlink a previous replica on an early abort An abort before the first write now leaves the destination files alone, but three removal paths still leaked past that gate: a partially-written .note, and the post-copy defer's .idx/.vif at the data base. Remove the .note unconditionally (it only ever marks an in-flight copy) and gate the data-file removals in the outer defer on copyOwnsFiles too. * volume.copy: refuse a mount or configure while a copy is in flight A copy claims the volume id, then waits on the source before it clears or overwrites the destination files. A mount or configure landing in that window loads the replica whose index files the copy is about to remove. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume.copy: abort when a replica registers during the source RPCs The existing-volume snapshot runs before the copy calls the source, so a mount that lands in between escapes it. Recheck registration just before the destructive clear, which can share the index directory with the mounted replica's bases. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume.copy: keep files whenever a replica of the volume stays registered The cleanup already kept files after a refused delete; a replica a racing mount registered during the copy needs the same protection, since its index directory can coincide with the bases being cleaned. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume.copy: skip the index cleanup when the copy never picked bases An early failure leaves the base names empty, so the removals would otherwise run against ".ldb"/".sdx"/".rdb" in the process directory. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume.copy: serialize mount and configure registration under a per-volume guard The in-flight flag check in VolumeMount and VolumeConfigure was not atomic with volume registration, and the copy's registration recheck was not atomic with its stale-index clearing. A mount that passed the check just before the copy claimed the volume could register after the recheck and then lose its index files to the clearing. Replace the bare flag with a per-volume guard: the copy claims it for the copy's life, mounts and configures hold it across check plus registration, and the copy holds it across the recheck plus clearing. Whichever side takes the guard first wins, so a mount either registers before the recheck (and aborts the copy) or sees the claim and refuses. Guards persist in the map so a mutex handed to a waiter is never replaced mid-wait. * volume.copy: refuse allocation and staged adoption under the copy guard AllocateVolume and adoptStagedVolume still registered a volume while a copy of it held the destination: NewVolume loads the index before registration, and the copy then truncates or replaces files the live replica already opened, leaving it mounted on a stale index. Both paths now take the per-volume guard through their registration, so they either register before the copy's recheck (and make it abort) or see the claim and refuse with Unavailable. * volume.copy: serialize reload loads with the per-volume guard A SIGHUP reload ran loadExistingVolume outside the copy's guard: its .note branch could unlink a copy's files mid-write, and its NewVolume+SetVolume could register a replica between the copy's recheck and its file clearing, leaving a live volume on a stale index. loadExistingVolume now consults a per-location hook the server installs: a vid with a copy in flight (or whose guard the copy currently holds) is skipped for the next pass; otherwise the returned release keeps the guard held through registration, so the copy's recheck either sees the volume and aborts or clears first. * volume.copy: skip compaction-state reconcile while a copy owns the volume * volume.copy: let a guarded operation's own mount past the load guard VolumeMount, VolumeConfigure, adoptStagedVolume hold the volume guard locked, and VolumeCopy holds copying=true, while calling store.MountVolume. The mount reaches loadExistingVolume where the new load guard saw the caller's own hold (TryLock fails, or copying set) and skipped the disk: every guarded mount failed 'volume not found on disk' — a completed copy deleted its copied files, configure left the volume unmounted, staged adoption failed after renaming. Plumb ownLoad through MountVolume -> LoadVolume -> loadExistingVolume via the new MountVolumeClaimed entry point so the owner's own load proceeds while unrelated loads are still skipped. * volume.copy: fix own-load test call, assert unguarded mounts still skip Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * volume.copy: refuse unmount and delete while a copy owns the volume --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
ad6257ebbc |
filer.backup: add -s3.flattenVersions to mirror the current S3 view of versioned buckets (#12255)
* filer.backup: add -s3FlattenVersions to mirror the current S3 view Versioned buckets are stored as <key>.versions/v_<versionId> files plus a latest-version pointer on the .versions directory, so a backup mirror held internal version files and could never be restored through the S3 API. With -s3FlattenVersions the backup maps events on .versions directories onto the flattened object key: a new latest version uploads to <key>, a latest delete marker or a cleared pointer deletes <key> when -doDeleteFiles is set, and the suspended-versioning null object resolves to the regular path. Version files themselves are never mirrored. The initial snapshot seeds only the version the pointer names. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer.backup: serialize flattened writes per object key Same-path non-barrier dir updates run concurrently in the metadata processor, so two pointer updates for one object -- or a pointer update and a regular-path write to it -- could overlap and let the older upload finish last. Lock on the flattened key: versions events lock the object key, every other event its own path, via a striped mutex. * filer.backup: apply exclusions to the flattened object key File-name and path excludes ran against the .versions entry, so an excluded object was still copied flat and its deletion still removed the destination copy. Match the rules against the flattened key on both the apply and the versions-dir-removal paths. * filer.backup: re-resolve the pointer on version-file events A tag or attribute update on the current version touches the v_* file, not the .versions pointer, so dropping every version-file event left the mirror with stale tags until the next pointer flip. Re-resolve the pointer on each version-file event; noncurrent versions still stay out. * filer.backup: tolerate vanished versions dirs in flattened snapshot The flattened seed path returned source errors before the existing missing-source policy ran, so one version deleted mid-walk could force a full re-walk. Apply the same 404/superseded skip as the generic seed branch, and cover the review fixes with tests. * filer.backup: lock both flattened keys an event touches A rename or delete can retire one flattened key while writing another, so serialize on the old- and new-side keys together. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer.backup: refresh the flattened key only for the still-current child A v_* event on a non-pointed version cannot move the flattened key, so skip the re-upload; tag and attribute updates on the current version still flow through. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * replication: let resolved-latest writes bypass the source staleness check A filer sink skips a CreateEntry whose source path no longer holds the entry, but a flattened write lands on a key whose source path is not the one it was read from, so the check dropped every flattened upload. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer.backup: reclaim destination chunks on flattened deletes Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer.backup: route regular-path events for versioned keys through the pointer A null-object write or delete at the regular path can coexist with a .versions pointer that names a live version; the pointer decides the flattened key, so deleting the null object must not remove it. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer.backup: make the checkpoint identity cover the flattening mode Resuming a checkpoint taken in the other mode would skip the flattened re-seed and leave mirrored version files behind; a mode change now starts a fresh snapshot. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test: cover flattened regular-path routing and per-mode checkpoints Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * filer.backup: resolve both sides of a flattened rename * test: cover flattened renames with one versioned side * replication: let resolved-latest writes overwrite a newer destination The stale-destination skip trusted a newer destination mtime, but a resolved-latest write can legitimately install an older version over the copy of a since-deleted latest. Skip only on provably identical content (matching chunk provenance, or equal ETag with full byte coverage). * filer.backup: rescan .versions when the latest null object is gone Deleting ?versionId=null removes the regular-path object without touching the null-is-latest pointer, so the flattened key kept its stale copy. Fall back to the newest remaining version (or a flat delete), the same rescan the S3 read path applies. * replication: compare inline and remote content in the destination-holds checks A chunk-less entry keeps its bytes in Content and a remote-only entry in the object RemoteEntry points at, but the identical-content shortcuts only compared chunk lists and chunk-derived ETags. Two different inline files looked identical, so a flattened re-copy or a same-mtime rewrite kept the previous version's bytes. Compare Content bytes and remote object identity wherever the checks claim the destination already holds the reference's content. * filer.backup: paginate the null-latest .versions rescan A limit=0 listing is capped at DirListingLimit server-side, and old-format version names sort oldest-first, so a history longer than one page left the flattened copy on an older version after the null object was removed. Page through the directory like the S3 read path does. * replication: require matching writable metadata for resolved-latest skips A skip keeps the destination's stored metadata, so identical bytes were not enough: stale attributes or extended entries (e.g. a superseded version id) survived under equal content. Compare the fields a rewrite controls — attributes minus filer-assigned inode/crtime, with the store's atime/ctime zero-defaults applied — plus the extended map, in both the provenance and ETag fast paths. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * replication: require the extended key to exist when comparing metadata * replication: ignore a stored rule TTL when the resolved write carries none * filer.backup: rename -s3FlattenVersions to -s3.flattenVersions Match the -s3.* flag naming convention used by the other s3 options. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
1a62b1d09f |
filer: buffer recently fetched chunks so parallel readers do not refetch (#12254)
* s3sink: upload through a concurrent-safe ReaderAt so parts do not thrash
s3manager splits a seekable body into parallel section readers, and every
part's ReadAt landed on the single chunk buffer in ChunkStreamReader: parts
at different offsets kept evicting each other's chunk and refetching it
whole, which produced massive read amplification against the volume
servers and collapsed throughput.
Build the upload body from filer.ChunkReadAt instead, the same
concurrent-safe reader the mount and query paths use: each part's range is
served from a shared reader cache with parallel chunk fetches and
range-reads for clipped views, so every byte is fetched once per pass.
Generated with [Devin](https://devin.ai)
Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
* filer: buffer recently fetched chunks for concurrent readers
* Revert "s3sink: upload through a concurrent-safe ReaderAt so parts do not thrash"
This reverts commit
|
||
|
Nguyễn Lộc Phúcandchrislusf
|
eba2cb926d |
fix(s3tables): refuse a create that a namespace delete can lose (#12239)
* s3tables: refuse a create that a namespace delete can lose DeleteNamespace checks the namespace for children and then deletes its directory recursively in a second filer call. The filer locks each path on its own, so a table created under the namespace between the two is not seen by the check and is removed by the delete, while both requests answer 200. The mirror case loses the table too: a create that passed its namespace check before the delete finished re-created the namespace directory through the filer's parent creation and recorded a table in a namespace that no longer exists. Each side now writes before it reads. DeleteNamespace claims the namespace record with a fresh token before it scans, deletes only while the record still holds that claim, and lifts its own claim when it stops. CreateTable, RegisterTable and CreateView re-read the namespace after writing their record and back out when it is claimed or gone: they remove what they wrote and answer 409 or 404 instead of reporting an entry that does not survive, or 500 when the record could not be removed and may remain. An entry the scan missed was written after the claim, so its create sees the claim; a create and a delete can both fail, but no longer both succeed. A create is also refused up front while a claim is held, but only after its authorization check, so a caller that may not create there does not learn of the delete. A create writes its record marked "create" and lifts the mark once the namespace is confirmed open. Until then GetTable, GetView, UpdateTable and a second create treat the record as absent, so no request is told about a table or view that the first create may still back out. * s3tables: keep a create's unfinished record out of reads and listings * s3tables: refuse catalog writes on a record an operation claimed * s3tables: answer 404 when a create's write path loses the namespace * s3tables: confirm the rename destination's namespace like a create * s3tables: cover unfinished-record reads, claimed-record writes, rename claim * s3tables: lift a leftover claim when a no-token delete clears the record * s3tables: answer 409 when a claimed record turns away a write UpdateView, TagResource, and UntagResource surfaced ErrConcurrentUpdate as a 500, so an expected conflict looked like a server fault to the caller. Map it to Conflict the way UpdateTable and the policy handlers already do. * s3tables: name the lost source when a rename finds it deleted * s3tables: report an entry deleted before its update as not found The filer answers an UpdateEntry on an entry removed since the lookup with codes.NotFound. mutateEntryExtended returned that error unchanged instead of filer_pb.ErrNotFound, so a rename whose source was deleted mid-claim answered 500 instead of NoSuchTable or NoSuchView. MemFiler now answers a missing entry the same way, so the rename tests for tables and views take the real filer's path. * s3tables: unpublish the destination when a rename's source clear fails * s3tables: refuse to delete a record another operation claimed * s3tables: release catalog claims on an uncanceled cleanup context * s3tables: answer 404 when a racing delete already removed the table * s3tables: keep the rename destination marked until the source clears * s3tables: guard the rename rollback against a recreated source and an unfinished destination * s3tables: drop the unfinished destination only once the source is restored * s3tables: re-resolve the purge target on the record a delete claims * s3tables: cover a delete that meets a moved metadata location * s3tables: answer 409 when a create's record changed under it * s3tables: adopt a record still marked by its unfinished create * s3tables: skip unfinished creates in the namespace empty check * s3tables: delete only the record the caller was authorized on DeleteView and DeleteTable check permissions on the record they read first, then read the name again to claim it. A record that took the name in between, such as a table or another owner's view created after a concurrent delete, was claimed and removed on the first record's authorization, and DeleteView never compared the caller's version token with it. The second read now repeats the first one's checks: a record of the other kind or an unfinished create answers 404, and a token that no longer matches answers 409. A delete without a token also compares the record's version token with the authorized one and answers 409 when they differ. * s3tables: re-check the parent after a create lifts its mark A create that passed checkParentOpen and paused before lifting its mark could still confirm while a namespace delete held the claim: the delete scans the still-marked record, skips it, and removes it under a create that already answered success. confirmParent now re-reads the same parent record after the mark clears and backs the confirmed record out when the namespace moved on. * s3tables: mark the rename destination with a claim of its own claimCatalogEntry adopted every create-marked record, including a rename's destination: a create landing after the source cleared could replace the destination before the rename published it, and with the source name reused the restore could leave the table on neither name. The destination now marks with the rename's own claim, which readers still treat as unpublished but a create must refuse, and the commit re-checks the destination namespace after the record goes live. The source keeps its rename claim until the commit lands so a namespace delete scanning the cleared record still refuses. * s3tables: compare the parent's createdAt, not its editable metadata A namespace update rewrites ExtendedKeyMetadata in place, so a create or rename whose second parent read landed across an UpdateNamespace saw a changed blob and reported the still-live namespace as gone: creates backed their record out and answered 404, renames undid their destination. createdAt survives the edit but changes on a delete+recreate, so it still identifies the record. * s3tables: retry lifting the rename's claim off the cleared source The release's only failure mode is a transient filer error — nothing else can write the cleared record — yet a single loss left the claim behind: the old name stayed refused to creates and the namespace counted the record as a child. Retry the release before warning. * s3tables: page the namespace scan before deleting it A page of skippable entries — bare directories or unfinished creates — ended the scan with hasChildren false, so a table on a later page was removed with the namespace. Keep listing with StartFromFileName until a child shows or the listing ends. --------- Co-authored-by: chrislusf <chrislusf@users.noreply.github.com> |
||
|
Chris Lu
|
ee7e4a64d2 |
s3vectors: record request metrics on the dispatch path (#12241)
* s3vectors: record request metrics on the dispatch path The S3 API tracks every operation through S3RequestHistogram, S3RequestCounter, and S3InFlightRequestsGauge; the vectors dispatch rode the same listener with no instrumentation. The op name labels each series under s3vectors.<Operation>. * s3vectors: count auth rejects and keep bucketless metrics alive |
||
|
Chris LuandDevin
|
72ad975803 |
s3vectors: bound GetVectors and DeleteVectors key lists (#12238)
* s3vectors: bound GetVectors and DeleteVectors key lists Both forwarded req.Keys to the engine unbounded; AWS caps these at 500, the same window a PutVectors batch shares. * s3vectors: cap GetVectors at the AWS 100-key limit GetVectors allows at most 100 keys per request while DeleteVectors allows 500, so give vectorKeysBounds the per-call maximum instead of sharing the put-batch window. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3vectors: assert out-of-window key lists never reach the engine Record get/delete calls on the fake engine and cover both handlers rejecting empty and oversized lists with 400 ValidationException. 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
|
94e9b8a49a |
fix(volume): return conflict and explicit proof for read-only upload refusals (#12236)
* fix(volume): report read-only HTTP refusals in Rust * fix(volume): mirror HTTP write refusal proof in Go * test(volume): cover HTTP read-only refusal parity * test(volume): cover local refusal after remote append * fix(volume): honor decoded replica controls before retry proof * fix(operation): reassign proven HTTP conflict refusals * fix(operation): stop same-target retries on HTTP conflict * fix(volume): replace replica marker when forwarding requests * fix(java): gate conflict reassignment on refusal proof * fix(volume): preserve authentication query encoding in replication * fix(operation): stop unproven conflicts in forever retries |
||
|
|
9ee8604ff3 |
webdav: answer 204 to a PUT that replaces a file (#12233)
* webdav: answer 204 to a PUT that replaces a file * webdav: decide the 204 from what OpenFile actually replaced A Stat before the write left a window where a delete or create in between made the status wrong, and it cost a second filer lookup. The OpenFile create path already knows whether it removed an existing entry, so it now marks the request context and the writer maps 201 to 204 only on a real replace. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * test/webdav: cover OpenFile's own put-outcome flag for both create paths Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * webdav: take the put outcome from the create's OldEntry A stat-then-create still races: a second writer landing between the check and the write makes the reported status describe a state the write never saw. The filer decides create-vs-replace under its per-path lock and reports the previous entry in the response's metadata event, so flag the PUT replace from OldEntry instead of a pre-write stat. The in-place update keeps the replaced entry's crtime and inode and reuses the filer's own chunk cleanup, so the separate removeAll goes away, and an exclusive create now defers to the filer's atomic O_EXCL instead of a racy local check. * webdav: send no body with the 204 a replacing PUT answers --------- Co-authored-by: user.email <chrislusf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
cba409cae7 |
backup: carry index-only tombstones through incremental copies (#12231)
* volume server: add idx_content to VolumeIncrementalCopyResponse A remote-tier delete is an index-only tombstone row with no .dat record, so it cannot reach an incremental copy through the .dat stream. Carrying the .idx tail verbatim lets the backup reproduce those rows at their exact positions. * backup: carry index-only tombstones through incremental copies A remote-tier delete leaves an offset-0 tombstone in .idx with no .dat record. The copy only streamed .dat bytes and regenerated .idx from them, so the tombstone never reached the backup: the deleted needle stayed live and every later incremental diverged and rebuilt again. backupCursor and ResumeByLastEntry now accept an index-only tombstone as a boundary, matched by key and size since there is no record to read a cookie from. VolumeIncrementalCopy appends the .idx tail after the .dat tail, and IncrementalBackup writes it at the index boundary, truncates, and loads the new rows — so tombstones land at the exact positions the replica has them. * seaweed-volume: mirror index-only tombstone handling in incremental copy resume_by_last_entry accepts an offset-0 tombstone boundary matched on key and size, skips its missing cookie check, and resumes .dat streaming at the next entry that has a record — index-only rows ride the new idx_content stream instead of reporting divergence. The server captures the .idx bound before the .dat one (a row is published only after its bytes) and streams the tail after the data. * backup: keep incremental copy bounds and streams consistent Review fixes: * Bound capture moved after the resume reads and reads .dat/.idx sizes together (Go FileStat, Rust file_stat under one lock), so every row resume saw is inside the index bound and each row's record is always inside the data bound. * A compaction commit changes CompactionRevision: the server now rejects the copy when the revision moved between resume, file opens, or the end of the stream, so a backup never mixes revisions. The .idx is opened up front to pin the inode. * An index tail that ends before the captured bound is an error, not a silent success. * IncrementalBackup rolls the .idx tail and .dat bytes back to the pre-copy boundary when the stream fails, so a partial row cannot wedge backupCursor and unapplied rows cannot pass for current ones. * The index tail is replayed through a small indexTailReceiver interface, covering the leveldb needle maps too, not just the in-memory map. * backup: bound the resume scan to the captured index size The resume and the stream bounds are now captured in one order: revision, then the .dat/.idx pair, then the scan. ResumeByLastEntry and resume_by_last_entry take the index bound and scan no further, so a row appended while the copy is being set up can never enter the index stream without its .dat bytes. The index-tail replay stays on the in-memory map only: the leveldb map's DoOffsetLoading writes to the .cpldb compaction database, not the live one, so it cannot serve a live tail replay; other maps keep regenerating the index from .dat as before. * seaweed-volume: guard incremental copy total against a located offset past .dat A corrupt index can place the resume offset beyond the captured .dat size; has_data already refuses the reader, but total still evaluates dat_size - s and underflows. * backup: bound the incremental .dat tail by index-covered records A record staged but not yet indexed can sit inside the captured .dat bound while its row stays outside the index bound — the fsync gap between staging and publishing is not covered by one stat. Streaming those bytes leaves them unindexed on the backup, and the next run appends the record again. Bound the .dat tail by the last record the streamed rows cover on both producers, and cover the bound walk with tests. * backup: cap the .dat stream at the captured bound and roll back replay failures sendFileContent could read past stopOffset on its last chunk, so a needle appended after FileStat arrived without its index row and the next copy appended at the wrong position. Each read is now limited to the remaining captured bytes. A failed truncateIndex or DoOffsetLoading after a successful stream left the index tail on disk unapplied, which a later backupCursor could mistake for applied rows; both now roll the tail back to the pre-copy boundary like the stream-error path. * backup: bound the incremental .dat tail by the maximum covered offset A key-sorted .idx ends on the highest-key row, which need not be the record physically last in .dat, so the last-row bound could cut off live records. Take the end of the highest-offset row inside the bound instead, and cover the case with a sorted-idx test in Go and Rust. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * backup: restore .dat and the needle map when index replay fails A failed DoOffsetLoading left the received .dat bytes in place and could leave partial mappings for rows the index rollback just removed, so the next copy appended the same records again or read stale entries. Truncate .dat back to the copy boundary and restore the touched mappings and metrics from a pre-replay snapshot. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * backup: cover no index rows with the superblock bound When no row inside the index bound names a record — an empty index or a bound of index-only tombstones — the covered end is the superblock, not the captured .dat size, so staged bytes are never streamed without a covering row. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * seaweed-volume: apply the covered bound even when the index streams no rows The bound was gated on idx_end > idx_start, so an empty captured index skipped it entirely and a staged .dat tail still shipped. Gate on the stream-index predicate instead, matching Go's streamIndex check. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * backup: propagate index coverage read errors * backup: rebuild the needle map from the truncated index on replay failure * seaweed-volume: propagate index coverage read errors --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
|
0a4668fcfa |
webdav: overwrite on MOVE when the Overwrite header is missing (#12232)
* webdav: overwrite on MOVE when the Overwrite header is missing * webdav: defer a MOVE destination's delete into the rename x/net removes an existing destination before Rename, so a refused or failed move — a CanRename rejection, a vanished parent, a transient filer error — loses the target entirely. For an overwriting MOVE onto an existing destination, run the request against a filesystem that skips the destination RemoveAll and lets the filer's rename replace the target inside one transaction instead; a file vs. collection clash still takes the delete + rename path. Rename itself now tolerates an existing target, which the atomic filer rename already handles. * webdav: only default Overwrite when the header has no values An Overwrite header sent with an empty first value makes Header.Get return "", which the defaulting treated as absent and rewrote to T — overriding a client's explicit F in a later value. Check the value list instead so a present-but-empty header is left alone. * webdav: move a collection onto a non-empty one via delete + rename deferredDeleteFS skips the destination RemoveAll so the filer rename can replace the target in one transaction, but the filer only replaces an empty collection: moveSelfEntry deletes the target non-recursively and refuses ErrNonEmptyFolder. Detect a non-empty collection destination and take the depth-infinity delete + rename path moveFiles would have run. * webdav: park a clashing MOVE target aside until the rename lands A refused rename (cross-collection move, bucket target) after deleting the destination lost its contents; the target now renames to a hidden sibling first and is restored when the winning rename fails. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * webdav: restore a parked MOVE target under a detached context A client disconnect cancels the request context mid-move, which also canceled the restore and stranded the destination under its hidden sibling name; the restore and parked-entry removal now run under context.WithoutCancel with a bounded deadline. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * webdav: bound the parked MOVE target's restore after the move The detached restore deadline started before the source rename, so a long move could exhaust it; each detached context now starts where its work begins. A failed parked-entry removal retries briefly and its warning names the parked sibling for recovery. Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * webdav: restore a parked MOVE target without replacing a new claimant A client can create the destination name while the old target is parked, so the restore rename must fail instead of replacing that new entry. * webdav: propagate RemoveAll's context to the filer delete The parked-target cleanup runs under a bounded detached context, but the delete RPC discarded it; passing ctx through makes the deadline effective. --------- Co-authored-by: user.email <chrislusf@users.noreply.github.com> Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Ping QiuandClaude Opus 5.5
|
2a1afadcd0 |
s3api: return NoSuchUpload when aborting an upload that was already completed (#12227)
s3: AbortMultipartUpload answers NoSuchUpload for an upload that no longer exists
Complete and abort both remove .uploads/<id>. abortMultipartUpload answered
204 whenever that record was missing, so aborting an upload that had already
completed reported success while the object stayed in place, and a client
could read it as the object being discarded. AWS returns 404 NoSuchUpload
("The specified multipart upload does not exist") for that case, and
ListParts here already does.
Answer ErrNoSuchUpload when the record is not found. checkUploadStillOpen
reuses abort for cleanup after the upload is gone; NoSuchUpload is not a
failed cleanup there.
The leftover-directory path from #11385 is unchanged: a .uploads/<id> that
outlived its completed object is still removed metadata-only and answered.
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
||
|
Denis Kieftanduser.email
|
a2f595b207 |
backup: don't Close a volume after Destroy (#12225)
Co-authored-by: user.email <chrislusf@users.noreply.github.com> |
||
|
Chris Lu
|
349576b63e |
backup: resume incremental copies by index position instead of appendAtNs (#12228)
* volume server: add last-entry cursor fields to VolumeIncrementalCopy * backup: resume incremental copies by index position instead of appendAtNs appendAtNs is stamped by whichever replica received the write, so the same needle carries a different timestamp on every replica. A backup that resumes from a different replica than last time can append the last needle again or skip needles written inside the clock gap. Send the local copy's index entry count and last entry identity (key, size, cookie) with VolumeIncrementalCopy. The replica verifies the same record sits at that position and streams from the next index entry, so the resume no longer depends on any replica's clock. When the boundary record does not match, or the local copy is ahead, the replica returns FailedPrecondition and the backup rebuilds the local copy from scratch. Old servers ignore the new fields and keep the timestamp behavior. * rust volume server: mirror positional resume in VolumeIncrementalCopy * volume server: add entries_hash prefix checksum to VolumeIncrementalCopy * backup: verify the whole index prefix, not just the boundary entry * rust volume server: verify the index prefix hash in resume_by_last_entry * backup: keep the source's compaction revision on rebuilt copies * backup: diverge on a zero-offset boundary entry A remote-tier tombstone lives in the .idx with no .dat record, so readNeedleAt(0) parses the superblock and its error bypassed the ErrIncrementalCopyDiverged mapping; the backup got a generic failure instead of FailedPrecondition and never rebuilt. * rust volume server: mirror zero-offset boundary divergence * rust volume server: gate the empty-dat early return on entry_count With a positional resume the Go handler still reaches ResumeByLastEntry, so an empty replica reports divergence; the Rust early return answered a clean empty stream and weed backup kept a stale copy. * backup: hash the index entry's size field, not trailing offset bytes In a 5BytesOffset build the offset occupies bytes 8-12 and the size starts at byte 13, so the hash folded in one byte of the offset. Matching records at different offsets then hashed differently and a valid backup was reported diverged and rebuilt. * backup: read the index in whole-entry chunks for the prefix hash When entries are 17 bytes a 1 MiB read buffer does not end on an entry boundary: the partial tail entry was skipped by the loop but still counted as consumed, so every later chunk was hashed at a misaligned position. * rust volume server: hash the index entry's size field after the offset Mirrors the Go hashIndexEntries change: with the 5bytes feature the offset occupies bytes 8-12, so starting the size slice at byte 12 folded one offset byte into the hash and matching records at different offsets compared diverged. * rust volume server: read the index in whole-entry chunks for the prefix hash Mirrors the Go change. With 17-byte entries a 1 MiB buffer does not end on an entry boundary, so slicing the partial tail entry panicked once a backup held 61681 entries. * rust volume server: default the new incremental copy request fields in tests |
||
|
Chris LuandDevin
|
d0441bbdbb |
s3vectors: validate index ARNs and expand live integration coverage (#12222)
* test: align vector bucket policy coverage with vector actions * s3vectors: validate ARN service and resource shape before resolving indexes * test: exercise signed vector bucket policies and catalog lifecycle * ci: require the expanded vector integration matrix and all gateway tests * test: reject index ARNs in bucket-level API requests * s3vectors: scope ARN resolution to the gateway namespace * s3vectors: reject name fields that conflict with resolved ARNs bucketARNs and resolveIndexARNs already reject an indexArn that names a different bucket than vectorBucketArn, but a vectorBucketName or indexName disagreeing with the ARNs was silently dropped. Validate the plain names against the resolved ARN values the same way. * test: cover conflicting vector names and ARNs in live requests --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris Lu
|
cc5912b8b8 |
s3vectors: e2e conformance for vector bucket policy evaluation (#12221)
* s3vectors: reject empty boolean-operator lists and sub-objects in filters An empty $and/$or list or a nested empty object rendered '()'' — invalid SQL surfaced as an engine error instead of a clean ValidationException. * s3vectors: e2e conformance for vector bucket policy evaluation The handler-level test covers statements; this runs the whole stack a client sees — HTTP dispatch, authentication, policy evaluation, catalog, engine — and walks the full lifecycle: a non-owner is denied without a grant, an Allow on the bucket ARN grants every index under it, a Deny vetoes an Allow on the same action, and deleting the policy restores owner-only access. * s3vectors: prove deny precedence and grant removal in policy e2e * s3vectors: reject field operator objects with no operators * test: use an s3vectors action in the vector policy round-trip |
||
|
Chris Lu
|
9d5929a73f |
s3vectors: reject empty boolean-operator lists and sub-objects in filters (#12220)
* s3vectors: reject empty boolean-operator lists and sub-objects in filters An empty $and/$or list or a nested empty object rendered '()'' — invalid SQL surfaced as an engine error instead of a clean ValidationException. * s3vectors: reject field operator objects with no operators |
||
|
Ping QiuandClaude Opus 5.5
|
3f954e34dc |
s3api: label bucket-level requests correctly in the access log (#12224)
* s3api: label bucket-level requests correctly in the access log getResourceType treated a request as bucket-level only when the object key was "/". NormalizeObjectKey strips the leading slash, so bucket routes arrive with an empty key and every bucket operation was logged as an object operation: ListObjectsV2 as REST.GET.OBJECT, DeleteObjects as REST.POST.OBJECT, and the lifecycle, ACL and policy calls likewise. Treat an empty object key as bucket-level too. The new test covers five bucket-level and two object-level request shapes; it fails without the fix. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * s3api: label only POST ?delete as a batch delete in the access log With bucket-level requests now reaching the bucket branch, any method on /bucket?delete was labelled BATCH.DELETE.OBJECT. Only the POST route is DeleteObjects; label other methods as the plain bucket operation. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com> |
||
|
Chris Lu
|
fd493f2547 |
mount: refuse to attach opens to a draining file handle (#12219)
* mount: refuse to attach opens to a draining file handle With the writeback cache, releasing the last open of a file leaves the shared FileHandle in the map at counter zero while an async flush drains it; the worker removes the map entries once the flush commits. AcquireHandle waits for a pending flush and then attaches, but the two steps are not atomic: a release can slip in between, drop the counter to zero, and queue the drain while the opener goes on to mint a new file-handle id on the draining handle. When the drain completes, RemoveFileHandle deletes every fh id for the inode -- including the one just minted. The kernel still holds that id, so its next FUSE_READ gets ENOENT from GetHandle; the failed read leaves the folio not uptodate and the kernel surfaces EIO to userspace. This was the flaky TestWritebackCacheConcurrentMixedOps failure. AcquireFileHandle now refuses to attach when the shared handle has reached a zero counter and reports it as draining. AcquireHandle and the Create path retry the whole wait-resolve-attach sequence; the next pass blocks on the pending flush (flag registration and the counter drop share pendingAsyncFlushMu) and builds a fresh handle from the post-flush entry. The DLM-failure release in AcquireHandle also goes through ReleaseHandle so it cannot strand a counter-zero handle outside the drain bookkeeping. Removes the refreshEntryForRead retry from #12206, which worked around the symptom by refetching the entry after the fh id was already gone. Test: TestAcquireFileHandleRefusesDrainingHandle covers the refusal and fresh-handle path; TestWritebackCacheConcurrentMixedOps and an amplified concurrent-overwrite repro run clean. * mount: keep refresh-on-error retry for stale-chunk reads |
||
|
Chris Lu
|
ac7b4f4cf6 |
s3vectors: bound ListVectorBuckets and ListIndexes maxResults (#12218)
* s3vectors: bound ListVectorBuckets and ListIndexes maxResults Both pass maxResults to the catalog unbounded; AWS caps these list pages at 100, the same way ListVectors is bounded and topK is bounded. * s3vectors: raise ListVectorBuckets/ListIndexes maxResults cap to 500 AWS documents a valid range of 1-500 for both list APIs; the shared limit of 100 rejected legitimate page sizes. * s3vectors: reject explicit maxResults of zero on list calls AWS requires maxResults >= 1, but a plain int field cannot tell an omitted field from an explicit zero. Making MaxResults a pointer keeps the catalog default for omitted fields while a supplied zero fails validation. * s3vectors: test max ListIndexes page against an existing bucket |
||
|
Chris LuandDevin
|
f122f313e1 |
s3tables: keep the admin bypass off an identified caller's empty action list (#12217)
Generated with [Devin](https://devin.ai) Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris Lu
|
229ad84e2d |
s3tables: reject s3vectors resources in every ARN partition (#12215)
* s3tables: reject s3vectors resources in bucket policies An arn:aws:s3vectors: Resource is as dead as an s3vectors action: the evaluator compares against the generated s3tables ARN, so the rule the writer believes enforced never matches. * s3tables: reject s3vectors resources in every ARN partition * s3tables: test wildcard-partition and mixed-list s3vectors resources |
||
|
Chris LuandDevin
|
8a2fec8610 |
filer.sync: skip superseded replays before fetching any chunks (#12212)
* filer.sync: skip superseded replays before fetching any chunks On a directory under heavy overwrite, most entries the sync replays are already superseded on the source. The sink only learned that after chunk reads failed — or worse, after fetching and uploading bytes a later event replaces — so the backlog drained slowly and the log filled with 404s. CreateEntry and UpdateEntry now check sourceMovedOn before touching a chunk: one entry lookup skips a stale replay entirely. A mid-replication supersession is still caught by the existing retry gates. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * replication: bound the source liveness probes with a timeout The supersession checks added a source entry lookup to the replication path, and every lookup ran on context.Background() with no deadline. A source that accepts the call but whose metadata store stalls would hold the sync forever — before a single chunk fetch, even when every chunk is still readable. Bound the probe at 30s; an unanswered lookup proves nothing, so replication proceeds and the existing retry gates decide. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * replication: sync the live version a superseded replay points at Skipping a stale replay assumed a later event always re-carries the current content. It does not when the superseding write was itself replicated in from another cluster: those events are dropped before they reach a filer sink, so the destination could be left missing the file. When the source reports a live entry newer than the replay, commit that version through the normal path instead — equivalent to applying the later event, with no dependence on its delivery. A gone source entry still skips, since the destination already matches the source. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * replication: reconcile moved entry content after a native rename AtomicRenameEntry relocates whatever version the destination held. If the preceding update replay was skipped because the source had already moved to the new name, the moved entry keeps the old bytes and — with no further writes — stays stale forever. MoveEntry now runs the same reconciliation as an in-place update against the rename's snapshot, so the moved entry picks up pending content changes. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * replication: repair from the live snapshot a superseded replay resolves to An incremental diff against the replayed oldEntry can resolve manifests the overwrite already vacuumed, which fails every retry of the event. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris LuandDevin
|
98fe70a247 |
s3vectors: keep bucket policies in vector form and evaluate them in the gateway (#12214)
* s3vectors: translate vector bucket policies to the catalog form The document is stored verbatim on the shared table bucket and evaluated against s3tables actions and ARNs, so a policy written for the vector API was a dead rule. Actions now map to the catalog operation the request authorizes against, and an arn:aws:s3vectors: resource becomes the bucket/.../table/vector/... ARN the index's catalog entry answers to. * s3tables: expose PrincipalFor and PolicyContext for façade evaluation A façade authorizing against its own action namespace needs the same identity resolution and evaluation context the catalog uses internally. * s3vectors: keep bucket policies in vector form and evaluate them in the gateway The earlier approach translated the document to catalog actions and ARNs, which lost the caller's original policy for GetVectorBucketPolicy and could not cover data-plane calls that never touch the catalog. The document now stays in s3vectors form under a dedicated bucket attribute and every handler evaluates it through authorizeVector against the action and ARN the request names; a dedicated key also keeps a vector grant from ever authorizing a catalog call. * s3vectors: close policy-mutation races and keep vector grants visible - Authorize Put/DeleteVectorBucketPolicy inside the policy slot's compare-and-swap, so a grant revoked between reads cannot authorize the pending write. - Assert the policy attributes stay absent in the update precondition, so a concurrent policy or tag write trips the retry instead of being silently reverted. - Migrate a raw vector document out of the shared policy slot on replace or delete, keeping its catalog rules. - Pass the unresolved identity to PolicyContext so open-by-default deployments still work. - Filter ListVectorBuckets on the vector policy itself, so buckets a caller was granted under s3vectors actions stay listed. * s3vectors: reuse SnapshotExtended for the vector-policy CAS precondition * s3vectors: skip buckets that vanish mid-list instead of failing the page * s3vectors: reject non-vector bucket formats inside authorizeVector * s3vectors: check bucket format after authorization in authorizeVector An unauthorized caller probing an ICEBERG bucket got NoSuchBucket,\nleaking that the bucket exists. The format check now runs after the\npermission check so denied callers always see AccessDenied. * s3vectors: cover legacy shared-slot policy removal on delete * s3tables: resolve the forwarded identity name like getAccountID in PrincipalFor A synthesized manager request resolves its caller through the identity- name branch of getAccountID, which normalizes the name (ARNs collapse to their trailing segment, whitespace trims) before falling back to the handler's account ID. PrincipalFor returned the raw name, so ARN-shaped identities never matched their own grants. * s3tables: assert the vector policy key in update preconditions s3vectors-policy is stored on table bucket entries but was missing from s3tablesExtendedKeys, so a catalog update that read the entry before the first vector policy was saved could overwrite it without tripping the compare-and-swap. Asserting the key makes such a race fail instead of silently erasing the policy. * s3vectors: reject rules the evaluator cannot honor in vector bucket policies The shared Statement shape has no negated fields, the vector evaluation supplies no condition keys, and a document that fails the typed decode is discarded wholesale at evaluation — including denies the writer believed active. Reject NotPrincipal, Condition, foreign-service actions and resource ARNs, and require the typed PolicyDocument decode. Also cover that an admin-named identity still reaches its own buckets. * s3vectors: read vector policy fields the way the evaluator decodes them Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> * s3tables: match ARN policy principals through caller normalization Caller principals are normalized ids, so a statement naming a full IAM ARN could never match: an ARN-scoped deny was silently dead and an ARN-scoped allow unreachable. Compare arn: specs against the caller under the same normalizePrincipalID the ownership path uses, while an ARN caller keeps verbatim wildcard matching. * s3tables: grant the admin bypass on the caller's actions, not the name A principal that only normalizes to the admin id skipped every policy deny. When the request carries an identity, the bypass now requires the identity's own admin action — the same gate getAccountID applies — while name-only transports with no action list keep it. PolicyContext takes the request so it can carry IdentityActions, and the vector context helpers route through the shared identity keys so the resolved name and actions agree. * s3tables: match ARN policy principals only for deny statements Normalization drops the ARN's account and path, so an allow matched on the trailing name could grant a same-named caller the statement never verified. Denies keep the normalized match so the rule cannot be ducked through the same collapse. * s3tables: scope service wildcards to their own service s3tables:* counted as blanket admin inside the shared permission check, so an identity holding only catalog grants administered s3vectors operations too. hasAdminActionFor now requires the wildcard to name the operation's service; the global admin markers and s3:* stay service-agnostic. --------- Co-authored-by: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> |
||
|
Chris Lu
|
4ef123d764 |
s3vectors: fix shared API behavior and test Go/Rust volume parity (#12216)
* s3vectors: reuse shared authenticated identity context * s3vectors: encode empty results as arrays * test: cover S3 Vectors with Go and Rust volume servers * ci: require both volume implementations in vector integration tests * s3vectors: keep identityOf as a shim over the shared identity context Concurrent branches add identityOf call sites; deleting the helper would break the build when they merge in either order. Route it through GetIdentityNameFromContext so every caller resolves the same identity. * test: wait for the vector engine to register before running The gateway resolves weed-vector lazily through the master cluster list, so a listening port does not mean the engine is reachable yet; the first vector write or query could race KeepConnected and fail. Restore the cluster.ps registration wait the previous harness had, and ignore the pytest cache directory the suite creates. * test: wait for master registration across OSS and enterprise Use the exact engine registration event after AddClusterNode completes. The current enterprise cluster.ps does not list vector nodes, so waiting on its output prevents the shared fixture from starting there. * s3vectors: keep contextWithIdentity as a shim alongside identityOf In-flight callers still write the caller name through it; route the write through SetIdentityNameInContext so both helpers agree on the shared key whichever lands first. |