Horton

Re: Improve WALRead() to suck data directly from WAL buffers when possible

Andres Freund <andres@anarazel.de>

View thread
From: Andres Freund <andres@anarazel.de>
To: Jeff Davis <pgsql@j-davis.com>
Cc: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, 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: 2024-02-12T20:18:53Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add XLogCtl->logInsertResult

  2. Add assert to WALReadFromBuffers().

  3. Read WAL directly from WAL buffers.

  4. Additional write barrier in AdvanceXLInsertBuffer().

  5. Use 64-bit atomics for xlblocks array elements.

  6. Don't trust unvalidated xl_tot_len.

Hi,

On 2024-02-12 11:33:24 -0800, Jeff Davis wrote:
> On Wed, 2024-01-31 at 14:30 +0530, Bharath Rupireddy wrote:
> > Please see the attached v22 patch set.
>
> Committed 0001.

Yay, I think this is very cool. There are plenty other improvements than can
be based on this...


One thing I'm a bit confused in the code is the following:

+    /*
+     * Don't read past the available WAL data.
+     *
+     * Check using local copy of LogwrtResult. Ordinarily it's been updated by
+     * the caller when determining how far to read; but if not, it just means
+     * we'll read less data.
+     *
+     * XXX: the available WAL could be extended to the WAL insert pointer by
+     * calling WaitXLogInsertionsToFinish().
+     */
+    upto = Min(startptr + count, LogwrtResult.Write);
+    nbytes = upto - startptr;

Shouldn't it pretty much be a bug to ever encounter this? There aren't
equivalent checks in WALRead(), so any user of WALReadFromBuffers() that then
falls back to WALRead() is just going to send unwritten data.

ISTM that this should be an assertion or error.

Greetings,

Andres Freund