storage/gcp: skip schema DDL when the Spanner schema already exists - #1138
Draft
eperrine-ant wants to merge 2 commits into
Draft
storage/gcp: skip schema DDL when the Spanner schema already exists#1138eperrine-ant wants to merge 2 commits into
eperrine-ant wants to merge 2 commits into
Conversation
initDB (used by Appender and MigrationWriter) and antispam.NewAntispam currently apply their CREATE TABLE IF NOT EXISTS / ALTER TABLE ... ADD COLUMN IF NOT EXISTS statements through a DatabaseAdmin client, and then insert the seed rows, every time they are called - not only when a log is first created. Spanner executes DDL as schema-update operations, which take on the order of seconds each and are serialised per database even when IF NOT EXISTS means they end up changing nothing, so simply restarting a log costs several schema-update operations. When many logs share one database via SpannerTablePrefix this adds up quickly, and concurrent restarts queue up behind one another. Make initDB first do a cheap read-only check using the data client which Appender/MigrationWriter have already created: if every table it would create exists, PubCoord already has its size column, every seed row is present, and the stored compatibilityVersion matches the library's, then the DDL and seeding would be a no-op and are skipped. If anything is missing, different, or cannot be read, initDB carries on and creates or migrates the schema exactly as before, so the check can only ever elide work which would have done nothing. The same client is now also used to apply the seed mutations, rather than a temporary one. The antispam storage gets the same treatment for its FollowCoord and IDSeq tables. NewAntispam now resolves its Spanner client (the provided one, or a new one) before initialising the schema, so that one client serves the check, the seeding, and the returned storage. There is no API or schema change, and behaviour against an empty or partially initialised database is unchanged. Tests cover the check against initialised, empty, and several partially initialised or mismatched databases, and show that re-running initDB and NewAntispam against an existing schema succeeds and leaves existing state alone. The spannertest emulator rejects CREATE TABLE for an existing table regardless of IF NOT EXISTS, so those re-runs would fail if any DDL were still being applied.
…ged, shorten comments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
storage/gcp'sinitDBandstorage/gcp/antispam'sNewAntispamnow first check, with a few single-row reads through the existing Spanner data client, whether the schema they are about to create is already fully present, and skip theUpdateDatabaseDdlcall and seed inserts when it is. If anything is missing, different, or unreadable, the existing create/migrate path runs unchanged. No API or schema change.Why
Spanner runs DDL as schema-update operations — on the order of seconds each and serialised per database — even when
IF NOT EXISTSmakes them no-ops, and today everyAppender/MigrationWriter/NewAntispamconstruction issues the full set. Re-opening an existing log therefore costs several schema-update operations before the appender is usable, and withSpannerTablePrefix(#1040) letting many logs share one database, N logs opening together queue N× that work behind the database's single schema-change pipeline. After this change a re-open is a handful of reads.How "already present" is decided
schemaInitialisedmirrorsinitDBitem for item: a read of theid = 0seed row of each seeded table naming exactly the columnsinitDBcreates (so a missing table, column — e.g.PubCoord.sizeon an older schema — or row fails the read), aLimit: 1read of the unseededSeq/IDSeq, andTessera.compatibilityVersion == SchemaCompatibilityVersion. Any error returns false, so it can only skip work that would have been a no-op;checkDataCompatibilitystill runs on every open.NewAntispamnow resolves its Spanner client before the schema step so the same client serves the check.Testing
TestSchemaInitialised(both packages) covers the initialised, empty, and each partially-initialised / mismatched case.TestInitDBExistingSchemaandTestNewAntispamExistingSchemare-open an initialised database and check existing state is untouched; since the spannertest emulator rejectsCREATE TABLEon an existing table regardless ofIF NOT EXISTS, these also show no DDL is issued on re-open (andt.Skipshould that emulator behaviour change).go test ./storage/gcp/...,gofmt,golangci-lintclean.