From 83a2cfbfcfa13b2b5bcf06d169c665ef7da58288 Mon Sep 17 00:00:00 2001 From: Ahmet Gedemenli Date: Thu, 6 Apr 2023 11:42:49 +0300 Subject: [PATCH] Move cleanup record test to upgrade schedule (#6794) DESCRIPTION: Move cleanup record test to upgrade schedule --- .../after_citus_upgrade_coord_schedule | 1 + .../before_citus_upgrade_coord_schedule | 1 + src/test/regress/expected/multi_extension.out | 62 ------------------- .../regress/expected/upgrade_basic_after.out | 25 ++++++-- .../upgrade_pg_dist_cleanup_after.out | 13 ++++ .../upgrade_pg_dist_cleanup_after_0.out | 30 +++++++++ .../upgrade_pg_dist_cleanup_before.out | 13 ++++ .../upgrade_pg_dist_cleanup_before_0.out | 36 +++++++++++ src/test/regress/sql/multi_extension.sql | 29 --------- src/test/regress/sql/upgrade_basic_after.sql | 17 ++++- .../sql/upgrade_pg_dist_cleanup_after.sql | 15 +++++ .../sql/upgrade_pg_dist_cleanup_before.sql | 22 +++++++ 12 files changed, 165 insertions(+), 99 deletions(-) create mode 100644 src/test/regress/expected/upgrade_pg_dist_cleanup_after.out create mode 100644 src/test/regress/expected/upgrade_pg_dist_cleanup_after_0.out create mode 100644 src/test/regress/expected/upgrade_pg_dist_cleanup_before.out create mode 100644 src/test/regress/expected/upgrade_pg_dist_cleanup_before_0.out create mode 100644 src/test/regress/sql/upgrade_pg_dist_cleanup_after.sql create mode 100644 src/test/regress/sql/upgrade_pg_dist_cleanup_before.sql diff --git a/src/test/regress/after_citus_upgrade_coord_schedule b/src/test/regress/after_citus_upgrade_coord_schedule index b36151ce6..6a2a5255a 100644 --- a/src/test/regress/after_citus_upgrade_coord_schedule +++ b/src/test/regress/after_citus_upgrade_coord_schedule @@ -1,6 +1,7 @@ # this schedule is to be run only on coordinators test: upgrade_citus_finish_citus_upgrade +test: upgrade_pg_dist_cleanup_after test: upgrade_basic_after test: upgrade_partition_constraints_after test: upgrade_pg_dist_object_test_after diff --git a/src/test/regress/before_citus_upgrade_coord_schedule b/src/test/regress/before_citus_upgrade_coord_schedule index 169a7f418..0e0eaa091 100644 --- a/src/test/regress/before_citus_upgrade_coord_schedule +++ b/src/test/regress/before_citus_upgrade_coord_schedule @@ -4,4 +4,5 @@ test: upgrade_basic_before test: upgrade_partition_constraints_before test: upgrade_pg_dist_object_test_before test: upgrade_columnar_metapage_before +test: upgrade_pg_dist_cleanup_before test: upgrade_post_11_before diff --git a/src/test/regress/expected/multi_extension.out b/src/test/regress/expected/multi_extension.out index 85e77160a..8d68f1aa3 100644 --- a/src/test/regress/expected/multi_extension.out +++ b/src/test/regress/expected/multi_extension.out @@ -1197,44 +1197,7 @@ SELECT * FROM multi_extension.print_extension_changes(); -- Test downgrade to 11.1-1 from 11.2-1 ALTER EXTENSION citus UPDATE TO '11.2-1'; --- create a table with orphaned shards to see if orphaned shards will be dropped --- and cleanup records will be created for them -SET citus.shard_replication_factor to 1; -CREATE TABLE table_with_orphaned_shards (a int); -SELECT create_distributed_table('table_with_orphaned_shards', 'a'); - create_distributed_table ---------------------------------------------------------------------- - -(1 row) - --- show there are 4 placements -SELECT * FROM pg_dist_placement ORDER BY shardid; - placementid | shardid | shardstate | shardlength | groupid ---------------------------------------------------------------------- - 1 | 102008 | 1 | 0 | 0 - 2 | 102009 | 1 | 0 | 0 - 3 | 102010 | 1 | 0 | 0 - 4 | 102011 | 1 | 0 | 0 -(4 rows) - --- mark two of them as orphaned -UPDATE pg_dist_placement SET shardstate = 4 WHERE shardid % 2 = 1; ALTER EXTENSION citus UPDATE TO '11.1-1'; --- show placements and cleanup records -SELECT * FROM pg_dist_placement ORDER BY shardid; - placementid | shardid | shardstate | shardlength | groupid ---------------------------------------------------------------------- - 1 | 102008 | 1 | 0 | 0 - 2 | 102009 | 4 | 0 | 0 - 3 | 102010 | 1 | 0 | 0 - 4 | 102011 | 4 | 0 | 0 -(4 rows) - -SELECT * FROM pg_dist_cleanup; - record_id | operation_id | object_type | object_name | node_group_id | policy_type ---------------------------------------------------------------------- -(0 rows) - -- Should be empty result since upgrade+downgrade should be a no-op SELECT * FROM multi_extension.print_extension_changes(); previous_object | current_object @@ -1243,21 +1206,6 @@ SELECT * FROM multi_extension.print_extension_changes(); -- Snapshot of state at 11.2-1 ALTER EXTENSION citus UPDATE TO '11.2-1'; --- verify that the placements are deleted and cleanup records are created -SELECT * FROM pg_dist_placement ORDER BY shardid; - placementid | shardid | shardstate | shardlength | groupid ---------------------------------------------------------------------- - 1 | 102008 | 1 | 0 | 0 - 3 | 102010 | 1 | 0 | 0 -(2 rows) - -SELECT * FROM pg_dist_cleanup; - record_id | operation_id | object_type | object_name | node_group_id | policy_type ---------------------------------------------------------------------- - 1 | 0 | 1 | table_with_orphaned_shards_102009 | 0 | 0 - 2 | 0 | 1 | table_with_orphaned_shards_102011 | 0 | 0 -(2 rows) - ALTER EXTENSION citus_columnar UPDATE TO '11.2-1'; -- Make sure that we defined dependencies from all rel objects (tables, -- indexes, sequences ..) to columnar table access method ... @@ -1295,16 +1243,6 @@ SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend; (1 row) DROP TABLE columnar_schema_members, columnar_schema_members_pg_depend; --- error out as cleanup records remain -ALTER EXTENSION citus UPDATE TO '11.0-4'; -ERROR: pg_dist_cleanup is introduced in Citus 11.1 -HINT: To downgrade Citus to an older version, you should first cleanup all the orphaned resources and make sure pg_dist_cleanup is empty, by executing CALL citus_cleanup_orphaned_resources(); -CONTEXT: PL/pgSQL function inline_code_block line XX at RAISE --- cleanup -SET client_min_messages TO ERROR; -CALL citus_cleanup_orphaned_resources(); -DROP TABLE table_with_orphaned_shards; -RESET client_min_messages; SELECT * FROM multi_extension.print_extension_changes(); previous_object | current_object --------------------------------------------------------------------- diff --git a/src/test/regress/expected/upgrade_basic_after.out b/src/test/regress/expected/upgrade_basic_after.out index e1fab2685..ff118c593 100644 --- a/src/test/regress/expected/upgrade_basic_after.out +++ b/src/test/regress/expected/upgrade_basic_after.out @@ -39,16 +39,29 @@ SELECT nextval('pg_dist_colocationid_seq') = MAX(colocationid)+1 FROM pg_dist_co t (1 row) -SELECT nextval('pg_dist_operationid_seq') = MAX(operation_id)+1 FROM pg_dist_cleanup; - ?column? +-- 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)+1 + END AS check_operationid + FROM pg_dist_cleanup; + check_operationid --------------------------------------------------------------------- - + t (1 row) -SELECT nextval('pg_dist_cleanup_recordid_seq') = MAX(record_id)+1 FROM pg_dist_cleanup; - ?column? +SELECT + CASE WHEN MAX(record_id) IS NULL + THEN true + ELSE nextval('pg_dist_cleanup_recordid_seq') = MAX(record_id)+1 + 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; diff --git a/src/test/regress/expected/upgrade_pg_dist_cleanup_after.out b/src/test/regress/expected/upgrade_pg_dist_cleanup_after.out new file mode 100644 index 000000000..a55945e55 --- /dev/null +++ b/src/test/regress/expected/upgrade_pg_dist_cleanup_after.out @@ -0,0 +1,13 @@ +\set upgrade_test_old_citus_version `echo "$CITUS_OLD_VERSION"` +SELECT substring(:'upgrade_test_old_citus_version', 'v(\d+)\.\d+\.\d+')::int > 11 OR + (substring(:'upgrade_test_old_citus_version', 'v(\d+)\.\d+\.\d+')::int = 11 AND + substring(:'upgrade_test_old_citus_version', 'v\d+\.(\d+)\.\d+')::int >= 2) +AS upgrade_test_old_citus_version_gte_11_2; + upgrade_test_old_citus_version_gte_11_2 +--------------------------------------------------------------------- + t +(1 row) + +\gset +\if :upgrade_test_old_citus_version_gte_11_2 +\q \ No newline at end of file diff --git a/src/test/regress/expected/upgrade_pg_dist_cleanup_after_0.out b/src/test/regress/expected/upgrade_pg_dist_cleanup_after_0.out new file mode 100644 index 000000000..d71fad887 --- /dev/null +++ b/src/test/regress/expected/upgrade_pg_dist_cleanup_after_0.out @@ -0,0 +1,30 @@ +\set upgrade_test_old_citus_version `echo "$CITUS_OLD_VERSION"` +SELECT substring(:'upgrade_test_old_citus_version', 'v(\d+)\.\d+\.\d+')::int > 11 OR + (substring(:'upgrade_test_old_citus_version', 'v(\d+)\.\d+\.\d+')::int = 11 AND + substring(:'upgrade_test_old_citus_version', 'v\d+\.(\d+)\.\d+')::int >= 2) +AS upgrade_test_old_citus_version_gte_11_2; + upgrade_test_old_citus_version_gte_11_2 +--------------------------------------------------------------------- + f +(1 row) + +\gset +\if :upgrade_test_old_citus_version_gte_11_2 +\q +\endif +-- verify that the orphaned placement is deleted and cleanup record is created +SELECT COUNT(*) FROM pg_dist_placement WHERE shardid IN (SELECT shardid FROM pg_dist_shard WHERE logicalrelid='table_with_orphaned_shards'::regclass); + count +--------------------------------------------------------------------- + 32 +(1 row) + +SELECT * FROM pg_dist_cleanup; + record_id | operation_id | object_type | object_name | node_group_id | policy_type +--------------------------------------------------------------------- + 1 | 0 | 1 | table_with_orphaned_shards_980001 | 1 | 0 +(1 row) + +CALL citus_cleanup_orphaned_resources(); +NOTICE: cleaned up 1 orphaned resources +DROP TABLE table_with_orphaned_shards; diff --git a/src/test/regress/expected/upgrade_pg_dist_cleanup_before.out b/src/test/regress/expected/upgrade_pg_dist_cleanup_before.out new file mode 100644 index 000000000..ebfe15ebe --- /dev/null +++ b/src/test/regress/expected/upgrade_pg_dist_cleanup_before.out @@ -0,0 +1,13 @@ +\set upgrade_test_old_citus_version `echo "$CITUS_OLD_VERSION"` +SELECT substring(:'upgrade_test_old_citus_version', 'v(\d+)\.\d+\.\d+')::int > 11 OR + (substring(:'upgrade_test_old_citus_version', 'v(\d+)\.\d+\.\d+')::int = 11 AND + substring(:'upgrade_test_old_citus_version', 'v\d+\.(\d+)\.\d+')::int >= 2) +AS upgrade_test_old_citus_version_gte_11_2; + upgrade_test_old_citus_version_gte_11_2 +--------------------------------------------------------------------- + t +(1 row) + +\gset +\if :upgrade_test_old_citus_version_gte_11_2 +\q diff --git a/src/test/regress/expected/upgrade_pg_dist_cleanup_before_0.out b/src/test/regress/expected/upgrade_pg_dist_cleanup_before_0.out new file mode 100644 index 000000000..a0cf9ceb1 --- /dev/null +++ b/src/test/regress/expected/upgrade_pg_dist_cleanup_before_0.out @@ -0,0 +1,36 @@ +\set upgrade_test_old_citus_version `echo "$CITUS_OLD_VERSION"` +SELECT substring(:'upgrade_test_old_citus_version', 'v(\d+)\.\d+\.\d+')::int > 11 OR + (substring(:'upgrade_test_old_citus_version', 'v(\d+)\.\d+\.\d+')::int = 11 AND + substring(:'upgrade_test_old_citus_version', 'v\d+\.(\d+)\.\d+')::int >= 2) +AS upgrade_test_old_citus_version_gte_11_2; + upgrade_test_old_citus_version_gte_11_2 +--------------------------------------------------------------------- + f +(1 row) + +\gset +\if :upgrade_test_old_citus_version_gte_11_2 +\q +\endif +-- create a table with orphaned shards to see if orphaned shards will be dropped +-- and cleanup records will be created for them +SET citus.next_shard_id TO 980000; +CREATE TABLE table_with_orphaned_shards (a int); +SELECT create_distributed_table('table_with_orphaned_shards', 'a'); + create_distributed_table +--------------------------------------------------------------------- + +(1 row) + +-- show all 32 placements are active +SELECT COUNT(*) FROM pg_dist_placement WHERE shardstate = 1 AND shardid IN (SELECT shardid FROM pg_dist_shard WHERE logicalrelid='table_with_orphaned_shards'::regclass); + count +--------------------------------------------------------------------- + 32 +(1 row) + +-- create an orphaned placement based on an existing one +INSERT INTO pg_dist_placement(placementid, shardid, shardstate, shardlength, groupid) + SELECT nextval('pg_dist_placement_placementid_seq'::regclass), shardid, 4, shardlength, 3-groupid + FROM pg_dist_placement + WHERE shardid = 980001; diff --git a/src/test/regress/sql/multi_extension.sql b/src/test/regress/sql/multi_extension.sql index d202227ae..c43003469 100644 --- a/src/test/regress/sql/multi_extension.sql +++ b/src/test/regress/sql/multi_extension.sql @@ -529,33 +529,13 @@ SELECT * FROM multi_extension.print_extension_changes(); -- Test downgrade to 11.1-1 from 11.2-1 ALTER EXTENSION citus UPDATE TO '11.2-1'; - --- create a table with orphaned shards to see if orphaned shards will be dropped --- and cleanup records will be created for them -SET citus.shard_replication_factor to 1; -CREATE TABLE table_with_orphaned_shards (a int); -SELECT create_distributed_table('table_with_orphaned_shards', 'a'); --- show there are 4 placements -SELECT * FROM pg_dist_placement ORDER BY shardid; --- mark two of them as orphaned -UPDATE pg_dist_placement SET shardstate = 4 WHERE shardid % 2 = 1; - ALTER EXTENSION citus UPDATE TO '11.1-1'; - --- show placements and cleanup records -SELECT * FROM pg_dist_placement ORDER BY shardid; -SELECT * FROM pg_dist_cleanup; - -- Should be empty result since upgrade+downgrade should be a no-op SELECT * FROM multi_extension.print_extension_changes(); -- Snapshot of state at 11.2-1 ALTER EXTENSION citus UPDATE TO '11.2-1'; --- verify that the placements are deleted and cleanup records are created -SELECT * FROM pg_dist_placement ORDER BY shardid; -SELECT * FROM pg_dist_cleanup; - ALTER EXTENSION citus_columnar UPDATE TO '11.2-1'; -- Make sure that we defined dependencies from all rel objects (tables, @@ -589,15 +569,6 @@ SELECT COUNT(*)=5 FROM columnar_schema_members_pg_depend; DROP TABLE columnar_schema_members, columnar_schema_members_pg_depend; --- error out as cleanup records remain -ALTER EXTENSION citus UPDATE TO '11.0-4'; - --- cleanup -SET client_min_messages TO ERROR; -CALL citus_cleanup_orphaned_resources(); -DROP TABLE table_with_orphaned_shards; -RESET client_min_messages; - SELECT * FROM multi_extension.print_extension_changes(); -- Test downgrade to 11.2-1 from 11.3-1 diff --git a/src/test/regress/sql/upgrade_basic_after.sql b/src/test/regress/sql/upgrade_basic_after.sql index 2a4c20b3a..03c218a06 100644 --- a/src/test/regress/sql/upgrade_basic_after.sql +++ b/src/test/regress/sql/upgrade_basic_after.sql @@ -8,8 +8,21 @@ SELECT nextval('pg_dist_placement_placementid_seq') = MAX(placementid)+1 FROM pg SELECT nextval('pg_dist_groupid_seq') = MAX(groupid)+1 FROM pg_dist_node; SELECT nextval('pg_dist_node_nodeid_seq') = MAX(nodeid)+1 FROM pg_dist_node; SELECT nextval('pg_dist_colocationid_seq') = MAX(colocationid)+1 FROM pg_dist_colocation; -SELECT nextval('pg_dist_operationid_seq') = MAX(operation_id)+1 FROM pg_dist_cleanup; -SELECT nextval('pg_dist_cleanup_recordid_seq') = MAX(record_id)+1 FROM pg_dist_cleanup; +-- 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)+1 + 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)+1 + 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; diff --git a/src/test/regress/sql/upgrade_pg_dist_cleanup_after.sql b/src/test/regress/sql/upgrade_pg_dist_cleanup_after.sql new file mode 100644 index 000000000..e84c35b60 --- /dev/null +++ b/src/test/regress/sql/upgrade_pg_dist_cleanup_after.sql @@ -0,0 +1,15 @@ +\set upgrade_test_old_citus_version `echo "$CITUS_OLD_VERSION"` +SELECT substring(:'upgrade_test_old_citus_version', 'v(\d+)\.\d+\.\d+')::int > 11 OR + (substring(:'upgrade_test_old_citus_version', 'v(\d+)\.\d+\.\d+')::int = 11 AND + substring(:'upgrade_test_old_citus_version', 'v\d+\.(\d+)\.\d+')::int >= 2) +AS upgrade_test_old_citus_version_gte_11_2; +\gset +\if :upgrade_test_old_citus_version_gte_11_2 +\q +\endif + +-- verify that the orphaned placement is deleted and cleanup record is created +SELECT COUNT(*) FROM pg_dist_placement WHERE shardid IN (SELECT shardid FROM pg_dist_shard WHERE logicalrelid='table_with_orphaned_shards'::regclass); +SELECT * FROM pg_dist_cleanup; +CALL citus_cleanup_orphaned_resources(); +DROP TABLE table_with_orphaned_shards; diff --git a/src/test/regress/sql/upgrade_pg_dist_cleanup_before.sql b/src/test/regress/sql/upgrade_pg_dist_cleanup_before.sql new file mode 100644 index 000000000..62ec8a1fb --- /dev/null +++ b/src/test/regress/sql/upgrade_pg_dist_cleanup_before.sql @@ -0,0 +1,22 @@ +\set upgrade_test_old_citus_version `echo "$CITUS_OLD_VERSION"` +SELECT substring(:'upgrade_test_old_citus_version', 'v(\d+)\.\d+\.\d+')::int > 11 OR + (substring(:'upgrade_test_old_citus_version', 'v(\d+)\.\d+\.\d+')::int = 11 AND + substring(:'upgrade_test_old_citus_version', 'v\d+\.(\d+)\.\d+')::int >= 2) +AS upgrade_test_old_citus_version_gte_11_2; +\gset +\if :upgrade_test_old_citus_version_gte_11_2 +\q +\endif + +-- create a table with orphaned shards to see if orphaned shards will be dropped +-- and cleanup records will be created for them +SET citus.next_shard_id TO 980000; +CREATE TABLE table_with_orphaned_shards (a int); +SELECT create_distributed_table('table_with_orphaned_shards', 'a'); +-- show all 32 placements are active +SELECT COUNT(*) FROM pg_dist_placement WHERE shardstate = 1 AND shardid IN (SELECT shardid FROM pg_dist_shard WHERE logicalrelid='table_with_orphaned_shards'::regclass); +-- create an orphaned placement based on an existing one +INSERT INTO pg_dist_placement(placementid, shardid, shardstate, shardlength, groupid) + SELECT nextval('pg_dist_placement_placementid_seq'::regclass), shardid, 4, shardlength, 3-groupid + FROM pg_dist_placement + WHERE shardid = 980001;