At Tue, 28 Aug 2018 18:50:31 +0300, Alexander Korotkov <a.korot...@postgrespro.ru> wrote in <capphfduqeyyjlxcnx_t7k2ugcdgvw7wlkl+zrfded5wzkvm...@mail.gmail.com> > On Tue, Aug 21, 2018 at 4:48 AM Kyotaro HORIGUCHI > <horiguchi.kyot...@lab.ntt.co.jp> wrote: > > > > At Fri, 10 Aug 2018 15:33:26 +0300, Alexander Kuzmenkov > > <a.kuzmen...@postgrespro.ru> wrote in > > <5142559a-82e6-b3e4-d6ed-8fd2d240c...@postgrespro.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.
Agreed. > 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. As replied to Michael's commnet, I agree to the change. > 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. Thanks for the test. Documentaion and help message looks fine including the changed ordering. (180 seconds retry may be a bit too long but I'm fine with it.) regards. -- Kyotaro Horiguchi NTT Open Source Software Center
