Date: Fri, 12 Jul 2024 04:43:22 -0700
From: Noah Misch <noah@leadboat.com>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>,
	Nathan Bossart <nathandbossart@gmail.com>,
	Melanie Plageman <melanieplageman@gmail.com>,
	Michael Paquier <michael@paquier.xyz>,
	Fujii Masao <masao.fujii@oss.nttdata.com>,
	"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: race condition when writing pg_control
Message-ID: <20240712114322.08.nmisch@google.com>
References: <20200603020259.GA89559@paquier.xyz>
 <CA+hUKGKToqjdeRtr8ZHVbPToShun5ifjioj6x3nsjseOkhCMEQ@mail.gmail.com>
 <CAAKRu_YNGwEYrorQYza_W8tU+=toXRHG8HpyHC-KDbZqA_ZVSA@mail.gmail.com>
 <20240516183558.GA1620903@nathanxps13>
 <2136775.1715885450@sss.pgh.pa.us>
 <20240516185805.hnbt2tr23q7xbx3m@awork3.anarazel.de>
 <2138498.1715886091@sss.pgh.pa.us>
 <20240516190744.6beu43cjdvrml6uw@awork3.anarazel.de>
 <CA+hUKG+5GfOzJyH5dMh1ttgLcn8RwUS6Q=jJ+Cw_nPhq1cwsbw@mail.gmail.com>
 <CA+hUKGJU378uN_OOc28pN0hgjj2TFOb-jYZX6kamkGB9BBMK5w@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+hUKGJU378uN_OOc28pN0hgjj2TFOb-jYZX6kamkGB9BBMK5w@mail.gmail.com>
User-Agent: Mutt/2.2.12 (2023-09-09)
Archived-At: 
 <https://www.postgresql.org/message-id/20240712114322.08.nmisch%40google.com>
Precedence: bulk

On Sat, May 18, 2024 at 05:29:12PM +1200, Thomas Munro wrote:
> On Fri, May 17, 2024 at 4:46 PM Thomas Munro <thomas.munro@gmail.com> wrote:
> > The specific problem here is that LocalProcessControlFile() runs in
> > every launched child for EXEC_BACKEND builds.  Windows uses
> > EXEC_BACKEND, and Windows' NTFS file system is one of the two file
> > systems known to this list to have the concurrent read/write data
> > mashing problem (the other being ext4).

> First idea idea I've come up with to avoid all of that: pass a copy of
> the "proto-controlfile", to coin a term for the one read early in
> postmaster startup by LocalProcessControlFile().  As far as I know,
> the only reason we need it is to suck some settings out of it that
> don't change while a cluster is running (mostly can't change after
> initdb, and checksums can only be {en,dis}abled while down).  Right?
> Children can just "import" that sucker instead of calling
> LocalProcessControlFile() to figure out the size of WAL segments yada
> yada, I think?  Later they will attach to the real one in shared
> memory for all future purposes, once normal interlocking is allowed.

I like that strategy, particularly because it recreates what !EXEC_BACKEND
backends inherit from the postmaster.  It might prevent future bugs that would
have been specific to EXEC_BACKEND.

> I dunno.  Draft patch attached.  Better plans welcome.  This passes CI
> on Linux systems afflicted by EXEC_BACKEND, and Windows.  Thoughts?

Looks reasonable.  I didn't check over every detail, given the draft status.