Change in osmo-hlr[master]: add database schema versioning to the HLR database

[Neels Hofmeyr]

Neels Hofmeyr has posted comments on this change. ( https://gerrit.osmocom.org/11898 )

Change subject: add database schema versioning to the HLR database
......................................................................


Patch Set 5: Code-Review-1

(3 comments)

the idea is to first add the schema version evaluation in one patch, without changing the schema, and making that user_version = 0. Then add the v1 column in a separate patch, bump the version and add the upgrade_to_v1. So the v1 patch really only is v1 related.

Also open is the argument to add a --db-upgrade option, to avoid upgrading implicitly. Otherwise, only because the new intern tried the new version once the operator cannot operate anymore, and "sorry, there is no downgrade path".

It should also be fairly easy to add a db-upgrade test. Thinking a shell script to pipe some sql to sqlite3, then start osmo-hlr --db-upgrade on it and see that an sqlite3 .dump produces expected results.

The above is what I'd like to see, unless it takes inifinitely too much time.

https://gerrit.osmocom.org/#/c/11898/5/src/db.c
File src/db.c:

https://gerrit.osmocom.org/#/c/11898/5/src/db.c@378
PS5, Line 378: 	rc = sqlite3_exec(dbc->db, "PRAGMA journal_mode=WAL; PRAGMA synchonous = NORMAL;", 0, 0, &err_msg);
(interesting typo in old code: "synchronous"
is that breaking something?)


https://gerrit.osmocom.org/#/c/11898/5/src/db.c@383
PS5, Line 383: 	if (!db_is_bootstrapped(dbc)) {
imagine some future scheme adds a new table. Then db_is_bootstrapped() will return false because one of the tables is missing? And then we try to populate a db assuming it is empty and fail even before we hit the db upgrade path we should be running instead?

Maybe db_is_bootstrapped() should just check for the subscriber table and otherwise rely on the user_version.

Checking whether all tables are really there as expected might be an additional last step after all upgrades are done.


https://gerrit.osmocom.org/#/c/11898/5/src/db.c@403
PS5, Line 403: 		LOGP(DDB, LOGL_ERROR, "Unable to read user version number from database '%s'\n", dbc->fname);
Is the user_version always present in an sqlite db? otherwise, if I read this right, when osmo-hlr now hits a current .db file, the version is -1 because there is none, and then we don't upgrade?



-- 
To view, visit https://gerrit.osmocom.org/11898
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-hlr
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I8aeaa9a404b622657cbc7138106f38aa6ad8d01b
Gerrit-Change-Number: 11898
Gerrit-PatchSet: 5
Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Vadim Yanitskiy <axilirator at gmail.com>
Gerrit-Comment-Date: Tue, 27 Nov 2018 22:21:12 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181127/9a6be8a0/attachment.htm>