From 02b3c009e7aed34291f6405f1cf26a03665a5769 Mon Sep 17 00:00:00 2001 From: Onur Tirtir Date: Tue, 26 Sep 2023 17:52:52 +0300 Subject: [PATCH] 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 (cherry picked from commit 27ac44eb2a6be1c12c06f193d8d8511509a54bca) --- .../after_citus_upgrade_coord_schedule | 1 + src/test/regress/after_pg_upgrade_schedule | 2 +- .../citus_tests/upgrade/citus_upgrade_test.py | 14 ++- .../regress/expected/upgrade_basic_after.out | 94 ------------------- .../upgrade_basic_after_non_mixed.out | 94 +++++++++++++++++++ src/test/regress/sql/upgrade_basic_after.sql | 42 --------- .../sql/upgrade_basic_after_non_mixed.sql | 42 +++++++++ 7 files changed, 147 insertions(+), 142 deletions(-) create mode 100644 src/test/regress/expected/upgrade_basic_after_non_mixed.out create mode 100644 src/test/regress/sql/upgrade_basic_after_non_mixed.sql diff --git a/src/test/regress/after_citus_upgrade_coord_schedule b/src/test/regress/after_citus_upgrade_coord_schedule index f4f6bb29f..83dd1d9eb 100644 --- a/src/test/regress/after_citus_upgrade_coord_schedule +++ b/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 diff --git a/src/test/regress/after_pg_upgrade_schedule b/src/test/regress/after_pg_upgrade_schedule index b47763bdb..82e05cf3f 100644 --- a/src/test/regress/after_pg_upgrade_schedule +++ b/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 diff --git a/src/test/regress/citus_tests/upgrade/citus_upgrade_test.py b/src/test/regress/citus_tests/upgrade/citus_upgrade_test.py index 87b00c83c..1ab448031 100755 --- a/src/test/regress/citus_tests/upgrade/citus_upgrade_test.py +++ b/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( diff --git a/src/test/regress/expected/upgrade_basic_after.out b/src/test/regress/expected/upgrade_basic_after.out index 1bfbfc989..7c0ebfb29 100644 --- a/src/test/regress/expected/upgrade_basic_after.out +++ b/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 diff --git a/src/test/regress/expected/upgrade_basic_after_non_mixed.out b/src/test/regress/expected/upgrade_basic_after_non_mixed.out new file mode 100644 index 000000000..8dbc13bab --- /dev/null +++ b/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) + diff --git a/src/test/regress/sql/upgrade_basic_after.sql b/src/test/regress/sql/upgrade_basic_after.sql index b40501a1e..855c06008 100644 --- a/src/test/regress/sql/upgrade_basic_after.sql +++ b/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 diff --git a/src/test/regress/sql/upgrade_basic_after_non_mixed.sql b/src/test/regress/sql/upgrade_basic_after_non_mixed.sql new file mode 100644 index 000000000..17b8367fb --- /dev/null +++ b/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' + );