Re: Parallel heap vacuum

On Sat, Apr 5, 2025 at 1:32 PM Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> Hi,
>
> On 2025-04-04 14:34:53 -0700, Masahiko Sawada wrote:
> > On Fri, Apr 4, 2025 at 11:05 AM Melanie Plageman
> > <melanieplageman(at)gmail(dot)com> wrote:
> > >
> > > On Tue, Apr 1, 2025 at 5:30 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
> > > >
> > > >
> > > > I've attached the new version patch. There are no major changes; I
> > > > fixed some typos, improved the comment, and removed duplicated codes.
> > > > Also, I've updated the commit messages.
> > >
> > > I haven't looked closely at this version but I did notice that you do
> > > not document that parallel vacuum disables eager scanning. Imagine you
> > > are a user who has set the eager freeze related table storage option
> > > (vacuum_max_eager_freeze_failure_rate) and you schedule a regular
> > > parallel vacuum. Now that table storage option does nothing.
> >
> > Good point. That restriction should be mentioned in the documentation.
> > I'll update the patch.
>
> I don't think we commonly accept that a new feature B regresses a pre-existing
> feature A, particularly not if feature B is enabled by default. Why would that
> be OK here?

The eager freeze scan is the pre-existing feature but it's pretty new
code that was pushed just a couple months ago. I didn't want to make
the newly introduced code complex further in one major release
especially if it's in a vacuum area. While I agree that disabling
eager freeze scans during parallel heap vacuum is not very
user-friendly, there are still many cases where parallel heap vacuum
helps even without the eager freeze scan. FYI the parallel heap scan
can be disabled by setting min_parallel_table_scan_size. So I thought
we can incrementally improve this part.

>
>
> The justification in the code:
> + * One might think that it would make sense to use the eager scanning even
> + * during parallel lazy vacuum, but parallel vacuum is available only in
> + * VACUUM command and would not be something that happens frequently,
> + * which seems not fit to the purpose of the eager scanning. Also, it
> + * would require making the code complex. So it would make sense to
> + * disable it for now.
>
> feels not at all convincing to me. There e.g. are lots of places that run
> nightly vacuums. I don't think it's ok to just disable eager scanning in such
> a case, as it would mean that the "freeze cliff" would end up being *higher*
> because of the nightly vacuums than if just plain autovacuum would have been
> used.

That's a fair argument.

> I think it was already a mistake to allow the existing vacuum parallelism to
> be introduced without integrating it with autovacuum. I don't think we should
> go further down that road.

Okay, I think we can consider how to proceed with this patch including
the above point in the v19 development.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com