Latch-ifying the syslogger process

On 05/12/2012 03:36 PM, Tom Lane wrote:

I noticed a large oversight in our efforts to reduce the server's idle
wakeup frequency: if you've got logging_collector turned on, the
syslogger process will wake up once a second, whether it has anything
to do or not. But the only reasons it has for waking up are signals,
data arrival, and time-based logfile rotation, and it is easy to
calculate the time until the next logfile rotation event. So this
seems really easy to latch-ify, and I would like to apply the attached
patch if there are not objections. I do not however have the ability
to test the Windows side of it, so it'd be nice if someone would check
that that still works (particularly, that it shuts down cleanly).

I can do that. I'm doing some Windows investigation ATM so this won't be
hard to add on to it.

It's worth pointing out that the buildfarm client doesn't currently test
the syslogger at all. It probably should, at least optionally. That
wouldn't be too hard to arrange. A SMOP :-)

cheers

andrew

On 05/12/2012 04:00 PM, Andrew Dunstan wrote:

On 05/12/2012 03:36 PM, Tom Lane wrote:

I noticed a large oversight in our efforts to reduce the server's idle
wakeup frequency: if you've got logging_collector turned on, the
syslogger process will wake up once a second, whether it has anything
to do or not. But the only reasons it has for waking up are signals,
data arrival, and time-based logfile rotation, and it is easy to
calculate the time until the next logfile rotation event. So this
seems really easy to latch-ify, and I would like to apply the attached
patch if there are not objections. I do not however have the ability
to test the Windows side of it, so it'd be nice if someone would check
that that still works (particularly, that it shuts down cleanly).

I can do that. I'm doing some Windows investigation ATM so this won't
be hard to add on to it.

It's worth pointing out that the buildfarm client doesn't currently
test the syslogger at all. It probably should, at least optionally.
That wouldn't be too hard to arrange. A SMOP :-)

Everything looks kosher on my Windows machine (tested both MSVC and
Mingw64 builds)

cheers

andrew

Andrew Dunstan <andrew@dunslane.net> writes:

On 05/12/2012 03:36 PM, Tom Lane wrote:

... I do not however have the ability
to test the Windows side of it, so it'd be nice if someone would check
that that still works (particularly, that it shuts down cleanly).

Everything looks kosher on my Windows machine (tested both MSVC and
Mingw64 builds)

Great, thanks for testing! I've committed that patch, and am back to
wondering what the heck was wrong with the stats collector latch
patch...

regards, tom lane

I wrote:

While testing this I discovered a pre-existing bug in the Unix
implementation of WaitLatchOrSocket: EOF on the socket is reported as
POLLHUP not POLLIN (at least on my Linux box), which results in
WaitLatchOrSocket going into an infinite loop, because poll() returns
immediately but the result bitmask never becomes nonzero.

BTW, I just came across this in Microsoft's documentation of
WSAEventSelect:

Note that Windows Sockets will record only an FD_CLOSE network event
to indicate closure of a virtual circuit. It will not record an
FD_READ network event to indicate this condition.

which seems to me to indicate that the Windows version of
WaitLatchOrSocket has a related bug. We want socket EOF to result in
WL_SOCKET_READABLE being returned, no? Otherwise the caller may never
realize that it has an EOF condition to deal with.

regards, tom lane