Re: Problem while setting the fpw with SIGHUP

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Problem while setting the fpw with SIGHUP
Date: 2018-03-27 07:46:30
Message-ID: 20180327074630.GD9940@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On Mon, Mar 26, 2018 at 02:32:22PM +0530, Dilip Kumar wrote:
> On Fri, Mar 23, 2018 at 1:19 PM, Michael Paquier <michael(at)paquier(dot)xyz>
> wrote:
> In StartupXLOG, just before the CreateCheckPoint() call, we are calling
> LocalSetXLogInsertAllowed(). So, I am thinking can we just remove
> InitXLogInsert from CreateCheckpoint. And, we don't need to move it to
> bootstrap.c. Or am I missing something?

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?
--
Michael

Attachment Content-Type Size
0001-Fix-WAL-insert-when-updating-full_page_writes-for-ch.patch text/x-diff 1.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Teodor Sigaev 2018-03-27 07:53:12 Re: [HACKERS] GSoC 2017 : Patch for predicate locking in Gist index
Previous Message David G. Johnston 2018-03-27 07:24:07 Re: PQHost() undefined behavior if connecting string contains both host and hostaddr types