Page MenuHomePhabricator

[Chronik] Add metadata and schema version to DB
ClosedPublic

Authored by tobias_ruck on Mar 14 2023, 21:56.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABC79fda1a0a190: [Chronik] Add metadata and schema version to DB
Summary
  1. Add a column family meta to store metadata about the DB
  2. Add a field SCHEMA_VERSION in that CF, which tells us which version the database is
  3. If the compiled version doesn't match the version in the db, return with an error

Generally, it seems to be a bad idea to allow running an old Chronik version on a new database, unless we define some clear stability guarantees.

The current behavior is very strict, where the version has to match exactly, to prevent invalid states where e.g. the txs in the database are at a different height than the blocks.

Newer Chronik versions could always be more relaxed, and/or allow upgrading the existing database without reindexing.

Test Plan

ninja check-crates, also all the chronik_* functional tests

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tobias_ruck updated this revision to Diff 38581.

what year is this

Fabien requested changes to this revision.Mar 15 2023, 09:27
Fabien added inline comments.
chronik/chronik-indexer/src/indexer.rs
277 ↗(On Diff #38581)

This is assuming that no version means no existing db. Can we check this is the case ? If I ran chronik before this patch for example it should tell me to reindex. More important is the case of db metadata corruption, you don't want to blindly reuse the other data without reindexing.

280 ↗(On Diff #38581)
This revision now requires changes to proceed.Mar 15 2023, 09:27
chronik/chronik-indexer/src/indexer.rs
277 ↗(On Diff #38581)

Yes I'll thread a bool in here indicating whether the DB exists. Indeed someone could corrupt their DB and the indexer would go haywire

280 ↗(On Diff #38581)

At this point, both the indexer and the database have version N, so I just use "Chronik" here to kinda refer to both

Add a check that the database must be empty before we set the schema version

Fabien requested changes to this revision.Mar 15 2023, 13:48
Fabien added inline comments.
chronik/chronik-db/src/db.rs
122 ↗(On Diff #38596)

Please remove the trailing whitespaces

130 ↗(On Diff #38596)

remove

167 ↗(On Diff #38596)

I don't see the point in testing rocksdb, that's not our code

chronik/chronik-indexer/src/indexer.rs
279 ↗(On Diff #38596)

Remove

This revision now requires changes to proceed.Mar 15 2023, 13:48
This revision is now accepted and ready to land.Mar 16 2023, 13:40