Re: Reopen logfile on SIGHUP

From: Alexander Korotkov <a(dot)korotkov(at)postgrespro(dot)ru>
To: Kyotaro HORIGUCHI <horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp>
Cc: Alexander Kuzmenkov <a(dot)kuzmenkov(at)postgrespro(dot)ru>, sk(at)zsrv(dot)org, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andres Freund <andres(at)anarazel(dot)de>, Greg Stark <stark(at)mit(dot)edu>, Anastasia Lubennikova <a(dot)lubennikova(at)postgrespro(dot)ru>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Grigory Smolkin <g(dot)smolkin(at)postgrespro(dot)ru>
Subject: Re: Reopen logfile on SIGHUP
Date: 2018-08-28 15:50:31
Message-ID: CAPpHfduqEyyjLXCNx_t7K2ugCDGVW7WLKL+zrfDEd5wzkvmg-w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Aug 21, 2018 at 4:48 AM Kyotaro HORIGUCHI
<horiguchi(dot)kyotaro(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>
> At Fri, 10 Aug 2018 15:33:26 +0300, Alexander Kuzmenkov <a(dot)kuzmenkov(at)postgrespro(dot)ru> wrote in <5142559a-82e6-b3e4-d6ed-8fd2d240c77e(at)postgrespro(dot)ru>
> > On 08/09/2018 10:33 AM, Kyotaro HORIGUCHI wrote:
> > >
> > > - Since I'm not sure unlink is signal-handler safe on all
> > > supported platforms, I moved unlink() call out of
> > > checkLogrotateSignal() to SysLoggerMain, just before rotating
> > > log file.
> >
> > Which platforms specifically do you have in mind? unlink() is required
> > to be async-signal-safe by POSIX.1-2001, which is required by UNIX 03,
> > and these are rather old.
>
> I suspect that something bad can happen on Windows. Another
> reason for the movement I didn't mention was it is not necesarry
> to be there. So I applied the principle that a signal handler
> should be as small and simple as possible. As the result the
> flow of logrotate signal handling becomes similar to that for
> promote signal.

I went through this thread. It started from discussion about changing
signal handling in syslogger, which has spotted set of problems.
However, now there is a patch which add new pg_ctl command, which
issues SIGUSR1 to syslogger. It seems that nobody in the thread
object against this feature.

I've revised this patch a bit. It appears to me that only postmaster
cares about logrotate file, while syslogger just handles SIGUSR1 as it
did before. So, I've moved code that deletes logrotate file into
postmaster.c.

Also I found that this new pg_ctl isn't covered with tests at all. So
I've added very simple tap tests, which ensures that when log file was
renamed, it reappeared again after pg_ctl logrotate. I wonder how
that would work on Windows. Thankfully commitfest.cputube.org have
Windows checking facility now.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

Attachment Content-Type Size
pg_ctl_logrotate_v6.patch application/octet-stream 14.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2018-08-28 16:06:42 Re: Something's busted in plpgsql composite-variable handling
Previous Message Alvaro Herrera 2018-08-28 15:38:06 logical decoding: ABI break in 10.5 et al