diff --git a/PLAN.md b/PLAN.md index 2b61653..2959b07 100644 --- a/PLAN.md +++ b/PLAN.md @@ -434,6 +434,19 @@ the unevaluated expression, `$literal` is echoed back, and a `$match` after a answer `ok: 1` with a wrong result; only `$addFields` fails honestly. Doing the command surface first leaves those alive for a further milestone. +*Measured after Tier 0 landed, and it corrects this amendment:* pricing the 13 +failures one by one shows **6 of them cannot turn green in M2 at all** — three +need `$listLocalSessions` (M4) *and* `$addFields` (M2.5), two need the +expression engine, one needs a collation. The gate reads "0 fail among the +seven reachable", not "0 fail". And the seven that remain, `$out` and `$merge`, +need a lock-model decision this amendment did not anticipate: they write to a +collection the pipeline is not reading, which the dispatch contract, the static +lock table and `Collection.lock`'s one-at-a-time invariant all stand against. +It is the same objection that kept `$lookup` out of M2.5's first cut, and the +review failed to apply it to the write stages. Both options are set out in +`docs/M2_DESIGN_REVIEW.md` §7; nothing past Tier 0 is implemented until one is +chosen. + **Which is why M2 carries the refusals.** Every construct the engine does not implement stops answering `0` and starts answering an error, with the code measured against mongod: unknown accumulators, non-path expressions where a @@ -452,7 +465,7 @@ answers, and the trade is only acceptable because the lie is removed first. |---|---|---|---| | M0 | **mmap + WAL foundation** | data file format, page/extent allocator, mmap slab + B+tree arena, copy-on-write + page free list (A1/A2), watermark/replay, checkpoint (= compaction repurposed), leaf payload → slab offset (A3), drop docs hashmap, churn measurement | D7 (6 items) | | M1 | **Cursors + wire polish** | getMore / killCursors / batchSize; server-side cursor state with idle timeout; sessions plumbing (lsid accepted) as drivers send it; hello advertisement updates; **`moreToCome` on requests** (see the bug below); command-monitoring assertions in the spec runner | crud spec suite green; e2e green | -| M2 | **The `aggregate` command surface** | `$out`, `$merge`, `db.aggregate()` (`{aggregate: 1}`), collation, `let`; plus refusing every pipeline construct the engine does not implement instead of answering `0` (amendment A6) | `aggregate-*.json` in the crud corpus: 0 fail | +| M2 | **The `aggregate` command surface** | `$out` and `$merge` (7 of the 13 failures), and refusing every pipeline construct the engine does not implement instead of answering `0` (amendment A6). The other 6 failures are blocked on M2.5, M4 and M8 — see `docs/M2_DESIGN_REVIEW.md` §7 | `aggregate-*.json`: 0 fail among the 7 reachable cases | | M2.5 | **The aggregation engine** | expression evaluator, per-stage document iterator, the accumulators, `$unwind`; `$lookup`/`$facet` explicitly out of the first cut (amendment A6) | a purpose-built stage corpus, every expectation measured against mongod | | M3 | **Update operators + index types** | $setOnInsert, $addToSet, $mul, $min/$max, $pop, $pullAll, $currentDate, pipeline updates; partial + hashed indexes | remaining crud coverage; e2e3/e2e4 green | | M4 | **Sessions + transactions** | logical sessions, snapshot isolation on the mmap engine, write concern at commit | sessions + transactions spec suites green | diff --git a/docs/M2_DESIGN_REVIEW.md b/docs/M2_DESIGN_REVIEW.md index 5b723a5..8f1f87f 100644 --- a/docs/M2_DESIGN_REVIEW.md +++ b/docs/M2_DESIGN_REVIEW.md @@ -273,3 +273,71 @@ memory bound on `$group`/`$sort` joins Tier 2 rather than trailing it. - Whether the expression engine is shared with `update`'s pipeline-update form, which M3 will need. Probably yes; it changes where the code lives. - Index use inside a pipeline beyond the existing leading-`$match` pushdown. + +--- + +## 7. Post-decision measurement: the gate is not reachable either + +Written after Tier 0 landed, when the 13 failures were priced one by one +instead of counted. Section 1 established that the *corpus* named in the plan +was the wrong instrument; this establishes that the *target* set on the +corpus we kept is unreachable too. Both were found by looking rather than by +assuming, and this one only by opening each case. + +| cases | what they need | milestone | +|---|---|---| +| 5 `aggregate-merge.json` | `$merge` with a plain `into` | M2, blocked below | +| 2 `aggregate-out.json` | `$out` with a plain target | M2, blocked below | +| 3 `db-aggregate*.json` | `$listLocalSessions` **and** `$addFields` | M4 + M2.5 | +| 2 `aggregate-let.json` | `let`, `$$var`, `$expr` | M2.5 | +| 1 `aggregate-collation.json` | a case-insensitive collation | M8 | + +So **6 of 13 can never turn green in M2**, whatever is built, and the gate has +to read *0 fail among the seven this milestone can reach*, with the other six +named and attributed. Note what the three `db.aggregate()` cases really need: +implementing `{aggregate: 1}` moves none of them, because each then fails on +`$listLocalSessions` instead. The design review priced that line "orthogonal, +and cheap"; it was cheap and it was worth nothing. + +### And the seven need a decision this review has no authority to take + +`$out` and `$merge` write to a collection the pipeline is not reading. Three +things in the current design stand against that, and none of them is a detail: + +- `aggregate` is declared `.kind = .read`, and the dispatch contract is + explicit that "only `.write` commands may call engine mutation functions". +- dispatch acquires locks from a **static table keyed on the command name**, + before the handler runs, for the one collection named by the command. +- `Collection.lock`'s own comment states the invariant that decides this: + *never more than one collection lock at a time*. Taking the target's + exclusive lock while holding the source's shared one breaks it directly. + +This is the same shape as the objection that kept `$lookup` out of M2.5's first +cut — "a stage that takes a second collection's lock while the pipeline holds +one is a lock-order question that should not ride along" — and the review +should have applied it to the write stages in the same breath. It did not, +which is the review's own error and is recorded here rather than quietly +corrected. + +Two ways out, both real: + +**(a) A pipeline that ends in a write stage is a write command.** Compute the +effective lock shape from the pipeline rather than the name: catalog exclusive, +no collection lock from dispatch, and the handler takes source-shared → +release → target-exclusive in that order. Heavier than mongod, and it +serializes such an aggregate against the whole catalog, but it keeps every +existing invariant intact and is honest about what the command is. + +**(b) The write stages run in the dispatch epilogue,** after every lock is +released, next to the commit and the checkpoint that already live there. The +handler stashes what to write. Cheaper and better-behaved under concurrency, +but it amends the `.read`/`.write` contract, so the contract's comment has to +change with it and say why. + +Either way the durability question is open and belongs with whichever is +chosen: mongod's `$out` replaces the target collection *atomically*, and this +engine has no cross-collection atomicity. A pipeline that fails after writing +half its output must not leave the target half-replaced. + +**Nothing is implemented past Tier 0 until that is decided.** +