Re: Using XLogFileNameP in critical section

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Masahiko Sawada <masahiko(dot)sawada(at)2ndquadrant(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Using XLogFileNameP in critical section
Date: 2019-12-04 05:48:16
Message-ID: 20191204054816.GA23277@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Dec 03, 2019 at 09:35:00AM -0300, Alvaro Herrera wrote:
> You didn't attach anything, but I concur about the low probability
> aspect: the assertion failure does not occur in production builds
> (obviously); and only an out-of-memory situation is a real problem
> when
> an fsync fails. Anyway this should be a very localized fix, right?

Sorry. You get something like the attached. The recent refactoring
work you committed in this area causes already conflicts on
REL_12_STABLE.

> I'm not sure that the internationalization stuff in issue_xlog_fsync
> is correct. I think the _() should be gettext_noop(), or alternatively
> the errmsg() should be errmsg_internal(); otherwise the translation is
> invoked twice. (I didn't verify this.)

Hmm. We actually do both in tablecmds.c:ATWrongRelkindError(), and
that's the code I was looking at yesterday when thinking about the
problem.. However, parse_agg.c, parse_expr.c and parse_func.c among
others like vacuumlazy.c use directly errmsg_internal() without
translating the string first. So there is indeed duplicated work for
both. Does the attached patch look correct to you?
--
Michael

Attachment Content-Type Size
gettext-fix.patch text/x-diff 896 bytes
0001-Remove-use-of-XLogFileNameP-in-error-string-generati-v12.patch text/x-diff 11.1 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2019-12-04 06:10:55 Re: pgbench -i progress output on terminal
Previous Message Kyotaro Horiguchi 2019-12-04 05:33:07 Re: Session WAL activity