Skip to content

Make storage version upgrades a ceremony (warn, opt-out flag, strict version mapping) - #788

Closed
jonasvanderhaegen wants to merge 1 commit into
LadybugDB:mainfrom
skylence-be:storage/upgrade-ceremony-v1
Closed

Make storage version upgrades a ceremony (warn, opt-out flag, strict version mapping)#788
jonasvanderhaegen wants to merge 1 commit into
LadybugDB:mainfrom
skylence-be:storage/upgrade-ceremony-v1

Conversation

@jonasvanderhaegen

Copy link
Copy Markdown
Contributor

What

Makes storage-format upgrades a visible ceremony instead of a silent side effect:

  1. Warning on auto-upgrade. When a checkpoint stamps a database file written by an older release with the current storage version (after which older binaries refuse the file), a warning is printed to stderr naming both versions.
  2. Opt-out flag. New SystemConfig field / CALL option allow_storage_version_upgrade (default true, so behavior is unchanged unless you set it). When false, such a checkpoint throws a RuntimeException naming both versions instead of upgrading; the file stays readable by the release that wrote it. The check runs before checkpointStorage(), so on refusal the data file is untouched. Database::~Database already catches close-time checkpoint exceptions, so a refusal on close simply skips the upgrade.
  3. Throw on unmapped build version. StorageVersionInfo::getStorageVersion() previously fell back to the max storage version in the map when LBUG_CMAKE_VERSION had no entry. That fallback only fires when a release forgot to update the map — and silently guessing the newest format in that state converts a release-process mistake into invisible format skew. It now throws with an actionable message. The current version string (0.20.0) is mapped, and 4-component nightly strings are truncated to 3 components before lookup, so no supported build path hits the throw.
  4. Header read no longer swallows version mismatches. DatabaseHeader::readDatabaseHeader caught every RuntimeException and returned "no header". A version-mismatched header now propagates; only a magic-bytes mismatch (the optimistic pre-checkpoint write case) still reads as "no header". Visible effect: shadow-file recovery under a version-mismatched header now reports the real version error instead of "The database is corrupted, please recreate it."
  5. Tests for the old-binary refusal path, resolving the TODO(Guodong) in database_header.cpp: unit tests for the version mapping (including the throw), api tests that rewrite the header's storage version and assert the open-time refusal message, the readDatabaseHeader propagation/no-magic behaviors, and the checkpoint opt-out round-trip; plus an e2e case exercising allow_storage_version_upgrade end to end.

Why

A storage-version upgrade is a one-way door: after it, every older binary refuses the file. Today the door closes silently on the first checkpoint. This PR keeps the default behavior but makes the door visible (warning), lockable (flag), and honest about failure modes (strict version mapping, propagated version mismatch).

Tests

  • api_test --gtest_filter='StorageVersionMappingTest.*:StorageVersionCeremonyTest.*' (7 new tests)
  • e2e storage_version.test: new StorageVersionUpgradeOptOut case
  • existing storage_version.StorageVersionFunctionAndUpgrade unchanged and passing

🤖 Generated with Claude Code

- warn on stderr when a checkpoint upgrades the on-disk storage version
- new SystemConfig/CALL option allow_storage_version_upgrade (default
  true): when false, such a checkpoint throws instead of upgrading; the
  check runs before WAL rotation so a refusal mutates nothing
- getStorageVersion() now throws on a version string with no mapping
  instead of silently using the max known storage version
- readDatabaseHeader only treats a magic-bytes mismatch as 'no header';
  a version mismatch now propagates (shadow-file recovery previously
  reported it as corruption)
- tests for the old-binary refusal path (resolves the TODO in
  database_header.cpp)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jonasvanderhaegen

Copy link
Copy Markdown
Contributor Author

Closing: this work now lives in our fork (skylence-be/ladybug) rather than as an upstream contribution. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant