Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Zhang Mingli <zmlpostgres(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>, Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>
Subject: Re: Issue in GIN fast-insert: XLogBeginInsert + Read/LockBuffer ordering
Date: 2022-10-12 02:09:41
Message-ID: Y0Yh5Xarpb894cRH@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Sep 30, 2022 at 06:22:02PM +0800, Zhang Mingli wrote:
> In the same function, there is disorder of XLogBeginInsert and START_CRIT_SECTION.
>
> ```
> collectordata = ptr = (char *) palloc(collector->sumsize);
>
>  data.ntuples = collector->ntuples;
>
>  if (needWal)
>    XLogBeginInsert();
>
>  START_CRIT_SECTION();
> ```
>
> Shall we adjust that too?

Nice catches, both of you. Let's adjust everything spotted in one
shot. Matthias' patch makes the ordering right, but the
initialization path a bit harder to follow when using a separate list.
The code is short so it does not strike me as a big deal, and it comes
from the fact that we need to lock an existing buffer when merging two
lists. For the branch where we insert into a tail page, the pages are
already locked but it looks to be enough to move XLogBeginInsert()
before the first XLogRegisterBuffer() call.

Would any of you like to write a patch?
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message wangw.fnst@fujitsu.com 2022-10-12 02:10:59 RE: Perform streaming logical transactions by background workers and parallel apply
Previous Message David Rowley 2022-10-12 01:50:58 Re: shadow variables - pg15 edition