Don't sleep with vacuum_delay_point() while holding locks

It's a bad idea to sleep while holding locks, because you might block
another process that wants to acquire the same lock.  Furthermore,
it's not safe to call ProcessConfigFile() in a critical section or
while holding locks.  So avoid calling vacuum_delay_point() while
holding locks, and also make vacuum_delay_point() return quickly
without sleeping if it's called while interrupts cannot be processed.

To find the vacuum_delay_point() calls that were made while holding
locks, I added "Assert(INTERRUPTS_CAN_BE_PROCESSED())" in
vacuum_delay_point() and ran the regression tests.  I didn't include
that Assert in this commit, because there might be more places that do
that that are not covered by the regression tests, including
extensions.

In ginInsertCleanup(), there was already another vacuum_delay_point()
call later in the loop, while not holding any locks, so just remove
the other call that was made while holding the lock.

In hashbucketcleanup(), move the vacuum_delay_point() call up the
stack to its caller.  The call in hashbucketcleanup() was not able to
handle interrupts because it held a lock, and because there were no
vacuum_delay_point() or CHECK_FOR_INTERRUPTS() calls in the caller's
loop, the whole hash index vacuuming phase was uninterruptible by
pending shutdown or query cancel.  Now it can be interrupted between
buckets.  Unfortunately, hashbucketcleanup() has to process all the
bucket's pages in one go without pausing; fixing that would require
changing how hashbucketcleanup()'s lock chaining works.

In acquire_sample_rows(), move the vacuum_delay_point() call in the
loop to between pages, to a time where we're not holding the buffer
lock.

In master, also remove a misleading CHECK_FOR_INTERRUPTS() call from
pgstat_write_statsfile().  It was a no-op because we're always holding
a lock on the current dshash partition at that point.  A call that
does nothing is harmless, but it makes you think that the loop is
interruptible when in reality it's not.

Author: Kevin Rocker <me@kevinrocker.com>
Author: Andrey Borodin <amborodin@acm.org>
Author: Mostafa <mostafa.nabil.nafie@gmail.com>
Reported-by: Greg Burd <greg@burd.me>
Reported-by: Sergei Kornilov <sk@zsrv.org>
Reviewed-by: Kirill Reshke <reshkekirill@gmail.com>
Reviewed-by: Neil Chen <carpenter.nail.cz@gmail.com>
Tested-by: Greg Burd <greg@burd.me>
Discussion: https://postgr.es/m/19628-c2b17d358181a1ea@postgresql.org
Discussion: https://postgr.es/m/492c6247-43d3-477b-8981-fb0c56767b38%40app.fastmail.com
Backpatch-through: 14

src/backend/access/gin/ginfast.c

@@ -897,8 +897,6 @@ ginInsertCleanup(GinState *ginstate, bool must_empty_list,
 		 */
 		processPendingPage(&accum, &datums, page, FirstOffsetNumber);
 
-		vacuum_delay_point();
-
 		/*
 		 * Is it time to flush memory to disk?	Flush if we are at the end of
 		 * the pending list, or if we have a full row and memory is getting
@@ -1004,10 +1002,12 @@ ginInsertCleanup(GinState *ginstate, bool must_empty_list,
 			UnlockReleaseBuffer(buffer);
 		}
 
+		/* call vacuum_delay_point while not holding any buffer lock */
+		vacuum_delay_point();
+
 		/*
 		 * Read next page in pending list
 		 */
-		vacuum_delay_point();
 		buffer = ReadBuffer(index, blkno);
 		LockBuffer(buffer, GIN_SHARE);
 		page = BufferGetPage(buffer);

src/backend/access/hash/hash.c

@@ -500,6 +500,9 @@ loop_top:
 		Page		page;
 		bool		split_cleanup = false;
 
+		/* call vacuum_delay_point while not holding any buffer lock */
+		vacuum_delay_point();
+
 		/* Get address of bucket's start page */
 		bucket_blkno = BUCKET_TO_BLKNO(cachedmetap, cur_bucket);
 
@@ -713,8 +716,6 @@ hashbucketcleanup(Relation rel, Bucket cur_bucket, Buffer bucket_buf,
 		bool		retain_pin = false;
 		bool		clear_dead_marking = false;
 
-		vacuum_delay_point();
-
 		page = BufferGetPage(buf);
 		opaque = HashPageGetOpaque(page);
 

src/backend/commands/vacuum.c

@@ -2258,6 +2258,14 @@ vacuum_delay_point(void)
 {
 	double		msec = 0;
 
+	/*
+	 * If we're holding locks or holding interrupts for some other reason,
+	 * don't sleep, because we don't want to hold locks any longer than
+	 * necessary.
+	 */
+	if (!INTERRUPTS_CAN_BE_PROCESSED())
+		return;
+
 	/* Always check for interrupts */
 	CHECK_FOR_INTERRUPTS();