Move cleanup record test to upgrade schedule (#6794)

DESCRIPTION: Move cleanup record test to upgrade schedule

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

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

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
 ---------------------------------------------------------------------

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;

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

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;

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

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;

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

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;

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;

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;