Skip to content

Store the schema version in the database.#1956

Closed
shlevy wants to merge 1 commit intoNixOS:masterfrom
shlevy:safe-schema-upgrade
Closed

Store the schema version in the database.#1956
shlevy wants to merge 1 commit intoNixOS:masterfrom
shlevy:safe-schema-upgrade

Conversation

@shlevy
Copy link
Member

@shlevy shlevy commented Mar 7, 2018

Before this, there was a gap between the database update and changes
to the schema version file, leading to #1954. Now database changes and
schema bumps happen in a single transaction.

To avoid gratuitous backwards incompatibility, we still write 10 to
the old version file, and will write 11 on the next schema bump. We
won't write 12 to it unless we're moving away from storing the schema
in the database.

Fixes #1954.

@shlevy shlevy requested a review from edolstra March 7, 2018 00:36
@shlevy
Copy link
Member Author

shlevy commented Mar 7, 2018

@edolstra If this approach looks good to you, I'll add tests, so far I've only compile-tested.

@edolstra
Copy link
Member

edolstra commented Mar 7, 2018

I don't really see the need for this. Removing the "legacy" schema would make it impossible to change to a different storage (e.g. replace SQLite with something else).

@shlevy
Copy link
Member Author

shlevy commented Mar 7, 2018

@edolstra The need for this is evidenced by #1954.

This PR handles the case where we want to move to a different storage: We keep the legacy schema around, bump it to 11 at the next schema change so 2.0 and earlier know the schema is too new, then leave it at 11 until we decide to move away from sqlite at which point we bump it to 12 or higher.

@shlevy
Copy link
Member Author

shlevy commented Mar 17, 2018

@edolstra Do you have a suggestion for how to fix issues like #1954?

@dezgeg
Copy link
Contributor

dezgeg commented Mar 19, 2018

Sounds a good idea to me, but I wonder if the pragma user_version is actually transactionally updated in SQLite?

@shlevy
Copy link
Member Author

shlevy commented Mar 19, 2018

@dezgeg
Copy link
Contributor

dezgeg commented Mar 19, 2018

I wonder if this could be done simpler though: just read the schema from both places, but if the one stored in SQLite is non-zero, consider it as the authoritative one, and upgrade the flat file to match.

@shlevy
Copy link
Member Author

shlevy commented Mar 20, 2018

Waiting on @edolstra to decide what we want to do about this, but that seems reasonable.

@edolstra
Copy link
Member

Yeah that sounds good to me.

Before this, there was a gap between the database update and changes
to the schema version file, leading to NixOS#1954. Now database changes and
schema bumps happen in a single transaction.

Fixes NixOS#1954.
@shlevy shlevy force-pushed the safe-schema-upgrade branch from 64e7d8a to 60af4ee Compare March 20, 2018 20:43
@shlevy
Copy link
Member Author

shlevy commented Mar 20, 2018

@edolstra updated.

@shlevy
Copy link
Member Author

shlevy commented Apr 1, 2018

@edolstra ping

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants