Move InitTables and have it check for table existence before creation #107
Conversation
…e only created if they don't exist
|
I'll have to review this in a couple hours, but you may want to use the optional-create syntax rather than deal with sqlite breakage. You could construct the tables with: |
|
Thanks for making this change! Have been wanting local support for persistent tables. I would also recommend the "CREATE TABLE IF NOT EXISTS" approach. |
|
Another approach: Probably it's not necessary to include routines to initialize the tables in the Go code. Certainly in production we will not be initializing these tables more than once, so it's dev-only code. Why not move all the CREATE TABLEs into a separate init.sql file, and provide first-time-user instructions to run |
|
@jcjones kicks self -- I'm not sure how I forgot about @jsha I agree that this will essentially be one-time/dev-only code, but I feel moving it out to another file may make it easier down the line to forget to propagate changes made to How would you feel about adding a flag to the monolith ( |
|
What about consolidating all the initialization SQL into a single, long backtick-delimited blockquote? The code right now seems excessively verbose, checking for error and rolling back after each individual statement. |
|
After short experimentation |
|
Got it, thanks for checking! Let's go ahead with the --initdb. |
| if err != nil { | ||
| tx.Rollback() | ||
| return | ||
| } | ||
|
|
||
| // Create certificates table | ||
| _, err = tx.Exec("CREATE TABLE certificates (serial string, digest TEXT, value BLOB);") | ||
| _, err = tx.Exec("CREATE TABLE IF NOT EXISTS certificates (serial INTEGER, digest TEXT, value BLOB);") |
There was a problem hiding this comment.
We're storing this as a hex-encoded string for a couple of reasons. Mainly that it's easier to eyeball, since there's some internal structure to the serial: The first 8 bits are an intermediate identifier, the next 56 bits are a sequential counter, and the final 64 bits are random. It's useful to be able to eyeball these in the DB. It's also nice to be able to easily query for serials with a given prefix, in order to enumerate all certs by the counter part. So I'd like this to stay as a string.
There was a problem hiding this comment.
I'm not sure why I changed this... Anyway, it's been reverted to STRING now.
Not sure how that got changed...
Move InitTables and have it check for table existence before creation
This PR moves
sa.SqlStorageAuthority.InitTablesone usage inboulder/cmd/bouldertosa.NewSQLStorageAuthorityso that bothboulder(mono) andboulder-sa(poly) will create the necessary tables if they don't already exist.It also alters
InitTablesto add checks for the existence of the tables before creating them using theinformation_schema.tablestable. Because this relies on existence of theinformation_schemaschema the new checks cannot be used with thesqlite3driver since SQLite doesn't support this, to deal with this I've added a basic check inNewSQLStorageAuthorityand aboolfield toSQLStorageAuthoritythat indicates if the driver supports theinitCheck, if it doesn't it will just revert to the previous behavior of always trying to create the tables. Since at the moment the only other driver being used ismysqlwhich does supportinformation_schemaso the check is simplydrive != "sqlite3", but this should also work with most other drivers/DBs we might want to use/support in the future (Postgres and MariaDB both supportinformation_schema).This consequently also allows restarting the monolithic client with a MySQL DB without having to wipe all the tables in the DB each time.