From 87ff6cc66510233b1b86a250e8ac48e52b7d54c2 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon, 22 Jun 2020 16:36:14 -0500
Subject: [PATCH 3/4] Move error information into a separate struct..

..to avoid copying large LVRelStats struct.

Andres things this is a problem but Tom thinks it's a micro-optimization.

See: b61d161c146328ae6ba9ed937862d66e5c8b035a

This also moves relname and relnamespace (see ef75140fe).

Discussion: https://www.postgresql.org/message-id/20200622200939.jkuniq3vtiumeszj%40alap3.anarazel.de
---
 src/backend/access/heap/vacuumlazy.c | 116 ++++++++++++++-------------
 1 file changed, 62 insertions(+), 54 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 40173480b8..6bd2867660 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -288,10 +288,17 @@ typedef struct LVParallelState
 	int			nindexes_parallel_condcleanup;
 } LVParallelState;
 
-typedef struct LVRelStats
+typedef struct
 {
 	char	   *relnamespace;
 	char	   *relname;
+	char	   *indname;
+	BlockNumber blkno;			/* used only for heap operations */
+	VacErrPhase phase;
+} LVSavedPosition;
+
+typedef struct LVRelStats
+{
 	/* useindex = true means two-pass strategy; false means one-pass */
 	bool		useindex;
 	/* Overall statistics about rel */
@@ -313,10 +320,7 @@ typedef struct LVRelStats
 	TransactionId latestRemovedXid;
 	bool		lock_waiter_detected;
 
-	/* Used for error callback */
-	char	   *indname;
-	BlockNumber blkno;			/* used only for heap operations */
-	VacErrPhase phase;
+	LVSavedPosition errinfo; /* Used for error callback */
 } LVRelStats;
 
 /* A few variables that don't seem worth passing around as parameters */
@@ -388,7 +392,7 @@ static void end_parallel_vacuum(Relation *Irel, IndexBulkDeleteResult **stats,
 static LVSharedIndStats *get_indstats(LVShared *lvshared, int n);
 static bool skip_parallel_vacuum_index(Relation indrel, LVShared *lvshared);
 static void vacuum_error_callback(void *arg);
-static void update_vacuum_error_info(LVRelStats *errinfo, int phase,
+static void update_vacuum_error_info(LVSavedPosition *errinfo, int phase,
 									 BlockNumber blkno, char *indname);
 
 
@@ -475,10 +479,10 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
 
 	vacrelstats = (LVRelStats *) palloc0(sizeof(LVRelStats));
 
-	vacrelstats->relnamespace = get_namespace_name(RelationGetNamespace(onerel));
-	vacrelstats->relname = pstrdup(RelationGetRelationName(onerel));
-	vacrelstats->indname = NULL;
-	vacrelstats->phase = VACUUM_ERRCB_PHASE_UNKNOWN;
+	vacrelstats->errinfo.relnamespace = get_namespace_name(RelationGetNamespace(onerel));
+	vacrelstats->errinfo.relname = pstrdup(RelationGetRelationName(onerel));
+	vacrelstats->errinfo.indname = NULL;
+	vacrelstats->errinfo.phase = VACUUM_ERRCB_PHASE_UNKNOWN;
 	vacrelstats->old_rel_pages = onerel->rd_rel->relpages;
 	vacrelstats->old_live_tuples = onerel->rd_rel->reltuples;
 	vacrelstats->num_index_scans = 0;
@@ -502,7 +506,7 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
 	 * information is restored at the end of those phases.
 	 */
 	errcallback.callback = vacuum_error_callback;
-	errcallback.arg = vacrelstats;
+	errcallback.arg = &vacrelstats->errinfo;
 	errcallback.previous = error_context_stack;
 	error_context_stack = &errcallback;
 
@@ -538,7 +542,8 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
 		 * which we add context information to errors, so we don't need to
 		 * revert to the previous phase.
 		 */
-		update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_TRUNCATE,
+		update_vacuum_error_info(&vacrelstats->errinfo,
+								 VACUUM_ERRCB_PHASE_TRUNCATE,
 								 vacrelstats->nonempty_pages, NULL);
 		lazy_truncate_heap(onerel, vacrelstats);
 	}
@@ -650,8 +655,8 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
 			}
 			appendStringInfo(&buf, msgfmt,
 							 get_database_name(MyDatabaseId),
-							 vacrelstats->relnamespace,
-							 vacrelstats->relname,
+							 vacrelstats->errinfo.relnamespace,
+							 vacrelstats->errinfo.relname,
 							 vacrelstats->num_index_scans);
 			appendStringInfo(&buf, _("pages: %u removed, %u remain, %u skipped due to pins, %u skipped frozen\n"),
 							 vacrelstats->pages_removed,
@@ -786,13 +791,13 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 	if (aggressive)
 		ereport(elevel,
 				(errmsg("aggressively vacuuming \"%s.%s\"",
-						vacrelstats->relnamespace,
-						vacrelstats->relname)));
+						vacrelstats->errinfo.relnamespace,
+						vacrelstats->errinfo.relname)));
 	else
 		ereport(elevel,
 				(errmsg("vacuuming \"%s.%s\"",
-						vacrelstats->relnamespace,
-						vacrelstats->relname)));
+						vacrelstats->errinfo.relnamespace,
+						vacrelstats->errinfo.relname)));
 
 	empty_pages = vacuumed_pages = 0;
 	next_fsm_block_to_vacuum = (BlockNumber) 0;
@@ -828,7 +833,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 			if (params->nworkers > 0)
 				ereport(WARNING,
 						(errmsg("disabling parallel option of vacuum on \"%s\" --- cannot vacuum temporary tables in parallel",
-								vacrelstats->relname)));
+								vacrelstats->errinfo.relname)));
 		}
 		else
 			lps = begin_parallel_vacuum(RelationGetRelid(onerel), Irel,
@@ -948,7 +953,8 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 
 		pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno);
 
-		update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_SCAN_HEAP,
+		update_vacuum_error_info(&vacrelstats->errinfo,
+								 VACUUM_ERRCB_PHASE_SCAN_HEAP,
 								 blkno, NULL);
 
 		if (blkno == next_unskippable_block)
@@ -1582,7 +1588,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 				 && VM_ALL_VISIBLE(onerel, blkno, &vmbuffer))
 		{
 			elog(WARNING, "page is not marked all-visible but visibility map bit is set in relation \"%s\" page %u",
-				 vacrelstats->relname, blkno);
+				 vacrelstats->errinfo.relname, blkno);
 			visibilitymap_clear(onerel, blkno, vmbuffer,
 								VISIBILITYMAP_VALID_BITS);
 		}
@@ -1603,7 +1609,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 		else if (PageIsAllVisible(page) && has_dead_tuples)
 		{
 			elog(WARNING, "page containing dead tuples is marked as all-visible in relation \"%s\" page %u",
-				 vacrelstats->relname, blkno);
+				 vacrelstats->errinfo.relname, blkno);
 			PageClearAllVisible(page);
 			MarkBufferDirty(buf);
 			visibilitymap_clear(onerel, blkno, vmbuffer,
@@ -1713,7 +1719,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 	if (vacuumed_pages)
 		ereport(elevel,
 				(errmsg("\"%s\": removed %.0f row versions in %u pages",
-						vacrelstats->relname,
+						vacrelstats->errinfo.relname,
 						tups_vacuumed, vacuumed_pages)));
 
 	/*
@@ -1742,7 +1748,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 
 	ereport(elevel,
 			(errmsg("\"%s\": found %.0f removable, %.0f nonremovable row versions in %u out of %u pages",
-					vacrelstats->relname,
+					vacrelstats->errinfo.relname,
 					tups_vacuumed, num_tuples,
 					vacrelstats->scanned_pages, nblocks),
 			 errdetail_internal("%s", buf.data)));
@@ -1820,15 +1826,16 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
 	int			npages;
 	PGRUsage	ru0;
 	Buffer		vmbuffer = InvalidBuffer;
-	LVRelStats	olderrinfo;
+	LVSavedPosition	olderrinfo;
 
 	/* Report that we are now vacuuming the heap */
 	pgstat_progress_update_param(PROGRESS_VACUUM_PHASE,
 								 PROGRESS_VACUUM_PHASE_VACUUM_HEAP);
 
 	/* Update error traceback information */
-	olderrinfo = *vacrelstats;
-	update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_VACUUM_HEAP,
+	olderrinfo = vacrelstats->errinfo;
+	update_vacuum_error_info(&vacrelstats->errinfo,
+							 VACUUM_ERRCB_PHASE_VACUUM_HEAP,
 							 InvalidBlockNumber, NULL);
 
 	pg_rusage_init(&ru0);
@@ -1845,7 +1852,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
 		vacuum_delay_point();
 
 		tblk = ItemPointerGetBlockNumber(&vacrelstats->dead_tuples->itemptrs[tupindex]);
-		vacrelstats->blkno = tblk;
+		vacrelstats->errinfo.blkno = tblk;
 		buf = ReadBufferExtended(onerel, MAIN_FORKNUM, tblk, RBM_NORMAL,
 								 vac_strategy);
 		if (!ConditionalLockBufferForCleanup(buf))
@@ -1874,12 +1881,12 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
 
 	ereport(elevel,
 			(errmsg("\"%s\": removed %d row versions in %d pages",
-					vacrelstats->relname,
+					vacrelstats->errinfo.relname,
 					tupindex, npages),
 			 errdetail_internal("%s", pg_rusage_show(&ru0))));
 
 	/* Revert to the previous phase information for error traceback */
-	update_vacuum_error_info(vacrelstats,
+	update_vacuum_error_info(&vacrelstats->errinfo,
 							 olderrinfo.phase,
 							 olderrinfo.blkno,
 							 olderrinfo.indname);
@@ -1905,13 +1912,14 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
 	int			uncnt = 0;
 	TransactionId visibility_cutoff_xid;
 	bool		all_frozen;
-	LVRelStats	olderrinfo;
+	LVSavedPosition	olderrinfo;
 
 	pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_VACUUMED, blkno);
 
 	/* Update error traceback information */
-	olderrinfo = *vacrelstats;
-	update_vacuum_error_info(vacrelstats, VACUUM_ERRCB_PHASE_VACUUM_HEAP,
+	olderrinfo = vacrelstats->errinfo;
+	update_vacuum_error_info(&vacrelstats->errinfo,
+							 VACUUM_ERRCB_PHASE_VACUUM_HEAP,
 							 blkno, NULL);
 
 	START_CRIT_SECTION();
@@ -1991,7 +1999,7 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
 	}
 
 	/* Revert to the previous phase information for error traceback */
-	update_vacuum_error_info(vacrelstats,
+	update_vacuum_error_info(&vacrelstats->errinfo,
 							 olderrinfo.phase,
 							 olderrinfo.blkno,
 							 olderrinfo.indname);
@@ -2404,7 +2412,7 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
 	IndexVacuumInfo ivinfo;
 	const char *msg;
 	PGRUsage	ru0;
-	LVRelStats	olderrinfo;
+	LVSavedPosition	olderrinfo;
 
 	pg_rusage_init(&ru0);
 
@@ -2417,8 +2425,8 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
 	ivinfo.strategy = vac_strategy;
 
 	/* Update error traceback information */
-	olderrinfo = *vacrelstats;
-	update_vacuum_error_info(vacrelstats,
+	olderrinfo = vacrelstats->errinfo;
+	update_vacuum_error_info(&vacrelstats->errinfo,
 							 VACUUM_ERRCB_PHASE_VACUUM_INDEX,
 							 InvalidBlockNumber,
 							 RelationGetRelationName(indrel));
@@ -2434,12 +2442,12 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
 
 	ereport(elevel,
 			(errmsg(msg,
-					vacrelstats->indname,
+					vacrelstats->errinfo.indname,
 					dead_tuples->num_tuples),
 			 errdetail_internal("%s", pg_rusage_show(&ru0))));
 
 	/* Revert to the previous phase information for error traceback */
-	update_vacuum_error_info(vacrelstats,
+	update_vacuum_error_info(&vacrelstats->errinfo,
 							 olderrinfo.phase,
 							 olderrinfo.blkno,
 							 olderrinfo.indname);
@@ -2459,7 +2467,7 @@ lazy_cleanup_index(Relation indrel,
 	IndexVacuumInfo ivinfo;
 	const char *msg;
 	PGRUsage	ru0;
-	LVRelStats	olderrinfo;
+	LVSavedPosition	olderrinfo;
 
 	pg_rusage_init(&ru0);
 
@@ -2473,8 +2481,8 @@ lazy_cleanup_index(Relation indrel,
 	ivinfo.strategy = vac_strategy;
 
 	/* Update error traceback information */
-	olderrinfo = *vacrelstats;
-	update_vacuum_error_info(vacrelstats,
+	olderrinfo = vacrelstats->errinfo;
+	update_vacuum_error_info(&vacrelstats->errinfo,
 							 VACUUM_ERRCB_PHASE_INDEX_CLEANUP,
 							 InvalidBlockNumber,
 							 RelationGetRelationName(indrel));
@@ -2482,7 +2490,7 @@ lazy_cleanup_index(Relation indrel,
 	*stats = index_vacuum_cleanup(&ivinfo, *stats);
 
 	/* Revert back to the old phase information for error traceback */
-	update_vacuum_error_info(vacrelstats,
+	update_vacuum_error_info(&vacrelstats->errinfo,
 							 olderrinfo.phase,
 							 olderrinfo.blkno,
 							 olderrinfo.indname);
@@ -2597,7 +2605,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats)
 				vacrelstats->lock_waiter_detected = true;
 				ereport(elevel,
 						(errmsg("\"%s\": stopping truncate due to conflicting lock request",
-								vacrelstats->relname)));
+								vacrelstats->errinfo.relname)));
 				return;
 			}
 
@@ -2630,7 +2638,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats)
 		 * were vacuuming.
 		 */
 		new_rel_pages = count_nondeletable_pages(onerel, vacrelstats);
-		vacrelstats->blkno = new_rel_pages;
+		vacrelstats->errinfo.blkno = new_rel_pages;
 
 		if (new_rel_pages >= old_rel_pages)
 		{
@@ -2663,7 +2671,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats)
 
 		ereport(elevel,
 				(errmsg("\"%s\": truncated %u to %u pages",
-						vacrelstats->relname,
+						vacrelstats->errinfo.relname,
 						old_rel_pages, new_rel_pages),
 				 errdetail_internal("%s",
 									pg_rusage_show(&ru0))));
@@ -2728,7 +2736,7 @@ count_nondeletable_pages(Relation onerel, LVRelStats *vacrelstats)
 				{
 					ereport(elevel,
 							(errmsg("\"%s\": suspending truncate due to conflicting lock request",
-									vacrelstats->relname)));
+									vacrelstats->errinfo.relname)));
 
 					vacrelstats->lock_waiter_detected = true;
 					return blkno;
@@ -3521,14 +3529,14 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
 	 * Initialize vacrelstats for use as error callback arg by parallel
 	 * worker.
 	 */
-	vacrelstats.relnamespace = get_namespace_name(RelationGetNamespace(onerel));
-	vacrelstats.relname = pstrdup(RelationGetRelationName(onerel));
-	vacrelstats.indname = NULL;
-	vacrelstats.phase = VACUUM_ERRCB_PHASE_UNKNOWN; /* Not yet processing */
+	vacrelstats.errinfo.relnamespace = get_namespace_name(RelationGetNamespace(onerel));
+	vacrelstats.errinfo.relname = pstrdup(RelationGetRelationName(onerel));
+	vacrelstats.errinfo.indname = NULL;
+	vacrelstats.errinfo.phase = VACUUM_ERRCB_PHASE_UNKNOWN; /* Not yet processing */
 
 	/* Setup error traceback support for ereport() */
 	errcallback.callback = vacuum_error_callback;
-	errcallback.arg = &vacrelstats;
+	errcallback.arg = &vacrelstats.errinfo;
 	errcallback.previous = error_context_stack;
 	error_context_stack = &errcallback;
 
@@ -3559,7 +3567,7 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
 static void
 vacuum_error_callback(void *arg)
 {
-	LVRelStats *errinfo = arg;
+	LVSavedPosition *errinfo = arg;
 
 	switch (errinfo->phase)
 	{
@@ -3600,7 +3608,7 @@ vacuum_error_callback(void *arg)
 
 /* Update vacuum error callback for the current phase, block, and index. */
 static void
-update_vacuum_error_info(LVRelStats *errinfo, int phase, BlockNumber blkno,
+update_vacuum_error_info(LVSavedPosition *errinfo, int phase, BlockNumber blkno,
 						 char *indname)
 {
 	errinfo->blkno = blkno;
-- 
2.17.0

