public inbox for [email protected]  
help / color / mirror / Atom feed
From: Andres Freund <[email protected]>
To: Peter Geoghegan <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Nonrandom scanned_pages distorts pg_class.reltuples set by VACUUM
Date: Thu, 17 Feb 2022 10:33:03 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAH2-Wz=uuc5W8aa9hWcttE2QMrkJT8MSEATeQ34HvJYjAdBY7g@mail.gmail.com>
References: <CAH2-WzkNKORurux459M64mR63Aw4Jq7MBRVcX=CvALqN3A88WA@mail.gmail.com>
	<CAH2-WzkNDbL9dKTwq+CAzkMk-XO=duYSZ-iLLjE=FpMujpahzQ@mail.gmail.com>
	<[email protected]>
	<CAH2-Wz=uuc5W8aa9hWcttE2QMrkJT8MSEATeQ34HvJYjAdBY7g@mail.gmail.com>

Hi,

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.


> > You didn't really give a heads up that you're about to commit either. There's
> > a note at the bottom of [1], a very long email, talking about committing in a
> > couple of weeks. After which there was substantial discussion of the patchset.
>
> How can you be surprised that I committed 44fa8488? It's essentially
> the same patch as the first version, posted November 22 -- almost 3
> months ago.

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

  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.


> And it's certainly not a big patch (though it is complicated).

For me a vacuum change with the following diffstat is large:
3 files changed, 515 insertions(+), 297 deletions(-)


> > It doesn't look to me like there was a lot of review for 44fa8488, despite it
> > touching very critical parts of the code. I'm listed as a reviewer, that was a
> > few months ago, and rereading my review I don't think it can be read to agree
> > with the state of the code back then.
>
> Can you be more specific?

Most importantly:

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.

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...


Greetings,

Andres Freund






view thread (10+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: Nonrandom scanned_pages distorts pg_class.reltuples set by VACUUM
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox