diff --git a/PLAN.md b/PLAN.md index f0bcc77..45ae8ce 100644 --- a/PLAN.md +++ b/PLAN.md @@ -394,6 +394,55 @@ per-collection gate judges what reclamation left. Same six rounds, 256 MB. Numbers and reproduction in `tests/e2e/results/m0-gates.txt` under `[M1.1]` and `[M1.2]`. +### Amendment A6 — M2 splits in two, and its gate did not exist (amends the M2 row) + +The M2 design review (`docs/M2_DESIGN_REVIEW.md`) found that the milestone's +gate names a corpus that is not there. `mongodb/specifications` has no +aggregation suite: the thirteen `aggregate-*.json` files this project runs live +inside `crud` and test the aggregate *command* — cursor and `batchSize`, +`readConcern` routing, the write semantics of `$out`/`$merge`, `collation`, +`let`. They touch stages barely and expressions not at all; `$lookup`, +`$unwind`, `$facet`, `$addFields` and `$replaceRoot` appear nowhere in the +pinned corpus. A literal "green" is unreachable besides, since 10 of the skips +are version- or topology-gated and cannot pass on a standalone. + +So one milestone name was covering two milestones, and they are split. + +**M2 is the command surface**, gated on the corpus that already exists: `$out`, +`$merge`, `db.aggregate()`, collation, `let`. That is 8 of the 13 current +failures in the write stages alone. It is small and fully measurable today. + +**M2.5 is the engine**, gated on a corpus this project has to write, in the +unified format the runner already reads, with every expectation measured +against mongod 8.3.7 rather than recalled — the discipline that corrected three +assumptions in M1's session work. The tiers, in dependency order: the +expression evaluator, then a per-stage document iterator (the stream is a +materialized window today, which cannot express 1→N), then the accumulators. +`$unwind` is in the first cut because it falls out of the iterator for free. +`$lookup` is out: a stage that takes a second collection's lock while the +pipeline holds one is a lock-order question that should not ride along. +`$facet` is out: sub-pipeline execution should be allowed to settle after the +iterator exists. + +**Sequencing: command surface first, engine second** — chosen deliberately, and +with the cost stated rather than glossed. The engine is what gives wrong +answers *now*: measured against a live server, `$avg`, `$max` and `$push` all +return `0`, a compound `_id` collapses every document into one group keyed by +the unevaluated expression, `$literal` is echoed back, and a `$match` after a +`$project` still sees the projected-away field. Six of eight probed pipelines +answer `ok: 1` with a wrong result; only `$addFields` fails honestly. Doing the +command surface first leaves those alive for a further milestone. + +**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 +path is expected, `$project` anywhere but last. This is the same move +`expectEvents` made before the free list in M1 — it will push the scorecard +*down*, and that is the point. An unrecognised stage is a bug report; an `$avg` +that returns `0` is a corrupted report nobody files. Without this the sequencing +above would be trading a measurable milestone for a year of silent wrong +answers, and the trade is only acceptable because the lie is removed first. + --- ## 3. Milestones and gates @@ -402,7 +451,8 @@ Numbers and reproduction in `tests/e2e/results/m0-gates.txt` under `[M1.1]` and |---|---|---|---| | 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 | **Aggregation expansion** | pipeline stages and expression engine (tiered scope defined at M2 design review) | aggregate spec suite 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.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 | | M5 | **Change streams** | change feed + resume tokens (likely log-seq based), getMore integration | change-streams spec suite green | @@ -969,8 +1019,13 @@ has to be its own commit with its own re-recorded scorecard. taking a `session` argument, and no `lsid` assertion. The value here is protocol hygiene, not a number — and after Stage 1 a wrongly-refused command would have shown up as a changed event stream rather than silently. -- **M2 aggregation**: stage/expression tiers, which spec-test files are - the gate, whether $lookup/$unwind/facet make the first cut. +- **M2 aggregation** — *settled, see amendment A6 and `docs/M2_DESIGN_REVIEW.md`.* + The three questions this entry held are answered: the tiers, the gate (the + named one does not exist), and the first cut ($unwind in, $lookup and $facet + out). What the review did *not* settle and the engine milestone must: + `$out`/`$merge` durability semantics, whether the expression evaluator is + shared with M3's pipeline updates, and whether `allowDiskUse` has to stop + being a lie. - **M4 transactions**: snapshot isolation over mmap (COW vs undo), read concern snapshot, conflict → TransientTransactionError semantics, retryable-writes interplay. diff --git a/docs/M2_DESIGN_REVIEW.md b/docs/M2_DESIGN_REVIEW.md index 0b8acc4..edfe378 100644 --- a/docs/M2_DESIGN_REVIEW.md +++ b/docs/M2_DESIGN_REVIEW.md @@ -219,7 +219,23 @@ out of scope. --- -## 5. Recommendation +## 5. Decision + +**Taken: the command surface first (Option A as M2), the engine second (M2.5), +with Tier 0 folded into M2.** Recorded as PLAN amendment A6; the §3 rows are +rewritten and §6's deferred entry now points here. + +The cost of that order is on the record: the engine is what gives wrong answers +today, and this leaves them alive for a further milestone. Tier 0 is what makes +the trade acceptable rather than merely convenient — every unimplemented +construct stops answering `0` and starts answering an error, so the wrong +answers become visible bug reports instead of quiet ones while the command +surface is built. It is not optional under this sequencing. + +The recommendation the review arrived at independently is kept below, since a +decision is easier to revisit when the argument it overrode is still legible. + +## 5.1 Recommendation as written before the decision **Tier 0 first, alone, before any scoping decision is final** — the same argument that put `expectEvents` before the free list in M1. Six silent wrong