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