Fix mixed Citus upgrade tests (#7218)

When testing rolling Citus upgrades, coordinator should not be upgraded
until we upgrade all the workers.

---------

Co-authored-by: Jelte Fennema-Nio <github-tech@jeltef.nl>
(cherry picked from commit 27ac44eb2a)

src/test/regress/after_citus_upgrade_coord_schedule

@@ -3,4 +3,5 @@
 test: upgrade_citus_finish_citus_upgrade
 test: upgrade_pg_dist_cleanup_after
 test: upgrade_basic_after
+test: upgrade_basic_after_non_mixed
 test: upgrade_post_11_after

src/test/regress/after_pg_upgrade_schedule

@@ -1,4 +1,4 @@
-test: upgrade_basic_after upgrade_ref2ref_after upgrade_type_after upgrade_distributed_function_after upgrade_rebalance_strategy_after upgrade_list_citus_objects upgrade_autoconverted_after upgrade_citus_stat_activity upgrade_citus_locks upgrade_single_shard_table_after upgrade_schema_based_sharding_after
+test: upgrade_basic_after upgrade_ref2ref_after upgrade_type_after upgrade_distributed_function_after upgrade_rebalance_strategy_after upgrade_list_citus_objects upgrade_autoconverted_after upgrade_citus_stat_activity upgrade_citus_locks upgrade_single_shard_table_after upgrade_schema_based_sharding_after upgrade_basic_after_non_mixed
 
 # This test cannot be run with run_test.py currently due to its dependence on
 # the specific PG versions that we use to run upgrade tests. For now we leave

src/test/regress/citus_tests/upgrade/citus_upgrade_test.py

@@ -115,9 +115,9 @@ def remove_tar_files(tar_path):
 
 def restart_databases(pg_path, rel_data_path, mixed_mode, config):
     for node_name in config.node_name_to_ports.keys():
-        if (
-            mixed_mode
-            and config.node_name_to_ports[node_name] == config.chosen_random_worker_port
+        if mixed_mode and config.node_name_to_ports[node_name] in (
+            config.chosen_random_worker_port,
+            config.coordinator_port(),
         ):
             continue
         abs_data_path = os.path.abspath(os.path.join(rel_data_path, node_name))
@@ -148,7 +148,10 @@ def restart_database(pg_path, abs_data_path, node_name, node_ports, logfile_pref
 
 def run_alter_citus(pg_path, mixed_mode, config):
     for port in config.node_name_to_ports.values():
-        if mixed_mode and port == config.chosen_random_worker_port:
+        if mixed_mode and port in (
+            config.chosen_random_worker_port,
+            config.coordinator_port(),
+        ):
             continue
         utils.psql(pg_path, port, "ALTER EXTENSION citus UPDATE;")
 
@@ -158,7 +161,8 @@ def verify_upgrade(config, mixed_mode, node_ports):
         actual_citus_version = get_actual_citus_version(config.bindir, port)
         expected_citus_version = MASTER_VERSION
         if expected_citus_version != actual_citus_version and not (
-            mixed_mode and port == config.chosen_random_worker_port
+            mixed_mode
+            and port in (config.chosen_random_worker_port, config.coordinator_port())
         ):
             print(
                 "port: {} citus version {} expected {}".format(

src/test/regress/expected/upgrade_basic_after.out

@@ -9,100 +9,6 @@ SELECT * FROM pg_indexes WHERE schemaname = 'upgrade_basic' and tablename NOT LI
  upgrade_basic | tp        | tp_pkey   |            | CREATE UNIQUE INDEX tp_pkey ON upgrade_basic.tp USING btree (a)
 (3 rows)
 
-SELECT nextval('pg_dist_shardid_seq') > MAX(shardid) FROM pg_dist_shard;
- ?column?
----------------------------------------------------------------------
- t
-(1 row)
-
-SELECT nextval('pg_dist_placement_placementid_seq') > MAX(placementid) FROM pg_dist_placement;
- ?column?
----------------------------------------------------------------------
- t
-(1 row)
-
-SELECT nextval('pg_dist_groupid_seq') > MAX(groupid) FROM pg_dist_node;
- ?column?
----------------------------------------------------------------------
- t
-(1 row)
-
-SELECT nextval('pg_dist_node_nodeid_seq') > MAX(nodeid) FROM pg_dist_node;
- ?column?
----------------------------------------------------------------------
- t
-(1 row)
-
-SELECT nextval('pg_dist_colocationid_seq') > MAX(colocationid) FROM pg_dist_colocation;
- ?column?
----------------------------------------------------------------------
- t
-(1 row)
-
--- while testing sequences on pg_dist_cleanup, they return null in pg upgrade schedule
--- but return a valid value in citus upgrade schedule
--- that's why we accept both NULL and MAX()+1 here
-SELECT
-    CASE WHEN MAX(operation_id) IS NULL
-    THEN true
-    ELSE nextval('pg_dist_operationid_seq') > MAX(operation_id)
-    END AS check_operationid
-    FROM pg_dist_cleanup;
- check_operationid
----------------------------------------------------------------------
- t
-(1 row)
-
-SELECT
-    CASE WHEN MAX(record_id) IS NULL
-    THEN true
-    ELSE nextval('pg_dist_cleanup_recordid_seq') > MAX(record_id)
-    END AS check_recordid
-    FROM pg_dist_cleanup;
- check_recordid
----------------------------------------------------------------------
- t
-(1 row)
-
-SELECT nextval('pg_dist_background_job_job_id_seq') > COALESCE(MAX(job_id), 0) FROM pg_dist_background_job;
- ?column?
----------------------------------------------------------------------
- t
-(1 row)
-
-SELECT nextval('pg_dist_background_task_task_id_seq') > COALESCE(MAX(task_id), 0) FROM pg_dist_background_task;
- ?column?
----------------------------------------------------------------------
- t
-(1 row)
-
-SELECT last_value > 0 FROM pg_dist_clock_logical_seq;
- ?column?
----------------------------------------------------------------------
- t
-(1 row)
-
--- If this query gives output it means we've added a new sequence that should
--- possibly be restored after upgrades.
-SELECT sequence_name FROM information_schema.sequences
-  WHERE sequence_name LIKE 'pg_dist_%'
-  AND sequence_name NOT IN (
-    -- these ones are restored above
-    'pg_dist_shardid_seq',
-    'pg_dist_placement_placementid_seq',
-    'pg_dist_groupid_seq',
-    'pg_dist_node_nodeid_seq',
-    'pg_dist_colocationid_seq',
-    'pg_dist_operationid_seq',
-    'pg_dist_cleanup_recordid_seq',
-    'pg_dist_background_job_job_id_seq',
-    'pg_dist_background_task_task_id_seq',
-    'pg_dist_clock_logical_seq'
-  );
- sequence_name
----------------------------------------------------------------------
-(0 rows)
-
 SELECT logicalrelid FROM pg_dist_partition
   JOIN pg_depend ON logicalrelid=objid
   JOIN pg_catalog.pg_class ON logicalrelid=oid

src/test/regress/expected/upgrade_basic_after_non_mixed.out

@@ -0,0 +1,94 @@
+SELECT nextval('pg_dist_shardid_seq') > MAX(shardid) FROM pg_dist_shard;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+SELECT nextval('pg_dist_placement_placementid_seq') > MAX(placementid) FROM pg_dist_placement;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+SELECT nextval('pg_dist_groupid_seq') > MAX(groupid) FROM pg_dist_node;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+SELECT nextval('pg_dist_node_nodeid_seq') > MAX(nodeid) FROM pg_dist_node;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+SELECT nextval('pg_dist_colocationid_seq') > MAX(colocationid) FROM pg_dist_colocation;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+-- while testing sequences on pg_dist_cleanup, they return null in pg upgrade schedule
+-- but return a valid value in citus upgrade schedule
+-- that's why we accept both NULL and MAX()+1 here
+SELECT
+    CASE WHEN MAX(operation_id) IS NULL
+    THEN true
+    ELSE nextval('pg_dist_operationid_seq') > MAX(operation_id)
+    END AS check_operationid
+    FROM pg_dist_cleanup;
+ check_operationid
+---------------------------------------------------------------------
+ t
+(1 row)
+
+SELECT
+    CASE WHEN MAX(record_id) IS NULL
+    THEN true
+    ELSE nextval('pg_dist_cleanup_recordid_seq') > MAX(record_id)
+    END AS check_recordid
+    FROM pg_dist_cleanup;
+ check_recordid
+---------------------------------------------------------------------
+ t
+(1 row)
+
+SELECT nextval('pg_dist_background_job_job_id_seq') > COALESCE(MAX(job_id), 0) FROM pg_dist_background_job;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+SELECT nextval('pg_dist_background_task_task_id_seq') > COALESCE(MAX(task_id), 0) FROM pg_dist_background_task;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+SELECT last_value > 0 FROM pg_dist_clock_logical_seq;
+ ?column?
+---------------------------------------------------------------------
+ t
+(1 row)
+
+-- If this query gives output it means we've added a new sequence that should
+-- possibly be restored after upgrades.
+SELECT sequence_name FROM information_schema.sequences
+  WHERE sequence_name LIKE 'pg_dist_%'
+  AND sequence_name NOT IN (
+    -- these ones are restored above
+    'pg_dist_shardid_seq',
+    'pg_dist_placement_placementid_seq',
+    'pg_dist_groupid_seq',
+    'pg_dist_node_nodeid_seq',
+    'pg_dist_colocationid_seq',
+    'pg_dist_operationid_seq',
+    'pg_dist_cleanup_recordid_seq',
+    'pg_dist_background_job_job_id_seq',
+    'pg_dist_background_task_task_id_seq',
+    'pg_dist_clock_logical_seq'
+  );
+ sequence_name
+---------------------------------------------------------------------
+(0 rows)
+

src/test/regress/sql/upgrade_basic_after.sql

@@ -3,48 +3,6 @@ BEGIN;
 -- We have the tablename filter to avoid adding an alternative output for when the coordinator is in metadata vs when not
 SELECT * FROM pg_indexes WHERE schemaname = 'upgrade_basic' and tablename NOT LIKE 'r_%' ORDER BY tablename;
 
-SELECT nextval('pg_dist_shardid_seq') > MAX(shardid) FROM pg_dist_shard;
-SELECT nextval('pg_dist_placement_placementid_seq') > MAX(placementid) FROM pg_dist_placement;
-SELECT nextval('pg_dist_groupid_seq') > MAX(groupid) FROM pg_dist_node;
-SELECT nextval('pg_dist_node_nodeid_seq') > MAX(nodeid) FROM pg_dist_node;
-SELECT nextval('pg_dist_colocationid_seq') > MAX(colocationid) FROM pg_dist_colocation;
--- while testing sequences on pg_dist_cleanup, they return null in pg upgrade schedule
--- but return a valid value in citus upgrade schedule
--- that's why we accept both NULL and MAX()+1 here
-SELECT
-    CASE WHEN MAX(operation_id) IS NULL
-    THEN true
-    ELSE nextval('pg_dist_operationid_seq') > MAX(operation_id)
-    END AS check_operationid
-    FROM pg_dist_cleanup;
-SELECT
-    CASE WHEN MAX(record_id) IS NULL
-    THEN true
-    ELSE nextval('pg_dist_cleanup_recordid_seq') > MAX(record_id)
-    END AS check_recordid
-    FROM pg_dist_cleanup;
-SELECT nextval('pg_dist_background_job_job_id_seq') > COALESCE(MAX(job_id), 0) FROM pg_dist_background_job;
-SELECT nextval('pg_dist_background_task_task_id_seq') > COALESCE(MAX(task_id), 0) FROM pg_dist_background_task;
-SELECT last_value > 0 FROM pg_dist_clock_logical_seq;
-
--- If this query gives output it means we've added a new sequence that should
--- possibly be restored after upgrades.
-SELECT sequence_name FROM information_schema.sequences
-  WHERE sequence_name LIKE 'pg_dist_%'
-  AND sequence_name NOT IN (
-    -- these ones are restored above
-    'pg_dist_shardid_seq',
-    'pg_dist_placement_placementid_seq',
-    'pg_dist_groupid_seq',
-    'pg_dist_node_nodeid_seq',
-    'pg_dist_colocationid_seq',
-    'pg_dist_operationid_seq',
-    'pg_dist_cleanup_recordid_seq',
-    'pg_dist_background_job_job_id_seq',
-    'pg_dist_background_task_task_id_seq',
-    'pg_dist_clock_logical_seq'
-  );
-
 SELECT logicalrelid FROM pg_dist_partition
   JOIN pg_depend ON logicalrelid=objid
   JOIN pg_catalog.pg_class ON logicalrelid=oid

src/test/regress/sql/upgrade_basic_after_non_mixed.sql

@@ -0,0 +1,42 @@
+SELECT nextval('pg_dist_shardid_seq') > MAX(shardid) FROM pg_dist_shard;
+SELECT nextval('pg_dist_placement_placementid_seq') > MAX(placementid) FROM pg_dist_placement;
+SELECT nextval('pg_dist_groupid_seq') > MAX(groupid) FROM pg_dist_node;
+SELECT nextval('pg_dist_node_nodeid_seq') > MAX(nodeid) FROM pg_dist_node;
+SELECT nextval('pg_dist_colocationid_seq') > MAX(colocationid) FROM pg_dist_colocation;
+
+-- while testing sequences on pg_dist_cleanup, they return null in pg upgrade schedule
+-- but return a valid value in citus upgrade schedule
+-- that's why we accept both NULL and MAX()+1 here
+SELECT
+    CASE WHEN MAX(operation_id) IS NULL
+    THEN true
+    ELSE nextval('pg_dist_operationid_seq') > MAX(operation_id)
+    END AS check_operationid
+    FROM pg_dist_cleanup;
+SELECT
+    CASE WHEN MAX(record_id) IS NULL
+    THEN true
+    ELSE nextval('pg_dist_cleanup_recordid_seq') > MAX(record_id)
+    END AS check_recordid
+    FROM pg_dist_cleanup;
+SELECT nextval('pg_dist_background_job_job_id_seq') > COALESCE(MAX(job_id), 0) FROM pg_dist_background_job;
+SELECT nextval('pg_dist_background_task_task_id_seq') > COALESCE(MAX(task_id), 0) FROM pg_dist_background_task;
+SELECT last_value > 0 FROM pg_dist_clock_logical_seq;
+
+-- If this query gives output it means we've added a new sequence that should
+-- possibly be restored after upgrades.
+SELECT sequence_name FROM information_schema.sequences
+  WHERE sequence_name LIKE 'pg_dist_%'
+  AND sequence_name NOT IN (
+    -- these ones are restored above
+    'pg_dist_shardid_seq',
+    'pg_dist_placement_placementid_seq',
+    'pg_dist_groupid_seq',
+    'pg_dist_node_nodeid_seq',
+    'pg_dist_colocationid_seq',
+    'pg_dist_operationid_seq',
+    'pg_dist_cleanup_recordid_seq',
+    'pg_dist_background_job_job_id_seq',
+    'pg_dist_background_task_task_id_seq',
+    'pg_dist_clock_logical_seq'
+  );