Repository navigation
FRE-14a — refuse a 0.1.x data directory instead of opening it as an empty store - #95
Merged
Merged
Conversation
A WeftDB 0.1.x data directory keeps one database per aspect: `metadata.db` at the top, then `<subject>/<aspect>/measurements.db` two levels down. That top `metadata.db` is one of `CONTROL_PLANE_FILES`, so `control_plane_present` said yes and the open went ahead: it created the three control-plane databases it was missing, made `segments/`, wrote a `STORE_FORMAT` marker beside the old data, ran the layout-1 migrations, and returned an empty store. Every measurement vanished without an error, and the marker then described a tree that was not there. The probe for the shape runs as step 4a of `open_on`: after the marker gate, before any database is opened, and only when the root has no marker — which is exactly where a 0.1.x root sits, since markers begin at layout 2. It reports the `<subject>/<aspect>` pairs it found in `StoreError::RowsModeDataDirectory`, which X-ERR already maps to 503 `incompatible_store` and CLI exit 65, so no new status code is involved. By then the open has taken `LOCK` and written `LOCK.holder`, and the variant's doc says those two files are all it touches: the test asserts the rest of the directory is unchanged, that no marker is added, and that a second open refuses the same way. A refusal that annotated the directory would be read by the next version as understanding it, which is the lie this closes. The other half of the box is the upgrade test, and it compares against the release rather than against this build: the fixture carries v0.1.0's own `aspect_stats.txt` and `stats.json`, so the migrated store has to count 3/725/14 bytes-of-rows for `price` over `0..4070900000`, 2/72/7 for `temp`, 1/80/0 for `flow`, and 877 rows / 21 nulls / 3220 bytes store-wide. Then the directory is moved with `rename` and reopened: no migration runs again, the same integers come back, and the six frames under `segments/` are still six. `ordinal()` is the whole of the registry's ordering, and its `unwrap_or(0)` is silent: an id that does not start with a number is ordinal 0, so it is neither pending nor after and simply never runs. Nothing pinned the format, so the registry test now requires four digits, an underscore, and a name. The earlier note for this slice said `migrations_lock.rs` holds a `MAX_MIGRATION_ID` that should be tied to the registry; that file and constant do not exist in this tree, and the note is corrected rather than the phantom wired up. Reading the rows themselves is FRE-14b, and it is a slice of its own: 0.1.x wrote eight databases per aspect, and `correlations` and `patterns` have no frame equivalent at all.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Base:
xname/declare-validation— the live tip of the stack this sits on (#92, #93 and #94 allmerged into it; its tip is
e57e87d, and this branch is rebased onto it, so the diff is this commitalone). A PR against
mainwould re-show the eight commits of that stack, which are merged into thestack branch and not yet into
main. FRE-14b — reading the rows of such a tree — is stacked on thisbranch as
fre14/rows-mode-import.What lands
A v0.1.0 data directory is refused on
open, with the series it found named, instead of beingopened as an empty store over the top of it. Second half of
ROADMAP.md:1163's FRE-14: two of itsthree parts are now done, and the box stays open for the third.
The hole this closes
A WeftDB 0.1.x data directory keeps one database per aspect:
metadata.dbat the top, then<subject>/<aspect>/measurements.dbtwo levels down. That topmetadata.dbis one ofCONTROL_PLANE_FILES, socontrol_plane_presentsaid yes and the open went ahead: it created thethree control-plane databases it was missing, made
segments/, wrote aSTORE_FORMATmarker besidethe old data, ran the layout-1 migrations, and returned an empty store. Every measurement vanished
without an error, and the marker then described a tree that was not there. That is what "the input
FRE-14 has to take over" meant in the fixture slice's note.
Where the probe runs, and what a refusal may touch
The probe for the shape runs as step 4a of
open_on: after the marker gate, before any database isopened, and only when the root has no marker — which is exactly where a 0.1.x root sits, since
markers begin at layout 2. It reports the
<subject>/<aspect>pairs it found inStoreError::RowsModeDataDirectory, which X-ERR already maps to 503incompatible_storeand CLIexit 65, so no new status code is involved.
By then the open has taken
LOCKand writtenLOCK.holder, and the variant's doc says those twofiles are all it touches: the test asserts the rest of the directory is unchanged, that no marker is
added, and that a second open refuses the same way. A refusal that annotated the directory would be
read by the next version as understanding it, which is the lie this closes.
The upgrade test compares against the release, not against this build
The other half of the box is the upgrade test, and it compares against what the release itself
recorded rather than against this build: the fixture carries v0.1.0's own
aspect_stats.txtandstats.json, so the migrated store has to count 3/725/14 bytes-of-rows forpriceover0..4070900000, 2/72/7 fortemp, 1/80/0 forflow, and 877 rows / 21 nulls / 3220 bytesstore-wide. Then the directory is moved with
renameand reopened: no migration runs again, the sameintegers come back, and the six frames under
segments/are still six.rows.csv— the grid the fixture's three trees were written from — is the input to the next part,and it stays in the fixture for that reason.
Diff
weftdb/src/types/store_format.rs+104 — the probe for the shape (rows_mode_aspects), and themarker rules it sits beside.
weftdb/src/types/error.rs+15 —StoreError::RowsModeDataDirectory, itsIncompatibleStoremapping, and the Display text.
weftdb/src/types/segment_store.rs+11 — step 4a ofopen_on.weftdb/src/types/migrations.rs+11/-5 — the layout notes that say which roots have no marker.weftdb/tests/compat_v010.rs+149/-5 — the refusal tests, the byte-identity check, and theupgrade test against the release's recorded counts.
CHANGELOG.md+12.Deliberately not in this slice
Reading the rows is a slice of its own: 0.1.x wrote a catalog plus a database per aspect, and the
catalog's
correlationsandpatternshave no frame equivalent at all. That is FRE-14b, stacked onthis branch. The tooling an operator would run to perform a reading is FRE-14c.
Gates
The stacked 14b branch carries the runs, and its tree contains every line of this one (the tree is
byte-for-byte the one the gates ran against, before and after the rebase onto
e57e87d):cargo +stable test -p weftdb --lib, thecompat_v010fixture gate,cargo +nightly fmt --all -- --check,cargo +stable clippy --workspace --all-targets --all-features -- -D clippy::correctness -D clippy::suspicious -D clippy::perf, andRUSTDOCFLAGS="-D warnings" cargo +stable doc --workspace --no-deps— the flags ci.yml passes, andRUSTFLAGS=""in front of each because the toolchain'sdefault
-C linker-features=lld,cmubreaks linking for proc-macro crates here. Note that thismachine's rustup default is a nightly that cannot build the
aws-lc-sysdependency, so every gatehere runs
cargo +stable;cargo +nightlyis used only for rustfmt, which needs nightly's--edition 2024to agree withrustfmt.toml.