| From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
|---|---|
| To: | Greg Burd <greg(at)burd(dot)me> |
| Cc: | solai v <solai(dot)cdac(at)gmail(dot)com>, Nikita Malakhov <hukutoc(at)gmail(dot)com>, Michael Paquier <michael(at)paquier(dot)xyz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: problems with toast.* reloptions |
| Date: | 2026-08-11 21:43:19 |
| Message-ID: | anuXd35G8WLTUaBR@nathan |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, Aug 10, 2026 at 11:30:17AM -0500, Nathan Bossart wrote:
> On Sun, Aug 09, 2026 at 09:37:49AM -0400, Greg Burd wrote:
>> 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.
Here is a new patch set with an attempt at the above. The new 0005
contains the log_autovacuum_min_duration default change along with an
assertion that all TOAST storage parameters have unsettable defaults. 0006
is just some prerequisite refactoring for 0007. And 0007 is the fix with
the merge done based on the main reloption list instead of new arrays.
>> 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.
Done.
--
nathan
| Attachment | Content-Type | Size |
|---|---|---|
| v8-0001-Remove-extract_autovac_opts.patch | text/plain | 11.5 KB |
| v8-0002-Make-autovacuum_enabled-a-ternary-reloption.patch | text/plain | 3.6 KB |
| v8-0003-Add-an-unset-value-for-vacuum_index_cleanup.patch | text/plain | 3.5 KB |
| v8-0004-Simplify-autovacuum-s-TOAST-to-main-relation-relo.patch | text/plain | 3.8 KB |
| v8-0005-Give-TOAST-storage-parameters-unsettable-defaults.patch | text/plain | 5.0 KB |
| v8-0006-Move-the-StdRdOptions-parse-table-to-file-scope.patch | text/plain | 7.8 KB |
| v8-0007-Fix-VACUUM-and-autovacuum-handling-of-TOAST-stora.patch | text/plain | 22.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Nathan Bossart | 2026-08-11 21:45:49 | Re: problems with toast.* reloptions |
| Previous Message | Alexander Korotkov | 2026-08-11 21:36:34 | Re: MERGE/SPLIT PARTITIONS issues/questions |