public inbox for [email protected]help / color / mirror / Atom feed
pgsql: Add vacuum_truncate configuration parameter. 6+ messages / 4 participants [nested] [flat]
* pgsql: Add vacuum_truncate configuration parameter. @ 2025-03-20 15:17 Nathan Bossart <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Nathan Bossart @ 2025-03-20 15:17 UTC (permalink / raw) To: [email protected] Add vacuum_truncate configuration parameter. This new parameter works just like the storage parameter of the same name: if set to true (which is the default), autovacuum and VACUUM attempt to truncate any empty pages at the end of the table. It is primarily intended to help users avoid locking issues on hot standbys. The setting can be overridden with the storage parameter or VACUUM's TRUNCATE option. Since there's presently no way to determine whether a Boolean storage parameter is explicitly set or has just picked up the default value, this commit also introduces an isset_offset member to relopt_parse_elt. Suggested-by: Will Storey <[email protected]> Author: Nathan Bossart <[email protected]> Co-authored-by: Gurjeet Singh <[email protected]> Reviewed-by: Laurenz Albe <[email protected]> Reviewed-by: Fujii Masao <[email protected]> Reviewed-by: Robert Treat <[email protected]> Discussion: https://postgr.es/m/Z2DE4lDX4tHqNGZt%40dev.null Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/0164a0f9ee12e0eff9e4c661358a272ecd65c2d4 Modified Files -------------- doc/src/sgml/config.sgml | 29 +++++++++++++++++++++++++++ doc/src/sgml/ref/create_table.sgml | 13 ++++-------- doc/src/sgml/ref/vacuum.sgml | 3 ++- src/backend/access/common/reloptions.c | 14 ++++++++++++- src/backend/commands/vacuum.c | 17 ++++++++++++---- src/backend/utils/misc/guc_tables.c | 10 +++++++++ src/backend/utils/misc/postgresql.conf.sample | 4 ++++ src/include/access/reloptions.h | 1 + src/include/commands/vacuum.h | 1 + src/include/utils/guc_tables.h | 1 + src/include/utils/rel.h | 1 + src/test/regress/expected/vacuum.out | 27 +++++++++++++++++++++++++ src/test/regress/sql/vacuum.sql | 10 +++++++++ 13 files changed, 116 insertions(+), 15 deletions(-) ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: pgsql: Add vacuum_truncate configuration parameter. @ 2025-03-20 15:30 Tom Lane <[email protected]> parent: Nathan Bossart <[email protected]> 0 siblings, 2 replies; 6+ messages in thread From: Tom Lane @ 2025-03-20 15:30 UTC (permalink / raw) To: Nathan Bossart <[email protected]>; +Cc: [email protected] Nathan Bossart <[email protected]> writes: > Since there's presently no way to determine whether a Boolean > storage parameter is explicitly set or has just picked up the > default value, this commit also introduces an isset_offset member > to relopt_parse_elt. Uh, what? Why is it a good idea to distinguish those states? Seems like that risks some very surprising behavior, ie if the default is "true", why shouldn't that act exactly like an explicit setting of "true"? regards, tom lane ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: pgsql: Add vacuum_truncate configuration parameter. @ 2025-03-20 15:39 David G. Johnston <[email protected]> parent: Tom Lane <[email protected]> 1 sibling, 0 replies; 6+ messages in thread From: David G. Johnston @ 2025-03-20 15:39 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Nathan Bossart <[email protected]>; [email protected] <[email protected]> On Thursday, March 20, 2025, Tom Lane <[email protected]> wrote: > Nathan Bossart <[email protected]> writes: > > Since there's presently no way to determine whether a Boolean > > storage parameter is explicitly set or has just picked up the > > default value, this commit also introduces an isset_offset member > > to relopt_parse_elt. > > Uh, what? Why is it a good idea to distinguish those states? > Seems like that risks some very surprising behavior, ie if the > default is "true", why shouldn't that act exactly like an > explicit setting of "true"? > In order to implement what amounts to coalesce(…) for settings (use global value unless the table value overrides) one needs a sentinel value that means unset because settings cannot take on the null value. There is no such possible sentinel value for boolean so another field is required. The hazards of choosing Boolean instead of text for settings. David J. ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: pgsql: Add vacuum_truncate configuration parameter. @ 2025-03-20 23:07 David Rowley <[email protected]> parent: Tom Lane <[email protected]> 1 sibling, 1 reply; 6+ messages in thread From: David Rowley @ 2025-03-20 23:07 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Nathan Bossart <[email protected]>; [email protected] On Fri, 21 Mar 2025 at 04:30, Tom Lane <[email protected]> wrote: > > Nathan Bossart <[email protected]> writes: > > Since there's presently no way to determine whether a Boolean > > storage parameter is explicitly set or has just picked up the > > default value, this commit also introduces an isset_offset member > > to relopt_parse_elt. > > Uh, what? Why is it a good idea to distinguish those states? > Seems like that risks some very surprising behavior, ie if the > default is "true", why shouldn't that act exactly like an > explicit setting of "true"? I was thinking about this yesterday as the topic of a user-configurable options for truncation threshold came up in [1]. I find it a bit annoying the boolean vacuum_truncate reloption was added (a few years ago) as we could have instead added a more flexible truncate_scale_factor that could be set to -1 to disable truncation. Maybe it's too late now as reloptions are not easy to remove. Adding this GUC now does put us a bit further down the path of the boolean option. ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: pgsql: Add vacuum_truncate configuration parameter. @ 2025-03-21 16:39 David G. Johnston <[email protected]> parent: David Rowley <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: David G. Johnston @ 2025-03-21 16:39 UTC (permalink / raw) To: David Rowley <[email protected]>; +Cc: Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; [email protected] On Thu, Mar 20, 2025 at 4:07 PM David Rowley <[email protected]> wrote: > On Fri, 21 Mar 2025 at 04:30, Tom Lane <[email protected]> wrote: > > > > Nathan Bossart <[email protected]> writes: > > > Since there's presently no way to determine whether a Boolean > > > storage parameter is explicitly set or has just picked up the > > > default value, this commit also introduces an isset_offset member > > > to relopt_parse_elt. > > > > Uh, what? Why is it a good idea to distinguish those states? > > Seems like that risks some very surprising behavior, ie if the > > default is "true", why shouldn't that act exactly like an > > explicit setting of "true"? > > I was thinking about this yesterday as the topic of a > user-configurable options for truncation threshold came up in [1]. I > find it a bit annoying the boolean vacuum_truncate reloption was added > (a few years ago) as we could have instead added a more flexible > truncate_scale_factor that could be set to -1 to disable truncation. > Maybe it's too late now as reloptions are not easy to remove. Adding > this GUC now does put us a bit further down the path of the boolean > option. Not seeing much point in trying to get rid of the on/off switch. It just won't make sense to choose a tunable value of zero to disable something, and probably should be prohibited. I'm seeing an implementation detail discussion here, not a behavior one. The field complaint that we don't let the DBA control this at the GUC level is valid and reasonably solved. The "default" behavior hasn't changed but now instead of hard-coding the default we moved it to a GUC. The storage parameter is no longer documented as having a default, which is correct. It now behaves like most of the other storage parameters in that if unset a GUC provides the value. David J. ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: pgsql: Add vacuum_truncate configuration parameter. @ 2025-03-23 22:22 David Rowley <[email protected]> parent: David G. Johnston <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: David Rowley @ 2025-03-23 22:22 UTC (permalink / raw) To: David G. Johnston <[email protected]>; +Cc: Tom Lane <[email protected]>; Nathan Bossart <[email protected]>; [email protected] On Sat, 22 Mar 2025 at 05:40, David G. Johnston <[email protected]> wrote: > Not seeing much point in trying to get rid of the on/off switch. It just won't make sense to choose a tunable value of zero to disable something, and probably should be prohibited. Can you explain what does not make sense about it? We have plenty of GUCs and reloptions where -1 means "inherit the setting from somewhere else". Do those also not make sense, or is this one somehow different? > I'm seeing an implementation detail discussion here, not a behavior one. The field complaint that we don't let the DBA control this at the GUC level is valid and reasonably solved. The "default" behavior hasn't changed but now instead of hard-coding the default we moved it to a GUC. The storage parameter is no longer documented as having a default, which is correct. It now behaves like most of the other storage parameters in that if unset a GUC provides the value. The reason I was pointing this out is that I wanted to ensure that this was considered before we release code with the new GUC. It's true removing a GUC isn't as hard as removing a reloption and we already have the reloption. If everyone thinks we'll likely not need user-tunable options to specify the number of pages required before truncation occurs then that's fine. The problem I see is that we already have lots of GUCs and it'd be nice not to have semi-redundant ones in the future because we've failed to consider something. I was just pointing out the "something" part that we might want to consider. David ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2025-03-23 22:22 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2025-03-20 15:17 pgsql: Add vacuum_truncate configuration parameter. Nathan Bossart <[email protected]> 2025-03-20 15:30 ` Tom Lane <[email protected]> 2025-03-20 15:39 ` David G. Johnston <[email protected]> 2025-03-20 23:07 ` David Rowley <[email protected]> 2025-03-21 16:39 ` David G. Johnston <[email protected]> 2025-03-23 22:22 ` David Rowley <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox