Thread: [HACKERS] Coverage improvements of src/bin/pg_basebackup and pg_receivewal --endpos
[HACKERS] Coverage improvements of src/bin/pg_basebackup and pg_receivewal --endpos
From
Michael Paquier
Date:
Hi all, The coverage of pg_basebackup is reaching 50%, which is not bad: https://coverage.postgresql.org/src/bin/pg_basebackup/index.html In this set pg_receivewal.c is the bad student with less than 20%. There are a couple of causes behind that, with no tests like: - option interactions like --create-slot and --drop-slot. - replication slot drop. - end of streaming. - check handling of history files. - looking at compressed and uncompressed WAL data. - restart of pg_receivewal on an existing folder. - pg_basebackup's tests don't use the progress report. While it is possible to tackle some of those issues independently, like pg_basebackup stuff, it seems to me that it would be helpful to make the tests more deterministic by having an --endpos option for pg_receivewal, similarly to what exists already in pg_recvlogical. Thoughts? -- Michael
Re: [HACKERS] Coverage improvements of src/bin/pg_basebackup and pg_receivewal --endpos
From
Michael Paquier
Date:
On Wed, Jun 7, 2017 at 9:20 AM, Michael Paquier <michael.paquier@gmail.com> wrote: > While it is possible to tackle some of those issues independently, > like pg_basebackup stuff, it seems to me that it would be helpful to > make the tests more deterministic by having an --endpos option for > pg_receivewal, similarly to what exists already in pg_recvlogical. > > Thoughts? I have just played with that, and attached is a patch to implement the so-said option with a basic set of tests, increasing code coverage of pg_receivewal.c from 15% to 60%. I'll park that in the next CF of September. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
Re: [HACKERS] Coverage improvements of src/bin/pg_basebackup andpg_receivewal --endpos
From
Peter Eisentraut
Date:
On 6/9/17 02:08, Michael Paquier wrote: > On Wed, Jun 7, 2017 at 9:20 AM, Michael Paquier > <michael.paquier@gmail.com> wrote: >> While it is possible to tackle some of those issues independently, >> like pg_basebackup stuff, it seems to me that it would be helpful to >> make the tests more deterministic by having an --endpos option for >> pg_receivewal, similarly to what exists already in pg_recvlogical. >> >> Thoughts? > > I have just played with that, and attached is a patch to implement the > so-said option with a basic set of tests, increasing code coverage of > pg_receivewal.c from 15% to 60%. I'll park that in the next CF of > September. Looks like a good idea. A couple of thoughts: - I think the tests should be written in the style of $node->command_fails instead of just command_fails. At least, that's how it's done in the pg_basebackup tests. - I think the tests will fail if libz support is not built. Again, pg_basebackup doesn't have tests for that. So maybe omit that and address it later. - The variable exit_on_stop seems redundant with time_to_abort. They do the same thing, so maybe your patch could reuse it. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Re: [HACKERS] Coverage improvements of src/bin/pg_basebackup andpg_receivewal --endpos
From
Michael Paquier
Date:
On Tue, Sep 5, 2017 at 10:19 PM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > On 6/9/17 02:08, Michael Paquier wrote: >> I have just played with that, and attached is a patch to implement the >> so-said option with a basic set of tests, increasing code coverage of >> pg_receivewal.c from 15% to 60%. I'll park that in the next CF of >> September. > > Looks like a good idea. A couple of thoughts: Thanks. > - I think the tests should be written in the style of > $node->command_fails instead of just command_fails. At least, that's > how it's done in the pg_basebackup tests. Good idea. > - I think the tests will fail if libz support is not built. Again, > pg_basebackup doesn't have tests for that. So maybe omit that and > address it later. Let's address it now. This can be countered by querying pg_config() before running the command of pg_receivexwal which uses --compress. > - The variable exit_on_stop seems redundant with time_to_abort. They do > the same thing, so maybe your patch could reuse it. Yes, that's doable. time_to_abort does not seem a variable name adapted to me though if both are combined, so I have renamed that to time_to_stop, which maps more with the fact that stop can be also willingly wanted without a SIGINT. Attached is a new version of the patch. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Attachment
Re: [HACKERS] Coverage improvements of src/bin/pg_basebackup andpg_receivewal --endpos
From
Peter Eisentraut
Date:
On 9/6/17 00:54, Michael Paquier wrote: >> - I think the tests will fail if libz support is not built. Again, >> pg_basebackup doesn't have tests for that. So maybe omit that and >> address it later. > > Let's address it now. This can be countered by querying pg_config() > before running the command of pg_receivexwal which uses --compress. I have committed this patch, after a perltidy run, but the way the libz detection was implemented was a bit too hackish for me, so I have omitted that for now. I think a more robust way would be to parse Makefile.global, perhaps by a function in TestLib, so it can be reused in other tests. (Even more robust would be to write out an actual Perl file with configuration information somewhere, but maybe we don't need to go there yet.) Or maybe have the respective make variables exported when make check is called (could be included in the prove_check variable?). Anyway, some more pondering could lead to a more general solution. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Coverage improvements of src/bin/pg_basebackup andpg_receivewal --endpos
From
Michael Paquier
Date:
On Tue, Sep 12, 2017 at 5:57 AM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > I have committed this patch, after a perltidy run, but the way the libz > detection was implemented was a bit too hackish for me, so I have > omitted that for now. Thanks. > I think a more robust way would be to parse > Makefile.global, perhaps by a function in TestLib, so it can be reused > in other tests. (Even more robust would be to write out an actual Perl > file with configuration information somewhere, but maybe we don't need > to go there yet.) Or maybe have the respective make variables exported > when make check is called (could be included in the prove_check > variable?). Anyway, some more pondering could lead to a more general > solution. There is always room for improvement, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Coverage improvements of src/bin/pg_basebackup andpg_receivewal --endpos
From
Peter Eisentraut
Date:
On 9/11/17 18:21, Michael Paquier wrote: > On Tue, Sep 12, 2017 at 5:57 AM, Peter Eisentraut > <peter.eisentraut@2ndquadrant.com> wrote: >> I have committed this patch, after a perltidy run, but the way the libz >> detection was implemented was a bit too hackish for me, so I have >> omitted that for now. > > Thanks. > >> I think a more robust way would be to parse >> Makefile.global, perhaps by a function in TestLib, so it can be reused >> in other tests. (Even more robust would be to write out an actual Perl >> file with configuration information somewhere, but maybe we don't need >> to go there yet.) Or maybe have the respective make variables exported >> when make check is called (could be included in the prove_check >> variable?). Anyway, some more pondering could lead to a more general >> solution. > > There is always room for improvement, I interpret that as that you are not working on that right now, so I've closed this patch. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Coverage improvements of src/bin/pg_basebackup andpg_receivewal --endpos
From
Michael Paquier
Date:
On Tue, Sep 12, 2017 at 9:19 PM, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote: > On 9/11/17 18:21, Michael Paquier wrote: >> On Tue, Sep 12, 2017 at 5:57 AM, Peter Eisentraut >> <peter.eisentraut@2ndquadrant.com> wrote: >>> I think a more robust way would be to parse >>> Makefile.global, perhaps by a function in TestLib, so it can be reused >>> in other tests. (Even more robust would be to write out an actual Perl >>> file with configuration information somewhere, but maybe we don't need >>> to go there yet.) Or maybe have the respective make variables exported >>> when make check is called (could be included in the prove_check >>> variable?). Anyway, some more pondering could lead to a more general >>> solution. >> >> There is always room for improvement, > > I interpret that as that you are not working on that right now, so I've > closed this patch. Indeed, I have no plans for that now. There are enough patches to look at these days. My apologies for not making that clear. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers