mirror of
https://github.com/seaweedfs/seaweedfs.git
synced 2026-08-28 20:06:14 +00:00
master
163
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
  Chris LuandGitHub
|
627b5e9d59 |
shell: parse every collection filter the same way (#10955)
* worker: move the collection filter parser into weed/util/wildcard
The parser sits beside the volume-list filtering it was written for, in
weed/plugin/worker, which imports weed/shell — so the shell commands that
parse the same filter three other ways can never call it. Move it down to
weed/util/wildcard, next to the comma-separated wildcard helper it already
replaced, leaving the behavior unchanged.
* shell: parse every collection filter the same way
The shell parsed a collection filter three ways: compileCollectionPattern
compiled one regex for ec.encode, ec.decode, volume.balance and the tier
commands; volume.list and volume.deleteEmpty matched a single wildcard; and
volume.tier.move, volume.fix.replication and volume.configure.replication
called filepath.Match on their own. None of them took a list, so
"ec.encode -collection=a,b" selected nothing, the same way the admin UI did.
They all go through the shared matcher now: a comma-separated list of names,
"*" and "?" wildcards, "_default" for the collection with no name, and regex
entries. The one thing that stays per-command is what an empty value means -
every collection for -collectionPattern, the unnamed collection for the ec
and tier -collection flag - so compileCollectionPattern keeps that mapping.
The matchers are compiled once per command instead of once per volume, and a
regex entry now has to match the whole name unless it anchors itself, so
-collection=bucket no longer picks up mybucket2.
* shell: keep dots in collection names, and commas inside a regex
A dot no longer marks an entry as a regex, so a collection named "my.bucket"
matches itself and not "my-bucket" - the difference decides which volumes
volume.deleteEmpty and volume.tier.move touch. A dot still counts when it is
quantified, so "bucket.*" stays a prefix regex.
The comma split also leaves alone the commas inside a character class or a
repetition count, so "bucket[0-9]{1,3}" stays one entry instead of becoming
two broken fragments.
* shell: let a regex entry match its own spelling
A collection named after regex syntax, say "logs(2024)", was unreachable:
the entry compiled to a pattern that matches "logs2024" instead. Match the
entry verbatim as well, so naming a collection always selects it, whatever
characters it holds.
* shell: reject a collection filter that names no collection
A value of "," parsed to no entries and then matched every collection, so a
typo widened ec.encode or volume.deleteEmpty to the whole cluster. Only a
genuinely empty filter means "all collections"; anything else has to name one.
* shell: keep commas inside a regex group out of the entry split
The split already left alone the commas inside a character class or a
repetition count, but not the ones inside a group, so "bucket(foo,bar)"
was cut into two fragments that no longer compile.
* shell: cover escaping a collection name that is not a regex
A name like "logs(2024" does not parse as a regex on its own; escaping it,
"logs\(2024", reaches it. Pin that so the escape hatch does not regress.
* shell: split entries only on commas inside a closed regex construct
An unmatched "{" or "[" made the splitter swallow every comma after it, so
"foo{bar,videos" became one entry that matches neither collection - the
silent no-op this filter work exists to remove. A construct now has to close
before its commas stop separating entries.
* shell: skip character classes while scanning a regex group
A ")" inside a class is a literal, so "(a[)],b)" ended its group early and
split into two fragments that no longer compile.
* shell: cover escaping a comma inside a collection name
A comma separates entries, so a name holding one is reached by escaping it.
* shell: follow the regexp parser when scanning a character class
A "]" leading a class is a member of it, and a POSIX class such as
"[:alpha:]" carries its own "]", so stopping at the first one cut a valid
filter like "(a[]),],b)" into fragments and rejected it.
|
||
|
Chris LuandGitHub
|
e482e67971 |
admin: accept a list of collections in the task collection filter (#10953)
The collection filter was parsed twice with two syntaxes: the master-side volume listing compiled the whole string as one regex, while EC encode and EC balance detection split it on commas and matched each entry as a wildcard. A volume had to pass both, so "collection-a,collection-b" matched nothing (no collection is named that), and the ALL_COLLECTIONS sentinel, which the master side skips, dropped every volume at the task side. Parse it once, in one place: a comma-separated list where an entry is a name with optional * and ? wildcards, or a regex when it carries regex syntax. A regex entry now has to match the whole name unless it anchors itself, so listing a collection no longer picks up its longer namesakes. |
||
 孙超andGitHub
|
c80664ec21 |
s3: propagate storage rule fsync to volume server uploads (#10906)
The storage rule's fsync decision was computed by the filer (detectStorageOption -> rule.Fsync) and applied on the filer's own HTTP write path, but was never carried onto the chunk uploads S3 issues: the AssignVolumeResponse had no fsync field, so the s3api client could not learn the decision, and the chunked upload URL was hardcoded without it. Every S3 write to a path with fsync configured went to the volume server as a non-fsync write. Carry the decision through the assign response: - filer.proto: AssignVolumeResponse gains bool fsync, filled from the storage option the assign resolved. - operation.AssignResult gains Fsync, so uploadChunk can append ?fsync=true to the volume server upload URL (single and replica fan-out paths). - The S3 PUT/UploadPart assignFunc, the S3 copy path, the admin file browser upload, and the Iceberg worker assign functions all forward the response field. Adds TestUploadReaderInChunksAppendsFsyncWhenAssigned. |
||
 Chris Lu
|
3b8931c2f6 | admin: address review feedback on the maintenance scanner fix | ||
 Junker der ProvinzandGitHub
|
8d2c0273bd |
admin: stop the maintenance scanner pinning itself to one scan per second after a transient failure (#10887)
* admin: honour persisted task configs when building the maintenance policy
buildPolicyFromTaskConfigs passed a literal nil to vacuum, erasure_coding
and balance LoadConfigFromPersistence. Those functions look for their
LoadXTaskPolicy() accessor via a type assertion, which a nil interface can
never satisfy, so every call fell through to NewDefaultConfig() and the
policy came back with the compiled-in defaults - Enabled: true among them.
A task disabled on disk was therefore still scheduled, and the only trace
was a glog.V(1) "Using default ... configuration" line.
Thread the real ConfigPersistence through instead. There are two copies of
this function: the one in weed/admin/dash builds config.Policy on the
normal admin startup path and can simply take cp as its receiver, and the
one in weed/admin/maintenance is the fallback used when the config carries
no policy yet, which now receives the store from NewMaintenanceManager.
weed/admin/dash already imports weed/admin/maintenance, so the maintenance
side has to keep the duck-typed interface{} parameter that the task
loaders already use rather than importing the concrete type back.
The store is only handed over when a data directory is configured: an
unconfigured one has nothing to read, and a typed nil pointer would pass
the loaders' type assertion and then panic on first use.
Fixes #10874
* admin: restore the maintenance scan cadence after an error backoff
scanLoop shortens its ticker to the error backoff delay after a failed
scan, but it decided whether to replace the ticker by comparing the
target interval against the configured scan interval instead of against
the interval the ticker was actually running at. Once the errors stopped,
getScanInterval returned the configured interval again, the comparison
came out false, and the ticker was left at the backoff delay - so a
single transient scan failure pinned the scanner to one scan per second
for the rest of the process lifetime. That is the ~1/second cadence in
issue #10874: 658 KB/s of "Cancelled N stale pending balance tasks
before re-detection" and 193k orphaned task files over two days.
Track the interval the ticker is running at and compare against that, so
both entering the backoff and returning to the normal cadence replace the
ticker.
While in here:
- defer ticker.Stop() bound the ticker that was current when the defer
was registered, so every replacement ticker leaked on return. Wrap it
in a closure.
- running was written by Start/Stop and read by all three background
loops without synchronisation. Guard it with the existing mutex, fold
the running check in triggerScanInternal into the lock it already
takes, and make Stop a no-op when not running so a second call cannot
close the stop channel twice.
Refs #10874
* admin: make the maintenance policy actually reach the task detectors
Loading the persisted task configs into the maintenance policy only
matters if something reads that policy, and nothing did.
MaintenanceIntegration pushes the policy into every registered detector
and scheduler through interface{ SetEnabled(bool) } and
interface{ SetMaxConcurrent(int) } type assertions. Every task registered
through base.RegisterTask is backed by base.GenericDetector and
base.GenericScheduler, and neither implemented either method, so all four
assertions failed silently for every task on every startup. The policy's
enabled flag reached nothing: ScanWithTaskDetectors gates on
detector.IsEnabled(), and the queue's policy lookups for max concurrent
and repeat interval are fallbacks that only fire when the scheduler
reports zero, which the generic scheduler never does.
Add the setters, delegating to the TaskConfig.SetEnabled the interface
already declares and to TaskDefinition.MaxConcurrent, which is what
GetMaxConcurrent returns.
Applying the policy required three more fixes, because with the
assertions working the policy could now do damage as well as good:
- IsTaskEnabled reports false for a task type the policy has no entry
for, so applying it unconditionally would have disabled every task the
policy does not list. Skip task types with no policy entry: no entry
means no opinion, not disabled.
- ec_balance was exactly such a task. It is registered like the other
three but had no entry in the policy builder and no accessor on
ConfigPersistence at all, so its configuration could never be
persisted. Add SaveEcBalanceTaskPolicy/LoadEcBalanceTaskPolicy, the
task_ec_balance.pb file, the SaveTaskPolicy dispatcher case, and the
policy entry.
- InitMaintenanceManager ran before loadTaskConfigurationsFromPersistence,
which replaces each task's whole config object, so the policy was
applied and then immediately thrown away. Swap the order. Both read the
same files, so the policy is now the last writer and stays
authoritative.
MaintenanceManager.UpdateConfig also updated the queue's and the
scanner's policy but not the integration's, so a policy changed at
runtime never reached the detectors. Add MaintenanceIntegration.SetPolicy
and call it.
While building the policy, stop hand-copying each task's fields and use
the task's own ToTaskPolicy(). The hand-written version was a second
definition of every task's policy and had already lost the erasure coding
preferred tags and replica placement and the balance IO rate limit. For
the same reason, the "nothing persisted yet" branches of
LoadVacuumTaskPolicy, LoadErasureCodingTaskPolicy and
LoadBalanceTaskPolicy now derive from each task's NewDefaultConfig()
instead of a third hand-written copy. Those copies had drifted, so with a
data directory but no config file on disk the effective defaults differed
from what the task and the admin UI schema both advertise:
vacuum scan interval 24h -> 2h
balance scan interval 6h -> 30m
balance imbalance 0.1 -> 0.2
erasure coding scan interval 168h -> 1h
erasure coding fullness 0.90 -> 0.95
erasure coding min volume 1024MB -> 30MB
Finally, weed/admin/dash and weed/admin/maintenance each carried a copy
of the policy builder and they had already diverged. Export the
maintenance one as BuildPolicyFromTaskConfigs and have dash call it.
Refs #10874
* worker: warn when a config store cannot supply a task's persisted config
LoadConfigFromPersistence logged a single glog.V(1) "Using default X
configuration" for every way of not loading anything, so the bug in
issue #10874 - a store handed in that the type assertion rejects, leaving
a task running on compiled-in defaults - looked exactly like the normal
"no data directory configured" case. The reporter had to read the source
to work out why their disabled task kept running, and asked for this
specifically.
Separate the cases. A non-nil store that does not provide the accessor is
always a wiring bug and is now logged at warning level, naming the type
and the missing method. A read error or a policy that will not apply is
also a warning. No persistence configured, and a store with nothing saved
yet, stay at V(1): those are normal.
Refs #10874
* admin: stop GetTaskPolicy panicking on a maintenance policy that is nil
GetTaskPolicy dereferenced its MaintenancePolicy argument to look at
TaskPolicies, so IsTaskEnabled, GetMaxConcurrent and GetRepeatInterval
all took the admin process down when handed a nil policy. A nil policy is
not a programming error here: MaintenanceConfig.Policy is unset until
something builds one, DefaultMaintenanceConfig returns a config with no
policy at all, and UpdateConfig installs whatever config it is given.
Found by calling IsTaskEnabled with the policy from a freshly defaulted
MaintenanceConfig.
Treat a nil policy as "no entry": no task enabled, the safe concurrency
default of 1, and a repeat interval of 0 so callers fall back to their
own default instead of reading DefaultRepeatIntervalSeconds off nil.
Also add the startup test this was found with. It walks the admin
server's startup sequence over a data directory that has balance saved as
disabled and checks the state that decides whether issue #10874 happens:
the balance detector reports disabled, vacuum stays enabled, and tasks
whose config was never saved keep their compiled-in default.
Refs #10874
* admin: document the synchronisation SetPolicy would need beyond startup
ConfigureTasksFromPolicy now really writes TaskDefinition.Config and
TaskDefinition.MaxConcurrent, which the scan loop reads through
detector.IsEnabled() with nothing synchronising the two. Every caller
runs during admin server startup today, before the scan loop exists, so
there is no live race - but the next caller has to add the locking, and
the same already applies to UpdateAllConfigs replacing the whole config
object. Write it down at the seam instead of leaving it to be
rediscovered.
Refs #10874
|
||
|
Chris LuandGitHub
|
6faa9d20e8 |
iceberg: stop compaction from corrupting dictionary-encoded columns (#10857)
* deps: upgrade parquet-go to v0.32.0 Iceberg compaction writes the merged file with the schema of its first input, encodings included. parquet-go before v0.31.0 took the deprecated PLAIN_DICTIONARY encoding that DuckDB writes at face value and encoded those pages as plain int32 indices, but the spec gives PLAIN_DICTIONARY the same bit-width-prefixed RLE layout as RLE_DICTIONARY. Every dictionary-encoded column in a compacted file then decoded onto a single dictionary entry, and anything past one page failed to decode at all. * iceberg: cover compaction of dictionary-encoded input The fixture is a DuckDB-written file, so it carries the PLAIN_DICTIONARY encoding a Go writer will not produce. * iceberg: tally whole rows in the dictionary merge test Counting each column on its own passes a merge that remaps names while leaving their cardinality intact. |
||
|
Chris LuandGitHub
|
8c7d714d5e |
Lance catalog, and a Rust plugin worker to maintain it (#10841)
* iceberg: skip tables the maintenance worker does not own A Lance dataset registered through the Lance namespace's Iceberg REST adapter arrives as an Iceberg table with a placeholder schema and table_type=lance, and keeps its fragments under data/ - the same subdirectory the orphan cleaner walks. Every fragment is unreferenced by the Iceberg metadata, so a maintenance pass deletes the dataset. Views share the entry shape and were only skipped because parsing their metadata happened to fail first. Gate the scan and the execution path on the entry actually being an Iceberg table. Maintenance is off by default, so this was latent rather than live. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * s3tables: let a table declare a format the catalog does not interpret CreateTable accepted ICEBERG and nothing else. A Lance table has no metadata file for the catalog to maintain - the entry records a name and the dataset root, and the client owns everything under it - so accept LANCE, and carry the declared format on the entry instead of hardcoding it back on the way out. ListTables now reports format and metadataLocation, so listing a catalog that holds both kinds takes one pass rather than a GetTable per row. AWS omits both fields; adding them is additive. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * s3tables: move the in-memory filer into its own package The Lance namespace tests need the same harness, and copying it would leave two of them to keep in step. Extracted as it was, plus the two fidelity gaps that only surface once a paginating caller uses it: ListEntries ignored startFromFileName and limit, so a caller that paginates re-read the first page until it hit its own cap and reported the same entry over and over, and GetFilerConfiguration was missing, which CreateTableBucket needs to resolve the buckets directory. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance: serve the Lance Namespace REST spec A second catalog surface beside the Iceberg one, over the same table buckets: the namespace and table metadata operations, the $-delimited identifier codec, the spec's numeric error model, the directory-catalog marker files, and storage_options vending through the STS path the Iceberg catalog already uses. Listens on -port.lance, 9101 by default, and inherits ARNs, policies and tags from the storage layer, so a Lance table needs no second permission model. Identifiers map bucket / namespace / table onto the three levels Lance clients already use, which is why there is no warehouse selector to invent. The data plane needs Lance format support that does not exist in Go and answers with the spec's Unsupported code rather than a bare 404. Two things it deliberately will not do: create a table bucket as a side effect of creating a namespace inside one, since a bucket carries its own policy and lifecycle, and resolve an Iceberg table's location for a Lance client, which would hand it a table another engine owns. The design note this follows is in design-lance-catalog.md, including the .lance directory suffix it proposed and this does not implement. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * mini: give the Lance port the same treatment as the Iceberg one The flag was registered but nothing else knew about it, so mini would start the server without reserving its port, waiting for it, or saying where it is. Adds it to the startup service list, the conflict resolver, the gRPC allocator's reserved set, the readiness wait, the stop reporting and the banner. The admin server still takes only the Iceberg port, because there is no Lance page for it to link to. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance: stop deregister and repoint from deleting the dataset Deregistering preserves data by definition, and this did the opposite: the catalog entry is the dataset directory, so DeleteTable took the files with it. Registering over an existing name had the same shape, destroying the dataset the name used to hold. Found by driving the running server rather than the in-memory filer, where both looked like success because the table did stop being listed. Deregistering is now a state on the entry - the marker file hides it, and declaring or registering the name again brings it back. Repointing a name at another dataset is an UpdateTable against the version token, so neither dataset loses files. Drop is left alone; it is the operation that does remove data. The storage endpoint now falls back to the advertised -ip where the Iceberg derivation gives up. An Iceberg client brings its own s3.endpoint and advertising the wrong one hijacks it, but storage_options is the only place a Lance client learns where the store is, and without it object_store quietly talks to real AWS. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * s3tables: refuse to create a table over one of another format Creating a table that already exists is idempotent, and that path returned the existing table without looking at its format. A Lance declare over an Iceberg table answered 200 and handed back a directory Iceberg owns, so the client would write its dataset on top. The view check immediately above it already guards the same class of collision. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * s3tables: let a table bucket hold a format other than Iceberg The S3 door validated every object written into a table bucket against Iceberg's file layout, so a Lance client could not write its dataset at all: it got 403 on data/*.lance, on _versions/, and on the _transactions/ directory it turned out to write as well. Table buckets were only neutral containers by intention; in practice they were Iceberg-shaped and enforced as such. The allowed set is now the union of what the supported formats write, because the validator runs where the table's format is not in hand. Underscore-prefixed directories are treated as belonging to the format, since enumerating them means guessing at the next one - _transactions is exactly the one this missed - and their contents are checked only for traversal. Iceberg writes none of them, so it loses nothing. Marker files at the table root are admitted too, which the namespace/table/dir/file shape had rejected as too shallow. Describe also honours the request-body spellings of with_table_uri, load_detailed_metadata and check_declared. The spec puts them in the query string, but real clients send both. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * design: record what the implementation found The table bucket being an Iceberg-shaped container, enforced at the S3 door, was the premise this design never questioned and the one that had to change before anything worked end to end. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * iceberg: prove the data loss the foreign-format guard prevents The guard landed with a unit test for the predicate and nothing showing what it saves. These seed what the Lance namespace's Iceberg REST adapter actually leaves behind - an Iceberg table with a placeholder schema and table_type=lance whose directory holds a Lance dataset - and assert both halves: orphan collection does flag the dataset's fragments, because the Iceberg metadata beside them references nothing, and the scan never reaches the table. An ordinary Iceberg table in the same shape is still scanned, so the guard is not just skipping everything. Confirmed against a running gateway first: our Iceberg catalog accepts the adapter's registration, and a real Lance client then writes a dataset into that table's location. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * s3tablestest: make the in-memory filer safe to race against Two gaps that only matter once a test drives concurrent writers, which is what an exclusive create has to be tested with: the entry map had no lock, and CreateEntry ignored O_EXCL entirely, so both writers of the same name would have won and the test would have passed while proving nothing. The BeforeUpdate hook runs before the lock is taken. Its whole purpose is to land a competing write in a handler's read-to-write window, and that write needs the lock the hook would otherwise be holding. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance: make the namespace an external manifest store Lance commits a version by writing _versions/{v}.manifest with put-if-not-exists. The S3 layer in front of this same filer evaluates If-None-Match by looking the entry up and then writing without a precondition, so two writers can both pass the check and one commit is lost. The filer itself has the primitive: CreateEntry with o_excl. Adds the four version operations a Lance client actually calls - create, list, describe and batch-delete - recording one entry per version under _lance_versions/, and advertises managed_versioning so the client routes its commits here. Reserving a version is the exclusive create, so exactly one of several racing writers wins and the rest rebase. Off by default, behind -lance.managedVersioning. Turning it on moves where a table's version history lives, and a reader that does not come through this namespace no longer sees all of it; that is the operator's call, not a default. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * design: record what managed versioning does and does not reach The first commit through a namespace-backed store works and is recorded the way the protocol specifies. Later commits do not, because lance 4.0.0 refuses put_if_exists on that path in its own code, so the feature is capped upstream rather than here. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * test: integration tests for the Lance namespace Everything this surface got wrong so far - a deregister that deleted the dataset, an S3 door that refused every Lance file, a version reservation that could not actually be exclusive - passed against an in-memory filer first. So these run against a live gateway, and where the claim is about data they check storage rather than visibility. Five Go tests on the shared harness: namespace and table lifecycle including that deregister keeps the bytes and drop removes them, that a Lance client cannot resolve or declare over an Iceberg table, that a Lance dataset's files get past the table-bucket layout guard while junk still does not, and that eight writers racing for one version produce exactly one winner. One Docker-gated test drives the real Lance client, which is the only way to check that the location and storage_options the namespace vends are between them enough to write and read a dataset. It overrides the endpoint with the container's view of the same gateway, because the shared harness binds a wildcard address and so vends none. The harness gains a Lance port and turns managed versioning on; the flag touches nothing outside that surface. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * s3tables: a directory with no namespace metadata is a missing namespace Three callers resolved a namespace by reading its metadata attribute and each tested only for a missing entry, so a directory that carried no metadata came back as an internal error saying "attribute not found". Creating a table under a namespace that does not exist answered 500. Collapses the three copies into one helper that reports both conditions as absent, which is what they are: a directory without namespace metadata is not a namespace. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * iceberg: stop reporting storage-layer refusals as server faults writeManagerError recognised a missing table bucket and sent everything else to 500, so a missing namespace, a duplicate name and a commit conflict all reached the client as InternalServerError with nothing to act on. Creating a table in a namespace that does not exist is the case that turned up: 500 where the spec wants 404 NoSuchNamespaceException. Maps the storage error types onto the exception names this package already uses, and keeps the existing bucket message, which explains how to select a table bucket. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * iceberg: skip a foreign-format table by name, not by failing to parse it A table the namespace created as LANCE carries no Iceberg metadata, so the worker skipped it only because the parse failed, and logged that as damaged metadata. The catalog records the format on the entry and this never read it. Reading it turns an accident into a decision, and separates a mixed catalog from a corrupt one in the logs. The property check beside it still covers the other shape: a real Iceberg table wearing table_type=lance, which is what the Lance namespace's Iceberg REST adapter writes. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * design: answer whether a Lance table needs maintenance It does, and index optimization has no Iceberg equivalent: rows written after an index was built are not covered by it, so a vector search quietly misses them. None of the three jobs can run in the Go worker, and there is no useful subset, because deciding what an old version still references means parsing Lance manifests. Version cleanup at least has an answer that needs nothing from us - Lance can enable it on the dataset itself. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * design: the Lance maintenance worker is a plugin worker, in Rust Framing it as a sidecar was wrong. plugin.proto already defines a language-agnostic gRPC contract for external maintenance workers, and "weed worker -admin=..." is the Go reference implementation of it from outside the admin process. seaweed-volume already compiles protos out of weed/pb with tonic_build, so a Lance worker is that build plus plugin.proto and the lance crate. Scheduling, retries, dedupe, progress and the admin settings page all come from the protocol: a worker that answers RequestConfigSchema with a descriptor gets its configuration form rendered without a line of Go. The data plane is the part that genuinely does need a process answering HTTP, and this had the two conflated. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * seaweed-worker: Rust plugin worker workspace, with Lance as the first one plugin.proto is language-agnostic and the Rust toolchain was already in the tree, so a Lance maintenance worker needs no new integration surface: core is the contract and nothing else, and a worker crate beside it supplies handlers and a binary. A second worker is a new member here rather than a fork of the protocol, which is why this is seaweed-worker and not seaweed-lance-worker. Verified against a running admin: it connects, is accepted, and admin prefetches descriptors for lance_compact, lance_optimize_indices and lance_cleanup_versions, so their settings pages render from the Rust side without a line of Go. The stream stays up across heartbeats. The job bodies are stubs that report failure. Doing the work means adding the lance crate and opening the dataset, and claiming success before that would be worse than saying so. Two things running it caught that reading the proto did not: the admin address has to be converted to the gRPC port the way pb.ServerToGrpcAddress does, or the dial fails as an h2 frame error; and the generated field names differ from the Go ones in several places, so JobCompleted carries success rather than a state enum. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance worker: implement compaction Detection lists tables from the namespace, opens each one, and proposes a job for any with more fragments than the policy allows; opening a dataset reads its manifest and not its data, so a sweep stays cheap. Execution re-resolves the table rather than trusting what detection saw - it may have been repointed, and the vended credentials expire - then compacts and reports the fragment counts either side. Verified against a live gateway: a twelve-fragment dataset became one fragment with all twelve rows intact. The test drives the handler directly and skips unless WEED_LANCE_NAMESPACE names a namespace, the way the Go integration tests skip without Docker. Running it turned up a gap the design had not: a gateway without STS vends no credentials at all, so the worker could not open anything and detection quietly proposed nothing. --access-key/--secret-key are the fallback, and whatever the namespace vends still wins over them. Two API assumptions did not survive contact either. Datasets open through DatasetBuilder::with_storage_options, not ReadParams, and lance 10's ObjectStoreParams has no storage_options field at all. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance worker: implement index optimization and version cleanup Index optimization is the job with no Iceberg equivalent: rows appended after an index was built are invisible to a search of it until this runs. Detection reads num_unindexed_rows from each index's statistics and proposes a table once more rows sit outside its indices than the budget allows; a table with no indices is skipped, which is different from one whose indices have fallen behind. Cleanup applies a retention window, refusing rather than silently dropping a tagged version, and leaving unverified files alone because they may belong to a commit still in flight. Both verified against a live gateway: 512 uncovered rows became 0, and a fourteen-version table lost its old ones. Each test now seeds what it needs, including building an IVF_PQ index and appending rows outside it. The first version of these depended on state a script had left, so the second run found the work already done and asserted nothing - a test that passes by doing nothing is worse than no test. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance: answer an empty catalog with an empty list, not null ListAllTables built its result from a nil slice, so a namespace holding no tables answered {"tables":null} on a field the spec marks required. A generated client may decode that differently from an empty list. Found running the namespace on a dev box, where the catalog was empty. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * admin: give Lance maintenance its own scheduler lane Lane assignment is a hardcoded map, so the three lance_* job types fell through to the default lane. That lane serialises its work under the cluster admin lock because volume management shares global state, which would queue a table's compaction behind volume balancing for no reason - Iceberg has its own lock-free lane for exactly this. Adds the lane, maps the three job types to it, and puts it in the sidebar beside Iceberg and Lifecycle. The lane routes were already generic, so only the nav was hand-written. The lane-coverage test spelled out the three known lanes, so a fourth failed it. It now checks against AllLanes(), which is the property it was reaching for and does not need editing next time. Found by connecting the Rust worker to a real admin: it registered fine and its job types were known, but they were filed under "default" and had no page. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance worker: log what detection saw "Detection proposed nothing" and "the worker could not read the table" look identical from the admin side, and the second is what a missing credential produces. One line per table separates them. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance worker: fix a leaked heartbeat and a silent reconnect loop spawn_heartbeat returned a handle to an empty task rather than the ticker it had just spawned, so aborting it aborted nothing and every reconnect left another heartbeat running against a dead channel. A stream that admin closes cleanly is not an error, but reconnecting in silence hides why. Two workers sharing an id evict each other forever and the log shows nothing but a login every five seconds - which is exactly how this presented on a dev box, and it took a look at the admin's own log to see it. The message now names the id to check. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance: a namespace cannot be created without its parent Storage keeps a namespace's parts flattened, so creating "a.b" with no "a" was accepted and left an intermediate that only existed inside a name. Listing derives child names by slicing those parts, so it reported "a", while describe and exists on "a" both answered 404 - a client walking the tree got a 404 on something the listing had just handed it. The spec asks for NamespaceNotFound when the parent is missing, which is also what keeps listing and describe telling the same story. Namespaces created through the S3 Tables API still bypass this, so listing keeps deriving intermediates rather than hiding whatever is already there. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * admin: say why a non-Iceberg table shows no schema The table pages read Iceberg metadata for schema and snapshots, and a Lance table has none, so both panels rendered "No schema available" - which reads as an empty table rather than a table this page cannot describe. The dataset behind the one that prompted this holds 1024 rows. The format is already on the entry and shown two rows above, so the empty states now use it: the catalog records where a LANCE table lives, not what is in it. Reading the schema for real needs Lance format code, which is the same wall as the data plane. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * seaweed-worker: run rustfmt over the workspace Committed the crates unformatted, so `cargo fmt --all --check` failed on files nothing had touched since. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * plugin: let a worker report what it saw about an object Admin cannot read a Lance table: it knows where the dataset lives and nothing else, so the details page had a location and two empty panels. The worker already opens every dataset during detection to decide whether it needs compacting, so it knows the schema, the row count and the fragment count at that moment. It just had no way to say so. Add a WorkerObservations body to the worker stream. Admin caches the last observation per object and serves it back, timestamped, for display; nothing schedules from it. The Lance compaction sweep reports what it opened, and the S3 Tables details page fills its schema panel from the cache when it has no metadata of its own, badged with when the worker looked and which worker it was. Nothing about this is Lance-specific past the reporting side, which is the point: any format admin cannot parse can describe itself the same way. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * design: record the observation channel Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * plugin: ask a worker for sample rows of a table admin cannot read Browse Data reads an Iceberg table's Parquet files directly, so it shows real rows. For a Lance table it showed "Table has no Iceberg metadata" and an empty grid, because there is no Go Lance reader and never will be one worth maintaining. The worker has the reader. Add RequestObjectPreview / ObjectPreviewResponse to the stream, mirroring the config-schema round trip that already exists, and give the Rust worker a PreviewProvider that scans the dataset and formats the rows with Arrow's own formatter, so a vector column reads as a vector. Admin picks the worker from the observation store: whichever one last described this table is the one that can read it. Unlike an observation the rows are not cached. They are the table's data rather than a description of it, and a copy sitting in admin would be both stale and nobody's business. The page fetches on load, bounded at 200 rows and a 15 second round trip, and drops the snapshot and data-file panels that only mean something for Iceberg. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * design: record the preview channel Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * test: disable the lance listener when two gateways share a host * test: keep AllocatePorts away from the lance default port * s3tables: let a table bucket declare the format it holds A bucket is a catalog, and a catalog serves one protocol. Format was recorded per table, so nothing could answer "where do I point a client at this bucket" without opening a table first, and an empty bucket had no answer at all. CreateTableBucket takes an optional format, stored with the rest of the bucket metadata and returned by Get and List. Empty means ICEBERG, which is what AWS S3 Tables serves and therefore what an SDK that has never heard of the field means. CreateTable refuses a table of another format, and CreateView refuses outright in a bucket that is not Iceberg, since a view is Iceberg metadata. Buckets that already exist carry no declaration and keep accepting anything, so nothing is migrated and nothing that worked stops working. The Lance namespace declares LANCE for the buckets it creates, which is what stops one of them being described to a client as an Iceberg catalog. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * admin: take the Lance port the way it takes the Iceberg one The UI cannot name the endpoint that serves a Lance bucket without it, and every format-aware page below needs to. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * admin: show which format a table bucket holds The bucket list printed an Iceberg endpoint for every bucket, including ones holding Lance datasets, where that endpoint serves nothing. It was the most visible place the UI assumed one format. The list gains a Format column and its endpoint column follows the bucket's declaration. The banner names both endpoints rather than asserting everything is Iceberg, and says so only for the servers that are actually running. Create Bucket picks a format with two cards rather than a dropdown, since what matters is not the name but which clients can read the result, and the endpoint under them updates as you choose so the operator leaves the modal knowing where to point one. A bucket from before the declaration existed shows "unset" in an outline badge, explained on hover. It is a fact about the bucket's age, not a fault, so nothing nags about it. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * admin: carry the bucket's format into the pages inside it Namespaces and tables are reached through a bucket, so both now say which catalog they belong to rather than making you go back up to find out. The tables list gains a Format column and a Rows column filled from what a worker last observed, since for a format admin cannot read that is the only row count there is; a table nothing has looked at shows a dash, not a zero. Create Table stops offering a choice the bucket has already made: in a declared bucket the format is fixed and says why, and only an undeclared one still offers both. Before this the select had exactly one option, hardcoded, which made a Lance table impossible to create from the UI at all. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * admin: let the table page speak the table's own format Partitions and Snapshot History are Iceberg's shape. Rendering them empty for a Lance table reads as a fault; a Lance table has neither, and says so by not showing them. In their place is a Versions panel, which is what that format calls its history, carrying the worker's timestamp so it is clear the numbers are a cached look rather than something read live. The breadcrumb carries the format badge, so the page names what it is looking at before you read a panel and wonder why it is empty. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * admin: show how to connect to either catalog, and group the two format workers The client examples on the buckets page were Iceberg's alone, so the one thing an operator wants after creating a Lance bucket - what to type to reach it - was not written down anywhere in the UI. Both formats now get a pair of snippets, and only for a server that is running. In the Workers menu, Iceberg moves below Lifecycle so it sits next to Lance: the two table-format workers together, the two cluster-wide ones above them. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * shell: create a table bucket of either format s3tables.bucket -create takes -format, so a Lance bucket can be made without going through the UI. The integration harness passes it too: its Lance tests were creating Iceberg buckets and getting away with it only because nothing checked. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * design: record that a bucket declares its format Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance: drop managed versioning; the store already orders commits The namespace offered itself as an external manifest store, so that a commit could reserve a version through a real put-if-not-exists. That was designed around a gateway that no longer exists: If-None-Match: * is reduced to a filer WriteCondition and evaluated at the object's owner under its per-path lock, or under the object write lock on the fallback path. Sixteen writers racing one fresh key get a single 200 and fifteen 412s, every time. Lance needs nothing else. commit_handler_from_url hands every s3:// dataset a ConditionalPutCommitHandler, which puts with PutMode::Create, which object_store sends as If-None-Match: *. So the feature solved a problem this store does not have, while moving a table's version history out of the dataset and into the catalog - and lance could not use it past the first commit anyway, since its own namespace-backed store answers "put_if_not_exists is not supported" to the second. The version operations answer Unsupported with the rest, managed_versioning is false, and the flag is gone. In place of the reserve-once test there is one that races eight writers at the manifest key through S3, which is the path a commit actually takes. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance worker: honour the version floor, the slot limits, and a shutdown Five findings from review, all of them things the worker claimed to do and did not. The version floor was checked when a cleanup job was proposed and ignored when it ran, so a table whose versions had aged past the retention window in between could be taken below the count the operator asked to keep. Execution now computes the floor itself and passes it as before_version; CleanupPolicy ANDs its clauses, so a version has to be both too old and below the floor to go. Both settings are clamped to the range the form offers, since Duration::hours panics on a large enough value and a negative min-versions wraps to a huge usize. Admin's shutdown was answered by returning from the stream, which the reconnect loop read as a healthy close and logged straight back in: the worker could not be stopped. serve_once now says which of the two happened. The advertised concurrency limits bounded nothing - every request spawned a task - and the heartbeat reported zero slots in use whatever was running. Both now go through semaphores sized from the limits, with the permits held for the life of the request and reported in the heartbeat. A namespace call had no timeout, so a gateway that accepted the connection and went quiet held a detection slot forever. And one table whose stats could not be read failed the whole sweep, losing the proposals for every table already scanned; it is now skipped and warned about, like a table that cannot be opened. The tests drove one shared catalog concurrently, which is why one of them asserted "no proposals at all" and passed by luck. They now take a lock and judge only their own tables. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * admin: fix the review findings on the format-aware pages The endpoint hint in Create Bucket built its HTML by concatenating the bucket name the operator is typing, so a name like <img onerror=...> ran in the admin origin as they typed it. It is built from DOM nodes now. A preview reply looked its channel up under the lock and then sent outside it, which Shutdown can close in between: a Gosched in that gap panics with "send on closed channel" every time. The send now happens under the lock. Observations were looked up by path alone, so a table dropped and remade in another format at the same path was described by the observation left behind. Lookups now have to agree on the format. Also: the Lance namespace caps a request body rather than reading whatever arrives; the details action no longer says "Iceberg" over a Lance table; mini stops advertising a catalog port when it is not running S3; a format whose server this cluster does not run cannot be picked in the modal or accepted by the API, since a bucket nothing can reach is not worth creating; and the unused catalogPortFor helper is gone. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance worker: let the control stream use mTLS The channel was hardcoded to http://, so off loopback the stream carried preview rows and execution commands in the clear - and a cluster with grpc TLS turned on would refuse the worker outright. --tls-ca, --tls-cert and --tls-key take the same certificates the Go worker reads from the [grpc.worker] section of security.toml, and must be given together: a CA on its own would quietly mean one-way TLS, which a mutual setup rejects anyway. Without them the stream stays plaintext, which is what the Go worker also does when nothing is configured. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance: answer null properties rather than an empty map The catalog does not keep a table's properties. Declare echoed the request's back and describe answered {}, both of which claim they were stored and are empty. Null says the catalog does not keep them, which is what the spec distinguishes and what is true here. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance worker: test the slot accounting The heartbeat reporting and the waiting are the two things the semaphores are for, and neither is observable from outside without catching a sweep mid-flight. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * test: fix the mixed-format catalog test, and name the binary it drives The integration suite passed locally and failed in CI on TestLanceRefusesIcebergTables. Both were right: CI builds the binary first, my tree had one from the day before, so locally the test drove a gateway with no format enforcement at all. The test itself no longer holds as written. It made a bucket, put an Iceberg table in it, and checked the Lance surface hid it - but a bucket that declares LANCE now refuses the Iceberg table outright. The invariant still matters from the other side, so it starts from an Iceberg bucket instead: Lance must not describe or list a table whose format it does not serve, and must refuse to declare one beside it. The harness now prints which weed binary it is about to run and when that was built. `make test` rebuilds first; a plain `go test` will happily drive a weeks-old binary and report a pass for code it never ran, which is exactly what happened here. Also make the row-limit conversion in the preview request explicitly bounded: CodeQL flagged the int-to-int32 conversion, and clamping by reassignment beforehand is not a form it recognises. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * lance: prove concurrent commits are kept, and preselect the only format on offer Two more from review. The commit test asserted that exactly one writer wins the conditional PUT, which is the mechanism, not the claim. The claim is that nothing is lost: the losers see the conflict, rebase and commit again. So there is now a test that has eight writers append to one dataset at once and counts the rows afterwards - all eight batches survive. That is also the sequence managed versioning could not finish, since its store refuses the second commit outright. And when Iceberg's endpoint is not running, the format picker offered two options with neither selected, so Create Bucket submitted no format at all, fell back to ICEBERG, and was refused by the guard added last round. Lance is preselected when it is the only format this cluster serves. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * Clamp the remaining worker settings, and bootstrap buckets in a served format Compaction and index optimization read their thresholds and cast straight to usize and u64, so a negative arrives as an enormous number and turns the threshold into "never": compaction and reindexing both go quiet with nothing to say. The cleanup job was fixed last round; these are the same bug. Clamped to the values that stay meaningful rather than to what the form offers - zero uncovered rows is a real setting, meaning reindex as soon as anything is not covered, so the floor there is zero and not the form's thousand. mini pre-creates the buckets named by -tableBucket, and did so without a format, which now means Iceberg. Started with the Iceberg endpoint off and the Lance one on, that left buckets nothing could reach and which refused every Lance table. It takes the format from the endpoint that is actually running, and creates nothing when neither is. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm * s3: allow-unordered is a listing parameter, not an unimplemented subresource The guard that stops a bucket GET with an unknown subresource from being answered with a listing does not know about allow-unordered, so it answers 501 NotImplemented - to a parameter the listing handlers already read and already validate against delimiter. This is why test_bucket_list_unordered and test_bucket_listv2_unordered fail in the Ceph s3-tests suite. They fail on master too; this is not a Lance change and can be taken on its own. Claude-Session: https://claude.ai/code/session_01Rkp1Mw5E89Jp6dzJFYiMrm |
||
|
Chris LuandGitHub
|
5d5ea63b3f |
Fix what the Go 1.26 language bump breaks (#10794)
* worker: log the balance move stage through a constant format string Go 1.26's printf analyzer now follows printf wrappers reached through an interface, so passing the stage straight to Logger.Info is a vet failure. * s3api: bracket the IPv6 host in the signature test URL A bare IPv6 literal is legal in a Host header but never in a URL. Go 1.26 stopped parsing it leniently, so carry the two forms separately and set r.Host to the value the client would actually have signed. * mini: bracket IPv6 addresses in the readiness probe URLs An IPv6-only host hands mini a bare literal, and %s:%d pasted it into a URL unbracketed. Under Go 1.26 that URL no longer parses, so waiting for the admin server never succeeds and mini refuses to start. |
||
|
Chris LuandGitHub
|
a80259d362 |
iceberg maintenance: fix the test build master merged broken (#10780)
#10774 gave buildTestMetadata its refs and age parameters while #10773 added a caller with the old arity. Each was green against a master that did not yet have the other, and the merge of both does not compile, so vet and the unit tests fail on master. |
||
|
Chris LuandGitHub
|
5f6dd4d3e5 |
iceberg maintenance: keep the snapshots that branches and tags pin (#10774)
* iceberg maintenance: keep the snapshots that branches and tags pin expireSnapshots only ever protected the current snapshot, so a snapshot held by a tag or a non-main branch was expired once it aged out of the retention window. iceberg-go's RemoveSnapshots drops any ref whose snapshot is gone without complaint, so the tag disappeared and the files behind it were deleted as unreferenced. Protect every ref target, and honour a branch's own min-snapshots-to-keep / max-snapshot-age-ms over the ancestors behind its head. Detection skips pinned snapshots for the same reason: proposing a job whose only outcome is a no-op keeps the worker busy forever. * iceberg maintenance: re-plan when a ref appears mid-commit, and stop proposing no-op expiry Three follow-ups from review of the ref-aware expiry: The commit guard only compared the table head, so a tag created between planning and commit could pin a snapshot the plan was about to expire. Re-check the refs against the metadata the commit actually reads. Detection now asks snapshotsToExpire what execution would remove instead of approximating with its own count-and-age rules. Expiry always requires a snapshot past the retention window, so a table over the quota whose snapshots are all young was being proposed for a job that could only no-op. The branch retention test could not tell "retained the whole lineage" from "honoured min-snapshots-to-keep", because the branch had exactly as many ancestors as the count. Give it one more, and cover max-snapshot-age-ms too. Both need snapshots genuinely older than a retention window, which iceberg-go will not accept at build time, so the fixture backdates the metadata after building it. * iceberg maintenance: fold the metadata test builders back into one buildTestMetadata, buildTestMetadataWithRefs, buildTestMetadataAged and buildTestMetadataNow were four names for one thing. Keep the original and give it the refs and age it needs. |
||
|
Chris LuandGitHub
|
eef6f3d1e6 |
s3tables: add the maintenance configuration APIs (#10773)
* s3tables: add the maintenance configuration APIs Stores the configuration verbatim as the wire shape under a new s3tables.maintenance extended attribute, so Get hands back what Put took and no translation layer can drift from the AWS model. Nothing reads the configuration yet. Put merges a single type into the stored map so configuring compaction does not drop snapshot management, and asserts the attribute's prior value so two concurrent Puts cannot silently clobber each other. * iceberg: apply the maintenance configuration in the worker The worker now reads the per-table and per-bucket maintenance configuration written by the control plane, so the wildcard plugin config is a default rather than the only setting a table can have. Table properties still win by default, since a table declaring its own layout is what every engine honours and the compactor has to agree with whoever writes the files. Clearing table_properties_override makes the maintenance configuration authoritative instead. Status is not part of that contest: a disabled type drops its operations and no property can re-enable them, so the operator's kill switch always holds. Manifest and delete-file rewrites have no AWS equivalent and ride with compaction. Detection reads both attributes from entries it already lists. * s3tables: report maintenance job status The worker records the outcome of each run in its own extended attribute, separate from the configuration so operator and worker writes do not contend, and GetTableMaintenanceJobStatus reads it back. Only the types a run touched are written, so a partial run cannot erase what an earlier one recorded. The reader fills in the rest: Disabled when the configuration switched a type off, Not_Yet_Run otherwise. Status is advisory, so a lost race is logged rather than failing a job whose work already committed. * s3tables: route the maintenance APIs over REST The five actions were only reachable by X-Amz-Target dispatch, which the AWS CLI and SDK do not use for this service. They address the operations by path, so the APIs were unreachable from any official client. * s3tables: fix the table bucket ARN field name GetTableBucketMaintenanceConfiguration emitted tableBucketArn where the wire field is tableBucketARN, as every other response in this package already spells it. Official SDK deserializers ignore the unknown key, so the required field came back unset. * s3tables: carry the compaction strategy through to the worker IcebergCompactionSettings modelled only targetFileSizeMB, so a request naming a strategy was accepted and then dropped on the way to storage. The worker now maps binpack and sort onto its own rewrite strategy and lets auto defer to the worker configuration. z-order is rejected rather than accepted and quietly binpacked. * s3tables: report bucket-level maintenance status GetTableMaintenanceJobStatus read only the table's configuration, so unreferenced file removal — which is configured on the bucket — reported Not_Yet_Run or a stale success after an operator disabled it. The merge helper now lives in this package and the worker shares it. * iceberg: delete orphans only after the non-current window AWS marks a file non-current once it has been unreferenced for unreferencedDays, then deletes it a further nonCurrentDays later. The cutoff was taken from unreferencedDays alone, so a 3/10 configuration hard-deleted on day three and threw away the ten day recovery window. remove_orphans deletes in one step rather than marking, so the cutoff is now the sum of the two. * s3tables: assert every attribute when rewriting an entry UpdateEntry writes the whole entry back from the snapshot the caller read, and its precondition only covers the keys the caller names. Both maintenance writers named one key, so a job status write could revert a maintenance configuration an operator had just disabled, turning an advisory write into a silent re-enable. Both now assert the entry's full attribute set, including the target key when absent so a concurrent create also fails the precondition. * s3tables: assert absent attributes when rewriting an entry The precondition covered the attributes present when the writer read the entry, so an attribute created between that read and the write was absent from it. A first-time PutTableMaintenanceConfiguration disabling a type therefore lands, passes the per-key checks, and is then deleted by the stale whole-entry write. Every attribute this package stores is now asserted, absent ones included. The metadata commit and planning index writers rewrite the same entries and had the same exposure, so both use the shared snapshot too. * iceberg: implement the auto compaction strategy auto was accepted, stored and read back, but left the worker on its own default, so a sorted table configured as auto was compacted with binpack. AWS defines auto as sorting tables that declare a sort order and bin-packing the rest. That needs the table metadata, so the choice is made where the rewrite plan is resolved: an unsorted table falls back to binpack rather than failing the way an explicit sort request does. * s3tables: validate the maintenance setting ranges PUT accepted zero, negative and oversized values for every numeric setting. The worker then ignores a non-positive value and saturates an oversized one, so the configuration read back was not the one that ran. AWS bounds all five to 1..2147483647, which is now enforced. The fields are pointers so an explicit zero is distinguishable from an omitted one and can be rejected rather than silently ignored. * s3tables: give every entry writer the same compare-and-swap updateExtendedAttribute asserted the entry's attributes, but the helpers behind the metadata, policy and tag handlers still wrote the whole entry unconditionally. Any of them could land on a stale snapshot and delete a maintenance configuration an operator had just written. They all share one read-modify-write loop now, so the precondition and the bounded retry apply wherever an entry is rewritten. * s3tables: move the maintenance configuration with a renamed table RenameTable carried the metadata, version, policy and tags to the new name but left the maintenance configuration and job status behind. A table with snapshot management disabled came back enabled under its new name, and the stale configuration stayed on the old name where a table created there would inherit it. The decoupled-delete cleanup left the same two attributes behind. * s3tables: accept every AWS partition in ARNs The route regexes and the ARN patterns both hardcoded arn:aws, so valid aws-cn and aws-us-gov ARNs never reached a handler. The router now shares the partition-tolerant prefix with the parser, and a generated ARN uses the partition its region belongs to so it parses back. * s3tables: generate ARNs in the region's partition The handler's own ARN generators still formatted arn:aws directly rather than going through the partition-aware builder, so a China or GovCloud deployment routed the request but then returned a commercial ARN and matched IAM policies against it. The round-trip test missed this because parsing accepts any partition, so it now asserts the prefix the region implies. * s3tables: complete the ARN partition table aws-iso-e, aws-iso-f and aws-eusc were missing, so eu-isoe-*, us-isof-* and eusc-* regions fell through to the commercial partition. * s3tables: do not let a rename swallow a concurrent maintenance write Rename copied the source attributes early and cleared the source at the end, so a Put landing in between missed the copy to the destination and was then deleted by the cleanup. It succeeded and vanished. The cleanup now clears the source only while it still holds exactly what was copied, and returns a conflict otherwise. Put checks the catalog identity inside the same conditional mutation, so it also cannot write to a name that a rename or delete has already soft-deleted. |
||
|
Chris LuandGitHub
|
a1d3fe236f |
iceberg: let table properties override the worker config (#10772)
* iceberg: carry snapshot retention in milliseconds Config stored retention as hours, so any sub-hour value would have to be truncated to 0 and then clamped back up to the 168 hour default. Keep the plugin config key in hours and convert once at parse time. * iceberg: let table properties override the worker config Every other Iceberg implementation lets a table's own properties win over engine defaults; the worker ignored them entirely. A writer honouring write.target-file-size-bytes and a compactor rewriting to the plugin config's size would rewrite each other's output forever. Resolved once per job rather than per operation, so compaction committing new metadata mid-job cannot change the settings underneath it. * iceberg: clamp the orphan cutoff so it cannot overflow collectOrphanCandidates converts the cutoff to a time.Duration. Past roughly 2.5 million hours that multiplication wraps negative, putting the cutoff in the future so every file walked looks like an orphan and gets deleted, including data a concurrent writer has not yet committed. Reachable today through orphan_older_than_hours. |
||
|
Chris LuandGitHub
|
944d967502 |
refactor: extract EC orchestration into a shared weed/ec package (#10760)
* shell: move ErrorWaitGroup to weed/util * shell: remove unused CandidateEcNode and EcRack types * ec: extract EC orchestration logic from weed/shell into weed/ec Move the EC node/topology model, balance engine, encode pipeline, decode pipeline, and rebuild engine into a new weed/ec package so shell commands and maintenance workers can share the logic. Shell commands keep flag parsing and delegate through a small ec.Env (dial option, topology fetch, volume locations, lock check). Tests move along with the code. * shell: remove unused proportional-rebalance type stubs * ec: move scrub, replication check, and shard unmount engines into weed/ec * worker: share the EC generation-aware shard counter from weed/ec * ec: gofmt * shell: drop EC aliases with no remaining callers * ec: guard a missing topology hook and nil disk entries in topology helpers * ec: drop trailing newlines from decode error strings * ec: re-check the shell lock before applying shard unmounts * shell: trim -node entries in ec.scrub |
||
|
Chris LuandGitHub
|
4f50c5b0d4 |
feat: throughput limits for replicate, EC shard, and worker-driven moves (#10749)
* feat: throughput limits for replicate, EC shard, and worker-driven moves VolumeCopy was the only rate-limitable transfer; EC shard copies, replica creation, and worker-driven moves all ran at whatever the receiving server's maintenance rate allowed, with no per-operation control. - proto: VolumeEcShardsCopyRequest and the balance / ec_balance task params and configs gain io_byte_per_second; 0 keeps today's behavior (the volume server's own maintenance rate governs). - volume server: VolumeEcShardsCopy throttles with one WriteThrottler per request, shared across the shard, .ecx, .ecj, .vif, and .ecsum copies so the limit caps the transfer as a whole - the same shape as VolumeCopy. - volume_move: ReplicateVolume accepts the limit; EcMoveOptions carries it through MoveEcShards/CopyAndMountEcShards into the copy request, with fake-client tests asserting propagation. - shell: ec.balance gains -ioBytePerSecond; volume.tier.move's replication top-up honors the command's existing -ioBytePerSecond instead of running unthrottled. - worker: balance and ec_balance configs gain io_byte_per_second (surfaced in the admin config schema), carried through detection and plugin job parameters into task params and handed to the shared mover; batch balance jobs inherit the limit from their detection results. The limit is per copy stream, so maxParallelization multiplies the aggregate ceiling. * worker plugins: expose io_byte_per_second in the plugin config and derive it The plugin-driven detection path derives its task Config from the plugin configuration values, and both balance and ec_balance left IoBytePerSecond at zero there - a configured limit silently reverted to the server maintenance rate. Both derive functions now read the field (clamped at zero), and the plugin descriptors expose it with defaults so the configuration form carries it. |
||
|
Chris LuandGitHub
|
4500bdf88e |
iceberg: accept lowercase parquet file format when planning compaction (#10751)
* iceberg: accept lowercase parquet file format when planning compaction * iceberg: expect absolute added-file paths in compaction integration test |
||
|
Chris LuandGitHub
|
8714f42abf |
erasure_coding: share the EC shard teardown primitive (#10740)
The unmount+full-teardown of EC shards was duplicated: the plugin-worker EC task had unmountAndDeleteEcShards and the shell had unmountAndDeleteEcShardsQuiet, byte-identical apart from a fence parameter and a sentinel error. That duplication is how the teardown fence semantics drifted between the two paths. Distribute, mount and verify already live in weed/storage/erasure_coding and are shared by both callers; move the teardown there too, as UnmountAndDeleteEcShards plus the shared ErrFullTeardownNotAcked sentinel. Both paths now call the one function, so the fence semantics cannot diverge again. The shell keeps a thin type-converting wrapper and aliases the sentinel; behavior is unchanged. |
||
|
Chris LuandGitHub
|
6408f32232 |
EC worker: clear stale/interrupted shards at task start and on failure (#10738)
* EC worker: clear stale/interrupted shards at task start and on failure
The EC encode task cleared stale shards from a prior interrupted encode only
at 55% progress (after mark-readonly, copy, and generate), and used a
generation-fenced teardown. Two gaps left orphan shards behind:
- a retried encode's prior attempt carries the same admin-issued encodeTsNs,
and the server's teardown fence preserves same-or-newer generations, so the
prior attempt's shards were never cleared;
- shards left by an interrupted distribute often have an unreadable .vif
generation (the sidecar never landed), which the fence also preserves.
Both survive the next volume-server restart as orphans and make detection
refuse the volume (Manual intervention required).
Move the cleanup to a Step 0 preflight that runs before any destructive step,
and switch it to the server's blanket (generation-independent) teardown -- the
same wipe the shell ec.encode pre-cleanup uses. The admin dedupe key already
prevents a concurrent newer encode of the volume, and the blanket path aborts
rather than clobber a live newer mount.
Add rollbackDistribute: a failure after distribute begins but before verify
commits the EC copy now tears down the shards it wrote and restores the sources
to writable, so a terminally-failed encode (a single-attempt job, or the last
of a retry series, which has no successor preflight) leaves nothing behind.
The preflight also rejects a plan with no targets or no source before marking
the source readonly.
* EC worker: reject malformed targets and keep source readonly on incomplete teardown
Address review feedback:
- ensureCleanEcStart only rejected an empty target slice; a target with an
empty Node (or no shard ids) passed the length check, was then silently
skipped by cleanupStaleEcShards, and let Execute mark the source readonly
with nothing to distribute to. Validate each target before the first
destructive step. Add regression cases.
- rollbackDistribute marked the source writable even when the shard teardown
returned an error, exposing a writable source beside stale (possibly mounted)
shards -- reads/writes could diverge and orphan cleanup will not remove a
writable source. On an incomplete teardown, leave the source readonly for the
next preflight or an operator to reconcile.
|
||
|
Chris LuandGitHub
|
0799084e98 |
refactor: share volume and EC shard move logic between shell and workers (#10727)
* operation: add shared volume_move package for volume and EC shard moves The shell commands (volume.move, volume.balance, ec.balance, tier moves) and the maintenance workers (balance, ec_balance) each carried their own copy of the move RPC sequences, and the copies had drifted: the worker verified the target before deleting the source but dropped the disk type and IO throttle; the shell passed those but deleted the source unverified. volume_move.Mover carries the merged sequences, keeping the stricter behavior from each side: - LiveMoveVolume: check-then-hard-freeze the source (VolumeStatus's IsReadOnly also covers low-disk and readonly-but-can-delete states, which still accept needle deletes), copy with disk type and IO throttle, tail, verify the target is not behind the source before the destructive source delete (a target that is ahead holds writes it accepted during the tail and the move commits to keep them), and restore the source's writability when a failure precedes the delete and this move did the freezing. Aborts clean up the incomplete target copy; a failed cleanup or an ambiguous source delete keeps the source readonly (ErrSourceKeptReadonly) so callers do not thaw a source next to a possibly-authoritative copy. With a readonly source, an existing or unknown-state target refuses the move outright: no client-side observation can prove such a copy is a stale remnant rather than the authoritative copy of an unfinished move. - MoveEcShards: copy with the .ecx/.ecj/.vif/.ecsum sidecars, mount, verify the target registered every shard before unmount+delete on the source, and reject same-server moves (the EC delete is server-wide). Server identity is the grpc endpoint (SameServer), so node:8080 and node:8080.18080 compare equal while test servers sharing a degenerate HTTP address stay distinct; addresses are validated non-fatally before dialing and before being embedded in copy/tail requests, since both the client dialer and the receiving server normalize them through a parser that aborts the process on a malformed port. The Rust volume server's codes.NotFound counts as a definitively absent probe answer alongside the Go server's plain-error code Unknown. All RPCs go through an injectable ClientFunc, so the sequences are unit tested against a fake volume server client: RPC order, request fields, and that verification failures keep the source intact. * shell, worker: delegate volume and EC shard moves to operation/volume_move LiveMoveVolume and the copy/tail/delete/mark-writable helpers become thin wrappers over the shared mover, keeping their signatures; the EC helpers keep their per-step output and delegate the RPCs. BalanceTask and ECBalanceTask keep their parameter validation, progress reporting, and guards (same-node cross-disk rejection, dedup keep-node verification, shard ids range-checked before the uint8 narrowing) and hand the RPC sequences to the mover. volume.tier.move skips its thaw-on-failure when the mover deliberately kept the source readonly, since reopening the replicas beside a possibly-authoritative target copy would fork the volume. The tail-failure tolerance moves inside the mover: a failed tail is tolerated only when the volume was already readonly before the move began, backstopped by a stability re-read across the idle window, so volume.balance's -skipTailError-by-readonly heuristic and tier-move's unconditional skip both become the same authoritative rule. * volume_move: keep the source readonly when a failed copy leaves a target of unknown origin A failed copy can leave a complete, mounted copy on the target (the server finishes after the client loses the stream). The abort probed the target only when its pre-copy state was known-absent; an unknown prior state skipped both the probe and the cleanup and then reopened the source - two writable replicas of one volume, diverging from the next write on. The abort now probes the target on every failed copy and restores the source only when the target provably holds nothing. A copy whose provenance cannot be proven (unknown prior state, a pre-existing replica, or an unreachable target) is never deleted, and the source stays readonly with ErrSourceKeptReadonly naming the recovery. * test: teach the plugin worker harness the shared move sequence The fake volume server lacked VolumeStatus, which the shared mover now issues before freezing the source, and the batch execution test's status-read accounting predates the pre-copy target probe and the verification reads. Mirrors the harness the enterprise tree already carries. |
||
|
Chris LuandGitHub
|
a7d5443125 |
ec: confirm a surviving copy before deleting a duplicate EC shard (#10719)
* ec: confirm a surviving copy before deleting a duplicate EC shard The dedup phase of EC balancing removes a shard it believes exists elsewhere. It copies nothing first, so the shard surviving on another node is the only thing that makes the delete safe -- and it took the plan's word for that. The plan is built from the master's topology, which can name a location that holds nothing: such a server answers "CopyFile not found ec volume id N" when something later tries to read the shard there. A shard listed on a phantom location and on a real one looks duplicated, so dedup deletes one of them. When it picks the real one the last copy is gone, and the job reports success -- the loss only surfaces later, as a rebuild that cannot assemble enough shards. The move phase already refuses to work on trust: it verifies the shard registered on the destination before removing the source. Dedup now holds to the same standard. The planner records which node it chose to keep, and both executors -- the worker task and the shell's ec.balance -- confirm that node really holds the shard before deleting. A keep node that cannot be queried is unknown rather than confirmed, and blocks the delete. Tests drive the destructive path against an in-process volume server that tracks what is actually on disk separately from what the plan claims, which is the distinction the bug turns on. Without the guard, two of them fail by deleting the only copy and returning success. * ec: check the collection and bound the wait when confirming a survivor Two gaps in the dedup survivor check. The inventory RPC is keyed by volume id alone, so a server holding the same number for a different collection answers "yes, I have that shard" to a question about this one. Accepting that deletes the last real copy on the strength of an unrelated volume. The response already carries the collection, so verify against it rather than widening the RPC. The shell path also queried on a background context, so a keep node that accepts the connection but never answers would hang the whole balance run instead of reporting that the survivor could not be confirmed. Bound it. The check moves into VerifyShardsOnServer next to the existing helper, shared by both executors, so the two paths cannot drift. |
||
|
Chris LuandGitHub
|
4b0d09683a |
iceberg: read manifest lists that omit the Avro format version (#10475)
* s3tables: read Iceberg manifest lists that omit the Avro format version The Iceberg spec pins the Avro header metadata of manifest files but says nothing about manifest lists, so writers disagree. Java and PyIceberg record "format-version"; DuckDB writes no header metadata at all. iceberg-go reads a missing entry as v1, so every v2 manifest listed in a DuckDB-written list is rejected with manifest file's 'format-version' metadata indicates version 2, but entry from manifest list indicates version 1 and, because v1 has no "content" field, delete manifests silently decode as data manifests. ReadManifestList derives the version from the record schema the writer embedded - v2 added "content" and the sequence numbers, v3 added "first_row_id" - and splices it into the header before handing the bytes to iceberg-go. Lists that already carry the entry, and input that is not a parseable Avro container, go through untouched. * iceberg: parse DuckDB-written manifest lists in maintenance and data preview Every manifest list read - the four maintenance operations and the admin table data preview - went straight to iceberg-go, so tables written by DuckDB failed detection and all of compact, remove_orphans, rewrite_manifests and expire_snapshots before they touched anything. Route them through s3tables.ReadManifestList, which recovers the format version the writer left out of the Avro header. This also restores the manifest content type on those tables: with the list read as v1 every delete manifest looked like a data manifest, which hid deletes from the compaction guard and made the preview report a table with position deletes as having none. |
||
 Lars LehtonenandGitHub
|
b3f35ed338 | chore(weed/worker): prune unused Registry (#10431) | ||
|
Chris LuandGitHub
|
7b3462be6a |
filer: stop an oversized metadata log flush from wedging the change feed (#10430)
* filer: write the metadata log in pieces a volume server will accept A single oversized metadata event grows the log buffer past the volume server's fileSizeLimitMB, and the flush of that buffer is then rejected forever: the retry loop has no exit, so the blob at the head of the queue blocks every later flush and the metadata feed stalls until restart. Split the flushed buffer into BufferSize pieces, on record boundaries where possible so each piece still decodes on its own, and retry each piece separately so a partial success is not replayed. Log files are already read as a chunk stream, with a whole-file fallback when a chunk does not decode standalone, so a record may cross a piece boundary. * log_buffer: let go of a window array grown for an oversized entry An entry larger than BufferSize grows the window array to 2*size+4, and window arrays cycle through SealBuffer rather than being freed. One such entry therefore leaves every later window carrying its size, and currentSnapshotView allocates a snapshot as wide as the array on each window, so a few KB of metadata keeps paying for it. Drop the array when SealBuffer hands it back. Growth is on demand, so the next oversized entry just reallocates. * iceberg maintenance: store merged data files as chunks, not inline saveFilerFile had no size threshold, so compaction wrote whole merged parquet files -- hundreds of MB -- as Entry.Content. That puts the parquet bytes verbatim in the filer store and sends them through the metadata change log again as one event. Keep manifests and metadata JSON inline, upload anything larger to volume servers in chunks, assigning through the filer so the path's storage rules apply. * filer: follow the file size limit the volume servers report The starting piece size is a constant, so a cluster whose -fileSizeLimitMB is set below it would reject every piece and wedge just the same. The rejection names the limit, so take it from there and re-cut the rest of the flush to fit. Piece the buffer one at a time rather than up front, since the size can change partway through a flush. |
||
|
Chris LuandGitHub
|
1e1b2bb2f9 |
iceberg maintenance: record file counters on the snapshots it commits (#10420)
A maintenance snapshot carried only its own labels — merged-files, delete-groups and friends — and no summary counters, so every engine that reads a table's size out of the current snapshot summary reported nothing for it: PyIceberg's inspect.snapshots, Trino's $snapshots and Spark's DESCRIBE all read total-records, total-data-files and total-files-size verbatim, and a table lost them the moment compaction touched it. Accumulate the files each operation adds and removes, and render them the way the spec defines: the added-*/removed-* counters from the files themselves, then the totals carried over from the parent snapshot. Carry a total only when the parent recorded it. Iceberg treats a missing total as zero, which turns a compaction replacing two files with one into a negative total-data-files, or a table with millions of rows into total-records: 0. Leaving the field out lets a reader fall back to the manifests instead of believing a made-up number. Compaction also accounts for the delete files it consumes, so a run that folds every delete into the rewritten data reports them as removed. |
||
|
Chris LuandGitHub
|
b4b0346f95 |
iceberg maintenance: resolve table files from the recorded location (#10418)
The worker assumed every file of a table sits under its catalog path, so loadFileByIcebergPath stripped the scheme off a recorded location and joined the remainder onto /buckets/<bucket>/<ns>/<table>. A table the REST catalog placed elsewhere in the bucket — which is what a client gets whenever the catalog path is already occupied — then resolves to a doubled path: lookup /buckets/lake/source/t/lake/source/t-0cd81bca-.../metadata/snap-.avro so the very first manifest list read fails and the job fails again on every scan interval, indefinitely. Resolve absolute references (s3:// URIs and /buckets paths) from the bucket root and keep relative ones under the table's own directory; the bucket-relative form is now the canonical key everywhere references are compared. That directory comes from the metadata location the catalog stores, so reads, writes and deletes all land where the table's other files are instead of splitting it across two trees. References outside the table's bucket are rejected rather than silently misresolved. Rewritten position-delete files now name their data file by absolute URI, the way the table itself names it, instead of a path relative to the table. |
||
|
Chris LuandGitHub
|
5a54beac80 |
EC decode: read shards with the encode-time block layout (#10385)
* erasure_coding: WriteDatFile takes the encode-time dat size for the shard block layout * volume server: derive EC decode layout from the encode-time dat size, not the live extent * erasure_coding: test decode after tail deletions shrink the live extent below a large-block row * seaweed-volume: write_dat_file_from_shards takes the encode-time dat size for the shard block layout * seaweed-volume: derive EC decode layout from the encode-time dat size, not the live extent * seaweed-volume: test decode after tail deletions shrink the live extent below a large-block row * erasure_coding: reject decoding with no data shards * worker: record the encode-time dat size in the .vif * erasure_coding: fall back to the shard-derived layout only when the encode-time dat size is missing * erasure_coding: reject an ambiguous shard-derived block layout * seaweed-volume: fall back to the shard-derived layout only when the encode-time dat size is missing * seaweed-volume: reject an ambiguous shard-derived block layout |
||
|
Chris LuandGitHub
|
07c9e0db85 |
iceberg maintenance: write absolute s3:// locations into table metadata (#10363)
The maintenance operations built manifest, manifest-list, data-file and
metadata-log paths with a bare path.Join("metadata", ...), so a single
maintenance run wrote scheme-less relative paths into the new snapshot,
the metadata-log and the table xattr. SeaweedFS reads those back fine
because normalizeIcebergPath accepts both forms, but strict readers
resolve every location through S3FileIO and fail with "Invalid S3 URI,
cannot determine scheme", leaving the whole table unreadable after any
maintenance commit.
Add absoluteIcebergPath, the inverse of normalizeIcebergPath, and apply
it at every site that authors locations: rewrite_manifests, compact,
rewrite_position_delete_files, and the shared commit path. The base is
derived from the bucket and table path since that is where the worker
physically writes, matching the locations the REST catalog generates.
|
||
|
Chris LuandGitHub
|
fba01ae2dd |
worker: balance pre-move check compares addresses in http form (#10360)
The pre-move stale check compared the master's location urls (host:port) literally against the proposal's node addresses, which carry the grpc suffix (host:port.grpcPort). Every move failed as "stale move: volume no longer on source" even though the volume was still there. Normalize both sides through pb.ServerAddress before comparing. |
||
|
Chris LuandGitHub
|
9a7cfaf371 |
admin: stop holding the shell admin lock across whole scheduler batches (#10290)
* admin: let waiting shell clients win the admin lock The admin lock manager re-acquired the cluster shell lock immediately after releasing it, while a waiting weed shell client only polls the master once a second, so an operator could lose the race indefinitely. Leave the lock free for slightly more than one poll interval before re-acquiring, and once overlapping reference-counted holds have kept the lock continuously held for over a minute, make new admin-side acquires wait for a full release before piggybacking. * admin/plugin: take the shell lock per detection and per job, not per batch The default-lane scheduler acquired the cluster shell lock once and held it across the whole pass: every due job type's detection plus all of its dispatched jobs, each drained to completion. A manual weed shell operation sharing that lock could stall behind the batch for up to the extended execution window. Hold the lock only while it protects something: around each detection scan and around each dispatched job. Manual shell operations now wait for at most one in-flight job. The admin UI detect+execute path stops wrapping dispatch in an outer hold, since a nested acquire would deadlock against the lock manager's fairness window; detection and per-job dispatch take the lock themselves. * admin/plugin: stop extending the dispatch window to the largest job estimate When any proposal's estimated runtime exceeded the remaining JobTypeMaxRuntime, the whole dispatch context was replaced with a fresh one capped at eight hours, so a single balance backlog could hold the default lane (and with it erasure_coding and vacuum detection) for that long. Keep the dispatch window at JobTypeMaxRuntime and instead detach each started attempt onto its own estimated-runtime deadline. Large jobs still get their full time once started; jobs not yet started when the window closes are canceled and re-proposed by a later detection, so sibling job types get a turn every window. * worker/balance: re-check each planned move against the master before executing A balance plan is computed at detection time, but the admin lock is released between detection and execution, so a manual shell operation can rearrange the volume in the gap. The task's own guards catch a vanished source, but a target that gained a replica in the meantime would be silently overwritten by VolumeCopy and the source delete would then reduce the volume to a single copy. Before executing each move, ask the master for the volume's current locations (uncached) and skip the move if the volume has left the source or the target already holds a replica. Skipped moves fail with a stale-move error and the next detection replans them. Without master addresses in the cluster context the check is skipped, preserving the old behavior with older admins. * admin: block re-acquire while the final lock release is in flight Release dropped the manager mutex before calling ReleaseLock, so a concurrent Acquire could see hold count zero and call RequestLock while the locker still considered itself locked. That request no-ops, leaving a hold with no live master lease. Track the in-flight release and make Acquire wait for it. Also normalize a nil release function from lock manager implementations, and make the yield and fairness windows per-instance fields so tests stop mutating globals. |
||
|
Lars LehtonenandGitHub
|
3a0021e144 | fix(weed/worker/tasks/balance): dropped test error (#10291) | ||
|
Chris LuandGitHub
|
9550b830d0 |
worker: project the moved volume when gating on disk fullness (#10171)
The disk-fullness gate only rejected destinations already at/above the mark, so a server just under it could take a large volume and overshoot. Project the selected volume's bytes onto the candidate: if the move would cross the mark, drop that destination for the rest of the cycle and re-pick instead of overshooting. Also note the per-location capacity-summing assumption on the Rust heartbeat side, to match the Go store.go comment. |
||
|
Chris LuandGitHub
|
d02ee6d5df |
balance: share replica-placement logic between shell and worker (#10169)
The replica-placement rule (data-center/rack/same-node limits plus host anti-affinity) existed three times: the shell's satisfyReplicaPlacement/isGoodMove used by volume.balance, fix.replication, and tier.move, and a line-for-line port in the maintenance balance worker. Move the canonical logic into weed/topology/balancer on a shared Location type; the shell and worker keep thin adapters that convert their own location representation and call it. Behavior is unchanged (the shared IsGoodMove keeps the shell's reject-move-to-self guard, and all four replica test suites pass). |
||
|
Chris LuandGitHub
|
bea1357d38 |
ec: skip physically near-full disks when placing EC shards (#10167)
EC placement scored destinations purely by free EC shard slots (derived from maxVolumeCount) and shard counts, blind to real disk fullness — the same defect as volume balancing. A disk that is physically full but still shows free EC slots kept being chosen, and EC shard bytes are captured by statfs free space yet not by any slot accounting, so the slot math is exactly the metric that can't see EC fullness. Treat a disk at/above 90% physical usage as having zero free EC slots at snapshot-build time, so every existing freeSlots>0 placement predicate excludes it. Applied in all three snapshot builders (shell countFreeShardSlots, the shared ecbalancer FromActiveTopology, and the worker ec_balance buildBalancerTopology) via the shared balancer.DiskTooFullAfter gate. Servers not reporting disk bytes fall back to slot-only behavior. ec.rebuild recovery is left ungated so shard recovery can still complete onto fuller disks. |
||
|
Chris LuandGitHub
|
77bf2a3ab0 |
volume.balance: gate on real physical disk usage (fixes #10160) (#10162)
* shell: add volume.balance -byDiskUsage to balance by actual data The default balancer ranks servers by slot density, dividing used volumes by MaxVolumeCount. When MaxVolumeCount is configured higher than the disk can hold, a physically near-full server looks nearly empty and gets picked as the move target, so balancing drains less-full servers onto an already-full one. -byDiskUsage ranks servers by the actual data they hold (sum of volume sizes) instead, so the fullest-by-data server is treated as full and balancing drains it. It assumes comparable disk sizes per disk type and still respects each server's free volume slots. Default behavior is unchanged. * plumb physical disk usage into topology, gate volume.balance on it Volume servers now report each disk's filesystem total/free bytes in the heartbeat, and the master stores them in DiskInfo. volume.balance uses them to skip any move target whose disk is already near full (-maxDiskUsagePercent, default 90), so an over-configured maxVolumeCount can no longer make a physically full server look empty and get drained onto. The gate judges each server against its own disk, so heterogeneous disk sizes are fine; servers that do not report bytes fall back to slot-only behavior. Rust seaweed-volume mirrors the heartbeat reporting. * admin: report real physical disk capacity when volume servers provide it The dashboard estimated server capacity as maxVolumeCount * volumeSizeLimit, which overstates it when maxVolumeCount is set higher than the disk holds. Prefer the filesystem capacity now reported per disk, falling back to the estimate for servers that do not report it. * worker: gate automatic balance on physical disk fullness too The maintenance balance worker selects the least slot-utilized server as the move destination, so an over-configured maxVolumeCount makes a physically full server look empty and get drained onto — the same defect as the shell command. Now that DiskInfo carries real disk bytes, skip any destination whose disk is at/above 90% used (per server, against its own disk); a full server can still be a source. When every candidate destination is full, create no tasks. Servers that do not report disk bytes are not gated. * balance: share the physical-disk-fullness gate between shell and worker The shell volume.balance command and the maintenance balance worker each grew their own copy of the disk-fullness gate (targetDiskTooFull / destinationDiskTooFull) and a maxDiskUsagePercent=90 constant. Pull both into weed/topology/balancer (DiskTooFullAfter + DefaultMaxDiskUsagePercent) so the policy has one home and the two balancers can't drift. * balance: harden the physical-disk gate Guard against a nil DiskInfo in the byte/slot lookups. Let a zero disk-capacity report clear previously stored bytes (0 means "not reported" for bytes, unlike maxVolumeCount), so a server that stops reporting falls back to slot-only instead of trusting stale capacity. In the worker, charge each planned move's bytes to its destination within a detection cycle so the gate sees a target fill up rather than only its heartbeat-time free space. Note the per-location capacity summing assumes one location per filesystem (the used ratio the gate relies on stays correct regardless; absolute capacity can over-report). |
||
|
Chris LuandGitHub
|
b0bad761ff |
worker: don't leak task goroutines on forced shutdown (#10062)
* worker: don't leak task goroutines on forced shutdown Stop() drains in-flight tasks for 30s, then terminates the manager loop. A task still running past that deadline later reports completion through w.cmds - getAdmin in completeTask, the ActionIncTask* send, removeTask - but with the loop gone and cmds unbuffered, those sends and the response reads behind getAdmin/getTaskLoad block forever, leaking the goroutine. Close a done channel when the loop exits and route the task-goroutine sends through it so they abort and return zero values instead of blocking. getAdmin can now return nil mid-shutdown, so collapse its double-call sites to a single nil-checked call to avoid a deref. * worker: abort remaining manager-loop sends after shutdown Extend the post-shutdown abort to the sends that still blocked: Stop()'s own ActionStop (so a second Stop, e.g. an admin-shutdown timer racing an explicit one, doesn't hang), setTask, and handleTaskCancellation. Route them through w.done so they return instead of blocking when the loop is gone. Stop is now idempotent. |
||
 
|
42ccfc0763 |
refactor: 将fmt.Errorf中的%v替换为%w以保留错误链 (#10050)
替换了多个文件中的错误格式化方式，使用%w包裹原始错误， 保留完整的错误调用链以提升调试时的错误追踪能力。 Co-authored-by: guant <guant@chinaunicom.cn> |
||
 
|
55a54574af |
fix: use %w instead of %v in fmt.Errorf to preserve error chain (#10047)
In ec_task.go, 23 fmt.Errorf calls used %v verb to wrap errors, breaking the error chain introduced in Go 1.13. This prevents callers from using errors.Is() and errors.As() to inspect the underlying error type. Changed all fmt.Errorf calls from %v to %w to properly wrap errors, preserving the error chain for upstream callers. Note: glog.* logging calls and fmt.Sprintf calls intentionally keep %v as they are not error wrapping contexts. Co-authored-by: 吴奇臻 <wuqizhen@cmict.chinamobile.com> |
||
  |
36f2ddcaea |
build(deps): bump github.com/apache/iceberg-go from 0.5.0 to 0.6.0 (#10038)
* build(deps): bump github.com/apache/iceberg-go from 0.5.0 to 0.6.0 Bumps [github.com/apache/iceberg-go](https://github.com/apache/iceberg-go) from 0.5.0 to 0.6.0. - [Release notes](https://github.com/apache/iceberg-go/releases) - [Commits](https://github.com/apache/iceberg-go/compare/v0.5.0...v0.6.0) --- updated-dependencies: - dependency-name: github.com/apache/iceberg-go dependency-version: 0.6.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> * iceberg: adapt worker to iceberg-go 0.6.0 API Fields() now yields iter.Seq2 (index, value); SortField.SourceID and PartitionField.SourceID are methods backed by SourceIDs; RemoveSnapshots takes a postCommit flag (false here, file cleanup runs through the filer). --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Chris Lu <chris.lu@gmail.com> |
||
|
Chris LuandGitHub
|
7e608c877a |
refactor(ec_balance): make the balance planner per-volume ratio-capable (#9960)
* refactor(ec_balance): make the balance planner per-volume ratio-capable Thread a per-volume EC ratio through the balance planner: Plan resolves each volume's data/parity from a new Options.VolumeRatio (falling back to the collection Ratio, then the build default, when it reports 0), and keys the global phase's ratio maps by volume instead of collection. The shell and worker balance paths build the per-volume lookup from each shard's heartbeat via the new ecbalancer.VolumeShardRatio. In OSS this is behavior-preserving: VolumeShardRatio returns 0 because the per-volume data_shards/parity_shards heartbeat fields are an enterprise feature, so every volume falls back to the collection ratio -- the existing standard-scheme behavior. The refactor keeps the shared planner in sync with the enterprise fork, which overrides VolumeShardRatio to classify and spread a mixed-ratio collection by each volume's own data/parity split. * perf(ec_balance): hoist the collection ratio out of the per-volume loop The collection ratio is constant for every volume in a collection, so resolve it once per collection instead of per volume; a custom Ratio func may do map lookups or locking. Addresses a review comment. |
||
|
Chris LuandGitHub
|
284796c7b6 |
fix(ec): fence stale-worker EC shard cleanup by encode generation (#9953)
* feat(ec): add encode_ts_ns to the EC task params, shard-unmount, and shard-delete RPCs The generation fence for stale EC-worker cleanup needs the encode generation on three messages: ErasureCodingTaskParams (admin issues it), VolumeEcShardsUnmountRequest, and VolumeEcShardsDeleteRequest (the worker carries it to the volume server). Additive fields only; 0 preserves the existing unfenced behavior. Mirror the two volume-server fields in the Rust volume server's proto copy. * feat(ec): issue the EC encode generation from the admin and carry it on the worker Stamp each EC proposal's encode_ts_ns from the admin's per-cycle DetectionSequence (a single-clock value) so generations are globally ordered even though detection runs on a rotating worker. The worker writes that generation into the distributed .vif and passes it on its shard unmount/delete RPCs; it falls back to a local timestamp for the .vif only on the unfenced legacy/shell path (keeping the read guard on). * fix(ec): fence the stale-worker EC shard unmount and teardown by generation A reaped-but-still-running EC worker's cleanupStaleEcShards issued a generation-blind unmount + full teardown that could unmount and then overwrite a newer run's live shards on a shared node. Both RPCs now carry the encode generation: the volume server unmounts/deletes a disk only when its .vif generation is strictly older than the request, and preserves a same-or-newer generation, a generation-0 (recovered or pre-upgrade) volume, and an unreadable .vif. Unload is per-disk, never node-wide. Request generation 0 keeps the blanket teardown for the shell pre-encode cleanup and pre-upgrade callers. Mirrored in the Rust volume server. * test(ec): cover the generation-fenced teardown and unmount End-to-end volume-server tests: a fenced FullTeardown wipes a strictly- older generation, preserves a newer one, preserves a generation-0 volume, and blanket-wipes on request generation 0; the gen-aware unmount preserves a same-or-newer mounted generation; and the .vif generation reader handles present/absent/no-config cases. * test(ec): pin the fenced .vif==teardown generation and the unreadable-.vif preserve A fenced run must stamp the admin generation verbatim into the .vif so it matches the generation sent on the teardown RPCs; add a regression test that sets the task generation and asserts the .vif carries it exactly. Also cover the present-but-unparseable .vif case (reads as generation 0, preserved) and correct the readEcGenerationTsNs docstring accordingly. * fix(ec): surface EC full-teardown filesystem errors in the Rust volume server remove_ec_volume_files(_full_teardown) discarded every fs::remove_file error, so a teardown that failed on permissions or a full disk still returned full_teardown_done=true and left stale artifacts to collide with the next encode. Return io::Result, ignore NotFound, propagate the first real error, and have the teardown RPC surface it -- matching the Go contract. The best-effort reconcile/load-cleanup callers keep ignoring it. * refactor(ec): reuse the EC volume lookup on unmount and short-circuit the gen read Address review: the Rust unmount fence reuses the ec_vol it already fetched instead of a second find_ec_volume; the Go .vif generation reader breaks out of the data/idx loop early when the two dirs are the same. |
||
|
Chris LuandGitHub
|
da243b9423 |
fix(ec): group orphan-source completeness by encode generation (topology encode_ts_ns) (#9952)
* feat(ec): carry the encode generation through the topology heartbeat Add encode_ts_ns (field 14) to VolumeEcShardInformationMessage and populate it from each EC volume's .vif identity. The volume server emits it on the full and incremental heartbeats; the master stores it on EcVolumeInfo and re-emits it via GetTopologyInfo, so the admin/worker layer can see which encode run produced each shard set. Field 14 avoids the enterprise fork's reserved 10-13. Mirror the proto field and both heartbeat emit sites in the Rust volume server. * fix(ec): group orphan-source shard completeness by encode generation countExistingEcShardsForVolume ORed EcIndexBits across every disk, so two interrupted encode runs whose shard sets overlap unioned into a false-complete set -- triggering the orphaned-source delete while no single generation was actually complete. Group shards by encode_ts_ns and return the largest single generation's count, so the trigger fires only when one run holds the full set. Shards from pre-upgrade servers (encode_ts_ns==0) form their own bucket. The heartbeat carries one encode_ts_ns per (volume, disk), so this separates generations on different disks; same-disk mixing is prevented upstream by the pre-encode artifact wipe and the cross-run read guard. * fix(ec): guard against a nil Ec shard info entry in the generation count Defensive: a manually-constructed or corrupted topology could carry a nil entry in EcShardInfos. Skip it rather than dereference. * fix(ec): carry the encode generation on the EC shard unmount delta The mount delta sets EncodeTsNs; the unmount deletion delta left it 0. Populate it from the Ec volume before unloading so both incremental deltas are consistent (the Rust volume server already does this via its snapshot diff). |
||
|
Chris LuandGitHub
|
240f82d6d2 |
fix(ec): persist EC source readonly mark and skip writable replicas on orphan cleanup (#9950)
* fix(ec): persist the EC source replica readonly mark markReplicasReadonly marked each regular replica readonly without persisting it, so a source-server restart during or after encoding silently reopened the volume to writes. Those writes are not in the EC shards, and the later orphan-source cleanup would then delete the replica, losing them. Send Persist:true so the mark survives a restart; rollbackReadonly still clears it via VolumeMarkWritable on a failed encode. * fix(ec): don't delete a writable source replica during orphan cleanup cleanupOrphanSourceReplicas issued VolumeDelete to every regular replica once the EC shard set looked complete, without checking the replica's current state. A replica that came back writable may hold writes the EC shards do not contain, so deleting it loses data. Re-probe each replica via VolumeStatus and skip any that is no longer readonly, logging a warning instead of deleting. |
||
|
Chris LuandGitHub
|
34f9b91d69 |
fix(storage): never let an empty .dat delete healthy distributed EC shards (#9930)
* fix(storage): never let an empty .dat delete healthy distributed EC shards A leftover empty .dat stub (a phantom from the pre-fix loader; zero needles) next to a distributed EC volume's local shards made startup classify the volume as an interrupted local encode: validateEcVolume requires >= dataShards local shards when a .dat is present, fails with the 1-2 shards a distributed volume keeps per disk, and the cleanup deletes those shards -- the only copies of that part of the volume. Repeated across restart waves this destroys enough shards cluster-wide to make the volume unrecoverable. Go: - loadExistingVolume: hoist the empty-stub sweep above the EC presence checks. Previously the .vif-next-to-.ecx guard returned before the sweep ever ran, so exactly the dangerous layout (stub + .ecx + local shards) kept its stub and then lost its shards in loadAllEcShards. - validateEcVolume / checkDatFileExists: treat a .dat <= a superblock (zero needles) as absent. An empty .dat cannot be the encode source, so it must never gate shard deletion; this also covers stubs without a .vif, which the sweep cannot prove are EC leftovers. Rust mirror (seaweed-volume): the same gate in validate_ec_volume and check_dat_file_exists (the Rust sweep already ran before validation); the volume-load skip keeps a plain existence check so fresh, needle-less volumes still load. Regression tests in Go and Rust reproduce the production layout (a zero-byte .dat beside .ecx/.ecj and two shards of a 10+4 volume, with and without a .vif) and fail without the fix with the shards deleted. * fix(ec): gate source volume deletion on a recoverable shard set After EC encode, the shell command and the (plugin) worker task refused to delete the source volume unless every shard was present, and aborted otherwise -- leaving the source .dat next to live shards, exactly the mixed state the startup cleanup mishandles. Replace the full-set requirement with a recoverability gate shared by both callers (RequireRecoverableShardSet): deleting a non-empty source .dat requires at least dataShards distinct shards cluster-wide. Below that the source is kept and the encode fails as before. A degraded but recoverable set (>= dataShards, < total) now proceeds with a warning instead of aborting: the missing shards can be rebuilt from the survivors, while keeping the source would preserve the dangerous mixed state. Empty stub replicas are still swept unguarded (OnlyEmpty) -- an empty .dat has nothing to lose. dataShards/totalShards stay parameters so enterprise custom EC ratios share the helper verbatim. * test(ec): use recoverable shard verification gate |
||
|
Chris LuandGitHub
|
37962e2445 |
admin: configure maintenance tasks via admin.toml (#9926)
* admin: configure maintenance tasks via admin.toml Maintenance task settings could only be edited in the admin UI and live under <dataDir>/conf, so they silently reverted to defaults whenever the data directory was recreated. An optional admin.toml now declares vacuum, balance, and erasure coding settings; keys set there are written through to the persisted task configs at every startup, overriding UI edits, so the configuration stays declarative. Generate an example with "weed scaffold -config=admin". * vacuum: round min volume age up to whole hours MinVolumeAgeSeconds was truncated by integer division when converted to the hour-granular protobuf field, so a sub-hour setting silently became 0 and disabled the age guard. * admin: split and normalize preferred_tags from admin.toml A comma-separated string, as set via environment variable, came through viper as a single slice element. Split on commas and reuse util.NormalizeTagList, matching the plugin config path. * scaffold: clarify admin.toml wording |
||
|
Chris LuandGitHub
|
3eb550a3f1 |
fix(tests): 32-bit build of EC e2e tests, type-check linux/386 in CI (#9922)
* fix(tests): keep EC e2e fid cookie arithmetic in uint32 The cookie constants 0x9490CA00 and 0x9500CA00 were added to the int loop variable before conversion, overflowing 32-bit int at compile time on linux/386 and linux/arm. Convert the loop variable instead so the addition stays in uint32. * fix(tests): pass s3client max backoff in milliseconds MaxBackoffDelay is documented as milliseconds and multiplied by 1e6 before use, but the example set it to 5s in nanoseconds, yielding an absurd backoff on 64-bit and a compile-time int overflow on 32-bit. * ci: type-check code and tests for linux/386 64-bit-only constant arithmetic keeps slipping into test files and breaking 32-bit downstream builds. Vet the whole root module under GOOS=linux GOARCH=386 so these fail in CI instead of after release. * fix(tests): convert s3client backoff to Duration before scaling The ms-to-ns multiplication ran in int, wrapping at runtime on 32-bit; scale by time.Millisecond after the Duration conversion instead. |
||
|
Chris LuandGitHub
|
79ac279fe1 |
fix(ec): don't mix EC shards from different encode runs (#9880)
* feat(ec): add encode_ts_ns to EC shard metadata and the shard read RPC EcShardConfig and VolumeEcShardReadRequest gain an int64 encode_ts_ns (encode time in unix nanos). It rides in .vif and the read request so a read can be scoped to the encode run that produced the index. * fix(ec): stamp each encode and reject cross-run shard reads Generate stamps EncodeTsNs into the volume's .vif. Reads carry it to the shard's owning volume (resolved together via FindEcVolumeWithShard, so a multi-disk server validates the disk that actually serves the bytes) and reject a shard from a different encode run, recovering from parity. A zero on either side (pre-upgrade volume) skips the guard. * fix(ec): stamp the encode identity on the worker-generated .vif The worker-local encode path now writes EncodeTsNs (and the resolved EC ratio) into the .vif, so the read guard is not silently off for volumes encoded by the maintenance worker. * fix(ec): wipe stale EC artifacts before re-encoding VolumeEcShardsGenerate evicts any in-memory EcVolume for the volume and removes its on-disk shard/index/sidecar files before writing fresh ones, so a retried encode never builds on a partial prior run and the unlink frees the inodes instead of leaving open fds serving old bytes. * fix(ec): unmount EC shards across all disks UnmountEcShards walked only the first disk holding the shard, leaving a duplicate copy mounted on a sibling disk (split-disk reconciled volumes) still serving and heartbeating. Traverse every disk and emit one deletion delta per disk. * fix(ec): delete orphan shards without a local .ecx deleteEcShardIdsForEachLocation gated shard-file removal on a local .ecx, so it could not clean an orphan .ecNN left by a failed copy on a disk with no index. Delete the requested shard files unconditionally; the index-file (.ecx/.ecj/.vif) routing stays gated as before. * fix(ec): clear stale EC shards cluster-wide before re-encoding ec.encode unmounts and deletes EC shards for the target volumes on every node before regenerating: fatal for the shards the topology reports (mounted leftovers), best-effort for the rest (a sweep that catches unmounted failed-copy orphans). A down node is a no-op. * fix(ec): don't nil EC fds on close so reads can't race eviction A reader resolves an EcVolume/shard under the lock then reads after it is released, so an eviction that nils ecxFile/ecdFile would race that read and panic. Close the fds without nilling the fields: the field is now write-once (no data race) and a concurrent read hits a closed fd, getting a clean error that the caller recovers from parity. * fix(ec): wipe stale EC artifacts on every disk and surface failures The pre-encode wipe only deleted beside the source volume, so a stale shard on a sibling disk survived and could be mounted against the new index at reconcile. Sweep every disk. Removal also ignored os.Remove errors, reporting a failed cleanup as success and letting a stale shard join the next generation; surface the first real failure (treating already-gone as success) from removeStaleEcArtifacts and the shard delete. * fix(ec): log when a local shard is skipped for a different encode run The cross-run guard returned errShardNotLocal, indistinguishable in logs from a genuinely-absent shard. Add a V(1) line naming both EncodeTsNs so operators can tell "wrong encode generation" from "shard not here". * fix(ec): surface metadata removal failures in the shard delete path deleteEcShardIdsForEachLocation still dropped os.Remove errors on the .ecx/.ecj/.vif/sidecar cleanup. A surviving stale .ecx is the orphan-index condition this path prevents, so route those through removeFileIfExists and return the first real failure instead of reporting cleanup as success. * fix(ec): fail orphan cleanup when a reachable node's delete fails The pre-encode orphan sweep swallowed every error for unreported (node, volume) pairs. That is only safe for an unreachable node, which cannot receive this encode's new generation. A reachable node whose delete genuinely failed (permission/IO) keeps an orphan shard that a later copy re-stamps with the new run's volume-level .vif identity, so the read guard would accept stale data. Surface those; stay best-effort only for unreachable nodes (gRPC Unavailable / no status). * fix(ec): guard ecjFile under its lock in the EC delete path EcVolume.Close nils ecjFile under ecjFileAccessLock; a delete that resolved its .ecx lookup before a concurrent eviction (the generate-time UnloadEcVolume) could then reach the journal append with a nil fd. Bail with a clear "volume closed" error under the lock instead. * fix(ec): reject an unstamped shard when the caller has an encode identity The read guard required both identities nonzero, so a current (stamped) caller accepted a holder with identity 0 and could be served a stale pre-upgrade shard. Reject when the caller is stamped and the holder differs (including unstamped); stay lenient only when the caller itself has no identity (pre-upgrade reader). A skipped shard recovers from parity. * fix(ec): full-teardown delete so cluster cleanup wipes a whole generation The pre-encode cluster sweep deleted only the listed canonical shards on remote nodes, leaving index/sidecar (and, on builds with versioned generations, those too) behind. Add a full_teardown flag to VolumeEcShardsDelete that evicts the volume and wipes every EC artifact for it on every disk via removeStaleEcArtifacts; the shell and worker pre-encode cleanup paths set it. Other delete callers (balance/decode/repair) are unchanged. * fix(ec): take ecjFileAccessLock before the nil-check in Sync and Close Sync and Close read ev.ecjFile before acquiring ecjFileAccessLock while Close nils it under the lock, a data race on the field. Take the lock first, then nil-check inside, in both. * fix(ec): acknowledge full_teardown so a pre-upgrade server can't fake success An old volume server silently ignores full_teardown and returns success for an ordinary delete, so the caller wrongly believes the generation was wiped and copies a fresh gen-0 onto an unwiped node. Echo full_teardown_done in the response; the worker destination cleanup fails when it is absent, and the shell cluster sweep fails for a reported (mounted) leftover while staying best-effort for an unreported node. encode_ts_ns stays an accepted transient (an old server just skips the new read guard, no regression). * fix(ec): fail the pre-encode sweep for any reachable node that can't ack teardown A reachable pre-upgrade server ignores full_teardown and returns success without wiping an orphan, which a later copy then folds into the new generation. Treat a missing full_teardown_done ack as fatal for every reachable node (best-effort only for a gRPC-unreachable one), not just for topology-reported pairs. * fix(ec): return the served shard identity and validate it client-side The encode identity was only enforced server-side, so a pre-upgrade server ignored the request field and served bytes unchecked. Echo the served shard's EncodeTsNs on every read response chunk and have the client reject a mismatch (including 0 from an old server), so the guard holds regardless of server version; a rejected read recovers from parity. * fix(ec): reject a short/empty remote shard read instead of serving zeros doReadRemoteEcShardInterval accepted an immediate EOF or a short stream and returned success with a partly zero-filled, unvalidated buffer (the server stamps the identity only on chunks that carry bytes). A non-deleted interval must arrive whole: require n == len(buf), exempting the is_deleted short-circuit (n=0), matching readLocalEcShardInterval's local check. A short read now fails so the caller recovers from parity. * test(ec): fake volume server echoes the full_teardown acknowledgement The worker now fails a teardown delete that isn't acknowledged (so a pre-upgrade server can't silently skip the wipe). The fake server's no-op VolumeEcShardsDelete returned an empty response, which the worker read as a skipped teardown and aborted the encode. Echo full_teardown_done. * feat(ec): mirror the encode-run identity guard + full_teardown into the Rust volume server The Go volume server stamps an encode-run identity (encode_ts_ns) into the .vif and rejects a read served from a shard of a different run; full_teardown wipes a whole generation and acknowledges it. The Rust volume server had none of it. Mirror the shared logic: load encode_ts_ns from the .vif onto the EcVolume, stamp it on every read response, and reject a request/response mismatch on both the server and the distributed-read client (recovering from parity); handle full_teardown by evicting the volume and wiping every EC artifact on each disk, echoing full_teardown_done so the caller can detect a server that ignored it. * fix(ec): remove a stale .vif on full teardown of a shard-only node A shard copy installs shards + .ecx before .vif, so an interrupted copy after a teardown could mount the new files under the previous run's identity / version / shard ratio / dat_file_size carried by the surviving .vif. Remove .vif during full teardown, gated on .idx absence so a source-volume holder keeps its live .vif. In Rust this lives in a teardown-only helper so the reconcile / load- fallback paths (which share the base removal) still preserve .vif. * fix(ec): treat a missing teardown ack as fatal, not as an unreachable node isNodeUnreachable returned true for any non-gRPC-status error, so a reachable pre-upgrade server's missing full_teardown_done ack (a plain error) was classified unreachable and the unreported pair was silently skipped. Classify only a real codes.Unavailable as unreachable, and wrap the missing ack in a sentinel the sweep treats as fatal regardless. A genuinely down node still surfaces as Unavailable from the RPC and stays best-effort. * fix(ec): reject a short shard read in the local EC needle reader read_ec_shard_needle ignored the byte count from shard.read_at and appended the whole pre-sized buffer, so a truncated shard's zero-filled tail passed the later length check and parsed as garbage. Require n == buf.len() per interval, erroring on a short read like the local interval reader already does. * fix(ec): probe reachability before skipping a node that returns Unavailable The pre-encode sweep skipped any node whose teardown delete returned codes.Unavailable, but a reachable volume server in maintenance mode also returns that code for the maintenance-gated delete, so its stale EC files were left behind on a node that can still receive the new generation. Confirm with a non-maintenance-gated empty-target Ping: skip only when the node fails the probe too (genuinely unreachable). * fix(ec): use try_exists for the teardown .vif .idx guard The teardown-only .vif removal gated on Path::exists(), which returns false on a permission/IO stat error, so a stat failure on a present .idx would read as a shard-only node and delete the live source volume's .vif. Gate on try_exists() == Ok(false) instead, preserving the sidecar on any stat error. * fix(ec): only skip a sweep node when a Ping confirms it is transport-down The pre-encode sweep skipped a node whenever its teardown delete and a liveness Ping both failed, but it treated ANY Ping error as down — an application-level Internal/ResourceExhausted, or Unimplemented from a pre-Ping server, left a reachable node's stale generation in place. Classify the Ping tri-state and skip only when it transport-fails with codes.Unavailable; a reachable or inconclusive node stays fatal. * fix(ec): exclude sweep-skipped nodes from the encode's rebalance The pre-encode sweep skips a genuinely-down node best-effort, but the rebalance then recollected the current topology — a node that recovered between the two could become a copy target and receive the new generation while still holding its stale, never-cleared shards. Have the sweep return the skipped set and exclude those nodes from the rebalance for this encode, so a node we could not clean cannot receive the new generation. Standalone ec.balance is unaffected. * fix(ec): re-sweep recovered nodes before generation so they aren't stranded A node skipped as down by the pre-encode sweep is excluded from the rebalance, but it can recover and become the generation host — mounting all shards locally, then being excluded from distribution. Union-only verification accepts all shards on one node and deletes the originals: a single point of failure. Re-sweep the skipped nodes just before generation; one whose teardown now succeeds leaves the skipped set and rebalances normally, while a node still down stays skipped. * fix(ec): abort the encode if a selected source is still skipped after re-sweep The re-sweep un-skips a recovered node, but the source was selected before it and a node can stay down through the re-sweep then recover just in time to be the generation host — mounting all shards locally while still excluded from the rebalance, which union-only verification accepts before deleting the originals. Abort the encode when a selected source remains skipped after the re-sweep. * fix(ec): batch delete returns retriable 503 when a volume became EC mid-batch If a volume is not EC at the batch-delete classification but is encoded to EC and its .dat deleted before the regular-volume mutation, the mutation returns an exact "not found" that the filer chunk-GC treats as completed, dropping the delete. Recheck EC presence under the mutation lock and return a retriable 503 with the "try again" token so the filer requeues it onto the EC path. * fix(ec): recheck EC state before the regular batch-delete mutation ec.encode mounts EC shards (copied from the .dat) before deleting the originals, so a volume can be EC while its .dat still exists. The batch delete only rechecked EC after a NotFound, so a successful regular-volume delete in that window wrote a tombstone to the soon-removed .dat — the delete was lost and the needle resurrected from the pre-tombstone shards. Recheck has_ec_volume under the write lock before delete_volume_needle and return a retriable 503 so the filer requeues onto the EC path. * fix(volume): make the metrics push test independent of test order test_push_metrics_once asserted the pushed body contains the request-counter family without ever touching the counter — a CounterVec with no children emits nothing, so the assertion only held when another test had already created a labelset in the shared registry. Create one in the test itself. |
||
|
Chris LuandGitHub
|
a549580e65 |
ec.balance: verify shard landed on destination before deleting the source (#9858)
* ec.balance: verify shard(s) landed on the destination before deleting source The EC balance task copied/mounted a shard to the destination and then immediately unmounted+deleted it from the source, reporting success as soon as the RPCs returned. A copy/mount can return OK while the shard isn't actually registered/loadable on the destination, so deleting the source then loses the shard (and the scanner re-issues the same move every cycle). Add a verification step (VolumeEcShardsInfo via VerifyShardsAcrossServers, the same check the EC encode task uses before deleting originals): if the destination doesn't report every moved shard, fail the task and keep the source so the move is retried instead of losing data. * address review: use comma-ok when reading destination shard inventory |
||
|
Chris LuandGitHub
|
f0d2a0d417 |
Treat co-located volume servers as one fault domain when balancing and allocating (#9854)
* admin/topology: carry the volume server address on DiskInfo The planning DiskInfo exposed only the node id, which can be an opaque label rather than ip:port. Record the address too so callers can resolve the physical machine a disk sits on. * ec.balance: spread a volume's shards across machines, not just nodes Volume servers sharing a host are one fault domain, but the within-rack spread treated them as independent nodes, so one box could end up holding more shards of a volume than EC can afford to lose. Add a machine (host) tier between rack and node: the within-rack pass spreads each volume across machines, and the global load phase no longer re-concentrates a volume onto a machine it already sits on. Host defaults to the node id, so clusters with one server per host are unchanged. * ec placement: prefer machines holding fewer of a volume's shards EC allocation and repair picked the least-loaded node in a rack with no regard for which physical machine it sits on, so a volume's shards could pile onto several servers of one box. Rank candidate nodes by their machine's shard count first, then the node's own. The machine is derived from the volume server address carried on DiskInfo, falling back to the node id, matching how the balancer resolves it. * volume.balance: don't move a replica onto a machine already holding one isGoodMove only rejected a move onto the same data node, so two replicas could land on two volume servers of one box and a single machine failure would lose both. Reject a target whose host already holds another replica of the volume. Best-effort: balancing simply skips and tries the next target. * volume allocation: spread same-rack replicas across machines PickNodesByWeight filled the same-rack replica picks by weight alone, so replicas could co-locate on one box. Prefer candidates on not-yet-used hosts, falling back when too few distinct machines exist. Data-center and rack tiers have no host, so their ordering is unchanged. * ec.balance: harden machine spread against re-concentration and capped machines Two cases where the machine-aware spread could still leave a volume badly placed: - The global load phase could move a shard of a volume onto a machine that already held it, raising that machine's count and undoing the within-rack spread (a 4/4/3/3 layout could become 3/5/3/3, past parity for 10+4). Limit the load-only fallback to same-machine moves, which leave a machine's count unchanged; cross-machine concentration is no longer allowed for load alone. - The within-rack spread chose a destination machine by free slots alone, so if that machine's only nodes were already at the SameRackCount cap it skipped the move instead of trying another machine. Require a machine to have a node that can actually take the shard before selecting it. * reduce comments across the machine-affinity change Trim narration down to the non-obvious why; one terse line where a block was overkill. * ec.balance: gate machine spread on fault-tolerance feasibility Spreading a volume evenly across machines only helps when there are enough that each can stay within EC's parity tolerance (numMachines >= ceil(total/parity)). With fewer -- or wildly unequal -- machines it can't make a machine loss survivable anyway, and forcing it fights capacity: e.g. a cluster of 12 volume servers on one host and 2 on another would have half of every volume crammed onto the 2-server box. So spread across machines only when it's achievable; otherwise fall back to per-node spread and let capacity/global balancing decide. The global load phase applies the same test: it protects a volume's machine spread (no cross-machine move that raises a machine's count past the source's) only where that spread is achievable, so heterogeneous clusters still level by fullness. * ec.balance worker: group servers by host when planning The worker built its planner topology without recording each server's host, so automated ec.balance treated ports on one machine as independent nodes and could concentrate a volume's shards on one physical box. Set the host from the volume server address, matching the shell path. * volume.balance worker: don't move a replica onto a machine holding one The worker compared only node ids, and the replica map dropped the server address, so it could move replicas onto different ports of one machine. Carry the host on ReplicaLocation (from the server address) and reject a target whose host already holds another replica of the volume. Best-effort, matching the shell. * ec.balance: judge machine-spread feasibility by the rack's shards The within-rack and global feasibility checks compared the whole volume's shard count against a rack's machine count, so a rack holding only part of a volume after cross-rack spreading -- e.g. 7 of a 10+4 volume across 2 machines -- was wrongly judged infeasible and fell back to node spread, which could pile 6 shards onto one host, past parity. Gate on the rack's own shard count of the volume instead. * ec.balance: spread a volume's shards across machines by combined count EC recovers from any loss within parity regardless of shard type, so what bounds a machine's exposure is its total shards of the volume, not data and parity separately. Spreading the two independently let each type's remainder land on the same machine -- ceil(d/M)+ceil(p/M) can exceed ceil(total/M), e.g. a 5/3 split where 4/4 was achievable, past parity. Balance the combined count in one pass; disk-level data/parity anti-affinity stays in pickBestDiskOnNode. * ec.balance: don't let the imbalance threshold skip an over-parity machine The within-rack spread gated on relative skew ((max-min)/avg > threshold), so a worker threshold of 0.5 skipped an exactly-50%-skewed layout like 5/4/3 for a 10+4 volume, leaving 5 shards -- past parity -- on one machine. The even cap (ceil(shards/groups)) is the real bound and the move loop already sheds only what exceeds it, so drop the threshold gate from the within-rack phase (machine and node): a balanced rack stays a no-op while any over-cap machine is always fixed. * ec.balance: keep the imbalance threshold for the node fallback Dropping the threshold from the whole within-rack phase made the node fallback too eager: it runs only when machine fault tolerance is unachievable, so it is cosmetic load distribution that should defer to the global utilization phase. Without the gate it would, for a one-server-per-host 6/4 split at threshold 0.5, schedule a count move that worsens utilization balance. Restore the threshold there; machine spreading keeps bypassing it, since that bound is durability, not cosmetic skew. |
||
 ahalaunandGitHub
|
3ce4e0dbdf | fix(s3/lifecycle): report success to admin via JobCompleted (#9787) | ||
|
Chris LuandGitHub
|
ca81c0c525 |
fix(ec): pass per-volume data-shard count to the parity-shard split (#9781)
* fix(ec): pass per-volume data-shard count to the parity-shard split ShardsInfo.DeleteParityShards/MinusParityShards looped ids 10..13, assuming the fixed 10+4 layout. For a non-default ratio this splits data vs parity wrong — a wide ratio (12+4, 16+6) drops real data ids >= 10, which breaks ec.decode. They now take a dataShards argument (<= 0 falls back to DataShardsCount) and clear ids dataShards..MaxShardCount. ec.decode threads the data-shard count from collectEcNodeShardsInfo to both split call sites, and admin LogicalSize passes DataShardsCount. Also: EC cleanup now sets an explicit per-disk storage impact (-len(ShardIds)) instead of falling back to the TotalShardsCount constant, so freed-capacity accounting matches the shards actually removed. OSS is always 10+4, so behavior is unchanged here; this keeps the split ratio-correct and the API aligned with the enterprise per-volume override. Adds parity-split ratio tests. * ec: clear parity shards in one locked pass Address review: DeleteParityShards looped si.Delete, taking the lock once per id. shards is sorted by Id and shardBits is a bitmap, so mask off the high bits and truncate the sorted slice at the first parity id (binary search) under a single lock. Preserves the dataShards<=0 -> DataShardsCount default. |
||
|
Chris LuandGitHub
|
9658f309d2 |
EC bitrot detection: per-shard checksum sidecars (#9761)
* ec: add EC bitrot checksum protobuf EcBitrotProtection/EcShardChecksums/ChecksumAlgorithm sidecar messages, copy_ecsum_file and unsafe_ignore_sidecar fields, and a CHECKSUM scrub mode. * ec: bitrot checksum sidecar format, validation, and per-volume load Per-shard CRC32C block checksums in an optional <base>.ecsum sidecar with a self-integrity header; validation, rolling builder, backfill primitive, and EcVolume load on mount + removal on destroy. * ec: capture per-shard checksums at encode; verify-and-exclude on rebuild WriteEcFilesWithContext returns the protection computed inline during encoding. generateMissingEcFiles verifies present inputs against the sidecar, excludes corrupt ones, regenerates in place, and re-verifies; fail-closed unless unsafe_ignore_sidecar, removing all generated outputs on failure. * ec: read-only checksum scrub with Reed-Solomon arbiter ChecksumScrub verifies each local shard against the sidecar and reconstructs flagged shards from the clean shards so stale-sidecar false positives are not reported. Wired to the gRPC CHECKSUM mode and ec.scrub -mode checksum. * ec: server-side bitrot sidecar write, copy, cleanup, and opportunistic backfill Write .ecsum at fresh encode; propagate it with copy_ecsum_file (tolerant); remove it on full delete and decode; rebuild honors unsafe_ignore_sidecar and opportunistically backfills a sidecar when all shards are reachable. * ec: volume server bitrot config flags -ec.bitrotChecksum (default on) and -ec.bitrotBlockSizeMB (default 16). * fix(ec_bitrot): bound -ec.bitrotBlockSizeMB before the int64 multiply Validate the MiB value is in [1, 1024] before multiplying by 1 MiB, so a huge flag value cannot overflow int64 and slip past the power-of-two check, and a block size cannot collapse a sidecar to a few oversized blocks. * fix(ec_bitrot): distribute the .ecsum sidecar from the worker encode path The worker EC encode wrote the generation-0 sidecar locally but never added it to shardFiles, so DistributeEcShards never shipped it and the distributed holders came up unprotected. Append it to shardFiles and map the ecsum shard type to its extension in the sender so it travels with the shards. * fix(ec_bitrot): remove orphaned sidecars when the generation is gone Gate sidecar removal on existingShardCount==0 alone rather than also requiring a stray .ecx. A sidecar whose shards have all been deleted is orphaned and must be removed even when no .ecx remains, or it leaks. .ecx/.ecj/.vif removal stays gated on hasEcxFile as before. * fix(ec_bitrot): do not fold checksum blocks scanned into TotalFiles ChecksumScrub's first return is blocks scanned, not files. Discard it so the scrub response's TotalFiles (a needle/file count) is not inflated by the block count for CHECKSUM mode. * test(ec_bitrot): clean up generated .ecsum sidecars in removeGeneratedFiles * fix(ec_bitrot): reject an oversized sidecar payload before the uint32 cast The header stores payload_len as a uint32; bound the payload before the conversion so a pathological manifest cannot truncate the length field and corrupt the sidecar. A real manifest is a few KB, so this never trips. * fix(ec_bitrot): cap -ec.bitrotBlockSizeMB at 64 MiB The block size becomes the per-shard scratch buffer the scrub/backfill path allocates, so an over-large value (e.g. 1 GiB) is a memory hazard per concurrent scrub worker. Lower the upper bound from 1024 to 64 MiB. * fix(ec_bitrot): add -ecUnsafeIgnoreSidecar to weed tool fix -ecx The -ecx recovery path reconstructs missing shards via RebuildEcFilesWithContext, which fails closed on a malformed/stale .ecsum. Without an override flag an operator could not complete the rebuild without manually deleting the sidecar. Expose -ecUnsafeIgnoreSidecar (default false) and thread it through. * fix(ec_bitrot): bound sidecar payload with a direct int constant; drop readFull Guard len(payload) against a plain int constant (1 GiB) before the allocation instead of a uint64 MaxUint32 compare, so the allocation-size value is provably bounded (clears the CodeQL overflow alert) and the math import is no longer needed. Inline os.File.ReadAt with io.EOF handling in verifyShardFileBlocks and remove the now-redundant readFull helper (os.File.ReadAt fills the slice or errors). * test(ec_bitrot): use slices.Contains instead of a hand-rolled containsU32 * refactor(ec): fold the EcFiles WithContext variants into the base functions RebuildEcFiles now takes the *ECContext directly (nil => derive from .vif as before) and WriteEcFiles takes it too (nil => default), removing the parallel RebuildEcFilesWithContext / WriteEcFilesWithContext names. Callers that had an explicit context drop the WithContext suffix; the default-context callers pass nil. No behavior change. * refactor(ec): pass BackgroundECContext instead of nil to Write/RebuildEcFiles Add a non-nil BackgroundECContext placeholder (analogous to context.Background()) and have callers with no specific layout pass it instead of a nil *ECContext. WriteEcFiles resolves a zero/background context to the default ratio and RebuildEcFiles resolves it from the .vif, so behavior is unchanged. * fix(ec_bitrot): make BackgroundECContext a func; RebuildEcFiles fails closed on bad .vif - BackgroundECContext is now a function returning a fresh *ECContext, so callers cannot mutate a shared singleton or race on it (and it mirrors context.Background, which is also a function). - RebuildEcFiles now propagates the MaybeLoadVolumeInfo error: a present-but- unreadable .vif fails closed instead of silently rebuilding with the default ratio (which would corrupt a custom-ratio volume). Pass an explicit ctx to override. |