From: | "Karl O(dot) Pinc" <kop(at)meme(dot)com> |
---|---|
To: | Gilles Darold <gilles(dot)darold(at)dalibo(dot)com> |
Cc: | Christoph Berg <myon(at)debian(dot)org>, 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 |
Date: | 2016-12-02 01:08:41 |
Message-ID: | 20161201190841.57f949e8@slate.meme.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, 27 Nov 2016 21:54:46 +0100
Gilles Darold <gilles(dot)darold(at)dalibo(dot)com> wrote:
> I've attached the v15 of the patch
> I've not applied patch patch_pg_current_logfile-v14.diff.backoff to
> prevent constant call of logfile_writename() on a busy system (errno =
> ENFILE | EMFILE).
I don't think it should be applied and included in the basic
functionality patch in any case. I think it needs to be submitted as a
separate patch along with the basic functionality patch. Backing off
the retry of the current_logfiles write could be overly fancy and
simply not needed.
> I think this can be done quite simply by testing if
> log rotate is still enabled. This is possible because function
> logfile_rotate() is already testing if errno = ENFILE | EMFILE and in
> this case rotation_disabled is set to true. So the following test
> should do the work:
>
> if (log_metainfo_stale && !rotation_disabled)
> logfile_writename();
>
> This is included in v15 patch.
I don't see this helping much, if at all.
First, it's not clear just when rotation_disabled can be false
when log_metainfo_stale is true. The typical execution
path is for logfile_writename() to be called after rotate_logfile()
has already set rotataion_disabled to true. logfile_writename()
is the only place setting log_metainfo_stale to true and
rotate_logfile() the only place settig rotation_disabled to false.
While it's possible
that log_metainfo_stale might be set to true when logfile_writename()
is called from within open_csvlogfile(), this does not help with the
stderr case. IMO better to just test for log_metaifo_stale at the
code snippet above.
Second, the whole point of retrying the logfile_writename() call is
to be sure that the current_logfiles file is updated before the logs
rotate. Waiting until logfile rotation is enabled defeats the purpose.
Regards,
Karl <kop(at)meme(dot)com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2016-12-02 01:10:38 | Re: Proposal for changes to recovery.conf API |
Previous Message | Simon Riggs | 2016-12-02 00:35:04 | Re: XactLockTableWait doesn't set wait_event correctly |