From 78d378fd22f74828981692e5e253214bccef4311 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Mon, 22 Jun 2020 16:36:14 -0500
Subject: [PATCH v2 4/5] 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 | 128 +++++++++++++++------------
 1 file changed, 72 insertions(+), 56 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 2393269565..9eb4bc66ae 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);
 
 
@@ -475,9 +479,9 @@ 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->phase = VACUUM_ERRCB_PHASE_UNKNOWN;
+	vacrelstats->errinfo.relnamespace = get_namespace_name(RelationGetNamespace(onerel));
+	vacrelstats->errinfo.relname = pstrdup(RelationGetRelationName(onerel));
+	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;
@@ -501,7 +505,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;
 
@@ -537,7 +541,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);
 		lazy_truncate_heap(onerel, vacrelstats);
 	}
@@ -649,8 +654,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,
@@ -785,13 +790,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;
@@ -827,7 +832,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,
@@ -947,7 +952,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);
 
 		if (blkno == next_unskippable_block)
@@ -1581,7 +1587,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);
 		}
@@ -1602,7 +1608,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,
@@ -1712,7 +1718,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)));
 
 	/*
@@ -1741,7 +1747,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)));
@@ -1819,15 +1825,16 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
 	int			npages;
 	PGRUsage	ru0;
 	Buffer		vmbuffer = InvalidBuffer;
-	int			oldphase = vacrelstats->phase,
-				oldblkno = vacrelstats->blkno;
+	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 */
-	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);
 
 	pg_rusage_init(&ru0);
@@ -1844,7 +1851,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))
@@ -1873,12 +1880,14 @@ 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, oldphase, oldblkno);
+	update_vacuum_error_info(&vacrelstats->errinfo,
+							 olderrinfo.phase,
+							 olderrinfo.blkno);
 }
 
 /*
@@ -1901,13 +1910,14 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
 	int			uncnt = 0;
 	TransactionId visibility_cutoff_xid;
 	bool		all_frozen;
-	int			oldphase = vacrelstats->phase,
-				oldblkno = vacrelstats->blkno;
+	LVSavedPosition	olderrinfo;
 
 	pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_VACUUMED, blkno);
 
 	/* Update error traceback information */
-	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);
 
 	START_CRIT_SECTION();
@@ -1987,7 +1997,9 @@ lazy_vacuum_page(Relation onerel, BlockNumber blkno, Buffer buffer,
 	}
 
 	/* Revert to the previous phase information for error traceback */
-	update_vacuum_error_info(vacrelstats, oldphase, oldblkno);
+	update_vacuum_error_info(&vacrelstats->errinfo,
+							 olderrinfo.phase,
+							 olderrinfo.blkno);
 	return tupindex;
 }
 
@@ -2397,8 +2409,7 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
 	IndexVacuumInfo ivinfo;
 	const char *msg;
 	PGRUsage	ru0;
-	int			oldphase = vacrelstats->phase,
-				oldblkno = vacrelstats->blkno;
+	LVSavedPosition	olderrinfo;
 
 	pg_rusage_init(&ru0);
 
@@ -2411,9 +2422,10 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
 	ivinfo.strategy = vac_strategy;
 
 	/* Update error traceback information */
+	olderrinfo = vacrelstats->errinfo;
 	/* The index name is also saved during this phase */
-	vacrelstats->indname = pstrdup(RelationGetRelationName(indrel));
-	update_vacuum_error_info(vacrelstats,
+	vacrelstats->errinfo.indname = pstrdup(RelationGetRelationName(indrel));
+	update_vacuum_error_info(&vacrelstats->errinfo,
 							 VACUUM_ERRCB_PHASE_VACUUM_INDEX,
 							 InvalidBlockNumber);
 
@@ -2428,13 +2440,15 @@ 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, oldphase, oldblkno);
-	pfree(vacrelstats->indname);
+	update_vacuum_error_info(&vacrelstats->errinfo,
+							 olderrinfo.phase,
+							 olderrinfo.blkno);
+	pfree(vacrelstats->errinfo.indname);
 }
 
 /*
@@ -2451,8 +2465,7 @@ lazy_cleanup_index(Relation indrel,
 	IndexVacuumInfo ivinfo;
 	const char *msg;
 	PGRUsage	ru0;
-	int			oldphase = vacrelstats->phase,
-				oldblkno = vacrelstats->blkno;
+	LVSavedPosition	olderrinfo;
 
 	pg_rusage_init(&ru0);
 
@@ -2466,17 +2479,20 @@ lazy_cleanup_index(Relation indrel,
 	ivinfo.strategy = vac_strategy;
 
 	/* Update error traceback information */
+	olderrinfo = vacrelstats->errinfo;
 	/* The index name is also saved during this phase */
-	vacrelstats->indname = pstrdup(RelationGetRelationName(indrel));
-	update_vacuum_error_info(vacrelstats,
+	vacrelstats->errinfo.indname = pstrdup(RelationGetRelationName(indrel));
+	update_vacuum_error_info(&vacrelstats->errinfo,
 							 VACUUM_ERRCB_PHASE_INDEX_CLEANUP,
 							 InvalidBlockNumber);
 
 	*stats = index_vacuum_cleanup(&ivinfo, *stats);
 
 	/* Revert back to the old phase information for error traceback */
-	update_vacuum_error_info(vacrelstats, oldphase, oldblkno);
-	pfree(vacrelstats->indname);
+	update_vacuum_error_info(&vacrelstats->errinfo,
+							 olderrinfo.phase,
+							 olderrinfo.blkno);
+	pfree(vacrelstats->errinfo.indname);
 
 	if (!(*stats))
 		return;
@@ -2589,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;
 			}
 
@@ -2622,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)
 		{
@@ -2655,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))));
@@ -2720,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;
@@ -3513,13 +3529,13 @@ 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.phase = VACUUM_ERRCB_PHASE_UNKNOWN; /* Not yet processing */
+	vacrelstats.errinfo.relnamespace = get_namespace_name(RelationGetNamespace(onerel));
+	vacrelstats.errinfo.relname = pstrdup(RelationGetRelationName(onerel));
+	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;
 
@@ -3550,7 +3566,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)
 	{
@@ -3591,7 +3607,7 @@ vacuum_error_callback(void *arg)
 
 /* Update vacuum error callback for the current phase and block. */
 static void
-update_vacuum_error_info(LVRelStats *errinfo, int phase, BlockNumber blkno)
+update_vacuum_error_info(LVSavedPosition *errinfo, int phase, BlockNumber blkno)
 {
 	errinfo->blkno = blkno;
 	errinfo->phase = phase;
-- 
2.17.0

