Hi,
On 2025-04-04 14:34:53 -0700, Masahiko Sawada wrote:
> On Fri, Apr 4, 2025 at 11:05 AM Melanie Plageman
> <melanieplageman@gmail.com> wrote:
> >
> > On Tue, Apr 1, 2025 at 5:30 PM Masahiko Sawada <sawada.mshk@gmail.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 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.
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.
Greetings,
Andres Freund
