[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



Ian Jackson writes ("Re: [OSSTEST PATCH 5/8] Schema: Support database schema 
updates"):
> It does say that updates are applied in order.  I will add `and
> monotonically increasing' after `unique'.  [etc etc]

Here is v3 of this patch.  I'll refrain from reposting the whole
series.

Ian.

From a7fe7187f6237043f8d6aedeb714409b606af8e0 Mon Sep 17 00:00:00 2001
From: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
Date: Thu, 10 Dec 2015 13:26:00 +0000
Subject: [OSSTEST PATCH 5/8] Schema: Support database schema updates

See schema/README.schema, introduced in this patch, for the design.

Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
---
v3: Fix spurious message from ./mg-schema-updates apply.
    Fix grammar error in README.updates.

v2: Slight increase schema update name length format.
    Docs fixes:
    Change erroneous `three' to `four'.
    Change `state' to `status' throghout.
    Explain scope of <status>.
    Sort out (and renumber) `Update order for Populate-then-rely'.
    Sort out "Statuses" explanations.
    Encourage use of DML update, rather than ad-hoc scripts,
     for populating new columns.
---
 Osstest/Executive.pm      |   82 +++++++++++++++
 mg-schema-create          |   20 ++++
 mg-schema-test-database   |    6 +-
 mg-schema-update          |  255 +++++++++++++++++++++++++++++++++++++++++++++
 schema/README.updates     |  179 +++++++++++++++++++++++++++++++
 schema/schema-updates.sql |    6 ++
 6 files changed, 547 insertions(+), 1 deletion(-)
 create mode 100755 mg-schema-update
 create mode 100644 schema/README.updates
 create mode 100644 schema/schema-updates.sql

diff --git a/Osstest/Executive.pm b/Osstest/Executive.pm
index fcef83f..e1fbe3b 100644
--- a/Osstest/Executive.pm
+++ b/Osstest/Executive.pm
@@ -56,6 +56,7 @@ BEGIN {
                       resource_check_allocated resource_shared_mark_ready
                       duration_estimator
                       db_pg_dsn opendb opendb_state
+                      db_schema_updates_applied db_schema_updates_intree
                       );
     %EXPORT_TAGS = ( colours => [qw($green $red $yellow $purple $blue)] );
 
@@ -128,6 +129,87 @@ sub grabrepolock_reexec {
     }
 }
 
+sub db_schema_updates_applied (;$) {
+    my ($cond) = @_;
+    my $r;
+    $cond //= '1=1';
+    eval {
+       local $dbh_tests->{PrintError} = 0;
+       $r = $dbh_tests->selectall_arrayref(<<END);
+           SELECT updatename, applytime
+              FROM schema_updates WHERE $cond
+END
+    };
+    if ($@) {
+       die unless
+           $dbh_tests->err()==7 && # DBD::Pg(3pm)
+           $dbh_tests->state() eq '42P01';
+       # http://www.postgresql.org/docs/current/static/errcodes-appendix.html
+       $r = [ ];
+    }
+    my @r;
+    foreach (@$r) {
+       push @r, { Name => $_->[0], Applied => $_->[1] };
+    }
+    return \@r;
+}
+
+sub db_schema_updates_intree (;$) {
+    my ($incommit) = @_;
+    # ->[]{Name}
+    # ->[]{Seq}
+    # ->[]{State}
+
+    my @results;
+
+    my @files;
+    if (!$incommit) {
+       @files = <schema/*.sql>;
+    } else {
+       local $/ = "\0";
+       open GLF, "-|", qw(git ls-tree -z), $incommit, "schema/" or die $!;
+       while (<GLF>) {
+           chomp;
+           next unless s/^\d+ blob \w+\t//;
+           push @files, $_;
+       }
+       $!=0; $?=0; close GLF or die "$! $? ($incommit)";
+    }
+
+  FILE: foreach my $f (@files) {
+        $f =~ m/\.sql$/ or next;
+       $f =~ m#/([a-z][0-9a-z-]+)\.sql$# or die "badly named .sql file $f\n";
+       my $name = $1;
+       next if $name eq 'initial';
+       if ($incommit) {
+           open SQLF, "-|", qw(git cat-file blob), "$incommit:$f" or die $!;
+       } else {
+           open SQLF, "<", $f or die "$f $!";
+       }
+       while (<SQLF>) {
+           chomp;
+           my $origl = $_;
+           next unless s/^\s*--\s*##OSSTEST##\s+//;
+           m/^0*([1-9]\d*)\s+(Harmless|Preparatory|Unfinished|Ready|Needed)\b/
+               or die "$origl ?";
+           push @results, {
+               Name => $name,
+               Seq => $1+0,
+               State => $2,
+           };
+           next FILE;
+       }
+       $!=0; $?=0; close SQLF; die "$f \`$name' no token ($! $?)";
+    }
+
+    @results = sort {
+       $a->{Seq} <=> $b->{Seq} ||
+           die "$a->{Name} $a->{Seq} == $b->{Name} $b->{Seq}"
+    } @results;
+
+    return \@results;
+}
+
 #---------- database access ----------#
 
 sub opendb_state () {
diff --git a/mg-schema-create b/mg-schema-create
index 54f1c76..1ee007b 100755
--- a/mg-schema-create
+++ b/mg-schema-create
@@ -30,6 +30,9 @@
 # Options:
 #
 #  -q                            don't print progress messages
+#  --no-updates                  apply no schema updates
+#  --stop-before --stop-after    only apply some schema updates -
+#                                 see mg-schema-update
 
 set -e
 set -o posix
@@ -38,6 +41,8 @@ set -o pipefail
 progress () { printf "%s\n" "$*"; }
 progress=progress
 quietopt=''
+do_updates=true
+updates=()
 
 while [ $# != 0 ]; do
     arg=$1; shift
@@ -46,16 +51,31 @@ while [ $# != 0 ]; do
         progress=:
         quietopt=-q
         ;;
+    --stop-before|--stop-after)
+       updates+=("$arg" "$1"); shift
+       ;;
+    --stop-before=*|--stop-after=*)
+       updates+=("$arg"); shift
+       ;;
+    --no-updates)
+       do_updates=false
+       ;;
     *)
         echo >&2 "bad usage ($arg)"; exit 127
         ;;
     esac
 done
 
+export OSSTEST_DB_USEREAL_IGNORETEST='.*'
+
 . ./cri-getconfig
 
 $progress "Populating database..."
 
 $(get_psql_cmd) $quietopt -f schema/initial.sql
 
+if $do_updates; then
+    ./mg-schema-update $quietopt apply-all "${updates[@]}"
+fi
+
 $progress "Database set up."
diff --git a/mg-schema-test-database b/mg-schema-test-database
index 3616c4d..5c6a935 100755
--- a/mg-schema-test-database
+++ b/mg-schema-test-database
@@ -344,6 +344,8 @@ END
        # Keep a copy as it came from dump, for comparison
        cp $t.schema $t.schema.orig
 
+       wantupdates=$(./mg-schema-update list-applied)
+
        # http://www.postgresql.org/message-id/26790.1306355327@xxxxxxxxxxxxx
        perl -i~ -pe '
                s/^/--/ if
@@ -391,7 +393,9 @@ END
        psql_do <<END
                CREATE DATABASE $dbname;
 END
-       withtest ./mg-schema-create -q
+       withtest ./mg-schema-create -q --no-updates
+
+       withtest ./mg-schema-update -q apply $wantupdates
 
        printf ".\n"
 
diff --git a/mg-schema-update b/mg-schema-update
new file mode 100755
index 0000000..f3ec1d6
--- /dev/null
+++ b/mg-schema-update
@@ -0,0 +1,255 @@
+#!/usr/bin/perl -w
+
+# This is part of "osstest", an automated testing framework for Xen.
+# Copyright (C) 2009-2015 Citrix Inc.
+# 
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU Affero General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+# 
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+# 
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+
+# Usages:
+#
+#  ./mg-schema-update [<options>] apply [<updatename>...]
+#  ./mg-schema-update [<options>] show
+#  ./mg-schema-update [<options>] apply-all
+#
+# Usual rune for applying updates:
+#
+#  ./mg-schema-update -o <oldest-running-git-ref-spec> apply-all
+#
+# Options:
+#
+#  -o<git-ref-spec> --oldest=<git-ref-spec>
+#     Specify the oldest version of osstest that is currently running
+#     anywhere against this DB.  Used to determine compatibility of
+#     updates.
+#
+#  -f
+#     Force.  May be repeated for greater effect:
+#
+#     Force installation of updates which break some old code, even
+#     though -o was not specified so it is not known whether such old
+#     code is still running anywhere.
+#
+#  -ff
+#     Force installation of updates which are known to break running
+#     code (including, perhaps, this very code right here).
+#
+#  -fff
+#     Force an attempt to install an already-installed update.  This
+#     is not likely to work.
+
+
+use strict qw(vars);
+use DBI;
+use Osstest;
+use Osstest::Executive;
+
+use Getopt::Long qw(:config bundling gnu_compat require_order no_ignore_case);
+
+csreadconfig();
+
+our (%state, @state);
+our $there;
+our $force=0;
+our $quiet=0;
+
+sub getstate () {
+    my $record = sub {
+       my ($key,$list) = @_;
+       foreach my $entry (@$list) {
+           $state{ $entry->{Name} }{ $key } = $entry;
+       }
+    };
+
+    $record->('Db', db_schema_updates_applied());
+    $record->('Here', db_schema_updates_intree());
+    $record->('There', db_schema_updates_intree($there)) if $there;
+
+    foreach my $name (keys %state) {
+       my $out = $state{$name};
+       $out->{Name} = $name;
+       $out->{Sortkey} =
+           $out->{Here}{Seq} //
+           $out->{There}{Seq} //
+           0;
+    }
+
+    foreach my $out (values %state) {
+       my $st = $out->{Here}{State} // 'missing';
+       # ->{Todo} = 0: no; 1: maybe (?old code); 2: always
+       #             -1: already applied
+       if ($out->{Db}{Applied}) {
+           $out->{Todo} = -1;
+           $out->{Msg} = "already applied";
+       } elsif ($st =~ m/Harmless|Preparatory/) {
+           $out->{Todo} = 2;
+           $out->{Msg} = "right away";
+       } elsif ($st =~ m/Ready|Needed/) {
+           my $tst = $out->{There}{State} // 'missing';
+           if (!$there) {
+               $out->{Todo} = 1;
+               $out->{Msg} = "would break any old code";
+           } elsif ($tst =~ m/Harmless|Preparatory/) {
+               $out->{Todo} = 2;
+               $out->{Msg} = "specified revision can cope";
+           } else {
+               $out->{Todo} = 0;
+               $out->{Msg} = "specified revision would break";
+           }
+       } else {
+           $out->{Todo} = 0;
+           $out->{Msg} = "not ready";
+       }
+       die unless defined $out->{Todo} && defined $out->{Msg};
+
+       $out->{File} = "schema/$out->{Name}.sql";
+    }
+
+    @state = sort { $a->{Sortkey} <=> $b->{Sortkey} } values %state;
+}
+
+sub cmd_list_applied () {
+    die if @ARGV;
+    getstate();
+    foreach my $v (@state) {
+       next unless $v->{Db}{Applied};
+       print $v->{Name}, "\n" or die $!;
+    }
+}
+
+sub cmd_show () {
+    die if @ARGV;
+
+    getstate();
+
+    printf "%-25s", "Name";
+    printf "    %-9s", "Worktree";
+    printf "  %-11.11s", (sprintf "%8.11s", $there) if $there;
+    printf "  DB update\n";
+
+    foreach my $v (@state) {
+       printf(" %-25s %5s %-5.5s",
+              $v->{Name},
+              $v->{Here}{Seq} // '',
+              $v->{Here}{State} // '-');
+       printf("  %5s %-5.5s",
+              $v->{There}{Seq} // '',
+              $v->{There}{State} // '-') if $there;
+       my $app;
+       if ($v->{Db}{Applied}) {
+           $app = show_abs_time($v->{Db}{Applied});
+       } else {
+           $app = (qw(No: Maybe: Ready!))[$v->{Todo}];
+           $app .= " ";
+           $app .= $v->{Msg};
+       }
+       printf("  %s\n", $app);
+    }
+
+    STDOUT->error and die $!;
+}
+
+sub want_apply ($) {
+    my ($v) = @_;
+    $v->{Todo} >= 2-$force;
+}
+
+sub applyone ($) {
+    my ($v) = @_;
+    die "Will not apply $v->{Name}.sql: $v->{Msg}\n"
+       unless want_apply($v);
+
+    my $fn = $v->{File};
+
+    db_retry($dbh_tests, \@all_lock_tables, sub {
+       print "Applying $fn...\n" unless $quiet;
+       open F, "<", $fn or die "$fn: $!";
+       local $/ = undef;
+       my $sql = <F>;
+       F->error and die $!;
+       close F or die $!;
+
+       $dbh_tests->do(<<END) if $quiet;
+            SET client_min_messages = warning;
+END
+
+       $dbh_tests->do($sql);
+
+       $dbh_tests->do(<<END, {}, $v->{Name}, time);
+           INSERT INTO schema_updates
+               (updatename, applytime)
+               VALUES (?, ?)
+END
+    });
+
+    print "Applying $fn done.\n" unless $quiet;
+}
+
+sub cmd_apply () {
+    print "No updates applied by calling apply with no update names.\n"
+       if !@ARGV && !$quiet;
+    foreach my $name (@ARGV) {
+       getstate();
+       my $ent = $state{$name};
+       die "unknown update \`$name'\n" unless $ent;
+       die "update \`$name' not in this tree\n" unless $ent->{Here}{Seq};
+       applyone($ent);
+    }
+}
+
+sub cmd_apply_all () {
+    my (%stopafter, %stopbefore);
+
+    GetOptions('--stop-after=s' => sub { $stopafter{$_[1]} = 1 },
+              '--stop-before=s' => sub { $stopbefore{$_[1]} = 1 });
+    die "further arguments to apply-all prohibited\n" if @ARGV;
+
+    getstate();
+    foreach my $v (@state) {
+       next unless $v->{Here}{Seq};
+       my $stop = sub {
+           my ($map) = @_;
+           return 0 unless
+               $map->{ $v->{Name} } ||
+               $map->{ $v->{Here}{Seq} };
+           print "Stopping at $v->{Name} ($v->{Here}{Seq}).\n"
+               unless $quiet;
+           return 1;
+       };
+
+       last if $stop->(\%stopbefore);
+
+       if (want_apply($v)) {
+           applyone($v);
+       } else {
+           print "Skipping $v->{File}: $v->{Msg}\n" unless $quiet;
+       }
+
+       last if $stop->(\%stopafter);
+    }
+
+    print "Appropriate updates applied.\n" unless $quiet;
+}
+
+GetOptions('f|force+' => \$force,
+          'q+' => \$quiet,
+          'o|oldest=s' => \$there);
+
+die "need operation\n" unless @ARGV;
+
+my $subcmd= shift @ARGV;
+$subcmd =~ s/-/_/g;
+my $subcmdproc = ${*::}{"cmd_$subcmd"};
+die "unknown subcommand" unless $subcmdproc;
+$subcmdproc->();
diff --git a/schema/README.updates b/schema/README.updates
new file mode 100644
index 0000000..25bc11a
--- /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.
+
+<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, <status> does not depend on the state of the
+database.  Applying a schema update to a database does not change its
+`status'.
+
+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
+
+ * 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
+
+ * 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.
+
+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
+     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'.
diff --git a/schema/schema-updates.sql b/schema/schema-updates.sql
new file mode 100644
index 0000000..cd8dc0c
--- /dev/null
+++ b/schema/schema-updates.sql
@@ -0,0 +1,6 @@
+-- ##OSSTEST## 001 Harmless
+
+CREATE TABLE schema_updates (
+    updatename TEXT PRIMARY KEY,
+    applytime integer NOT NULL
+);
-- 
1.7.10.4


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.