Subject: Re: Nonrandom scanned_pages distorts pg_class.reltuples set by VACUUM

From: Peter Geoghegan <[email-masked]>
Date: Thu, 17 Feb 2022 14:23:51 -0800

On Thu, Feb 17, 2022 at 10:33 AM Andres Freund <[email-masked]> wrote:
> On 2022-02-16 19:43:09 -0800, Peter Geoghegan wrote:
> > It makes the code in vacuumlazy.c much cleaner. In fact, that's how commit
> > 44fa8488 started off -- purely as refactoring work.
>
> The problem is that it didn't end up as that. You combined refactoring with
> substantial changes. And described it large and generic terms.

What substantial changes are you referring to? The one thing that did
change was the commit message, which framed everything in terms of the
later work. It really is true that the patch that I committed was
essentially the same patch as the one posted on November 22, in both
style and substance. Before I really even began to think about the
freezing stuff. This is a matter of record.

> It's a contentious thread. I asked for things to be split. In that context,
> it's imo common curtesy for non-trivial patches to write something like

I didn't see a way to usefully split up 0001 any further (having
already split it up into 0001 and 0002). That's not particularly
obvious, but it's true.

>   While the rest of the patchset is contentious, I think 0001 is ready to
>   go. I'd like to commit it tomorrow, unless somebody protests.

Okay. I'll be more explicit about it in this way in the future.

> On 2021-11-22 11:29:56 -0800, Andres Freund wrote:
> > I think this is a change mostly in the right direction. But as formulated this
> > commit does *WAY* too much at once.
>
> I do not know how to state more clearly that I think this is not a patch in a
> committable shape.

As I said, I dispute the idea that it could have been split up even
further. My reasons are complicated, and I can get into it if you'd
like.

> but also:
>
> On 2021-11-22 11:29:56 -0800, Andres Freund wrote:
> > I think it should be doable to add an isolation test for this path. There have
> > been quite a few bugs around the wider topic...

Yeah, I really should have included that in 0001 -- I accept that.
That didn't happen in the initial commit, but was high on my todo
list. I can take care of it in the next few days.

--
Peter Geoghegan

---

From: Andres Freund <[email-masked]>
Date: Thu, 17 Feb 2022 16:18:22 -0800

Hi,

> freezing stuff. This is a matter of record.

Here I was referencing your description of how the patch started ("purely as
refactoring work"), and then evolved into something not just a refactoring.

> already split it up into 0001 and 0002). That's not particularly

If helpful I can give a go at showing how I think it could be split up. Or
perhaps more productively, do that on a not-yet-committed larger patch.

Greetings,

Andres Freund

---

From: Peter Geoghegan <[email-masked]>
Date: Thu, 17 Feb 2022 16:20:38 -0800

> refactoring work"), and then evolved into something not just a refactoring.

Of course.

> perhaps more productively, do that on a not-yet-committed larger patch.

Any help is appreciated.

-- 
Peter Geoghegan