1892cb796951d91c483dec7116bf0e81dccce355
92 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
A.Shakhmatov
|
200228b0cb |
commands: arrayFilters and the query reach the update
The engine landed last commit with nothing feeding it: `$[<identifier>]` had no filters to bind and `$` had no query to resolve against, so both refused correctly and uselessly. This is the plumbing. `arrayFilters` is read per update statement on `update` and once on `findAndModify`, which is where each command carries it. Only the *shape* is checked here -- an array, of documents, TypeMismatch (14) with mongod's own field names for either -- because which identifier a filter names, whether it is spelled legally and whether the update ever uses it all need the update's paths, and those belong to `update.validate`. `validate` is called before `scan_matching`, not inside the per-document loop, and that placement is load-bearing in both directions: an array filter the update never uses is refused (9) even when the query matches nothing at all, and an identifier nothing binds is refused (2) before a single document is read. Both measured; a test pins each, with the mutation that reddens it named in the comment. Positional corpus 23 -> 51 of 51, the gate green. Pinned crud scorecard 204 -> 218 pass, 87 -> 73 fail: the whole arrayFilters cluster, which until two commits ago was answering ok: 1 having replaced the array with a document keyed by the path segment's literal text. 222/222 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz. |
||
|
A.Shakhmatov
|
a05874d597 |
update: a positional path resolves to the elements it names
`$[]`, `$[<identifier>]` and `$` stop being refused and start being walked.
A path with a positional segment names a *set* of concrete paths rather than
one -- `y.$[].b` on a two-element array names `y.0.b` and `y.1.b` -- so
`resolve` expands it against the document at hand and every operator then
walks paths it already knew how to walk. Nothing below `resolve` knows a
positional segment exists, which is why `$set`, `$inc`, `$unset`, `$push` and
`$pull` all get it at once.
The static half -- which identifier binds to which array filter, whether a
segment may sit in first position, whether a filter went unused -- depends
only on the update and the filters, so it runs once in `validate` before any
document is touched. The document-dependent half is resolution itself: an
absent or non-array path is a refusal there, because array updates address
what is there rather than creating it, unlike `$set` on a plain path.
Codes and messages measured on mongod 8.3.7, not recalled. Sixteen shapes
were run; the ones that changed what this commit does:
- `$[]` in first position answers the *array filter identifier* message,
not the `$` one -- mongod treats the two spellings as one check.
- an array filter may have several top-level fields as long as they all
name the same identifier: `{i.b: 3, i.c: 1}` is legal, `{i.b: 3, j.b: 1}`
is not. So the check is on the name, not on the count.
- an identifier is `[a-z][a-zA-Z0-9]*`: `aB2` yes, `Ab`, `a_b`, `1x` no.
- `$rename` refuses a positional path on *either* end, with its own message
for each, so it keeps the plain split rather than resolving.
- `$unset` of an element leaves a null in its place rather than shortening
the array -- the same answer `$unset: {"y.0": ""}` already gave.
- a scalar element with more path below it is PathNotViable (28), not a
field to create. Without that check the walk would hand `y.1.b` to
`set_path` and it would replace the `7` at `y.1` with `{b: 9}` -- a
smaller copy of the destruction this whole walk replaced.
One divergence, measured and deliberate: `{"y.b": 3, "y.c": 2}` matches
`[{b: 3, c: 1}, {b: 1, c: 2}]` without either element satisfying both, and
`$` then picks element 1 on mongod and element 0 here. mongod's answer is an
artefact of which predicate last wrote its match position; guessing at it
would be worse than recording it. PLAN §6.
The command handlers still pass neither the query nor any array filters, so
over the wire this is `$[]` working and the other two refusing -- for the
right reason and with the right code, which they did not before. The plumbing
is the next commit.
221/221 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz.
Positional corpus 15 -> 23 of 51.
|
||
|
A.Shakhmatov
|
3c2ac38fd9 |
commands: drop does not unlock the collection it just freed
A use-after-free. Dispatch held `drop`'s collection lock across the handler,
`drop_collection` freed the Collection the lock lives in, and dispatch then
ran `unlock_collection` on freed memory -- an atomic read-modify-write
inside `Io.RwLock.unlock`. One insert and one drop was enough.
Two things kept it hidden for this long. `drop` had no unit test at all:
before this commit every `parse_fake_msg("drop", ...)` in the tree was in a
test written to hunt it. And over the wire it does not fault -- 25
insert/drop cycles against a live server pass -- because the general
allocator leaves the freed page mapped and the atomic write lands somewhere
harmless. That was luck, not safety: the same undefined behaviour either
way, and testing.allocator is what makes it visible, which is why the
regression test is a unit test rather than an e2e script.
Fixed by giving `drop` no collection lock at all. The catalog lock is what
actually excludes here: every collection lock in this engine -- dispatch,
the TTL sweep, `compact`'s rebuild, `write_catalog`, `slab_stats`,
reclamation -- is taken while holding the catalog at least shared, so
holding it exclusively already keeps every one of them out. The collection
lock was buying exclusion that was already there and paying for it by
locking an object about to cease existing.
That exposed a second bug rather than creating one. The dispatch epilogue --
commit, then maybe checkpoint, then maybe compact -- fired on
`locks.coll == .exclusive` as a stand-in for "this was a write". It is now
keyed on `kind == .write`, because the two agreed only by accident:
`dropDatabase` is the one write that never held a collection lock, so it has
never reached that epilogue, and a dropped database waited for some later
write to trigger the checkpoint that records it.
Mutation-checked: restoring `.coll = .exclusive` on the drop row reproduces
the original SIGSEGV in the new test.
208/208 unit (2 new) in ReleaseFast and ReleaseSafe, 83/83 fuzz, crud
scorecard unchanged at 204/87, aggregation corpus 70/0, full e2e matrix,
crash-fuzz and the 25-cycle wire drop probe green.
Left open and recorded in PLAN §6: `drop_collection` still writes no log
record, so a dropped collection resurrects on reopen unless a checkpoint ran
(pre-existing, with its own test at db.zig:5542); and `apply_pending_write`
drops under `engine.rwlock` rather than the catalog lock, so the two drop
paths disagree about which lock protects a namespace.
|
||
|
A.Shakhmatov
|
f04e7125c9 |
update: refuse a positional path instead of destroying the array
`{$set: {"y.$[i].b": 2}}` did not fail to update the array. It replaced
`y: [{b: 3}, {b: 1}]` with `y: {"$[i]": {"b": 2}}` -- every element
discarded -- and answered ok: 1, modifiedCount: 1. Remotely reachable by any
client issuing an ordinary MongoDB update.
`arrayFilters` was not implicated: the string appears nowhere in src/, the
option is accepted off the wire and dropped. The destruction was in the
path, at `set_path`'s "treat as non-array: replace with a doc" branch, so it
fired for all three spellings of "descend into this array" -- `$`, `$[]` and
`$[<ident>]` -- under every operator. `$inc` through `$[i]` stored its
operand rather than incrementing.
Two refusals, because the branch held two different mistakes:
- a positional segment is refused up front, before anything is applied, so
an update naming a good path and a positional one lands neither. Its
code is BadValue (2), which is what mongod answers for every positional
path failure.
- a plain non-numeric segment under an array -- `y.nope.b`, `y.$x.b` -- is
PathNotViable (28), measured. It is never a field to create, which is
the opposite of what `set_path` does for a missing *document* field and
the reason this branch existed at all.
Numeric segments are untouched, including the null padding past the end,
which a test now pins.
Messages are this server's own words. mongod's PathNotViable text embeds a
shell-syntax rendering of the offending element (`Cannot create field 'nope'
in element {y: [ { b: 3 }, { b: 1 } ]}`) and no BSON formatter here produces
it. The code is what the corpus asserts and the code is exact; a half-copy
of the text would be worse than a clear sentence that does not pretend.
Re-running the 17-case probe against both servers: 16 of 17 diverged before,
0 are destructive now, and 5 agree with mongod's code exactly -- every case
where mongod also refuses. The rest refuse where mongod succeeds, which is
the honest not-implemented state and is what the design review chose.
Scorecard unchanged at 204 pass / 87 fail: the 14 arrayFilters cases still
fail, now reporting the refusal rather than a corrupted document. That was
the gate this review picked -- `docs/M3_ARRAYFILTERS_DESIGN_REVIEW.md` §5,
option D -- because the corpus has no `$[]` case and no bare `$` case at
all, so passing it would have certified two live ways to destroy an array.
206/206 unit (8 new) in ReleaseFast and ReleaseSafe, 83/83 fuzz, aggregation
corpus 70/0, full e2e matrix and crash-fuzz green. Both refusals are
mutation-checked: dropping the up-front scan reddens four tests on the
array's contents rather than on the error, and restoring the destructive
branch reddens the PathNotViable test.
|
||
|
A.Shakhmatov
|
1f141ef619 |
commands: distinct
A whole command that did not exist: five corpus cases answered "no such
command". Two things about it were measured against mongod 8.3.7 rather
than recalled, and the first is not what anyone would guess.
- The answer is **sorted in canonical BSON order**, not in the order the
values were met. `{s: "b"}, {s: "a"}, {s: null}` answers
`[null, "a", "b"]`. Insertion order is the obvious implementation, it
passes every test anybody would think to write by hand against
`[11, 22, 33]`, and it is wrong.
- Deduping is the same comparator, so an int32 `1` and a double `1.0`
collapse while `null` and `"1"` survive.
Both fall out of `bson.compare`, which `$sort` and `$min` already use --
and that is not luck: mongod accumulates into a `BSONElementSet` ordered by
the same `woCompare`. The rest reuses the shared read path: byte-walked
`collect_values_bytes` for the key, so the traversal, the multikey descent
and the numeric path segments are the ones the matcher and the index
already agree on.
Also measured: a terminal array contributes its elements exactly one level
deep (`[[7, 8], 9]` gives `[7, 8]` and `9`, never 7 and 8); a missing field
contributes nothing where an explicit null contributes null; an absent
collection, an absent database and an empty key are each `ok: 1` with an
empty array rather than an error; `query` absent and `query: null` are both
an empty filter; a missing `key` is IDLFailedToParse (40414) while a
wrong-typed one is TypeMismatch (14).
Running the identical probe against both servers now agrees on every
semantic row. Three divergences remain, all outside this command and
recorded in PLAN §6: an unknown query operator matches nothing instead of
erroring (shared with find/count/aggregate, and the same class as M2's six
silent wrong answers), a non-string collection name is refused by dispatch
as BadValue where mongod says InvalidNamespace, and an unknown top-level
field is tolerated -- deliberately, since `comment` and `rawData` arrive
through that door and the corpus requires both be ignored.
crud scorecard: 201 pass / 90 fail -> 204 / 87. distinct.json 0/2 -> 2/0,
distinct-rawdata 0/1 -> 1/0. distinct-comment nets zero: its "no such
command" is replaced by the pre-4.4.14 document-comment case, which
`estimatedDocumentCount` already carries as a standing failure -- and
emulating a bug fixed in 4.4.14 for one command would make the two
disagree. distinct-collation still needs M8, but now fails with the honest
"expected 1 elements, got 2".
198/198 unit (7 new) in ReleaseFast and ReleaseSafe, 83/83 fuzz, aggregation
corpus 70/0, full e2e matrix and crash-fuzz green.
|
||
|
A.Shakhmatov
|
fc611a2c64 |
commands: $project computes, renames and narrows
The last three cases of the corpus, which is now 70 pass / 0 fail -- every
answer byte-identical to mongod 8.3.7 across the accumulators, the expressions
and the document stages.
The fix turned out to need nothing from `query.project`, which `find` shares
and which I had expected to have to rewrite. A nested spec *is* a dotted path:
`{n: {x: 1}}` and `{"n.x": 1}` are the same projection, and dotted paths are
something the existing projection already narrows correctly. So `$project` is
flattened into inclusion/exclusion flags plus a list of computed fields, and
both halves reuse machinery that was already there -- `query.project` for the
flags, `set_path` from the document stages for the computed fields. A bare path
(`{value: "$a"}`) is a rename, which is a computed field like any other.
Both shapes used to read as *falsy*, which flipped the whole projection into
its exclusion branch and returned the entire document minus the field. That was
recorded during M2 as broken rather than unimplemented; this is the fix it was
waiting for.
One case `query.project` genuinely cannot express, so it is built directly: a
projection that only computes keeps `_id` and nothing else, and with no non-`_id`
flag that function reads the spec as an exclusion and returns everything. It
cost two failures and a `id_only` flag to find, which is what a recorded corpus
is for -- the answer is obvious once seen and not before.
`$project` now goes through the same `Rewrite` path as `$addFields`, `$unset`,
`$replaceRoot` and `$unwind`, so its own branch is gone. Its refusal shrank to
the one shape mongod also refuses, mixing inclusion with exclusion, judged on
the flattened flags so a nested spec is treated like a dotted one.
191/191 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2
concurrent, e2e3 16, e2e4 17, e2e6 72, e2e7 86, crud corpus unchanged at
201/90/196.
|
||
|
A.Shakhmatov
|
1a5386ff00 |
commands: the stages that rewrite a document
`$addFields`, `$set`, `$unset`, `$replaceRoot` and `$unwind`. The corpus goes
0 pass / 24 fail to 21 / 3, and the three left are `$project`'s computed
fields, renames and nested inclusions, which want the `query.project` that
`find` shares and are their own change.
**The design review was wrong about what this needed, and the corpus is what
settled it.** Tier 2 was scoped as a per-stage iterator on the grounds that
`$unwind` is 1->N and "there is no way to express that in a window over the
input". That was true of the window as it stood, and stopped being true the
moment `$project` was made to rebuild the stream instead of moving bounds over
it -- a stage that rebuilds can emit as many documents as it likes, or none. So
all five share one shape: read the window, build a new list, replace the
stream. No iterator, no rewrite.
What the recording settled, and what a hand-written test would have got wrong:
- `$addFields` whose expression resolves to nothing leaves the field out
entirely rather than setting it to null -- so `set_path` is only reached
when there is a value, and `eval_expr`'s absent/null distinction earns its
keep a second time.
- `$addFields: {"n.z": 1}` sets the nested path and keeps its siblings, and
an existing field is replaced *where it stands*, which is what makes the
stage "add or overwrite" rather than "append".
- `$unwind` drops a document whose field is missing or an empty array, keeps
one whose field is not an array *whole*, and numbers `includeArrayIndex`
from zero. Three separate behaviours where one guess would have covered
them all wrongly.
- `$replaceRoot` of a missing path and of a non-document are the same error,
40228.
`ReplaceRootNotDocument` joins `EvalError` rather than being reported at the
stage: it is a failure only a document can produce, which is the line that set
already draws.
191/191 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e3 16,
e2e4 17, e2e6 72, e2e7 86, crud corpus unchanged at 201/90/196.
|
||
|
A.Shakhmatov
|
37cfa863ee |
commands: the aggregation expression evaluator
The whole corpus is green: 46 pass, 0 fail, byte-identical to mongod 8.3.7 on
every case including all 27 expressions and the compound `_id` that was the
last accumulator failure.
Expressions are *compiled once per pipeline and evaluated per document*, and
that split is the point rather than an optimisation: it keeps the property M2's
refusals bought, which is that a pipeline that cannot be answered is refused
before a single document is read instead of half way through with part of the
work already reported. `Expr` is the compiled tree, `compile_expr` reports,
`eval_expr` cannot.
Nineteen operators: `$literal`, the five arithmetic ones, seven comparisons,
`$and`/`$or`/`$not`, `$cond` in both its forms, `$ifNull` and `$switch`. Plus
the two shapes that are not operators at all -- a compound document, which is
what a `$group` `_id` usually is, and an array.
Everything the corpus recorded, and none of it guessable:
- absent and a present null are *different* internally, because `$ifNull`
treats them alike and `$push` does not. Hence `?bson.Value` throughout,
where the obvious shortcut is to fold absent into `.null` at the boundary
and lose the distinction for good.
- arithmetic over absent or null is `null` -- not an error, not zero -- and
over a string is an error, 7157723.
- `$divide` by zero is 4848401, `$switch` with no branch and no default is
40069, and both are failures only a document can produce, so `EvalError`
exists and `report_eval_error` maps it.
- two operators in one expression document is 15983 and *not* `$group`'s
40238: mongod distinguishes an expression from an accumulator there.
- truthiness is MongoDB's, so `-5` is true and `0.0` is false.
- `$mod` follows the dividend's sign, so -5 mod 4 is -1.
- `$not` takes a bare argument as readily as a one-element array.
`compile_expr` and `compile_operator` call each other, so their error set is
written out rather than inferred -- Zig cannot infer a cycle, and the failure
mode is a "dependency loop" message that says nothing about expressions.
Three cases left the Tier 0 refusal test, because a compound `_id`, `$literal`
and a `$multiply` argument all work now. What is refused should be what is
missing, so an unknown operator and a wrong operand count took their place.
191/191 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e2
concurrent, e2e3 16, e2e4 17, e2e6 72, e2e7 86, crud corpus unchanged at
201/90/196.
|
||
|
A.Shakhmatov
|
76afa75efe |
commands: the $group accumulators
Nine of them -- `$sum`, `$avg`, `$min`, `$max`, `$first`, `$last`, `$push`,
`$addToSet`, `$count` -- and the corpus goes 9 pass / 10 fail to 18 pass /
1 fail. The one left is the compound `_id`, which needs the expression
evaluator.
Landed before that evaluator, against the tier order the design review set
out, and the corpus is why: every one of these takes a single value per
document, a path or a constant, so nine of its ten failures turned out to be
reachable without one. `classify_expr` already produced exactly that value.
What the recording caught, which is the argument for measuring expectations
rather than writing them:
- `$avg` over a group with no numeric value is **null**, not `0`. A divisor
that counted documents rather than numbers would pass every test anybody
would think to write by hand, and be wrong on the one group that matters.
- `$min`/`$max` compare across types in canonical BSON order, so the maximum
of `30`, `7` and `"not a number"` is the string.
- `$push` skips an absent field but would push an explicit null, so "resolved
to nothing" and "resolved to null" cannot be the same value internally --
which is why the accumulators take `?bson.Value` and not `.null`.
- `$first`/`$last` follow input order, including when the value is absent:
`$last` of a missing field is null, not the last present one.
`AccState` is one struct rather than a union: the fields are small and every
site already switches on the kind, so a union would add a tag test where a
switch was going to be anyway. Its arrays are the gpa's, the values inside them
the reply arena's -- they outlive the group and travel with the documents.
`numeric_value` is the int32-or-double narrowing MongoDB reports, shared now
between the accumulators and `cmd_aggregate`'s count fast path. It was written
twice before; a divergence between them would make `countDocuments` disagree
with the pipeline it is a shortcut for.
`$avg` and `$push` came out of the Tier 0 refusal test, replaced by
`$stdDevPop` and `$mergeObjects`. The refusal is a property of what is missing
rather than of a list, and the test should read that way.
191/191 unit tests in ReleaseFast and ReleaseSafe, 83/83 fuzz, e2e 49, e2e3 16,
e2e4 17, e2e6 72, e2e7 86, crud corpus 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
|
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
|
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
|
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
|
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
|
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
|
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
|
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.
|
|||
| 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). |
|||
| 21489723a9 |
db: a write that changes nothing is not a write
`nModified` counted every write, so an update that altered nothing was reported
as a modification. MongoDB counts a document as modified only if applying the
update changed it, and writes no oplog entry when it did not: `$set: {x: 11}`
on a document already holding `x: 11` is matched and not modified. The spec
suite says it plainly -- `bulkWrite` with four updateOne operations expects
matchedCount 2 and modifiedCount 1.
Decided in the engine rather than the command, because that is where the
document is already serialized: the comparison is against the bytes that would
actually be stored, and it lands before the log append, so a no-op costs no log
record, no fsync, no slab bytes and no garbage. `Engine.replace` returns
`Written.modified` or `.unchanged` and `cmd_update` counts the first.
That exposed a second difference. A replacement keeps `_id` at the front, so
replacing a document with itself was a byte-level change whenever `_id` was not
stored first -- and it usually was not: the Node driver fills a missing `_id` by
assigning the property, which in JavaScript appends it, so `insertOne({name,
age})` reaches the server as `{name, age, _id}` and we stored it that way.
MongoDB moves `_id` to the front whatever order it arrives in. Now so does
`serialize_with_id`, for every document rather than only the ones whose `_id` it
generates. Visible to clients as `_id` coming back first, as it does from
MongoDB.
spec scorecard 161 pass / 131 fail -> 163 pass / 129 fail
bulkWrite.json 8 pass / 2 fail -> 10 pass / 0 fail
e2e.js 45 checks -> 49
No spec file regressed. Mutation: delete the byte comparison in `upsert`'s
`.replace` arm -- red on the log growing, on the garbage counters moving, and on
`replace` claiming `.modified`.
|
|||
| 53f88e6d3b |
update: replacement-style writes
`replaceOne`, `findOneAndReplace` and `bulkWrite`'s `replaceOne` all failed
with "bad update". `update.apply` rejected any update document whose first key
was not `$`-prefixed, so a replacement document -- which by definition has no
operators -- could not get through at all.
MongoDB decides on the first field and nothing else: `$`-prefixed means
operators, anything else means the document *is* the new content. An empty
document is a replacement too, and a legal one. `is_replacement` says which,
`apply_replacement` does the work, and because all three call sites already go
through `apply`, that one branch covers the update command, findAndModify and
the upsert builder.
What a replacement means, precisely:
- every field is replaced except `_id`, which is immutable and keeps its
position at the front, where it is stored and where the `_id_` index
descends on it;
- a replacement may restate the same `_id` but not a different one -- that
is `ImmutableId`, because otherwise a rewrite would silently change a
document's identity while the index entry kept the old key;
- when the target has no `_id` yet, the replacement supplies it. That is the
upsert path: `build_upsert_doc` seeds a document from the filter's
equalities, so `replaceOne({_id: 99}, {u: 1}, {upsert: true})` inserts
`{_id: 99, u: 1}` and not a generated ObjectId;
- a mixed document is refused from either side, rather than guessed at.
Two options on update specs are refused rather than ignored:
- `multi` with a replacement (FailedToParse). A replacement describes one
document; applying it to many would leave every match identical apart from
its `_id`.
- `sort`, a MongoDB 8.0 addition this server does not implement. Ignoring it
is the worst of the three answers -- `sort` chooses *which* match to write,
so the client would silently get a different document than it asked for.
spec scorecard 131 pass / 161 fail -> 161 pass / 131 fail
e2e.js 35 checks -> 45
Sixteen spec files improved and none regressed. The two `-sort` files briefly
did: they had been passing on their "server-side error" case, which our
"bad update" failure satisfied by accident, and passing for the wrong reason is
how a gap survives a scorecard.
Five mutations, each verified red: seeding the replacement from the old pairs,
dropping the `_id` comparison, dropping the `_id` a replacement supplies, and
removing the mixed-document guard from either loop.
Note `nModified` is still wrong for a write that changes nothing -- MongoDB
counts a document as modified only if applying the update altered it. That is
the remaining bulkWrite failure and is fixed next, separately.
|
|||
| 4b70ce6da9 |
pager: a page reservation belongs to its consumer, not to the pager
The promise `reserve_pages` makes was a single counter on the pager, and the
first concurrent benchmark since the data file landed aborted the server on
it, reliably, at four clients:
assertion failed: page allocation overran reserve_pages' promise
src/index.zig:955 in alloc_node
src/db.zig:794 in upsert
Two upserts on different collections hold different collection locks, so they
run at the same time. Each ends by dropping "whatever is still promised" --
and `release_reservation` zeroed the shared counter, so the first to publish
released the second's promise while the second was still between its log
append and its supposedly infallible allocation. The tripwire fired, which is
the good outcome; the bad one is a growth that never happened and a store past
the mapped end.
This is PLAN risk 3 ("a shared pager makes alloc_tail and free_pending a
global mutex on every insert"), whose mitigation -- private pre-allocated runs
-- was never built. So: `pager.Reservation` is a per-consumer promise, held by
every Index, every Collection (for its doc slab) and the checkpoint, and each
one releases only its own. The pager keeps the sum, which is all `grow_to`
needs. `Engine.release_write_reservations` drops exactly the buckets one
upsert reserved through.
The allocator's own state -- the tail, the total, the free lists, the
unpublished set, file growth -- is now behind `alloc_lock`, taken
uncancelable. It is never held across the log append: that is precisely what
per-consumer reservations buy, and why group commit is unaffected.
concurrent durable insertOne 4 clients 21697 docs/s (was aborting)
16 clients 30678 docs/s
Mutation: make `release_reservation` zero `self.reserved_pages` again. Red on
the new pager test and on three command tests.
|
|||
| 5228ed740a |
db/pager: reclaim what churn abandons
The churn gate (PLAN D6.2 as amended, D7.4) measured a data file growing linearly and without bound: 50% churn over six rounds reached 7.2x the live data and was still climbing when the run was stopped. Three separate bugs, each of which alone was enough to make reclamation impossible. **The compaction trigger had been dead since commit 14.** `note_compact` gated on `log.data_bytes`, which was the right question while the log was the only copy of the data. A checkpoint now truncates the log, and `truncate_to_header` zeroes that counter -- so the first gate stopped being reachable and compaction never fired again. Retarget it at the data file, where the garbage now lives: `Engine.live_bytes`/`dead_bytes`, in bytes rather than document counts because a rewrite copies bytes. The engine's live total is the sum over collections by construction, checked in `write_catalog`, which walks every collection anyway. **`stable_pages` is a bound, not a membership test.** `page_mut_cow` asked `p >= stable_pages`, which is right for tail-bumped pages and wrong for recycled ones -- they come off the free list *below* the mark and are nonetheless writable, because two-generation retention means no live image references them. So every write to a recycled node page copied and freed it again, and both append cursors (the doc slab, the overflow slab) abandoned each recycled extent after a single record. Nothing was ever really reused. Replaced with an exact `unpublished` bit set, cleared at each publish: 32 KiB per GiB, one load against the 4 KiB copy it avoids. **First fit let one-page requests dismantle the extents.** Copy-on-write asks for a single page thousands of times per generation while the doc slab asks for 2048-page extents; first fit carved a page off the front of the largest run every time, so the free list drained to empty every generation with the file still growing by the whole write volume. Best fit keeps the runs whole -- nothing else wants the one-page holes -- and `publish` now coalesces adjacent runs, without which the list only ever fragments. Also: a rebuild publishes twice. One publish moves the abandoned extents from `pending` to `hold`; the space is not reusable until a second, so the next rebuild grew the file instead of reusing what the last one freed. Safe for the reason the delay exists -- what the second publish releases is what the pre-rebuild image referenced, and that image is no longer the fallback. Measured, sustained-churn steady state, 40k x 16 KiB documents: delete half and refill, 6 rounds 4.10x climbing -> 1.65x flat random $set over 5x the collection 3.58x -> 2.47x flat Above the 1.3x the amended D6.2 hoped for, and structurally so: a rebuild needs a whole second copy of the live data before the first can be freed. The gate's purpose was to decide whether doc-level free lists are needed post-M0, and this is the answer -- yes, for M1. Five mutations, each verified red: the numeric mark in `page_mut_cow`, first fit in `take_free`, dropping `coalesce_free_ready`, dropping `mark_unpublished`, and dropping the rebuild's second checkpoint. |
|||
| b20ae92cbf |
db: drop the docs hashmap; the _id_ index is the lookup
The last structure holding the engine to RAM. At the target scale it cost 64-100 bytes per document -- 10+ GB at 100M documents -- and PLAN D4 rules it out for exactly that reason. `_id_` was already an ordered B+tree over the canonical `bson.encode_key`, and since the leaf payload became a slab offset it has held everything the map did. So the internal key changes from `serialize_value` to `encode_key` throughout, `lookup_exact` replaces `docs.get`, and the tree's ordered walk replaces the map's hash-order iteration in `create_index`, `rebuild_index`, the rebuild and the TTL sweep -- which reads the slab sequentially where the map read it scattered. `Collection.doc_count` remains, because the compaction trigger wants a count the tree cannot give in O(1). This is what unblocked the milestone's central claim, and the mechanism is worth naming. Opening from a checkpoint had to rebuild the map, and rebuilding it meant reading *every document* to recover its `_id` -- which faulted the entire database in and made "RSS = working set" impossible no matter what else was true. Deleting the map deleted that scan. Measured, 512 MB of documents in 16 KB records, reopening from a checkpoint: RSS after reopen 523 MB -> 50 MB after 4 point lookups 523 MB -> 51 MB The remaining 50 MB is the working set: index pages plus the un-checkpointed log tail being replayed. A checkpoint immediately before shutdown would shrink it further; the point is that it tracks what is touched rather than what is stored. -- Replay now maintains `_id_` as it goes, always, not just after a checkpoint. It is no longer an optimisation: the tree is the only way the next record can find the document it supersedes. Secondaries still wait for the bulk build. And PLAN amendment A4's migration hazard is handled where it actually bites. A database written before `_id_` was canonical could hold two documents whose `_id`s compare equal -- int32 1 and int64 1 -- and replaying it now keeps only the later one. That is MongoDB's semantics and a one-way migration, so replay compares the superseded document's `_id` bytes with the incoming record's and says so out loud when they differ, naming the namespace. |
|||
| 148e03ac9f |
db: compaction becomes a data-file rebuild
`compact` used to re-emit every live document into a fresh log and rename it over the old one. That is the wrong shape twice over now: the log is not where the data lives, and a re-emitted record carries a sequence a later watermark can cover, which would make the next open skip it (PLAN section 4). The log re-emission is deleted; the checkpoint at the end reclaims the log instead. What it reclaims is what a checkpoint cannot. A checkpoint publishes the structures where they already are, and it cannot move a document, because every index leaf holds that document's physical offset. So reclaiming a replaced document's bytes means rewriting the documents *and* repacking every index against the new offsets, together -- which is the whole of `rebuild_collection`. Documents are copied in _id order, so the new slab reads sequentially afterwards. Old extents and old node pages go to the free list rather than being reused immediately, so a crash mid-rebuild simply loses the rebuild: the previous watermark still describes the previous layout, intact. Adds `Collection.slab_used`, because `slab_tail` cannot answer "how many bytes are in use" -- it is an absolute file offset and jumps forward with each new extent. That is also the number the rebuild trigger wants. -- The test is the part worth reading. My first version asserted that every document was still findable and had the replaced contents, and it was nearly useless: two mutations -- not repacking the indexes at all, and not republishing the docs-map offsets -- both left it green. Freed extents go on the free list rather than being overwritten, so a stale offset still reads a perfectly plausible document. What actually distinguishes a repacked index from a stale one is *where* the offset points: after a rebuild every live offset must fall inside an extent the collection currently owns. Asserting that, plus that the index and the map agree, turns all three mutations red -- including repacking `_id_` but forgetting the secondaries. |
|||
| 138b7f706f |
db/storage: reclaim the log once a checkpoint covers it
The point of a lagging checkpoint: a record whose effect the data file already holds is redundant, so the log can go back to just its header. Without this the log only grows and every open pays for every write ever made. Ordering, which is the whole safety argument: publish the watermark, *then* truncate. The other way round, a crash between them leaves the records gone from the log and absent from any image. A failed truncation is a warning rather than an error -- it costs space and replay time, and loses nothing, so it must not fail a checkpoint that already succeeded. Also wires checkpointing up, which nothing did before. `note_checkpoint` arms it when the log passes a threshold, and the write epilogue and the TTL monitor both claim it -- outside any collection lock, for the same reason compaction runs there: it takes the log lock. The threshold is separate from the compaction one on purpose: compaction is about the garbage share of the data, a checkpoint is about how much replay an open would otherwise do. -- Two things the tests taught me. The first version measured the log before the checkpoint and found 16 bytes -- just the header. Appends buffer in the log's open block and only a commit seals and writes it, so there was nothing on disk to shrink. The test commits first now, and says why. And the "no valid watermark" warning fired for every young database, which is its normal state before the first checkpoint. It now distinguishes a watermark that was *written and cannot be read* from one that was never written -- warning about the ordinary case is how people learn to ignore the warning that matters. Mutation-checked, red: skipping the truncation. Not covered, and the test says so: moving the truncation before the publish, whose failure mode is a crash landing between the two. That needs process-level crash injection, which an in-process test cannot express. |
|||
| 58e645b969 |
db: checkpoint the engine, and open from it
`Engine.checkpoint()` publishes the current state: commit first, then snapshot the catalog under the catalog lock, then validate the snapshot against an unchanged `seq` under the log lock before publishing -- the same bounded-retry shape compaction has always used. The crash-recovery invariant (PLAN D6) reduces to that ordering, and it is asserted: `snapshot_seq <= committed_seq`. The catalog holds what the pages cannot say for themselves: db and collection names, slab extents and tails, and for each index its spec, tree position, overflow extents and id->page table. Written wholesale into fresh pages each time, never mutated in place, so the previous copy stays valid under the previous watermark until the new one switches over -- untearable by construction, which is why there is no incremental update path. Every read is bounds-checked, because the bytes come off disk and a scrambled catalog must produce an error the caller can fall back from. `Log.replay` takes a `from_seq` and skips below it before the BSON parse. The walk still visits every block, because that is what leaves `end_pos` correct for the next append; making opens *fast* is the job of truncating the log, next. A failed catalog load warns, discards what it loaded, and replays the log in full. The log is untouched at this commit, so that fallback is real rather than aspirational -- which is the reason to land this before truncation. -- The docs hashmap is deliberately *not* in the catalog. It is still the authoritative _id lookup, but putting it there means writing a format the commit that drops it would only delete again; it is rebuilt by walking the `_id_` tree, which the data file already holds. -- One real bug, and it is the interesting part. Replay does not maintain index entries -- it puts documents in place and lets `build_all_indexes` bulk-pack afterwards, which is O(n log n) once rather than per record. After a checkpoint that is wrong: the indexes arrive already populated, `rebuild_index` skips a non-empty one by design, and every record replayed on top was invisible to every index. The symptom was a document present in the collection and absent from `_id_` -- which, once the hashmap goes, means simply absent. Replay now maintains entries when it opened from a checkpoint, and keeps the bulk path for a full one. `Engine.seq` is restored, which it never was: it restarted at 0 on every open. The mutation for it is *not* covered and the test says so rather than implying otherwise -- the sequence is seeded from the watermark, so it only drifts by the records replayed on top, and the catalog carries those same records in every sequence a unit test can reach. Observing the drift needs a crash between a duplicate-sequence append and the checkpoint that would have captured it. The line stays because a log without monotonic sequences has no total order. |
|||
| d7f7ebb994 |
pager: copy-on-write above the stable mark
The invariant everything else in the crash story rests on (PLAN amendment A1): no page belonging to the last published image is ever stored into, so recovery is `image + replay(seq > watermark)` and the image's bytes are exactly what the watermark described. `page_mut_cow` takes a *pointer to the owner's page number*. That is the load- bearing detail: copy-on-write relocates the page, so the owner has to be told, and a second reference would still aim at the abandoned copy. For the B+tree the owner is the id->page table slot -- which is precisely why node ids are not page numbers. Inert until a checkpoint publishes something, since the stable mark starts at zero. Append-only consumers keep writing in place, except that a checkpoint landing mid-extent freezes the page their tail points into, so both slabs now start a fresh extent rather than writing inside the image. Waste is bounded by one extent per collection per checkpoint. The free list is wired into allocation, which it was not before: copy-on-write abandons every page it touches in every generation, so without reuse the file grows by `generations x touched_set` without bound. That is the difference between a free list being defense-in-depth and being a prerequisite (A2). -- Three things I got wrong on the way, all worth recording. I added a `p >= stable_pages` assert to `page_mut` and had to take it back out. A page recycled off the free list *is* below the mark and *is* legitimately writable -- freed two generations ago, referenced by no live image -- so the page number alone cannot tell a violation from a reuse. The invariant is enforced the two ways A1 actually describes: structurally through `page_mut_cow`, and mechanically through mprotect. The comment says so, since the assert looks like an obvious thing to add. The watermark slots needed a narrow exception, because overwriting the inactive slot is the publication mechanism rather than a violation. It is a separate non-public accessor that asserts its argument is a slot, so it cannot become a general escape hatch. And the mprotect belt: `std.posix.mprotect` does not exist in Zig 0.16, so it is a libc call. It compiled only in ReleaseFast, where the branch is comptime- eliminated -- ReleaseSafe caught that immediately, which is the argument for running both. What the belt's test asserts is that the protection is really applied, not that a violating write faults. A SIGSEGV cannot be caught in-process, and the fault is the OS's behaviour rather than this code's; an mprotect that failed silently would leave a belt that looks present and does nothing, which is the failure worth guarding here. Stated in the test rather than implied. Mutation-checked, all red: COW returning without copying; copying without moving the slot; copying when already above the mark; never reusing a freed page. |
|||
| 2e7f72074f |
index: the node arena and overflow slab live in the data file
The last structures move onto the pager, so the whole engine's storage is now
one mapped file plus the WAL.
Node ids are deliberately *not* page numbers. PLAN amendment A1 explains why:
`Node.parent`, `next`, `prev` and an internal slot's `extra` are back-pointers
by id, so copy-on-write moving a page would force every node referring to it to
move as well -- COWing one leaf cascades through the leaf level, one internal
node through its whole subtree. An in-RAM id->page table makes the table slot
the single owner of a page number, so COW has exactly one pointer to fix. It
costs one dependent load per node access and 4 bytes per node, about 5.6 MB at
100M documents, against the 64-100 bytes *per document* this milestone removes.
The overflow slab becomes extents too, so `Slot.off` for a spilled record is an
absolute file offset -- the same change documents went through.
--
Two bugs, both found by measuring rather than by reading, and both worth
recording because the second one would have been invisible until the churn gate.
The reservation was a tail mark, and it cannot be: an upsert reserves tree pages
for every index *and* slab room for the document, all before one log append. The
second reserver overwrote the first one's promise and the first one's allocation
then asserted. Caught on a 512 MB load by the tripwire added in the
`reserve_for` commit, which is the entire reason that assert exists. It is a
count now, and the multi-consumer ordering is pinned by a test.
And a reservation was never released. It is scoped to one write -- taken before
the log append so the publish cannot fail -- but a tree reservation covers the
worst case of several splits while a typical insert causes none, so the promise
accumulated by a handful of pages per write and dragged the file up with it. The
data file was **1.89 GB for 512 MB of documents**; releasing the unclaimed
promise at the end of each write brings it to 551 MB, or 1.08x, which is the
extent slack and the node pages.
--
Measured on one harness, 512 MB / 16 KB docs, against the in-RAM engine this
replaces:
bulk insert throughput 742.6 MB/s -> 736.4 MB/s
insertOne (sequential) 0.20 ms -> 0.22 ms
createIndex({k: 1}) 26.8 ms -> 16.5 ms
countDocuments({}) 2.1 ms -> 1.1 ms
findOne({k: 500}) indexed 0.75 ms -> 0.56 ms
find({p: range}).count() 6.6 ms -> 4.6 ms
aggregate $group by k 5.8 ms -> 3.8 ms
updateMany({k: 7}, {$inc}) 1.2 ms -> 1.0 ms
Reads gain from one contiguous mapping; the two write rows are within noise of
flat. RSS is still unchanged and still cannot improve, for the reason given in
the previous commit: every open replays the whole log and rebuilds everything.
The dev harnesses each open their own data file now. `zig build fuzz` caught all
four of them, again.
|
|||
| 9dda943f26 |
db: documents live in the data file
Collection's slab of malloc'd 8 MiB segments becomes extents in the data file,
and a document's offset becomes an absolute file offset. That makes `doc_bytes`
base + off instead of a binary search over segment starts, and it removes the
hazard the segment list carried: the mapping's base never moves, so a slice into
it cannot be invalidated by growth. phase8 records a dangling-slab-pointer bug
of exactly the shape this deletes.
`slab_reserve` now runs before the log append and `slab_append` after it, and is
infallible. It had no reservation before because appending to an ArrayList could
only fail on OOM; a file-backed slab can also fail on growth, and failing after
the record is durable would report an error for a write the next open produces
anyway.
The data file is still recreated empty on every open and the log still replays in
full, so nothing durable depends on it yet and open/close semantics are
byte-for-byte what they were. The watermark that turns it into a checkpoint comes
next.
--
One bug, and it is worth reading because the milestone keeps producing this
shape. Collection holds a `*Pager`, and `Engine.open` builds an Engine on the
*stack* and returns it by value -- so every pointer taken during replay dangled
the moment it was moved. It surfaced as SIGBUS, then as a corrupt hashmap in the
*second* engine of an unrelated test: nothing resembling its cause. The pager is
heap-allocated now, as `Index` already was, for the same reason.
--
Measured on one harness, 512 MB / 16 KB docs, before and after:
bulk insert throughput 742.6 MB/s -> 746.7 MB/s
createIndex({k: 1}) 26.8 ms -> 16.2 ms
countDocuments({}) 2.1 ms -> 1.1 ms
findOne({k: 500}) indexed 0.75 ms -> 0.53 ms
find({p: range}).count() 6.6 ms -> 4.1 ms
aggregate $group by k 5.8 ms -> 3.7 ms
insertOne (sequential) 0.20 ms -> 0.20 ms
Every row equal or faster. The regression this commit was scheduled early to
catch -- document bytes now reaching disk uncompressed on top of the LZ4 log --
did not appear at this size; bulk insert is flat and the reads gain from one
contiguous mapping instead of separately allocated segments.
What did *not* improve, stated plainly because it is the milestone's headline
claim: RSS is unchanged, 552 MB against 563 MB for 512 MB of data. It cannot
improve yet. Every open still replays the whole log and rewrites the whole slab,
so every page is touched and resident regardless of where it lives. "RSS =
working set" only becomes measurable once an open loads a checkpoint instead of
rebuilding, and the honest test for it is the multi-GB reopen in big.js, not this
microbenchmark.
|
