Check all AO segment files during concurrent AO VACUUM

We currently exit VACUUM early when there is a concurrent operation on
an AO relation. Instead of exiting early, go through the rest of the
AO segment files to see if they have crossed threshold for compaction.

src/backend/access/appendonly/appendonlywriter.c

@@ -854,14 +854,22 @@ SetSegnoForCompaction(Relation rel,
 	for (i = 0; i < MAX_AOREL_CONCURRENCY; i++)
 	{
 		AOSegfileStatus *segfilestat = &aoentry->relsegfiles[i];
+		bool		in_compaction_list = list_member_int(compactedSegmentFileList, i);
 
 		ereportif(Debug_appendonly_print_segfile_choice, LOG,
 				  (errmsg("segment file %i for append-only relation \"%s\" (%d): state %d",
 						  i, RelationGetRelationName(rel), RelationGetRelid(rel), segfilestat->state)));
 
+		/*
+		 * Find an available AO segment to use that is currently not in state
+		 * AWAITING_DROP_READY or COMPACTED_AWAITING_DROP, not being used by a
+		 * concurrent transaction, and not already marked to be compacted by
+		 * being in the in_compaction_list.
+		 */
 		if ((segfilestat->state == AWAITING_DROP_READY ||
 			 segfilestat->state == COMPACTED_AWAITING_DROP) &&
-			!usedByConcurrentTransaction(segfilestat, i))
+			!usedByConcurrentTransaction(segfilestat, i) &&
+			!in_compaction_list)
 		{
 			ereportif(Debug_appendonly_print_segfile_choice, LOG,
 					  (errmsg("Found segment awaiting drop for append-only relation \"%s\" (%d)",

src/backend/commands/vacuum.c

@@ -880,25 +880,21 @@ vacuumStatement_Relation(VacuumStmt *vacstmt, Oid relid,
 
 			if (!onerel)
 			{
-				/*
-				 * Couldn't get AccessExclusiveLock.
-				 *
-				 * Since the drop phase needs to be skipped, we need to
-				 * deregister the segnos which were marked for drop in the
-				 * compaction phase
-				 */
-				DeregisterSegnoForCompactionDrop(relid, compactNowList);
-
+				/* Couldn't get AccessExclusiveLock. */
 				PopActiveSnapshot();
 				CommitTransactionCommand();
 
 				/*
-				 * To ensure that vacuum decreases the age for appendonly
-				 * tables even if drop phase is getting skipped, perform
-				 * cleanup phase so that the relfrozenxid value is updated
-				 * correctly in pg_class.
+				 * Skip the performing DROP and continue with other segfiles
+				 * in case they have crossed threshold and need to be
+				 * compacted or marked as AOSEG_STATE_AWAITING_DROP (depending
+				 * if above try_relation_open succeeds or not). To ensure that
+				 * vacuum decreases the age for appendonly tables even if drop
+				 * phase is getting skipped, perform cleanup phase when done
+				 * iterating through all segfiles so that the relfrozenxid
+				 * value is updated correctly in pg_class.
 				 */
-				break;
+				continue;
 			}
 
 			if (HasSerializableBackends(false))

src/test/isolation2/expected/mark_all_aoseg_await_drop.out

+-- Ensure all segfiles crossing vacuum threshold but cannot compact
+-- due to concurrent transaction should be marked in state
+-- AOSEG_STATE_AWAITING_DROP.
+
+CREATE TABLE mark_all_aoseg_await_drop (a int) WITH (appendonly=true);
+CREATE
+
+-- Create 3 aoseg entries
+0: BEGIN;
+BEGIN
+1: BEGIN;
+BEGIN
+2: BEGIN;
+BEGIN
+0: INSERT INTO mark_all_aoseg_await_drop SELECT i FROM generate_series(1, 10)i;
+INSERT 10
+1: INSERT INTO mark_all_aoseg_await_drop SELECT i FROM generate_series(1, 10)i;
+INSERT 10
+2: INSERT INTO mark_all_aoseg_await_drop SELECT i FROM generate_series(1, 10)i;
+INSERT 10
+0: COMMIT;
+COMMIT
+1: COMMIT;
+COMMIT
+2: COMMIT;
+COMMIT
+
+DELETE FROM mark_all_aoseg_await_drop;
+DELETE 30
+
+-- We should see all 3 aosegs in state 1
+0U: SELECT segno, state FROM gp_toolkit.__gp_aoseg_name('mark_all_aoseg_await_drop');
+segno|state
+-----+-----
+1    |1    
+2    |1    
+3    |1    
+(3 rows)
+
+-- VACUUM while another session holds lock
+1: BEGIN;
+BEGIN
+1: SELECT COUNT(*) FROM mark_all_aoseg_await_drop;
+count
+-----
+0    
+(1 row)
+1: INSERT INTO mark_all_aoseg_await_drop SELECT i FROM generate_series(1, 10)i;
+INSERT 10
+2: VACUUM mark_all_aoseg_await_drop;
+VACUUM
+1: END;
+END
+
+-- We should see segno 2 and 3 in state 2 (AOSEG_STATE_AWAITING_DROP)
+-- and segno 1 and 4 in state 1 (AOSEG_STATE_DEFAULT). Segno 1 is not
+-- marked as state 2 because a concurrent transaction has written to
+-- it and did not commit before the vacuum got to it. The vacuum
+-- correctly skipped segno 1 and continued to cycle through the aoseg
+-- entries.
+0U: SELECT segno, state FROM gp_toolkit.__gp_aoseg_name('mark_all_aoseg_await_drop');
+segno|state
+-----+-----
+1    |1    
+2    |2    
+3    |2    
+4    |1    
+(4 rows)

src/test/isolation2/isolation2_schedule

@@ -61,7 +61,7 @@ test: uao/vacuum_self_serializable3_row
 test: uao/vacuum_while_insert_row
 test: uao/vacuum_while_vacuum_row
 test: uao/vacuum_cleanup_row
-test: reorganize_after_ao_vacuum_skip_drop truncate_after_ao_vacuum_skip_drop
+test: reorganize_after_ao_vacuum_skip_drop truncate_after_ao_vacuum_skip_drop mark_all_aoseg_await_drop
 
 # Tests on Append-Optimized tables (column-oriented).
 test: uao/alter_while_vacuum_column uao/alter_while_vacuum2_column

src/test/isolation2/sql/mark_all_aoseg_await_drop.sql

+-- Ensure all segfiles crossing vacuum threshold but cannot compact
+-- due to concurrent transaction should be marked in state
+-- AOSEG_STATE_AWAITING_DROP.
+
+CREATE TABLE mark_all_aoseg_await_drop (a int) WITH (appendonly=true);
+
+-- Create 3 aoseg entries
+0: BEGIN;
+1: BEGIN;
+2: BEGIN;
+0: INSERT INTO mark_all_aoseg_await_drop SELECT i FROM generate_series(1, 10)i;
+1: INSERT INTO mark_all_aoseg_await_drop SELECT i FROM generate_series(1, 10)i;
+2: INSERT INTO mark_all_aoseg_await_drop SELECT i FROM generate_series(1, 10)i;
+0: COMMIT;
+1: COMMIT;
+2: COMMIT;
+
+DELETE FROM mark_all_aoseg_await_drop;
+
+-- We should see all 3 aosegs in state 1
+0U: SELECT segno, state FROM gp_toolkit.__gp_aoseg_name('mark_all_aoseg_await_drop');
+
+-- VACUUM while another session holds lock
+1: BEGIN;
+1: SELECT COUNT(*) FROM mark_all_aoseg_await_drop;
+1: INSERT INTO mark_all_aoseg_await_drop SELECT i FROM generate_series(1, 10)i;
+2: VACUUM mark_all_aoseg_await_drop;
+1: END;
+
+-- We should see segno 2 and 3 in state 2 (AOSEG_STATE_AWAITING_DROP)
+-- and segno 1 and 4 in state 1 (AOSEG_STATE_DEFAULT). Segno 1 is not
+-- marked as state 2 because a concurrent transaction has written to
+-- it and did not commit before the vacuum got to it. The vacuum
+-- correctly skipped segno 1 and continued to cycle through the aoseg
+-- entries.
+0U: SELECT segno, state FROM gp_toolkit.__gp_aoseg_name('mark_all_aoseg_await_drop');