|From:||Gilles Darold <gilles(dot)darold(at)dalibo(dot)com>|
|To:||"Karl O(dot) Pinc" <kop(at)meme(dot)com>|
|Cc:||Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, "Shulgin, Oleksandr" <oleksandr(dot)shulgin(at)zalando(dot)de>, Robert Haas <robertmhaas(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: Patch to implement pg_current_logfile() function|
|Views:||Raw Message | Whole Thread | Download mbox|
Le 07/04/2016 08:30, Karl O. Pinc a écrit :
> On Thu, 7 Apr 2016 01:13:51 -0500
> "Karl O. Pinc" <kop(at)meme(dot)com> wrote:
>> On Wed, 6 Apr 2016 23:37:09 -0500
>> "Karl O. Pinc" <kop(at)meme(dot)com> wrote:
>>> On Wed, 6 Apr 2016 22:26:13 -0500
>>> "Karl O. Pinc" <kop(at)meme(dot)com> wrote:
>>>> On Wed, 23 Mar 2016 23:22:26 +0100
>>>> Gilles Darold <gilles(dot)darold(at)dalibo(dot)com> wrote:
>>>>> Thanks for the reminder, here is the v3 of the patch after a
>>>>> deeper review and testing. It is now registered to the next
>>>>> commit fest under the System Administration topic.
>> This is what I see at the moment. I'll wait for replies
>> before looking further and writing more.
> Er, one more thing. Isn't it true that in logfile_rotate()
> you only need to call store_current_log_filename() when
> logfile_open() is called with a "w" mode, and never need to
> call it when logfile_open() is called with an "a" mode?
> Karl <kop(at)meme(dot)com>
> Free Software: "You don't pay back, you pay forward."
> -- Robert A. Heinlein
Thank you very much for the patch review and please apologies this too
long response delay. I was traveling since end of April and totally
forgotten this patch. I have applied all your useful feedbacks on the
patch and attached a new one (v4) to this email :
- Fix the missing <row> in doc/src/sgml/func.sgml
- Rewrite pg_current_logfile descritption as suggested
- Remove comment in src/backend/postmaster/syslogger.c
- Use pgrename to first write the filename to a temporary file
before overriding the last log file.
- Rename store_current_log_filename() into logfile_writename() -
logfile_getname is used to retrieve the filename.
- Use logfile_open() to enable CRLF line endings on Windows
- Change log level for when it can't create pg_log_file, from
WARNING to LOG
- Remove NOTICE message when the syslogger is not enabled, the
function return null.
On other questions:
> "src/backend/postmaster/syslogger.c expects to see fopen() fail with
ENFILE and EMFILE. What will you do if you get these?"
- Nothing, if the problem occurs during the log rotate call, then
log rotation file is disabled so logfile_writename() will not be called.
Case where the problem occurs between the rotation call and the
logfile_writename() call is possible but I don't think that it will be
useful to try again. In this last case log filename will be updated
during next rotation.
> "Have you given any thought as to when logfile_rotate() is called?
Since logfile_rotate() itself logs with ereport(), it would _seem_ safe
to ereport() from within your store_current_log_filename(), called from
- Other part of logfile_rotate() use ereport() so I guess it is safe
to use it.
> "The indentation of the ereport(), in the part that continues over
- This was my first thought but seeing what is done in the other
call to ereport() I think I have done it the right way.
> "Isn't it true that in logfile_rotate() you only need to call
store_current_log_filename() when logfile_open() is called with a "w"
mode, and never need to call it when logfile_open() is called with an
- No, it is false, append mode is used when logfile_rotate used an
existing file but the filename still change.
|Next Message||Rajeev rastogi||2016-06-28 10:01:28||Re: Improving executor performance|
|Previous Message||Etsuro Fujita||2016-06-28 07:22:28||Re: Postgres_fdw join pushdown - wrong results with whole-row reference|