Re: Patch to implement pg_current_logfile() function

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Karl O(dot) Pinc" <kop(at)meme(dot)com>, Gilles Darold <gilles(dot)darold(at)dalibo(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Patch to implement pg_current_logfile() function
Date: 2017-03-07 00:45:49
Views: Raw Message | Whole Thread | Download mbox
Lists: pgsql-hackers

On Tue, Mar 7, 2017 at 3:21 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Sat, Mar 4, 2017 at 10:32 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Without having actually looked at this patch, I would say that if it added
>> a direct call of fopen() to backend-side code, that was already the wrong
>> thing. Almost always, AllocateFile() would be a better choice, not only
>> because it's tied into transaction abort, but also because it knows how to
>> release virtual FDs in event of ENFILE/EMFILE errors. If there is some
>> convincing reason why you shouldn't use AllocateFile(), then a safe
>> cleanup pattern would be to have the fclose() in a PG_CATCH stanza.
> I think that my previous remarks on this issue were simply muddled
> thinking. The SQL-callable function pg_current_logfile() does use
> AllocateFile(), so the ERROR which may occur afterward if the file is
> corrupted is no problem. The syslogger, on the other hand, uses
> logfile_open() to open the file, but it's careful not to throw an
> ERROR while the file is open, just like other code which runs in the
> syslogger. So now I think there's no bug here.

- /*
- * No space found, file content is corrupted. Return NULL to the
- * caller and inform him on the situation.
- */
+ /* Uh oh. No newline found, so file content is corrupted. */
This one just made me smile.

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2017-03-07 00:53:30 Re: Need a builtin way to run all tests faster manner
Previous Message Tom Lane 2017-03-07 00:45:27 Re: Need a builtin way to run all tests faster manner