Date: Mon, 15 Jul 2024 18:52:26 -0700
From: Noah Misch <noah@leadboat.com>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Melanie Plageman <melanieplageman@gmail.com>,
	vignesh C <vignesh21@gmail.com>, Andres Freund <andres@anarazel.de>,
	Pg Hackers <pgsql-hackers@postgresql.org>,
	Heikki Linnakangas <hlinnaka@iki.fi>,
	Nazir Bilal Yavuz <byavuz81@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>,
	"Andrey M. Borodin" <x4mmm@yandex-team.ru>
Subject: Re: Confine vacuum skip logic to lazy_scan_skip
Message-ID: <20240716015226.60.nmisch@google.com>
References: 
 <CAAKRu_ZuYsf4MNGzm7_KvG0SKsu-EtYRhFHy2x3q0heX8KLMTg@mail.gmail.com>
 <CAAKRu_bQgf4e9OveCQX++zTEJLKbZbqLeapA7SNkNuMjxcCO1w@mail.gmail.com>
 <20240306234733.nd4a636colxkgq2e@liskov>
 <CAAKRu_aiaf+CufVA5Rrksz4Lw2fvZ3M-OgdBZ8WpzvYoQY96MQ@mail.gmail.com>
 <CA+hUKGKHb3i8Wy72VCKZGA2B5djoj7tAzYAzbeS=Gwr_SdhgRw@mail.gmail.com>
 <CAAKRu_byDppRvNJ+p5kq-SmXZDQuL6L8N6D06pHbO6WnBnanWQ@mail.gmail.com>
 <CA+hUKGLY4Q4ZY4f1rvnFtv6+PkjNf8MejdPkcju3Qii9DYqqcQ@mail.gmail.com>
 <CAAKRu_bbkmwAzSBgnezancgJeXrQZXy4G4kBTd+5=cr86H5yew@mail.gmail.com>
 <20240707144944.58.nmisch@google.com>
 <CA+hUKGK3sB-T+Ao8EBCOu5M1MsCMPnPpNyXuQGkQYHU4WC35eA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CA+hUKGK3sB-T+Ao8EBCOu5M1MsCMPnPpNyXuQGkQYHU4WC35eA@mail.gmail.com>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20240716015226.60.nmisch%40google.com>
Precedence: bulk

On Mon, Jul 15, 2024 at 03:26:32PM +1200, Thomas Munro wrote:
> On Mon, Jul 8, 2024 at 2:49 AM Noah Misch <noah@leadboat.com> wrote:
> > what is the scope of the review you seek?
> 
> The patch "Refactor tidstore.c memory management." could definitely
> use some review.

That's reasonable.  radixtree already forbids mutations concurrent with
iteration, so there's no new concurrency hazard.  One alternative is
per_buffer_data big enough for MaxOffsetNumber, but that might thrash caches
measurably.  That patch is good to go apart from these trivialities:

> -	return &(iter->output);
> +	return &iter->output;

This cosmetic change is orthogonal to the patch's mission.

> -		for (wordnum = 0; wordnum < page->header.nwords; wordnum++)
> +		for (int wordnum = 0; wordnum < page->header.nwords; wordnum++)

Likewise.