Re: Problem while setting the fpw with SIGHUP

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: michael(at)paquier(dot)xyz
Cc: dilipbalaut(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Problem while setting the fpw with SIGHUP
Date: 2018-03-27 12:01:20
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

At Tue, 27 Mar 2018 16:46:30 +0900, Michael Paquier <michael(at)paquier(dot)xyz> wrote in <20180327074630(dot)GD9940(at)paquier(dot)xyz>
> I have finally been able to spend more time on this issue, and checked
> for a couple of hours all the calls to RecoveryInProgress() that could
> be triggered within a critical section to see if the move I suggested
> previously is worth it ot not as this would cost some memory for
> standbys all the time, which would suck for many read-only sessions.
> There are a couple of calls potentially happening in critical sections,
> however except for the one in UpdateFullPageWrites() those are used for
> sanity checks in code paths that should never trigger it, take
> XLogInsertBegin() for example. So with assertions this would trigger
> a failure before the real elog(ERROR) message shows up.
> Hence, I am changing opinion still I think that instead of the patch you
> proposed first we could just do a call to InitXLogInsert() before
> entering the critical section. This is also more consistent with what
> CreateCheckpoint() does. That's also less risky for a backpatch as your
> patch increases the window between the beginning of the critical section
> and the real moment where the check for RecoveryInProgress is needed. A
> comment explaining why the initialization needs to happen is also
> essential.
> All in all, this would give the simple patch attached.
> Thoughts?

At the first look I was uneasy that the patch initializes xlog
working area earlier than required.

The current UpdateFullPageWrites is safe on standby and promotion
so what we should consider is only the non-standby case. I think
what we should do is just calling RecoveryInProgress() at the
beginning of CheckPointerMain, which is just the same thing with
InitPostgres, but before setting up signal handler to avoid
processing SIGHUP before being ready to insert xlog.


Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
setup_xlog_workarea_when_not_standby.patch text/x-patch 653 bytes

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2018-03-27 12:08:31 Re: pgbench - test whether a variable exists
Previous Message Robert Haas 2018-03-27 12:00:31 Re: Changing WAL Header to reduce contention during ReserveXLogInsertLocation()