mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-16 04:06:44 +00:00
08f0ba55645fa97fe19bab2b2117d792e3ee75c5
14684
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Chris LuandGitHub
|
08f0ba5564 |
topology: clamp the deleted-vs-total subtractions in volume stats (#10633)
VolumeLocationList.Stats subtracts the deleted figures from the totals to report live size and needle count. Both deleted figures are maintained as counters independent of the totals they come off, so either can transiently exceed its total, and neither subtraction was clamped. Unclamped, the size wraps to ~16 EB. The count is signed so it merely goes negative, but VolumeLayout.Stats converts it with uint64(fileCount), which turns it into ~1.8e19 just the same. Either one swamps the cluster totals behind /dir/status, /vol/status and Topology.CollectionVolumeStats. commandFsMergeVolumes.getVolumeSize had the same unclamped subtraction, where a wrapped size reads as a volume far too large to join any merge plan. Clamped to zero, matching the guards already in CollectionInfo.LogicalSize and the admin server's logical-size accumulator. |
||
|
Chris LuandGitHub
|
4527947afc |
mount: absorb the WinFsp metadata cache window in the concurrent-reader test (#10636)
WriteFile's own existence probe runs while the file does not exist, and WinFsp may serve that answer from its metadata cache for up to the mount's FileInfoTimeout. A reader racing into that window failed its open with not-found, which is the cache being a cache, not a defect in concurrent reading. Establish visibility once before racing the readers, so the test exercises what it is named for. |
||
|
Chris LuandGitHub
|
0b78381513 |
wdclient: keep the location of a volume reported added and removed at once (#10635)
* wdclient: keep the location of a volume reported added and removed at once A volume moved between a server's disks arrives in both lists of one message, and the server still has it. Additions were applied before removals, so the removal won and the client was left with no location for a volume that never went anywhere. Reordering would swap the bug for a window where the volume resolves nowhere, since the two updates take the lock separately. Skip the removal instead, so the order the lists are applied in stops mattering. * wdclient: build each ec update explicitly in the move test Reusing one response object and adding the deletion to it left the overlap the test turns on implicit, and reading it as a delete-only update is the natural mistake. |
||
|
Chris LuandGitHub
|
5ec813b4f1 |
topology: follow a volume that moved between a server's disks (#10628)
* topology: follow a volume that moved between a server's disks The heartbeat diff asked only whether a volume id was reported anywhere on the node, so a volume that moved to a disk of another type stayed on the disk it left as well. The master then held two copies of it forever: the volume count was overstated, and GetVolumesById returned whichever disk the map iterated first, so lookups could hand back the disk the volume had already left. Track which disk types the heartbeat named each volume on, and treat a volume named on another disk as absent from this one. Disk types are interned to an index because a server reports a handful of them across hundreds of thousands of volumes. A volume named on two disks at once is a stale twin rather than a move, and is still kept on both -- dropping one would tell the master a replica vanished. Only a volume named twice on one disk type is unrepresentable, so that is now what marks the node, rather than any repeat of an id. * master: do not tell clients a moved volume left the node A volume moved between a node's disks is removed from one and added to the other, so it lands in both lists of the same heartbeat. Clients apply additions before deletions, so the removal wins and they end up with no location for a volume that never went anywhere. Skip removals for volumes the node still holds, as the ec shard paths already do, and update the topology before judging the delta removals so an unmount that really did happen is still reported. * trim the comments on this change to the parts that are not evident * master: judge a volume removal on normal replicas alone HasVolumesById answers for ec shards as well, so a replica encoded into ec shards looked like it was still on the node and clients were never told the normal location had gone. They hold normal and ec locations separately and prefer the normal one from the same generation, so that location would have gone on shadowing the shards. |
||
|
Chris LuandGitHub
|
75ae33ade8 |
mount: let the kernel cache directory listings (#10634)
Every enumeration of a directory walked the whole FUSE machinery, so
reopening a folder cost what opening it did. The kernel has a cache for
exactly this: with FOPEN_CACHE_DIR the listing lives in the directory's
page cache and a repeat enumeration never reaches the mount at all.
Local mutations already drop that cache in the kernel. Remote ones
arrive through the metadata subscription, so the entry invalidation
worker now also tells the kernel which directory changed. The worker is
the one place this is safe from: notifying from a thread serving a
kernel request can deadlock against the page it holds, which is why the
file paths deliberately avoid InodeNotify.
Measured in a Linux container, 20k-entry directory, ls repeated:
warm listing before 199-355ms after 6-9ms
A file written from outside the mount appeared in the next listing
within a second, through the subscription notify, and the listing
re-cached after.
The memory is the kernel's page cache: reclaimed under pressure, owned
per-directory, and covering read-through directories the mount-side
caches never see.
|
||
|
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
|
5532a316c5 |
telemetry: put the version pie back beside the stacked chart (#10626)
* telemetry: put the version pie back, beside the stacked chart The two answer different questions and the pie was the better answer to one of them: what the fleet is on right now, at a glance. Restore it under its old name and give the stack its own card as Versions Over Time, so the pie is the last day of the chart below it. * telemetry: draw the distribution pies at the size of their cards Both pies kept the canvas tag's 2:1 ratio at the card's full width, so they came out around 560px tall and spilled past the card they sit in. Give them a height to fill instead, and build every chart after the dashboard is shown: a canvas in a display:none container measures zero, and a pie sized from that never grows back. |
||
|
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
|
a49cf11e16 |
telemetry: version distribution over time (#10625)
* telemetry: keep the reported version in the daily history The version only ever lived on the instance record, which holds a cluster's latest report, so there was no way to ask what anything ran last Tuesday. Record it per sample, and let the daily axis carry strings as well as counts. State written before this has no version on its samples. The newest sample is the report the instance record itself came from, so fill that one in on load rather than starting a version series a day late. * telemetry: serve the fleet's version make-up over time /api/versions gives how many clusters ran each release per day, on the same axis and hold-forward rule as the cluster sizes. Releases are ordered by number rather than by size: the caller stacks them, and a stack whose order changes with the counts is unreadable over time. The tail past the limit is summed into "other" so the stack still adds up. Days with no version are dropped before the axis is built, so the series spans the days it knows a version for instead of climbing out of blanks. * telemetry: draw version distribution as a stacked growth chart The pie only ever showed today. Stacked over 30 days the height is the confirmed fleet and each band is a release, so one chart carries the growth and the rollouts at once. Newest release on the floor, so the band being read is anchored to the axis instead of riding on everything below it. Eight fixed hues instead of the evenly spaced ones the cluster stacks use: evenly spaced put a green and a cyan close enough to be hard to tell apart, which matters for a set you read rather than a wall of anonymous ids. Versions past the eighth fold into "other", and each band carries its own number so the chart reads without matching colours against the legend. |
||
|
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
|
fb92d46e2d |
helm: enterprise license Secret, and a persistent-claim option for master data (#10601)
* helm: mount an enterprise license Secret into every component
Running the enterprise image under this chart meant hand-rolling
extraVolumes and extraVolumeMounts on every component. Missing one is
easy and quiet: a component without the license silently drops to
community mode, and on the admin that surfaces only as Data Recovery and
Point-in-Time Recovery refusing to enable, with the master looking fine.
Add global.seaweedfs.license.existingSecret. The Secret is mounted
read-only into master, volume, filer, s3, sftp, admin, worker and
all-in-one, and SEAWEED_LICENSE points every process at the file rather
than relying on the binary's search paths, which depend on the working
directory.
The mount is a directory, never a subPath: kubelet refreshes Secret
contents in place, but a subPath is resolved once at container start and
never updates, which would break license renewal. There is deliberately
no checksum annotation on the pod template either — that would roll every
pod on renewal, the opposite of what is wanted. Verified on kind: the
renewed file reached a running master ~70s after the Secret was patched,
same pod UID, restartCount 0.
Also documents that only the master re-reads the license on a timer
today; the other components pick a renewal up on their next restart.
* helm: keep master data on a claim by default
The master's -mdir holds its Raft log and snapshots, and with them the
cluster's topology UUID — the identity an enterprise license is issued
against. It defaulted to a hostPath under /ssd, which does not follow a
rescheduled pod: the master came back with an empty data directory, a
freshly generated cluster UUID, and a license that no longer matched.
With the chart's default of a single master replica there is no peer to
recover the identity from either.
Default master.data.type to persistentVolumeClaim, sized 1Gi (Raft state
is small). hostPath stays available for anyone who wants it.
This is breaking for existing releases: volumeClaimTemplates is immutable,
so helm upgrade on a release installed with the old default fails with
"updates to statefulset spec for fields other than ... are forbidden".
Verified on kind, along with both ways out — pinning
master.data.type=hostPath upgrades cleanly, and the documented migration
(stop the master, pre-seed a claim named after the StatefulSet, upgrade)
preserves the cluster UUID. Seeding has to happen while the master is
stopped; copying into a live pod loses the state, since the running
master rewrites its Raft files before the restart.
* helm: mount the license on masters only
The master is what reads the license file: it validates it, enforces the
capacity limit and binds it to the cluster UUID. Mounting the Secret on
volume, filer, s3, sftp, admin and worker put it in six more containers
that never look at it, so drop it there and keep master plus all-in-one,
which runs `weed server -master`.
Two fixes from review while here:
- project only the configured key out of the Secret, so an unrelated
key in the same Secret is not exposed to the container. Verified the
key-scoped projection still updates in place: patching the Secret
reached the running master in ~50s, same pod UID, restartCount 0.
- drop SEAWEED_LICENSE from merged extraEnvironmentVars while a
license Secret is configured. It used to be possible to render the
key twice in one container, with the user's value winning over the
path the chart actually mounts.
CI now pins the scope (master only, all-in-one separately), the
key-scoped projection, readOnly, and that SEAWEED_LICENSE renders once.
* helm: fix the documented master-data migration
The seed pod in the migration never mounted the claim it was supposed to
seed, so following the steps verbatim copied the Raft state onto the
pod's ephemeral filesystem and threw it away with the pod — landing the
reader in exactly the empty-claim, new-cluster-UUID state the section
exists to avoid. Give the pod the volume.
The names were assembled as <release>-seaweedfs-*, which is wrong
whenever the release name already contains the chart name or an override
is set; read the StatefulSet name from the cluster instead and derive the
claim from it. Also scope the procedure to the chart's single-master
default, and create the Secret in the release namespace.
Trims the enterprise prose this section had accumulated: this is the OSS
chart, and the master-data default is a durability fix that stands on its
own.
* helm: quote the projected license key, reserve it on the secret env path
A secretKey that YAML reads as a non-string (123, yes, no) rendered
unquoted into the volume's items, so the projection would not match the
Secret's key. Quote both key and path.
all-in-one renders secretExtraEnvironmentVars itself, outside the merge
helper that already drops SEAWEED_LICENSE, so an entry there could still
render the variable twice. Skip it there too while a license Secret is
configured. The master template has no such block, so this is the only
remaining path.
* helm: correct the license helper comments after scoping to masters
* helm: keep hostPath as the master data default
Defaulting master.data.type to a claim broke every existing release:
volumeClaimTemplates is immutable on a StatefulSet, so helm upgrade
failed with "updates to statefulset spec for fields other than ... are
forbidden" before it changed anything.
Keep hostPath as the default and document the claim as the option to
choose — for a new install, or for an existing one via the migration
already in the README. The chart supported both types all along; only
the default moves back.
Every immutable field of every rendered StatefulSet is now identical to
upstream under default values, so an in-place upgrade cannot trip the
API. Verified on kind: install with the unmodified upstream chart,
upgrade to this branch (ok), upgrade again turning the license Secret on
(ok, volume added in place). A fresh install with
master.data.type=persistentVolumeClaim binds its claim as before.
The whole PR is additive now: nothing renders differently until a value
is set.
* helm: scope the migration's StatefulSet lookup to the release
* helm: trim the comments added by this change
|
||
|
Chris LuandGitHub
|
a8c8372b99 |
rust: stop quarantining v2 volumes, load a disk's volumes concurrently (#10602)
* rust: only compare the .dat tail on v3 volumes Go's verifyNeedleIntegrity does the "does .dat end exactly at the last indexed needle" comparison inside its v3 branch -- it rides along with the v3 append-timestamp read -- so a v1/v2 volume carrying an unindexed trailing record loads read-write and silent. The Rust check ran it at every version, so booting the Rust server on a legacy cluster warned on and quarantined volumes the Go server had been serving happily. * rust: load a disk's volumes concurrently Opening a volume is dominated by reading its .idx into the needle map, and the loader did them one at a time, so a disk holding thousands of volumes needed thousands of serial index reads before the server came up. Go's concurrentLoadingVolumes spreads the same work over max(cores, 10) workers; do the same, keeping the directory pre-pass and the insert serial so only the open is parallel. * rust: let a failed volume open fall back to the next candidate Two collections can name the same volume id on one disk. Deduping the load queue by id claimed the id for whichever candidate the scan saw first, so a corrupt one shadowed a good one behind it; the serial loader this replaced only claimed an id once a volume had actually opened. Carry every claiming collection per id and try them in scan order until one loads. * rust: trim the new comments in the volume loader |
||
|
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. |
||
 Andrei StroianandGitHub
|
aa04889f05 |
fix(helm): mark filer db-init ConfigMap volume optional (#10596)
The filer StatefulSet declares db-schema-config-volume unconditionally, referencing <release>-seaweedfs-db-init-config. The chart never renders that ConfigMap and the README documents it as operator-supplied, so any deployment that has not pre-created it ends up with a pod spec pointing at a non-existent object. No container mounts the volume, so this is currently inert, but it is misleading and would become a pod-start failure if a volumeMount is ever added. |
||
 github-actions[bot]
|
de34a1a87c | 4.41 4.41 | ||
 |
c01fe1493d |
build(deps): bump github.com/rclone/rclone from 1.74.3 to 1.75.0 (#10595)
Bumps [github.com/rclone/rclone](https://github.com/rclone/rclone) from 1.74.3 to 1.75.0. - [Release notes](https://github.com/rclone/rclone/releases) - [Changelog](https://github.com/rclone/rclone/blob/master/RELEASE.md) - [Commits](https://github.com/rclone/rclone/compare/v1.74.3...v1.75.0) --- updated-dependencies: - dependency-name: github.com/rclone/rclone dependency-version: 1.75.0 dependency-type: direct:production ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> |
||
  
|
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
|
69aa6d7adc |
test(s3): give the copying suite room for a collection per bucket (#10590)
Every test bucket is its own collection and each grows 7 volumes, so the suite asks for 140 while the job caps the volume server at 100. Slots come back only when the volume server's next full heartbeat tells the master the deleted collections are gone, and the suite finishes inside one 5s pulse: the run survives on whichever buckets happened to be dropped before that single tick. The last run cleared by two slots, this one wedged the final PutObject with 'No writable volumes and no free volumes left'. |
||
|
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
|
3c549b33ab |
ci: deal volume server tests across shards instead of bucketing by letter (#10576)
Both workflows split the suite with ^Test[A-H] / ^Test[I-S] / ^Test[T-Z]. Test names cluster, so shard 2 drew 50 of the 114 grpc tests and 30 of the 64 http ones, and spent 13m51s against 7m48s and 9m09s for its peers. Listing the tests and dealing them out one at a time splits them 38/38/38 and 21/22/21, and keeps splitting evenly as tests are added. The pattern is computed once into the environment rather than repeated in the summary step, where the two copies had to be kept in agreement by hand. |
||
|
Chris LuandGitHub
|
f5fd5450d8 |
ci: cross-compile each target in its own job (#10575)
The four targets ran in a shell loop at ~3m13s each, so the job took 13m29s and gated the whole workflow by itself: everything else finished within 8m13s. A matrix runs them concurrently, ~3m45s wall clock. fail-fast is off so one broken target still reports the other three, instead of one target per push. |
||
|
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. |