mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-17 04:36:50 +00:00
dd73fee077e97b4cf9f19dceffafe26a1a24df8e
9576
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Chris LuandGitHub
|
dd73fee077 |
mount: read oversized directories through instead of caching them (#10631)
* mount: read oversized directories through instead of caching them Visiting a directory pulls every child from the filer into the local LevelDB before the first listing returns. For a directory of a few million entries that is minutes of streaming, gigabytes of local store, and gigabytes of decoded entries in flight -- paid by a mount that may only walk the directory once. A build that crosses -cacheDirMaxEntries (default ten thousand) now stops, cleans up, and marks the directory read-through: listings stream from the filer with pagination, the way update-hot directories already do, and lookups in it consult the filer per entry as any uncached directory does. The refusal is remembered, so the next visit fails fast instead of streaming to the limit again, and an oversized ancestor is stepped over when caching its subdirectories rather than wedging every listing beneath it. The direct path keeps the same pagination state on the handle, so a walk that crosses the limit mid-flight carries on from where the cached walk reached. * mount: an ancestor found oversized must not fail its descendants Visiting a directory builds its whole uncached ancestor chain in one group, so the first discovery that an ancestor is oversized cancelled the group and surfaced as the listed directory's own refusal: the descendant build was aborted and the caller marked the descendant read-through, leaving a perfectly cacheable directory streaming from the filer until its inode was forgotten. The earlier test missed this by pre-marking the ancestor, which exercises only the fast path. The refusal of any directory other than the one being listed is now kept out of the group's result; it is already remembered for the next visit. |
||
|
Chris LuandGitHub
|
506ce0850b |
telemetry: count erasure-coded volumes in the reported totals (#10632)
collectVolumeStats walked only DataNode.GetVolumes(), which returns the regular volumes on each disk. An encoded volume leaves that set and is reported through GetEcShards instead, so total_disk_bytes and total_volume_count silently excluded every erasure-coded volume: a cluster that encoded everything reported zero bytes and zero volumes while still counting as a volume server. Sum each holder's shard sizes into the byte total, parity and extra copies included, matching how a replicated volume's used size counts every replica and how CollectionEcVolumeStats already reports EC footprint. Count volume ids rather than shard entries, since one volume's shards are spread over many nodes and would otherwise multiply the volume count by the number of holders. |
||
|
Chris LuandGitHub
|
6d08b08f37 |
heartbeat: carry a volume digest and verify it (#10627)
* pb: carry a volume digest on the heartbeat The full volume list is the only way a master notices a volume that vanished without a delta, so it cannot simply be dropped. A digest gives the same guarantee without the list, and a way back to the list when they disagree. The digest has explicit presence: a server holding no volumes reports 0, which has to stay distinguishable from a server that does not compute one at all. * volume: report a digest of the volumes each heartbeat carries Digests exactly what goes on the wire: volumes skipped as quarantined, phantom or expired are absent from both the list and the digest, so the master compares against the same set the server meant to report. Runs the master's own hash over the master's own conversion of the message, so the two ends cannot drift into disagreeing about a field. * master: check the reported volume digest and ask for the list on a mismatch Compared after everything the heartbeat carried has been applied, so agreement means the master is current rather than that nothing changed. Servers reporting no digest are untouched, and a mismatch on a heartbeat that already carried the full list is reported rather than answered: there is nothing further to ask for, so asking again would loop. Nodes reporting one volume id twice are skipped for the same reason. * rust: report the heartbeat volume digest Mirrors the Go volume server. The master compares this against a digest it computes itself, so the hash has to agree byte for byte across the two implementations, not merely be a hash of the same fields: report_hash_vectors pins it against values generated by the Go side, and the ttl and replica placement narrowing the master applies when it decodes a message is applied here too rather than assumed away. A drift there would not corrupt anything, but every volume server on this implementation would report a digest the master can never match and fall back to sending its whole volume list forever, which is the cost the digest exists to avoid. * master: pin what the digest check does to each kind of report The upgrade story rests on these: a server that reports no digest is never asked for anything, so the two sides can be upgraded in either order, and a disagreement that resending cannot fix is reported rather than re-asked, so it cannot loop. * topology: enumerate the digest coverage test from the message The list of fields was written out by hand, so a field added to VolumeInformationMessage later would fall outside the digest while the test went on passing, and a change to it would never reach the master. Walk the message descriptor instead. Some fields are narrowed or normalised on the way into VolumeInfo, so the smallest change to the wire value can land back on the stored one; the test offers several values per field and asks only that some change is visible. |
||
|
Chris LuandGitHub
|
553bc5ab90 |
topology: digest the volumes a master believes each node holds (#10619)
* topology: digest the volumes a master believes each node holds A volume server resends its whole volume list every heartbeat because that list is the only way the master can notice a volume that vanished without a delta. A digest gives the master the same guarantee without the list: the two ends agree iff the master's copy is current. VolumeInfo.ReportHash covers every field of VolumeInformationMessage, so a change the hash misses is a change the master would never hear about. Both ends run it over the same converted VolumeInfo, so they cannot drift apart. Disk keeps the xor of its volumes' hashes, which is order-independent and its own inverse, so add, update and remove each stay O(1) and the running value needs no per-volume storage. Nothing reads the digest yet; the heartbeat protocol change comes next. * topology: test that a changed-volumes-only heartbeat reconciles The digest is not a change detector -- in a live cluster some volumes always have changed. It answers whether the master holds what the volume server holds once the heartbeat's own changes are applied, so reporting three volumes out of fifty has to reconcile while a volume lost without a delta must not. * topology: digest the lookup index too, not just the disk maps The reported digest answers whether the master holds what the volume server holds. It cannot answer whether the master can serve those volumes: the disk map and the lookup index are maintained separately, and a disconnect racing a reconnect drops a volume from the index while leaving it on the node. The server's report is identical either way, so a digest built from the disk maps alone matches while the volume answers 'volume id not found'. Track a second digest over volume ids on both sides of that split, so the master can see its own indexes disagree without the volume server's help, and without the O(volumes) scan the full heartbeat currently relies on. * topology: exclude nodes reporting a duplicate volume id from the digest A volume id can end up mounted on two disks of one server -- a stale twin re-attached after a disk repair, which the store handles rather than rejects. The server reports both copies with different disk ids, but the master keys volumes by id alone within a disk type and keeps only the last one. Its digest can then never equal the server's, and no amount of resending the full list would fix it. Detect it from the report itself, where deduplicating the ids already tells us the count, and mark the node. A marked node has to keep sending full lists; representing both copies is a separate question, and nesting the volume map by disk id would cost more memory than the digest saves. * topology: move the lookup digest with the entry, not the node passed in Two volume servers can hold one address: GetOrCreateDataNode keys on the id a server reports and refuses to merge a new id onto an address an older node still claims, while the lookup list keys on address alone. Registering the second server therefore displaces the first from the entry, and unregistering through either removes whichever node the entry named. Crediting the node handed to Set and Remove instead of the one actually displaced or removed left the digest on the wrong node. A displaced node went on reporting a consistent index while it could no longer serve the volume, which is exactly the silent unavailability the digest exists to catch. Set and Remove now return the node they displaced and removed, so ownership can be transferred rather than assumed. |
||
|
Chris LuandGitHub
|
12627d376d |
mount: fix four readdir pagination bugs (#10624)
* mount: size the direct listing slice from the batch, not the offset The limit passed here is skipCount+batchSize when a client resumes a fresh handle partway through a directory, so preallocating for it turns the client's cookie into an allocation: a readdir at offset 3,000,000 reserves 24MB before the first entry arrives, and an offset near the uint32 ceiling asks makeslice for ~4.29e9 elements. * mount: stop replaying a directory that shrank past the resume offset A client that opens a fresh handle and resumes at a cookie from an earlier, larger listing gets a preload that cannot reach the entry before that offset. The resume name was then left empty and the follow-up batch listed from the directory's first child again, so the client was handed every name a second time. The stream always runs from the first child, so failing to reach that entry means the directory is simply shorter than the offset. That is the end of it. * mount: page a directory from where the store reached The batch loader treated a short batch as the end of the directory, but the meta cache drops an expired child after the store has already spent it against the limit, so a batch that filled up could still deliver fewer entries than asked for. A directory with a handful of expired children would stop listing early and hide every child behind them; a whole batch of expired ones truncated the listing to nothing. ListDirectoryEntries now reports the name the store itself reached. That is both the sound end-of-directory signal -- the store returning nothing -- and the right cursor, since resuming from the last visible name would re-read the dropped children every round and never get past a batch that was entirely expired. * mount: drop the entries a directory walk has already passed entryStreamOffset was only ever written by reset, so the stream a handle holds grew for the life of the walk and a directory was retained whole even though nothing could read the entries behind the client's position again. A 10M-entry walk parked millions of entries per handle, and NFS-Ganesha opens several on the same directory. Offsets index into the stream from entryStreamOffset, so advancing the two together keeps them lined up. One entry is held back because the next batch resumes from the name immediately before the offset. Seeking back behind what is still held now restarts the directory, which is what the offset scheme can honestly support -- it previously returned nothing. |
||
|
Chris LuandGitHub
|
b46946ece5 |
filer: list directories without decoding chunk lists (#10616)
* filer: decode a listed entry without building its chunk list
A readdir reads attributes and never looks at chunks, but decoding an
entry builds the whole chunk list first: four allocations per chunk, all
of it thrown away. On a directory of ordinary 4MB-chunked files that is
most of what listing costs.
DecodeAttributesOnly walks the wire format and hands everything except
the chunks to the generated unmarshaller, so new fields in filer.proto
need no attention here. The chunks are still measured, because the S3
copy and multipart paths deliberately store a zero FileSize and let the
chunk extents define the size, but nothing is allocated to do it.
The blob is only re-encoded once a chunk is actually seen, so an entry
without any -- every directory, for one -- is unmarshalled where it lies
and pays nothing for the walk.
Listings opt in through the context, the way the lazy remote paths
already do; a store that ignores it stays correct.
chunks full attrs-only allocs
0 312.8n 310.1n ~ 1 -> 1
1 686.1n 411.1n -40.07% 7 -> 1
4 1.742u 667.4n -61.69% 24 -> 1
16 5.770u 1.544u -73.25% 86 -> 1
64 25.23u 6.004u -76.20% 328 -> 1
* mount: list directories with chunk lists omitted
The two meta cache listings behind a readdir are the only callers, and
neither reads a chunk. On 200k single-chunk files one enumeration goes
from 364ms to 277ms and drops a million allocations.
The read-through listing still fetches whole entries from the filer,
which would need the request to say it wants attributes only.
* mount: give the readdir benchmark's entries a chunk
Chunkless entries made the decode look far cheaper than it is, which is
the part of a listing worth measuring.
* filer: let a listing ask for entries without their chunk lists
The read-through readdir fetches whole entries over gRPC, and for a wide
directory the chunk lists are most of what crosses the wire and most of
what the client then unmarshals. A 4MB-chunked file is 113 bytes of
entry against 46 without its chunk.
ListEntriesRequest gains omit_chunks. The size a client needs is already
in the attributes, where the store decode folded the chunk extents in,
so dropping the list costs the client nothing.
The filer still reads the entries whole. A listing is where a TTL-expired
entry gets collected and deleted, and deleting one needs its chunks to
find the data, so omitting them there would leak. Only the response is
trimmed.
The hint moves to filer_pb so one context flag serves both transports:
the gRPC request sets omit_chunks, and a listing served from the local
store skips building the chunks. Cache population is unaffected either
way, since EnsureVisited starts from its own context.
* filer: reject a chunk the full decoder would reject
The walk skipped a chunk's bytes without looking inside them, so a
FileChunk carrying a corrupt nested fid, or a string that is not valid
UTF-8, sailed past the listing decoder while every other read of the same
entry still failed. The file listed with a plausible size and then gave
EIO on open, and corruption that used to fail the listing loudly was
hidden instead.
The chunk bytes are the one part of the blob the generated unmarshaller
never sees, so the two checks it would have made are made here: a
submessage has to parse, and a proto3 string has to be valid UTF-8.
FileChunk's only submessages are FileIds of scalars, so walking them is a
complete check. A descriptor-driven test fails if FileChunk ever gains a
field of either kind that the walk does not know to check, which is the
part that keeps this honest as filer.proto grows.
Taking the scratch buffer lazily, only once a chunk is actually dropped,
also takes the pool out of the path for entries that have none. Those
were measurably slower than the full decoder before; they are now level
with it. Each chunk's length prefix is parsed once rather than twice.
chunks full attrs-only vs base
0 171.4n 176.9n ~ (p=0.670)
1 366.6n 259.4n -29.24%
4 1.034u 500.2n -51.60%
16 3.905u 1.464u -62.52%
64 13.48u 5.195u -61.46%
* filer: carry the size before dropping chunks over the wire
Dropping the chunk list assumed every store folds the chunk extents into
FileSize when it decodes. A store that keeps entries as JSON rather than
as an encoded Entry never re-derives it, so an object written with a zero
FileSize kept its real size only in the chunks, and stripping them left
the client reading the file as empty. Stamp the size into the attributes
first, which costs nothing and does not depend on how the store loaded
the entry.
* mount: test that the readdir context reaches the store decode
Everything else exercises the decoder directly, so a refactor that
stopped threading the context would have reverted the whole thing with
every test still passing.
The benchmark's chunks also carried a constant legacy FileId, which
BeforeEntrySerialization reparses over Fid on the way in, so all 200k
entries stored one byte-identical chunk rather than the varying fixture
it looked like.
|
||
|
Chris LuandGitHub
|
af7cf6ab8a |
chore(weed/topology): drop the unused DataNode volume id listing (#10618)
GetVolumeIds ranged over a slice and collected the loop indices, so it reported 0-99 rather than the node's volume ids. Nothing calls it: the disk-level GetVolumeIds, which ranges over a map and is correct, is what ToDiskInfo and ToMap use. Its private getVolumes helper went with it, having no other caller. |
||
|
Chris LuandGitHub
|
cce3bab0e2 |
perf(weed/topology): gather a node's volumes into one slice (#10617)
* perf(weed/topology): preallocate the node's volume concatenation A node's volumes are gathered per disk and concatenated into a slice grown from nil, so a server with several disks reallocates and copies its way up. The writable-volume refresh loop does this for every node every few seconds. BenchmarkDataNodeGetVolumes/8Disks 322551844 B/op -> 121602326 B/op * perf(weed/topology): fill one slice across a node's disks Each disk built its own right-sized copy of its volumes, and the node then copied all of them again into the combined slice. Appending into the caller's slice makes it one allocation whatever the disk count, which halves even the single-disk case. BenchmarkDataNodeGetVolumes 1Disks 121602326 B/op 2 allocs/op -> 60801314 B/op 1 allocs/op 8Disks 121602326 B/op 9 allocs/op -> 60801024 B/op 1 allocs/op |
||
|
Chris LuandGitHub
|
228e850da1 |
perf(weed/topology): preallocate the client-facing topology snapshots (#10615)
* perf(weed/topology): preallocate the /dir/status volume list
ToVolumeMap boxes every volume on a node into an []interface{} grown from nil,
so the slice reallocates its way up alongside the boxing. The count is known.
* perf(weed/topology): preallocate the volume id list sent to clients
Every filer, s3 gateway, and mount that connects to the master gets one
VolumeLocation per data node carrying that node's whole volume id list, grown
from nil. The count is known.
The ec ids are left alone: shards of one volume can span disks, so the shard
count is an upper bound on the deduped vid count, not the count itself.
|
||
|
Chris LuandGitHub
|
9f1e21e73f |
perf(weed/topology): preallocate the per-disk VolumeList payload (#10614)
ToDiskInfo builds a protobuf message per volume and per ec shard on the disk, growing both lists from nil. Every VolumeList call runs it for every disk in the cluster, and the admin dashboard, the plugin worker, several shell commands and the s3 gateway's per-minute bucket metrics all call VolumeList. Both counts are already in hand. ToTopologyInfo over 550k volumes 202.2 MB -> 184.6 MB |
||
|
Chris LuandGitHub
|
0cfca436f1 |
perf(weed/topology): size the new-volume list from the actual delta (#10613)
A reconnecting volume server reports every volume it has as new, so newVolumes grew from nil to one entry per volume, reallocating and copying its way there. Sizing it to len(actualVolumes) instead would allocate the whole list on every steady-state heartbeat, where nothing is new. After the deletion pass everything left on the node is also in this heartbeat, so the difference is exactly what the node is about to gain: all of them on a reconnect, none in steady state. First registration of 550k volumes 1041.7 MB -> 667.4 MB |
||
|
Chris LuandGitHub
|
8aa57bef78 |
mount: stop churning the inode table on every readdir (#10606)
* mount: readdir enters a child in the inode table only when it takes a reference Only readdirplus into the kernel takes a reference on the children it reports, and only that reference brings a FORGET later to take the entry back out. Every other listing was inserting all its children anyway. On WinFsp that meant a listing looked each child up, took a reference, and immediately gave it back, so a walk of a wide directory paid three write-lock acquisitions per entry to leave the table exactly as it found it. On a plain kernel readdir nothing gives the entry back at all, so listing a directory of 200k files grew both maps by 200k entries that were never reclaimed. A dirent's inode number is informational either way: the kernel must LOOKUP before it can use a nodeid, and the WinFsp adapter re-resolves every operation by path. So report the number and let the mapping be built when something actually looks the entry up. * mount: take the readdirplus reference without a second full lookup The entry has just been resolved a few lines above, so redoing the whole lookup only rebuilds the child path and walks both maps again to reach a counter. Bump it directly, falling back to the full lookup if a Forget removed the entry in between. * mount: benchmark a readdir over a 200k directory Drives doReadDirectory against a meta cache holding 200k entries, one round of 4096 at a time, for the three front ends that behave differently: a plain kernel readdir, kernel readdirplus, and a WinFsp listing that gets attributes but never returns a reference. Reports what each leaves behind in the inode table alongside the usual metrics. The sink declares TakesLookupRef as an ordinary method rather than through the interface, so the same file runs unchanged against an older tree for comparison. * mount: stamp an inode on the benchmark's entries The filer stores one on every entry it writes, so a real listing arrives with an inode and never derives its own. Leaving it zero made every child in the benchmark fall through to the MD5 in AsInode, work no filer-backed mount does, and charged it to both sides of the comparison. |
||
|
Chris LuandGitHub
|
ee54fd6c08 |
perf(weed/storage/super_block): intern the byte-encoded replica placements (#10610)
NewReplicaPlacementFromByte formatted the byte with fmt.Sprintf and parsed the result back, allocating a string and a ReplicaPlacement every call. The master calls it once per volume in every heartbeat, and keeps the pointer for the lifetime of the volume, so a cluster with 1.6M volume replicas carries 1.6M of these where a handful of distinct values exist. The table is a flat pointer-free array, so it costs 6KB of static data and no heap objects however few placements a cluster actually uses. A byte only ever decodes to a valid placement, so the table is complete and the error return stays nil. BenchmarkSyncDataNodeRegistration/100000Volumes 500601 allocs/op -> 300589 allocs/op |
||
|
Chris LuandGitHub
|
33c36fc7a3 |
perf(weed/storage/needle): intern the stored ttl values (#10611)
The master decodes a TTL per volume in every heartbeat and keeps it for the volume's lifetime, so a cluster using TTLs carries one two-byte object per volume replica where at most 256 counts times 7 units exist. Share them, and decode the uint32 form directly instead of staging it through a byte slice. Clusters that set no TTL are unaffected; that path already returned the shared EMPTY_TTL. BenchmarkSyncDataNodeRegistration/100000Volumes, volumes carrying a ttl 600600 allocs/op -> 500597 allocs/op |
||
|
4f0322af86 |
perf(weed/topology): log writable-state changes, not every check (#10612)
* perf(weed/topology): log writable-state changes, not every check ensureCorrectWritables ran its three diagnostics whenever it was asked, so a volume that had always been read-only re-announced that on every registration. A volume server reconnecting with 550k read-only volumes made the master format over a million log lines before it could serve anything, which is exactly when it is already at its memory peak rebuilding the topology. removeFromWritable already reports the transition, and only when there is one. Explain it only then. Dropped the separate 'remove from writable' line, which said nothing that 'becomes unwritable' does not. BenchmarkRegisterReadOnlyVolumes, 100k volumes 285791480 B/op 1902069 allocs/op -> 234592088 B/op 1302572 allocs/op * Update weed/topology/volume_layout.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> |
||
|
Chris LuandGitHub
|
1d8d9570eb |
perf(weed/topology): preallocate the disk volume snapshot (#10609)
Disk.GetVolumes copies the disk's whole volume map into a fresh slice, and every caller that walks a node's volumes goes through it: the writable-volume refresh loop every few seconds, ToTopologyInfo on each VolumeList, telemetry, and node unregistration. Growing from nil reallocates and copies about twice the final size each time, which at 100k volumes per disk is 60MB of garbage per call. BenchmarkSyncDataNodeRegistration/100000Volumes 199670102 B/op -> 137728051 B/op |
||
|
Chris LuandGitHub
|
2ec899bdee |
perf(weed/topology): diff a heartbeat without copying the volume map (#10608)
* perf(weed/topology): keep only volume ids in the heartbeat membership set The map is used solely to test whether a known volume is still present, but it copied the whole 152-byte VolumeInfo for every volume in the heartbeat. Presize it too, since the count is known. BenchmarkSyncDataNodeRegistration/100000Volumes 199670102 B/op -> 180157310 B/op * perf(weed/topology): diff a heartbeat without copying the volume map To find volumes the data node no longer reports, UpdateVolumes copied every VolumeInfo on the node into a fresh slice, then deleted the missing ones one at a time. At 100k volumes that is a 15MB copy per heartbeat to usually find nothing. Scan the disk maps in place instead and return only what was removed. BenchmarkSyncDataNodeRegistration/100000Volumes 180157310 B/op -> 87806436 B/op |
||
|
Chris LuandGitHub
|
3fce1a938d |
perf(weed/topology): preallocate the heartbeat volume conversion slice (#10607)
* test(weed/topology): benchmark the per-heartbeat volume sync A volume server re-sends its entire volume list every VolumePulsePeriod, so SyncDataNodeRegistration is the master's steady-state per-server cost. Give it a benchmark so allocation regressions show up. * perf(weed/topology): preallocate the heartbeat volume conversion slice The slice grows to one entry per volume on the data node, so at 100k volumes the doubling copies allocate 60MB of garbage per heartbeat. The final length is known up front. BenchmarkSyncDataNodeRegistration/100000Volumes 199670102 B/op -> 137725872 B/op |
||
 Lars LehtonenandGitHub
|
2ff3dda7cd | chore(weed/s3api/policy_engine): prune dead code (#10599) | ||
|
Chris LuandGitHub
|
0cf62a921a |
admin: dashboard counts chunks, not files (#10598)
* admin: count each chunk once in the dashboard total The dashboard summed file_count from every node's volume list, so a chunk was counted once per replica and deleted chunks were never subtracted. Reuse the collection aggregation, which dedupes replicas and EC shard holders and nets out tombstones. * admin: the dashboard card counts chunks, so name it that Volumes store chunks, and a file is split into one or more of them, so the 'Total Files' card always read far higher than the number of files in the filer. Rename it to 'Total Chunks' and say so in the tooltip. * admin: collections pages count chunks once and say so The collections list and detail pages summed file_count straight off the topology, so replicas multiplied the count, tombstones stayed in it, and the detail page ignored EC volumes entirely. Take the numbers from the shared collection aggregation and label them chunks. * admin: dedupe replica chunk counts per volume instead of dividing Dividing each replica's live count by the copy count truncated a chunk per odd-sized volume, and reported half the count while a volume's second replica had not checked in yet. Replicas mirror each other's needles and deletes, so keep the fullest report per volume id. * admin: fix the collections CSV export column mapping The exporter read chunks from the EC-volume cell and shifted size and disk types with it. Read every column the table actually has. |
||
 github-actions[bot]
|
de34a1a87c | 4.41 | ||
  
|
d1f503181b |
[s3] force filer apply s3 expiry metadata (#10469)
* fix: apply S3 Expiry Metadata * add test Header X-Seaweedfs-Expires-S3 * resolve comments * test entry lookup by mtime * filer: skip s3 expiry stamp on versioned entries The s3 expiry path skips entries carrying a version id, so stamping one takes away its expiry rather than moving it onto mtime. Files under .versions/ are written once, so crtime already tracks their needles. --------- Co-authored-by: Konstantin Lebedev <whitefox@mayflower.work> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris LuandGitHub
|
a5e8254ffd |
s3: give a versioned metadata-only copy its own chunks (#10594)
* s3: give a versioned metadata-only copy its own chunks A self-copy that only rewrites metadata clones the source entry, chunk fids and all, and writes the clone back. With no versioning that is exactly right: the clone replaces the entry it came from, so one entry owns the needles the whole time. Under versioning the clone lands in a new .versions/ file and the source stays live, and nothing refcounts a plain shared chunk list -- deleting either version (a NoncurrentVersionExpiration rule, say) frees needles the other still points at, and the next vacuum makes that permanent. rclone hits this on every upload, since it stamps mtime with exactly this copy. Take the metadata-only path only where the write replaces the entry it read: the bare key of a bucket without versioning. Versioned, suspended, and versionId-pinned copies fall through to the regular copy path, which gives the destination its own chunks. * s3: reencrypt a versioned SSE-KMS key rotation instead of reusing the chunks A same-object copy that changes the KMS key id hands the source chunks straight back, on the assumption that the copy overwrites the entry they came from. A versioned bucket writes a new version beside the source instead, so the two end up sharing needles that nothing refcounts, and deleting either one frees the other's data. Reuse the chunks only when the destination really is the source entry; otherwise fall through to the reencrypt path, which also gives the new version the key it asked for rather than leaving it on the old one. * s3: make one predicate decide whether a copy replaces its source The metadata-only branch and the key-rotation strategy both answer the same question -- does this copy write back to the entry it read -- so let them share one predicate instead of pairing a same-destination check with it separately at each site. * test(s3): fail the copy regression tests when the vacuum does not run The helper swallowed a failed or non-200 request to the master, so a vacuum that never ran turned both chunk-ownership assertions into no-ops: the tombstoned needles were still readable and the surviving version looked fine either way. Require the endpoint, the request, and a 200. * ci(s3): run every versioning test in the regression gate The gate named the tests it wanted, so a new regression test sat there uncovered until someone remembered this file -- it fooled me into thinking two tests added in this PR never ran anywhere, when the comprehensive job had them all along. Invert it: run everything, and name a test only to keep it out. The delete job beside this one already works that way, and the suite costs about two minutes. Only the pagination stress tests are excluded; they build 1500+ versions, skip themselves without ENABLE_STRESS_TESTS, and have their own make target. Go's regexp has no negation, so the pattern is still assembled from a listing, the way the volume-server integration workflow does it. Note the trailing $$: make eats a lone trailing $ and takes the anchor with it. |
||
  
|
ab79d1f680 |
operation: re-assign chunk upload when replica volume is full (#10588)
* operation: re-assign chunk upload when replica volume is full When a volume reaches MaxPossibleVolumeSize, needle writes return 'Volume Size Exceeded' and the fan-out in uploadChunkToHolders fails. Previously the error propagated immediately, killing the entire chunked upload even though the master has other writable volumes. Fix: detect 'Volume Size' errors on the fan-out path, call AssignFunc again to get a fresh volume, and retry (up to 3 attempts). This avoids backup failures while a single replica volume is full and waiting for GC. Also add 'volume size exceeded' to the transient error messages so any retry path that checks IsTransientError recognises it. * util: fix transient error pattern for volume size errors The actual error message is 'Volume Size 34361499680 Exceeded 34359738368' where the numeric size separates 'Volume Size' from 'Exceeded'. The previous pattern 'volume size exceeded' would never match. Change to 'volume size' which correctly matches any capacity-full error. * operation: fix reassignment loop nits - Case-insensitive volume size match (strings.ToLower) - Propagate AssignFunc error so caller sees reassignment failure - Reset JWT fallback before each reassignment to avoid retaining stale auth * util: stop classifying a full volume as a transient error A volume at capacity does not become writable on the next attempt, so the entry only bought a retry loop's worth of sleeping before the same failure. It also reached four consumers that all retry the same target — the deletion queue, the replication sink, volume lookups, and Retry/MultiRetry — none of which reassign, and the widened "volume size" substring swallowed the replica-receive rejection from WriteNeedleBlob too. The chunked upload path recovers by asking for a different volume instead. * operation: reassign a chunk with the shared upload gate shouldReassignUpload already answers this question for the non-chunked path, keyed off the status the volume server returned rather than its message text. Reusing it covers a lost replica peer and an unreachable target as well as a full volume, and it stops a 4xx from being retried on a second volume that would reject it identically. Pulling the single attempt out into uploadChunk keeps the retry loop readable now that it wraps both the fan-out and the relay path. * test: cover chunk reassignment across volumes Pins the three outcomes the gate decides: a full volume moves the chunk to a fresh assignment, a 4xx stays put, and the loop gives up after chunkAssignAttempts volumes. * operation: roll back the fid a reassigned chunk abandons ReplicatedWrite commits the needle locally before it replicates, so a 5xx can leave a copy behind on a volume the chunk is about to walk away from. Nothing will ever reference that fid, and an unreferenced needle is not garbage vacuum can find — it is dead space until the volume is destroyed. uploadChunkToHolders rolls back only the holders that reported success, which misses the one whose write landed but whose response did not, and the relay path had no rollback at all. Delete from every holder of the abandoned assignment instead; deleting a needle that never landed is a no-op. * operation: stop the reassignment loop from multiplying work retriedUploadData already retries a chunk three times against the same URL, so wrapping it in three assignments made nine POSTs for one chunk. On the relay path that inner retry is redundant — the loop retries everything it would, and on a different volume — so cap it at one attempt per assignment and leave the budget where it was. The fan-out path keeps its inner retries: absorbing a blip on one holder beats cancelling the rest and re-uploading the whole chunk. Nothing bounded any of it by time. weed/s3api passes context.Background() so a chunk survives client disconnect, which also means no deadline cuts the loop short, and a chunk goroutine holds one of four buffer slots while it spins. Break out once another chunk has already failed the object. * operation: keep the reassignment gate's inputs deterministic uploadChunkToHolders reported whichever holder error won the channel race. That was cosmetic while the value was only logged; now it decides whether the chunk moves to another volume, so a 400 and a 500 arriving in either order made the retry behavior depend on scheduling. Prefer an error the caller can act on, and the same failure always retries the same way. A failed reassignment also overwrote the upload error that prompted it, which buried a full volume behind whatever the filer happened to say. Keep both in the chain. The tests grew a JWT per assignment, since the loop re-derives one and nothing covered it, and the bound is now spelled out rather than compared against the constant that defines it. * operation: roll back the last abandoned fid too The rollback ran only on the path that goes on to reassign, so the attempt that exhausts the budget — or stops because another chunk already failed the object, or because the error is not one a different volume fixes — left its fid behind. That is the case that matters most: no chunk names it, the caller gets no fid to clean up, and a 5xx can still mean the needle was committed. Roll back on every failed attempt instead, before deciding whether to retry. --------- Co-authored-by: timolow <timolow@users.noreply.github.com> Co-authored-by: timolow <tim@timolow.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris LuandGitHub
|
e8020910db |
iam: authorize IAM management actions as IAM actions (#10593)
* s3: keep a non-S3 action out of the request-shape resolver
ResolveS3Action reads the request shape before it looks at the base action, so
an iam: or sts: action on a request that happens to carry an S3 query parameter
came back as the S3 action for that parameter. An action that already names its
service is resolved; there is no S3 request shape to read for it.
* iam: authorize the standalone IAM server's actions as IAM, not as S3
The standalone `weed iam` server wrapped its single POST / route in the generic
S3 Auth middleware with ACTION_ADMIN. The route has no {bucket}, so the check
ran with an empty bucket and resolved to a coarse S3 action rather than the IAM
one. The embedded IAM surface checks iam:<Action>; the standalone one was never
updated to match.
Both now go through one authorization function, so they cannot drift apart
again. It also rejects the anonymous identity, which has no user of its own to
run a self-service action against, and reads UserName from the body only, where
the handlers read it from.
|
||
|
Chris LuandGitHub
|
c2b47967bd |
s3: retire the suspended null marker only once the PUT has committed (#10589)
The suspended PUT dropped the null delete marker before writing, so a failed write left the .versions pointer naming a marker that was gone. The read path heals a dangling pointer by promoting the newest survivor, so a key the caller had deleted came back serving an older version, and the heal persisted that pointer. Move the retire into afterCreate via the shared finalize, which also brings the ownership check the copy and multipart paths already have. |
||
|
Chris LuandGitHub
|
f09bc14165 |
s3: report the effective ownership when a bucket has none stored (#10591)
* s3: report the effective ownership when a bucket has none stored GetBucketOwnershipControls read Seaweed-X-Amz-Ownership straight out of the bucket entry, so a bucket that never had one written reported an empty ObjectOwnership. The object write path defaults the same missing attribute to BucketOwnerEnforced, so the API contradicted the behavior it describes. Resolve the stored value through one helper both readers share, and let PutBucketOwnershipControls persist unconditionally so setting the default value still gives DeleteBucketOwnershipControls something to remove. * test: cover the bucket ownership controls round trip Pins the behaviors the ownership default fix depends on: a bucket that never had ownership controls written reports BucketOwnerEnforced, and putting that same value on such a bucket still persists it, so the delete that follows has something to remove. The put-then-delete case gets its own bucket -- run after an ObjectWriter put, it would pass against an implementation that skips only the initial write. The acl workflow already runs this package against a live weed mini, so it needs no wiring. |
||
|
Chris LuandGitHub
|
5269d93fa8 |
s3: let a suspended-versioning multipart completion replace the null delete marker (#10585)
* s3: let a suspended-versioning multipart completion replace the null delete marker In a versioning-suspended bucket a DELETE writes a null delete marker into the key's .versions directory. CompleteMultipartUpload then writes the new null version at the regular path but left that marker in place, so the completion returned 200 and the object listed while HEAD and GET kept resolving the marker and answered NoSuchKey. PutObject already handles this; do the same on the multipart path. * s3: order the suspended-versioning null cleanup behind the multipart write Removing the null delete marker before writing left a failed completion having already published the key's newest real version: the marker was gone, the pointer still named it, so reads rescanned .versions and promoted the older version. Do both fixups only once the write commits, pointer first so reads never see a pointer aimed at a marker that is no longer there, and fail the completion when the pointer cannot be cleared instead of returning 200 for an object HEAD and GET still miss - a non-ErrNone finalize keeps the upload directory, so the caller's retry replays it. Also cover a pre-suspension real version in the regression test. * s3: skip the suspended null cleanup when a concurrent write won the key The completion's .versions fixups are unconditional rewrites of shared state and the routed path runs off the object write lock, so a DELETE landing between the multipart write and the cleanup had its own null delete marker erased - leaving a successfully deleted key readable as an older retained version. Re-read the object first and leave the cleanup alone unless it is still the one we wrote. This narrows the window rather than closing it; a compare-and-set pointer flip is the real answer and wants its own change. * s3: re-read the completed object from the filer that took the write The guard compared the object against our upload id through the routed read, which skips an owner it recently found unreachable and falls back local-first. A write that just landed on the owner could then read as superseded on another filer, skipping the cleanup and leaving the key unreadable - the bug this set out to fix. Read back from the filer the write went to instead. * s3: trim the suspended-completion comments to the non-obvious why * s3: lift the suspended null-write finalize into a named helper The pointer-then-marker ordering is policy shared by every suspended null write, not something the multipart path should be stating on its own; putSuspendedVersioningObject and the copy path each restate it today. Give it a home next to the versioned finalize helpers, and reuse the canonical key normalizer and the existing test helpers rather than open-coding both. * s3: retire the null delete marker on a suspended-versioning copy The suspended CopyObject branch cleared the .versions latest pointer but left the null delete marker a preceding DELETE wrote. While the regular-path object owns the null slot that marker is shadowed, so it reads and lists correctly - but it resurfaces as a phantom delete for a key nobody deleted once that null version goes away. Route the branch through the shared finalize. * s3: keep the suspended null cleanup from erasing a concurrent delete Retiring the marker on the copy path reopened the race the multipart path had already closed: a DELETE landing between the write and the cleanup lost its own marker, so a rescan promoted an older version under a deleted key. Move the ownership check into the shared finalize, keyed on the attribute that identifies the caller's write, so both paths get it. |
||
|
Chris LuandGitHub
|
7063b3e14c |
s3 lifecycle: bound the daily-replay pass so a quiet cluster stops wedging the job (#10578)
* s3 lifecycle: bound the daily-replay subscription at the pass boundary A pass opens one meta-log subscription and 16 shard drains, then waits on all of them. Nothing told the subscription where the pass ends, so the only exit was the fan-out spotting an event past runNow — i.e. some unrelated write landing under /buckets after the pass started. On a cluster that goes quiet the reader parks in Recv, every shard drain starves on an empty channel, and Run never returns. The job sits at stage "starting" with the executor slot held and no log line, so expiry stops cluster-wide until someone restarts the worker. The pass covers (globalStartTsNs, runNow], so say that: UntilNs on the subscribe request makes the filer end the stream once it has shipped that range. The reader then closes the event channel on the way out, which is what unblocks the fan-out and the drains when the stream finishes on its own rather than by cancellation. Same fix retires the other silent hang: a reader that failed early (subscribe error, stream error) also left every drain waiting forever. * s3 lifecycle: keep a halted shard from starving the shared fan-out A drain that halts mid-stream (BLOCKED / RETRY_LATER / an RPC error on dispatch) returns while the fan-out is still routing that shard's events. After 256 of them the per-shard buffer is full and the fan-out blocks on the send, so no other shard sees another event. Run's WaitGroup never drains, and the teardown that would cancel the reader sits behind that wait — the pass wedges exactly like an idle subscription did, with one S3 hiccup as the trigger. Keep discarding the channel after runShard returns. The events are past this shard's saved cursor and get re-scanned next pass anyway. * s3 lifecycle: assert the starved shard actually made progress The fan-out test only checked that Run returned, which a version that quietly dropped the second shard's events would also satisfy. Assert the dispatch landed and the cursor moved. recordingClient gains a per-object outcome map: the two shards dispatch from separate goroutines, so pinning BLOCKED by call index was a race waiting to pick the wrong shard. * s3 lifecycle: fail the pass when the shared subscription dies Closing the event channel on reader exit is what unblocks the shard drains, but it also means a subscribe that never opened, or a stream that broke mid-pass, now ends every drain cleanly. Run logged that at V(2) and returned the shard result — so a filer failure produced a green lifecycle job that had processed nothing. Surface it as the pass error. Cursors still hold what was processed and tomorrow resumes there; what changes is that the job stops claiming success. Cancellation has to stay a non-error — the shell driver's -runtime cap is a truncated pass, not a failed one — and a canceled gRPC stream arrives as a status code, not a wrapped context.Canceled, so isCanceled checks both forms the way the rest of the tree does. * s3 lifecycle: decide reader cancellation by intent, not status code A stream we cancel and a stream the filer cancels both arrive as codes.Canceled, so classifying the reader's exit by its error let a truncated pass report success whenever the failure happened to carry a cancellation status. Intent is knowable exactly, so read that instead: the pass stops on purpose only when the caller's context ended (the shell driver's -runtime cap) or the fan-out hit the pass boundary itself. Everything else is a broken subscription and fails the pass. TestRun_ServerSideCancelFailsThePass and TestRun_CappedPassIsNotAFailure are the same codes.Canceled from the reader with opposite verdicts — the pair only passes because the decision no longer looks at the error. * s3 lifecycle: time out a subscription that stops delivering UntilNs ends a healthy stream and gRPC keepalive catches a dead connection, but neither reaches a filer that keeps answering pings while its handler has stopped producing. The pass would wait on that forever, since s3_lifecycle is the one job type with no execution timeout. Bound the wait for each response at 20 minutes, and opt into the filer's idle heartbeats so a caught-up stream proves liveness instead of looking stalled. The default sits above the filer's 15-minute metadata-gap recovery budget, so a subscriber legitimately parked on a gap is never mistaken for a stalled one. Recv is only interruptible by killing the RPC, so it moves to its own goroutine behind a per-response deadline. The timer covers only the wait on the filer — dispatch to Events happens outside it, so a slow consumer can't trip the watchdog. Approach and the 20-minute figure are from #10577 by way of comparing the two fixes; the wiring differs because the reader here ends the pass by closing its event channel rather than cancelling the fan-out. * s3 lifecycle: trim the comments added by this branch Keep the non-obvious why, drop the prose restating what the code says. * s3 lifecycle: snapshot reader intent where the reader stops Sampling ctx.Err() during teardown reads it after the drains and cursor saves have run. A reader that failed while the deadline was still live, on a pass whose teardown then outlives that deadline, was classified as an intentional stop and reported success. Sampling earlier in Run is not the fix either: before the shard wait, a legitimately capped pass has not reached its deadline yet and would be misclassified the other way. Intent belongs where the reader actually stops, so the reader goroutine records it next to the error it returns. Reported by greptile on #10578. * s3 lifecycle: cover the worker-dispatched pass with nothing due The e2e suite drives the shell command in 14 of 15 files; the one test on the real admin->worker path backdates an object, so its own delete pushes a meta-log event past the pass boundary and ends the pass. The branch where a pass has nothing to dispatch was never exercised through the worker. Cover it, asserting the pass returns on its own: no admin cancellation, and the executor slot free for the next one. This is not a regression test for the wedge. A pass used to end when any write landed past its boundary, and on a shared test cluster something usually does — the whole suite passes on the unfixed build, verified. The deterministic guards stay the dailyrun unit tests; this one would catch a pass that hangs unconditionally. |
||
|
Chris LuandGitHub
|
44e546a933 |
shell: pick tier.move replica targets with the shared placement picker (#10582)
The command chose destinations by walking its location list in order, so it neither preferred a node near the source nor spread a burst of copies. Replica placement and "this node already holds the volume" move into the Accept predicate; the ranking and the per-pick reservation come from placement. Claude-Session: https://claude.ai/code/session_01Ks16jnt4S7gdDk8cheQ3xu |
||
|
Chris LuandGitHub
|
7d6c83b126 |
s3: stop treating a directory marker as a versioned object (#10573)
* s3: delete a directory marker instead of versioning it The key "dir/" is stored as the filer directory itself, so a delete marker cannot stand in for it without hiding the children underneath, and its history has to sit inside the directory it describes, where listings keep meeting it. Delete it the way an unversioned bucket already does: remove the directory when nothing is left under it, demote it to a plain directory when children remain, and drop a history an older build recorded for it. * s3: stop resolving directory markers through a version history Nothing records one for them any more, so the lookups that read it are dead weight - and the one in the listing was a filer round trip per directory marker returned, which for a bucket that keeps a marker per directory is the whole listing cost. A listing reads what a directory stands for straight off the entry it already has; a unit test pins that N markers cost one ListEntries rather than N+1. The guard that keeps a history left inside a directory by an older build from surfacing as a key named after it stays. * s3: do not let deleting "dir/" destroy the object at "dir" Writing under an existing object turns that object's entry into a directory while it keeps its data, so the keys "m2" and "m2/" end up sharing one entry. Stripping the entry to delete "m2/" therefore wiped the object at "m2" - a different key, and in a versioned bucket one no delete marker records. Leave a directory holding uploaded data alone; "m2/" does not name it. * s3: make the directory-marker delete fail closed and take the write lock The guard that spares a promoted file only fired when the entry read succeeded, so a transient filer error fell through to the delete and could destroy the object at "dir" anyway. Fail the request instead, take the object write lock so the entry cannot change between the check and the delete, and report a stale history that cannot be removed rather than leaving it to keep naming the key in ListObjectVersions. * s3: check If-Match inside the directory-marker delete lock The lock belongs to the caller: taking it inside the delete nested it under the batch handler's own lock, and since every lock from a gateway shares one owner the inner release would have freed it while the outer caller still assumed it held it. Both callers now own the lock, the single-object path re-checks If-Match inside it the way the other delete paths do, and a batch delete of a trailing-slash key in an unversioned bucket goes through the same marker path instead of the raw delete. A history lookup that fails now fails the delete. |
||
|
Chris LuandGitHub
|
03388c4beb |
placement: let callers reject candidates placement cannot judge (#10581)
Replica placement rules and "this node already holds the volume" are constraints the picker has no way to model. The predicate receives the candidate's rack and data center, because the constraints needing them are exactly the ones a bare node cannot express. Claude-Session: https://claude.ai/code/session_01Ks16jnt4S7gdDk8cheQ3xu |
||
|
Chris LuandGitHub
|
aceab0802e |
placement: move the target picker out of the shell package (#10580)
weed/shell is the CLI command layer: every file there registers a command in init(). Importing it for placement drags that whole surface, and its registration side effects, into callers that run no CLI. The picker now depends only on master_pb and storage/types, with a node type of its own -- smaller than the balancer's Node, which also carries the volumes it holds, and placement never needs those. Claude-Session: https://claude.ai/code/session_01Ks16jnt4S7gdDk8cheQ3xu |
||
|
Chris LuandGitHub
|
4b12af036c |
shell: add a reusable target picker for volume moves (#10579)
* shell: add a reusable target picker for volume moves Picks the emptiest node near the source: locality first (same rack, then same data center), emptiest within a tier. Free bytes decide where the cluster reports them, free slots break the tie. The pick is spent in the passed topology, so planning several moves from one snapshot spreads them instead of stacking every one on whichever node started emptiest. Claude-Session: https://claude.ai/code/session_01Ks16jnt4S7gdDk8cheQ3xu * shell: order targets on one metric, and reserve the real volume size Comparing some pairs on free bytes and others on free slots is intransitive, so the winner depended on sort order. One node too old to report filesystem bytes now puts every candidate on slots. Reserving the tier average let a batch of large volumes overcommit a destination; callers pass what the move actually consumes. Claude-Session: https://claude.ai/code/session_01Ks16jnt4S7gdDk8cheQ3xu |
||
|
Chris LuandGitHub
|
505049a4de |
volume: skip directory fsync on Windows, report a failed makeupDiff (#10572)
* volume: skip directory fsync on Windows * ci: run the windows jobs for the whole vacuum path Both windows jobs start the same weed mini cluster, so both exercise the volume server's vacuum path, but only one of them watched a single file in it. Cover the compact, reconcile and load files in both. * volume: report a failed makeupDiff instead of discarding it The cleanup removes assigned to the same err the makeupDiff failure was held in, so an aborted compaction returned nil once both removes succeeded. The master then recorded the vacuum as committed and the volume reloaded against the discarded generation. * volume: correct the fsyncDir comments after the windows skip Both comments described the old shape, where windows fell through to a sync whose error was swallowed. * volume: keep the makeupDiff failure ahead of its cleanup errors A failed remove of .cpd/.cpx outranked the failure that abandoned the compaction, so the caller saw the cleanup error instead of the cause. Log it and return the original, matching the Rust do_commit_compact. A leftover temp file is rolled back by reconcile on the next start. |
||
|
Chris LuandGitHub
|
d448e9db7b |
iceberg: withhold the S3 endpoint from credential-vending clients (#10570)
* iceberg: withhold the S3 endpoint from credential-vending clients A client that sends X-Iceberg-Access-Delegation: vended-credentials builds its storage credential out of the LoadTable config and drops the one it was configured with. We vend no credentials, so the endpoint we advertised left DuckDB signing nothing: every metadata and data file came back 403, and its attempt to refresh the empty credential 404ed on stage-created tables. Answer those clients with no config at all so they keep their own credentials. Clients that do not ask for delegation still get the endpoint. * iceberg: mark load responses as varying on the delegation header The FileIO config in a table or view load response now depends on whether the client asked for vended credentials, so a cache between us and the client must key on that header rather than on the URL alone. * test: cover the DuckDB vended-credentials access pattern Runs weed mini with -s3.externalUrl, which is what makes the catalog advertise an endpoint at all, and checks both halves: a plain LoadTable still gets the endpoint, while one asking for vended credentials never gets an endpoint without the credentials to sign with. The DuckDB round trip creates a table from a query and reads it back, which is the flow that failed with 403 on every data file. |
||
|
Chris LuandGitHub
|
474a0713b0 |
s3: honor the version history of a directory marker (#10571)
* s3: stop listing a directory marker whose latest version is a delete marker A directory marker is stored as the filer directory itself, so deleting the key "dir/" writes its delete marker into dir/.versions while the directory keeps its mime and stays a key object. Every listing kept reporting the key. Consult that history before treating the entry as a key, and demote it in memory when it is delete-marked so live children still hold the prefix. Also skip the container's own .versions entry while listing inside it: the suffix match read it as the history of a nested object named "", which surfaces as a phantom dir/dir key as soon as a live directory version exists. * s3: a directory marker with version history is not also the latest null version The directory entry behind the key "dir/" is that key's null version, but list-object-versions reported it with IsLatest hardcoded true. After a delete the key came back twice, once as the delete marker and once as a null version, both claiming to be latest. Read the pointer under the directory instead. * s3: resolve directory markers through their version history on GET and HEAD GET and HEAD short-circuit any trailing-slash key straight to the filer directory, so a directory marker kept answering 200 after its delete marker was written. Resolve the key from dir/.versions first when the bucket is versioned: a delete-marked current version answers 404 with x-amz-delete-marker, a named delete-marker version answers 405, and a key with no history keeps today's directory-probe behavior untouched. * s3: re-creating a directory marker retires its delete marker PutObject on a trailing-slash key never looked at the bucket's versioning state, so re-creating a marker after a delete left the latest-version pointer on the delete marker and the key stayed invisible to every versioned read. Point the key back at the directory entry, which is its null version, and drop the null version .versions may still hold — the same two steps a suspended write already takes, now shared. * s3: fail a directory-marker request whose version history cannot be read Every lookup of dir/.versions treated any error as "no history", so a filer hiccup served the directory entry for a key whose current version may be a delete marker, reported a null version as latest over one, and let a PUT report success without retiring the delete marker it was meant to retire. Only a confirmed absence takes the no-history path now. * s3: cancel the directory-marker probe stream instead of abandoning it The probe answers off the first entry and returns, leaving the ListEntries stream open for the life of the parent context. Give it a context of its own. |
||
|
Chris LuandGitHub
|
312cfe5ae1 |
Fix volume.merge corrupting every needle it copies (#10565)
* Give volume.merge the needle size the target actually indexes by needleBlobFromNeedle returned the size Append reports, which is Size(n.DataSize) - payload bytes only. The .dat header, the needle map and WriteNeedleBlobRequest.Size all use n.Size, which additionally covers the flags, name, mime and lastModified fields. Every needle volume.merge copied therefore landed with a too-small size. The target indexed it at that length, so every later read failed the header check in ReadBytes with a size mismatch, and on v3 the fresh AppendAtNs stamp landed NeedleHeaderSize+DataSize+NeedleChecksumSize into the blob - exactly on the flags byte - overwriting flags, name size, mime size and the first mime bytes with the top of a timestamp. Needles came back with flags 0x18, no name, no mime and a phantom TTL parsed from two arbitrary timestamp bytes; the ones that decoded as expired 404 and vacuum would drop them. Since merge rebuilds every replica from the merged copy, no clean replica survives. Return n.Size, which Append fills in as it serializes, matching what the normal write path stores via nm.Put. * Reject needle blobs whose size disagrees with their own header WriteNeedleBlob trusts the caller's size for two destructive things: it is what goes into the needle map, and it is where the v3 AppendAtNs stamp is written inside the caller's buffer. A caller passing the payload-only DataSize convention corrupts both, and nothing surfaces until the needle is read back - by which point every replica may already have been rebuilt from it. Parse the blob's own header and refuse the write when the two disagree. Mirrored in the Rust volume server. |
||
|
Chris LuandGitHub
|
b1fecf3b44 |
mount: mark windows files archived and ignore a zero timestamp (#10559)
* mount: mark windows files archived and ignore a zero timestamp Windows synthesises NORMAL when a file reports no attributes at all, which is not the same as ARCHIVE and is what create_fileattr_test checks. Utimens also wrote a zero timestamp through. Windows sends zero for a field it is not setting, and storing it put 1970 in the atime overlay, which then overrode the entry's real time — so a file created a moment ago reported an access time of 1970 whenever the caller asked through an open handle. Reading the path instead went down a different route and looked right, which is why a probe of a fresh file showed nothing wrong. * mount: match the file type by its mask, and only treat the epoch as unset S_IFDIR is part of the multi-bit type field rather than a flag, so masking against it alone also matched a symlink, which shares the bit. A regular file is now identified by the type mask. Rejecting every timestamp at or below zero also rejected a date genuinely before 1970. Only the epoch itself is what Windows sends for a field it is not setting, so that is all that is refused. create_fileattr goes back on the known-failures list: the archive fix works and the test simply moves on to ask for READONLY too, which needs Chflags. Taking it off was premature. * mount: drop the time overlays when an inode is released atimeMap and dirMtimeMap are keyed by inode and were only ever trimmed by a random eviction at capacity. Inodes are derived from the path, so a delete and recreate hands the same number to a different file, which then reported the previous file's access time — a file created a moment ago answering with a time from long before it existed. Cleared when Forget actually releases the inode, not on every decrement: a partial forget still has users. Forget now reports that so callers holding state keyed by the inode know when to drop it. * ci: keep getfileinfo listed while its access time is unexplained Two causes have been fixed and neither closed it, so the honest state is listed-with-a-reason rather than removed in hope. * mount: drop timestamp overlays while the inode table is locked Forget released the inode under the table's lock but cleaned up the atime and dir-mtime overlays after returning from it. Inode numbers are derived from the path, so a lookup arriving in that window is handed the same number back and can store a time that the cleanup then deletes. Run the cleanup at the release point instead, as a callback under the lock. The directory-cache purge stays deferred until after the unlock, where it has to be. Claude-Session: https://claude.ai/code/session_01EgY2QA3iiPtiu6ww3P2EBn |
||
|
Chris LuandGitHub
|
b452a5e41b |
s3: honor a bucket owner recorded as an identity (#10567)
* s3: resolve a bucket owner recorded as an identity The admin UI and weed shell record a bucket's owner as an identity name in s3-identity-id and never write the account id the S3 API stores alongside it, so such a bucket looked unowned: its ACL owner fell back to the default admin account, and under the default BucketOwnerEnforced ownership every object uploaded to it was stamped with that account instead of the bucket owner. Resolve the identity to its account when no account id is recorded, in the one place both the bucket metadata and the bucket config derive the owner from. * s3: drop the recorded account when the bucket owner is reassigned Changing the owner of a bucket created through the S3 API left its old account id behind, and that outranks the identity when the owner is resolved, so the new owner never took effect for object ownership or the bucket ACL. |
||
 Mohit TalniyaandGitHub
|
0815ad78f6 |
fix(volume): persist the leveldb needle map watermark at batch boundaries (#10557)
levelDbWrite persists the replay watermark when its updateWatermark
argument is true. Put and Delete passed "watermark == 0", which is true
on exactly the writes that carry no checkpoint and false on the batch
boundary that carries one. The two cases were inverted:
recordCount % watermarkBatchSize != 0 -> watermark 0, flag true
-> re-persists a zero on 9999 of every 10000 writes
recordCount % watermarkBatchSize == 0 -> watermark N, flag false
-> drops the only value worth saving
The stored watermark therefore never left 0. Recovery stayed correct,
because replaying .idx from offset 0 is a superset of replaying from N
and replay is idempotent, so this never surfaced as a failure. It only
meant generateLevelDbFile walked the entire index on every rebuild, and
every needle write paid a second leveldb Put to rewrite the same zero.
Pass "watermark != 0" so the boundary write checkpoints and the writes
in between leave the key alone.
Verified on a 25000-needle volume: the stored watermark now reads 20000
instead of 0, and a rebuild replays 5000 entries instead of 25000.
The new test drives a full batch of Puts and a full batch of Deletes to
cover both call sites.
|
||
|
Chris LuandGitHub
|
f46b2a1925 |
Stop the filer test helpers from pinning gigabytes of log buffers (#10560)
* log buffer: wake the interval loop on shutdown instead of sleeping through it loopInterval parked in time.Sleep(flushInterval) and only re-checked IsStopping when it woke, so a buffer shut down early kept both loop goroutines - and the PreviousBufferCount+1 slabs of BufferSize they reach - alive for up to a full interval afterwards. Select on shutdownCh against a ticker instead, and give the loops a WaitGroup so a test can observe that they exit. * test: release the filers the server tests build Every helper here left its filer's meta log buffer running, so each test pinned PreviousBufferCount+1 buffers of BufferSize for the rest of the run: ~3.5GB of live heap across the package, which overruns the address space on linux/386 and kills the 32-bit job with an out-of-memory throw. Thread the test through the helpers so the buffer is shut down on cleanup, and shut the subscribe harness's filer down outright - its deletion loop keeps the whole filer reachable otherwise. That harness quiesces its flush path first, since Filer.Shutdown closes the store a flush still in flight would write through. |
||
|
Chris LuandGitHub
|
5a5cd15054 |
mount: report . and .. from windows directories (#10556)
* mount: report . and .. from windows directories WinFsp strips the dot entries for the root itself and expects every other directory to report them, the way a real NTFS enumeration does: its dirctl test asserts a subdirectory's first two entries are "." and ".." and that a hundred files enumerate as 102 entries. Dropping them unconditionally is what fails querydir_test. The Go test that guarded the old behaviour went with it: os.File.Readdir filters dot entries itself, so it could never have observed either way. * mount: give the windows dot entries their directory type The readdir fills an attribute block only for real children, so "." and ".." arrived with a zeroed one and were reported with mode 0. Windows refuses to enumerate a directory whose first entry is not marked as a directory, which is the assertion querydir_test fails on with STATUS_OBJECT_NAME_NOT_FOUND. They now carry the type the readdir already knew. The explorer walk also names any unexpected entry rather than only counting, so a dot entry leaking through reads differently from a missing file. |
||
|
Chris LuandGitHub
|
b8cba2982c |
mount: tell windows about changes made elsewhere (#10553)
* mount: tell windows about changes made elsewhere Nothing invalidates a Windows client's cache from this side, so a file created or removed by another mount, the S3 gateway or the filer API stayed invisible in Explorer until the user refreshed by hand. The mount already receives those events; they just had nowhere to go. WFS gains a listener for every applied metadata event, and on Windows that turns into the WinFsp notification for the path. A rename reports both ends, since the destination's own event may never arrive when it falls outside this mount. * mount: report a removed directory as a directory Entry is nil once a path is vacated, so asking it whether the thing that went away was a directory always answered no and every removal was reported as a file. Windows watches the two through different filters, so a folder removed elsewhere never refreshed. The invalidation now carries what used to be there, which the event already knew and simply was not passing on. * mount: report a rename destination once The event stream already carries a second invalidation describing the new path, so reporting RenamedTo here sent the destination twice — and always as a create, so a moved directory arrived as a create followed by a mkdir. |
||
|
Chris LuandGitHub
|
a0e278f86f |
mount: forward extended attributes on windows (#10554)
weed/mount implements all four xattr operations and the filer stores the values, but the Windows adapter overrode none of them, so cgofuse's defaults answered every call with 'not implemented'. WinFsp advertises extended attribute support either way, because cgofuse registers the callbacks unconditionally, so applications were told the volume has them and then refused on every use. Attributes written from Linux were invisible from Windows. Untested in CI: exercising Windows extended attributes needs the native NtSetEaFile path rather than anything in os or PowerShell. |
||
|
Chris LuandGitHub
|
e377149d39 |
mount: support mounting on Windows through WinFsp (#10536)
* mount: add the WinFsp filesystem adapter WinFsp speaks a path-based FUSE dialect; weed/mount implements the inode-based raw protocol the Linux kernel uses. This translates between them so Windows runs the same filesystem code as everywhere else rather than a second implementation: paths resolve to inodes one Lookup at a time, and the raw operations run unchanged underneath. Errno translation is spelled out rather than passed through. Go numbers Windows errnos as offsets from APPLICATION_ERROR, so the raw value would mean something unrelated by the time WinFsp read it. Hard links return ENOSYS since WinFsp has none, and byte-range locks stay with its kernel driver rather than the mount's lock table. Not reachable from the mount command yet. * mount: build the winfsp errno table with explicit precedence Platforms alias errnos differently: freebsd has no ENODATA and linux makes ENOATTR the same value as it. A map literal with colliding constant keys does not compile, so build the table and let the first entry win, keeping the general codes their own meaning. * mount: wire the winfsp adapter into the mount command RunMount was one function doing filer setup, mount-point preparation and serving. The setup is the same everywhere, so it moves to mount_common.go and each platform keeps only what differs. Windows differs mostly in the mount point: WinFsp wants a drive letter or a path that does not exist yet, so none of the unix preparation applies, and a bad one is worth rejecting up front because WinFsp reports failure as a bare false. Adds -windows.caseInsensitive for software that expects Windows naming rules. * ci: mount on windows and exercise it Builds weed.exe, installs WinFsp, starts a cluster, mounts S: and runs a test suite against it: round trips at several sizes, offset writes, rename, delete, nested directories, concurrent writers, and a directory wide enough to stand in for the case that prompted this. Nothing else here can run the Windows mount, so without this the adapter is only known to compile. * ci: build the windows mount without cgo The runner has MinGW, so cgo is on by default and cgofuse compiles its cgo variant, which needs WinFsp's headers. The nocgo variant loads the DLL at run time and is what the released weed.exe uses. * mount: make the winfsp path splitting portable and test it resolve and resolveParent had the splitting inline in a windows-tagged file, so the cases that matter most there — both separators, empty and dot components, the root having no parent to create in — could not be tested on any runner that builds this. * test: check the windows mount persists across a remount Reading a file back through the same live mount proves nothing about durability; the answer can come from the mount's own caches. Write the fixtures, confirm the filer serves them with the mount out of the path, then re-read after a teardown and remount. * test: cover the windows mount operations that had none Truncate, append, chtimes and the hard-link refusal were implemented but never exercised, and the errno table was only unit-tested for mapping, never end to end. Adds names that have to survive the UTF-16 boundary, rename over an existing target and across directories, and concurrent handles on one file rather than one file each. * ci: dial the filer over ipv4 and run the persistence phases localhost resolves to ::1 first on windows and the cluster binds ipv4 only, so the mount's grpc dial was refused while the http readiness probe passed by falling back to ipv4. * ci: pin the cluster to loopback and probe ports by connecting weed mini advertises the runner's LAN address and binds filer grpc there, so the mount's dial to 127.0.0.1:18888 was refused while http answered. The readiness probe also passed with nothing on 18888: Test-NetConnection reported success for a port that then refused a connection, so it now opens a socket instead. * ci: report listening ports before mounting The readiness probe connects to the filer grpc port and the mount is then refused on it, which cannot both be true; print the actual state. * ci: run the cluster, mount and tests in one step The runner tears down a step's process tree when its shell exits, so the cluster started in an earlier step was already gone: the readiness probe passed against a live filer, the step ended, and the mount then found nothing listening. A diagnostic step reported no weed.exe at all. Everything that needs those processes alive now shares a step. * mount: key windows file io on the handle, not the path Read and Write walked the path on every call to fill in a NodeId the raw filesystem never reads: both look the file up by handle. Under eight writers creating files in one directory the walk transiently missed and the write failed with ENOENT before reaching the filesystem at all. Same for flush, fsync and the release calls. O_EXCL now fails on an existing name instead of taking it over, and Symlink is refused: the entry is easy to create but WinFsp only follows it once the reparse point is wired up, so it read back as an empty file. * mount: translate cgofuse open flags for windows cgofuse reports MSVC's numbering and the raw filesystem tests Go's, so only the access mode and O_TRUNC lined up: O_EXCL arrived as O_APPEND and O_CREAT as nothing at all. Also report which handle a failed write was using, to tell a handle that was never issued from one released while still in use. * mount: report which step of a windows create failed A concurrent create fails with ENOENT and the path walk, the parent lookup and the create itself are indistinguishable from the caller. * ci: send weed logs to stderr on windows glog writes to its own files by default, so the mount's own error output never reached the redirected log. Its flags are global and have to come before the subcommand. * mount: resolve known paths from the inode table on windows Every create walked the parent chain with a filer lookup per component. With eight writers creating files in one directory that is hundreds of concurrent lookups of the same parent, and lookupEntry reports an authoritative ENOENT when the directory is cached, the entry is not in the cache and the inode table has no record — a window a concurrent refresh can open for a directory that plainly exists. A path the mount already tracks now resolves straight out of that table. * test: sync the windows persistence fixtures before closing The mount is killed rather than unmounted, so anything still queued for flush is legitimately lost and the test was measuring crash durability while calling it persistence. A 9MB file lost four chunks that way. * mount: keep the lookup refresh on the target path Resolving a tracked path straight from the inode table skipped Lookup, which is also what refreshes the entry: a truncate then read back the pre-truncate size. Only the parent chain takes the shortcut now, which is where the concurrent creates were racing anyway. * mount: log every windows resolve failure Open suppressed ENOENT and Getattr logged nothing, which hid the two callbacks that can report a missing file during a create. * mount: drop dot entries from windows directory listings readdir reports "." and ".." for the kernel, but Windows enumerates a directory without them and displays whatever it is handed, so a folder of 200 files listed 202. Go's ReadDir filters them, which is why only the PowerShell walk caught it. * mount: flush queued writes when windows mount is interrupted The signal handler exits the process the moment its hooks return, so the WaitForAsyncFlush after Serve never ran on ctrl-c and queued writes were dropped. * mount: let windows mount over an empty directory WinFsp turns a directory mount point into a reparse point, which NTFS allows on an empty directory and refuses on a populated one. The check rejected every existing directory, so the ordinary habit of creating the mount point first failed with a message saying it should not exist. CI now mounts over a pre-created directory and writes through it. * ci: run the windows mount check on any pull request It is the only thing that exercises the Windows mount, so restricting it to pull requests based on master skipped it for stacked ones. Replaces the branch name that was pushed to trigger it. * mount: do not log a missing windows entry as an error Windows probes for entries that do not exist as a matter of course, so ENOENT from getattr and open is an answer rather than a fault and would have filled the log. * mount: take the fast path for parent chains in every windows resolve Narrowing it to resolveParent left Getattr and Open re-walking the parent with a filer lookup per component, and those are what Windows calls before a create: eight writers in one directory still raced a meta cache refresh there. Only the final component needs the Lookup refresh. The pass that suggested otherwise came from a run five times slower than the failing ones, where the race had no room to appear. * mount: drop the windows path resolution shortcut Resolving from the inode table skipped the Lookup that refreshes an entry, and a truncate then read back its old size. Applying it only to the parent chain kept truncate correct but left concurrent creates failing, and applying it to the final component too inverted that. The two cannot both be satisfied this way, so this returns to looking up every component and leaves the concurrent create failure open. * mount: fall back to the open handle when a deferred entry is evicted A create that defers the filer write leaves the entry only in the local cache. Creating many files at once pushes the directory past the hot threshold and evicts it, taking that placeholder with it, so a lookup went to the filer, found nothing, and reported a file that plainly exists as missing. The handle still holding the unflushed entry is authoritative for it. Caught by concurrent creates over a Windows mount, which resolves a path on every call rather than relying on a kernel dentry cache. * mount: let cgofuse resolve to the version the module graph requires rclone already depends on cgofuse at a newer commit than the v1.6.0 pin, so readonly builds refused the go.mod until it matched what MVS picks. The interface and flag values the adapter uses are unchanged there. * mount: wait for a pending async flush before looking up on the filer Open, unlink and rename already wait, but a plain lookup went straight to the filer and read pre-close metadata: truncate a file, close it, and a path probe during the flush window reported the old size. The kernel attr cache hides this on linux; a front end that resolves paths on every operation hit it directly. * mount: reject a umask wider than the file mode it becomes ParseUint allowed 64 bits and the result is narrowed to os.FileMode, which is 32, so an out-of-range umask truncated silently instead of being reported as unparseable. * mount: address review findings on the windows mount WaitForAsyncFlush closed its channel unconditionally and shutdown reaches it from both the interrupt hook and the path that resumes after serving, so a ctrl-c could panic on a second close. The deferred-entry fallback read an open handle's entry without its lock, which is what the other two readers of that field take so FromPbEntry does not walk the chunk slice mid-append. The async-flush wait also sat ahead of the meta cache, making every stat of a recently closed file queue behind uploads; it belongs just before the filer is consulted. Windows entries were persisted as uid 0: the raw filesystem stores InHeader's owner and the adapter left it zero. They now carry the identity the mount was started with. The errno table used Linux numbering while cgofuse decodes MSVC's, so ENAMETOOLONG arrived as EDEADLK and five others were likewise wrong; a windows test pins each value to cgofuse's own constant. Also: break the filer handshake loop on success rather than always running ten rounds, accept a drive letter written S:\\, report a missing WinFsp instead of panicking, keep commas out of the volume label, and drop -windows.caseInsensitive, which told WinFsp the mount folds case while lookups stayed exact. * mount: return windows lookup references so the inode table stays bounded Every operation that hands back an EntryOut grants a reference the Linux kernel returns with FORGET. WinFsp has no FORGET, so the adapter took one per path component per call, plus one per child of every readdirplus, and never gave any back: inodeToPath grew for the life of the mount. Walking the 200k-file directory this exists for stranded 200k references. The adapter now plays the part the kernel plays. Each resolution releases what it took, and an open handle keeps the reference for its inode until Release, counted because the raw filesystem reuses one handle for repeated opens. Holding it is not optional: completeAsyncFlush skips the metadata flush when the saved path no longer maps to the inode, so releasing early would lose a close's metadata. Also stops persisting the display owner. -o uid=-1 makes WinFsp report the calling user whatever we say, but the value handed to the raw filesystem is written to the filer, and 4294967295 is what every other client would read. -windows.uid and -windows.gid set what is recorded. * mount: fix windows behaviours the reference implementations guard against WinFsp has no ro option — it discards the flag and leaves the volume writable — so -readOnly accepted writes and deletes. The refusal now happens in the operations themselves. Windows sends times around its own 1601 epoch, which arrive as a large negative second count; casting them through stored a year-1601 timestamp that every other client then read. Those are now left alone. rclone carries the same guard. Chown returned ENOSYS, and WinFsp passes a chown failure straight out of SetSecurity, so Explorer's Security tab and icacls failed for edits that were not about ownership. It now accepts and discards. Only create and mkdir presented a caller; the rest sent uid 0, which hasAccess treats as root, so deletes and renames skipped the permission check that creates got. Every operation presents the same identity now. A drive letter written S:\ reached WinFsp unnormalised, which recognises a drive only as exactly two characters and then failed as a directory path. A test also pins the open flag translation, since swapping O_EXCL and O_TRUNC would turn 'fail if it exists' into 'truncate it'. * mount: answer windows getattr and truncate from the open handle WinFsp keeps the path a handle was opened with and never updates it when the file is renamed, so resolving the path again fails on a handle that is still perfectly valid — the ordinary write-temp-then-rename save pattern. The handle already knows its inode, which also removes a full path walk from two operations WinFsp calls constantly. Readlink on the root now refuses. WinFsp probes there to decide whether the volume has symlinks and enables them unless it fails, and with them on it resolves a path a component at a time, each one reaching us as its own walk — all for a feature Symlink already refuses. * mount: require the windows mount directory not to exist WinFsp creates the directory itself with FILE_CREATE and removes it when the filesystem goes away, so an existing one — empty or not — fails with "mount point in use". Allowing an empty directory was wrong, and the CI check that appeared to prove otherwise was the vacuous one: listing a plain directory succeeds whether or not anything is mounted on it, so the step passed while the mount had failed and the writes went to local disk. That check now waits for the reparse point, which is what caught this. * mount: apply review comments on the windows mount -windows.uid and -windows.gid reached the adapter but not the filesystem parameters, which is what carries the owner written to the filer, so the flags changed nothing. Readdir re-resolved the path while Getattr and Truncate answer from the handle; a directory renamed during an enumeration then failed on the stale path WinFsp still holds. Utimens now honours UTIME_OMIT instead of writing whatever came with it. * mount: tag the unix-only lock tests away from windows The production lock files were tagged when the package was made to build on windows, but the tests that exercise them were not, so anything that compiles tests for windows still failed on syscall.F_WRLCK. * ci: vet the mount tests for each target too Only compiling the non-test build let an untagged test keep a per-OS syscall constant without anything noticing. |
||
|
Chris LuandGitHub
|
4992ac1ca9 |
mount: keep the xattr flag constants off freebsd (#10552)
* mount: keep the xattr flag constants off freebsd x/sys/unix has no XATTR_CREATE or XATTR_REPLACE there, and weedfs_xattr.go is already tagged away from freebsd for that reason. Putting them in a !windows file dragged them back in, so master stopped building for freebsd. * ci: cross-compile freebsd and darwin too The windows-only check missed a freebsd break in the very file it was added to guard, because nothing else on a pull request compiles them. |
||
|
Chris LuandGitHub
|
c191b2fe01 |
iceberg: let clients select their table bucket as the catalog warehouse (#10549)
* iceberg: accept bare bucket names and ARNs as the catalog warehouse Only s3://<bucket>/ was recognized. A warehouse spelled as a bare table bucket name or as the s3tables bucket ARN -- the two forms users reach for first, the latter being what AWS S3 Tables itself takes -- was silently dropped, so every call landed on the default "warehouse" bucket and failed with "table bucket warehouse not found". * iceberg: report a missing table bucket as 404, not 500 Pointing a client at a table bucket that does not exist -- which every client with no warehouse set does, since the default bucket "warehouse" rarely exists -- returned InternalServerError with a message naming a bucket the client never asked for. Answer 404 and say how to select one. * admin: show the warehouse in the PyIceberg example The example connected without one, so it always resolved to the default table bucket and every client that copied it failed on the first call. * test: pin bearer auth against a table bucket that exists The subtest called the catalog with no warehouse and accepted 500 as proof that auth had passed, since the default bucket does not exist. A missing table bucket now answers 404, which the test read as an auth failure. Give it a real table bucket so only 200 passes. * test: assert the missing-bucket guidance reaches the client The status and error type were checked but not the message, which is the part of the mapping that tells a user how to select a table bucket. * test: encode the warehouse query value The ARN case pasted raw colons and slashes into the query string. Go's parser tolerates them, so the test passed without modelling how a client actually sends the request. |
||
|
Chris LuandGitHub
|
cc2775d9f2 |
s3: register an identity's inline account instead of collapsing it into admin (#10548)
* s3: register an identity's inline account instead of collapsing it into admin Credential stores persist an account inline on the identity and never emit a top-level accounts list, so every user created through the IAM API or the admin UI with an email hit the "non exist account ID" branch and was given the shared admin account. Distinct users then presented the same owner id, so ownership checks could not tell them apart and each passed for the others' buckets. Treat an id missing from the account map as undeclared rather than invalid: register it, keeping an email another account already claimed. Both load paths now resolve the account through one helper. * s3: refresh an undeclared account from the identity that carries it The merge path starts from the live account cache, so an identity upserted with the same account id but a new email or display name kept the cached copy: the new address never reached the email index and the replaced one still resolved. Changing a user's email through the admin UI takes exactly that path. An account registered from an inline block is only described by the identity carrying it, so refresh it and move its email claim. Accounts from a top-level list and the predefined defaults are marked declared and stay authoritative. * s3: let an account reclaim an email once its holder moves away Two identities can carry the same email, and the second to load leaves the lookup with the first. Returning early when the incoming metadata matches the cached account meant the loser never re-ran the claim, so an address freed by the holder's update resolved to nobody until the loser itself changed. Re-index on the unchanged path, which is a no-op while another account still holds the address. |
||
|
Chris LuandGitHub
|
a9de90ae29 |
test: wait for every queued flush before deleting the log files (#10546)
TestSubscribeLoop_FlushProvenGapSkipsToRetained deleted the log files once the eviction watermark's own flush had landed, while the windows sealed after it were still queued. Those flushes then wrote their files back, and the subscriber served them from disk instead of taking the gap-skip path, so the windows whose files really were gone came out missing. Wait through the last sealed window instead. |