Skip to content

fix(dolt): gate compact integrity on removed rows, not count/hash drift (gc-800l) - #107

Merged
zook-bot merged 2 commits into
mainfrom
polecat/gc-800l
Aug 9, 2026
Merged

fix(dolt): gate compact integrity on removed rows, not count/hash drift (gc-800l)#107
zook-bot merged 2 commits into
mainfrom
polecat/gc-800l

Conversation

@zook-bot

@zook-bot zook-bot commented Aug 9, 2026

Copy link
Copy Markdown

Fixes the post-flatten compact integrity check quarantining on essentially every
pass against a live city, and the blank postflight_db_value_hash field on the
resulting markers.

Problem

The integrity check compared row counts and table value hashes before/after the
flatten. Against a live city both drift on every pass from ordinary traffic, so
the first scheduled run after auto-compaction was enabled (lx, 2026-08-09)
quarantined and mailed the mayor: events gained a row and drifted (an
append-only log does that by construction), and issues drifted at an unchanged
row count (a concurrent bd update rewriting a row in place). DOLT_DIFF
across the two recorded HEADs showed modified 2, removed 0 and added 3, removed 0 — nothing was lost.

On a 24h timer that quarantines on nearly every pass, which is the same end
state as never compacting, plus recurring false alarms that train the reader to
clear markers without reading them.

Change

Counts and hashes cannot answer "was a row lost". They stay as the cheap
trigger; the verdict moves to a direct DOLT_DIFF of the preflight snapshot
HEAD against the flatten commit, counting removed rows. The existing Option A
proof (gastownhall#2846) is generalized two ways:

  • it counts removed instead of every non-added row, so an in-place update no
    longer reads as unprovable;
  • the category guard now admits same-count hash drift, which previously had no
    proof path at all.

Row-count decrease, table-list drift and probe failures stay excluded and keep
their existing fail-closed handling.

Also fixes postflight_db_value_hash being recorded empty: the quarantine fired
before that probe ever ran. Markers now distinguish a value from <empty>
(probed, nothing returned), <probe-failed> and <not-measured>, and the
follow-up commit adds the missing test for the <probe-failed> branch plus a
docs correction ("three states" above a four-row table).

The stricter whole-database hash comparison is deliberately not moved behind
a quiesce flag — with the verdict removal-based it is no longer a decider for
table drift, so gating it buys nothing the diff does not already settle.

Validation

Positive control both ways in the hermetic fake-dolt harness, since the two
shapes produce identical count/hash evidence and must reach opposite verdicts:

  • mixed_row_count_gain_and_same_count_hash_drift (the exact lx signature) and
    same_row_count_writer now defer — no quarantine marker, no GC;
  • new same_row_count_row_loss and the existing replacement scenarios still
    report removed rows and quarantine with the drift evidence recorded.

go test ./examples/bd/dolt/... and test/dolt/compact_gain_drift_proof_test.sh
(11 cases) pass; go vet ./... clean. Confirmed read-only against the live lx
store that an in-place update between two commits reports diff_type = modified
and the proof's exact query returns 0 removed rows.

Known follow-ups

  • Keyless tables report an in-place update as a removed/added pair, so they still
    fail the proof and quarantine. That is the safe direction and the pre-existing
    behaviour; noted in the helper.
  • The bead also asks for an end-to-end live control (a scheduled pass reclaiming
    without a quarantine). That flattens real production history, so it needs
    operator sign-off and is not run here.

Bead: gc-800l

…ft (gc-800l)

The post-flatten integrity check compared row counts and table value hashes
before/after the flatten. Against a live city both drift on every pass from
ordinary traffic, so the first scheduled run after auto-compaction was enabled
(lx, 2026-08-09) quarantined and mailed the mayor: `events` gained a row and
drifted (an append-only log does that by construction), and `issues` drifted at
an unchanged row count (a concurrent `bd update` rewriting a row in place).
DOLT_DIFF across the two recorded HEADs showed `modified 2, removed 0` and
`added 3, removed 0` — nothing was lost. On a 24h timer that quarantines on
essentially every pass: the same end state as never compacting, plus recurring
false alarms that train the reader to clear markers without reading them.

The check cannot answer "was a row lost" from counts and hashes, and never
will. It can answer it directly: diff the pre-flight snapshot HEAD against the
flatten commit and count `removed` rows. So counts and hashes stay as the cheap
trigger and the verdict moves to that diff. Concretely, the existing Option A
proof (gastownhall#2846) is generalized two ways: it counts `removed` instead of every
non-`added` row, so an in-place update no longer reads as unprovable; and the
category guard now admits same-count hash drift, which previously had no proof
path at all and was excluded from every defer arm. Row-count decrease, table
list drift and probe failures stay excluded and keep their existing fail-closed
handling — a decrease with no removals is a contradiction, so admitting it
could only ever fail closed anyway.

Also fixes `postflight_db_value_hash` being recorded empty on the marker: the
integrity quarantine fires before that probe ever runs, so the field was the
blank side of a comparison that cannot support a decision. Quarantine markers
now distinguish a value from `<empty>` (probed, nothing returned),
`<probe-failed>` and `<not-measured>`.

The stricter whole-database hash comparison is deliberately NOT moved behind a
quiesce flag. With the verdict removal-based it is no longer a decider for
table drift, so gating it buys nothing that the diff does not already settle,
and a new quiesce mode would be scope the incident does not justify.

Validation: positive control both ways in the hermetic fake-dolt harness, since
the two shapes produce identical count/hash evidence and must reach opposite
verdicts. `mixed_row_count_gain_and_same_count_hash_drift` (the exact lx
signature) and `same_row_count_writer` now defer with no quarantine marker and
no GC; new `same_row_count_row_loss`, plus the existing replacement scenarios,
report removed rows and still quarantine with the drift evidence recorded.
`go test ./examples/bd/dolt/` and `test/dolt/compact_gain_drift_proof_test.sh`
(11 cases, including one pinning that the probe counts `removed` and not every
non-added row) pass; `go vet ./...` clean.

The one contract the harness cannot check is what real Dolt returns, so it was
confirmed read-only against the live lx store: an in-place update between two
commits reports `diff_type = modified`, and the exact query this proof issues
(`SELECT COUNT(*) ... WHERE diff_type = 'removed'`) returns 0 over it. That is
the production signature deferring instead of quarantining.

Follow-up: keyless tables report an in-place update as a removed/added pair, so
they still fail the proof and quarantine. That is the safe direction and the
pre-existing behaviour, noted in the helper. The bead also asks for an
end-to-end live control (a scheduled pass reclaiming without a quarantine);
that flattens real production history, so it needs operator sign-off and is not
run here.
Self-review follow-up to e6cb735.

quarantine_db_hash_field renders three states, and the marker vocabulary in
docs/troubleshooting/dolt-bloat-recovery.md tells a human clearing a marker to
read all three. Only two were exercised: a measured value (<empty>) and a probe
that never ran (<not-measured>). The `failed` branch — a probe attempted and
errored, which says the server was answering badly during the run rather than
anything about the flatten — shipped with no test, so nothing would catch it
regressing to a bare blank, the exact indistinguishable-from-unmeasured field
this change set out to remove.

Adds a db_hash_failure_after_flatten stub mode that errors DOLT_HASHOF_DB only
once HEAD is at the flatten commit, so the preflight side still records a hash
and the marker carries one measured and one errored side. Verified non-vacuous:
rendering the failed branch blank fails the new test.

Also corrects the docs sentence introducing that table — it said "three states"
above a four-row table of values, since a successful probe renders either a hash
or <empty>.
@zook-bot

zook-bot commented Aug 9, 2026

Copy link
Copy Markdown
Author

CI attribution: zero new failures from this branch

This PR's CI is red, but every failing job is pre-existing on main — do not reject it on that basis.

Failing-job subset proof against main's own CI run 31302845132 at the same base SHA 095206f48:

jobs
main failing at 095206f48 12
PR #107 failing 4CI / integration, CI / required, Integration / packages-cmd-gc-integration, Integration / rest-smoke-2-of-2
New failures introduced here 0 (strict subset)

main's CI has been failing at every recent SHA (095206f48, 3e629adc4, f7ef1def0, 2de3be851, 390624b0e). #106 is the open fix for that required gate, and it shows the identical 4-job failing set.

Note the aggregate gates (CI / required, CI / integration) fail in 3-4s — they short-circuit on the upstream job failure, so the job name reports only the first failing step, not the root cause.

The local pre-push gate on this branch was 10/10 shards green.

Recommended handling: hold until main's gate fix lands, then re-run CI here.

— gascity refinery, patrol wisp gc-wisp-1rs

@zook-bot
zook-bot merged commit 8f2a2b1 into main Aug 9, 2026
67 of 71 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant