On Tue, Jan 21, 2020 at 05:54:59PM -0300, Alvaro Herrera wrote:
> > On Tue, Jan 21, 2020 at 03:11:35PM +0900, Masahiko Sawada wrote:
> > > Some of them conflicts with the current HEAD(62c9b52231). Please rebase 
> > > them.
> > 
> > Sorry, it's due to other vacuum patch in this branch.
> > 
> > New patches won't conflict, except for the 0005, so I renamed it for cfbot.
> > If it's deemed to be useful, I'll make a separate branch for the others.
> 
> I think you have to have some other patches applied before these,
> because in the context lines for some of the hunks there are
> double-slash comments.

And I knew that, so (re)tested that the extracted patches would apply, but it
looks like maybe the patch checker is less smart (or more strict) than git, so
it didn't work after all.

3rd attempt (sorry for the noise).
>From c16989c79fe331a3280bbbfb2dd9c040948cff53 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Thu, 12 Dec 2019 20:54:37 -0600
Subject: [PATCH v12 1/5] vacuum errcontext to show block being processed

As requested here.
https://www.postgresql.org/message-id/20190807235154.erbmr4o4bo6vgnjv%40alap3.anarazel.de
---
 src/backend/access/heap/vacuumlazy.c | 38 ++++++++++++++++++++++++++++++++++--
 1 file changed, 36 insertions(+), 2 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index b331f4c..0c4ec7b 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -287,8 +287,12 @@ typedef struct LVRelStats
 	int			num_index_scans;
 	TransactionId latestRemovedXid;
 	bool		lock_waiter_detected;
-} LVRelStats;
 
+	/* Used by the error callback */
+	char		*relname;
+	char 		*relnamespace;
+	BlockNumber blkno;
+} LVRelStats;
 
 /* A few variables that don't seem worth passing around as parameters */
 static int	elevel = -1;
@@ -358,6 +362,7 @@ static void end_parallel_vacuum(Relation *Irel, IndexBulkDeleteResult **stats,
 								LVParallelState *lps, int nindexes);
 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);
 
 
 /*
@@ -721,6 +726,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 		PROGRESS_VACUUM_MAX_DEAD_TUPLES
 	};
 	int64		initprog_val[3];
+	ErrorContextCallback errcallback;
 
 	pg_rusage_init(&ru0);
 
@@ -867,6 +873,16 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 	else
 		skipping_blocks = false;
 
+	/* Setup error traceback support for ereport() */
+	vacrelstats->relnamespace = get_namespace_name(RelationGetNamespace(onerel));
+	vacrelstats->relname = relname;
+	vacrelstats->blkno = InvalidBlockNumber; /* Not known yet */
+
+	errcallback.callback = vacuum_error_callback;
+	errcallback.arg = (void *) vacrelstats;
+	errcallback.previous = error_context_stack;
+	error_context_stack = &errcallback;
+
 	for (blkno = 0; blkno < nblocks; blkno++)
 	{
 		Buffer		buf;
@@ -888,6 +904,8 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 #define FORCE_CHECK_PAGE() \
 		(blkno == nblocks - 1 && should_attempt_truncation(params, vacrelstats))
 
+		vacrelstats->blkno = blkno;
+
 		pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno);
 
 		if (blkno == next_unskippable_block)
@@ -985,11 +1003,13 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 			}
 
 			/* Work on all the indexes, then the heap */
+			/* Don't use the errcontext handler outside this function */
+			error_context_stack = errcallback.previous;
 			lazy_vacuum_all_indexes(onerel, Irel, indstats,
 									vacrelstats, lps, nindexes);
-
 			/* Remove tuples from heap */
 			lazy_vacuum_heap(onerel, vacrelstats);
+			error_context_stack = &errcallback;
 
 			/*
 			 * Forget the now-vacuumed tuples, and press on, but be careful
@@ -1594,6 +1614,9 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 			RecordPageWithFreeSpace(onerel, blkno, freespace);
 	}
 
+	/* Pop the error context stack */
+	error_context_stack = errcallback.previous;
+
 	/* report that everything is scanned and vacuumed */
 	pgstat_progress_update_param(PROGRESS_VACUUM_HEAP_BLKS_SCANNED, blkno);
 
@@ -3372,3 +3395,14 @@ parallel_vacuum_main(dsm_segment *seg, shm_toc *toc)
 	table_close(onerel, ShareUpdateExclusiveLock);
 	pfree(stats);
 }
+
+/*
+ * Error context callback for errors occurring during vacuum.
+ */
+static void
+vacuum_error_callback(void *arg)
+{
+	LVRelStats *cbarg = arg;
+	errcontext("while scanning block %u of relation \"%s.%s\"",
+			cbarg->blkno, cbarg->relnamespace, cbarg->relname);
+}
-- 
2.7.4

>From 53290bc3f70b9896a0c581fb0ed7eb235840890c Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Thu, 12 Dec 2019 20:34:03 -0600
Subject: [PATCH v12 2/5] add errcontext callback in lazy_vacuum_heap, too

---
 src/backend/access/heap/vacuumlazy.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 0c4ec7b..9f30344 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -1792,11 +1792,21 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
 	int			npages;
 	PGRUsage	ru0;
 	Buffer		vmbuffer = InvalidBuffer;
+	ErrorContextCallback errcallback;
 
 	/* Report that we are now vacuuming the heap */
 	pgstat_progress_update_param(PROGRESS_VACUUM_PHASE,
 								 PROGRESS_VACUUM_PHASE_VACUUM_HEAP);
 
+	/* Setup error traceback support for ereport() */
+	/* vacrelstats->relnamespace already set */
+	/* vacrelstats->relname already set */
+	vacrelstats->blkno = InvalidBlockNumber; /* Not known yet */
+	errcallback.callback = vacuum_error_callback;
+	errcallback.arg = (void *) vacrelstats;
+	errcallback.previous = error_context_stack;
+	error_context_stack = &errcallback;
+
 	pg_rusage_init(&ru0);
 	npages = 0;
 
@@ -1811,6 +1821,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
 		vacuum_delay_point();
 
 		tblk = ItemPointerGetBlockNumber(&vacrelstats->dead_tuples->itemptrs[tupindex]);
+		vacrelstats->blkno = tblk;
 		buf = ReadBufferExtended(onerel, MAIN_FORKNUM, tblk, RBM_NORMAL,
 								 vac_strategy);
 		if (!ConditionalLockBufferForCleanup(buf))
@@ -1831,6 +1842,9 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
 		npages++;
 	}
 
+	/* Pop the error context stack */
+	error_context_stack = errcallback.previous;
+
 	if (BufferIsValid(vmbuffer))
 	{
 		ReleaseBuffer(vmbuffer);
-- 
2.7.4

>From 8eae0a24deea12124c6e487aaa98c101ba3cb7f9 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 20 Jan 2020 14:28:45 -0600
Subject: [PATCH v12 3/5] Add vacrelstats.stage and distinct context message

---
 src/backend/access/heap/vacuumlazy.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index 9f30344..c980d96 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -292,6 +292,7 @@ typedef struct LVRelStats
 	char		*relname;
 	char 		*relnamespace;
 	BlockNumber blkno;
+	int			stage;	/* 0: scan heap; 1: vacuum heap */
 } LVRelStats;
 
 /* A few variables that don't seem worth passing around as parameters */
@@ -877,6 +878,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 	vacrelstats->relnamespace = get_namespace_name(RelationGetNamespace(onerel));
 	vacrelstats->relname = relname;
 	vacrelstats->blkno = InvalidBlockNumber; /* Not known yet */
+	vacrelstats->stage = 0;
 
 	errcallback.callback = vacuum_error_callback;
 	errcallback.arg = (void *) vacrelstats;
@@ -1802,6 +1804,8 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
 	/* vacrelstats->relnamespace already set */
 	/* vacrelstats->relname already set */
 	vacrelstats->blkno = InvalidBlockNumber; /* Not known yet */
+	vacrelstats->stage = 1;
+
 	errcallback.callback = vacuum_error_callback;
 	errcallback.arg = (void *) vacrelstats;
 	errcallback.previous = error_context_stack;
@@ -3417,6 +3421,10 @@ static void
 vacuum_error_callback(void *arg)
 {
 	LVRelStats *cbarg = arg;
-	errcontext("while scanning block %u of relation \"%s.%s\"",
-			cbarg->blkno, cbarg->relnamespace, cbarg->relname);
-}
+
+	if (cbarg->stage == 0)
+		errcontext(_("while scanning block %u of relation \"%s.%s\""),
+				cbarg->blkno, cbarg->relnamespace, cbarg->relname);
+	else if (cbarg->stage == 1)
+		errcontext(_("while vacuuming block %u of relation \"%s.%s\""),
+				cbarg->blkno, cbarg->relnamespace, cbarg->relname);
-- 
2.7.4

>From f92198c6bae4260bee53c6c683198d874c7253a8 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 20 Jan 2020 14:44:06 -0600
Subject: [PATCH v12 4/5] errcontext for lazy_vacuum_index

---
 src/backend/access/heap/vacuumlazy.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index c980d96..fcec582 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -292,7 +292,7 @@ typedef struct LVRelStats
 	char		*relname;
 	char 		*relnamespace;
 	BlockNumber blkno;
-	int			stage;	/* 0: scan heap; 1: vacuum heap */
+	int			stage;	/* 0: scan heap; 1: vacuum heap; 2: vacuum index */
 } LVRelStats;
 
 /* A few variables that don't seem worth passing around as parameters */
@@ -2356,6 +2356,8 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
 	IndexVacuumInfo ivinfo;
 	const char *msg;
 	PGRUsage	ru0;
+	ErrorContextCallback errcallback;
+	LVRelStats	vacrelstats; /* Used for error callback, only */
 
 	pg_rusage_init(&ru0);
 
@@ -2367,10 +2369,24 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
 	ivinfo.num_heap_tuples = reltuples;
 	ivinfo.strategy = vac_strategy;
 
+	/* Setup error traceback support for ereport() */
+	vacrelstats.relnamespace = get_namespace_name(RelationGetNamespace(indrel));
+	vacrelstats.relname = RelationGetRelationName(indrel);
+	vacrelstats.blkno = InvalidBlockNumber; /* Not used */
+	vacrelstats.stage = 2;
+
+	errcallback.callback = vacuum_error_callback;
+	errcallback.arg = (void *) &vacrelstats;
+	errcallback.previous = error_context_stack;
+	error_context_stack = &errcallback;
+
 	/* Do bulk deletion */
 	*stats = index_bulk_delete(&ivinfo, *stats,
 							   lazy_tid_reaped, (void *) dead_tuples);
 
+	/* Pop the error context stack */
+	error_context_stack = errcallback.previous;
+
 	if (IsParallelWorker())
 		msg = gettext_noop("scanned index \"%s\" to remove %d row versions by parallel vacuum worker");
 	else
@@ -3428,3 +3444,7 @@ vacuum_error_callback(void *arg)
 	else if (cbarg->stage == 1)
 		errcontext(_("while vacuuming block %u of relation \"%s.%s\""),
 				cbarg->blkno, cbarg->relnamespace, cbarg->relname);
+	else if (cbarg->stage == 2)
+		errcontext(_("while vacuuming relation \"%s.%s\""),
+				cbarg->relnamespace, cbarg->relname);
+}
-- 
2.7.4

>From 36270f49f0c1114c6d81982442a3474985e4aed8 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryz...@telsasoft.com>
Date: Mon, 20 Jan 2020 15:26:39 -0600
Subject: [PATCH v12 5/5] Avoid extra calls like GetRelationName

---
 src/backend/access/heap/vacuumlazy.c | 71 ++++++++++++++++++------------------
 1 file changed, 35 insertions(+), 36 deletions(-)

diff --git a/src/backend/access/heap/vacuumlazy.c b/src/backend/access/heap/vacuumlazy.c
index fcec582..eaff5fb 100644
--- a/src/backend/access/heap/vacuumlazy.c
+++ b/src/backend/access/heap/vacuumlazy.c
@@ -602,8 +602,8 @@ heap_vacuum_rel(Relation onerel, VacuumParams *params,
 			}
 			appendStringInfo(&buf, msgfmt,
 							 get_database_name(MyDatabaseId),
-							 get_namespace_name(RelationGetNamespace(onerel)),
-							 RelationGetRelationName(onerel),
+							 vacrelstats->relnamespace,
+							 vacrelstats->relname,
 							 vacrelstats->num_index_scans);
 			appendStringInfo(&buf, _("pages: %u removed, %u remain, %u skipped due to pins, %u skipped frozen\n"),
 							 vacrelstats->pages_removed,
@@ -702,7 +702,6 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 	BlockNumber nblocks,
 				blkno;
 	HeapTupleData tuple;
-	char	   *relname;
 	TransactionId relfrozenxid = onerel->rd_rel->relfrozenxid;
 	TransactionId relminmxid = onerel->rd_rel->relminmxid;
 	BlockNumber empty_pages,
@@ -731,18 +730,6 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 
 	pg_rusage_init(&ru0);
 
-	relname = RelationGetRelationName(onerel);
-	if (aggressive)
-		ereport(elevel,
-				(errmsg("aggressively vacuuming \"%s.%s\"",
-						get_namespace_name(RelationGetNamespace(onerel)),
-						relname)));
-	else
-		ereport(elevel,
-				(errmsg("vacuuming \"%s.%s\"",
-						get_namespace_name(RelationGetNamespace(onerel)),
-						relname)));
-
 	empty_pages = vacuumed_pages = 0;
 	next_fsm_block_to_vacuum = (BlockNumber) 0;
 	num_tuples = live_tuples = tups_vacuumed = nkeep = nunused = 0;
@@ -755,8 +742,31 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 	vacrelstats->scanned_pages = 0;
 	vacrelstats->tupcount_pages = 0;
 	vacrelstats->nonempty_pages = 0;
+
+	/* Setup error traceback support for ereport() */
+	vacrelstats->relnamespace = get_namespace_name(RelationGetNamespace(onerel));
+	vacrelstats->relname = RelationGetRelationName(onerel);
+	vacrelstats->blkno = InvalidBlockNumber; /* Not known yet */
+	vacrelstats->stage = 0;
+
+	errcallback.callback = vacuum_error_callback;
+	errcallback.arg = (void *) vacrelstats;
+	errcallback.previous = error_context_stack;
+	error_context_stack = &errcallback;
+
 	vacrelstats->latestRemovedXid = InvalidTransactionId;
 
+	if (aggressive)
+		ereport(elevel,
+				(errmsg("aggressively vacuuming \"%s.%s\"",
+						vacrelstats->relnamespace,
+						vacrelstats->relname)));
+	else
+		ereport(elevel,
+				(errmsg("vacuuming \"%s.%s\"",
+						vacrelstats->relnamespace,
+						vacrelstats->relname)));
+
 	/*
 	 * Initialize the state for a parallel vacuum.  As of now, only one worker
 	 * can be used for an index, so we invoke parallelism only if there are at
@@ -777,7 +787,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",
-								RelationGetRelationName(onerel))));
+								vacrelstats->relname)));
 		}
 		else
 			lps = begin_parallel_vacuum(RelationGetRelid(onerel), Irel,
@@ -874,17 +884,6 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 	else
 		skipping_blocks = false;
 
-	/* Setup error traceback support for ereport() */
-	vacrelstats->relnamespace = get_namespace_name(RelationGetNamespace(onerel));
-	vacrelstats->relname = relname;
-	vacrelstats->blkno = InvalidBlockNumber; /* Not known yet */
-	vacrelstats->stage = 0;
-
-	errcallback.callback = vacuum_error_callback;
-	errcallback.arg = (void *) vacrelstats;
-	errcallback.previous = error_context_stack;
-	error_context_stack = &errcallback;
-
 	for (blkno = 0; blkno < nblocks; blkno++)
 	{
 		Buffer		buf;
@@ -1553,7 +1552,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",
-				 relname, blkno);
+				 vacrelstats->relname, blkno);
 			visibilitymap_clear(onerel, blkno, vmbuffer,
 								VISIBILITYMAP_VALID_BITS);
 		}
@@ -1574,7 +1573,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",
-				 relname, blkno);
+				 vacrelstats->relname, blkno);
 			PageClearAllVisible(page);
 			MarkBufferDirty(buf);
 			visibilitymap_clear(onerel, blkno, vmbuffer,
@@ -1687,7 +1686,7 @@ lazy_scan_heap(Relation onerel, VacuumParams *params, LVRelStats *vacrelstats,
 	if (vacuumed_pages)
 		ereport(elevel,
 				(errmsg("\"%s\": removed %.0f row versions in %u pages",
-						RelationGetRelationName(onerel),
+						vacrelstats->relname,
 						tups_vacuumed, vacuumed_pages)));
 
 	/*
@@ -1716,7 +1715,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",
-					RelationGetRelationName(onerel),
+					vacrelstats->relname,
 					tups_vacuumed, num_tuples,
 					vacrelstats->scanned_pages, nblocks),
 			 errdetail_internal("%s", buf.data)));
@@ -1857,7 +1856,7 @@ lazy_vacuum_heap(Relation onerel, LVRelStats *vacrelstats)
 
 	ereport(elevel,
 			(errmsg("\"%s\": removed %d row versions in %d pages",
-					RelationGetRelationName(onerel),
+					vacrelstats->relname,
 					tupindex, npages),
 			 errdetail_internal("%s", pg_rusage_show(&ru0))));
 }
@@ -2394,7 +2393,7 @@ lazy_vacuum_index(Relation indrel, IndexBulkDeleteResult **stats,
 
 	ereport(elevel,
 			(errmsg(msg,
-					RelationGetRelationName(indrel),
+					vacrelstats.relname,
 					dead_tuples->num_tuples),
 			 errdetail_internal("%s", pg_rusage_show(&ru0))));
 }
@@ -2537,7 +2536,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats)
 				vacrelstats->lock_waiter_detected = true;
 				ereport(elevel,
 						(errmsg("\"%s\": stopping truncate due to conflicting lock request",
-								RelationGetRelationName(onerel))));
+								vacrelstats->relname)));
 				return;
 			}
 
@@ -2602,7 +2601,7 @@ lazy_truncate_heap(Relation onerel, LVRelStats *vacrelstats)
 
 		ereport(elevel,
 				(errmsg("\"%s\": truncated %u to %u pages",
-						RelationGetRelationName(onerel),
+						vacrelstats->relname,
 						old_rel_pages, new_rel_pages),
 				 errdetail_internal("%s",
 									pg_rusage_show(&ru0))));
@@ -2667,7 +2666,7 @@ count_nondeletable_pages(Relation onerel, LVRelStats *vacrelstats)
 				{
 					ereport(elevel,
 							(errmsg("\"%s\": suspending truncate due to conflicting lock request",
-									RelationGetRelationName(onerel))));
+									vacrelstats->relname)));
 
 					vacrelstats->lock_waiter_detected = true;
 					return blkno;
-- 
2.7.4

Reply via email to