Check subscription state in the wait_for_sync_event.

If no group's subscription is enabled, wait_for_sync_event may get stuck in
an infinite loop. Add a check of the subscription state in the waiting loop
to reflect the fact that an apply worker may quietly die, deactivating its
subscription.

Also, add the correct processing of this behaviour to the Z0DAN and
include a TAP test.

XXX:
This commit highlights the fact that in case of an unsuccessful add_node call,
there are some database objects left.

Author: danolivo

samples/Z0DAN/zodan.sql

@@ -2351,8 +2351,7 @@ BEGIN
         RAISE NOTICE '    OK: %', rpad('Waiting for sync event from ' || src_node_name || ' on new node ' || new_node_name || '...', 120, ' ');
     EXCEPTION
         WHEN OTHERS THEN
-            RAISE NOTICE '    ✗ %', rpad('Unable to wait for sync event from ' || src_node_name || ' on new node ' || new_node_name || ' (error: ' || SQLERRM || ')', 120, ' ');
-            RAISE;
+            RAISE EXCEPTION '    ✗ %', rpad('Unable to wait for sync event from ' || src_node_name || ' on new node ' || new_node_name || ' (error: ' || SQLERRM || ')', 120, ' ');
     END;
 END;
 $$;

sql/spock--6.0.0-devel.sql

@@ -383,8 +383,12 @@ RETURNS void RETURNS NULL ON NULL INPUT VOLATILE LANGUAGE c AS 'MODULE_PATHNAME'
 CREATE FUNCTION spock.sync_event()
 RETURNS pg_lsn RETURNS NULL ON NULL INPUT VOLATILE LANGUAGE c AS 'MODULE_PATHNAME', 'spock_create_sync_event';
 
-CREATE PROCEDURE spock.wait_for_sync_event(OUT result bool, origin_id oid, lsn pg_lsn, timeout int DEFAULT 0)
-AS $$
+CREATE PROCEDURE spock.wait_for_sync_event(
+	OUT result bool,
+	origin_id  oid,
+	lsn        pg_lsn,
+	timeout    int DEFAULT 0
+) AS $$
 DECLARE
 	target_id		oid;
 	elapsed_time	numeric := 0;
@@ -396,6 +400,17 @@ BEGIN
 	target_id := node_id FROM spock.node_info();
 
 	WHILE true LOOP
+		-- If an unresolvable issue occurs with the apply worker, the LR
+		-- progress gets stuck, and we need to check the subscription's state
+		-- carefully.
+		IF NOT EXISTS (SELECT * FROM spock.subscription
+					  WHERE sub_origin = origin_id AND
+							sub_target = target_id AND
+							sub_enabled = true) THEN
+			RAISE EXCEPTION 'Replication % => % does not have any enabled subscription yet',
+							origin_id, target_id;
+		END IF;
+
 		SELECT INTO progress_lsn remote_commit_lsn
 			FROM spock.progress
 			WHERE node_id = target_id AND remote_node_id = origin_id;
@@ -415,8 +430,12 @@ BEGIN
 END;
 $$ LANGUAGE plpgsql;
 
-CREATE PROCEDURE spock.wait_for_sync_event(OUT result bool, origin name, lsn pg_lsn, timeout int DEFAULT 0)
-AS $$
+CREATE PROCEDURE spock.wait_for_sync_event(
+	OUT result bool,
+	origin     name,
+	lsn        pg_lsn,
+	timeout    int DEFAULT 0
+) AS $$
 DECLARE
 	origin_id		oid;
 	target_id		oid;
@@ -430,6 +449,17 @@ BEGIN
 	target_id := node_id FROM spock.node_info();
 
 	WHILE true LOOP
+		-- If an unresolvable issue occurs with the apply worker, the LR
+		-- progress gets stuck, and we need to check the subscription's state
+		-- carefully.
+		IF NOT EXISTS (SELECT * FROM spock.subscription
+					  WHERE sub_origin = origin_id AND
+							sub_target = target_id AND
+							sub_enabled = true) THEN
+			RAISE EXCEPTION 'Replication % => % does not have any enabled subscription yet',
+							origin_id, target_id;
+		END IF;
+
 		SELECT INTO progress_lsn remote_commit_lsn
 			FROM spock.progress
 			WHERE node_id = target_id AND remote_node_id = origin_id;

tests/tap/t/012_zodan_basics.pl

@@ -31,9 +31,15 @@
 psql_or_bail(1, "CREATE TABLE test(x serial PRIMARY KEY)");
 psql_or_bail(1, "INSERT INTO test DEFAULT VALUES");
 
-print STDERR "All supporting stuff has been installed\n";
+print STDERR "All supporting stuff has been installed successfully\n";
 
-print STDERR "Call Z0DAN: n2 => n1";
+# ##############################################################################
+#
+# Basic check that Z0DAN correctly add node to the single-node cluster
+#
+# ##############################################################################
+
+print STDERR "Call Z0DAN: n2 => n1\n";
 psql_or_bail(2, "
     CALL spock.add_node(
         src_node_name := 'n1',
@@ -49,8 +55,13 @@
 
 psql_or_bail(1, "SELECT spock.sub_disable('sub_n1_n2')");
 
-print STDERR "Call Z0DAN: n3 => n2\n";
+# ##############################################################################
+#
+# Z0DAN reject node addition if some subscriptions are disabled
+#
+# ##############################################################################
 
+print STDERR "Call Z0DAN: n3 => n2\n";
 scalar_query(3, "
 	CALL spock.add_node(
 		src_node_name := 'n2',
@@ -63,7 +74,7 @@
 print STDERR "Z0DAN should fail because of a disabled subscription\n";
 
 psql_or_bail(1, "SELECT spock.sub_enable('sub_n1_n2')");
-scalar_query(3, "
+psql_or_bail(3, "
 	CALL spock.add_node(
 		src_node_name := 'n2',
 		src_dsn := 'host=$host dbname=$dbname port=$node_ports->[1] user=$db_user password=$db_password',
@@ -77,6 +88,40 @@
 ok($result eq '1', "Check state of the test table on N3 after the attachment");
 print STDERR "Z0DAN should add N3 to the cluster\n";
 
+# ##############################################################################
+#
+# Test that Z0DAN correctly doesn't add node to the cluster if something happens
+# during the SYNC process.
+#
+# ##############################################################################
+
+# Remove node from the cluster and data leftovers.
+psql_or_bail(3, "\\i ../../samples/Z0DAN/zodremove.sql");
+psql_or_bail(3, "CALL spock.remove_node(target_node_name := 'n3',
+	target_node_dsn := 'host=$host dbname=$dbname port=$node_ports->[2] user=$db_user password=$db_password',
+	verbose_mode := true)");
+psql_or_bail(3, "DROP TABLE test");
+psql_or_bail(3, "DROP EXTENSION lolor");
+
+psql_or_bail(1, "CREATE FUNCTION fake_fn() RETURNS integer LANGUAGE sql AS \$\$ SELECT 1\$\$");
+psql_or_bail(3, "CREATE FUNCTION fake_fn() RETURNS integer LANGUAGE sql AS \$\$ SELECT 1\$\$");
+scalar_query(3, "
+	CALL spock.add_node(
+		src_node_name := 'n2',
+		src_dsn := 'host=$host dbname=$dbname port=$node_ports->[1] user=$db_user password=$db_password',
+		new_node_name := 'n3', new_node_dsn := 'host=$host dbname=$dbname port=$node_ports->[2] user=$db_user password=$db_password',
+		verb := true)");
+
+# TODO:
+# It seems that add_node keeps remnants after unsuccessful execution. It is
+# happened because we have commited some intermediate results before.
+# It would be better to keep remote transaction opened until the end of the
+# operation or just remove these remnants at the end pretending to be a
+# distributed transaction.
+#
+# $result = scalar_query(3, "SELECT count(*) FROM spock.local_node");
+# ok($result eq '0', "N3 is not in the cluster");
+
 # Clean up
 destroy_cluster('Destroy test cluster');
 done_testing();