Re: Using XLogFileNameP in critical section - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: Using XLogFileNameP in critical section
Date
Msg-id 20191202065045.GH1696@paquier.xyz
Whole thread Raw
In response to Using XLogFileNameP in critical section  (Masahiko Sawada <masahiko.sawada@2ndquadrant.com>)
Responses Re: Using XLogFileNameP in critical section
List 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..
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Yugo Nagata
Date:
Subject: Re: Implementing Incremental View Maintenance
Next
From: Michael Paquier
Date:
Subject: Re: Allow CLUSTER, VACUUM FULL and REINDEX to change tablespace onthe fly