Re: MarkBufferDirtyHint() and LSN update - Mailing list pgsql-hackers

From Antonin Houska
Subject Re: MarkBufferDirtyHint() and LSN update
Date
Msg-id 13954.1573742911@antos
Whole thread Raw
In response to Re: MarkBufferDirtyHint() and LSN update  (Michael Paquier <michael@paquier.xyz>)
Responses Re: MarkBufferDirtyHint() and LSN update
List pgsql-hackers
Michael Paquier <michael@paquier.xyz> wrote:

> On Mon, Nov 11, 2019 at 10:03:14AM +0100, Antonin Houska wrote:
> > This looks good to me.
> 
> Actually, no, this is not good.  I have been studying more the patch,
> and after stressing more this code path with a cluster having
> checksums enabled and shared_buffers at 1MB, I have been able to make
> a couple of page's LSNs go backwards with pgbench -s 100.  The cause
> was simply that the page got flushed with a newer LSN than what was
> returned by XLogSaveBufferForHint() before taking the buffer header
> lock, so updating only the LSN for a non-dirty page was simply
> guarding against that.

Interesting. Now that I know about the problem, I could have reproduced it
using gdb: MarkBufferDirtyHint() was called by 2 backends concurrently in such
a way that the first backend generates the LSN, but before it manages to
assign it to the page, another backend generates another LSN and sets it.

Can't we just apply the attached diff on the top of your patch?

Also I wonder how checksums helped you to discover the problem? Although I
could simulate the setting of lower LSN, I could not see any broken
checksum. And I wouldn't even expect that since FlushBuffer() copies the
buffer into backend local memory before it calculates the checksum.

-- 
Antonin Houska
Web: https://www.cybertec-postgresql.com


Attachment

pgsql-hackers by date:

Previous
From: Fabien COELHO
Date:
Subject: Re: segfault in geqo on experimental gcc animal
Next
From: Peter Eisentraut
Date:
Subject: Re: Proposal: Add more compile-time asserts to exposeinconsistencies.