Conversation
ps-reclaim 0.1.4 closes a use-after-free. A retirement published between the
participant scan and the extraction of garbage was judged against a decision
taken before it existed, so a live reader's object could be reclaimed under it.
The fix is a sequence cutoff captured under the garbage mutex before the scan.
It also stops bounded drains being quadratic: `extract_if(..).take(k)` compacts
the unchecked tail on drop, so a 128,000 backlog at `advance_up_to(256)` moved
23.29 ms of records under that mutex and now moves 1.28 ms.
Both were already being picked up by resolution, because `^0.1` allows them.
Raising the floors says they are required rather than merely permitted, which
is what a correctness fix means.
arctic 0.1.11 is the release where `smr-ps-reclaim` stopped implying `std`.
It changes nothing here - WorkTable links `std` and takes ps-reclaim directly
with default features, so feature unification gives it `std` either way - but
the floor is what lets a no_std consumer downstream rely on it.
cargo test 927 passed, 0 failed
cargo clippy --all-targets clean
beta.19 changed the on-disk format and every `.wt.data` on this machine had to be thrown away and rebuilt on 6 September 2026, because nothing could read the old shape. That is a regeneration event, and the reason it happened is that a data page cannot be read without the index that points into it. This test states the requirement for beta.20 as an executable assertion rather than a paragraph in a release note: a page that describes itself can be read by a reader that has never seen the writer's index.
The CIDR submission deferred the lock-discipline scaling comparison, crash consistency, protocol checking, the cost of monomorphization, and baselines beyond redb and LMDB. This plans the paper those deferrals point at, and pins each candidate contribution to the code and the measurements that back it, so the writing starts from what has landed rather than from an outline.
`chunks_exact` with a constant chunk size is a lint on a newer clippy than the one installed here, so the local run was clean and CI was not. `as_chunks` also gives fixed-size arrays rather than slices, which is what the loop wanted.
`scripts/ci-local.sh` runs `cargo fmt --all --check` as its first step and CI did not, which made the local script stricter than CI rather than equal to it, so formatting drift could reach master unnoticed. Same command, same arguments. Measured before adding it: the tree already passes, exit 0 with no diffs, so this job is green as written rather than a red first run to clean up.
The log carried beta.18 and beta.19 and nothing before them, so the history a consumer needs in order to judge an upgrade was only in the commits. Entries are derived from this repository's own history.
`BatchOperation::validate` refuses an event stream with a hole in it. A hole is normally transient, and one that survives the whole deferral budget is not, but the stall message could not tell the two apart: it reported the range and nothing else. Two very different bugs produce that line. Either an id was assigned by the index and its event was dropped instead of queued, in which case nothing will ever deliver it, or the operation was queued and batch collection keeps assembling batches that exclude it. Distinguishing them needs a record of what was queued. Assignment happens inside the index and cannot be hooked from here, but every event reaching persistence passes through `Queue`, so an id present in the stream and absent from the ledger was assigned and never queued. That is the leak signature. Producers are named by `std::panic::Location`, which is why `push_at` takes the caller location and why the old `push` and `push_many` wrappers are deleted rather than kept: calling them loses exactly the location the ledger exists to record. This observes and does not fix. `docs/TODO.md` records the three leak sites the instrument points at, all in generated code: the `NotFound` arm of the update query returns without building an acknowledge while its `AlreadyExists` sibling builds one, and two `res?;` sites in update and delete propagate before acknowledging. The TODO's congee section is corrected while here. The crossbeam-epoch port was already done in congee-wt 0.4.4, and it was a design change rather than a rename: per-tree `Domain`, batched retires, guard provenance checked, and a new `Guard` that is `!Send` and tree-scoped, which constrains future callers.
The ledger from the previous commit points at three sites in generated persisted query code where an event the index had already assigned an id to was dropped instead of queued. A dropped event is a hole `BatchOperation::validate` refuses forever, so the table stalls and the message names a range rather than a cause. All three now do what the rollback arms already did: build an `Acknowledge` carrying the orphaned events and apply it before propagating. The `IndexError::NotFound` arm of the update insert path acknowledges the events its sibling `AlreadyExists` arm was already acknowledging, and the two bare `res?` sites, in `gen_process_diffs_remove_on_index` and after `delete_row_cdc`, became `if let Err(e) = res` blocks that acknowledge and then return. The events are moved into the acknowledge rather than cloned, and that is load-bearing rather than a preference. Cloning does not compile: the events type is still an inference variable at that point, pinned only by the `op.extend_secondary_key_events` call further down, and method resolution for `.clone()` needs the type known where the call is written. It fails as an `E0282` reported against the `worktable!` invocation with no inner span, which is expensive to diagnose. The comment in `docs/TODO.md` records that so nobody pays for it twice. The write failures are not forcible through the public API, so the wiring is pinned on the emitted tokens the way the surrounding tests already do it. Both assertions check the acknowledge is emitted *before* the return or the extend, not merely that one appears somewhere in the output. The in-memory generator has the same two `NotFound` arms and is deliberately untouched: there is no persistence stream behind them to gap.
`DataPages::pages` was a `Vec` behind an `ArcSwap`, and both append paths cloned the whole thing to add one page. Every page in it is an `Arc`, so the clone was one atomic increment per existing page, each touching a separately allocated page header: a cache miss apiece. Appending page N cost O(N), and filling a table cost O(N^2). It hid behind row size, because row size is what decides how many pages a table has. At 256 bytes a page holds sixty-odd rows and twenty thousand rows is three hundred pages, where the copy is invisible and per-row cost is flat. At 4 KiB a page holds three, the same rows are six thousand pages, and per-row cost climbed from 2.95 to 30.08 microseconds across the load. The pages now live in fixed-size chunks, so an append copies one chunk and the spine of chunk pointers rather than every page. Readers still take an `ArcSwap` snapshot and never block, and page access still goes through the directory first, so the read path is unchanged. Measured over twenty thousand 4 KiB rows, per-row cost goes flat - 1.53 to 1.95 microseconds against 2.95 to 30.08 - and inserting into an unpersisted table goes from 254 MB/s to 3062. A persisted table gains far less, 208 to 279 MB/s, because with the copy gone persistence is what the load now waits on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The directory holds stable page pointers so a point access does not take an `ArcSwap` snapshot, and it reached 64 * 64 = 4,096 pages, which is 64 MiB at the default page size. Past that `publish` returned early and every access fell back to the snapshot. A table of 4 KiB rows crosses that after twelve thousand rows, which is not a large table. Raising it to 1,024 roots reaches 1 GiB for an 8 KiB array of pointers. On its own it made no measurable difference to insert cost, because the page list copy was the term that mattered; this moves a ceiling rather than removing a cost, and it is committed separately so the two are not confused. The tests are the harness the page list work was measured with: whether insert cost scales with row size, whether it grows as the table fills, what the floor is with few pages, and what the persistence path sustains end to end. They are all `#[ignore]`d, since they are measurements and not assertions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An insert that fits in the page already open and one that has to add a page are different operations with different costs, and averaging them hides the second behind the first. How much it hides depends on row size: at 256 bytes one insert in sixty allocates, so allocation is a tail event, while at 4 KiB one in three does and it is a third of all traffic. Split by whether the page count moved, the page list change reads as what it is - a tail-latency fix that leaves the common path alone. On 4 KiB rows the allocating population goes from 59.67 to 7.08 microseconds at p50, 133.38 to 14.46 at p99, and 451.54 to 42.33 at its worst, while the existing-page population does not move. On 256-byte rows the same change is worth about a third, on the one insert in sixty that pays it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The chunked page list handed back an owned `Arc` from `get`, which costs an atomic increment and the matching decrement on drop. The link-based read is a read path and needs the page only for the length of one call, so it paid both for nothing. It measured. A delete went from 665 to 751 ns, while a select by primary key - which goes through the page directory and never touches this - did not move. Borrowing through `with_page` puts delete back at 654 ns, and the criterion cases either side of it are unchanged or better: insert 442 -> 86 ns, select by primary key 21.0 -> 20.1 ns. Found by running the benchmark suite that already existed rather than the ad-hoc probes the rest of this work was measured with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
It was named "bulk load", which reads as loading a table from disk. It inserts 25,000 rows. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collapses what was PR #102 onto the arctic, event-ledger and page-list work already on this branch, so WorkTable carries one pull request. #97, #99 and #100 are folded in; #58 is not, being 224 commits behind master and already conflicted, which is its own job. `fsx` and every persistence signature go through `nagoya::io`, so the production path names no runtime. A persisted table can set `page_size`, which was refused before because the seeks computed offsets from a crate constant while the generated table threaded the configured one. Four places where the folded branches disagreed, each resolved by keeping both rather than either: * `arctic` and `ps-reclaim` keep #97's raised version floors and gain the `default-features = false` the no_std work needs. * `allocated_bytes` keeps the page list as #100 left it and takes `core::mem` from the no_std work; the `.load()` in the older branch belonged to a shape #100 replaced. * `batch.rs` and `task.rs` take the `core`/`alloc` imports, plus the `Arc` and `Location` the no_std lists had dropped and the code still uses. * The s3 test goes back to tokio's extension traits. It talks to a tokio `TcpStream` it starts itself, so the sweep that took the storage path off tokio should never have touched it. `event_ledger` is new here and was written against `std`. Its bookkeeping moves to `core` and `alloc`; only the two parts that genuinely need an operating system are gated, the backtrace capture and reading `WT_EVENT_LEDGER`, so without `std` the ledger is simply never enabled. `futures/std` goes in this crate's own `std` feature rather than on the dependency line, where a `--no-default-features` build would still have turned it on. cargo test --workspace --all-targets 933 passed ... --all-features 935 passed ... --features versioned-row-publication 1029 passed cargo clippy --workspace --all-targets [--all-features] clean cargo check --no-default-features clean
`cargo check --no-default-features` passing said only that this crate's own
source is `std`-free. Its closure was not: `cargo tree
--no-default-features -e normal -i tokio` showed tokio linked with every
feature off, because none of the uses were ever gated. That is the same
shape as the `futures-io` mistake, where a crate compiled and exported
nothing.
Twelve production uses become one:
tokio::sync::RwLock x10 nagoya::sync::RwLock
tokio::sync::Semaphore x4 nagoya::sync::Semaphore
tokio::sync::Notify nagoya::sync::Notify
tokio::time::sleep x5 nagoya::sleep
tokio::task::yield_now nagoya::yield_now
tokio::pin! core::pin::pin!
tokio::select! (vacuum) nagoya::timeout
tokio::spawn (vacuum) crate::runtime::background().spawn
The vacuum `select!` was a timeout wearing a combinator: two arms racing
the waits against a fallback sleep. Saying `timeout` says the intent.
**The vacuum sweep stays inside the table**, so the spawn could not be
pushed onto the caller. The table starts two background threads of its
own, lazily, gated on `std`; without `std` the module does not exist and
neither does the sweep, which is the honest outcome for a build with no
threads rather than linking a runtime to pretend otherwise.
One production use remains, the persistence engine's `tokio::spawn`. It
needs `Debug` and an `&self` `abort` on nagoya's `JoinHandle`, which
currently consumes via `cancel`, plus the `select!` at task.rs:1793. Until
it lands tokio is still linked and the `no_std` claim stays qualified.
Not yet done: the vacuum tests have not been run against this scheduling
change, and three of them still call `.abort()` on the returned handle.
WorkTable declared `parking_lot = "0.12"`, plain upstream, while every other consumer in this house takes `parking_lot_lite_hack`. Renamed through `package`, so no call site changes. It pins the `feat/fair-mutex` branch rather than the published 0.12.7, because the published one does not have what this crate uses. Its own module comment says so: "What is gone is `Condvar`, `Once`, `FairMutex`". `empty_link_registry` needs `FairMutex` for `op_lock` and `targeted_pages`. **This leaves two copies of the crate in the lock** and that is not a resting place. WorkTablesIndex 0.0.13 takes the registry 0.12.7 while this takes the branch. Publishing `feat/fair-mutex` as 0.12.8 collapses them; until then a build carries both. Vanilla `parking_lot` now arrives only through tokio, so it leaves when tokio does.
`cargo tree -e normal -i tokio` now prints nothing, with default features and without. It printed a tree before, in both, which is what `cargo check --no-default-features` could never have told us: that check only proves this crate's own source is std-free, never its closure. Three things were holding it, and the engine was the smallest of them. The persistence worker spawned onto whatever ambient runtime the caller happened to be on. It runs on `nagoya::runtime::background` now, one pool per process, behind `std` so a --no-default-features build cannot silently acquire threads. Unlike the vacuum sweep, this one genuinely needs a thread: a flush loop drains a queue, so there is no foreground task it could be folded into. `wait_for_ops` raced a notify against a sleep, which is a timeout. And the part that actually kept tokio in every consumer's graph: `worktable!` emitted six `tokio::` paths into the crates it expands in, making a whole runtime part of the macro's contract whether or not the consumer ran one. Those go through `worktable::prelude` now, the same way generated code already reached `fsx`. Note what stops being caught: tokio's `JoinError` reported a panicking worker, and nagoya's handle does not, because the panic propagates out of the await instead. KNOWN FAILING, and pre-existing: `generation_swap_requirement` now fails racily. Not caused by this commit. At the parent commit, with tokio untouched, changing only `#[tokio::test]` to `flavor = "multi_thread"` reproduces it exactly. `#[tokio::test]` defaults to current_thread, so `tokio::spawn` had been putting the worker on the test's own thread, where it could only run at the test's await points and every insert pushed its whole CDC event sequence before the worker looked. 364 of this suite's tokio tests are current_thread against 34 multi_thread, so the persistence suite has been blind to concurrent producer and consumer throughout. Fixed separately.
It came through tokio. With tokio gone it comes through indexset 0.15 behind the vanilla-index default feature, so it is absent from a --no-default-features build and present in a normal one. The old comment would have had a reader expect it to leave on its own.
… "nothing yet" Two separate findings, one of which is the fix. The fix: `a_retired_generation_releases_its_memory` and `a_generation_can_report_what_it_holds` both used a single `DIR` const, each removing and recreating it on entry. The harness runs tests on parallel threads, so two tables attached to one set of files and filled them at once. Each table's event ids start at 0, so the loser saw the other's event 2 land on a node its own writes had already advanced to key 28, and reported a corrupt index. One directory per test. This is why `generation_swap_requirement` began failing when the persistence worker moved off `tokio::spawn`: it had been running on the test's own current_thread runtime, so the two tests' writes never actually overlapped in time. The engine was not at fault and neither was the move. The second finding is real but was not the cause, and is kept because it is proven separately. `LastEventIds` stored an id where it needed an `Option`: event ids start at 0 and `IndexChangeEventId::default()` is also 0, so "nothing applied yet" and "applied event 0" were the same value. The gap check could not ask a first batch whether it followed what came before, and exempted it. A first batch of ids 3.. is internally gapless, so nothing else rejected it either: it would be applied, advancing node maxima past events that had not arrived. Two tests cover it, and they fail with the exemption restored: a first batch skipping the head of the stream defers, and one starting at event 0 still applies rather than deadlocking on the ambiguity the exemption existed to avoid. The missing-page error now names the event id and the identity it wanted. The counts alone said a lookup failed and nothing about why; the id and key together are what distinguished these two causes.
This was referenced Sep 9, 2026
Owner
Author
|
Folded into #105 as commits 15 to 19, unchanged. #105 is based on master and contains this whole stack, so there is one place to review. One thing found while folding it in: cargo fmt --all --check was failing on this branch, and CI runs it as its own job. src/persistence/operation/batch.rs and tests/worktable/bench.rs, the second because the import moved when the crate it names changed from tokio to nagoya. Fixed in #105. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
cargo tree -e normal -i tokionow prints nothing, with default features and without. It printed a tree before, in both, which is whatcargo check --no-default-featurescould never tell us: that check only proves this crate's own source is std-free, never its closure.Three things held it, and the engine was the smallest.
The macro was the big one.
worktable!emitted sixtokio::paths into the crates it expands in, so a whole runtime was part of the macro's contract whether or not the consumer ran one. Those go throughworktable::preludenow, the same way generated code already reachedfsx.The persistence worker spawned onto whatever ambient runtime the caller happened to be on. It runs on
nagoya::runtime::backgroundnow, one pool per process, behindstdso a--no-default-featuresbuild cannot silently acquire threads. Unlike the vacuum sweep this one genuinely needs a thread: a flush loop drains a queue, so there is no foreground task to fold it into.wait_for_opsraced a notify against a sleep, which is a timeout.Note what stops being caught: tokio's
JoinErrorreported a panicking worker and nagoya's handle does not, because the panic propagates out of the await instead.Two fixes that came out of it
generation_swap_requirementbegan failing, and it was pre-existing. At the parent commit with tokio untouched, changing only#[tokio::test]toflavor = "multi_thread"reproduces it exactly. Two tests shared oneDIRconst, each removing and recreating it, and the harness runs tests on parallel threads: two tables attached to one set of files.tokio::spawnhad been hiding it by putting the worker on the test's own current_thread runtime. One directory per test now.Separately,
LastEventIdsstored an id where it needed anOption. Event ids start at 0 andIndexChangeEventId::default()is also 0, so "nothing applied yet" and "applied event 0" were the same value, and the gap check had to exempt the first batch. A first batch of ids 3.. is internally gapless, so nothing else rejected it either. Two tests cover it and both fail if the exemption is restored.State
933 tests pass, clippy clean with
--all-targets --all-features.Worth knowing for review: this suite has 364 current_thread tokio tests against 34 multi_thread, so the persistence path has never been exercised with a genuinely concurrent producer and consumer, which is the only way a real consumer runs it.