Обсуждение: Re: problems with toast.* reloptions
> I think we need to do something like the following to fix this: > > * Teach autovacuum to combine the TOAST reloptions with the main relation's > when processing TOAST tables (with the toast.* ones winning if both are > set). > > * Teach autovacuum to resolve reloptions for parameters like > vacuum_truncate instead of relying on vacuum_rel() to fill it in. >> These two points make sense here, yes. I investigated that this afternoon and identified two potential implementation approaches: 1) Create functions like resolve_toast_vac_opts() and resolve_toast_rel_opts(). These would then be used in table_recheck_autovac(), NeedsAutoVacTableForXidWraparound(), and do_autovacuum() after the toast table check. 2) When updating a table's relopt, also update the relopt of its associated TOAST table if it's not already set. Similarly, when creating a new TOAST table, it would inherit the parent's relopt. Option 2 seems more reasonable to me, as it avoids requiring customers to manually resolve these options, when they have different settings for the parent and TOAST tables."
On Sat, Jun 21, 2025 at 11:45:25PM -0400, shihao zhong wrote: > 2) When updating a table's relopt, also update the relopt of its > associated TOAST table if it's not already set. Similarly, when > creating a new TOAST table, it would inherit the parent's relopt. > > Option 2 seems more reasonable to me, as it avoids requiring customers > to manually resolve these options, when they have different settings > for the parent and TOAST tables." I like this one, but since it won't fix existing clusters, it might only be workable for v19. -- nathan
On Mon, Jun 23, 2025 at 10:59:51AM -0500, Nathan Bossart wrote: > On Sat, Jun 21, 2025 at 11:45:25PM -0400, shihao zhong wrote: >> 2) When updating a table's relopt, also update the relopt of its >> associated TOAST table if it's not already set. Similarly, when >> creating a new TOAST table, it would inherit the parent's relopt. >> >> Option 2 seems more reasonable to me, as it avoids requiring customers >> to manually resolve these options, when they have different settings >> for the parent and TOAST tables." > > I like this one, but since it won't fix existing clusters, it might only be > workable for v19. Actually, I think there's a problem with this approach. If we set the reloption for both the main relation and the TOAST table, then we won't know what to do for RESET. Take the following examples: ALTER TABLE test SET (vacuum_truncate = false); ALTER TABLE test RESET (vacuum_truncate); ALTER TABLE test SET (vacuum_truncate = false); ALTER TABLE test SET (toast.vacuum_truncate = false); ALTER TABLE test RESET (vacuum_truncate); After executing the commands in the first stanza, you'd expect the vacuum_truncate reloption to be unset for both the main relation and its TOAST table. After the second one, you'd expect it to be set for only the TOAST table. But unless there's some way to know the source of the TOAST table's reloption, we can't know which behavior is correct at RESET time. -- nathan
>> Actually, I think there's a problem with this approach... You're right. I forgot we can reset the table options. While we could use a placeholder and resolve it on-demand, that seems like too much work.
On Wed, Jul 2, 2025 at 10:44 AM shihao zhong <zhong950419@gmail.com> wrote:
>> Actually, I think there's a problem with this approach...
You're right. I forgot we can reset the table options. While we could
use a placeholder and resolve it on-demand, that seems like too much
work.
I started a conversation about TOAST table vacuum truncation parameter inheritance [1] and was pointed to this thread which I totally missed. I recently came across the issue where `vacuum_truncate` on TOAST tables wasn't being inherited even though the parent table had the setting, and the documentation mentioned that it should work [2]. I see that there are a lot of good conversations here already.
I'm curious to hear what folks think about the approach where we implement the inheritance logic directly in `vacuum_rel()` when `params.truncate == VACOPTVALUE_UNSPECIFIED`. This should address the point raised upthread about `vacuum_rel()` not looking up the main relation's reloptions when processing a TOAST and also what I discovered in [1]. For instance it could be something like:
1. For TOAST tables: When no explicit `toast.vacuum_truncate` is set, scan `pg_class.reltoastrelid` to find the parent table and inherit its `vacuum_truncate` setting.
2. For manual VACUUM: Pass the final truncate decision from main table to TOAST table in the `toast_vacuum_params`
3. For autovacuum: The inheritance happens naturally since autovacuum calls `vacuum_rel()` for each relation independently
This basically teaches `vacuum_rel()` to consult the main relation's reloptions for TOAST tables, which was the core issue identified upthread. It handles both manual VACUUM and autovacuum scenarios in one place, without changing function signatures (which helps with potential backporting).
The execution-layer fix should also help us avoid the parameter contamination issues that were fixed in commit 661643dedad9 (I believe?), since we're doing the lookup fresh each time `vacuum_rel()` is called on a TOAST table.
I realize backporting might not be preferred given this issue has existed for a long time. However, that is precisely why I feel like it might be worth patching it because, at least per the docs, my understanding was that if `vacuum_truncate` is set on the parent table then it applies to the TOAST as well and I wonder if there are others in the same boat as well.
Would something like this focused approach (just teaching `vacuum_rel()` to look up parent reloptions for TOAST tables) could be a reasonable fix that complements the broader reloptions work discussed upthread for v19 and also backportable to v13 onwards?
I am happy to help with this and split out the changes as it makes sense as well.
Thanks
Shayon
[1] https://www.postgresql.org/message-id/CANqtF-pLHBDdNM-DifXQqVq%2BVWA3DTYOx4%2B3m2%2BTFJ1zDOZETg%40mail.gmail.com