On Mon, Jun 23, 2025 at 10:38:40AM -0500, Nathan Bossart wrote:
> Is the idea to do something like this for v19 and this [0] for the
> back-branches?
Yes, that would be the idea. Let's do the bug fix first on all the
branches, then do the refactoring. Knowing that I'm indirectly
responsible for this mess, I would like to take care of that myself.
Would that be OK for you?
> I think the recurse-to-TOAST-table case is still broken with your patch.
> We should probably move the memcpy() for toast_vacuum_params to the very
> top of vacuum_rel(). Otherwise, your patch looks generally reasonable to
> me.
Ah, right, I have managed to mess up this part. Sounds to me that we
should provide more coverage for both the truncate and index cleanup
cases, as well.
The updated version attached includes tests that were enough to make
the recursive TOAST table case fail for TRUNCATE and INDEX_CLEANUP,
where I have relied on an EXTERNAL column to fill in the TOAST
relation with data cheaply. The pgstattuple case is less cheap but
I've minimized the load to be able to trigger the index cleanup. It
was tricky to get the threshold right for INDEX_CLEANUP in the case of
TOAST table, but that's small enough to have a short run time while it
should be large enough to trigger a cleanup of the TOAST indexes (my
CI quota has been reached this month, so I can only rely on the CF bot
for some pre-validation job). If the thresholds are too low, we may
need to bump the number of tuples. I'd be OK to remove the TOAST
parts if these prove to be unstable, but let's see. At least these
tests are validating the contents of the patch for the TOAST options,
and they were looking stable on my machine.
Another approach that we could use is an injection point with some
LOGs that show some information based on what VacuumParams holds.
That would be the cheapest method (no need for any data), and entirely
stable as we would look at the stack. Perhaps going down to that is
not really necessary for the sake of this thread.
Attached are two patches to have a clean git history:
- 0001 is the basic fix, to-be-applied first on all the branches.
- 0002 is the refactoring, for v19 once it opens up.
What do you think?
--
Michael
