Horton

Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

Justin Pryzby <pryzby@telsasoft.com>

View thread View on postgr.es
From: Justin Pryzby <pryzby@telsasoft.com>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>, Peter Geoghegan <pg@bowt.ie>, pgsql-hackers@postgresql.org, Robert Haas <robertmhaas@gmail.com>, David Rowley <dgrowleyml@gmail.com>
Date: 2023-03-11T19:16:19Z
Lists: pgsql-hackers 
On Sat, Mar 11, 2023 at 09:55:33AM -0500, Melanie Plageman wrote:
> Subject: [PATCH v3 2/3] use shared buffers when failsafe active
> 
> +		/*
> +		 * Assume the caller who allocated the memory for the
> +		 * BufferAccessStrategy object will free it.
> +		 */
> +		vacrel->bstrategy = NULL;

This comment could use elaboration:

** VACUUM normally restricts itself to a small ring buffer; but in
failsafe mode, in order to process tables as quickly as possible, allow
the leaving behind large number of dirty buffers.

> Subject: [PATCH v3 3/3] add vacuum option to specify ring_size and guc

>  #define INT_ACCESS_ONCE(var)	((int)(*((volatile int *)&(var))))
> +#define bufsize_limit_to_nbuffers(bufsize) (bufsize * 1024 / BLCKSZ)

Macros are normally be capitalized

It's a good idea to write "(bufsize)", in case someone passes "a+b".

> @@ -586,6 +587,45 @@ GetAccessStrategy(BufferAccessStrategyType btype)
> +BufferAccessStrategy
> +GetAccessStrategyWithSize(BufferAccessStrategyType btype, int ring_size)

Maybe it would make sense for GetAccessStrategy() to call
GetAccessStrategyWithSize().  Or maybe not.

> +		{"vacuum_buffer_usage_limit", PGC_USERSET, RESOURCES_MEM,
> +			gettext_noop("Sets the buffer pool size for operations employing a buffer access strategy."),

The description should mention vacuum, if that's the scope of the GUC's
behavior.

> +#vacuum_buffer_usage_limit = -1 # size of vacuum buffer access strategy ring.
> +				# -1 to use default,
> +				# 0 to disable vacuum buffer access strategy and use shared buffers
> +				# > 0 to specify size

If I'm not wrong, there's still no documentation about "ring buffers" or
postgres' "strategy".  Which seems important to do for this patch, along
with other documentation.

This patch should add support in vacuumdb.c.  And maybe a comment about
adding support there, since it's annoying when it the release notes one
year say "support VACUUM (FOO)" and then one year later say "support
vacuumdb --foo".

-- 
Justin

Commits

  1. Add unit to vacuum_buffer_usage_limit value in postgresql.conf.sample.

  2. Improve VACUUM/ANALYZE BUFFER_USAGE_LIMIT docs

  3. Add --buffer-usage-limit option to vacuumdb

  4. Add VACUUM/ANALYZE BUFFER_USAGE_LIMIT option

  5. Move various prechecks from vacuum() into ExecVacuum()

  6. Always make a BufferAccessStrategy for ANALYZE

  7. Rename BufferAccessStrategyData.ring_size to nbuffers

  8. Disable vacuum's use of a buffer access strategy during failsafe

  9. Only make buffer strategy for vacuum when it's likely needed

  10. Remove some global variables from vacuum.c

  11. Doc: add Buffer Access Strategy to the glossary