pg.ddx.io  pgsql-bugs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Paquier <michael@paquier.xyz>
Cc: Andrew Dunstan <andrew@dunslane.net>
Cc: Andres Freund <andres@anarazel.de>
Cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: Yuli Khodorkovskiy <yuli.khodorkovskiy@crunchydata.com>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #15804: Assertion failure when using logging_collector with EXEC_BACKEND
Date: Sun, 19 May 2019 12:09:41 -0400
Message-ID: <27719.1558282181@sss.pgh.pa.us> (raw)
In-Reply-To: <22995.1558278322@sss.pgh.pa.us>
References: <20190516021903.GB1415@paquier.xyz>
	<28927.1557977558@sss.pgh.pa.us>
	<20190516035617.GC1415@paquier.xyz>
	<20190516043618.GD1415@paquier.xyz>
	<31437.1557982294@sss.pgh.pa.us>
	<20190516060628.GE1415@paquier.xyz>
	<20190517042536.GM20887@paquier.xyz>
	<20190517202443.gw4zdfkpwgsjdel6@alap3.anarazel.de>
	<20190518093522.GA19212@paquier.xyz>
	<28565.1558196308@sss.pgh.pa.us>
	<20190519004344.GA3653@paquier.xyz>
	<22995.1558278322@sss.pgh.pa.us>

I went back to take a closer look at why the existing regression tests
failed to detect this problem.  As Michael already noted,
src/bin/pg_ctl/t/004_logrotate.pl does try to exercise the logging
collector, and we do have a couple of Windows machines that run the
TAP tests, so how'd it escape notice?

The answer indeed seems to be that it *was* noticed, but Andrew failed
to realize what he was seeing:

Author: Andrew Dunstan <andrew@dunslane.net>
Branch: master [d611175e5] 2019-03-03 18:19:44 -0500

    Don't do pg_ctl logrotate test on Windows
    
    The test crashes and burns quite badly, for some reason, but even if it
    didn't it wouldn't work, since Windows doesn't let you rename a file
    held by a running process.

But even when we do run it, it fails to detect any problem, because
it's only testing for the existence of a log file not whether anything
ever gets written into it.  As it happens, SysLogger_Start() creates
the initial log file right in the postmaster (to validate the logging
parameters), so as 004_logrotate.pl is constituted it fails to prove
that the syslogger process got launched at all.  That's why it
"passes" even when the syslogger is crashing instantly on launch.

Given the current unpleasantness, I think it's quite important that
we fix 004_logrotate.pl so that it (a) poses an end-to-end test
and (b) works on Windows.  I think (a) could be managed by having
the test execute guaranteed-to-fail SQL commands ("select 1/0" or
the like) and making it look into the log file to verify that the
expected error messages appear there.  (b) is a little harder in
view of Andrew's point that we can't just summarily rename or unlink
the log file.  But I don't much like using a fixed log file name
in the test anyway.  I propose that we let it use the default logfile
pattern (postgresql-%Y-%m-%d_%H%M%S.log), and that we just sleep for a
couple of seconds before requesting rotation, and that we verify that
the file name actually changed.  We can avoid difficult issues of
guessing what file name got used by having the test script look into
"current_logfiles" to get the name --- which is another feature that
there's exactly zero test coverage of right now, so that seems like
a win all around.

Thoughts, better ideas?

			regards, tom lane





view thread (40+ messages)  latest in thread

Message-ID: <27719.1558282181@sss.pgh.pa.us>
Permalink:  ../27719.1558282181@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/27719.1558282181@sss.pgh.pa.us

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-bugs@postgresql.org
  Cc: tgl@sss.pgh.pa.us, michael@paquier.xyz, andrew@dunslane.net, andres@anarazel.de, peter.eisentraut@2ndquadrant.com, yuli.khodorkovskiy@crunchydata.com, pgsql-bugs@lists.postgresql.org
  Subject: Re: BUG #15804: Assertion failure when using logging_collector with EXEC_BACKEND
  In-Reply-To: <27719.1558282181@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox