Message-ID: <c3455ab9da42e09ca9d059879b5c512b2d1f9681.camel@j-davis.com>
Subject: Re: Improve WALRead() to suck data directly from WAL buffers when
 possible
From: Jeff Davis <pgsql@j-davis.com>
To: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Cc: Andres Freund <andres@anarazel.de>, Dilip Kumar <dilipbalaut@gmail.com>,
  Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
 pgsql-hackers@lists.postgresql.org, Nathan Bossart
 <nathandbossart@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>
Date: Fri, 03 Nov 2023 12:47:24 -0700
In-Reply-To: <CALj2ACUXN42H=_Jzw4oYZxoW0gyx82gALJ7vicz9UOuLVRbjnw@mail.gmail.com>
References: 
	<CALj2ACXKKK=wbiG5_t6dGao5GoecMwRkhr7GjVBM_jg54+Na=Q@mail.gmail.com>
	 <20221212.115717.1183698540772933396.horikyota.ntt@gmail.com>
	 <CALj2ACXUbvON86vgwTkum8ab3bf1=HkMxQ5hZJZS3ZcJn8NEXQ@mail.gmail.com>
	 <CAFiTN-uCG-+axA4r02_9OYgKJU_GkJ1=tBCxq_5UvyWmWEBywA@mail.gmail.com>
	 <CALj2ACU9cfAcfVsGwUqXMace_7rfSBJ7+hXVJfVV1jnspTDGHQ@mail.gmail.com>
	 <628b07b79743e2614ab1724f516cbd41560d6551.camel@j-davis.com>
	 <20230114203403.4zpg72fw2qb34awf@awork3.anarazel.de>
	 <0d84c025e38673cb575dcf4987d050fe7b1e1a1f.camel@j-davis.com>
	 <20231011224353.cl7c2s222dw3de4j@awork3.anarazel.de>
	 <CALj2ACU2DGH-BB_187N7gfVkLdxfJ+doVTcteS0ugLn_c8mLOw@mail.gmail.com>
	 <CALj2ACVgVgA5BSSrEYO2eTMEGB=QUbcYosYm3vZ3R2=GPB6tNw@mail.gmail.com>
	 <5c6e8e60c5c156a9f51ccf850f70829e45a985a1.camel@j-davis.com>
	 <CALj2ACVjzHgQvONnTLffyBJRACPwhsf-cYG4TQ2KHKrFfFop-w@mail.gmail.com>
	 <4132fe48f831ed6f73a9eb191af5fe475384969c.camel@j-davis.com>
	 <CALj2ACV123Ke6wn05QCnCgJhFpcarGCv7sKM1p39t+qpUNHUgQ@mail.gmail.com>
	 <2ef04861c0f77e7ae78b703770cc2bbbac3d85e6.camel@j-davis.com>
	 <CALj2ACUXN42H=_Jzw4oYZxoW0gyx82gALJ7vicz9UOuLVRbjnw@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.44.4-0ubuntu2 
MIME-Version: 1.0
Archived-At: <https://www.postgresql.org/message-id/c3455ab9da42e09ca9d059879b5c512b2d1f9681.camel%40j-davis.com>
Precedence: bulk

On Fri, 2023-11-03 at 20:23 +0530, Bharath Rupireddy wrote:
> >=20
> OldestInitializedPage is introduced in v14-0001 patch. Please have a
> look.

I don't see why that's necessary if we move to the algorithm I
suggested below that doesn't require a lock.

> >=20
> Okay. Current patch doesn't support this [partial hit of newer pages]
> case.

OK, no need to support it until you see a reason.
> >=20

> >=20
> > I think it needs something like:
> >=20
> > =C2=A0 pg_atomic_write_u64(&XLogCtl->xlblocks[nextidx],
> > InvalidXLogRecPtr);
> > =C2=A0 pg_write_barrier();
> >=20
> > before the MemSet.
>=20
> I think it works. First, xlblocks needs to be turned to an array of
> 64-bit atomics and then the above change.

Does anyone see a reason we shouldn't move to atomics here?

>=20
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 pg_write_barrier();
>=20
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *((volatile XLogRecPtr *) &XLo=
gCtl->xlblocks[nextidx]) =3D
> NewPageEndPtr;

I am confused why the "volatile" is required on that line (not from
your patch). I sent a separate message about that:

https://www.postgresql.org/message-id/784f72ac09061fe5eaa5335cc347340c367c7=
3ac.camel@j-davis.com

> I think the 3 things that helps read from WAL buffers without
> WALBufMappingLock are: 1) couple of the read barriers in
> XLogReadFromBuffers, 2) atomically initializing xlblocks[idx] to
> InvalidXLogRecPtr plus a write barrier in AdvanceXLInsertBuffer(), 3)
> the following sanity check to see if the read page is valid in
> XLogReadFromBuffers(). If it sounds sensible, I'll work towards
> coding
> it up. Thoughts?

I like it. I think it will ultimately be a fairly simple loop. And by
moving to atomics, we won't need the delicate comment in
GetXLogBuffer().


Regards,
	Jeff Davis