14828 Commits

This Branch

This Branch

All Branches

Author	SHA1	Message	Date
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	2026-08-19 22:59:56 -07:00
Chris LuandGitHub	814ee75af4	s3: allow-unordered is a listing parameter, not an unimplemented subresource (#10846) ... 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	2026-08-19 20:09:23 -07:00
Chris LuandGitHub	e5edd8be3c	s3: place multipart part chunks by the destination object's storage rule (#10845) ... Multipart parts stage under /buckets/<bucket>/.uploads/<id>/, so the filer resolved filer.conf storage rules against that path when the gateway assigned volumes for them. A rule scoped to a key prefix - fs.configure -locationPrefix=/buckets/b/data/ -ttl=30d - then matched a small object but not the parts of a large one, so an object whose entry carried the rule's TTL had its bytes spread over TTL-less volumes. Assign part chunks against the destination object's filer path instead, the way the x-seaweedfs-destination header made the filer resolve it before the S3 write path moved off the filer proxy. Covers PutObjectPart and both UploadPartCopy paths. The part entry itself is still written under .uploads, so a read-only rule there still rejects it. The lifecycle XML Expiration.Days TTL keeps passing 0 for parts: that rule targets the user-visible object key and would start its clock before CompleteMultipartUpload.	2026-08-19 20:07:57 -07:00
Chris LuandGitHub	abd61de52c	S3: stamp the gateway's own uid/gid on PutObject and copy entries (#10844) ... * fix(s3): stamp the gateway's own ids on single-shot PutObject entries putToFiler builds the entry in the gateway now instead of proxying a PUT to the filer, and it hardcoded Uid/Gid 0 while every sibling write path stamps filer_pb.OS_UID/OS_GID. On a non-root deployment that leaves single-shot PUTs and multipart parts owned by root while directories and completed multipart objects keep the real ids, so a mount reader can list the tree but gets EACCES on every open once objects are not world-readable. * fix(s3): stamp mode and ownership on copy destinations CopyObject and UploadPartCopy build the destination attributes themselves and then assign them over the entry filer_pb.MkFile just stamped, so the copy landed with mode 0000 and uid/gid 0 - unreadable on a mount even by the filer's own user. Build the destination with the same mode PutObject resolves for the request and the gateway's own ids.	2026-08-19 18:57:37 -07:00
4 people	5d5fcdf07b	fix(filer): bound aggregated metadata reads by peer watermarks (#10803) ... * fix(filer): watermark-bound aggregated metadata subscription against multi-source merge races The aggregated metadata subscription (SubscribeMetadata) merges per-filer sources that become readable at independent paces, but tracks its progress with a single scalar cursor. Once the cursor passes a timestamp T, anything a source materializes below T afterwards is silently skipped: a peer recovering from a stall re-inserts its backlog late (late ring merge), and a source's flush can land a log file, or a later chunk of the same file, after a subscriber's disk pass listed the files (late persisted-log landing). This is the residual documented in #10501. Bound the subscriber's two read paths by what every source has provably made visible, each with its own watermark: - Delivery low-watermark -> in-memory reads. The meta aggregator tracks, per subscribed peer (self included), the newest timestamp received on that peer's stream - real events, or idle heartbeats (peer streams now opt into ClientSupportsIdleHeartbeat). The aggregated ring is complete up to the minimum across peers; in-memory reads hold at it. - Flush low-watermark -> persisted-log reads. Each filer reports its local log-buffer flush watermark on its stream: a new flushed_ts_ns response field, carried on idle heartbeats and on periodic flush reports (gated on ClientSupportsIdleHeartbeat). Disk passes freeze the minimum across peers before listing the log files and hold at it; the day-boundary cursor jump and the metadata-chunks ref listing are bounded the same way, the latter at minute-file granularity. - Held reads keep the cursor at the last entry actually delivered and retry; the retry re-lists the log files, which is what picks up a late-landing file. Both watermarks are relaxed by the settled horizon (2 x LogFlushInterval) as a liveness escape, so a peer stalled beyond it delays subscribers by at most the horizon instead of forever - any loss that escape allows was unconditional before. With reads held at the flush watermark, a disk advance below it is proven complete on every peer's disk, so the unproven-crossing counter now only counts crossings the horizon escape allowed past a stalled peer. Live delivery on the aggregated stream may lag by up to the idle-heartbeat interval when some peers are quiet; SubscribeLocalMetadata consumers are unaffected. * fix(filer): resume evicted aggregated readers from an original-space disk anchor The aggregated ring rewrites out-of-order peer arrivals to its head, so a subscriber tailing it advances its cursor in bumped (arrival) timestamps, while persisted logs keep original timestamps. When a slow reader's unread window is evicted (e.g. a peer backlog flooding in after a stall) and the reader falls back to disk, resuming from the bumped cursor skips every original-space entry below it that memory never delivered - reproduced as a ~66% silent loss on a 3-filer cluster with one peer's stream frozen for ~70s while the subscriber lagged. Track a disk anchor: the newest original-space position the stream is proven complete through. Disk passes advance it directly; contiguous memory reads advance it to the peers' delivery low-watermark observed before the read (per-peer streams are ordered, so everything with an original timestamp at or below that watermark had already arrived and was delivered). A reader kicked off the ring resumes the disk pass from the anchor instead of the bumped cursor - redelivering what memory already sent is within the subscription's at-least-once contract, skipping what it never sent is not. * fix(filer): close review findings on the peer-watermark subscription bounds Four correctness holes found in review, one generated-file cleanup: - The flush-through claim could assert durability for events still on their way into the buffer: an event is timestamped before notification work that can block, and only then appended. Track stamped-but-unappended events on the Filer (the stamp shares a lock with the reader, and appends are bumped monotonically past the buffer head), and cap the reported flush watermark just below the oldest in-flight stamp. - Removing a peer deleted its watermark entries while its stream kept running: its next signal recreated the deleted entry, which then pinned the low-watermark forever once the stream died. Watermarks now advance only for tracked peers, and peer removal cancels the subscription context so the stream stops feeding the aggregated buffer promptly. - The pipelined sender folded flush reports (TsNs 0 reads as far behind) into batch Events tails, where the aggregator's nil-notification guard dropped them - a busy backlog replay could starve the flush watermark until the settled-horizon escape opened a loss window. Control messages are now unbatchable on the sender, and the receiver also reads watermark state off nested batch entries as belt and braces. - A give-up skip's cursor was not anchored, so the next eviction rewind undid the counted decision and re-entered the same park forever when the evicted window carried bumped timestamps. The anchor now follows give-up skips; an anchored cursor makes the rewind a no-op and keeps the gap machinery's re-arm onto the retained window reachable. - Regenerated-file churn from a different protoc-gen-go-vtproto version is dropped: the vtproto file is upstream's, plus only the flushed_ts_ns marshal/size/unmarshal cases in the same generator style. New tests pin the in-flight floor, the no-resurrection rule for removed peers, and that control messages are never nested in batches. * fix(filer): keep a removed peer's watermarks through a grace period Deleting a peer's watermark entries the moment the master removes it reopened the loss the watermarks exist to prevent: a filer frozen or partitioned long enough to miss master heartbeats is removed from the cluster, its unflushed events still exist, and with its entries gone the low-watermarks snap forward to the healthy peers - subscribers advance past the absent peer's window and its late-landing log files are silently skipped. Reproduced on a 3-filer cluster: freezing two filers for ~70s got them removed ~28s in, and a catching-up subscriber lost their entire overlapping window. Removal now only marks the peer; its watermarks keep participating in the low-watermarks for a grace period (2 x LogFlushInterval, matching the subscribe loops' settled horizon, which already bounds a stale watermark's influence meanwhile). A re-added peer clears the mark and continues its values monotonically - the flap case costs nothing. A peer that stays gone is dropped when the grace expires, so a decommission cannot pin the low-watermarks, and a dropped peer's straggling signals cannot resurrect its entry. * fix(filer): cap delivery heartbeats by the in-flight floor; harden stamps Second review pass on the watermark bounds: - Idle heartbeats on the local stream claimed delivery-completeness through "now" while an event could still sit stamped-but-unappended behind blocking notification work. A peer aggregator turns that claim into its delivery low-watermark, so it could advance (and anchor credits with it) past an event that had not been streamed yet. The heartbeat timestamp is now capped just below the oldest in-flight stamp, like the flush claim already was. - In-flight stamps are forced monotonic against the registry's own history, so a wall-clock step backwards cannot slip a new stamp under an already-sampled floor. The cross-goroutine ordering still shares the meta log's global forward-clock assumption; the comments now say so instead of overclaiming. - Duplicate removal notifications no longer refresh a removed peer's grace deadline: the first removal time wins, so a decommissioned peer cannot sit in the watermark sets forever on repeated updates. - A failed buffer append clears the event's in-flight stamp on purpose: the event is dropped from the change stream entirely (a pre-existing defect of the append path, loudly logged), and a watermark waiting for it would pin this filer's claims forever. The comments now state the decision instead of implying the failure cannot happen. * docs(filer): tighten the watermark comments Comment-only: compress the narrative comments added on this branch down to their load-bearing invariants, and fix one stale sentence (peer removal no longer deletes the watermark entries immediately). No code changes. * fix(filer): subscribe to the local filer before remote peers Self's events reach the aggregated buffer only through the aggregator's own subscription to it, but bootstrap only seeded the peers the master already listed - and self's master registration races that listing, so the watermark set could hold remote peers without self. Once the remotes signalled, the low-watermarks would claim completeness for a stream that was still missing a merge source, letting aggregated subscribers advance past the local filer's events before its subscription started. Seed self first, unconditionally: before that the watermark set is empty (a documented safe state - reads hold at the settled horizon), and after it the set can never be remotes-only. The later master update for self, or a duplicate in the listed peers, is a no-op via the already-followed check in OnPeerUpdate. * fix(filer): fence watermark claims against wall-clock regression Record issued heartbeat/flush claims in the in-flight registry and stamp later events above them, so a backward clock step cannot land an event under a watermark a peer has already advanced to. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(filer): re-check the buffer head after fencing heartbeat claims An event appended between the caught-up check and the delivery claim was covered by the claim but not yet sent on the stream. The claims fence later stamps, so re-checking the head after them proves every covered event was already sent before the heartbeat. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(filer): cross the aggregated ring's pre-subscription range only on proof The eviction gate and the gap proofs read "nothing evicted yet" as "memory holds everything after the cursor". That is false for the merge-fed aggregated ring, which is born empty while every peer's history sits on disk: before the ring's first real eviction, a subscriber whose cursor was still below the bounded chunk pass's listing stop was served the ring's earliest entry inclusively, silently skipping the withheld pre-restart files - and the idle-wait callback credited the delivery low-watermark to the disk anchor in the same disconnected state. Mark everything at or below the subscriptions' start as evicted when the aggregator is built, credit the anchor only once the run is connected to the ring, and give the aggregated gap pass a real proof to cross the marked boundary with: each disk pass's proven coverage (the peer flush low-watermark capped by the pass's listing bound). An empty pass whose proof reaches the eviction watermark crosses to it silently - no park, no loss counter - so the mark costs a bounded catch-up delay instead of the 15-minute give-up. * fix(filer): keep shipped chunk tails at or below the hold point A log file spans past its named minute (window start plus up to a flush interval), and chunk-mode clients apply a shipped file whole - so a file tail past the hold point can become a persisted client checkpoint beyond what every peer has proven, and a crash inside that window resumes past another peer's late-but-in-contract flush. Stop the ref listing a minute plus a flush interval below the hold; the withheld band is served by the memory pass (ring retention far exceeds it) or by later passes as the hold advances, so freshness is unchanged. A frozen peer flushing one window that spans its whole freeze can still overshoot; that residual is bounded by the freeze and needs a crash inside it. * docs(filer): trim the review-fix comments --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-08-19 18:38:46 -07:00
Chris LuandGitHub	804111745a	mount: discard a path-cache insert that raced a purge (#10842) ... * mount: discard a path-cache insert that raced a purge The Windows adapter's walk resolves a component with a Lookup RPC and inserts the result holding no lock, so a purge can land in between - and what the walk just resolved is then the very name the purge removed. Anything opening the old path concurrently with a rename repopulates the cache with the vacated name, which the next stat is served from for up to a second. The release path already guards its equivalent insert; the walk had nothing. The cache counts purges now. A resolve snapshots the generation before its lookups and insert discards the entry when any purge ran in between, parking the reference in the graveyard so the in-flight caller keeps a valid inode either way. Seen once in CI as TestRenameOverExisting failing with 'source survived the rename': every SeaweedFS layer is synchronous with the rename, but a background open of the source - an antivirus scan of the just-written file fits - can requalify the stale name through this window. The assertion also reports what stat returned now, and whether it persisted, so a recurrence indicts a specific layer instead of reading as a mystery. * mount: cover the path-cache discard by key, and let a discard rest Review follow-ups. The generation was global, so any purge between a walk's snapshot and its insert discarded the entry whatever its name - and an open retries resolve-then-steal only four times before failing with EIO, so sustained unrelated churn could fail opens of untouched paths. Purges are remembered by key now and only one that covers the inserted name discards it; past the remembered window the insert is discarded without a check, which only costs a retry. A discard that itself tripped the sweep also handed its own reference straight to forget while the walker was still using the inode. The graveyard holds two generations now, so an appended reference always survives the sweep of the call that appended it - which the displaced-entry and purge paths needed too. Also restores the original path-cache test suite this branch had overwritten instead of extended, and rewords the semantics-test failure so it no longer claims the source survived when stat returned a transient error. * mount: take an open's reference directly instead of stealing it back resolveAndSteal cached the final component only to steal it back, so an open depended on that insert surviving whatever purges raced it - four attempts and then EIO. The keyed purge window narrowed how often an insert is discarded, but past the window the discard is blind again, so the cliff had only moved. A cached entry is still stolen; anything else is now looked up directly, with the caller owning the reference from the start. No retry loop, and no way for churn - covered, unrelated or overflowing the window - to fail an open. Also covers the whole-cache purge: purge of the root with prefix set clears every entry, but the covers check tested for a '/'-prefixed key that a normalised key never has, so it covered no in-flight insert at all.	2026-08-19 17:05:49 -07:00
Chris LuandGitHub	da1e5e714f	mount: move the inode table when a rename arrives from the cluster (#10822) ... * mount: leave the target alone when a move has no source MovePath cleared whatever sat at the target before it checked that the source was still there, so a move it then declined to make had already taken the target's mapping apart. The same rename reaching the table twice - once for an open handle, once for the invalidation behind it - was enough to leave the moved inode with no path at all. * mount: move the inode table when a rename arrives from the cluster A rename made by another client reaches this mount only as a metadata event, and the only table update on that path sat inside the open-file-handle branch. Every other inode the kernel still addresses by nodeid kept resolving to its pre-rename path, so the next operation on it went to a path the filer no longer has. Move the entry for every rename invalidation. The filer sends one event per moved entry, so a renamed directory's children follow their parent without a descendant walk. * mount: move the inode table exactly once per rename event Making MovePath return early on a missing source was the wrong half to fix. The source is also missing when the rename came from a client that never visited it, and there the destination really was replaced and has to be unlinked - the early return kept the name resolving to a file the rename destroyed, so a dirty handle on it could still flush over what took its place. The two cases are indistinguishable from inside MovePath, so leave it alone and stop calling it twice: invalidateOpenFileHandle reports whether it moved, and the handler moves only when it did not. Both paths mark a replaced file's handle deleted, which the no-handle path previously did not do at all. * mount: leave a rename alone unless the source is still ours to move Two ways the fallback move could act on state it did not own. A handle whose version guard skipped the event never reached RememberPath, so moving the table under it left the handle flushing to the pre-rename path; the handle path owns its inode's rename, so the fallback now runs only for an inode without one. And the subscription can redeliver a rename once it falls out of the 4096-entry dedup ring. A replay found no source and a live target, unlinked the mapping the first delivery had just made, and marked the moved file's handle deleted - worse than the stale path this set out to fix. Only a source still in the table is moved now. That gives up unlinking a destination the rename replaced when the source was never visited here, which is where this started. It is what the mount already did before this branch, and it is the safer of the two: retaining a stale name costs a wrong lookup, while unlinking the wrong one costs a file's dirty data. * mount: decide a rename move inside the table's lock The source-presence check sat outside MovePath, so two invalidations for one rename could both see the source and the loser would unlink what the winner had just placed - the same damage the check was added to prevent. MovePath makes the decision under its own lock now and reports that nothing moved. The handle a rename destroyed is also marked from the caller rather than from inside invalidateOpenFileHandle, which was setting isDeleted bare on a second handle while holding the first one's lock. markHandleDeleted already takes the lock the flush reads that flag under, and marking from the caller keeps it to one handle lock at a time. The invalidation test harness wires onEntryInvalidation now, the way the mount does, rather than reaching past it. * mount: apply a rename ahead of the handle's version fence The fence exists so an old event cannot roll a handle's entry back to stale content. A rename carries no content: it says the name the inode answered to is gone. Skipping one on the strength of the fence left the inode and the handle both pointing at a name the filer had vacated, and no fallback ran either, since a handle owns its inode's rename. Applied before the fence now, and only when MovePath reports the source was still ours to move - which is what keeps a replayed rename from remembering a path the handle has already moved past.	2026-08-19 14:57:45 -07:00
3 people	9a8b204a7e	Add monitoring label to filer servicemonitor (#10835) ... * Add monitoring label to filer servicemonitor * helm: label the headless filer service, not the client one The ServiceMonitor takes its job label from the service name, and the bundled dashboard queries job="seaweedfs-filer". Selecting the client service would have renamed the job and blanked those panels. The headless service also publishes not-ready addresses, so it keeps reporting while a filer is starting up or shutting down. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-08-19 14:21:36 -07:00
Chris LuandGitHub	cd3db76eed	ci: stop installing FUSE headers nothing links against (#10840) ... Four workflows ran apt to install libfuse3-dev before every FUSE job. Nothing needs it: go-fuse implements the protocol in pure Go, no cgo in the tree references fuse, and the package does not even provide the fusermount3 the mount actually execs - fuse3 does, and it is already on the runner image, which is why the setuid-repair step finds it. So the step downloaded a dev package to build against headers no compiler ever opened, and it is the step that has been hanging whenever the Ubuntu mirror goes slow. Configuring /etc/fuse.conf is all that is left.	2026-08-19 14:09:40 -07:00
Chris LuandGitHub	e0c4732e5e	rust: stop writing when a durable write's index flush fails (#10825) ... * rust: stop writing when a durable write's index flush fails A durable write flushes the .dat, publishes the needle map row, then flushes the .idx. If that last flush failed we returned the error and carried on: the row stayed live, the volume stayed writable, and the handler answered 500 without replicating. The primary then served a needle its replicas never saw, for a write the client was told had failed - and if the unflushed row was lost on restart, the durable .dat tail took the volume read only anyway. Taking the row back out is not an option: it means undoing published state on a disk that is already failing, and a truncate afterwards would leave an .idx row pointing past the end. So the volume stops taking writes instead, the same as when the truncate after a failed .dat flush cannot be done. Nothing more gets appended past a record whose index is in doubt, and the master routes writes elsewhere once the volume heartbeats read only. The divergence against the replicas is still there, but it is bounded and it is visible. A failed nm.put after the .dat is down leaves the same durable but unindexed record, so it takes the same route. * rust: drop the import the rollback removal left behind NeedleValue came in with rollback_unflushed_write, which went away when the durable path moved to flushing before it publishes. Nothing has used the type since. * rust: mark the test-only heartbeat helper as such collect_heartbeat has only ever been called from the tests - the send loop uses collect_heartbeat_with_snapshot, which it wraps - so a lib build rightly called it dead code. * rust: flush the index on a durable write that dedups A durable write matching content already in the volume flushed the .dat and returned before reaching the index flush. So a fsync=true write that deduped against an earlier non-durable one was acked with the row that indexes it still in the page cache - the same false promise the index flush exists to rule out, and the same read-only volume on restart if the row is lost. The dedup path now flushes both files, and the quarantine on a failed index flush moved into flush_idx so it applies wherever the flush is reached rather than only at the one call site that had it inline.	2026-08-19 14:01:19 -07:00
Chris LuandGitHub	3b18a635df	ci: call the apt helper from the workflow's working directory (#10832) ... The e2e workflow sets defaults.run.working-directory: docker, so the call I added resolved to docker/docker/apt-install and every FUSE Mount run has failed with 'sudo: docker/apt-install: command not found' since it merged.	2026-08-19 00:46:32 -07:00
Chris LuandGitHub	baead6901c	ci: build protoc into the crate instead of installing it per job (#10830) ... Every workflow that builds the Rust volume server first installed protoc from a package manager - twelve steps across apt, brew and choco. That is 37s per job on a good day, and this week archive.ubuntu.com stalled long enough for four jobs to burn their whole timeout without reaching a build. protoc-bin-vendored ships the compiler as a build-dependency, so it now arrives through the cargo registry the workflows already cache and there is nothing left to install. cargo build works on a machine with no protoc at all, which is worth as much locally as it is in CI. It also pins the version. The apt protoc on ubuntu-22.04 is 3.12, old enough to reject proto3 optional, which is why build.rs passes --experimental_allow_proto3_optional; the vendored one is 31.1. The flag stays, since it costs nothing and keeps a build against an older PROTOC working, and an explicit PROTOC still overrides the vendored binary for packagers who supply their own.	2026-08-19 00:18:30 -07:00
Chris LuandGitHub	1564244b1a	ci: install the runner's own packages through the mirror fallback too (#10831) ... The e2e job overwrote the runner's sources.list with two azure-only lines and installed fuse from it, so the same mirror outage that took out the image builds failed the step outright - this time on the runner rather than inside the container, where the image-side fallback cannot reach. Install through the same helper, and widen its rewrite to match any archive host so it works whether the pristine list came from the base image (archive.ubuntu.com) or from a CI runner (azure.archive.ubuntu.com). Keeping the runner's original list also restores the security and backports pockets, which the hand-written two-line replacement dropped. Verified against the outage itself: with the pristine list pointed at Azure, the build logged the skip after Azure timed out for real and installed from archive.ubuntu.com.	2026-08-19 00:17:51 -07:00
Chris LuandGitHub	05013ad3da	ci: fall through to another Ubuntu mirror when one is unreachable (#10828) ... The e2e image pointed both archive and security at azure.archive.ubuntu.com and nothing else, and the samba and pjdfstest images inherit that list. When Azure is unreachable the build has nowhere to go: Acquire::Retries just retries a dead host, every package fails, and apt exits 100 before a single test runs. Two different workflows lost runs to it tonight. Install through a helper that starts from the pristine sources.list each time and walks a list of mirrors, so Azure stays the preferred one - the reason it was pinned in the first place - without being the only one. Verified both paths against a real build: the normal one installs from Azure, and with the first entry pointed at an unroutable host the fallback logs the skip and installs from archive.ubuntu.com.	2026-08-19 00:03:04 -07:00
Chris LuandGitHub	da4f06ec12	Give the local Unix socket gRPC transport room to breathe (#10824) ... * Give the local Unix socket gRPC transport room to breathe Unix socket buffers default small and never autotune: 208KB on Linux, 8KB on macOS. Once the buffer cannot absorb what gRPC's loopyWriter emits for the in-flight streams the writer blocks on Write, and since v1.82.1 grpc-go counts per-RPC bookkeeping toward its control-buffer throttle, so both peers stop reading and the connection deadlocks for good. weed mini wedged at roughly 320 concurrent S3 PUTs with every filer RPC parked in waitOnHeader and no handler running. Force 8MB on both ends of the sockets we open. Best effort, since a kernel may clamp it lower; that only lowers the concurrency this survives. TCP loopback never hit this because its buffers start large and grow. * Set the buffer on accepted connections too Linux does not carry the listener's SO_SNDBUF onto sockets returned by accept, so only the dialing half was getting the headroom: measured 8388608 on the dialed side against the 212992 default on the accepted side. Wrap the listener and re-apply per connection. macOS inherits either way, which is why this did not show up locally.	2026-08-18 21:32:23 -07:00
Chris LuandGitHub	bb223967bd	mount: fold an inode's single link into its entry (#10818) ... InodeEntry held its one path in a slice, so every inode the kernel references cost a 16-byte backing array and a second heap object on top of the 32-byte entry. The extra links of a hard-linked file now hang off a pointer instead, which keeps the struct in the same 32-byte size class and leaves the ordinary single-link file with nothing to allocate. Populating the table with 1M children: 237.5 -> 221.5 B/inode at 85-character paths, 301.3 -> 285.6 at 148.	2026-08-18 20:56:06 -07:00
Chris LuandGitHub	9f15e3935c	mount: reuse the listed entry's path instead of rebuilding it (#10817) ... readdir built dirPath.Child(name) for every child while entry.FullPath was already that exact string, from NewFullPath in the meta cache store or from FromPbEntry on the read-through path. One allocation per entry, and on a wide tree with long paths that is most of what a listing allocates. BenchmarkReadDirectory/kernel_readdirplus over 200k entries: 2,039,656 -> 1,839,318 allocs/op, 174.5 -> 167.8 MB/op, 152.6 -> 135.1 ms/op.	2026-08-18 20:55:17 -07:00
Chris LuandGitHub	887910b377	rust: honor fsync on the volume server write path (#10816) ... The Rust volume server ignored the fsync parameter completely: nothing parsed it, and write_volume_needle -> write_needle -> append_needle never flushed. So a ?fsync=true upload was acked out of the page cache, and since ReplicatedWrite forwards the parameter, a Go primary handing a durable write to a Rust replica got the same empty promise. The upload handler now reads fsync the way Go's r.FormValue does, off the decoded query fields, and threads it down to the volume. A durable write appends, flushes the .dat, publishes the needle map entry, then flushes the .idx, and only then is it acked. Nothing points at bytes that are not down yet, so a failed flush only has to take its own append back off the end - the index never moved and the volume's counters never saw the rejected write. If that truncate cannot be done the volume stops taking writes, rather than letting a later append bury the rejected record mid-file where the tail integrity check cannot see it. The .idx flush is what keeps the ack honest: load() rebuilds the map from .idx, so an acked write whose row was lost comes back as a .dat tail the integrity check cannot account for, and the volume loads read only. A dedup hit flushes too: there is nothing to append, but the write it matched may have been non-durable, and the caller is asking for the content to be on disk. Batched writes carry the flag per request rather than one flush per batch, so the write queue's module doc no longer claims otherwise.	2026-08-18 20:22:42 -07:00
Chris LuandGitHub	358fd314ea	test(s3/versioning): read the whole version body instead of one Read (#10815) ... A single Read on the response body can return the last bytes together with io.EOF, so asserting NoError on it fails even though the body is complete. Use io.ReadAll, like every other test in this package.	2026-08-18 18:34:01 -07:00
Lars LehtonenandGitHub	ed75a61fb0	fix(test/s3/versioning): dropped test error (#10813)	2026-08-18 17:26:51 -07:00
4 people	9575032b4c	volume: forward fsync=true to replicas in ReplicatedWrite (#10805) ... * volume: forward fsync=true to replicas in ReplicatedWrite When a write request carries fsync=true, only the primary volume server flushed to disk: the replica fan-out URL in ReplicatedWrite only carried type/ttl/ts/cm, so replicas always wrote without fsync even when the client explicitly requested a durable write. Forward the fsync request parameter to the replica volume servers so a durable write means every replica has flushed to disk, not just the primary. Replicas without fsync are untouched (zero behavior change). * storage: flush a durable write inline while stopping The fsync flag on the write path really selects the async batch worker, and it was switched off once the store is stopping. So a fsync=true write landing during the pre-stop drain got acked without ever being flushed - and now that ReplicatedWrite forwards fsync, that covers replicas too. Flush it inline instead of queueing it. The drain keeps accepting writes, which is the whole point of preStopSeconds, and the ack still means the .dat is on disk. If the fsync fails, the append comes back off the .dat and the needle map goes back to what it pointed at before, so nothing resolves to an offset past the truncated end. * storage: make the store's stopping flag atomic SetStopping runs on the signal handler goroutine while the write and vacuum paths read the flag, so every read of it was racy. Nothing about the shutdown ordering changes; only the flag itself is now safe to read. * topology: check the errors the replication test was dropping The mock replica ignored its response write and the mock master ignored whatever Serve returned, so a broken mock would have shown up as a confusing timeout rather than a failure. Also drops the explicit listener close: grpc.Server.Stop already closes the listener it was given. --------- Co-authored-by: hzsunchao <hzsunchao@corp.netease.com> Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-08-18 17:17:50 -07:00
Chris LuandGitHub	1354b58675	s3: stop unrouted bucket subresources from being answered with a listing (#10814) ... * s3: answer GetBucketReplication, GetBucketWebsite and GetBucketNotificationConfiguration None of the three had a route, so they reached the unconstrained ListObjectsV1 catch-all and a client asking for a bucket's replication config got 200 and a <ListBucketResult> back. Replication and website report their configuration as absent the way AWS does; notification returns the empty configuration AWS returns for a bucket with no events wired up. * s3: stop an unrouted bucket subresource from being answered with a listing ListObjectsV1 is the catch-all GET on a bucket, so every subresource without a route of its own - ?torrent today, whatever AWS adds next - came back 200 with a <ListBucketResult>. A client that asked for a configuration and got a listing either fails its XML decode in a way that reads like corruption, or worse, tolerantly parses it. Refuse the request instead. The allow-list is the ListObjects parameters rather than the subresources, so a new one fails closed. Presigned URLs sign their credentials into the query string, so X-Amz-* and the SigV2 trio have to stay listable.	2026-08-18 16:03:08 -07:00
3 people	68a23a4b3c	filer: stop remote.unmount from deleting the remote objects (#10811) ... * filer: add filer.options.disable_remote_storage_deletion for cache-only deletes Deleting a filer entry under a remote.mount path also deletes the backing object from the remote store (maybeDeleteFromRemote). Deployments that use a remote mount as a read-through cache in front of an authoritative, externally-managed object store cannot allow this: the filer typically holds read-only credentials, so the remote delete fails and the entire delete errors out; and even where it would succeed, it destroys data the filer does not own. Add filer.options.disable_remote_storage_deletion (default false, so existing behaviour is unchanged). When enabled, maybeDeleteFromRemote is skipped for both single-entry and recursive folder deletes: local metadata and cached chunks are still removed, but the remote object is left intact. * filer: assert local removal in cache-only recursive delete test The recursive cache-only delete test only checked that no remote delete happened; it did not verify the local child and directory entries were removed. Add FindEntry assertions so a regression that skips local recursive deletion is caught. * filer: reload the remote mount mapping when /etc/remote changes The mapping was only read at startup, so remote.unmount left the mount live in the filer: the purge that follows the mapping delete then went to the remote store and wiped every object under the mount. Rebuild the rules trie and the conf map from scratch on each load, since ptrie cannot drop a key, and swap them under a lock. * filer: drop the filer-wide remote deletion switch With the mapping reloaded on unmount, the purge no longer reaches the remote store, so there is nothing left for the switch to protect against. --------- Co-authored-by: Chris Lu <chris.lu@gmail.com>	2026-08-18 15:38:02 -07:00
Chris LuandGitHub	f41595fb10	mount: drop consumed entries when reading a directory through (#10802) ... The cached readdir trims the head of the handle's entry stream as the client walks past it; the read-through path never did, so a directory too large to cache -- the only kind that takes that path -- was held whole in the handle for the length of the walk. Hoist the trim to cover both paths.	2026-08-17 21:22:03 -07:00
Chris LuandGitHub	3cf7d306a5	Give the WebDav chunk reader a bounded, invalidatable location cache (#10801) ... * mount: re-resolve volume locations after a failed chunk read NewChunkGroup passed nil as the ReaderCache's CacheInvalidator, so retryFetchAfterCacheInvalidation was dead code on the FUSE read path. A mount that cached a volume's locations while one server was down kept retrying that server after it died, then returned EIO, even though the master and filer both resolved the live replica. The S3 gateway already passes its filerClient; do the same for the mount. * test: FUSE integration tests for volume server failover One mount appends while a second tails, and a volume server is killed, started or restarted mid-stream against a 001-replicated cluster of three volume servers. Automates the scenario matrix reported for Docker Swarm mounts, including the large-file variant and a no-chaos control. * test: report the filer's own view when append content mismatches A mismatch between what the writer wrote and what the reader sees can come from either side's cache. Read the file back through the filer's HTTP handler as well, and let the mount verbosity be raised from the environment, so a failing run says which layer lost the data. * test: wait for the reader mount to converge before comparing A mount caches metadata for about a second, so reading the file the instant the writer's last close returned can legitimately come back short. Poll the reader until it matches or the timeout expires; content that is wrong rather than merely late never converges and still fails, now with the writer's mount and the filer's own view alongside it. * test: detect a failover cluster child that exited at startup Signal(0) succeeds for a zombie and nothing reaped these children until shutdown, so a process that died on startup looked alive until the readiness timeout expired. Reap each child as it is started and consult the result. * test: read a file the killed volume server actually holds Placement decides which two of three servers back each volume, so killing volume N and reading readfile-N could pass without the victim ever holding a replica of it. Resolve each file's volumes through the filer and the master, and pick one the victim backs, preferring a file the reader has not cached. * ci: stop persisting checkout credentials in the failover workflow The job does not use the token after cloning. Also tag the README's command block as bash and match the timeout the workflow actually uses. * test: discard the ignored errors errcheck flags in the failover harness * test: resolve manifests when mapping a file to its volumes A manifest chunk's own fid names the volume holding the manifest, not the volumes holding the data, so a large enough file would point the failover victim at the wrong server. * test: pin the stale-location recovery path with a primed reader Reading a file for the first time after a server dies proves nothing: the lookup is fresh and returns the survivor. Kill one holder and wait for the master to drop it, read a file on that volume so the reader caches the lone survivor, restart the first server, then kill the survivor. The reader's only cached location is now dead while the data is live elsewhere, which is the case the invalidator exists for: EIO without it, recovery with it. * filer: re-look-up a chunk's locations as soon as they all fail A read that fails against every location it was given is far more likely to be holding a stale list than to be hitting a cluster that is briefly slow, but the retry loops spent the whole backoff ladder, about 13 s, before the caller got a chance to invalidate and look the chunk up again. Give the loops a refresh hook and let the reader cache invalidate on the first fully failed pass, so recovery starts in milliseconds. Clients without an invalidator keep the old behavior. The filer's streaming read path has its own fetch loop and is not covered. * webdav: give the chunk reader a bounded, invalidatable location cache WebDav resolved chunk locations through filer.LookupFn, whose own doc asks long-running processes to prefer wdclient.FilerClient: its cache is unbounded, and it has no way to invalidate an entry, so the reader cache was constructed with a nil invalidator and a WebDav server that had cached a location kept reading from it after the volume moved or died. Use FilerClient, as the mount and the S3 gateway already do. * filer: refresh locations on the random-read path too readChunkSliceAt bypasses the chunk cacher in random-access mode and fetches the range directly, which left it without the invalidation the cacher does: a random reader parked on a stale location had no way back at all. Hoist the refresh hook onto the reader cache so both paths share it. * filer: compare chunk locations as a set, not in order Lookups shuffle the locations they return, so comparing positionally reads a reshuffle of the very same replicas as a fresh set and spends an immediate retry on locations that just failed. weed/filer already had an order-independent comparison for this; move it next to the retry loops so both callers share one helper.	2026-08-17 20:19:54 -07:00
Chris LuandGitHub	f3dc530919	Re-look-up a chunk's locations as soon as they all fail (#10800) ... * mount: re-resolve volume locations after a failed chunk read NewChunkGroup passed nil as the ReaderCache's CacheInvalidator, so retryFetchAfterCacheInvalidation was dead code on the FUSE read path. A mount that cached a volume's locations while one server was down kept retrying that server after it died, then returned EIO, even though the master and filer both resolved the live replica. The S3 gateway already passes its filerClient; do the same for the mount. * test: FUSE integration tests for volume server failover One mount appends while a second tails, and a volume server is killed, started or restarted mid-stream against a 001-replicated cluster of three volume servers. Automates the scenario matrix reported for Docker Swarm mounts, including the large-file variant and a no-chaos control. * test: report the filer's own view when append content mismatches A mismatch between what the writer wrote and what the reader sees can come from either side's cache. Read the file back through the filer's HTTP handler as well, and let the mount verbosity be raised from the environment, so a failing run says which layer lost the data. * test: wait for the reader mount to converge before comparing A mount caches metadata for about a second, so reading the file the instant the writer's last close returned can legitimately come back short. Poll the reader until it matches or the timeout expires; content that is wrong rather than merely late never converges and still fails, now with the writer's mount and the filer's own view alongside it. * test: detect a failover cluster child that exited at startup Signal(0) succeeds for a zombie and nothing reaped these children until shutdown, so a process that died on startup looked alive until the readiness timeout expired. Reap each child as it is started and consult the result. * test: read a file the killed volume server actually holds Placement decides which two of three servers back each volume, so killing volume N and reading readfile-N could pass without the victim ever holding a replica of it. Resolve each file's volumes through the filer and the master, and pick one the victim backs, preferring a file the reader has not cached. * ci: stop persisting checkout credentials in the failover workflow The job does not use the token after cloning. Also tag the README's command block as bash and match the timeout the workflow actually uses. * test: discard the ignored errors errcheck flags in the failover harness * test: resolve manifests when mapping a file to its volumes A manifest chunk's own fid names the volume holding the manifest, not the volumes holding the data, so a large enough file would point the failover victim at the wrong server. * test: pin the stale-location recovery path with a primed reader Reading a file for the first time after a server dies proves nothing: the lookup is fresh and returns the survivor. Kill one holder and wait for the master to drop it, read a file on that volume so the reader caches the lone survivor, restart the first server, then kill the survivor. The reader's only cached location is now dead while the data is live elsewhere, which is the case the invalidator exists for: EIO without it, recovery with it. * filer: re-look-up a chunk's locations as soon as they all fail A read that fails against every location it was given is far more likely to be holding a stale list than to be hitting a cluster that is briefly slow, but the retry loops spent the whole backoff ladder, about 13 s, before the caller got a chance to invalidate and look the chunk up again. Give the loops a refresh hook and let the reader cache invalidate on the first fully failed pass, so recovery starts in milliseconds. Clients without an invalidator keep the old behavior. The filer's streaming read path has its own fetch loop and is not covered. * filer: refresh locations on the random-read path too readChunkSliceAt bypasses the chunk cacher in random-access mode and fetches the range directly, which left it without the invalidation the cacher does: a random reader parked on a stale location had no way back at all. Hoist the refresh hook onto the reader cache so both paths share it. * filer: compare chunk locations as a set, not in order Lookups shuffle the locations they return, so comparing positionally reads a reshuffle of the very same replicas as a fresh set and spends an immediate retry on locations that just failed. weed/filer already had an order-independent comparison for this; move it next to the retry loops so both callers share one helper.	2026-08-17 20:19:28 -07:00
4 people	93227c6dc3	build(deps): bump go.etcd.io/etcd/client/v3 from 3.6.12 to 3.7.1 (#10789) ... Bumps [go.etcd.io/etcd/client/v3](https://github.com/etcd-io/etcd) from 3.6.12 to 3.7.1. - [Commits](https://github.com/etcd-io/etcd/compare/v3.6.12...v3.7.1) --- updated-dependencies: - dependency-name: go.etcd.io/etcd/client/v3 dependency-version: 3.7.1 dependency-type: direct:production update-type: version-update:semver-minor ... 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 <chrislusf@users.noreply.github.com>	2026-08-17 19:57:31 -07:00
Chris LuandGitHub	606a90b3b1	filer: close the empty-folder race by checking after each mutation (#10799) ... * filer: re-list a folder after deleting it, and put it back if it is not empty The emptiness check inside the delete and the removal of the folder entry are not atomic, so an entry can land between them and be left reachable by its own path but out of every listing. Looking again after the delete catches the ones whose create event has not arrived yet, and does not depend on the event stream or on the observation window holding. * filer: create the directories holding an entry after the entry A parent checked before the insert can be taken by the empty-folder cleaner before the entry lands, which leaves the entry reachable by its own path but out of every listing. Creating the parents afterwards cannot be undone by a delete that was authorised before the insert, and pairs with the cleaner re-listing after its own delete: whichever of the two acts second sees what the other did. Going second means the entry is already stored when the parent fails, so it is taken back out and the caller still sees the error it used to get. * filer: narrow a directory that came back wider than the one it replaced A writer recreating its own missing parent has only the entry it is inserting to go on, so the directory it mints can grant access the deleted one denied - a 0700 folder comes back 0751. The cleaner read the real attributes before deleting, so its restore now puts the original mode back instead of leaving the inferred one in place. It only ever narrows, so a directory deliberately tightened since is left as it is.	2026-08-17 19:57:05 -07:00
Chris LuandGitHub	1ddec72707	Recover from a dead volume server on the mount read path (#10798) ... * mount: re-resolve volume locations after a failed chunk read NewChunkGroup passed nil as the ReaderCache's CacheInvalidator, so retryFetchAfterCacheInvalidation was dead code on the FUSE read path. A mount that cached a volume's locations while one server was down kept retrying that server after it died, then returned EIO, even though the master and filer both resolved the live replica. The S3 gateway already passes its filerClient; do the same for the mount. * test: FUSE integration tests for volume server failover One mount appends while a second tails, and a volume server is killed, started or restarted mid-stream against a 001-replicated cluster of three volume servers. Automates the scenario matrix reported for Docker Swarm mounts, including the large-file variant and a no-chaos control. * test: report the filer's own view when append content mismatches A mismatch between what the writer wrote and what the reader sees can come from either side's cache. Read the file back through the filer's HTTP handler as well, and let the mount verbosity be raised from the environment, so a failing run says which layer lost the data. * test: wait for the reader mount to converge before comparing A mount caches metadata for about a second, so reading the file the instant the writer's last close returned can legitimately come back short. Poll the reader until it matches or the timeout expires; content that is wrong rather than merely late never converges and still fails, now with the writer's mount and the filer's own view alongside it. * test: detect a failover cluster child that exited at startup Signal(0) succeeds for a zombie and nothing reaped these children until shutdown, so a process that died on startup looked alive until the readiness timeout expired. Reap each child as it is started and consult the result. * test: read a file the killed volume server actually holds Placement decides which two of three servers back each volume, so killing volume N and reading readfile-N could pass without the victim ever holding a replica of it. Resolve each file's volumes through the filer and the master, and pick one the victim backs, preferring a file the reader has not cached. * ci: stop persisting checkout credentials in the failover workflow The job does not use the token after cloning. Also tag the README's command block as bash and match the timeout the workflow actually uses. * test: discard the ignored errors errcheck flags in the failover harness * test: resolve manifests when mapping a file to its volumes A manifest chunk's own fid names the volume holding the manifest, not the volumes holding the data, so a large enough file would point the failover victim at the wrong server. * test: pin the stale-location recovery path with a primed reader Reading a file for the first time after a server dies proves nothing: the lookup is fresh and returns the survivor. Kill one holder and wait for the master to drop it, read a file on that volume so the reader caches the lone survivor, restart the first server, then kill the survivor. The reader's only cached location is now dead while the data is live elsewhere, which is the case the invalidator exists for: EIO without it, recovery with it.	2026-08-17 17:30:33 -07:00
Chris LuandGitHub	6fda8c67f3	Guard the gcs credential path in FetchAndWriteNeedle like the other backends (#10796) ... * volume: accept only static-key gcs credentials on the fetch request An inline credentials document of a federated type points the SDK at a url, file or executable of the caller's choosing for the token exchange, so the request-supplied value is no longer just a key. * volume: guard the gcs token endpoint like the other remote endpoints Inline credentials pick where the token request goes, so route the gcs client through the same deny-list and rebinding-safe dialer used for S3 and azure. * rust volume: pin that gcs has no credential-driven dial path * volume: only check gcs credentials on a gcs remote conf Only the gcs backend reads that field, so another backend carrying a stale value should not fail the request. * gcs: load credentials with the type the caller expects The untyped loader is deprecated because it reads whatever the document claims to be; callers handling credentials they do not control now name the types they accept.	2026-08-17 16:40:56 -07:00
Chris Lu	9d8acbd244	Create icon.svg	2026-08-17 16:11:27 -07:00
Chris LuandGitHub	1bcd55eba2	go 1.26 (#10797)	2026-08-17 15:39:20 -07:00
Chris LuandGitHub	e383ee47cb	filer: use bind variables for request-controlled values in the arangodb store (#10795) ... * arangodb: bind list prefix, start file name and collection into the AQL query Concatenating them into the query text let a caller-supplied prefix or start name close the string literal and append arbitrary AQL, which runs with the filer's ArangoDB credentials against any collection. * arangodb: bind the folder path and collection into the recursive delete query A trailing-slash S3 key reaches DeleteFolderChildren through the directory-marker cleanup, so quotes in the path could turn the filter into a match-everything REMOVE over the whole bucket collection. * arangodb: match the real directory prefix in the recursive delete The prefix was built by re-joining the path segments with commas, so it never matched a stored directory and the subtree sweep did nothing.	2026-08-17 15:15:26 -07:00
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.	2026-08-17 14:46:25 -07:00
3 people	518da712b5	build(deps): bump github.com/rabbitmq/amqp091-go from 1.11.0 to 1.13.0 (#10791) ... Bumps [github.com/rabbitmq/amqp091-go](https://github.com/rabbitmq/amqp091-go) from 1.11.0 to 1.13.0. - [Changelog](https://github.com/rabbitmq/amqp091-go/blob/main/CHANGELOG.md) - [Commits](https://github.com/rabbitmq/amqp091-go/compare/v1.11.0...v1.13.0) --- updated-dependencies: - dependency-name: github.com/rabbitmq/amqp091-go dependency-version: 1.13.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-08-17 14:44:06 -07:00
3 people	d36073f8d7	build(deps): bump golang.org/x/net from 0.57.0 to 0.58.0 (#10790) ... Bumps [golang.org/x/net](https://github.com/golang/net) from 0.57.0 to 0.58.0. - [Commits](https://github.com/golang/net/compare/v0.57.0...v0.58.0) --- updated-dependencies: - dependency-name: golang.org/x/net dependency-version: 0.58.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-08-17 14:43:39 -07:00
3 people	42f2a48c90	build(deps): bump google.golang.org/api from 0.289.0 to 0.293.0 (#10792) ... Bumps [google.golang.org/api](https://github.com/googleapis/google-api-go-client) from 0.289.0 to 0.293.0. - [Changelog](https://github.com/googleapis/google-api-go-client/blob/main/CHANGES.md) - [Commits](https://github.com/googleapis/google-api-go-client/compare/v0.289.0...v0.293.0) --- updated-dependencies: - dependency-name: google.golang.org/api dependency-version: 0.293.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-08-17 13:15:45 -07:00
3 people	42699b0f98	build(deps): bump github.com/aws/aws-sdk-go-v2 from 1.43.4 to 1.43.5 (#10787) ... Bumps [github.com/aws/aws-sdk-go-v2](https://github.com/aws/aws-sdk-go-v2) from 1.43.4 to 1.43.5. - [Commits](https://github.com/aws/aws-sdk-go-v2/compare/v1.43.4...v1.43.5) --- updated-dependencies: - dependency-name: github.com/aws/aws-sdk-go-v2 dependency-version: 1.43.5 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-08-17 10:17:00 -07:00
3 people	52ebc9ea4e	build(deps): bump golang.org/x/crypto from 0.54.0 to 0.55.0 (#10786) ... Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.54.0 to 0.55.0. - [Commits](https://github.com/golang/crypto/compare/v0.54.0...v0.55.0) --- updated-dependencies: - dependency-name: golang.org/x/crypto dependency-version: 0.55.0 dependency-type: direct:production update-type: version-update:semver-minor ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>	2026-08-17 10:16:46 -07:00
Chris LuandGitHub	f4bcec60d7	readme: fold RustFS into the MinIO comparison (#10788) ... * readme: add RustFS to the file system comparison * readme: note RustFS write amplification and rigid layout * readme: correct RustFS version, parity and protocol details * readme: merge the RustFS comparison into the MinIO section	2026-08-17 10:11:48 -07:00
github-actions[bot]	f04da8e9ad	4.42 4.42	2026-08-17 07:15:32 +00:00
Chris LuandGitHub	5c43c03b76	filer: restore a folder that received an entry while it was deleted (#10783) ... * filer: restore a folder that received an entry while it was deleted The empty-folder cleaner checks that a folder is empty and then deletes it, and those two steps are not atomic. An entry created in between survives the delete but loses the directory holding it: still readable by its own path, yet absent from every listing until a later write happens to recreate the parent. Record the folders deleted in each pass and re-check them on the next one, putting back any that turned out to hold entries. The check waits a pass on purpose - a writer looks up the parent before inserting the child, so checking straight after the delete can still run ahead of the insert and see nothing. Restoring a directory that holds entries is always correct, and restoring one whose entry went away again just leaves an empty folder for a later pass to collect, so the repair needs no locking or coordination. Claude-Session: https://claude.ai/code/session_01HdLXMUopwgofPb1ZEmiE6r * filer: keep failed restores queued and inherit the ancestor's ownership Two gaps in the restore pass. A folder whose count or restore hit a transient store error was dropped from the tracking list and never looked at again, leaving its entries out of listings until some later write recreated the folder - the very thing the pass exists to avoid. Put those back for the next pass, still under the cap. A restored folder was minted with a fixed mode and no owner, so a directory that had been private came back world-readable and owned by root. Take the mode and ownership from the nearest ancestor still present instead. Claude-Session: https://claude.ai/code/session_01HdLXMUopwgofPb1ZEmiE6r * filer: let the redis stores keep a directory listing that still has entries On the redis stores the listing is not derived from the entries, it is the only record that they sit under that directory. DeleteEntry opened by dropping it outright, so an entry that arrived after the caller judged the directory empty lost its membership and became unreachable: readable by exact path, absent from every listing, and invisible to any later check, since counting the directory reads the listing that was just destroyed. Nothing could detect or repair it. Drop the listing in DeleteFolderChildren instead, alongside the children it describes, and leave it alone in DeleteEntry. redis3 needs it explicitly, since removeChildren clears the skip list nodes but not the list itself, and the plain redis store was leaking the key entirely. Claude-Session: https://claude.ai/code/session_01HdLXMUopwgofPb1ZEmiE6r * filer: restore folders with their own attributes, and observe them for a window Five gaps in the restore pass. The restored directory was reconstructed from whatever ancestor happened to still be present, and the mode was ORed with 0111 on the way. A private directory under a world-traversable parent came back granting traversal it had denied. Read the folder's own attributes before deleting it and put exactly those back. That also removes the ancestor walk, which treated a transient store error as "not found" and silently fell through to a broader ancestor. A single check a pass later was not a delay at all. Ticker sends coalesce, so when a pass runs long the next one starts immediately, and a writer already past its parent lookup can insert after the check has read zero - after which the folder was discarded for good. Keep each folder under observation for a bounded wall-clock window and re-check it on every pass until it expires. This narrows the exposure rather than closing it; only making the emptiness check and the delete atomic would do that. A delete that returned an error was never observed at all, though the redis stores drop the folder before its parent-list member, so a failure return is not proof the folder survived. Record the folder before the delete instead. Restores now run shallowest first, so a folder taken by the parent cascade is rebuilt with its own attributes before anything below it needs it as a parent. Claude-Session: https://claude.ai/code/session_01HdLXMUopwgofPb1ZEmiE6r * filer: recover a deleted folder from the create event for the entry that raced it Checking each deleted folder on a timer was the wrong instrument. It cost a listing per folder per pass, and it could only ever be a guess about when the racing write would land. The metadata stream already carries the answer. A folder is recorded before it is deleted, so any entry that can be orphaned is created after that record and its create event names that exact directory. Match the event against the recently deleted folders and the folder is known to need putting back, rather than inferred to. The window stops being a guess at the race and becomes what it should be: how far behind the event stream is allowed to run before a folder stops being watched. Listing is now done once, for a folder an event has already named, to skip the restore when the entry has since gone away again. Claude-Session: https://claude.ai/code/session_01HdLXMUopwgofPb1ZEmiE6r * filer: bound how long a folder is watched, and rebuild ancestors from themselves Four gaps found reviewing the restore pass. A folder whose restore kept failing was never let go: the written-to check ran before the age check, so it was picked up, retried, put back, and counted again on every pass for the life of the process. Apply the window first, whatever state the folder is in. At the cap, the folder being recorded was the one turned away, though it is the one whose race is still live - the older entries are already close to ageing out. Give up one of those instead, picked as the oldest of a small sample so the cost stays flat under heavy deletion rates. An ancestor taken by the same cascade was left to the descendant's restore to recreate, which minted it from the descendant's attributes and handed back access the ancestor never granted. Rebuild those from what they were, ahead of anything below them. Reading a directory's attributes assumed an entry came back. Some stores return nothing with no error, so treat that as not found. The mode is also taken whole rather than through Perm(), which was dropping setgid, setuid and sticky. Claude-Session: https://claude.ai/code/session_01HdLXMUopwgofPb1ZEmiE6r * redis3: take a directory listing left behind by a failed delete Removing the last name deletes the list, and if that delete fails the header survives pointing at a name that is gone. The retry finds nothing to remove, reports no changes, and returns before reaching the delete, so the key stays for good. Take it on that path too. Claude-Session: https://claude.ai/code/session_01HdLXMUopwgofPb1ZEmiE6r	2026-08-17 00:04:07 -07:00
Chris LuandGitHub	f530102c45	filer: do not sweep children when deleting a folder non-recursively (#10782) ... * filer: do not sweep children when deleting a folder non-recursively doBatchDeleteFolderMetaAndData lists a folder and bails out if it has any children, then calls Store.DeleteFolderChildren unconditionally. On the non-recursive path that bulk sweep has nothing legitimate to remove: it only runs once the listing came back empty, so the sole rows it can delete are ones inserted after the check. The S3 empty-folder cleaner deletes through this path, so a PUT landing between the listing and the sweep loses its entry after the write was already acknowledged. Neither side sees an error - the client has its 200 and the cleaner logs an ordinary empty-folder deletion - and the chunks leak, since the cleaner passes shouldDeleteChunks=false and nothing was enumerated to collect. Workloads that scatter objects over many shallow prefixes empty and refill those folders constantly, which is what makes the window reachable. Sweep only when the delete is recursive, or when the whole-bucket shortcut skipped the listing and depends on it. Claude-Session: https://claude.ai/code/session_01HdLXMUopwgofPb1ZEmiE6r * filer: pin the folder entry removal left by the racing-child test The surviving entry is reachable by path but drops out of listings until the folder comes back, and nothing in the test said so. Assert it, so the exposure that remains after this change is visible rather than implied. Claude-Session: https://claude.ai/code/session_01HdLXMUopwgofPb1ZEmiE6r	2026-08-16 22:09:23 -07:00
Chris LuandGitHub	7522e17b6d	iceberg: vend table-scoped credentials to clients that ask for delegation (#10777) ... * iceberg: vend table-scoped credentials to clients that ask for delegation The catalog recognised X-Iceberg-Access-Delegation: vended-credentials and then deliberately said nothing, because it had nothing to vend: it withheld even the S3 endpoint so the client would keep the credentials it was configured with. That left every engine expecting the catalog to hand out access - Snowflake, Databricks, Trino with vending, any multi-tenant setup - needing static S3 keys distributed out of band. Mint an STS session per request instead, scoped by a session policy to the table's own prefix plus the bucket listing needed to resolve it, and return it in the load response config and storage-credentials. The role to assume is named by -s3.iceberg.credentialRole; its trust policy is what decides whether a caller may assume it, and vending stays off until it is set. A failed mint falls back to the old silence rather than handing back an endpoint the client cannot sign for. * iceberg: keep vended credentials inside the table prefix Review follow-ups on credential vending: Listing was granted on the bucket ARN with no condition, so a credential vended for one table could enumerate every other table's object names. Constrain s3:prefix to the table's own prefix, which the S3 gateway already populates for list requests. A table location carrying * or ? would have gone into the policy's resource pattern unescaped and widened the session to sibling prefixes. Refuse to vend for such a location rather than escaping it; nothing the catalog generates contains those characters. DurationSeconds skipped the 900..43200 bounds the other assume-role paths enforce, so -s3.iceberg.credentialDurationSeconds could ask for a session outside them. The check is now shared by all three entry points. * iceberg: return the vended credentials from buildFileIOConfig itself buildStorageConfig was a second name for what buildFileIOConfig already did; it now returns the storage credentials alongside the properties, and callers that only want the properties drop them. * iceberg: split the vended bucket grants, and refuse a whole-bucket scope The prefix condition sat on a statement that also granted GetBucketLocation and ListBucketMultipartUploads, neither of which carries an s3:prefix to satisfy it, so both were denied for every vended credential. GetBucketLocation moves to its own unconditioned statement. ListBucketMultipartUploads is dropped: Iceberg writers complete and abort by upload id, and granting it either leaks in-flight keys bucket-wide or breaks on the same missing prefix. A table whose location has no prefix - one registered at the bucket root - would have been vended read and write over every other table in the bucket. Refuse, the way a location with wildcards is refused.	2026-08-16 12:57:12 -07:00
Chris LuandGitHub	ec37ef5aaa	iceberg: add view rename, scan-report and snapshots=refs to the catalog (#10776) ... * iceberg: add view rename, scan-report and snapshots=refs to the catalog Three gaps against the REST spec that clients hit in normal use: Views had no rename, though tables did and views are stored the same way, so the move is the same catalog-only pointer move. Tables and views share a namespace directory, so both renames now refuse the other kind instead of moving it. Engines POST a scan or commit report after planning; a 404 there turns into an error line per query. Accept the report and discard it - the catalog keeps no metrics store. LoadTable ignored ?snapshots=refs and always returned the whole snapshot history, which is what clients use the parameter to avoid on long-lived tables. * iceberg: authorize view rename against the view ARN, tighten the metrics endpoint Review follow-ups: The shared rename checked the source against a table ARN whatever the kind, so a policy scoped to a view's own ARN never matched and one written for a table ARN was evaluated for a view. The entry kind now carries the ARN builder. The metrics endpoint truncated a report at 1 MiB and then failed to parse it, answering 400 for a query that had actually succeeded. Read one byte past the limit to tell "fits" from "cut short", and discard an oversized report instead of rejecting it. Empty bodies and reports without a report-type are now rejected, which the REST schema requires. ?snapshots= is defined for LoadTable, so it no longer filters what CreateTable echoes back.	2026-08-16 12:56:45 -07:00
Chris LuandGitHub	d044839ab2	iceberg: make a table commit a compare-and-swap (#10775) ... * iceberg: make a table commit a compare-and-swap The catalog validated the caller's version token, ran its authorization checks, and only then wrote the new metadata xattr. Two engines committing against the same base both passed that check and both wrote, so the second silently dropped the first one's snapshot. Both also derive the same v{N}.metadata.json name and the file write overwrote, leaving the surviving pointer aimed at the loser's metadata - and the loser's conflict cleanup then deleted the winner's file. Write the metadata file with an exclusive create and update the xattr conditionally on the bytes the handler read, the way the maintenance worker already commits. A writer that lost the race re-reads and retries, and reports 409 CommitFailedException once out of attempts. * iceberg: stage a commit under a unique name when the versioned one is taken Two follow-ups from review of the commit compare-and-swap: Refusing to overwrite v{N}.metadata.json also refused to get past a file left behind by a commit that died between staging and updating the pointer. Every later commit derived the same name, saw the collision, and reported a conflict, so the table stayed uncommittable until an orphan sweep removed the file. Stage under v{N}-{uuid} instead: neither writer's file is overwritten and the catalog pointer still decides who won, which is how the maintenance worker has always staged its own metadata. metadataVersionFromLocation learned to read the version back out of that name. The conditional update guarded only the metadata attribute while the write replaced the whole entry, so a policy or tag written in the same window was silently reverted. Guard every catalog attribute, which turns that into a conflict the caller retries on fresh state. * iceberg: give saveMetadataFile the exclusive flag instead of a second name saveNewMetadataFile, saveMetadataBlobExclusive and uniqueMetadataFileName were three new names around one existing helper. The flag now rides on saveMetadataFile and saveMetadataBlob, and the unique-name construction sits where it is used. * iceberg: reuse the filer CAS helpers #10773 added, and stage transactions exclusively #10773 landed mutateEntryExtended, which already writes an entry back under a whole-entry precondition and retries. Drop the helper this branch added and route the table commit through it: the check that the metadata is still the one this request read now lives in the mutation, where it sees current state. The policy the request was authorized against is asserted too, so an administrator restricting it mid-commit sends the caller back through authorization instead of having a stale decision applied. Bucket and namespace policies live on other entries and a single-entry precondition cannot cover them. Multi-table transactions stage their metadata exclusively for the same reason single-table commits do, and carry the name they landed on into the pointer flip.	2026-08-16 12:55:42 -07:00
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.	2026-08-16 12:06:43 -07:00
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.	2026-08-16 10:48:19 -07:00
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.	2026-08-16 10:36:59 -07:00
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.	2026-08-16 09:16:05 -07:00