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

[Stefan Sperling]

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

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


Patch Set 7:

(5 comments)

https://gerrit.osmocom.org/#/c/11898/7/sql/hlr_index.sql
File sql/hlr_index.sql:

https://gerrit.osmocom.org/#/c/11898/7/sql/hlr_index.sql@3
PS7, Line 3: CREATE UNIQUE INDEX idx_subscr_imsi ON subscriber (imsi);
> we're now only using the hlr. […]
It is still necessary. Otherwise we get the following error when osmo-hlr is started without an existing database file:

20181129131330319 DLGLOBAL NOTICE Available via telnet 127.0.0.1 4258 (telnet_interface.c:104)
20181129131330323 DMAIN NOTICE hlr starting (hlr.c:639)
20181129131330323 DDB NOTICE using database: hlr.db (db.c:299)
20181129131330327 DDB NOTICE Missing database tables detected; Bootstrapping database 'hlr.db' (db.c:347)
20181129131330331 DDB ERROR (1) index idx_subscr_imsi already exists (db.c:79)
20181129131330331 DDB ERROR Unable to prepare SQL statement '-- These statements are executed after database tables have been bootstrapped.
-- Statements which create indices can only be prepared once corresponding tables have been created.
CREATE UNIQUE INDEX idx_subscr_imsi ON subscriber (imsi)
' (db.c:204)
20181129131330331 DDB ERROR Failed to bootstrap DB: (rc=1) index idx_subscr_imsi already exists (db.c:357)


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

https://gerrit.osmocom.org/#/c/11898/7/src/db.c@243
PS7, Line 243: static bool db_is_bootstrapped_v0(struct db_context *dbc)
> this is still kind of a mix between "is_bootstrapped" and "is_correct". […]
I would have thought that yes, indeed, this function will need to be revisted every time tables are added/removed with new schema versions.

I don't see how this problem is any different for just one "subscriber" table vs. N tables.
But well, since you insist, I'll change it...


https://gerrit.osmocom.org/#/c/11898/7/src/db.c@346
PS7, Line 346: 	if (!db_is_bootstrapped_v0(dbc)) {
> let's break it down: […]
I'll try to apply these suggestions.


https://gerrit.osmocom.org/#/c/11898/7/src/hlr.c
File src/hlr.c:

https://gerrit.osmocom.org/#/c/11898/7/src/hlr.c@651
PS7, Line 651: 	}
> (For a unit test, it would be helpful to exit the program right after a db upgrade, so first up I th […]
Can't the unit test simply be a separate program from osmo-hlr?


https://gerrit.osmocom.org/#/c/11898/7/src/hlr_db_tool.c
File src/hlr_db_tool.c:

https://gerrit.osmocom.org/#/c/11898/7/src/hlr_db_tool.c@64
PS7, Line 64: 	printf("  -U --upgrade-db            Allow HLR database schema upgrades.\n");
> (the variables and other options are all called 'db-foo', this is the only one called 'foo-db')
Thanks for spotting that. I'll fix it.



-- 
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: 7
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: Mon, 03 Dec 2018 14:49:44 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: No
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181203/9158903f/attachment.htm>