Thread: GIN, XLogInsert and MarkBufferDirty

GIN, XLogInsert and MarkBufferDirty

From
Heikki Linnakangas
Date:
Hi Teodor,

I think there's a little bug in ginInsertValue. A page is marked as 
dirty with MarkBufferDirty after writing the corresponding WAL record 
with XLogInsert. That's not safe, MarkBufferDirty needs to be called 
before XLogInsert to avoid a race condition in checkpoint, see comments 
in SyncOneBuffer in bufmgr.c for an explanation.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com


Re: GIN, XLogInsert and MarkBufferDirty

From
Teodor Sigaev
Date:
> with XLogInsert. That's not safe, MarkBufferDirty needs to be called 
> before XLogInsert to avoid a race condition in checkpoint, see comments 
> in SyncOneBuffer in bufmgr.c for an explanation.

Ugh, thank you fixed. It's a trace of misunderstood of WriteBuffer().
-- 
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
  WWW: http://www.sigaev.ru/
 


Re: GIN, XLogInsert and MarkBufferDirty

From
Tom Lane
Date:
Heikki Linnakangas <heikki@enterprisedb.com> writes:
> ... MarkBufferDirty needs to be called 
> before XLogInsert to avoid a race condition in checkpoint, see comments 
> in SyncOneBuffer in bufmgr.c for an explanation.

Right, see also the "Write-Ahead Log coding" section in
src/backend/access/transam/README (which is maybe not a very good place
for it, but it doesn't seem like bufmgr's turf either).
        regards, tom lane


Re: GIN, XLogInsert and MarkBufferDirty

From
Heikki Linnakangas
Date:
Tom Lane wrote:
> Heikki Linnakangas <heikki@enterprisedb.com> writes:
>> ... MarkBufferDirty needs to be called 
>> before XLogInsert to avoid a race condition in checkpoint, see comments 
>> in SyncOneBuffer in bufmgr.c for an explanation.
> 
> Right, see also the "Write-Ahead Log coding" section in
> src/backend/access/transam/README (which is maybe not a very good place
> for it, but it doesn't seem like bufmgr's turf either).

Yeah it could be documented more visibly, I didn't know about (or didn't 
remember) that rule until I saw that comment today. I found that issue 
in GIN by just quickly grepping for callers of MarkBufferDirty.

How about adding an Assert to XLogInsert to check that all buffers given 
to it are already marked as dirty? It wouldn't be completely 
water-tight, sometimes we don't pass the buffer to XLogInsert even 
though we stamp the LSN, but it would catch most cases.


--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com