Re: incorrect xlog.c coverage report

On 2018-Nov-22, Michael Paquier wrote:

> On Thu, Nov 22, 2018 at 10:56:39AM +0900, Masahiko Sawada wrote:
> > On Thu, Nov 22, 2018 at 10:43 AM Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com> wrote:
> >> Presumably you could add your own call to __gcov_flush() in
> >> quickdie(), so that we get GCOV data but no other atexit()-like stuff.
> >> I see that some people advocate doing that in signal handlers, but I
> >> don't know if it's really safe. If that is somehow magically OK,
> >> you'd probably also need the chdir() hack from proc_exit() to get
> >> per-pid files.
> >
> > That's probably a good idea, I'm also not sure if it's really safe
> > though. An alternative approach could be that we can do $node->restart
> > after recovered from $node->teardown_node() to write gcda file surely,
> > although it would make the tests hard to read.
>
> Thanks for looking at the details around that. I'd prefer much if we
> have a solution like what's outline here because we should really try to
> have coverage even for code paths which involve an immediate shutdown
> (mainly for recovery). Manipulating the tests to get a better coverage
> feels more like a band-aid solution, and does not help folks with custom
> TAP tests in their plugins.

On the contrary, I think we shouldn't mess with the exit sequence.
Today we have three shutdown modes -- smart, fast, immediate. If we add
stuff to the exit sequence of the immediate mode, we have four shutdown
modes: those three, plus an actual server crash which would be different
from immediate. I'd rather not do that, because we'll then grow a
totally untested code path.

Anyway I now think this problem can be fixed by careful changing of
teardown_node() into stop('fast') in some places. The places for which
it actually matters that a shutdown is immediate are not really
interested with the code that executes in the server that shuts down --
they are interested in the code run by the server that *doesn't* shut
down (the replica), or the server after it restarts (and which we can
shut down cleanly afterwards). No need to mess with the backend exit
code path ISTM.

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services