agora inbox for pgsql-hackers@postgresql.org
help / color / mirror / Atom feedFrom: Nathan Bossart <nathandbossart@gmail.com>
To: Greg Burd <greg@burd.me>
Cc: solai v <solai.cdac@gmail.com>
Cc: Nikita Malakhov <hukutoc@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: problems with toast.* reloptions
Date: Mon, 10 Aug 2026 11:30:17 -0500
Message-ID: <ann8mfCgOh_MYE0T@nathan> (raw)
In-Reply-To: <896e1dbc-5ca1-4d9d-9f85-ae5a2ccac4f4@app.fastmail.com>
References: <ahWsz2D9quWBgZ7s@nathan>
<CAF0whucUigX5c2_rAeDNKNXp-L6nJF4Td83opO1i2xBDNNPOvw@mail.gmail.com>
<CAN-LCVOCk0hcqcu3Fw1LSVYecmQD1J26YNGuc=sybN=ds=WpPg@mail.gmail.com>
<ah7jzDne6Ptu7fXs@nathan>
<CAN-LCVPtuuhUyM4QJ81Ma+yU1kyA9GwgX31pGM6ix9L2zWafqQ@mail.gmail.com>
<aihb_hMloqVDpbtw@nathan>
<CAF0whuf=D3AncYxfuwK05_TFY+4nK8-teTYtoiu7Zp-pzVAPDQ@mail.gmail.com>
<anT-1AlQKmXtcfrP@nathan>
<anYM78YFDXHTA6xE@nathan>
<896e1dbc-5ca1-4d9d-9f85-ae5a2ccac4f4@app.fastmail.com>
On Sun, Aug 09, 2026 at 09:37:49AM -0400, Greg Burd wrote:
> Am I misunderstanding this? It seems to me that making autovacuum_enabled
> a ternary and then merging it means a heap table with
> autovacuum_enabled=false and some toast.* option set now stops
> autovacuuming the TOAST table.
>
> [...]
>
> So main enabled=PG_TERNARY_FALSE + toast unset -> toast enabled becomes
> PG_TERNARY_FALSE -> av_enabled is false. Today's all-or-nothing bug
> leaves that TOAST table getting vacuumed. I agree the new behavior
> matches the documented contract, but it is a behavior change for the
> person who disabled autovac on a table they vacuum by hand and never
> thought about the TOAST side. Wraparound is still forced, but ordinary
> dead-tuple bloat on the TOAST relation is now on them. So, maybe a line
> in the commit message and in the CREATE TABLE docs to make that more
> explicit would help people avoid making that mistake in practice?
Eh... I don't see much reason to worry about making relopts work how
they're documented. I mean, that's the whole point of this patch. You
could make roughly the same argument about every other reloption with a
corresponding TOAST setting. From asking around, I get the idea that
setting toast.* relopts is pretty rare, anyway.
Perhaps there's an argument for improving the docs to make this behavior a
little more apparent, but I think we can take care of that separately.
> In merge_autovac_opts() the four offset arrays keyed by "which sentinel
> means unset", is that duplicating knowledge that already lives in the
> relopt tables in reloptions.c?
>
> [...]
>
> Add an AutoVacOpts field, or change a field's default sentinel, and
> forget to update the matching array here, and the merge silently keeps
> the TOAST table's default instead of inheriting, nothing fails to compile
> and no test goes red. Can this be driven off the relopt metadata
> (relopt_parse_elt already knows each option's type and default) instead
> of the hand-maintained offset arrays?
I'm looking into this. Since this is almost certainly a master-only change
at this point, it seems reasonable to spend some more time on making this
stuff less fragile.
> On testing: the coverage doesn't touch the risky code. There's one
> injection-point case, and it's manual VACUUM only, index_cleanup/truncate
> only:
>
> +-- TOAST table inherits main table's resolved values
> +CREATE TABLE vac_tab_toast_inherit(i int, j text) WITH
> + (autovacuum_enabled=false,
> + vacuum_index_cleanup=false,
> + vacuum_truncate=false, toast.vacuum_truncate=true);
> +VACUUM vac_tab_toast_inherit;
> +DROP TABLE vac_tab_toast_inherit;
>
> Nothing exercises the autovacuum decision path, autovacuum_enabled
> inheritance, or any of the numeric AutoVacOpts that merge_autovac_opts()
> actually resolves which is precisely the code I'm worried about above.
> FWIW the pg_stat_get_autovacuum_scores() SRF that 0005 extends looks like
> it could drive a deterministic test of the autovac path (compute the
> decision without spawning a worker), which sidesteps the flakiness worry
> raised upthread.
Will add some more coverage.
> In summary, solid work and I hope it lands. Just a few small issues to
> clean up.
Thanks for reviewing.
--
nathan
view thread (44+ messages) latest in thread
Message-ID: <ann8mfCgOh_MYE0T@nathan>
Permalink: ../ann8mfCgOh_MYE0T@nathan/
Also on: postgresql.org/message-id/ann8mfCgOh_MYE0T@nathan
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nathandbossart@gmail.com, greg@burd.me, solai.cdac@gmail.com, hukutoc@gmail.com, michael@paquier.xyz
Subject: Re: problems with toast.* reloptions
In-Reply-To: <ann8mfCgOh_MYE0T@nathan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox