3044a38d1c1d3f370bc6b96edce333c1bba39b8e
115 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
A.Shakhmatov
|
3044a38d1c |
tests/spec: an aggregation corpus, recorded from mongod
M2.5's gate, built before the milestone it gates -- the same order that put
`expectEvents` before the free list in M1 and Tier 0 before everything in M2.
`mongodb/specifications` has no aggregation suite, which is amendment A6's
central finding, so this milestone has to bring its own. The hazard in a corpus
we author is obvious and fatal: it can encode our own bugs as expectations and
then agree with us forever. So the split is enforced by the tooling.
`sources/*.json` holds documents and pipelines and nothing else; `record.js`
asks a real mongod 8.3.7 what each pipeline answers and writes the unified-
format file from the reply. Inputs authored, expectations measured -- the
discipline that corrected three assumptions in M1's session work and every
error code in M2, where the alternative would have shipped both times.
No second runner. `run.js --suite-dir` points the existing one somewhere else,
so the entity model, the matchers, the skip accounting and `expectEvents` come
for free; a second runner would drift from the first exactly where it mattered.
`--scorecard` is refused with `--suite-dir`, because `scorecard.txt` is the crud
corpus's record and the milestones are compared against it -- writing it from an
unrelated run would replace that record silently.
Errors record the code and not the message: message text is mongod's to change
between releases. Group pipelines end in a `$sort`, because group output order
is unspecified and a case depending on it would fail for the wrong reason on
either server.
The first source covers `$group`: nine accumulators including the edge cases
that decide an implementation -- `$avg` over a group whose values are not
numbers, `$min` of a field no document has, `$push` skipping a missing field,
`$first`/`$last` against input order, grouping on an array, a compound `_id`.
Where it starts, run against the M2 tip:
group-accumulators.json 9 pass 10 fail 0 skip
The nine include the four refusals M2 added, which answer with mongod's own
codes -- so the corpus already confirms that half. The ten are the milestone.
The crud corpus is unchanged at 201/90/196.
|
||
|
A.Shakhmatov
|
8ebeb9d4ec |
commands: $out and $merge, written by the dispatch epilogue
The seven reachable failures of M2, and the first commit of the milestone to move the scorecard: 194/97/196 -> 201/90/196, with `aggregate-*.json` going 9 pass / 13 fail to 16 pass / 6 fail. The seven that moved are exactly the seven priced as reachable, and the six that remain are exactly the six attributed to M2.5 ($addFields, the expression engine), M4 ($listLocalSessions) and M8 (collation). Both stages write to a collection the pipeline is not reading, and three things stood against doing that in the handler: `aggregate` is a `.read` command, dispatch takes locks from a static table keyed on the command name before the handler runs, and `Collection.lock` allows exactly one collection lock at a time. So the handler computes the output under the locks it has and leaves it in `Context.pending_write`; the epilogue applies it with nothing held, beside the commit and the checkpoint already there. The `.read`/`.write` contract is amended in its own comment rather than quietly broken. `pending_write` is cleared at the top of every dispatch, so a handler that errors before setting one cannot leave the previous command's write to fire. A failed write replaces the pipeline's `ok: 1` with the failure, because a client told the aggregation succeeded would believe the collection had been written. What the stages do not implement is refused, not ignored: `$merge`'s `whenMatched`, `whenNotMatched`, `on` and `let` all select behaviour this server does not have, and a `whenMatched: "fail"` that silently merged would be the same lie Tier 0 spent three commits removing. Codes measured against mongod 8.3.7. `$out` and `$merge` answer byte-identically to it on both the replace and the upsert case. NOT ATOMIC, and said out loud in the code rather than left to be discovered. mongod replaces an `$out` target atomically; this engine has no cross-collection atomicity and no rename to build one from, so a crash between the drop and the last insert leaves the target holding part of the new output where MongoDB would leave the whole of the old. The fix is write-to-temp-and-rename and rename is a command that does not exist here. The test's mutation is the argument for the epilogue in one line: apply the write inside the `$out` branch and it deadlocks rather than fails. 191/191 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2 concurrent, e2e3 16, e2e4 17, e2e6 72, e2e7 86. |
||
|
A.Shakhmatov
|
89eae1cd9c |
plan/docs: the M2 gate is not reachable as written either
Priced the 13 aggregate failures one by one instead of counting them. Six
cannot turn green in M2 whatever is built: three need $listLocalSessions (M4)
*and* $addFields (M2.5), two need the expression engine, one needs a collation
(M8). So the gate reads '0 fail among the seven reachable', with the other six
named and attributed.
Note what the three db.aggregate() cases actually need. Implementing
{aggregate: 1} moves none of them, because each then fails on
$listLocalSessions instead. The review priced that line 'orthogonal, and
cheap'; it was cheap and worth nothing.
The seven that remain are $out and $merge, and they need a decision the review
had no authority to take. They write to a collection the pipeline is not
reading, and three things stand against that: aggregate is declared .read and
the dispatch contract says only .write commands may mutate; dispatch acquires
locks from a static table keyed on the command name, before the handler runs;
and Collection.lock's own comment states the invariant that decides it -- never
more than one collection lock at a time.
That is the same objection that kept $lookup out of M2.5's first cut, and the
review failed to apply it to the write stages in the same breath. Recorded as
the review's own error rather than quietly corrected. Two options set out in
§7, plus the durability question neither of them answers: mongod's $out
replaces the target atomically and this engine has no cross-collection
atomicity.
Nothing past Tier 0 is implemented until one is chosen.
|
||
|
A.Shakhmatov
|
61ce9805c7 |
commands: $project is a stage, not a note about how to print the answer
It set a variable that the emit applied once. Three consequences, all measured
on a live server before this changed rather than read off the code:
- only the *last* `$project` in a pipeline had any effect;
- a `$match` after one still matched on the field it had removed;
- `{y: {$literal: 5}}` read as falsy, which flipped the whole projection into
its exclusion branch and returned every document minus `y`, where mongod
adds a computed `y`.
Now it transforms the stream where it stands, materializing into the same tree
form `$group` already produced. `$match`, `$sort` and `$group` after it read
what it left, which is what "stage" means.
What it cannot do is refused rather than mis-read. A computed field needs the
expression evaluator M2.5 brings, and a nested spec (`{a: {b: 1}}`) needs a
narrowing `query.project` does not do -- both were falsy, and falsy is the
answer that quietly returned the whole document. Codes read off mongod 8.3.7,
and the replies now match it on code, codeName and message text for the empty,
the mixed and the computed forms:
projection must have at least one field 51272 Location51272
cannot mix inclusion and exclusion 31254 Location31254
unknown expression in $project 31325 Location31325
Recorded, not fixed here: `query.project` is shared with `find`'s `projection`,
where a nested inclusion has the same falsy reading and the same wrong answer.
That is a real gap rather than an unimplemented feature -- narrowing is
something this projection should do -- so it is its own fix, not a refusal, and
it belongs with whichever milestone takes `find`'s projection seriously.
Scorecard unchanged again at 194/97/196. The corpus has no `$project` stage
tests either; A6 said so, and this is the second commit to confirm it.
Mutations: drop the refusals and the empty projection answers `ok: 1`; put the
projection back on the emit and the `$match`-after-`$project` case goes red.
190/190 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2
concurrent, e2e3 16, e2e4 17, e2e6 72, e2e7 86.
|
||
|
A.Shakhmatov
|
d9772c4ed5 |
commands: a pipeline may group what an earlier stage generated
A remote crash, present before this milestone and reachable by any client with
a two-stage pipeline:
aggregate: [{$group: {_id: "$k", n: {$sum: 1}}},
{$group: {_id: null, g: {$sum: 1}}}]
thread panic: index out of bounds: index 2, len 0
`$group` took `[]const u64` and was handed `offs.items[start..end]` whatever
the stream was made of. A pipeline starts as slab offsets -- matched and
reordered in place, never materialized -- and flips to generated documents the
moment a stage produces something the slab does not hold. After that `offs` is
empty while `start`/`end` count trees, so the slice ran off an empty list and
took the server thread down. There is no authentication in front of it.
Found while making `$project` a real stage, which reaches the same branch;
confirmed against the binary at the previous commit rather than assumed, so
this is a pre-existing defect and not a regression of that work. It is
committed on its own for that reason.
`Stream` names the two forms the rest of the pipeline had been carrying
implicitly in `in_trees`, and `$group` now reads whichever is live. The slab
side stays byte-walked -- grouping a million documents does not build a million
trees to read one field -- and `path_in_pairs` is the tree counterpart of
`query_path_value_bytes` for the other half.
The test groups by a key and then counts the groups, and sums `$n`, a path that
only resolves against the generated document. Its mutation -- hand `run_group`
the offsets unconditionally -- aborts the run rather than failing it, which is
why this wanted a test and not a code read.
Answers byte-identically to mongod 8.3.7 on the pipeline above.
189/189 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e3 16,
e2e4 17, e2e6 72, e2e7 86.
|
||
|
A.Shakhmatov
|
ae48e05a19 |
commands: $group refuses what it cannot compute
M2 Tier 0, and the first commit of the milestone: stop answering wrong numbers
with `ok: 1`.
`$group` had one accumulator, `$sum`, and one expression vocabulary, `$field`
paths and constants -- open-coded twice and applied to whatever arrived.
Everything outside that fell through to a zero. Measured on a live server
before this commit: `{$avg: "$x"}` answered `0`, and so did `$max` and `$push`;
a compound `_id` collapsed every document into one group keyed by the
unevaluated expression; `{$literal: 1}` came back echoed; `{$sum: {$multiply:
[...]}}` was `0`. Six of eight probed pipelines succeeded with a wrong result.
That is a worse failure than an unimplemented one. An unrecognised stage is a
bug report; an `$avg` that returns `0` is a corrupted report nobody files. It
is also invisible to the gate: the corpus this project runs has no aggregation
stage tests at all, which is what PLAN amendment A6 is about.
So the vocabulary is now named -- `GroupExpr` is a path or a constant, and
`classify_expr` is the single place the `$`-prefix distinction is made -- and
every accumulator is validated before a document is read. A pipeline that
cannot be answered is refused whole rather than half-answered.
Five error codes added, every one read off mongod 8.3.7 rather than recalled,
and the replies now match it on code and codeName for all six `$group` shapes
probed:
unknown group operator 15952 Location15952
a group specification needs _id 15955 Location15955
accumulator is not an object 40234 Location40234
two operators in one 40238 Location40238
unrecognized expression 168 InvalidPipelineOperator
`$avg` and friends are reported as *unknown* operators, which is the choice
`location_unrecognized_stage` already made for `$addFields`: this server
reports what it does not implement using MongoDB's own code for "no such
thing", because a code MongoDB never emits would break the error-code parity
every milestone is held to. The message names the construct.
`$sum` over a non-number still contributes nothing -- that is MongoDB's rule,
not a stand-in for an unimplemented one, and the comment says so where it
would otherwise read as another silent zero.
The scorecard does not move: 194/97/196 before and after, byte-identical. That
is not a disappointment, it is the design review's central finding arriving on
schedule -- the corpus cannot see any of this, which is why M2.5 has to bring
its own.
Mutations: drop the accumulator-name check and `$avg` answers `ok: 1` again;
drop the `_id` classification and the compound `_id` does.
188/188 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2
concurrent, e2e3 16, e2e4 17, e2e6 72, e2e7 86.
|
||
|
A.Shakhmatov
|
b481f39ca3 |
docs/plan: all 19 aggregate skips are environmental, not 13
The scorecard collapses a whole-file skip into a single `*` line, so counting the reason lines undercounted the cases they cover. Every one of the 19 is version- or topology-gated, which is why the gate reads *0 fail* rather than *all pass*. |
||
|
A.Shakhmatov
|
7ae81b8179 |
plan: M2 splits in two (amendment A6)
The design review found the milestone's gate names a corpus that is not there: `mongodb/specifications` has no aggregation suite, and the thirteen `aggregate-*.json` files this project runs live inside `crud` and test the aggregate *command*, not stages. One name was covering two milestones. M2 becomes the command surface -- $out, $merge, db.aggregate(), collation, let -- gated on the corpus that already exists, at 0 fail. M2.5 becomes the engine: expression evaluator, per-stage document iterator, accumulators, $unwind; $lookup and $facet explicitly out of the first cut, gated on a corpus this project writes with every expectation measured against mongod. Command surface first, engine second, chosen with the cost stated: the engine is what gives wrong answers now -- six of eight probed pipelines answer ok:1 with a wrong result -- so this order leaves them alive a milestone longer. Which is why M2 carries the refusals: every unimplemented construct stops answering 0 and starts answering an error with a measured code, the same move expectEvents made before the free list in M1. It will push the scorecard down, and that is the point. The review keeps its pre-decision recommendation verbatim, so the argument the decision overrode stays legible. |
||
|
A.Shakhmatov
|
37211a6cda |
docs: M2 design review
PLAN §3 gives M2 one line of scope and one line of gate, and §6 lists the three questions it deferred. This answers them, and reports one finding that has to be settled before the rest is worth discussing: the gate named in the plan does not exist. `mongodb/specifications` has no aggregation suite -- the thirteen `aggregate-*.json` files we run live inside `crud` and test the aggregate *command* surface, not stages. $lookup, $unwind, $facet, $addFields and $replaceRoot appear nowhere in the pinned corpus. Measured, not recalled. Seven stages exist, not the nine an earlier note in this project claimed -- $set and $unset are update operators. There is no expression engine: $field paths are open-coded in two places inside run_group and $sum is the only accumulator. Probed on a live server, six of eight pipelines answer ok:1 with a wrong result -- $avg, $max and $push all return 0, a compound _id groups everything into one bucket keyed by the unevaluated expression, $literal is echoed back, and a $match after a $project still sees the projected-away field. Only $addFields fails honestly. That drives the tiering. M2 is not "add stages to the chain": the stream is a materialized window and each stage moves its bounds, which cannot express 1->N ($unwind), a second collection ($lookup), or sub-pipelines ($facet), and $project is not a stage transform at all. Six tiers proposed, four gate options priced, and a recommendation: Tier 0 (refuse what is not implemented) alone first, for the same reason expectEvents preceded the free list in M1 -- it will move the scorecard down, and that is the point. No decision is recorded in PLAN.md yet; that is what the review is for. |
||
|
A.Shakhmatov
|
a748a3d08c |
db/pager/tests: cleanup pass over the free list
No behaviour is meant to change and the gate confirms it: 1.94x / 679.2 MB
reclaimed / 9 rebuilds and 2.46x / 934.0 MB / 13 on the two 16 KiB lines,
0.0 MB on the 200-byte line, all identical to the numbers recorded for them.
Deduplication. `pages_for` was written in db.zig and again in pager.zig and
twice more inline; there is now one, public, and the two pre-existing copies
call it. `pages_per_map_align` replaces three hand-rolled `map_align /
page_size`. `SlabRun.window_first` was a stored field that could never legally
disagree with `first` and was maintained by hand at two sites -- now a method.
`keep_piece` re-derived `SlabRun.window_count` character for character; it
calls it. `insert_run` scanned linearly for a position `run_of` binary-searches
for, which made loading a fragmented catalog quadratic; both now go through one
`run_lower_bound`. Freeing a run's window map was written four times; one
helper. The 20% rebuild share was stated in `note_compact` and again in
`wants_rebuild`, with a comment arguing at length that they must be the same
number -- `worth_rewriting` makes that structural.
Efficiency. The identity assert in `reclaim_windows` called `dead_located()`,
an O(every window) walk, and `assert_msg` is live in ReleaseFast -- so it
doubled the scan the reclamation was about to make (2.75 MB streamed twice per
reclaiming checkpoint at the 21 GB the gate targets). `dead_located` is now a
maintained counter, the check is O(1) in every build, and the scan cross-checks
it while it is there. `SlabRun.full` lets a run with nothing to give be copied
without its counters being read at all, so the common case is O(runs) rather
than O(windows).
The pager's two allocation policies were hand-copying the claim step, and the
copy had already lost two of the three preconditions -- `alloc_slab_run` never
checked `pages <= reserved_pages`. Both now go through `claim_locked`.
`reclaimed_bytes` moves from Collection to Engine, beside `compactions`, which
is how it is read and the only place it can be honest: a life-of-the-process
total must not lose a dropped collection's share. Both join `Counters`, so
`slab_stats` stops opening `counter_lock` by hand.
The ownership assertion in `write_catalog` was gated on `is_test or Debug`, a
predicate nothing else in the codebase uses, which left the one silent failure
this design can produce unchecked in ReleaseSafe. It is now `!= ReleaseFast`,
the line `protect_stable` already draws. Measured: no change to the suite's
runtime.
Altitude. `note_checkpoint` was called from exactly one place, the tail of
`upsert` -- so a delete armed no checkpoint by any route, which is why
reclamation only ever ran when the *rebuild* trigger fired and the rebuild then
reset the window map it would have used. `remove` and the TTL sweep arm one
now, next to the `note_compact` calls that were added for the same omission a
milestone ago. `compact`'s leading checkpoint stays, demoted in its comment
from the mechanism to the local ordering it actually guarantees.
serverStatus reports `allocTailBytes`/`freeReadyBytes` instead of page counts,
so the harness stops hard-coding 4096 -- the kind of constant this milestone
was blindsided by once already.
tests/e2e/churn.js: `deleteMany({_id: {$in: [5000 ids]}})` exceeded
`index.max_combos`, so the planner refused the index and every delete became a
full collection scan re-filtering each document against 5000 members. That was
the entire runtime of the harness. One delete spec per id instead: the 40k x
16 KiB gate goes 57 s -> 6 s, and the 150k x 200 B line 483 s -> 3 s, with
identical output. Also: the per-round `countDocuments` is gone (the harness
knows the count), and the server log is a bounded ring rather than a rope that
grows with everything the server ever said.
Reverted from the review: reusing one MongoClient across the startup poll. A
client whose first connect fails tears its topology down and every later
command on it fails identically, so it turns "not up yet" into "never comes
up" -- it broke the first run. The reason is now a comment.
187/187 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2
concurrent 2 and the crash pair, e2e3 16, e2e4 17, e2e6 72, e2e7 86,
crash-fuzz 60 cycles.
|
||
|
A.Shakhmatov
|
36311d0faa |
plan: the in-place replace commit, declined on the measurement
The plan made it conditional on the update line still being the worst. It is -- but the gate says the peak is set by the first rebuild's second copy, not by write amplification, so generating less garbage moves the number by nothing. That would be a change to the write path, on the stretch that runs after the log record is durable, for no measured gain. Recorded as declined with the reason rather than done because it was on the list. |
||
|
A.Shakhmatov
|
1491a47479 |
plan/results: the M1 churn numbers
Amendment A5 and the `[M1.1]`/`[M1.2]` blocks. What they record is a result with two halves, and the value is in keeping both: The mechanism works. 934 MB reclaimed over the update run, occupancy at 1.06-1.26x its live data, and the counters that say so are in `serverStatus` rather than inferred. The ratio did not move. 1.94x delete-heavy and 2.46x update-heavy, identical to the end-of-Stage-2 binary measured with the same harness. `file / live` is a high-water mark because the data file never shrinks, and the mark is set in the first round by the one thing reclamation cannot avoid: a rebuild needs a whole second copy of the live data before the first can be freed. So ~2x is the floor of a rebuild-based design and no threshold reaches it -- rebuilding earlier lowers the garbage term and nothing else, rebuilding later raises it. The plan said in advance what to do if this happened, which was to write it down rather than tune, and to name incremental compaction through a doc-id-to-offset indirection layer as the successor. Recorded, with its cost: a second copy-on-write B+tree per collection, a second random read on point lookup, and it undoes A3. A second lever is named that the plan had not: 52% of the steady-state file is space the database owns and is not using, so returning it to the filesystem is worth more here than reclaiming harder. It needs the file never to shrink below what the fallback generation references, which is its own crash-safety pass. D7.4's 1.65x for the delete line is corrected to 1.94x, and the correction is the harness rather than a regression -- the same 1.94x comes out of the binary that predates any of this work. The old ad-hoc version sampled ids to delete blindly, which re-picks dead ones, so it deleted fewer documents than it inserted and measured a collection that was quietly growing. The update line reproduces D7.4 exactly, 2.46 against 2.47. 200-byte documents reclaim nothing, exactly as forecast, and the forecast being written down beforehand is what makes that a result instead of a disappointment. 3.93x on both binaries. Also noted, because the number invites misreading: at that document size the two index trees are comparable to the documents themselves and the file is already 2.18x before any churn -- index structure, not slab garbage. |
||
|
A.Shakhmatov
|
d492726881 |
db/pager: the catalog may not claim a page that is on the free list
Reclamation runs as a checkpoint phase, so it frees pages concurrently with everything else -- and the one failure that arrangement can produce is silent. A catalog that claims a page already handed to the pager gets that page back two generations later, written over by somebody else; the crash that falls back to that generation then reads a document which is no longer there. Nothing fails at the time, and the `seq` retry cannot see it because neither a reclamation nor a rebuild appends a log record. So `write_catalog` now asserts it, per run, in test and Debug builds -- three list scans where every run is being walked anyway. It is proven to fire: have `reclaim_windows` free the pages and keep the old run list, and the suite panics on it. This is the double-ownership detector the plan said an enlarged free list deserves. And `checkpoint` takes a lock of its own. Two can be in flight -- a writer's epilogue claims the pending flag while another is inside `compact`, which checkpoints of its own. The publish was always safe, since it runs under `log_lock`; the phase in front of it is new. One checkpoint's `reclaim_slabs` frees pages under a collection's lock that the other's `write_catalog` may already have serialized, and that is exactly the shape above. Stated plainly: the argument for the lock is by construction, and no test reproduces the interleaving -- removing it leaves the new concurrency test green. What that test does do is run reclamation under two checkpointers and a writer with the ownership assertion armed, which is the harness that would catch the argument being wrong. This is the second instance of the shape PLAN records as still open (a rebuild frees pages under only the collection's lock while a checkpoint may have snapshotted a catalog claiming them). Reclamation is now excluded from it; `compact`'s rebuild walk still is not, and that remains recorded rather than fixed here. 187/187 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, the full e2e matrix, crash-fuzz 60 cycles. |
||
|
A.Shakhmatov
|
8f63c6df70 |
tests/e2e: the churn gate, as a committed harness
D7.4 was the only block in `tests/e2e/results/m0-gates.txt` without a `reproduce:` line. The numbers were real and the harness was not committed, so the one measurement the whole free-list decision rested on could not be re-run against a change. This is that harness. Self-contained like `e2e6.js`: it spawns its own server on a fresh database. Two modes, delete-and-refill and repeated update, over `--docs` documents of `--doc-size`, with `--index` to put index maintenance inside the churn rather than beside it. Three things it does that the ad-hoc version did not: Live bytes are computed here, from the serialized size of one document, rather than read off the server. That is what makes a run against an older binary comparable -- and the first thing this harness was used for was measuring the pre-Stage-3 binary, which has no `multifora` section at all. Deleted ids are sampled from the ids actually live. Sampling blind from the id space re-picks dead ones, so a round deletes fewer documents than it inserts and a supposedly flat-live measurement quietly grows. The first run of this harness ended with 3211 documents where it should have had 2000. And it prints `inUse` beside `ratio`. The data file never shrinks, so `file / live` is a high-water mark and cannot come down however well reclamation works; `inUse` is `(allocTail - freeReady) / live`, which is what the database is actually occupying. On the update line those two read 2.46x and 1.06-1.26x for the same run, and the difference between them is the whole finding. A fixed seed, so two runs churn the same documents in the same order and a difference between them is the code rather than the dice. `--target x` fails the run above a ratio, for use as a gate; without it the harness measures and reports. |
||
|
A.Shakhmatov
|
343b25adc8 |
db: a rebuild reclaims before it copies
Found by the churn harness, and it is the difference between reclamation working and reclamation being unreachable. A checkpoint is what hands back empty slab windows, and a checkpoint is armed by log volume. A delete logs only an `_id`. So deleting half of a 190 MB collection moved the log by a couple of megabytes, no checkpoint ran, and the garbage sailed straight past the rebuild threshold -- the rebuild got there first every time and reset the window map it would have used. Measured before this: six rounds of delete-and-refill, six rebuilds, 1 MB reclaimed. After: the same six rounds, 256 MB reclaimed. The fix is one line of ordering. `compact` now checkpoints before it walks the collections, so the cheap half of the job runs first: a checkpoint hands back whole windows for the cost of one publish, where a rebuild copies every live byte in the database. The per-collection gate then judges what reclamation left rather than what it was about to take, so a collection whose garbage was all in empty windows is not rewritten at all. No new threshold and no new state -- the gate that decides is the one added in "a rebuild copies only the collections that have garbage", now reading a post-reclamation number. 186/186 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, the full e2e matrix, crash-fuzz 60 cycles. |
||
|
A.Shakhmatov
|
8bf35e707c |
commands: serverStatus reports what the slab is doing
A `multifora` subdocument -- named so nobody mistakes it for a MongoDB section -- carrying `liveBytes`, `deadBytes`, `slabBytes`, `reclaimedBytes`, `slabRuns`, `freeReadyPages`, `allocTail` and `compactions`. The milestone's gate cannot be read without them. A steady-state size ratio can look respectable while reclamation does nothing at all: the file grows, a rebuild periodically halves it, and the average comes out fine. What distinguishes the two is `reclaimedBytes` rising while `allocTail` stays put, and no ratio shows that. Same for rebuilds -- "the ratio improved" and "the ratio improved because reclamation worked rather than because a rebuild ran" are different results, so `compactions` counts collections rewritten. The byte figures are summed from the collections rather than read off the engine's running totals, so this reports the same side of the comparison `checkpoint` asserts. A counter that had drifted from the catalog would otherwise make the gate measure the drift. Every field is present at zero. A gate that cannot tell "no pages ready" from "field missing" cannot be read at all. 186/186 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, and the full e2e matrix. |
||
|
A.Shakhmatov
|
0dd9a0c908 |
db: a rebuild copies only the collections that have garbage
`compact` walked every collection unconditionally, so garbage in one paid for a full copy of the other thirty-nine. A copy is not free even where it reclaims nothing: it rewrites every document and every index, and it bumps the layout epoch, which kills every open cursor on a collection that had no reason to be touched. The gate is the share `note_compact` already applies to the engine's totals, and using the same one is what keeps them from disagreeing. If no collection passes it then `dead_i < live_i / 4` for every one, so `sum(dead) < sum(live) / 4` and the engine's trigger could not have fired either -- a compaction that runs always rebuilds at least one collection and cannot spin re-arming itself over garbage no rebuild will take. An absolute floor per collection would break exactly that: forty collections each under the floor can sum to well over it. Two existing tests needed real garbage, which is the change working. "a rebuild kills an offsets cursor and spares a streaming one" and "the epochs that invalidate a cursor move exactly when they must" both called `compact` on a clean collection and relied on it rewriting anyway. The first now rewrites all 60 documents (a replace, not a delete, so the drain still checks that the stream yields exactly 60 once each -- and with a changed field, since an identical replace is deliberately not a write). The new test asserts both halves: the dirty collection's epoch moves, the clean one's does not, and the clean one's `slab_used` is unchanged -- a repack starts the slab over, so that number could not survive one. Mutation: delete the guard, and the clean collection's epoch moves too. 185/185 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2 concurrent 2 and the crash pair, e2e3 16, e2e4 17, e2e6 72, e2e7 86, crash-fuzz 60 cycles. |
||
|
A.Shakhmatov
|
7fe1009243 |
db/pager: a slab extent comes off the free list when one fits
Without this the previous commit is decorative. Windows go back, the free list fills up, and the file grows by the whole write volume anyway -- because nothing asks for the pages in the shape they arrive in. `take_free` cannot serve a slab extent from reclaimed windows, and that is on purpose. It is best fit precisely so the thousands of single-page copy-on-write requests per generation cannot dismantle the large runs; the consequence is that a 2048-page extent request never matches anything smaller, and reclamation hands back runs a few windows at a time. So `alloc_slab_run` is a second policy in the same allocator: at least `min_pages`, at most `max_pages`, longest available so the collection switches extents as rarely as possible, ties to the smallest source run so the big ones stay as whole as they can. It takes a partial run when it cannot have a whole one and it is allowed to trim a larger one -- there is no cannibalisation to fear when the request is itself at least 1 MiB, and what it leaves behind is a run rather than a hole. `take_free` is untouched and its pinned first-fit mutation test still passes. What it hands out is aligned to `map_align`, which is not cosmetic. That is the granularity writeback tears at and the granularity reclamation gives back at, so a run starting mid-system-page both wastes its first window and shares a kernel page with whatever holds the rest of it -- for a page still in the published image, exactly the tearing `mark_appendable` refuses to risk. The trimmed edges stay on the free list. The caller's floor is 1 MiB: a shorter extent is exhausted after a handful of documents and every exhaustion writes off what is left of the one before it. Measured, in the new engine-level test: delete-and-refill of 400 16 KiB documents per round, four rounds. The tail stands at 2068 pages after the first round and 2083 after three more of the same volume -- 15 pages of growth against 4800 pages written. That is the number the whole milestone is about, and it is the one Risk 5 in the plan says to check directly rather than inferring from a ratio. Three tests. The churn one above. The pager's policy: what it hands out starts and ends on a system-page boundary, comes out of the long run rather than the short one and past the long one's unaligned first page, and everything not handed out is still on the list; a run below the floor is left alone. Mutations: raise `slab_run_min_pages` to a whole extent and the churn test goes red (which is also the measurement saying the floor has to stay well under an extent); drop the alignment and the pager test goes red on an odd-page run; drop the `usable < min_pages` test and the short run is handed out. 184/184 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2 concurrent 2 and the crash pair, e2e3 16, e2e4 17, e2e6 72, e2e7 86, crash-fuzz 60 cycles. |
||
|
A.Shakhmatov
|
afdb44fe90 |
db: a slab window with nothing live in it goes back to the pager
The reclamation itself. A checkpoint now begins by handing back every `map_align` window whose dead-byte counter has reached `map_align`, splitting the runs around what is kept. Counting is the entire liveness test, and that is what makes this cheap. `evict_doc` removes a document's index entries before marking its bytes dead, so a window reaches `map_align` only once every document with a byte in it is unreachable -- "no live bytes" and "no reference to these bytes" are the same statement, established without scanning anything. Inside `checkpoint` rather than a hook after it, because reclamation changes two things that must agree: `slab_runs`, which the catalog describes, and the pager's free list. One `publish` makes both durable. A crash before it leaves the old catalog and the old free list -- no reclamation happened -- and a crash after leaves both describing the new ownership. No new record type, no replay path, no ordering in between to get wrong. It also means the write path pays only for a counter update and the cadence of returning pages is the checkpoint threshold. `full_windows` counts windows that have reached `map_align`, so a collection with nothing to give back is not scanned at all. Without it every checkpoint would walk every window of every collection -- O(slab) regardless of workload, and the workload this design is known not to help (small documents on 16 KiB pages) is exactly the one that would pay it for nothing. A failure changes nothing: the replacement run list is built whole before the old one is touched, so out of memory means the garbage stays and the next checkpoint tries again. Past the last fallible step the list is swapped in first and the pages handed over second; a `free_pages` that fails there leaks the run, which costs space. The other order would leave pages owned twice. `slab_used` changes meaning from "bytes ever appended" to "slab consumed and not yet given back". That is what keeps `slab_used - live_bytes` equal to the garbage the collection still has, with no new persistent field -- both halves are already in the catalog. `layout_epoch` is bumped when, and only when, a collection actually gave something back. Reclamation does not move a live document, so a cursor's live offsets stay good; but a saved offset list can name a page now on the free list, and reading it would succeed and return plausible garbage rather than fail. `cursor_still_valid` kills such a cursor with QueryPlanKilled, as a rebuild already does. Not bumping it otherwise matters just as much: every cursor on a busy collection would die on the checkpoint cadence for nothing. Three tests. The survivor: 199 of 200 documents deleted, and the window holding the last one is not given back, still reads, and goes back only once it is empty too. The deferral: a reclaimed page is in `free_hold` after the publish that freed it and in `free_ready` only after the next one -- asserted on the pager's lists, since `free_ready_pages()` also moves for copy-on-write victims. And the epoch, added to the existing three-promise cursor test: unmoved by a checkpoint that reclaims nothing, moved by one that does. Two Stage 0.6 tests changed, in the direction that was the point. "the slab counts what the appender skips" asserted that an abandoned extent tail survives a checkpoint, because only a rebuild could reclaim it; now the checkpoint gives 4 MiB of it straight back and what stays is the edges. "a rebuild leaves behind what its own copying skipped" asserted 2-4 MiB left after a rebuild; the checkpoint inside `compact` reclaims most of that now, so it asserts the same thing about a smaller number -- the counter is not zeroed and equals what the collection has. Mutations: relax the fullness test to accept a window 2 KiB short of empty and the suite aborts on the append-cursor assertion (the appender's own window is the first thing a loosened test reaches); drop that assertion too and the survivor test goes red alone. Remove the `reclaim_slabs` call, 5 tests fail. Remove the epoch bump, 1. Invert the `full_windows` shortcut, 5. 182/182 unit tests in ReleaseFast and ReleaseSafe, 82/82 fuzz, e2e 49, e2e2 concurrent 2 and the crash pair, e2e3 16, e2e4 17, e2e6 72, e2e7 86 (the cursor suite, which the epoch bump was most likely to redden), crash-fuzz 60 cycles. |
||
|
A.Shakhmatov
|
23b283ff44 |
db: the slab knows where its dead bytes are
Inert on its own: nothing is reclaimed yet and no behaviour changes. What
changes is that a collection can now answer *where* its garbage is, which is
the precondition for handing any of it back.
A slab extent becomes a `SlabRun`: the same two u32s plus a dense array of
dead-byte counters, one per `map_align` window. The window is the unit because
it is the smallest thing that can be given back at all -- `mark_appendable`
refuses an unaligned start and `protect_stable` rounds outwards -- so a
counter never exceeds `map_align` and its width follows from that. Two bytes
per window is the entire memory cost: 2.7 MB for a 21 GB slab on 16 KiB pages.
The shapes that track dead *documents* instead (an interval set, a free-run
list) cost gigabytes at the 200-byte document scale of D7.3, and would make
`evict_doc` allocate after the write is already committed, which is a failure
with nowhere to go.
`mark_dead` is therefore infallible, and is called from the two places slab
dies: `evict_doc`, after every index entry naming the bytes is gone, and
`note_skip`, for what the appender writes off at a checkpoint or when it
abandons the tail of an extent. Ordering `mark_dead` last in `evict_doc` is
what will make reclamation by counting alone sound -- a window reaches
`map_align` dead only once every document touching it has been through there.
The run list is now sorted by page number rather than allocation order. That
was free while an extent could only be appended to; a recycled run arrives
*below* one the collection already owns, and `run_of` is a binary search.
Sortedness and non-overlap are asserted at the single point runs enter.
Two counters accompany it. `dead_unlocated` holds garbage that has no window:
the head and tail of a run outside its whole windows, and -- the larger share
-- everything that died before the last restart. It exists so one identity
stays exact:
sum of window counters + dead_unlocated == slab_used - live_bytes
Left side is where, right side is how much; reclamation reads the first and
the compaction trigger reads the second, and a drift between them is either a
rebuild firing on a clean database or a window handed back with a live
document in it. `reclaimed_bytes` is inert here and exists for the churn gate,
which cannot otherwise tell "the ratio improved because reclamation worked"
from "the ratio improved for another reason".
The catalog is byte-identical: still `u32 count, (u32 first, u32 pages)*`, so
`catalog_version` stays 1 and there is no second read path. The window map is
deliberately not persisted -- an open puts the whole amount into
`dead_unlocated` instead. The consequence runs one way: a forgotten dead byte
is a window that is not handed back, never a live window that is. Reading
inserts sorted rather than appending, so a catalog written before this commit
loads into an ordered list.
Five tests. The accounting identity across both kinds of death; run edges
counted but not placed, driven against `mark_dead` directly since the
alignment of a real extent is the allocator's business; documents never
straddling a run, over a collection with an oversized document in a run of its
own; a run recycled to a lower address keeping the list ordered and findable;
and a restart forgetting where the garbage is but not how much.
Mutations, each red on its own: drop `mark_dead` from `note_skip` (30824 vs
0), from `evict_doc` (151304 vs 30824), the run-tail branch (16384 vs 12288),
the run-head branch (26 tests crash on the underflowed window index), the
sorted insert (overlap assert fires), and the `dead_unlocated` line in
`read_catalog` (40240 vs 0).
180/180 unit tests in ReleaseFast and ReleaseSafe, 82/82 fuzz, e2e 49, e2e2
concurrent 2 and the crash pair, e2e3 16, e2e4 17, e2e6 72, e2e7 86,
crash-fuzz 60 cycles.
|
||
|
A.Shakhmatov
|
e416ad179a |
plan: sessions, and the three measurements that corrected it
Records what `lsid` support is and what it deliberately is not, so M4 inherits the decisions rather than the questions. The part worth keeping is not the design but its corrections: three of the assumptions this stage was planned on turned out to be wrong when measured against a real mongod, and one of them -- that unknown fields inside `lsid` are tolerated -- would have shipped a divergence nothing in the test corpus could have caught. Also states that the scorecard did not move, 194/97/196 either side, which was the prediction rather than a surprise: the corpus has no session entities at all. What is new is that the prediction is now checkable -- after the event assertions landed, a command wrongly refused here would show up as a changed event stream instead of silently. |
||
|
A.Shakhmatov
|
3e2d0ab134 |
commands: a count reply is an int32
Two of the tests added alongside endSessions read the count reply's `n` as int64. ReleaseFast does not check the union tag, so both passed there and aborted in ReleaseSafe -- which is the whole argument for running the suite in both, and the reason the plan makes ReleaseSafe non-optional. 175/175 in ReleaseFast and ReleaseSafe. |
||
|
A.Shakhmatov
|
f5c827e581 |
commands: endSessions judges the array it is handed
It stays a no-op -- there is nothing to end -- but stops being a blind `ok`. This is the one session command that arrives in normal operation: a driver sends it on close for every session it handed out. Validating an argument that is then discarded looks like ceremony, and is not: `ok: 1` to a malformed `endSessions` says the server understood something it never read, which is the same class of answer the previous commit removed for `txnNumber`. Codes and messages measured against mongod 8.3.7, including the field path `endSessions.endSessionsFromClient` -- an IDL artefact, since the command's own field is `endSessions` and the parsed argument carries a different name. It is reproduced rather than tidied: a path that differs from the real server's is worse than an odd one. With this the raw probe's replies are identical to mongod's for every lsid and endSessions shape. What is left is five deliberate divergences, all recorded: three because mongod knows per command whether `txnNumber` is even accepted and answers Location50889 or 263 before reaching the standalone refusal, and two because `startSession` and `refreshSessions` are not implemented -- a driver calls neither, generating session ids locally, so CommandNotFound is the honest answer. `commitTransaction` joins them, and it can only be reached by a client whose write this server already refused. 175/175 unit tests. |
||
|
A.Shakhmatov
|
54092f44be |
commands: accept a well-formed lsid, refuse what would be a lie
A driver puts `lsid` on every acknowledged command already, because `add_server_info` advertises `logicalSessionTimeoutMinutes`. So this is not new plumbing, it is a decision about input that has been arriving all along and being ignored. Accepting it and doing nothing is honest: a session here would own nothing -- no transactions to scope, no retryable writes, cursors that outlive their connection for their own reasons. There is deliberately no session registry; it would be a mutex on the dispatch path guarding state nothing reads, and M4's transaction state machine is what should decide its shape. `txnNumber` is a different matter, and ignoring it would be the lie this commit exists to remove. A transactional write would run non-transactionally, answer `ok`, and become durable; the client would find out at `commitTransaction`, by which time the data is on disk. It is refused, with `startTransaction` and `autocommit` alongside it for the same reason. Every code and every message was measured against mongod 8.3.7 through a raw OP_MSG probe -- the driver overwrites `lsid` with its own session, so a malformed one cannot be sent through it and none of this was checkable the usual way. Three things the measurement settled that guessing would have got wrong: an unknown command with a malformed lsid answers CommandNotFound, so the lookup comes first and this check belongs exactly where it sits; the codes for a bad session id are IDL parser codes (40414, 40415) rather than anything resembling the rest of our table; and a bad UUID length is InvalidUUID 207 while a bad subtype is TypeMismatch 14, which no amount of reasoning would have produced. Three divergences from mongod, all one cause: it keeps a per-command table of which commands accept `txnNumber` at all, and answers Location50889 or OperationNotSupportedInTransaction 263 for those that do not, before reaching the standalone refusal. We have no such table and give the standalone answer uniformly. For every CRUD command -- everything a driver would actually send these fields on -- the replies are identical; they differ only on things like `ping`, where mongod is more specific rather than differently right. Checked before any lock is taken, and the test for that is the second half of each refusal: keep using the engine afterwards, with a write that has to take the catalog exclusive to create a collection. Mutation-checked by moving the call below `lock_catalog` -- the test run hangs, which is precisely how the nameless-command lock leak presented, since a leaked *shared* lock is invisible to every reader. 174/174 unit tests. |
||
|
A.Shakhmatov
|
0034da4293 |
wire: parse a logical session id out of a command body
An accessor over the command envelope, next to `db_name` and the same shape: called from dispatch, never from `parse`, because a malformed session id is a command that gets an error reply, not a connection that gets torn down. It returns the 16 bytes or names what is wrong; the codes stay in the command layer, where they were measured. The tolerated fields were measured against mongod 8.3.7 rather than recalled, and the measurement contradicted the assumption this was designed on. The design said unknown fields inside `lsid` would be tolerated, on the reasoning that the server tolerates unknown fields everywhere and pinpoint strictness would be inconsistent. mongod answers IDLUnknownField (40415) -- it is strict here and the reasoning was simply wrong. It also accepts `uid`, the hash of the credentials owning the session, which a driver starts sending the moment authentication is on; rejecting that would have broken every command in M7, and the test says so where a future reader will meet it. `txnNumber` and `txnUUID` inside `lsid` are refused. They are not the retryable-write `txnNumber` that sits outside it: together they name an *internal* session, one that runs a transaction on another session's behalf. mongod refuses them on a standalone too. Also `bson.Value.type_name`, which is mongod's name for a type rather than Zig's -- a TypeMismatch message quotes it, and a driver that matches on the text is matching on these. 170/170 unit tests. |
||
|
A.Shakhmatov
|
e65da2740e |
plan: what the event assertions found
Four defects and one deliberate non-fix, recorded where the milestone can see them rather than only in five commit messages. The one worth carrying forward: the runner dropped `collectionOptions`, so every "unacknowledged write" case in the corpus had been running an acknowledged write and passing, because the two produce results the expectation accepts either way. Only the wire distinguished them and nothing read the wire. Also states plainly that scorecards from before this are not comparable with ones after, and why `bypassDocumentValidation` is left failing: both available refusals are worse than four undeserved entries in the fail column, and one of them is the shape that turns a scorecard into flattery. |
||
|
A.Shakhmatov
|
6d5c860e11 |
tests/spec: an event's command is compared in the shape it was sent
A command-monitoring event hands over the command as the driver holds it in
memory, and that is not always the shape it puts on the wire: a sort is a JS
`Map` (driver lib/sort.js). `Object.keys` on a Map is empty, so the matcher
reported every key of an expected sort as missing from a command that in fact
carried it -- five cases, all of them the runner's fault and none the
engine's.
This one is worth the paragraph because of how well it hides. EJSON serializes
a Map exactly like a document, so `MFDB_DUMP_EVENTS` prints
`"sort":{"_id":1}` next to a failure that says `sort._id` is missing, and the
dump -- the tool built for exactly this triage in the commit that added the
buffers -- reads as evidence that the matcher is wrong about something else.
It took `Object.keys(formatSort({_id: 1}))` returning `[]` to see it.
Converted for the comparison only, and at every depth, since a sort also
appears inside `updates[i]`. `match` stays a plain reading of the spec's
Evaluating Matches with no driver knowledge in it.
Mutation-checked: pass the event's own value through and
findOne.json "FindOne with filter, sort, and skip" goes red again with the
original message.
189/102/196 becomes 194/97/196.
|
||
|
A.Shakhmatov
|
548c882d47 |
commands: the wire version says what the version string says
`buildInfo` has always reported 4.4.0 and the handshake has always reported maxWireVersion 8, which is 4.2. A driver believes the wire version: it refused client-side to send `hint` on an unacknowledged delete or findAndModify (the error is "only supported on MongoDB 4.4+", raised without a round trip), and withheld `comment` from getMore, listCollections and listDatabases (lib/operations/get_more.js:43 and its neighbours). Both are things this engine handles -- the acknowledged hint suites pass, and getMore ignores fields it does not know -- so the effect was purely the number disagreeing with itself. The 8 was not arbitrary. The comment above it tied it to omitting `topologyVersion`, which is what keeps a driver off the streaming hello protocol we do not implement -- a real bug, once visible as Compass reconnecting every heartbeat. Checked before touching it, in the driver rather than from memory: `useStreamingProtocol` (lib/sdam/monitor.js:154) returns false whenever `topologyVersion` is absent and never looks at the wire version at all. The omission is the whole mechanism; the wire version was a second line of defence that never existed. The comment now says so. A test asserts the two agree, so they cannot drift apart again silently, which is the actual defect here -- not the value. Spec suites 173/118/196 -> 189/102/196: 16 cases fixed, none broken. Ten are the unacknowledged-hint cases the previous commit uncovered, six are `comment` forwarding. 166/166 unit tests in ReleaseFast and ReleaseSafe, 82/82 fuzz, e2e 49, e2e2 concurrent 2 + crash pair, e2e3 16, e2e4 17, e2e6 72, e2e7 86, crash-fuzz 60 cycles. |
||
|
A.Shakhmatov
|
7f426cdd33 |
tests/spec: a collection entity gets the options it was declared with
`buildEntities` built every collection as `db.collection(name)` and every
database as `client.db(name)`, dropping `collectionOptions` and
`databaseOptions` on the floor. 15 collection entities declare a
`writeConcern`, 7 a `readConcern`, one a `readPreference` -- and the 15 are all
`{w: 0}`, so every "unacknowledged write" case in this corpus has been running
an acknowledged write against a driver that was never told otherwise.
They passed anyway, because an acknowledged and an unacknowledged write of the
same document produce results a `$$unsetOrMatches` expectation accepts either
way. Only the command on the wire distinguished them, and nothing was reading
the command until the previous commits. This is the first thing the event
assertions found, and it is a fair answer to what they cost.
Option documents are unwrapped from their BSON types on the way to the driver.
The suites are parsed with `relaxed: false`, so `{w: 0}` arrives as an Int32
and the driver gates `writeConcern.w` on `typeof w === 'number'` -- the same
trap NUMERIC_OPTIONS already documents for operation options, and a silent one:
the option would simply not apply. Wholesale unwrapping is safe here in a way
it is not there, since these are settings the driver consumes rather than
values an assertion compares. An option key outside the spec's
`collectionOrDatabaseOptions` set is reported unsupported rather than ignored,
which is the lesson of the bug itself.
159/132/196 becomes 173/118/196. 14 cases fixed, none broken.
The other 10 unacknowledged cases now fail differently, and that is progress
of a sort: with `w: 0` actually applied, the driver refuses client-side to send
`hint` on a delete or findAndModify to a server older than 4.4. This engine
reports itself as 4.4.0 with maxWireVersion 8, and 4.4 is wire 9. That
inconsistency is ours, it is the same one behind the `comment`-on-getMore
failures, and it gets the next commit.
|
||
|
A.Shakhmatov
|
bb8cdd964b |
tests/spec: the scorecard no longer disclaims expectEvents
The disclaimer was accurate for as long as it stood -- events were not read,
so `pass` was an upper bound and saying otherwise would have been a lie about
the number. It is now a lie in the other direction, so it goes, replaced by
what is actually true: events are compared exactly, in number and in order,
which is what makes a pass mean the engine answered correctly *and* was asked
the right question. The header says plainly that scorecards recorded before
this are not comparable, and enumerates what is still skipped inside events
rather than leaving "asserted" to be read as "asserted completely".
Two facts in the docs had gone stale and are corrected here because this is
the commit that rereads them:
- README said `--op-timeout-ms` defaults to 3 s. It has been 10 s since the
commit that explains, at length and directly above the constant, why 3 s
was wrong. A stale number in exactly the place that warns against
tightening it is worse than no number.
- `MAX_SCHEMA` is [1, 24]; the comment above it still claimed 1.0-1.9.
Totals unchanged at 159/132/196 -- this commit only rewrites prose, and the
scorecard is re-recorded so its header matches the runner that produced it.
|
||
|
A.Shakhmatov
|
54ad124c18 |
tests/spec: a CSOT-rewritten maxTimeMS cannot be asserted
Every client entity is built with CSOT `timeoutMS` (OP_TIMEOUT_MS, 10 s), and CSOT overwrites `maxTimeMS` on each command with what is left of that budget. An expectation of `maxTimeMS: 6000` therefore meets the harness's 10000, and no amount of engine correctness would change it. Reported unsupported rather than failed: a FAIL is a claim about the engine, and this is a claim about the runner. Refused unconditionally when an expected command mentions `maxTimeMS`, not only when the two values differ, so it can never become a pass by coincidence. Exactly one case in the corpus asserts it -- estimatedDocumentCount.json, "estimatedDocumentCount with maxTimeMS" -- so the whole cost of the hatch is one case, which is why it is worth taking instead of dropping `timeoutMS`. That option is not open anyway: `timeoutMS` is what replaced the outer race that once turned ~190 good cases into phantom timeout FAILs. This is the only escape hatch in the runner. Everything else is either an honest FAIL or an enumerated unsupported feature. 159/133/195 becomes 159/132/196: one case, fail to skip, and nothing else moves. |
||
|
A.Shakhmatov
|
97e3e3a556 |
tests/spec: assert expectEvents
The headline is not the delta, it is that `pass` changed meaning. 354 of the
487 cases declare `expectEvents` and until now the runner read none of them,
so a case could send the wrong command entirely and still be counted a pass
as long as the *result* came back right. The old column was an upper bound by
construction. 193/99/195 becomes 159/133/195, and the two numbers are not
comparable.
Two rules decide how far the assertion reaches, both taken from the spec
rather than from what would be convenient:
- `command` and `reply` match as *root* documents
(unified-test-format.md:1020-1022, :1037-1039). The driver hangs `lsid`,
`$db` and `maxTimeMS` off nearly everything it sends; as nested documents
essentially the whole corpus would fail on keys no expectation was ever
written to mention, and the number would say nothing.
- the event list is exact in number and order, not a prefix
(unified-test-format.md:3088-3091). 23 cases expect an empty list and a
prefix rule would pass every one of them without looking.
The assertion runs after the operations, so a wrong result is still reported
as a wrong result rather than being masked, and after the listeners are
disabled, so the teardown's own commands cannot reach the buffer.
`cmap` and `sdam` event types, `ignoreExtraEvents`, and any event field beyond
`command`/`reply`/`commandName`/`databaseName` are reported unsupported at the
point of assertion. None occurs in this corpus -- all 354 blocks are
`eventType: command`, carrying 349 `commandStartedEvent` and 6
`commandSucceededEvent` -- so nothing is being quietly waived.
All 34 newly-failing cases, triaged. Not one is a wrong answer from the
engine; every one is a command the driver never sent:
- 22x `command.writeConcern: missing` -- runner gap, and the sharpest thing
this commit found. `buildEntities` drops `collectionOptions` on the floor,
so `writeConcern: {w: 0}` never reached the driver and every "unacknowledged
write" case in the corpus has been running an acknowledged write. They
passed because the results of the two agree. This is precisely the class of
error the instrument was built to find, and it was invisible to the result
column.
- 5x `command.sort.<key>: missing` -- runner gap. The driver holds a sort as
a JS `Map` (lib/sort.js), so `Object.keys` on it is empty and the matcher
reports every expected key as absent. Measured, not guessed: EJSON prints a
`Map` exactly like a document, which is why the dump looks correct.
- 4x `command.bypassDocumentValidation: missing` -- unclassified. The option
is absent from the wire for the `false` cases; the driver only forwards it
when true on some paths (lib/operations/find_and_modify.js:19), and whether
the runner also drops it has not been established.
- 2x `command.comment: missing` on getMore -- server gap, most likely. The
driver gates it on `maxWireVersion >= 9` (lib/operations/get_more.js:43)
and this engine advertises 8 while reporting itself as 4.4.0, which is
wire 9. The inconsistency is ours.
- 1x `command.maxTimeMS: expected 6000, got 10000` -- the CSOT rewrite, dealt
with in the next commit.
Each of those gets its own commit, and none of them is fixed here: a check and
the fix for what the check caught do not belong in one change.
|
||
|
A.Shakhmatov
|
6560aec915 |
tests/spec: buffer command-monitoring events per client entity
Plumbing only: a client entity that declares `observeEvents` now gets `monitorCommands` and a buffer, and nothing reads the buffer. That is the point of splitting it out -- the totals not moving *is* this commit's test. Command monitoring changes how the driver builds every command it sends, and if that alone shifted a result there would be no way to tell it apart from the assertions landing in the next commit. 193/99/195 before, 193/99/195 after, 175/175 files, 0 errored. The rules the buffer already enforces, so that the next commit is only about comparing: `ignoreCommandMonitoringEvents` by command name; sensitive commands dropped unless `observeSensitiveCommands` says otherwise, with `hello` and legacy hello inferred sensitive from the driver having redacted them to empty documents (unified-test-format.md:3070-3075). Neither fires on this corpus -- 136 client entities observe `commandStartedEvent`, 6 also `commandSucceededEvent`, and not one sets either field -- but a rule that only exists where it is exercised is a rule that will be missing when M7 brings auth. `cmap` and `sdam` observations are collected by nobody; a test that goes on to assert them is reported unsupported where it asserts, not where it declares. Two things about placement, both load-bearing. Listeners are attached after `connect()`, so a client's own handshake is not in its own buffer -- measured rather than assumed: with the buffers dumped, find.json's five cases show exactly `find`, `getMore`, `getMore` and nothing else. And they are disabled after the operations and before the outcome check (unified-test-format.md:3081), plus again unconditionally in the teardown `finally`, because the outcome check and the teardown both issue commands and a buffer still growing through them would make the assertion a function of the harness rather than of the engine. `MFDB_DUMP_EVENTS=1` prints each case's buffer. That is how the handshake question above was settled and how a failing event assertion will be triaged. |
||
|
A.Shakhmatov
|
afa5c6ef9d |
tests/spec: $$unsetOrMatches does not change root-ness
`special()` passed a hard `false` for `root` into its recursion, so a value
standing behind `$$unsetOrMatches` was matched as a nested document even when
it sat at the top of an `expectResult`. The spec says the opposite in so many
words -- "This operator does not influence whether or not an actual document
value is considered a root-level document" (unified-test-format.md:2873, and
:2821 for `$$matchesEntity`) -- and that distinction is the whole of the
extra-key rule: only a root document may carry keys the expectation does not
mention.
`root` now threads through `match` -> `special` -> the recursion. From under a
key it is always false, which is what it already was; from the top level it is
whatever the caller had.
25 cases go from FAIL to pass and none moves the other way. Every one is the
same shape -- an `expectResult` of `{$$unsetOrMatches: {acknowledged: false}}`
against a driver write result that also carries its counts, or the
`insertedId`/`insertedIds` forms of the same thing -- and every one was the
runner failing a result the engine had got right. 168/124/195 becomes
193/99/195; the scorecard is rewritten here so the delta belongs to this
change alone.
Mutation-checked: put the `false` back in the `$$unsetOrMatches` arm and
bulkWrite-deleteMany-hint-unacknowledged.json returns to 0 pass, 2 fail. The
`$$matchesEntity` arm is the same one-word change on the same sentence of the
spec, but the crud corpus does not use that operator once, so it rests on the
spec text rather than on a red test.
Two consecutive full runs, both 193/99/195, 175/175 files, 0 errored, no
lingering timers.
|
||
|
A.Shakhmatov
|
e66030f43e |
plan: the eight bugs cleared before the free list
Four of them were not in the plan that started this work -- they were found by tests written for the three that were, which is M0's gate lesson arriving a milestone early. The record has to match what happened, or the next session reads a milestone that looks like it went as designed. Also records what nobody is fixing yet: `rebuild_collection` frees pages under only the collection's lock while a concurrent checkpoint may have snapshotted a catalog that claims them, and the `seq` retry cannot see it because a rebuild appends no log record. Written down so the free list does not add a second instance of the same shape. |
||
|
A.Shakhmatov
|
332206e5dd |
db: the slab counts what the appender skips
`slab_used` only ever grew by a document's length, so the two places the appender writes slab off went uncounted: the gap left when a checkpoint freezes the page the cursor points into and the cursor resumes at the next system page, and the tail of an extent abandoned for a document that no longer fits. Both are real garbage -- only a rebuild gets them back -- and both were invisible to the trigger that decides whether a rebuild is worth doing. An abandoned tail can be most of 8 MiB. `note_skip` counts them into `slab_used` where they happen and hands the number back for the caller to charge to the engine, which keeps the identity the last commit established: `dead_bytes` is the sum of `slab_used - live_bytes` over the collections that exist. That identity is also why `compact` no longer zeroes `dead_bytes`. A repack appends through the same slab, so it abandons a tail of its own whenever the next document does not fit; zeroing was true only if a rebuild leaves nothing behind, and it does not. `sum_dead_bytes` recomputes it from the collections, each under its own lock. Carried with it, because this commit is what exposed it: the four engine counters get a lock of their own. They are the only engine-wide mutable state a writer touches while holding nothing but its own collection's lock, so two writers on different collections reach them with no lock in common -- and the checkpoint's consistency check read them ordered against nothing, while the per-collection figures it compares them to were read under each collection's lock. Before this commit `dead_bytes` moved on nearly every write, so that check was skipped almost every time; with skips counted it stands still between checkpoints, the check runs, and it aborted three of eight ReleaseSafe runs of "a checkpoint runs alongside writers on several collections". Not mutation-checked, and worth saying so: reverting the lock did not re-trigger the abort in 24 further runs, and neither did the exact pre-fix revision in 10. The rate depends on machine load, and a mutation check that cannot be relied on to go red is not a check. The lock stands on inspection instead -- an unsynchronized read-modify-write on a counter shared by threads holding no common lock is a defect whatever its rate -- and the concurrency test now asserts the identity once everything is quiet, which is the half of it that does not depend on a race being caught in the act. Mutation-checked, each red on its own: drop either `note_skip` call in `slab_reserve`; put `self.dead_bytes = 0;` back in `compact`. The `note_skip` in `slab_append` is covered by the concurrency test, the only place a publish lands between a reservation and its append. 165/165 unit tests in ReleaseFast and ReleaseSafe, 82/82 fuzz, e2e 49, e2e2 concurrent 2 + crash pair, e2e3 16, e2e4 17, e2e6 72, e2e7 86, crash-fuzz 60 cycles. |
||
| e15d7f2ed0 |
db/pager: the concurrency test writes the way the server does
"a checkpoint runs alongside writers on several collections" drove its writers through `Engine.lock()` -- the legacy whole-engine lock, which the server has not used since the locks were decomposed. That serialized the writers against each other, so the overlap the test is named for never happened: `write_catalog` takes each collection's lock shared, and nothing it was racing against took that lock at all. Drive them the way `commands.zig` dispatch does instead: catalog shared, then the target collection exclusive. The test then does what it says, and immediately found something -- two appenders on different collections calling `bytes_mut` at the same time corrupt the pager's `dirty` set, which is an unsynchronized hash map. ReleaseSafe aborts in `getOrPutContextAdapted`; three runs in five. `dirty` is test-only instrumentation (`track_dirty = builtin.is_test`), so this is a harness bug rather than a server one -- but it is the one shared structure on a write path whose writers are otherwise kept apart by owning different pages, and it needs a lock of its own. Six ReleaseSafe runs clean afterwards. 163/163 unit tests in ReleaseFast and ReleaseSafe, 82/82 fuzz. |
|||
| 992cc2a5ab |
db: a dropped collection is reclaimed, not deadened
`free_collection` charged the engine's `dead_bytes` with the dropped collection's live bytes, and then, three lines down, handed every page that collection owned back to the pager. A drop therefore asked for a rebuild -- a full copy of every collection that was left -- to reclaim space that had already been reclaimed. Its own garbage was wrong the other way: bytes that died before the drop stayed on the engine's books after the pages holding them were freed. Both halves of that are the same statement: `dead_bytes` is the sum of `slab_used - live_bytes` over the collections that still exist. Make it so on the drop path, then stop storing it separately at all -- `read_catalog` recomputes it from the collections the catalog lists, so the watermark's copy is now a hint for anything inspecting the header rather than a second source of truth. It would be wrong in one specific way if it stayed one: a collection dropped after the last checkpoint is gone from the catalog but still charged for in the hint. `write_catalog` now returns the dead sum beside the live one and the checkpoint asserts it the same way, under the same quiescence condition. That is what makes the accounting checkable rather than merely intended. Mutation-checked three ways, each red on its own: charge the drop again; delete the subtraction of the collection's own garbage; delete the accumulation in `read_catalog`. 163/163 unit tests in ReleaseFast and ReleaseSafe, 82/82 fuzz, e2e 49, e2e2 concurrent 2 + crash pair, e2e3 16, e2e4 17, e2e6 72, e2e7 86, crash-fuzz 60 cycles. |
|||
| f8a39a0965 |
db: the catalog snapshot is read under each collection's lock
`write_catalog` walks every collection's slab extents, byte counters and index metadata while holding only the *shared catalog* lock -- which is the same lock a writer holds, taking the collection's lock exclusively. So the snapshot read structures their owners were free to mutate underneath it. `slab_extents` makes it more than a torn read: it is an ArrayList that `slab_reserve` appends to, and an append that reallocates leaves the serializer walking freed memory. What it writes from that walk is the catalog the next open trusts to find every extent the collection owns. Now under each collection's lock, shared, taken inside the catalog lock -- the same order `compact` uses, so no new ordering to reason about. Not the commit that found the concurrency bugs above; those needed a checkpoint racing writers, which this lock is orthogonal to. It is the one that makes the snapshot legal rather than merely lucky. Verified: `zig build test` in ReleaseFast and ReleaseSafe. |
|||
| 44be427490 |
db/pager: a document append cannot land in the published image
`slab_reserve` asks `is_unpublished_at` whether the append cursor is still
writable; `slab_append` copies the bytes there. Between them sits the log append
and its fsync, and `publish` clears the entire unpublished set and mprotects the
image. So the answer was routinely stale by the time it was used, and the copy
stored into the durable image.
In ReleaseSafe that is a bus error. In ReleaseFast, where `protect_stable` is
compiled out, there is no fault at all: the store simply overwrites bytes the
last checkpoint published, and the damage surfaces later as a document that
reads back as something else. ReleaseFast is the mode the server ships in.
Present since M0 -- reproduced on
|
|||
| bafbc95898 |
db: a checkpoint publishes only what the log has made durable
`checkpoint` snapshotted `self.seq`, walked the catalog, and then asserted that the snapshot was at or below `committed_seq`. It is not, whenever a writer appended before the snapshot and has not finished committing -- between `insert` and `commit`, or inside `commit` waiting on the leader's fsync. The existing `self.seq != snapshot_seq` retry does not catch it: nothing appended *during* the walk, the append was already there when it started. The window is as wide as an fsync, and it reproduces in seconds: four writers following the dispatch epilogue's insert-then-commit against a checkpoint loop trip it on every run. It has stayed hidden because a checkpoint fires once per 32 MiB of log, so the two rarely meet -- which stops being true for exactly the churn workloads M1 is about to measure. Publishing there would claim durability for a record still in the log's buffer, and `truncate_to_header` immediately afterwards would throw it away: the client gets its acknowledgement, the record is gone. That is the failure the whole watermark ordering exists to prevent (PLAN D6), and expressing it as an assertion turned it into a server abort rather than a wrong answer -- which is the better of the two, but it is not a fix. Now a retry. `commit` seals every append made so far, so sealing and re-snapshotting converges in one more round rather than spinning against sustained writes. The assertion moves to the line above `publish`, where `log_lock` has been held since the check and `committed_seq` only grows, so it is a tripwire for future edits rather than a live hazard. Also here, because the same test found it: the catalog's live-byte sum was asserted against the engine total *inside* `write_catalog`, where the sum is accumulated across collections over time while the total moves under it. A writer landing mid-walk tripped it on a database that was perfectly consistent. The check moves to the caller and runs only when the engine total did not move across the walk. What it guards against -- a path that updates one level and not the other -- is deterministic wherever it exists, so a check that skips under sustained writes still catches it. Verified: `zig build test` 162/162 in ReleaseFast and ReleaseSafe, three consecutive runs of the new concurrency test. Reverting either half reproduces its own panic within one run. |
|||
| 51eed826fb |
pager: a checkpoint gives back the generation it replaced
Both streams a publish writes -- the catalog and the free list -- are allocated into fresh pages every time, so that a crash leaves the previous copy readable. Nothing ever gave those pages back. A server checkpoints on log volume rather than on having anything new to say, so an idle database grew its data file forever, two runs per checkpoint. The magnitude is not the two pages it looks like: the catalog carries a `u32` per index node page, so at the tens-of-GB target that is hundreds of KB abandoned at every checkpoint. It is the same shape as the reclamation bugs the M0 churn gate found -- a mechanism that works once and never twice -- and it was invisible for the same reason, that no test ran enough checkpoints to see a trend. A publish overwrites the watermark slot of the generation *two* back, since the two slots hold the new generation and its predecessor. That is the generation whose streams nothing can reach again, so `Pager` now remembers where the last two generations put theirs and frees the older pair. Process-local rather than recorded in the watermark: only a running pager needs to know, because an open reads the slot it loads and the other slot is its fallback. The pages go through `free_pages` like anything else, so they are still withheld for two more generations. Steady state is therefore a handful of pages in flight, not zero growth, and the test asserts the number does not track the publish count: forty publishes over an otherwise idle pager move `alloc_tail` by at most eight pages. An open derives the loaded generation's page counts from the lengths in its watermark, which can be one page short for a free-list stream whose final length fell inside the page its bound reserved. One page, once per open, against an unbounded leak. Two existing tests had pinned the leak's arithmetic and now assert the invariant instead of the number. Verified: `zig build test` 161/161 in ReleaseFast and ReleaseSafe, `zig build fuzz`, e2e 49, e2e2 concurrent 2 + crash pair 1/3, e2e3 16, e2e4 17, e2e6 72, e2e7 86, `crash-fuzz.js` 60 cycles. Mutation-checked: dropping the two frees takes the growth from a handful of pages to one per publish. |
|||
| f5471f73fc |
pager: the free lists are read and written under the allocation lock
`free_pages` appended to `free_pending` with no lock at all, and `write_freelist` walked all three lists the same way -- while `publish` rotated them under `alloc_lock`. Both are reachable concurrently in production: the hot caller of `free_pages` is `page_mut_cow`, which runs under a *collection* lock, and a checkpoint holds only the shared catalog lock, so copy-on-write in one collection races a checkpoint and a second collection's copy-on-write freely. The append race loses or duplicates entries. The read race is worse: an append that reallocates leaves `write_freelist`'s loop walking freed memory, and it is walking it to decide which pages are safe to hand out again. Both now take `alloc_lock`. `write_freelist` holds it across reading the lists *and* allocating the pages it writes them into, which the mutex being non-reentrant makes awkward, so `reserve_pages` and `alloc_pages_assume_reserved` grow `_locked` bodies and thin locking wrappers. A free that lands while the stream is being written simply waits for the next generation's list -- the page stays allocated one generation longer, which is the safe direction. The read race is what the new test actually caught: written to assert only the append side, it tripped the size assertion added in the previous commit on its first run, because a concurrent free had grown the list between the bound and the loop. That is a bug no reading of `free_pages` alone would have found. The test asserts page *identity* rather than a total, because a publish allocates its stream off this very list and a plain count is short by however many publishes found a fit. Every page left on the lists must be one a freer put there, exactly once. Probabilistic, as any test of a data race is -- it is evidence only when red. Mutation-checked per the repo's second ground rule: dropping the lock from `free_pages` crashes it in roughly two runs out of three; five consecutive runs with the lock in place are green. Verified: `zig build test` 160/160 in ReleaseFast and ReleaseSafe, e2e 49, e2e2 concurrent 2 + crash pair 1/3, e2e3 16, e2e4 17, e2e6 72, e2e7 86, `crash-fuzz.js` 60 cycles. |
|||
| 5c3a759429 |
pager: the persisted free list survives allocating its own pages
`write_freelist` captured the entry count, sized the buffer from it, and only then called `alloc_pages` for the pages it was about to write into. That allocation goes through `take_free` like any other, and on an exact fit `take_free` removes the entry it took. The header then claimed one entry more than the loop wrote, the hash landed eight bytes short of where `read_freelist` looks for it, and the next open printed "data file free list is corrupt" and dropped the whole list -- every page on it staying in use forever. This is the ordinary case, not a corner. The stream is one page whenever the list is smaller than 511 entries, and a one-page run is the commonest thing on the list because copy-on-write returns thousands of them per generation. So the free list was being discarded at essentially every reopen that had anything to discard, which is the same symptom class as the reclamation bugs the M0 churn gate found: the mechanism works once and never twice. The existing two-generation test misses it because its free run is two pages and the stream asks for one -- shrinking an entry leaves the count right, only removing one does not. Fixed by sizing from an upper bound and counting the entries actually written. `take_free` never adds an entry, so one allocation is enough and the bound holds; an assertion pins that the list shrank by at most the one entry the allocation could have taken. Found while designing the M1 document free list, which multiplies the traffic through this path. Verified: `zig build test` 159/159 in ReleaseFast and ReleaseSafe, `zig build fuzz`, e2e 49, e2e2 concurrent 2 + crash pair 1/3, e2e3 16, e2e4 17, e2e6 72, e2e7 86, and `crash-fuzz.js` 60 cycles with the prefix invariant holding. Mutation-checked per the repo's second ground rule: restoring the count-before-allocate ordering turns the new test red with the corruption warning. |
|||
| f2844e7894 |
cursors: server-side cursors for find, aggregate and the listing commands
Every reply came back in a single batch with `cursor.id = 0`, `getMore` was a
stub answering an empty `nextBatch` on the literal namespace `test.$cmd`, and
nothing read `batchSize`. That caps the useful collection size at what fits in
one 48 MiB message, which is the opposite of the tens-of-GB target and the
reason M0 made whole-index scans stream: the streaming candidate generator
existed with no consumer that could suspend.
## What a cursor is allowed to remember
A cursor holds no lock between requests, so everything it saves has to survive
arbitrary concurrent mutation. Nothing here is a pointer, and the two things
that look like stable addresses are not: `reset_tree` re-creates node ids 0 and
1 as different nodes, and `rebuild_collection` moves every document. Three
sources, chosen by query shape, each with a different memory contract:
- **stream** -- an index-ordered walk resumed from a `(key, off)` anchor plus a
`(leaf, slot)` hint. O(key) state, so this is what lets a cursor walk a
collection larger than memory. Survives a rebuild, because a repack changes
no key.
- **offsets** -- the matched slab offsets a narrowed plan already materialized,
8 bytes each. Killed by a rebuild with `QueryPlanKilled`, because those
offsets now name unrelated bytes.
- **buffered** -- canonical BSON copies, for a sort no index provides and for
aggregate/listing output. Depends on nothing, which is what lets a listing
hold a cursor over a `$cmd.*` namespace no collection backs.
`Collection.layout_epoch` and `Index.epoch` are the invalidation tokens, both
checked as error returns rather than assertions since a client reaches them by
keeping a cursor open across maintenance.
## Resume
`resume_forward`/`resume_reverse` are O(1) while the hint holds and fall back to
an exact-order band walk bounded by `resume_walk_max`. Without the hint, `seek`
lands at the *start* of an equal-key band, so `sort({status: 1})` over three
distinct values across 10M documents would cost ~5e10 comparisons to drain.
Two hazards found by draining a collection while writing to it, neither
predictable from reading the code:
- A deleted anchor must resume at its *band position*, or the rest of an
equal-key band is silently dropped -- most of the collection on a
low-cardinality index. Hence `band_index`.
- On a **unique** index a same-key entry can only be the anchor rewritten, so
resuming at it returned updated documents twice. Observed as duplicate `_id`s
while updating underneath a drain.
## Protocol
Measured against mongod 8.3.7 rather than recalled, which corrected three
assumptions: a bare `getMore` does *not* inherit the find's `batchSize` (4998 of
5000 documents come back), a namespace mismatch is `Unauthorized` (13) not
`CursorNotFound`, and `CursorInUse` is 143 not 12051.
`internalQueryFindCommandBatchSize` is 101, `cursorTimeoutMillis` 600000,
`clientCursorMonitorFrequencySecs` 4.
The rule everything follows is **never look ahead**: a batch that met its target
leaves the cursor open even when the source is in fact exhausted, so four
documents at `batchSize: 2` take three commands. `limit` acts as an EOF source,
which is what makes `batchSize == limit` close in one round trip. `skip` is
consumed once. `batchSize: 0` returns an empty batch with a live cursor.
Cursor ids are `(nonce << 20) | slot`, always positive. The nonce is not
decoration: without it a recycled slot serves one client another's documents.
Cursors are not connection-pinned, since the driver spec allows a `getMore` on
any connection to the same server; they end at exhaustion, `killCursors`, or the
idle sweep (a second monitor fiber, separate from the TTL one because the
cadences differ by an order of magnitude and a TTL failure must not stop
reclamation). The registry is fixed-capacity and evicts the least recently used
cursor, whose client sees the same 43 an idle timeout gives.
Fixed alongside, because cursors are what expose them:
- `listCollections` reported `"<db>."` with an *empty* collection part, which
makes the driver throw client-side -- so it would have broken the moment its
cursor stopped being id 0. Now `<db>.$cmd.listCollections`, as mongod uses.
- `count` ignored `skip` and `limit` entirely.
- `wire.end_message` now bounds a reply by the 48 MiB we advertise rather than
by `maxInt(u32)`; a reply past what we told the client to expect is not a large
reply, it is a desynchronized connection.
- Two `codeName` strings were wrong: 72 is `InvalidOptions` (MongoDB has no
`InvalidArgument`), and 40324 reports as `Location40324`.
## Verification
Unit 160/160 in ReleaseFast and ReleaseSafe; `tests/e2e/e2e7.js` adds 86 cursor
checks across five phases (batching/lifecycle/errors, streaming across churn,
aggregate+listings+count, expiry+capacity, restart) and is self-contained
because cursor behaviour is only observable with non-default flags. No
regressions: e2e 49, e2e3 16, e2e4 17, e2e2 2, e2e6 72. Spec 168 pass / 124
fail, +5 against the previous scorecard.
Mutation-checked, per the repo's second ground rule: `hint_slot + 1`, the
`band_index` off-by-one, both epoch bumps, the id nonce, the at-least-one-
document rule, and `stream_shape` returning null each turn the intended test
red. One claim was withdrawn rather than kept -- swapping `std.mem.order` for
`cmp_prefix` in the band walk changes nothing observable, so the comment now
says so instead of asserting a check that does not hold.
|
|||
| cd88e1a4d1 |
index/pager: place a split's new sibling positionally, and fix mmap growth alignment
Two bugs, both of which the crash fuzzer surfaced and neither of which any
existing test could see.
**A split put the new sibling in the wrong slot when separators repeat.**
`split_leaf` located the new right sibling with `separator_pos(node, key)`, a
search for the promoted key. That agrees with "immediately after `left`" only
while separators are distinct. When several children share one -- ten distinct
values across thousands of documents, so each value spans dozens of leaves --
`separator_pos` returns the slot after the *whole* equal-key run, which puts
the sibling at the end of that run while the leaf chain has it right after
`left`.
Parent child order then stops matching leaf chain order, and that is the one
thing a lookup cannot survive: `descend_lower` picks the last child of the equal
run, and `lookup_eq` walks forward from there over keys *smaller* than the one
it wants, stops at the first mismatch, and reports nothing. Every entry is
present, the chain is correctly ordered, `count()` is right -- and the query
returns empty. `crash-fuzz.js` found it after ~700 heavy cycles as
`find({k: 3})` returning 0 of 401 documents while every other key was exact.
Fixed by `child_slot_after`, which is positional by construction.
**mmap growth rounded with a non-power-of-two alignment.** Past 64 MiB the
growth chunk becomes a proportion of the current size (`mapped_pages / 8`),
which is not a power of two -- and `std.mem.alignForward` asserts that it is.
In safe builds that panicked; in ReleaseFast, where the assert is compiled out,
it computed `(addr + align - 1) & ~(align - 1)` with a non-power-of-two mask,
which can round *down*. A mapping shorter than intended is survivable, but a
mapping longer than the file is exactly what this function exists to prevent: a
store into a mapped page past end-of-file raises SIGBUS, which no error path
catches. `alignForwardAnyAlign` instead. Never noticed because no unit test grew
a pager past 64 MiB.
Also here, because both bugs were invisible rather than merely unfixed:
- `assert_indexes_cover_every_document` (db.zig) checks the index invariant
directly -- an index generates candidates and the full filter is re-applied to
those, so a missing entry is a missing query result nothing else detects.
- `Index.unreachable_key_count` counts keys present in the leaf chain but not
reachable by descending from the root, which is precisely the state above:
healthy by every other measure.
- `Index.dbg_root` dumps parent/chain agreement. Marked TEMPORARY; drop it once
the invariant checks have earned their keep.
- `crash-fuzz.js` now asks the same question without the index, so a failure
says whether the documents are wrong or only the index's answer about them,
and reports per-key totals so one lost leaf is distinguishable from an empty
index.
Verified: `zig build test` in ReleaseFast and ReleaseSafe, and seeded fuzzer
runs that previously reproduced the split bug.
|
|||
| 62caf9fefc |
tests/fuzz: tighten the listIndexes NamespaceNotFound comment
Comment-only cleanup: the six-line rationale restated the scenario twice (first-cycle crash at prefix 0 = kill during the first in-flight command on a fresh log) and echoed 'expected state' with 'exactly the case worth verifying'. Four lines keep all three points: real MongoDB answers NamespaceNotFound too, it is expected when nothing durable created the collection, and treating it as a harness error broke verification of that case. |
|||
| a9f625f82a |
tests/fuzz: listIndexes on a namespace the prefix never created is expected
`crash-fuzz.js` aborted with "harness error: MongoServerError: ns not found" whenever the surviving prefix contained no write that created the collection -- a kill during the first in-flight command on a fresh log. `listIndexes` on a missing namespace is NamespaceNotFound, which is what real MongoDB answers too, so the server was right and the harness treated a legitimate state as its own failure. Worse, it aborted the run instead of verifying that state, which is exactly the state worth verifying. Reproduces with `--seed 1234 --rounds 60` and is why seeded runs were unusable; `--heavy` happened to miss it. Confirmed against the previous commit before changing anything, so it is the harness and not the engine. Both seeds now pass 60 cycles. |
|||
| 319a515b89 |
pager: reuse the data file when there is no checkpoint to honour
`Pager.open` set `alloc_tail` to the end of the existing file -- "everything already in the file is allocated until a watermark narrows it down". Safe when a watermark exists. When one does not, it is the opposite of safe: nothing in the file is referenced, the log is the whole truth and replay is about to rebuild the slab, the trees and the overflow from it, so every reopen started allocating *above* the previous copy. With no watermark there is also no free list, so the old copy was never given back. Linear growth per open, unbounded. It does not need a crash. A database small enough never to reach the 32 MB checkpoint threshold never publishes a watermark at all, so *every* clean reopen took this path: 20 documents inserted per cycle, 12 reopen cycles before 17, 34, 50, 67, 85, 102, 118, 135, 168, 201, 236, 269 MB after 17 MB, flat 240 documents in a 269 MB file, heading for `DatabaseTooLarge`. The crash fuzzer shows the same thing under a real workload -- 60 crash/reopen cycles with ~460 documents ended at 2735 MB before, 17 MB after, with the prefix invariant holding either way. That number was sitting in its own output as `data=2735MB` and reads as normal until you divide it by the document count. The file is deliberately not truncated. The mapping already covers these pages and `grow_to` extends the file only when the mapping is too small, so shortening the file behind a mapping that still spans it would turn a later write into SIGBUS. Reusing from the front is what the unbounded growth needed; giving the disk back is a separate change to the same function. Mutation: leave `alloc_tail` at the file end -- red on the new test, which opens, writes and closes three times without a checkpoint and requires the third tail to be within one slab extent of the first. |
|||
| 1814020df9 |
db/pager: an append resumes inside its extent after a checkpoint
`slab_reserve` and `reserve_overflow` abandoned the rest of their extent whenever a checkpoint froze the page the tail pointed into, and took a fresh 8 MiB one. The comment called the waste "bounded by one extent per collection per checkpoint", which is true per checkpoint and says nothing about the sum: nothing reclaims it except a rebuild, and a rebuild only runs when there is garbage. A pure-insert workload produces none. Measured, 40 collections of inserts with incompressible payloads so the log actually reaches the checkpoint threshold: live data file log 29 MB 340 MB 29 MB 38 MB 542 MB 5 MB <- checkpoint 67 MB 681 MB 33 MB 76 MB 1076 MB 9 MB <- checkpoint 115 MB 1357 MB 14 MB <- checkpoint 11.8x the live data and climbing by ~335 MB per checkpoint (40 x 8 MiB), which would exhaust the 64 GB address-space reservation after roughly 6 GB of real data -- and after ~1.2 GB with 200 collections. `DatabaseTooLarge` on a database that is nowhere near too large. The fix is what the plan called for and never got: round the cursor up to the next *system* page and keep the extent. Only the page holding the live tail is in the published image; the rest of the extent holds nothing referenced by the image or by an index, so `Pager.mark_appendable` hands it back for appending (and unprotects it, since it may sit below the stable mark where `protect_image` made it read-only). System pages rather than 4 KiB ones because writeback tears at the granularity the kernel manages: a 4 KiB store dirties a whole 16 KiB page on Apple Silicon, and tearing there would take out the published bytes sharing it. Same 40 collections after: 340 MB -> 352 MB across three checkpoints, the ratio falling monotonically toward the 8 MiB-per-collection floor. 64,000 documents across 8 collections verified byte-for-byte and after a kill -9. The churn gate is unchanged at 1.65x, big.js at 4 GB unchanged (4.32 GB file, reopen 0.5 s, RSS after reopen 130 MB). Two mutations, verified red: dropping the resume branch (a fresh extent per checkpoint), and rounding to `page_size` instead of `map_align` (the resumed append then shares a system page with the published image). |
