As part of the ongoing effort to reduce wake-ups when idle/power
consumption, the attached patch modifies the background writer to
hibernate in ten second bursts once the bgwriter laps the clock sweep.
It's fairly well commented, so a description of how it works here
would probably be redundant. The most controversial aspect of this
patch is the fact that it adds a SetLatch() call to MarkBufferDirty(),
though I went to some lengths to ensure that that call will very
probably find the latch already set when it actually matters, so it
only checks a single flag.

Thoughts? I can produce benchmarks, if that helps, but I think it's
fairly unlikely that the patch introduces a measurable performance
regression.

-- 
Peter Geoghegan       http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training and Services
diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
new file mode 100644
index e3ae92d..0fcaf01
*** a/src/backend/bootstrap/bootstrap.c
--- b/src/backend/bootstrap/bootstrap.c
*************** AuxiliaryProcessMain(int argc, char *arg
*** 416,421 ****
--- 416,422 ----
  
  		case BgWriterProcess:
  			/* don't set signals, bgwriter has its own agenda */
+ 			ProcGlobal->bgwriterLatch = &MyProc->procLatch; /* Expose */
  			BackgroundWriterMain();
  			proc_exit(1);		/* should never return */
  
diff --git a/src/backend/port/unix_latch.c b/src/backend/port/unix_latch.c
new file mode 100644
index fc1a579..a5248ba
*** a/src/backend/port/unix_latch.c
--- b/src/backend/port/unix_latch.c
***************
*** 50,55 ****
--- 50,56 ----
  #include "miscadmin.h"
  #include "postmaster/postmaster.h"
  #include "storage/latch.h"
+ #include "storage/proc.h"
  #include "storage/shmem.h"
  
  /* Are we currently in WaitLatch? The signal handler would like to know. */
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
new file mode 100644
index 1f8d2d6..ad8a056
*** a/src/backend/postmaster/bgwriter.c
--- b/src/backend/postmaster/bgwriter.c
***************
*** 51,56 ****
--- 51,58 ----
  #include "storage/ipc.h"
  #include "storage/lwlock.h"
  #include "storage/pmsignal.h"
+ #include "storage/proc.h"
+ #include "storage/procsignal.h"
  #include "storage/shmem.h"
  #include "storage/smgr.h"
  #include "storage/spin.h"
*************** static volatile sig_atomic_t shutdown_re
*** 73,78 ****
--- 75,82 ----
  /*
   * Private state
   */
+ #define BGWRITER_HIBERNATE_MS			10000
+ 
  static bool am_bg_writer = false;
  
  /* Prototypes for private functions */
*************** BackgroundWriterMain(void)
*** 123,129 ****
  	pqsignal(SIGQUIT, bg_quickdie);		/* hard crash time */
  	pqsignal(SIGALRM, SIG_IGN);
  	pqsignal(SIGPIPE, SIG_IGN);
! 	pqsignal(SIGUSR1, SIG_IGN);			/* reserve for ProcSignal */
  	pqsignal(SIGUSR2, SIG_IGN);
  
  	/*
--- 127,133 ----
  	pqsignal(SIGQUIT, bg_quickdie);		/* hard crash time */
  	pqsignal(SIGALRM, SIG_IGN);
  	pqsignal(SIGPIPE, SIG_IGN);
! 	pqsignal(SIGUSR1, procsignal_sigusr1_handler);
  	pqsignal(SIGUSR2, SIG_IGN);
  
  	/*
*************** BackgroundWriterMain(void)
*** 238,278 ****
  	for (;;)
  	{
  		/*
! 		 * Emergency bailout if postmaster has died.  This is to avoid the
! 		 * necessity for manual cleanup of all postmaster children.
  		 */
! 		if (!PostmasterIsAlive())
! 			exit(1);
! 
! 		if (got_SIGHUP)
  		{
! 			got_SIGHUP = false;
! 			ProcessConfigFile(PGC_SIGHUP);
! 			/* update global shmem state for sync rep */
  		}
! 		if (shutdown_requested)
  		{
  			/*
! 			 * From here on, elog(ERROR) should end with exit(1), not send
! 			 * control back to the sigsetjmp block above
  			 */
! 			ExitOnAnyError = true;
! 			/* Normal exit from the bgwriter is here */
! 			proc_exit(0);		/* done */
! 		}
  
! 		/*
! 		 * Do one cycle of dirty-buffer writing.
! 		 */
! 		BgBufferSync();
  
! 		/* Nap for the configured time. */
! 		BgWriterNap();
  	}
  }
  
  /*
   * BgWriterNap -- Nap for the configured time or until a signal is received.
   */
  static void
  BgWriterNap(void)
--- 242,337 ----
  	for (;;)
  	{
  		/*
! 		 * Do one cycle of dirty-buffer writing, potentially hibernating if
! 		 * there have been no buffers to write.
  		 */
! 		if (!BgBufferSync())
  		{
! 			/* Clock sweep was not "lapped" - nap for the configured time. */
! 			BgWriterNap();
  		}
! 		else
  		{
+ 			int res = 0;
  			/*
! 			 * Initiate hibernation by "arming" the process latch. This usage
! 			 * differs from the standard pattern for latches outlined in
! 			 * latch.h, where the latch is generally reset immediately after
! 			 * WaitLatch returns, in the next iteration of an infinite loop.
! 			 *
! 			 * It should only be possible to *really* set the latch from client
! 			 * backends while the bgwriter is idle, and not during productive
! 			 * cycles where buffers are written, or shortly thereafter. It's
! 			 * important that the SetLatch() call within the buffer manager
! 			 * usually inexpensively returns having found that the latch is
! 			 * already set. For write-heavy workloads, it's quite possible that
! 			 * those calls will never actually get the opportunity to really
! 			 * set the latch.
! 			 *
! 			 * By only giving backends the opportunity to really set the
! 			 * latch at this point via ResetLatch(), we avoid the overhead of
! 			 * sending a signal perhaps as much as once per bgwriter cycle, or
! 			 * maybe even more frequently than that due to the asynchronous
! 			 * nature of signals.
  			 */
! 			ResetLatch(&MyProc->procLatch);
  
! 			/*
! 			 * Another nap and sync, in case some buffers were dirtied since
! 			 * the immediately prior BgBufferSync() call and we quite
! 			 * naturally didn't hear about it because the SetLatch calls
! 			 * found the latch already set (i.e. it was "disarmed").
! 			 */
! 			BgWriterNap();
  
! 			/*
! 			 * When syncing, take the opportunity to check one last time if
! 			 * we're still "lapping" the clock sweep, and only proceed if we are.
! 			 */
! 			if (BgBufferSync())
! 				/*
! 				 * Set a timeout so that we can still update BufferAlloc stats
! 				 * reasonably promptly.
! 				 */
! 				res = WaitLatch(&MyProc->procLatch,
! 						WL_LATCH_SET | WL_TIMEOUT | WL_POSTMASTER_DEATH,
! 						BGWRITER_HIBERNATE_MS);
! 			else
! 				/*
! 				 * Changed our mind, set own proc latch, thereby often avoiding
! 				 * signaling from a backend.
! 				 */
! 				SetLatch(&MyProc->procLatch);
! 
! 			/*
! 			 * Ensure that no less than BgWriterDelay ms has passed between
! 			 * BgBufferSyncs - WaitLatch() might have returned instantaneously.
! 			 *
! 			 * Napping here rather than immediately prior to the WaitLatch()
! 			 * call ensures that a sudden burst of dirtied buffers won't result
! 			 * in the initial background writer cycle ineffectually cleaning
! 			 * only a few pages dirtied earlier in the burst, rather than
! 			 * cleaning a number that is somewhere closer to optimal for the
! 			 * cycle, as modeled by the background writing strategy. We'll also
! 			 * want to nap if we changed our minds, before starting a new cycle.
! 			 *
! 			 * We wake on a buffer being dirtied. It's possible that some
! 			 * useful work will become available for the bgwriter to do without
! 			 * a buffer actually being dirtied, as when a dirty buffer's usage
! 			 * count is decremented to zero or it's unpinned. This corner case
! 			 * is judged as too marginal to justify adding additional SetLatch()
! 			 * calls in very hot code paths, cheap though those calls may be.
! 			 */
! 			if (!(res & WL_TIMEOUT))
! 				BgWriterNap();
! 		}
  	}
  }
  
  /*
   * BgWriterNap -- Nap for the configured time or until a signal is received.
+  *
+  * Respond to a signal handler having set a flag for us.
   */
  static void
  BgWriterNap(void)
*************** BgWriterNap(void)
*** 285,290 ****
--- 344,373 ----
  	pgstat_send_bgwriter();
  
  	/*
+ 	 * Emergency bailout if postmaster has died.  This is to avoid the
+ 	 * necessity for manual cleanup of all postmaster children.
+ 	 */
+ 	if (!PostmasterIsAlive())
+ 		exit(1);
+ 
+ 	if (got_SIGHUP)
+ 	{
+ 		got_SIGHUP = false;
+ 		ProcessConfigFile(PGC_SIGHUP);
+ 		/* update global shmem state for sync rep */
+ 	}
+ 	if (shutdown_requested)
+ 	{
+ 		/*
+ 		 * From here on, elog(ERROR) should end with exit(1), not send
+ 		 * control back to the sigsetjmp block above
+ 		 */
+ 		ExitOnAnyError = true;
+ 		/* Normal exit from the bgwriter is here */
+ 		proc_exit(0);		/* done */
+ 	}
+ 
+ 	/*
  	 * Nap for the configured time, or sleep for 10 seconds if there is no
  	 * bgwriter activity configured.
  	 *
*************** BgWriterNap(void)
*** 293,302 ****
  	 * sleep into 1-second increments, and check for interrupts after each
  	 * nap.
  	 */
! 	if (bgwriter_lru_maxpages > 0)
! 		udelay = BgWriterDelay * 1000L;
! 	else
! 		udelay = 10000000L;		/* Ten seconds */
  
  	while (udelay > 999999L)
  	{
--- 376,382 ----
  	 * sleep into 1-second increments, and check for interrupts after each
  	 * nap.
  	 */
! 	udelay = BgWriterDelay * 1000L;
  
  	while (udelay > 999999L)
  	{
*************** bg_quickdie(SIGNAL_ARGS)
*** 351,362 ****
--- 431,454 ----
  static void
  BgSigHupHandler(SIGNAL_ARGS)
  {
+ 	int save_errno = errno;
+ 
  	got_SIGHUP = true;
+ 	if (MyProc)
+ 		SetLatch(&MyProc->procLatch);
+ 
+ 	errno = save_errno;
  }
  
  /* SIGTERM: set flag to shutdown and exit */
  static void
  ReqShutdownHandler(SIGNAL_ARGS)
  {
+ 	int save_errno = errno;
+ 
  	shutdown_requested = true;
+ 	if (MyProc)
+ 		SetLatch(&MyProc->procLatch);
+ 
+ 	errno = save_errno;
  }
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
new file mode 100644
index 91cc001..f38f76a
*** a/src/backend/storage/buffer/bufmgr.c
--- b/src/backend/storage/buffer/bufmgr.c
*************** MarkBufferDirty(Buffer buffer)
*** 968,973 ****
--- 968,976 ----
  	Assert(PrivateRefCount[buffer - 1] > 0);
  	/* unfortunately we can't check if the lock is held exclusively */
  	Assert(LWLockHeldByMe(bufHdr->content_lock));
+ 	/* The bgwriter may need to be woken. */
+ 	if (ProcGlobal->bgwriterLatch)
+ 		SetLatch(ProcGlobal->bgwriterLatch);
  
  	LockBufHdr(bufHdr);
  
*************** BufferSync(int flags)
*** 1307,1314 ****
   * BgBufferSync -- Write out some dirty buffers in the pool.
   *
   * This is called periodically by the background writer process.
   */
! void
  BgBufferSync(void)
  {
  	/* info obtained from freelist.c */
--- 1310,1321 ----
   * BgBufferSync -- Write out some dirty buffers in the pool.
   *
   * This is called periodically by the background writer process.
+  *
+  * Returns a value indicating if the clocksweep has been "lapped", or if the
+  * bgwriter has been effectively disabled due to finding bgwriter_lru_maxpages
+  * at 0.
   */
! bool
  BgBufferSync(void)
  {
  	/* info obtained from freelist.c */
*************** BgBufferSync(void)
*** 1365,1371 ****
  	if (bgwriter_lru_maxpages <= 0)
  	{
  		saved_info_valid = false;
! 		return;
  	}
  
  	/*
--- 1372,1378 ----
  	if (bgwriter_lru_maxpages <= 0)
  	{
  		saved_info_valid = false;
! 		return true;
  	}
  
  	/*
*************** BgBufferSync(void)
*** 1584,1589 ****
--- 1591,1598 ----
  			 recent_alloc, strategy_delta, scans_per_alloc, smoothed_density);
  #endif
  	}
+ 
+ 	return bufs_to_lap == 0;
  }
  
  /*
diff --git a/src/include/postmaster/bgwriter.h b/src/include/postmaster/bgwriter.h
new file mode 100644
index 6cc4b62..970faa2
*** a/src/include/postmaster/bgwriter.h
--- b/src/include/postmaster/bgwriter.h
***************
*** 13,18 ****
--- 13,19 ----
  #define _BGWRITER_H
  
  #include "storage/block.h"
+ #include "storage/latch.h"
  #include "storage/relfilenode.h"
  
  
diff --git a/src/include/storage/bufmgr.h b/src/include/storage/bufmgr.h
new file mode 100644
index a03c068..de1bbd0
*** a/src/include/storage/bufmgr.h
--- b/src/include/storage/bufmgr.h
*************** extern bool HoldingBufferPinThatDelaysRe
*** 213,219 ****
  extern void AbortBufferIO(void);
  
  extern void BufmgrCommit(void);
! extern void BgBufferSync(void);
  
  extern void AtProcExit_LocalBuffers(void);
  
--- 213,219 ----
  extern void AbortBufferIO(void);
  
  extern void BufmgrCommit(void);
! extern bool BgBufferSync(void);
  
  extern void AtProcExit_LocalBuffers(void);
  
diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
new file mode 100644
index 358d1a4..4520f27
*** a/src/include/storage/proc.h
--- b/src/include/storage/proc.h
*************** typedef struct PROC_HDR
*** 188,193 ****
--- 188,195 ----
  	PGPROC	   *freeProcs;
  	/* Head of list of autovacuum's free PGPROC structures */
  	PGPROC	   *autovacFreeProcs;
+ 	/* BGWriter process latch */
+ 	Latch *bgwriterLatch;
  	/* Current shared estimate of appropriate spins_per_delay value */
  	int			spins_per_delay;
  	/* The proc of the Startup process, since not in ProcArray */
-- 
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to