Re: Changing WAL Header to reduce contention during ReserveXLogInsertLocation() - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Changing WAL Header to reduce contention during ReserveXLogInsertLocation()
Date
Msg-id 23445.1522164780@sss.pgh.pa.us
Whole thread Raw
In response to Re: Changing WAL Header to reduce contention during ReserveXLogInsertLocation()  (Simon Riggs <simon@2ndquadrant.com>)
List pgsql-hackers
Simon Riggs <simon@2ndquadrant.com> writes:
> On 27 March 2018 at 14:58, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The point of the xl_prev links is, essentially, to allow verification
>> that we are reading a coherent series of WAL records, ie that record B
>> which appears to follow record A actually is supposed to follow it.
>> This throws away that cross-check just as effectively as the other patch
>> did, leaving us reliant solely on the per-record CRCs.  CRCs are good,
>> but they do have a false-positive rate.

> You aren't reliant solely on the CRC.

> If you have a false-positive CRC then xl_curr allows you to validate
> the record, just as you would with xl_prev. The xl_curr value points
> to itself using just as many bits as the xl_prev field, so the
> probability of false validation is the same as now.

No.  xl_curr says that this record is where it thinks it should be.
It fails to provide indication as to which record it should follow,
and that in my opinion is important context-sensitivity that xl_curr
would lack.

The XLOG_SWITCH example speaks to this.  Now that we have variable
wal segment size, it is not an ironclad certainty that all onlookers
agree as to where the segment boundaries are.  Suppose someone reads
an XLOG_SWITCH, and thinks it means "skip to the next 16MB boundary"
whereas the writer thought it meant "skip to the next 8MB boundary".
With xl_prev, the prev-link mismatch will clue that reader that
something is wrong.  With xl_curr, there is no mismatch to detect,
and the reader will go happily on its way having missed 8MB worth
of WAL.

The impression I have, without having really studied the details,
is that all of the patches proposed in this thread are trying to
buy concurrency by removing context-sensitivity of the WAL headers.
I do not think that's a good idea.

            regards, tom lane


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Parallel safety of binary_upgrade_create_empty_extension
Next
From: Tom Lane
Date:
Subject: Re: Changing WAL Header to reduce contention during ReserveXLogInsertLocation()