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

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Ian Campbell writes ("Re: [OSSTEST PATCH 5/8] Schema: Support database schema 
updates"):
> On Thu, 2015-12-10 at 17:12 +0000, Ian Jackson wrote:
> > +<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?

It does say that updates are applied in order.  I will add `and
monotonically increasing' after `unique'.

> > +<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?

I reordered this, and the `so' is spurious.  It should just say `But,
<status> does not depend...'.

> 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?

Yes.  I think it should be clear from the rest of the discussion and
I'm not sure that adding some more text here would help clarify
things overall.

> > +   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?

If code starts to depend on the schema change, it should be changed to
Needed when the code is changed.  In practice this is not likely to be
a problem because the update will have been applied a long time ago.

> > + * 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.

Yes.

> > +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.

Yes.

> > +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"?

Well, if you say
   ./mg-schema-update apply this-update-does-not-exist
it will bomb out.

And
   ./mg-schema-update apply-all
will not invent imaginary schema changes out of thin air so that
it can apply them.

So, application of nonexistent schema changes is indeed prevented by
their nonexistence.


Ian.

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