Re: [Xen-devel] [OSSTEST PATCH 5/8] Schema: Support database schema updates

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Thu, 2015-12-10 at 17:12 +0000, Ian Jackson wrote:
> diff --git a/schema/README.updates b/schema/README.updates
> new file mode 100644
> index 0000000..622410c
> --- /dev/null
> +++ b/schema/README.updates
> @@ -0,0 +1,179 @@
> +SCHEMA DEFINITION AND SCHEMA UPDATES (PRODUCTION `EXECUTIVE' MODE)
> +==================================================================
> +
> +To generate a new DB, we apply the original schema (in initial.sql)
> +and then apply all the updates, in order.
> +
> +We maintain a table in the DB which records which updates are applied.
> +
> +
> +Schema update snippet format
> +----------------------------
> +
> +Schema update snippets should be called
> +ÂÂÂschema/<updatename>.sql
> +
> +They should contain DDL commands (ALTER TABLE etc.) to make whatever
> +changes are needed.
> +
> +They MUST NOT contain BEGIN or COMMIT.
> +
> +They must contain a special comment near the top:
> +
> +ÂÂ-- ##OSSTEST## <sequence> <status>
> +
> +<updatename> is a string (/^[a-z][0-9a-z-]+$/) which uniquely identifies
> +the update.ÂÂIt must not be changed because existing installations
> +rely on updates having stable names.
> +
> +<sequence> is a positive integer, which should be unique.ÂÂUpdates are
> +applied in order.

Don't these also need to be monotonically increasing over time/commits?

i.e. committing (and applying through all the states) sequence #42 and then
later committing #12 would be at best confusing and at worse perhaps
produce different results when recreating the db (which, I think, would run
#12 first).

So maybe the rule needs to be something about being larger than the largest
currently applied patch?


> +
> +<status> reflects the compatibility of various schema versions.ÂÂIt is
> +a literal string naming one of the statuses shown in `Update orders',
> +below.
> +
> +<status> depends on the nature of the specific database change, and
> +the behaviour and capabilities of the other code in the same revision
> +of osstest.git.ÂÂBut, so <status> does not depend on the state of the
> +database.ÂÂApplying a schema update to a database does not change its
> +`status'.

"But, so ..." ? I think maybe s/But, s/S/ is what you meant? Or maybe there
was a missing thought?

It's not stated outright, but AIUI the <status> of an update changes in a
commit which either adds/edits a schema update, or which adds code which
adds compatibility/requirements for a particular schema update. Is that
right?

> +In principle, each update can separately be in applied or not applied
> +in any one moment in time (in various databases), and simultaneously
> +have different statuses in different relevant versions of osstest.git.
> +So overall the possible states of an update in the whole world are the
> +cross product of (i) status in each relevant osstest revision, and
> +(ii) appliedness (boolean) in each relevant database instance.
> +
> +
> +Update orders
> +-------------
> +
> +There are four reasonable plans for schema changes:
> +
> + * Fully intercompatible: both old code and new code are each
> +ÂÂÂcompatible with both old schema and new schema.ÂÂThe code and
> +ÂÂÂschema updates may be done in any order.
> +
> +ÂÂÂSuch a schema change always has status:
> +ÂÂÂÂÂÂHarmless

What happens if some subsequent change (perhaps a long time later) causes
the code to require the changes made by a "Harmless" schema update?

Would that be a bug in that later code for not coping with the old schema,
or a bug in the commit adding it for not updating the header of the schema
update (to "Needed", presumably) or is it not an issue because there is
some point at which a schema update becomes part of the assumed baseline?

> +
> + * Explicit conditional: first update the code to understand both
> +ÂÂÂversions of the schema; then update the schema; then drop the
> +ÂÂÂcompatibility code.
> +
> +ÂÂÂSuch a schema change always has status:
> +ÂÂÂÂÂÂUnfinished (or absent)ÂÂÂin old code
> +ÂÂÂÂÂÂReadyÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂin intermediate code
> +ÂÂÂÂÂÂNeededÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂin the final code

So, a plausible sequence of commits to osstest.git might be:

1: Add schema/foo.update with initial status "Unfinished".
2a: Add code to partially implement compat with the new schema:
        no status change
[2b,2c...] more compat or unrelated changes
3: Add final code to completely implement compat with the new schema:
        status changed to "Ready" in that same commit
4a,b,c: Maybe other unrelated changes
5: Remove any piece code which provides support for the old schema:
        status changes to "Needed"
6: Eventually remove remove other compat code.

I've broken down the addition/removal of compat code into stages to
illustrate, that code might really all come/go in a single commit.

(I see now that there is a more comprehensive example further down the doc)

> +
> + * Code first: the new code works with either old or new schema,
> +ÂÂÂbut the old code cannot cope with the new schema.
> +
> +ÂÂÂSuch a schema change has status:
> +ÂÂÂÂÂÂUnfinished (or absent)ÂÂÂin old code
> +ÂÂÂÂÂÂReadyÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂin new code
> +
> + * Schema first: the new schema works with any code; but the old
> +ÂÂÂschema does not work with new code.
> +
> +ÂÂÂSuch a schema change has status:
> +ÂÂÂÂÂÂPreparatoryÂÂÂÂÂÂÂÂÂÂÂÂÂÂin old code
> +ÂÂÂÂÂÂNeededÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂÂin the new code
> +
> +
> +Update order for Populate-then-rely
> +-----------------------------------
> +
> +This is for when we want to record new information and then later rely
> +on it.ÂÂThere are typically two schema changes:
> +
> +* To add the column(s).ÂÂI will call this `add'.ÂÂIt is a `Schema
> +ÂÂfirst' change, in the taxonomy above.
> +
> +* To add appropriate constraints, to prevent the new information being
> +ÂÂleft blank.ÂÂI will call this `constraint'.ÂÂThis is a `Code first'
> +ÂÂor `Explicit conditional' change in the taxonomy above.
> +
> +1. Commit: new schema update `add', status Preparatory.
> +
> +2. Commit: new schema update `constraint', status Unfinished.

At this point we want to wait for those commits to pass the push gate,
before we can apply `add', since applying `add' should be done from a
"properly acked version of osstest.git".

This is made pretty clear by the following commentary, for `Apply' but I
wanted to check I'd got the placement of the wait correct.

> +3. Apply: `add'.
> +
> +4. Optionally commit: code to read new column, but which tolerates
> +ÂÂÂboth complete absence of the column, and/or it containing NULL
> +ÂÂÂ(or whatever the DEFAULT value is).
> +
> +5. Commit: code to populate new column; changing `add' to status
> +ÂÂÂNeeded and `constraint' to status Ready.
> +
> +6. Optionally commit: code which read new column, but which tolerates
> +ÂÂÂit containing NULL/DEFAULT.ÂÂ(`add' is already Needed.)
> +
> +7. If necessary commit: idempotent utility script to populate missing
> +ÂÂÂdata.ÂÂ(Alternatively, this can be done with DML statements in the
> +ÂÂÂ`constraint' schema update .sql file.ÂÂThis is better if it is
> +ÂÂÂpossible.)
> +
> +8. Wait for all executions of old code to finish.ÂÂ(This obviously
> +ÂÂÂimplies first getting a push of all the commits mentioned above.)
> +
> +8. If necessary, execute utility script to populate missing data.
> +
> +9. Apply: `constraint'.
> +
> +10. Optionally commit: code which relies on new column, and does not
> +ÂÂÂnecessarily tolerate NULL/DEFAULT; changing `constraint' to Needed.
> +
> +
> +`Commit' means committing somewhere public and probably pushing to
> +osstest.git#pretest, but not necessarily getting a push.ÂÂ(It
> +necessarily precedes any formal testing of the relevant changes on a
> +production instance.)
> +
> +`Apply' (and `execute utility script') should only be done using a
> +properly acked version of osstest.git.ÂÂIf verifying the sanity of the
> +schema change is nontrivial then ad-hoc tests may need to have been
> +run with a testing instance of the database.ÂÂUsing only a pushed
> +production version is a good idea to avoid the possibility that the
> +production database might contain changes which are not evident in
> +published code (or worse, which are different in future versions).
> +
> +Subject to those conditions, `Apply' means an administrator running
> +./mg-schema-update as osstest; if `wait for executions of old code to
> +finish is needed', this will usually involve passing an appropriate
> +`-o' option.
> +
> +
> +Statuses and rules for push and db update
> +-----------------------------------------
> +
> +ÂÂHarmless
> +ÂÂPreparatory
> +ÂÂÂÂÂNo restrictions
> +
> +ÂÂUnfinished
> +ÂÂ(sql fragment entirely missing is equivalent to Unfinished)
> +ÂÂÂÂÂSchema update: prevented

In the case of "entirely missing" "prevented" must really mean "there can't
possibly be anything to do/prevent"?

> +ÂÂÂÂÂCode push: unrestricted
> +
> +ÂÂReady
> +ÂÂÂÂÂSchema update: need all live code to be Preparatory/Ready/Needed
> +ÂÂÂÂÂCode push: unrestricted
> +
> +ÂÂNeeded
> +ÂÂÂÂÂSchema update: need all live code to be Preparatory/Ready/Needed
> +ÂÂÂÂÂCode push: depends on schema update
> +
> +
> +"Code push: depends on schema update" is not currently implemented.
> +However, many (most?) such changes would cause the push gate itself to
> +fail.
> +
> +"Need all live code to be ..." means to look for the status of this
> +schema update in other running versions of osstest.ÂÂAn attempt at
> +this is provided in the form of the `-o' option to mg-schema-update.
> +It is the administrator's responsibility to select an appropriate
> +argument to `-o'.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel