Re: Using XLogFileNameP in critical section

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Using XLogFileNameP in critical section
Date: 2019-12-02 06:50:45
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Fri, Nov 29, 2019 at 06:44:58PM +0100, Masahiko Sawada wrote:
> I encountered that the assertion error is reported instead of a proper
> PANIC message when failed to fsync WAL. The cause is that there are
> multiple places where we call XLogFileNameP function that calls palloc
> during critical section, for example XLogWrite function.
> TRAP: FailedAssertion("CritSectionCount == 0 ||
> (context)->allowInCritSection", File: "mcxt.c", Line: 956)
> As far as I can see there are five places we need to fix. I've
> attached a patch.

+ msg = "could not fdatasync file \"%s\": %m";
Missing some translations, no?

You are missing a couple of code paths in walreceiver.c,
XLogWalRcvWrite(), where XLogFileNameP is used on a PANIC. This
brings me the following points:
1) If you look closely, all the callers of XLogFileNameP() are used
for the generation of error strings.
2) I am ready to bet that we'll have the same discussion in the future
because somebody will make the same mistake for a new code path.

I think that we had better just get rid of XLogFileNameP() (on HEAD)
and just change those code paths so as they use a buffer of size
MAXFNAMELEN, with XLogFileName() generating the file name. This leads
actually to some simplifications, see for example XLogWalRcvWrite..

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2019-12-02 08:21:34 Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace on the fly
Previous Message Yugo Nagata 2019-12-02 06:42:08 Re: Implementing Incremental View Maintenance