Merge pull request #917 from citusdata/isolationtester

Basic Isolationtester infrastructure including some basic tests.

Makefile.global.in

@@ -11,6 +11,9 @@
 
 citus_abs_srcdir:=@abs_top_srcdir@/${citus_subdir}
 citus_abs_top_srcdir:=@abs_top_srcdir@
+postgres_abs_srcdir:=@POSTGRES_SRCDIR@
+postgres_abs_builddir:=@POSTGRES_BUILDDIR@
+
 PG_CONFIG:=@PG_CONFIG@
 PGXS:=$(shell $(PG_CONFIG) --pgxs)
 

configure

@@ -586,6 +586,8 @@ PACKAGE_URL=''
 
 ac_subst_vars='LTLIBOBJS
 LIBOBJS
+POSTGRES_BUILDDIR
+POSTGRES_SRCDIR
 CITUS_CFLAGS
 OBJEXT
 EXEEXT
@@ -2540,6 +2542,14 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
 
 CFLAGS="$SAVE_CFLAGS"
 
+
+# Locate source and build directory of the postgres we're building
+# against. Can't rely on either still being present, but e.g. optional
+# test infrastructure can rely on it.
+POSTGRES_SRCDIR=$(grep ^abs_top_srcdir $(dirname $($PG_CONFIG --pgxs))/../Makefile.global|cut -d ' ' -f3-)
+POSTGRES_BUILDDIR=$(grep ^abs_top_builddir $(dirname $($PG_CONFIG --pgxs))/../Makefile.global|cut -d ' ' -f3-)
+
+
 # check for a number of CFLAGS that make development easier
 
 # CITUSAC_PROG_CC_CFLAGS_OPT
@@ -2938,6 +2948,10 @@ fi
 
 CITUS_CFLAGS="$CITUS_CFLAGS"
 
+POSTGRES_SRCDIR="$POSTGRES_SRCDIR"
+
+POSTGRES_BUILDDIR="$POSTGRES_BUILDDIR"
+
 
 ac_config_files="$ac_config_files Makefile.global"
 

configure.in

@@ -63,6 +63,14 @@ SAVE_CFLAGS="$CFLAGS"
 AC_PROG_CC([$($PG_CONFIG --cc)])
 CFLAGS="$SAVE_CFLAGS"
 
+
+# Locate source and build directory of the postgres we're building
+# against. Can't rely on either still being present, but e.g. optional
+# test infrastructure can rely on it.
+POSTGRES_SRCDIR=$(grep ^abs_top_srcdir $(dirname $($PG_CONFIG --pgxs))/../Makefile.global|cut -d ' ' -f3-)
+POSTGRES_BUILDDIR=$(grep ^abs_top_builddir $(dirname $($PG_CONFIG --pgxs))/../Makefile.global|cut -d ' ' -f3-)
+
+
 # check for a number of CFLAGS that make development easier
 
 # CITUSAC_PROG_CC_CFLAGS_OPT
@@ -102,6 +110,8 @@ CITUSAC_PROG_CC_CFLAGS_OPT([-Wmissing-declarations])
 CITUSAC_PROG_CC_CFLAGS_OPT([-Wmissing-prototypes])
 
 AC_SUBST(CITUS_CFLAGS, "$CITUS_CFLAGS")
+AC_SUBST(POSTGRES_SRCDIR, "$POSTGRES_SRCDIR")
+AC_SUBST(POSTGRES_BUILDDIR, "$POSTGRES_BUILDDIR")
 
 AC_CONFIG_FILES([Makefile.global])
 AC_CONFIG_HEADERS([src/include/citus_config.h])

src/test/regress/Makefile

@@ -14,7 +14,7 @@ endif
 ## Citus regression support
 ##
 MULTI_INSTALLDIR=$(CURDIR)/tmp_check/install
-pg_regress_multi_check = $(PERL) $(citus_abs_srcdir)/pg_regress_multi.pl --pgxsdir="$(pgxsdir)" --bindir="$(bindir)" --libdir="$(libdir)" --majorversion="$(MAJORVERSION)"
+pg_regress_multi_check = $(PERL) $(citus_abs_srcdir)/pg_regress_multi.pl --pgxsdir="$(pgxsdir)" --bindir="$(bindir)" --libdir="$(libdir)" --majorversion="$(MAJORVERSION)" --postgres-builddir="$(postgres_abs_builddir)"  --postgres-srcdir="$(postgres_abs_srcdir)"
 MULTI_REGRESS_OPTS = --inputdir=$(citus_abs_srcdir) $(pg_regress_locale_flags)
 
 # XXX: Can't actually do useful testruns against install - $libdir
@@ -49,6 +49,10 @@ check-multi: all tempinstall-main
 	$(pg_regress_multi_check) --load-extension=citus \
 	-- $(MULTI_REGRESS_OPTS) --schedule=$(citus_abs_srcdir)/multi_schedule $(EXTRA_TESTS)
 
+check-isolation: all tempinstall-main
+	$(pg_regress_multi_check) --load-extension=citus --isolationtester \
+	-- $(MULTI_REGRESS_OPTS) --schedule=$(citus_abs_srcdir)/isolation_schedule $(EXTRA_TESTS)
+
 check-multi-task-tracker-extra: all tempinstall-main
 	$(pg_regress_multi_check) --load-extension=citus \
 	--server-option=citus.task_executor_type=task-tracker \

src/test/regress/expected/isolation_cluster_management.out

@@ -0,0 +1,13 @@
+Parsed test spec with 1 sessions
+
+starting permutation: s1a
+step s1a: 
+    SELECT master_add_node('localhost', 57637);
+    SELECT master_add_node('localhost', 57638);
+
+master_add_node
+
+(1,1,localhost,57637,default,f)
+master_add_node
+
+(2,2,localhost,57638,default,f)

src/test/regress/expected/isolation_concurrent_dml.out

@@ -0,0 +1,30 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s1-begin s1-insert s2-update s1-commit
+master_create_worker_shards
+
+               
+step s1-begin: 
+    BEGIN;
+
+step s1-insert: 
+    INSERT INTO test_table VALUES(1);
+
+step s2-update: 
+    UPDATE test_table SET data = 'blarg' WHERE test_id = 1;
+ <waiting ...>
+step s1-commit: 
+    COMMIT;
+
+step s2-update: <... completed>
+
+starting permutation: s1-insert s2-update
+master_create_worker_shards
+
+               
+step s1-insert: 
+    INSERT INTO test_table VALUES(1);
+
+step s2-update: 
+    UPDATE test_table SET data = 'blarg' WHERE test_id = 1;
+

src/test/regress/expected/isolation_dml_vs_repair.out

@@ -0,0 +1,200 @@
+Parsed test spec with 2 sessions
+
+starting permutation: s2-invalidate-57637 s1-begin s1-insertone s2-repair s1-commit
+master_create_worker_shards
+
+               
+step s2-invalidate-57637: 
+    UPDATE pg_dist_shard_placement SET shardstate = '3' WHERE shardid = 102008 AND nodeport = 57637;
+
+step s1-begin: 
+    BEGIN;
+
+step s1-insertone: 
+    INSERT INTO test_table VALUES(1, 1);
+
+step s2-repair: 
+    SELECT master_copy_shard_placement(102008, 'localhost', 57638, 'localhost', 57637);
+ <waiting ...>
+step s1-commit: 
+    COMMIT;
+
+step s2-repair: <... completed>
+master_copy_shard_placement
+
+               
+
+starting permutation: s1-insertone s2-invalidate-57637 s1-begin s1-insertall s2-repair s1-commit
+master_create_worker_shards
+
+               
+step s1-insertone: 
+    INSERT INTO test_table VALUES(1, 1);
+
+step s2-invalidate-57637: 
+    UPDATE pg_dist_shard_placement SET shardstate = '3' WHERE shardid = 102008 AND nodeport = 57637;
+
+step s1-begin: 
+    BEGIN;
+
+step s1-insertall: 
+    INSERT INTO test_table SELECT test_id, data+1 FROM test_table;
+
+step s2-repair: 
+    SELECT master_copy_shard_placement(102008, 'localhost', 57638, 'localhost', 57637);
+ <waiting ...>
+step s1-commit: 
+    COMMIT;
+
+step s2-repair: <... completed>
+master_copy_shard_placement
+
+               
+
+starting permutation: s2-invalidate-57637 s2-begin s2-repair s1-insertone s2-commit s2-invalidate-57638 s1-display s2-invalidate-57637 s2-revalidate-57638 s1-display
+master_create_worker_shards
+
+               
+step s2-invalidate-57637: 
+    UPDATE pg_dist_shard_placement SET shardstate = '3' WHERE shardid = 102008 AND nodeport = 57637;
+
+step s2-begin: 
+    BEGIN;
+
+step s2-repair: 
+    SELECT master_copy_shard_placement(102008, 'localhost', 57638, 'localhost', 57637);
+
+master_copy_shard_placement
+
+               
+step s1-insertone: 
+    INSERT INTO test_table VALUES(1, 1);
+ <waiting ...>
+step s2-commit: 
+    COMMIT;
+
+step s1-insertone: <... completed>
+step s2-invalidate-57638: 
+    UPDATE pg_dist_shard_placement SET shardstate = '3' WHERE shardid = 102008 AND nodeport = 57638;
+
+step s1-display: 
+    SELECT * FROM test_table WHERE test_id = 1;
+
+test_id        data           
+
+1              1              
+step s2-invalidate-57637: 
+    UPDATE pg_dist_shard_placement SET shardstate = '3' WHERE shardid = 102008 AND nodeport = 57637;
+
+step s2-revalidate-57638: 
+    UPDATE pg_dist_shard_placement SET shardstate = '1' WHERE shardid = 102008 AND nodeport = 57638;
+
+step s1-display: 
+    SELECT * FROM test_table WHERE test_id = 1;
+
+test_id        data           
+
+1              1              
+
+starting permutation: s2-invalidate-57637 s1-prepared-insertone s2-begin s2-repair s1-prepared-insertone s2-commit s2-invalidate-57638 s1-display s2-invalidate-57637 s2-revalidate-57638 s1-display
+master_create_worker_shards
+
+               
+step s2-invalidate-57637: 
+    UPDATE pg_dist_shard_placement SET shardstate = '3' WHERE shardid = 102008 AND nodeport = 57637;
+
+step s1-prepared-insertone: 
+    EXECUTE insertone;
+
+step s2-begin: 
+    BEGIN;
+
+step s2-repair: 
+    SELECT master_copy_shard_placement(102008, 'localhost', 57638, 'localhost', 57637);
+
+master_copy_shard_placement
+
+               
+step s1-prepared-insertone: 
+    EXECUTE insertone;
+ <waiting ...>
+step s2-commit: 
+    COMMIT;
+
+step s1-prepared-insertone: <... completed>
+error in steps s2-commit s1-prepared-insertone: ERROR:  prepared modifications cannot be executed on a shard while it is being copied
+step s2-invalidate-57638: 
+    UPDATE pg_dist_shard_placement SET shardstate = '3' WHERE shardid = 102008 AND nodeport = 57638;
+
+step s1-display: 
+    SELECT * FROM test_table WHERE test_id = 1;
+
+test_id        data           
+
+1              1              
+step s2-invalidate-57637: 
+    UPDATE pg_dist_shard_placement SET shardstate = '3' WHERE shardid = 102008 AND nodeport = 57637;
+
+step s2-revalidate-57638: 
+    UPDATE pg_dist_shard_placement SET shardstate = '1' WHERE shardid = 102008 AND nodeport = 57638;
+
+step s1-display: 
+    SELECT * FROM test_table WHERE test_id = 1;
+
+test_id        data           
+
+1              1              
+
+starting permutation: s2-invalidate-57637 s1-insertone s1-prepared-insertall s2-begin s2-repair s1-prepared-insertall s2-commit s2-invalidate-57638 s1-display s2-invalidate-57637 s2-revalidate-57638 s1-display
+master_create_worker_shards
+
+               
+step s2-invalidate-57637: 
+    UPDATE pg_dist_shard_placement SET shardstate = '3' WHERE shardid = 102008 AND nodeport = 57637;
+
+step s1-insertone: 
+    INSERT INTO test_table VALUES(1, 1);
+
+step s1-prepared-insertall: 
+    EXECUTE insertall;
+
+step s2-begin: 
+    BEGIN;
+
+step s2-repair: 
+    SELECT master_copy_shard_placement(102008, 'localhost', 57638, 'localhost', 57637);
+
+master_copy_shard_placement
+
+               
+step s1-prepared-insertall: 
+    EXECUTE insertall;
+ <waiting ...>
+step s2-commit: 
+    COMMIT;
+
+step s1-prepared-insertall: <... completed>
+error in steps s2-commit s1-prepared-insertall: ERROR:  prepared modifications cannot be executed on a shard while it is being copied
+step s2-invalidate-57638: 
+    UPDATE pg_dist_shard_placement SET shardstate = '3' WHERE shardid = 102008 AND nodeport = 57638;
+
+step s1-display: 
+    SELECT * FROM test_table WHERE test_id = 1;
+
+test_id        data           
+
+1              1              
+1              2              
+step s2-invalidate-57637: 
+    UPDATE pg_dist_shard_placement SET shardstate = '3' WHERE shardid = 102008 AND nodeport = 57637;
+
+step s2-revalidate-57638: 
+    UPDATE pg_dist_shard_placement SET shardstate = '1' WHERE shardid = 102008 AND nodeport = 57638;
+
+step s1-display: 
+    SELECT * FROM test_table WHERE test_id = 1;
+
+test_id        data           
+
+1              1              
+1              2              

src/test/regress/isolation_schedule

@@ -0,0 +1,3 @@
+test: isolation_cluster_management
+test: isolation_concurrent_dml
+test: isolation_dml_vs_repair

src/test/regress/pg_regress_multi.pl

@@ -26,8 +26,11 @@ sub Usage()
     print "  pg_regress_multi [MULTI OPTIONS] -- [PG REGRESS OPTS]\n";
     print "\n";
     print "Multi Options:\n";
+    print "  --isolationtester   Run isolationtester tests instead of plain tests\n";
     print "  --bindir            Path to postgres binary directory\n";
     print "  --libdir            Path to postgres library directory\n";
+    print "  --postgres-builddir Path to postgres build directory\n";
+    print "  --postgres-srcdir Path to postgres build directory\n";
     print "  --pgxsdir           Path to the PGXS directory\n";
     print "  --load-extension    Extensions to install in all nodes\n";
     print "  --server-option     Config option to pass to the server\n";
@@ -35,9 +38,12 @@ sub Usage()
 }
 
 # Option parsing
+my $isolationtester = 0;
 my $bindir = "";
 my $libdir = undef;
 my $pgxsdir = "";
+my $postgresBuilddir = "";
+my $postgresSrcdir = "";
 my $majorversion = "";
 my @extensions = ();
 my @userPgOptions = ();
@@ -47,10 +53,15 @@ my %fdwServers = ();
 my %functions = ();
 my %operators = ();
 
+my $serversAreShutdown = "TRUE";
+
 GetOptions(
+    'isolationtester' => \$isolationtester,
     'bindir=s' => \$bindir,
     'libdir=s' => \$libdir,
     'pgxsdir=s' => \$pgxsdir,
+    'postgres-builddir=s' => \$postgresBuilddir,
+    'postgres-srcdir=s' => \$postgresSrcdir,
     'majorversion=s' => \$majorversion,
     'load-extension=s' => \@extensions,
     'server-option=s' => \@userPgOptions,
@@ -71,6 +82,25 @@ if (defined $libdir)
     $ENV{PATH} = "$libdir:".($ENV{PATH} || '');
 }
 
+my $plainRegress = "$pgxsdir/src/test/regress/pg_regress";
+my $isolationRegress = "${postgresBuilddir}/src/test/isolation/pg_isolation_regress";
+if ($isolationtester && ! -f "$isolationRegress")
+{
+    die <<"MESSAGE";
+
+isolationtester not found at $isolationRegress.
+
+isolationtester tests can only be run when source (detected as ${postgresSrcdir})
+and build (detected as ${postgresBuilddir}) directory corresponding to $bindir
+are present.
+
+Additionally isolationtester in src/test/isolation needs to be built,
+which it is not by default if tests have not been run. If the build
+directory is present locally
+"make -C ${postgresBuilddir} all" should do the trick.
+MESSAGE
+}
+
 # Set some default configuration options
 my $masterPort = 57636;
 my $workerCount = 2;
@@ -170,7 +200,6 @@ for my $port (@workerPorts)
 }
 
 # Routine to shutdown servers at failure/exit
-my $serversAreShutdown = "FALSE";
 sub ShutdownServers()
 {
     if ($serversAreShutdown eq "FALSE")
@@ -204,6 +233,9 @@ END
     }
 }
 
+# Signal that servers should be shutdown
+$serversAreShutdown = "FALSE";
+
 # Start servers
 system("$bindir/pg_ctl",
        ('start', '-w',
@@ -309,8 +341,17 @@ push(@arguments, @ARGV);
 my $startTime = time();
 
 # Finally run the tests
-system("$pgxsdir/src/test/regress/pg_regress", @arguments) == 0
+if (!$isolationtester)
-    or die "Could not run regression tests";
+{
+    system("$plainRegress", @arguments) == 0
+	or die "Could not run regression tests";
+}
+else
+{
+    push(@arguments, "--dbname=regression");
+    system("$isolationRegress", @arguments) == 0
+	or die "Could not run isolation tests";
+}
 
 my $endTime = time();
 

src/test/regress/specs/isolation_cluster_management.spec

@@ -0,0 +1,8 @@
+session "s1"
+step "s1a"
+{
+    SELECT master_add_node('localhost', 57637);
+    SELECT master_add_node('localhost', 57638);
+}
+
+permutation "s1a"

src/test/regress/specs/isolation_concurrent_dml.spec

@@ -0,0 +1,40 @@
+setup
+{
+    CREATE TABLE test_table (test_id integer NOT NULL, data text);
+    SELECT master_create_distributed_table('test_table', 'test_id', 'hash');
+    SELECT master_create_worker_shards('test_table', 4, 2);
+}
+
+teardown
+{
+    DROP TABLE IF EXISTS test_table CASCADE;
+}
+
+session "s1"
+
+step "s1-begin"
+{
+    BEGIN;
+}
+
+step "s1-insert"
+{
+    INSERT INTO test_table VALUES(1);
+}
+
+step "s1-commit"
+{
+    COMMIT;
+}
+
+session "s2"
+
+step "s2-update"
+{
+    UPDATE test_table SET data = 'blarg' WHERE test_id = 1;
+}
+
+# verify that an in-progress insert blocks concurrent updates
+permutation "s1-begin" "s1-insert" "s2-update" "s1-commit"
+# but an insert without xact will not block
+permutation "s1-insert" "s2-update"

src/test/regress/specs/isolation_dml_vs_repair.spec

@@ -0,0 +1,110 @@
+setup
+{
+    ALTER SEQUENCE pg_catalog.pg_dist_shardid_seq RESTART 102008;
+    CREATE TABLE test_table (test_id integer NOT NULL, data int);
+    SELECT master_create_distributed_table('test_table', 'test_id', 'hash');
+    SELECT master_create_worker_shards('test_table', 1, 2);
+}
+
+teardown
+{
+    DROP TABLE IF EXISTS test_table CASCADE;
+}
+
+session "s1"
+
+setup
+{
+    DEALLOCATE all;
+    TRUNCATE test_table;
+    PREPARE insertone AS INSERT INTO test_table VALUES(1, 1);
+    PREPARE insertall AS INSERT INTO test_table SELECT test_id, data+1 FROM test_table;
+}
+
+step "s1-begin"
+{
+    BEGIN;
+}
+
+step "s1-insertone"
+{
+    INSERT INTO test_table VALUES(1, 1);
+}
+
+step "s1-prepared-insertone"
+{
+    EXECUTE insertone;
+}
+
+step "s1-insertall"
+{
+    INSERT INTO test_table SELECT test_id, data+1 FROM test_table;
+}
+
+step "s1-prepared-insertall"
+{
+    EXECUTE insertall;
+}
+
+step "s1-display"
+{
+    SELECT * FROM test_table WHERE test_id = 1;
+}
+
+step "s1-commit"
+{
+    COMMIT;
+}
+
+session "s2"
+
+
+step "s2-begin"
+{
+    BEGIN;
+}
+
+step "s2-invalidate-57637"
+{
+    UPDATE pg_dist_shard_placement SET shardstate = '3' WHERE shardid = 102008 AND nodeport = 57637;
+}
+
+step "s2-revalidate-57637"
+{
+    UPDATE pg_dist_shard_placement SET shardstate = '1' WHERE shardid = 102008 AND nodeport = 57637;
+}
+
+step "s2-invalidate-57638"
+{
+    UPDATE pg_dist_shard_placement SET shardstate = '3' WHERE shardid = 102008 AND nodeport = 57638;
+}
+
+step "s2-revalidate-57638"
+{
+    UPDATE pg_dist_shard_placement SET shardstate = '1' WHERE shardid = 102008 AND nodeport = 57638;
+}
+
+step "s2-repair"
+{
+    SELECT master_copy_shard_placement(102008, 'localhost', 57638, 'localhost', 57637);
+}
+
+step "s2-commit"
+{
+    COMMIT;
+}
+
+# verify that repair is blocked by ongoing modifying simple transaction
+permutation "s2-invalidate-57637" "s1-begin" "s1-insertone" "s2-repair" "s1-commit"
+
+# verify that repair is blocked by ongoing modifying insert...select transaction
+permutation "s1-insertone" "s2-invalidate-57637" "s1-begin" "s1-insertall" "s2-repair" "s1-commit"
+
+# verify that modifications wait for shard repair
+permutation "s2-invalidate-57637" "s2-begin" "s2-repair" "s1-insertone" "s2-commit" "s2-invalidate-57638" "s1-display" "s2-invalidate-57637" "s2-revalidate-57638" "s1-display"
+
+# verify that prepared plain modifications wait for shard repair (and then fail to avoid race)
+permutation "s2-invalidate-57637" "s1-prepared-insertone" "s2-begin" "s2-repair" "s1-prepared-insertone" "s2-commit" "s2-invalidate-57638" "s1-display" "s2-invalidate-57637" "s2-revalidate-57638" "s1-display"
+
+# verify that prepared INSERT ... SELECT waits for shard repair  (and then fail to avoid race)
+permutation "s2-invalidate-57637" "s1-insertone" "s1-prepared-insertall" "s2-begin" "s2-repair" "s1-prepared-insertall" "s2-commit" "s2-invalidate-57638" "s1-display" "s2-invalidate-57637" "s2-revalidate-57638" "s1-display"