Simplify autovacuum work-item implementation

The initial implementation of autovacuum work-items used a dynamic
shared memory area (DSA).  However, it's argued that dynamic shared
memory is not portable enough, so we cannot rely on it being supported
everywhere; at the same time, autovacuum work-items are now a critical
part of the server, so it's not acceptable that they don't work in the
cases where dynamic shared memory is disabled.  Therefore, let's fall
back to a simpler implementation of work-items that just uses
autovacuum's main shared memory segment for storage.

Discussion: https://postgr.es/m/CA+TgmobQVbz4K_+RSmiM9HeRKpy3vS5xnbkL95gSEnWijzprKQ@mail.gmail.com

src/backend/postmaster/autovacuum.c

@@ -245,6 +245,24 @@ typedef enum
 	AutoVacNumSignals			/* must be last */
 }			AutoVacuumSignal;
 
+/*
+ * Autovacuum workitem array, stored in AutoVacuumShmem->av_workItems.  This
+ * list is mostly protected by AutovacuumLock, except that if an item is
+ * marked 'active' other processes must not modify the work-identifying
+ * members.
+ */
+typedef struct AutoVacuumWorkItem
+{
+	AutoVacuumWorkItemType avw_type;
+	bool		avw_used;		/* below data is valid */
+	bool		avw_active;		/* being processed */
+	Oid			avw_database;
+	Oid			avw_relation;
+	BlockNumber avw_blockNumber;
+} AutoVacuumWorkItem;
+
+#define NUM_WORKITEMS	256
+
 /*-------------
  * The main autovacuum shmem struct.  On shared memory we store this main
  * struct and the array of WorkerInfo structs.  This struct keeps:
@@ -255,10 +273,10 @@ typedef enum
  * av_runningWorkers the WorkerInfo non-free queue
  * av_startingWorker pointer to WorkerInfo currently being started (cleared by
  *					the worker itself as soon as it's up and running)
- * av_dsa_handle	handle for allocatable shared memory
+ * av_workItems		work item array
  *
  * This struct is protected by AutovacuumLock, except for av_signal and parts
- * of the worker list (see above).  av_dsa_handle is readable unlocked.
+ * of the worker list (see above).
  *-------------
  */
 typedef struct
@@ -268,8 +286,7 @@ typedef struct
 	dlist_head	av_freeWorkers;
 	dlist_head	av_runningWorkers;
 	WorkerInfo	av_startingWorker;
-	dsa_handle	av_dsa_handle;
+	AutoVacuumWorkItem av_workItems[NUM_WORKITEMS];
-	dsa_pointer av_workitems;
 } AutoVacuumShmemStruct;
 
 static AutoVacuumShmemStruct *AutoVacuumShmem;
@@ -284,32 +301,6 @@ static MemoryContext DatabaseListCxt = NULL;
 /* Pointer to my own WorkerInfo, valid on each worker */
 static WorkerInfo MyWorkerInfo = NULL;
 
-/*
- * Autovacuum workitem array, stored in AutoVacuumShmem->av_workitems.  This
- * list is mostly protected by AutovacuumLock, except that if it's marked
- * 'active' other processes must not modify the work-identifying members,
- * though changing the list pointers is okay.
- */
-typedef struct AutoVacuumWorkItem
-{
-	AutoVacuumWorkItemType avw_type;
-	Oid			avw_database;
-	Oid			avw_relation;
-	BlockNumber avw_blockNumber;
-	bool		avw_active;
-	dsa_pointer avw_next;		/* doubly linked list pointers */
-	dsa_pointer avw_prev;
-} AutoVacuumWorkItem;
-
-#define NUM_WORKITEMS	256
-typedef struct
-{
-	dsa_pointer avs_usedItems;
-	dsa_pointer avs_freeItems;
-} AutovacWorkItems;
-
-static dsa_area *AutoVacuumDSA = NULL;
-
 /* PID of launcher, valid only in worker while shutting down */
 int			AutovacuumLauncherPid = 0;
 
@@ -356,8 +347,6 @@ static void av_sighup_handler(SIGNAL_ARGS);
 static void avl_sigusr2_handler(SIGNAL_ARGS);
 static void avl_sigterm_handler(SIGNAL_ARGS);
 static void autovac_refresh_stats(void);
-static void remove_wi_from_list(dsa_pointer *list, dsa_pointer wi_ptr);
-static void add_wi_to_list(dsa_pointer *list, dsa_pointer wi_ptr);
 
 
 
@@ -631,29 +620,6 @@ AutoVacLauncherMain(int argc, char *argv[])
 	 */
 	rebuild_database_list(InvalidOid);
 
-	/*
-	 * Set up our DSA so that backends can install work-item requests.  It may
-	 * already exist as created by a previous launcher; and we may even be
-	 * already attached to it, if we're here after longjmp'ing above.
-	 */
-	if (!AutoVacuumShmem->av_dsa_handle)
-	{
-		LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
-		AutoVacuumDSA = dsa_create(AutovacuumLock->tranche);
-		/* make sure it doesn't go away even if we do */
-		dsa_pin(AutoVacuumDSA);
-		dsa_pin_mapping(AutoVacuumDSA);
-		AutoVacuumShmem->av_dsa_handle = dsa_get_handle(AutoVacuumDSA);
-		/* delay array allocation until first request */
-		AutoVacuumShmem->av_workitems = InvalidDsaPointer;
-		LWLockRelease(AutovacuumLock);
-	}
-	else if (AutoVacuumDSA == NULL)
-	{
-		AutoVacuumDSA = dsa_attach(AutoVacuumShmem->av_dsa_handle);
-		dsa_pin_mapping(AutoVacuumDSA);
-	}
-
 	/* loop until shutdown request */
 	while (!got_SIGTERM)
 	{
@@ -1697,14 +1663,6 @@ AutoVacWorkerMain(int argc, char *argv[])
 	{
 		char		dbname[NAMEDATALEN];
 
-		if (AutoVacuumShmem->av_dsa_handle)
-		{
-			/* First use of DSA in this worker, so attach to it */
-			Assert(!AutoVacuumDSA);
-			AutoVacuumDSA = dsa_attach(AutoVacuumShmem->av_dsa_handle);
-			dsa_pin_mapping(AutoVacuumDSA);
-		}
-
 		/*
 		 * Report autovac startup to the stats collector.  We deliberately do
 		 * this before InitPostgres, so that the last_autovac_time will get
@@ -1987,6 +1945,7 @@ do_autovacuum(void)
 	int			effective_multixact_freeze_max_age;
 	bool		did_vacuum = false;
 	bool		found_concurrent_worker = false;
+	int			i;
 
 	/*
 	 * StartTransactionCommand and CommitTransactionCommand will automatically
@@ -2557,35 +2516,18 @@ deleted:
 	/*
 	 * Perform additional work items, as requested by backends.
 	 */
-	if (AutoVacuumShmem->av_workitems)
-	{
-		dsa_pointer wi_ptr;
-		AutovacWorkItems *workitems;
-
 	LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
-
+	for (i = 0; i < NUM_WORKITEMS; i++)
-		/*
-		 * Scan the list of pending items, and process the inactive ones in
-		 * our database.
-		 */
-		workitems = (AutovacWorkItems *)
-			dsa_get_address(AutoVacuumDSA, AutoVacuumShmem->av_workitems);
-		wi_ptr = workitems->avs_usedItems;
-
-		while (wi_ptr != InvalidDsaPointer)
 	{
-			AutoVacuumWorkItem *workitem;
+		AutoVacuumWorkItem *workitem = &AutoVacuumShmem->av_workItems[i];
 
-			workitem = (AutoVacuumWorkItem *)
+		if (!workitem->avw_used)
-				dsa_get_address(AutoVacuumDSA, wi_ptr);
+			continue;
+		if (workitem->avw_active)
+			continue;
 
-			if (workitem->avw_database == MyDatabaseId && !workitem->avw_active)
+		/* claim this one, and release lock while performing it */
-			{
-				dsa_pointer next_ptr;
-
-				/* claim this one */
 		workitem->avw_active = true;
-
 		LWLockRelease(AutovacuumLock);
 
 		perform_work_item(workitem);
@@ -2603,19 +2545,11 @@ deleted:
 
 		LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
 
-				/* Put the array item back for the next user */
+		/* and mark it done */
-				next_ptr = workitem->avw_next;
+		workitem->avw_active = false;
-				remove_wi_from_list(&workitems->avs_usedItems, wi_ptr);
+		workitem->avw_used = false;
-				add_wi_to_list(&workitems->avs_freeItems, wi_ptr);
-				wi_ptr = next_ptr;
 	}
-			else
-				wi_ptr = workitem->avw_next;
-		}
-
-		/* all done */
 	LWLockRelease(AutovacuumLock);
-	}
 
 	/*
 	 * We leak table_toast_map here (among other things), but since we're
@@ -3252,104 +3186,32 @@ void
 AutoVacuumRequestWork(AutoVacuumWorkItemType type, Oid relationId,
 					  BlockNumber blkno)
 {
-	AutovacWorkItems *workitems;
+	int			i;
-	dsa_pointer wi_ptr;
-	AutoVacuumWorkItem *workitem;
 
 	LWLockAcquire(AutovacuumLock, LW_EXCLUSIVE);
 
 	/*
-	 * It may be useful to de-duplicate the list upon insertion.  For the only
+	 * Locate an unused work item and fill it with the given data.
-	 * currently existing caller, this is not necessary.
 	 */
-
-	/* First use in this process?  Set up DSA */
-	if (!AutoVacuumDSA)
-	{
-		if (!AutoVacuumShmem->av_dsa_handle)
-		{
-			/* autovacuum launcher not started; nothing can be done */
-			LWLockRelease(AutovacuumLock);
-			return;
-		}
-		AutoVacuumDSA = dsa_attach(AutoVacuumShmem->av_dsa_handle);
-		dsa_pin_mapping(AutoVacuumDSA);
-	}
-
-	/* First use overall?  Allocate work items array */
-	if (AutoVacuumShmem->av_workitems == InvalidDsaPointer)
-	{
-		int			i;
-		AutovacWorkItems *workitems;
-
-		AutoVacuumShmem->av_workitems =
-			dsa_allocate_extended(AutoVacuumDSA,
-								  sizeof(AutovacWorkItems) +
-								  NUM_WORKITEMS * sizeof(AutoVacuumWorkItem),
-								  DSA_ALLOC_NO_OOM);
-		/* if out of memory, silently disregard the request */
-		if (AutoVacuumShmem->av_workitems == InvalidDsaPointer)
-		{
-			LWLockRelease(AutovacuumLock);
-			dsa_detach(AutoVacuumDSA);
-			AutoVacuumDSA = NULL;
-			return;
-		}
-
-		/* Initialize each array entry as a member of the free list */
-		workitems = dsa_get_address(AutoVacuumDSA, AutoVacuumShmem->av_workitems);
-
-		workitems->avs_usedItems = InvalidDsaPointer;
-		workitems->avs_freeItems = InvalidDsaPointer;
 	for (i = 0; i < NUM_WORKITEMS; i++)
 	{
-			/* XXX surely there is a simpler way to do this */
+		AutoVacuumWorkItem *workitem = &AutoVacuumShmem->av_workItems[i];
-			wi_ptr = AutoVacuumShmem->av_workitems + sizeof(AutovacWorkItems) +
-				sizeof(AutoVacuumWorkItem) * i;
-			workitem = (AutoVacuumWorkItem *) dsa_get_address(AutoVacuumDSA, wi_ptr);
 
-			workitem->avw_type = 0;
+		if (workitem->avw_used)
-			workitem->avw_database = InvalidOid;
+			continue;
-			workitem->avw_relation = InvalidOid;
+
+		workitem->avw_used = true;
 		workitem->avw_active = false;
-
-			/* put this item in the free list */
-			workitem->avw_next = workitems->avs_freeItems;
-			workitems->avs_freeItems = wi_ptr;
-		}
-	}
-
-	workitems = (AutovacWorkItems *)
-		dsa_get_address(AutoVacuumDSA, AutoVacuumShmem->av_workitems);
-
-	/* If array is full, disregard the request */
-	if (workitems->avs_freeItems == InvalidDsaPointer)
-	{
-		LWLockRelease(AutovacuumLock);
-		dsa_detach(AutoVacuumDSA);
-		AutoVacuumDSA = NULL;
-		return;
-	}
-
-	/* remove workitem struct from free list ... */
-	wi_ptr = workitems->avs_freeItems;
-	remove_wi_from_list(&workitems->avs_freeItems, wi_ptr);
-
-	/* ... initialize it ... */
-	workitem = dsa_get_address(AutoVacuumDSA, wi_ptr);
 		workitem->avw_type = type;
 		workitem->avw_database = MyDatabaseId;
 		workitem->avw_relation = relationId;
 		workitem->avw_blockNumber = blkno;
-	workitem->avw_active = false;
 
-	/* ... and put it on autovacuum's to-do list */
+		/* done */
-	add_wi_to_list(&workitems->avs_usedItems, wi_ptr);
+		break;
+	}
 
 	LWLockRelease(AutovacuumLock);
-
-	dsa_detach(AutoVacuumDSA);
-	AutoVacuumDSA = NULL;
 }
 
 /*
@@ -3429,6 +3291,8 @@ AutoVacuumShmemInit(void)
 		dlist_init(&AutoVacuumShmem->av_freeWorkers);
 		dlist_init(&AutoVacuumShmem->av_runningWorkers);
 		AutoVacuumShmem->av_startingWorker = NULL;
+		memset(AutoVacuumShmem->av_workItems, 0,
+			   sizeof(AutoVacuumWorkItem) * NUM_WORKITEMS);
 
 		worker = (WorkerInfo) ((char *) AutoVacuumShmem +
 							   MAXALIGN(sizeof(AutoVacuumShmemStruct)));
@@ -3473,59 +3337,3 @@ autovac_refresh_stats(void)
 
 	pgstat_clear_snapshot();
 }
-
-/*
- * Simplistic open-coded list implementation for objects stored in DSA.
- * Each item is doubly linked, but we have no tail pointer, and the "prev"
- * element of the first item is null, not the list.
- */
-
-/*
- * Remove a work item from the given list.
- */
-static void
-remove_wi_from_list(dsa_pointer *list, dsa_pointer wi_ptr)
-{
-	AutoVacuumWorkItem *workitem = dsa_get_address(AutoVacuumDSA, wi_ptr);
-	dsa_pointer next = workitem->avw_next;
-	dsa_pointer prev = workitem->avw_prev;
-
-	workitem->avw_next = workitem->avw_prev = InvalidDsaPointer;
-
-	if (next != InvalidDsaPointer)
-	{
-		workitem = dsa_get_address(AutoVacuumDSA, next);
-		workitem->avw_prev = prev;
-	}
-
-	if (prev != InvalidDsaPointer)
-	{
-		workitem = dsa_get_address(AutoVacuumDSA, prev);
-		workitem->avw_next = next;
-	}
-	else
-		*list = next;
-}
-
-/*
- * Add a workitem to the given list
- */
-static void
-add_wi_to_list(dsa_pointer *list, dsa_pointer wi_ptr)
-{
-	if (*list == InvalidDsaPointer)
-	{
-		/* list is empty; item is now singleton */
-		*list = wi_ptr;
-	}
-	else
-	{
-		AutoVacuumWorkItem *workitem = dsa_get_address(AutoVacuumDSA, wi_ptr);
-		AutoVacuumWorkItem *old = dsa_get_address(AutoVacuumDSA, *list);
-
-		/* Put item at head of list */
-		workitem->avw_next = *list;
-		old->avw_prev = wi_ptr;
-		*list = wi_ptr;
-	}
-}