Re: Reopen logfile on SIGHUP

From: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
To: a(dot)kuzmenkov(at)postgrespro(dot)ru
Cc: tgl(at)sss(dot)pgh(dot)pa(dot)us, robertmhaas(at)gmail(dot)com, andres(at)anarazel(dot)de, stark(at)mit(dot)edu, a(dot)lubennikova(at)postgrespro(dot)ru, pgsql-hackers(at)postgresql(dot)org, g(dot)smolkin(at)postgrespro(dot)ru
Subject: Re: Reopen logfile on SIGHUP
Date: 2018-04-24 03:09:50
Message-ID: 20180424.120950.17896065.horiguchi.kyotaro@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Fri, 20 Apr 2018 21:51:49 +0300, Alexander Kuzmenkov <a(dot)kuzmenkov(at)postgrespro(dot)ru> wrote in <c7cd74db-dde3-49e7-cdde-b909b0fdac0b(at)postgrespro(dot)ru>
> On 04/16/2018 05:54 AM, Kyotaro HORIGUCHI wrote:
> > We can provide a new command "pg_ctl logrotate" to hide the
> > details. (It cannot be executed by root, though.)
>
> I like this approach.

Thanks. I found that the SIGUSR1 path is already ignoring
rotation_disabled so we don't need bother with the flag.

> I looked at the patch and changed some things:
> - cleaned up the error messages

Thanks for cleaning up crude copy'n pastes and wording.

> - moved checkLogrotateSignal to postmaster.c, since it has no reason to
> - be in xlog.c

Agreed. But

there seem to be no convention that static function starts with a
lower case letter. checkControlFile initMasks are minor instances
but..

> - added some documentation I had from my older patch
>
> Other than that, it looks good to me. The updated patch is attached.

Thanks for the documentation, but I see a description for the
same objective and different measure just above there.

https://www.postgresql.org/docs/devel/static/logfile-maintenance.html

> Alternatively, you might prefer to use an external log rotation
> program if you have one that you are already using with other
...
> rotation, the logrotate program can be configured to work with
> log files from syslog.

It seems that the additional description needs to be meld into
this at the first place? And some caveat may be needed on failure
cases. And in the attached the comment for "if
(rotateion_requested)" is edited.

regards.

--
Kyotaro Horiguchi
NTT Open Source Software Center

Attachment Content-Type Size
pg_ctl_logrotate_v2_diff.patch text/x-patch 4.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Gould 2018-04-24 03:17:23 Re: [patch] BUG #15005: ANALYZE can make pg_class.reltuples inaccurate.
Previous Message Amit Langote 2018-04-24 02:23:28 Re: Should we add GUCs to allow partition pruning to be disabled?