Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store the schema version in the database. #1956

Closed
wants to merge 1 commit into from

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
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.

None yet

3 participants