Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
Justin Pryzby <pryzby@telsasoft.com>
From: Justin Pryzby <pryzby@telsasoft.com>
To: Melanie Plageman <melanieplageman@gmail.com>
Cc: David Rowley <dgrowleyml@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, 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>
Date: 2023-04-01T17:57:23Z
Lists: pgsql-hackers
On Sat, Apr 01, 2023 at 01:29:13PM -0400, Melanie Plageman wrote: > Hi, > > I was just doing some cleanup on the main patch in this set and realized > that it was missing a few things. One of which is forbidding the > BUFFER_USAGE_LIMIT with VACUUM FULL since VACUUM FULL does not use a > BAS_VACUUM strategy. > > VACUUM FULL technically uses a bulkread buffer access strategy for > reading the original relation if its number of blocks is > number of > shared buffers / 4 (see initscan()). The new rel writing is done using > smgrextend/write directly and doesn't go through shared buffers. I > think it is a stretch to try and use the size passed in to VACUUM by > BUFFER_USAGE_LIMIT for the bulkread strategy ring. When you say that it's a stretch, do you mean that it'd be a pain to add arguments to handful of functions to pass down the setting ? Or that it's unclear if doing so would be the desirable/needed/intended/expected behavior ? I think if VACUUM FULL were going to allow a configurable strategy size, then so should CLUSTER. But it seems fine if they don't. I wonder if maybe strategy should be configurable in some more generic way, like a GUC. At one point I had a patch to allow INSERT to use strategy buffers (not just INSERT SELECT). And that's still pretty desirable. Also COPY. I've seen load spikes caused by pg_dumping tables which are just below 25% of shared_buffers. Which is exacerbated because pg_dump deliberately orders tables by size, so those tables are dumped one after another, each causing eviction of ~20% of shared buffers. And exacerbated some more because TOAST don't seem to use a ring buffer in that case. > I somehow feel like VACUUM (FULL, BUFFER_USAGE_LIMIT 'x') should error > out instead of silently not using the buffer usage limit, though. > > I am looking for others' opinions. Sorry, no opinion here :) One thing is that it's fine to take something that previously throw an error and change it to not throw an error anymore. But it's undesirable to do the opposite. For that reason, there's may be a tendency to add errors for cases like this. -- Justin
Commits
-
Add unit to vacuum_buffer_usage_limit value in postgresql.conf.sample.
- b72f564d87d1 16.0 landed
-
Improve VACUUM/ANALYZE BUFFER_USAGE_LIMIT docs
- c0235013c13b 16.0 landed
-
Add --buffer-usage-limit option to vacuumdb
- ae78cae3be62 16.0 landed
-
Add VACUUM/ANALYZE BUFFER_USAGE_LIMIT option
- 1cbbee033857 16.0 landed
-
Move various prechecks from vacuum() into ExecVacuum()
- b9b125b9c143 16.0 landed
-
Always make a BufferAccessStrategy for ANALYZE
- bccd6908ca82 16.0 landed
-
Rename BufferAccessStrategyData.ring_size to nbuffers
- 8d928e3a9fe7 16.0 landed
-
Disable vacuum's use of a buffer access strategy during failsafe
- 4830f1024325 16.0 landed
-
Only make buffer strategy for vacuum when it's likely needed
- 32fbe0239b03 16.0 landed
-
Remove some global variables from vacuum.c
- 3f476c953495 16.0 landed
-
Doc: add Buffer Access Strategy to the glossary
- c8f8d0eb1868 16.0 landed