Re: Problem while setting the fpw with SIGHUP

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Michael Paquier <michael(at)paquier(dot)xyz>, Robert Haas <robertmhaas(at)gmail(dot)com>, hlinnaka <hlinnaka(at)iki(dot)fi>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Problem while setting the fpw with SIGHUP
Date: 2018-04-20 12:36:39
Message-ID: CAA4eK1JPKMdT68WJbLAksuUyHi+L0j0ygqVRU1Lbn4DQ2D5=+w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Apr 20, 2018 at 11:40 AM, Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
> By the way, I think I found a bug of FPW.
>
> The following steps yields INSERT record that doesn't have a FPI
> after a checkpoint.
>
> (Start server with full_page_writes = off)
> CREATE TABLE t (a int);
> CHECKPOINT;
> INSERT INTO t VALUES (1);
> ALTER SYSTEM SET full_page_writes TO on;
> SELECT pg_reload_conf();
> CHECKPOINT;
> INSERT INTO t VALUES (1);
>
> The last insert is expected to write a record with FPI but it
> doesn't actually. No FPI will be written for the page after that.
>
> It seems that the reason is that XLogInsertRecord is forgetting
> to check doPageWrites' update.
>
> In the failure case, fpw_lsn has been set by XLogRecordAssemble
> but doPageWrite is false at the time and it considers that no FPI
> is required then it sets fpw_lsn to InvalidXLogRecPtr.
>
> After that, XLogInsertRecord receives the record but it thinks
> that doPageWrites is true since it looks the shared value.
>
>> if (fpw_lsn != InvalidXLogRecPtr && fpw_lsn <= RedoRecPtr && doPageWrites)
>
> So this line thinks that "no FPI is omitted in this record" but
> actually the record is just forgotten to attach them.
>
> The attached patch lets XLogInsertRecord check if doPageWrites
> has been turned on after the last call and cause recomputation in
> the case.
>

I think you have correctly spotted the problem and your fix looks good
to me. As this is a separate problem and fix is different from what
we are discussing here, I think this can be committed it separately.

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tatsuo Ishii 2018-04-20 12:38:22 Re: Built-in connection pooling
Previous Message Craig Ringer 2018-04-20 11:32:25 Re: Built-in connection pooling