Re: [HACKERS] TAP backpatching policy

[Craig Ringer]

On 1 June 2017 at 01:13, Tom Lane  wrote:
> Alvaro Herrera  writes:
>> My main concern is how widely is the buildfarm going to test the new
>> test frameworks.  If we backpatch PostgresNode-based tests to 9.2, are
>> buildfarm animals going to need to be reconfigured to use
>> --enable-tap-tests?
>
> Yes.  The animals that are doing it at all are using code more or less
> like this:
>
> if ($branch eq 'HEAD' or $branch ge 'REL9_4')
> {
> push(@{$conf{config_opts}},"--enable-tap-tests");
> }
>
> (verbatim from longfin's config script)
>
> So maybe that's an argument for not going back before 9.4.  OTOH,
> you may be giving the buildfarm owners too little credit for
> willingness to update their configurations.

Honestly, it didn't occur to me to back-patch past the introduction of
TAP in 9.4. I was more thinking of bringing that up to current
functionality, and trying to maintain that in future so that TAP tests
in extensions, test scripts for bugs, etc could be easily used on all
back branches.

I don't have a particular objection to doing so, but initially I was
really aiming for bringing 9.5 and 9.6 up to pg10 level, since
PostgresNode is already present in 9.5 so it's a much simpler target
for backporting the pg10 stuff. Then maintaining from there going
forward, so by the time pg12 is out, everything has solid and pretty
consistent TAP infrastructure.

I'm not too fussed if everyone decides it's all too hard / not worth
it. I'll just extract src/test/modules into a separate github repo.
For use in extensions I'll teach it how to overwrite the stock
PostgresNode etc in a Pg install tree. For use for in-tree testing
it'd have a Makefile that finds and clobbers the in-tree
PostgresNode.pm etc. So it's a hassle, but not the end of the world.

I just suspect we'll all benefit from making it easier to write tests
that work across more releases, and that updating the test modules in
back branches isn't an unduly invasive thing to do.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, 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] TAP backpatching policy

[Joshua D. Drake]

On 05/31/2017 07:49 AM, Robert Haas wrote:

On Tue, May 30, 2017 at 9:12 PM, Craig Ringer  wrote:

Thoughts? Backpatch new TAP methods, etc, into back branches routinely?


[...]


 When customers start to doubt that, then they
become reluctant to apply new minor versions in their entirety and ask
for individual commits to be cherry-picked, or just don't upgrade at
all. 


This may be true, on the other hand that isn't .Org's problem. Customers 
are CMD, EDB, 2Qs problem. .Org's problem is: How do we ensure the best 
possible result for PostgreSQL.


I think comprehensive testing (which I am sure you agree) is bullet 
point on that list.



One could argue that commits to the testing framework shouldn't
make customers nervous, but what people should be worried about and
what they are actually worried about do not always match.  It is
useful to be able to show a customer a list of things that were done
in a minor release and see nothing in there that can be described as
optional tinkering.


This is about narrative. You don't say "optional tinkering". You say, 
"Initiate code modification for comprehensive TAP (testing) framework".


That makes customers knees weak and they swoon.


back-patching (to avoid churning a supposedly-stable branch).  I'm not
sure exactly what I think about this issue, but I'm very skeptical of
the idea that back-patching less conservatively will have no
downsides.


There is never not a downside. The question is, "Does the upside 
outweigh the downside?". In this case I would argue it is fairly obvious.


Thanks,

JD



--
Command Prompt, Inc.  http://the.postgres.company/
+1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.
Unless otherwise stated, opinions are my own.


--
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] TAP backpatching policy

[Stephen Frost]

Tom, Alvaro,

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Alvaro Herrera  writes:
> > My main concern is how widely is the buildfarm going to test the new
> > test frameworks.  If we backpatch PostgresNode-based tests to 9.2, are
> > buildfarm animals going to need to be reconfigured to use
> > --enable-tap-tests?
> 
> Yes.  The animals that are doing it at all are using code more or less
> like this:
> 
> if ($branch eq 'HEAD' or $branch ge 'REL9_4')
> {
> push(@{$conf{config_opts}},"--enable-tap-tests");
> }
> 
> (verbatim from longfin's config script)
> 
> So maybe that's an argument for not going back before 9.4.  OTOH,
> you may be giving the buildfarm owners too little credit for
> willingness to update their configurations.

I'm certainly on the optomistic side of the equation here when it comes
to buildfarm owners.  Generally speaking, I've seen them be pretty
reasonably responsive when asked to make a change or update something,
and a lot of them are also regular PG contributors, but even those who
aren't seem to take the buildfarm seriously and I expect an email going
out to them would certainly have a majority positive response.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] TAP backpatching policy

[Stephen Frost]

Robert,

* Robert Haas (robertmh...@gmail.com) wrote:
> On the other hand, we tell our users that we only back-patch security
> and stability fixes.  

Perhaps I missed where this changed, but my recollection is that we also
back-patch bug-fixes, and I don't think we should change that.

> When customers start to doubt that, then they
> become reluctant to apply new minor versions in their entirety and ask
> for individual commits to be cherry-picked, or just don't upgrade at
> all.  One could argue that commits to the testing framework shouldn't
> make customers nervous, but what people should be worried about and
> what they are actually worried about do not always match.  It is
> useful to be able to show a customer a list of things that were done
> in a minor release and see nothing in there that can be described as
> optional tinkering.

If the perception issue is that customers aren't comfortable with
changes being made in the main repo which are due to adding testing then
we should consider having an independent test repo which is also able to
be run through the buildfarm.  Perhaps we would be able to consider
having a more relaxed policy around that repository also, along with a
way to distinguish failures on the main repo vs. failures from this
other repo (where the issue might be the test itself rather than an
issue in the code, or it might be an issue in the code that the test
exposed but isn't yet fixed, but allows people to see that their
independent changes on the main repo aren't what caused the buildfarm to
turn red).

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] TAP backpatching policy

[Tom Lane]

Alvaro Herrera  writes:
> My main concern is how widely is the buildfarm going to test the new
> test frameworks.  If we backpatch PostgresNode-based tests to 9.2, are
> buildfarm animals going to need to be reconfigured to use
> --enable-tap-tests?

Yes.  The animals that are doing it at all are using code more or less
like this:

if ($branch eq 'HEAD' or $branch ge 'REL9_4')
{
push(@{$conf{config_opts}},"--enable-tap-tests");
}

(verbatim from longfin's config script)

So maybe that's an argument for not going back before 9.4.  OTOH,
you may be giving the buildfarm owners too little credit for
willingness to update their configurations.

regards, tom lane


-- 
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] TAP backpatching policy

[Joshua D. Drake]

On 05/30/2017 09:52 PM, Stephen Frost wrote:

Tom,



Um ... but we still have 2 live pre-9.4 branches.  If your proposal
doesn't extend to back-porting all of this stuff as far as 9.2,
I don't see what this is really buying.  We'd still need version cutoff
checks in the tests.


I don't believe the explicit goal of this is to remove the version
checks but rather to provide improved testing coverage in our
back-branches.  If we have to keep a version cutoff check for that, so
be it.


+1




(If you *do* propose back-patching all this stuff as far as 9.2, I'm not
quite sure what I'd think about that.  But the proposal as stated seems
like questionable half measures.)


I find that to be an extremely interesting idea, for my own 2c, but I'm
not sure how practical it is.
It is perfectly reasonable to say, "We have added comprehensive testing 
through the TAP project. Unfortunately, it is only reasonable (due to 
code changes) to back port them from 9.4 and above."


*IF* we were to try to go back farther than 9.4, I would say that 9.3 is 
the only one that would be worth it anyway. 9.2 was a great release but 
it's day is over or at least it is a spectacle of a setting sun:


https://www.postgresql.org/support/versioning/

Thanks,

JD




--
Command Prompt, Inc.  http://the.postgres.company/
+1-503-667-4564
PostgreSQL Centered full stack support, consulting and development.
Everyone appreciates your honesty, until you are honest with them.
Unless otherwise stated, opinions are my own.


--
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] TAP backpatching policy

[Alvaro Herrera]

Robert Haas wrote:
> On Tue, May 30, 2017 at 9:12 PM, Craig Ringer  wrote:
> > Thoughts? Backpatch new TAP methods, etc, into back branches routinely?
> 
> So, on the one hand, it is certainly useful to be able to commit tests
> to back-branches as well as to master, and it's hard to do that if the
> infrastructure isn't there.

Agreed.

> On the other hand, we tell our users that we only back-patch security
> and stability fixes.

I think being strict about what we backpatch for the production code is
a valued principle.  I am not sure that not backpatching new
test-harness features is valued in the same way.  One possible problem
is that the new tests cause test failures, and thus failure to create
packages for some platforms.  That would be a net loss.  But it won't
corrupt your data and it won't make your applications incompatible.

> It is useful to be able to show a customer a list of things that were
> done in a minor release and see nothing in there that can be described
> as optional tinkering.

In my experience, some customers are going to take our word that we've
done a good job keeping the branch clean of unnecessary changes, and
others are going to cherry-pick individual fixes regardless of what's in
there.  The percentages in each group might or might not change slightly
if they see some new Perl test code, but I doubt it'd change
dramatically.

My main concern is how widely is the buildfarm going to test the new
test frameworks.  If we backpatch PostgresNode-based tests to 9.2, are
buildfarm animals going to need to be reconfigured to use
--enable-tap-tests?  (9.2 and 9.3 do not have --enable-tap-tests) If the
old branches need to be reconfigured, then the tests are not going to
run on a majority of animals, and that is going to cause pain on obscure
platforms that three months from now we figure didn't get any buildfarm
coverage.  But if they start picking up the new tests without need for
human intervention, then the coverage should be decent enough that it
shouldn't be a problem.  So my vote is that if a majority of buildfarm
animals pick up PostgresNode tests without reconfiguration, +1 for
backpatching; otherwise -1.

Being unable to backpatch tests for bugfixes is a net loss, IMO.

-- 
Álvaro Herrerahttps://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] TAP backpatching policy

[Robert Haas]

On Tue, May 30, 2017 at 9:12 PM, Craig Ringer  wrote:
> Thoughts? Backpatch new TAP methods, etc, into back branches routinely?

So, on the one hand, it is certainly useful to be able to commit tests
to back-branches as well as to master, and it's hard to do that if the
infrastructure isn't there.

On the other hand, we tell our users that we only back-patch security
and stability fixes.  When customers start to doubt that, then they
become reluctant to apply new minor versions in their entirety and ask
for individual commits to be cherry-picked, or just don't upgrade at
all.  One could argue that commits to the testing framework shouldn't
make customers nervous, but what people should be worried about and
what they are actually worried about do not always match.  It is
useful to be able to show a customer a list of things that were done
in a minor release and see nothing in there that can be described as
optional tinkering.

The TAP tests seem to be evolving incredibly quickly, with new
infrastructure being built out all the time.  That strengths both the
argument for back-patching (to keep things in sync) and for not
back-patching (to avoid churning a supposedly-stable branch).  I'm not
sure exactly what I think about this issue, but I'm very skeptical of
the idea that back-patching less conservatively will have no
downsides.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
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] TAP backpatching policy

[Stephen Frost]

Tom,

On Wed, May 31, 2017 at 01:16 Tom Lane  wrote:

> Stephen Frost  writes:
> > In the end, the experiences I've had with pg_dump of late and trying to
> > ensure that pg_dump 9.6 is able to work all the way back to *7.0*, makes
> > me think that this notion of putting the one-and-only real test-suite we
> > have into the core repo almost laughable.
>
> um... why are you trying to do that?  We moved the goalposts last fall:
>
> Author: Tom Lane 
> Branch: master [64f3524e2] 2016-10-12 12:20:02 -0400
>
> Remove pg_dump/pg_dumpall support for dumping from pre-8.0 servers.


Right... for v10. Last I checked we still wish for pg_dump 9.6 to work
against 7.0, as I mention in the portion you quote above.  I agree that we
don't need to ensure that v10 pg_dump works against 7.0, which is helpful,
certainly, but there's still a lot left on the floor in the back branches
as it relates to code coverage and the buildfarm.

Thanks!

Stephen

Re: [HACKERS] TAP backpatching policy

[Tom Lane]

Stephen Frost  writes:
> In the end, the experiences I've had with pg_dump of late and trying to
> ensure that pg_dump 9.6 is able to work all the way back to *7.0*, makes
> me think that this notion of putting the one-and-only real test-suite we
> have into the core repo almost laughable.

um... why are you trying to do that?  We moved the goalposts last fall:

Author: Tom Lane 
Branch: master [64f3524e2] 2016-10-12 12:20:02 -0400

Remove pg_dump/pg_dumpall support for dumping from pre-8.0 servers.

regards, tom lane


-- 
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] TAP backpatching policy

[Stephen Frost]

Tom,

* Tom Lane (t...@sss.pgh.pa.us) wrote:
> Stephen Frost  writes:
> > * Craig Ringer (cr...@2ndquadrant.com) wrote:
> >> At the moment that'd be 9.5, since that's where PostgresNode was
> >> introduced. But if I can find the time I'd quite like to backport
> >> PostgresNode to 9.4 too.
> 
> > Makes sense to me.
> 
> Um ... but we still have 2 live pre-9.4 branches.  If your proposal
> doesn't extend to back-porting all of this stuff as far as 9.2,
> I don't see what this is really buying.  We'd still need version cutoff
> checks in the tests.

I don't believe the explicit goal of this is to remove the version
checks but rather to provide improved testing coverage in our
back-branches.  If we have to keep a version cutoff check for that, so
be it.

> (If you *do* propose back-patching all this stuff as far as 9.2, I'm not
> quite sure what I'd think about that.  But the proposal as stated seems
> like questionable half measures.)

I find that to be an extremely interesting idea, for my own 2c, but I'm
not sure how practical it is.

Based on all of the feedback and discussion, I'm really inclined to
suggest that we support an alternative testing structure to the in-repo
regression suite.  Such a testing structure would allow us to have,
perhaps, somewhat longer running tests than what developers run
typically (frankly, we already have this, but we have perversely decided
that it's "ok" when it's a configure option or a #define, but seemingly
not otherwise), or tests built in a framework which simply didn't exist
at the time of the major release which is being tested (such as the TAP
tests), but wouldn't also force the burden of supporting those tests on
our packagers (which has the other advantage of avoiding pushing new
requirements on those packagers, which might be quite difficult to
fulfill).

In the end, the experiences I've had with pg_dump of late and trying to
ensure that pg_dump 9.6 is able to work all the way back to *7.0*, makes
me think that this notion of putting the one-and-only real test-suite we
have into the core repo almost laughable.  We aren't going to back-patch
things into 7.0, nor are we likely to run 7.0 across all members of the
buildfarm, so how are we to test the combinations which we claim to
support?  On one-off builds/installs on individual developer systems
with, in some cases, hacked-up versions of PG (just to get them to
build!).  Is that really testing what we're worried about?  Perhaps in a
few cases, but I've little doubt that any serious 7.0-era deployment is
going to require more effort to migrate forward than running a 9.6
pg_dump against it.

That does push back a bit on if we should really be trying to support
such ancient versions, but I have to admit that I'm as much of a pureist
as anyone in that regard and I'd really hate to drop support for those
older branches too, at least for pg_dump, since that's how one moves
forward.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] TAP backpatching policy

[Tom Lane]

Stephen Frost  writes:
> * Craig Ringer (cr...@2ndquadrant.com) wrote:
>> At the moment that'd be 9.5, since that's where PostgresNode was
>> introduced. But if I can find the time I'd quite like to backport
>> PostgresNode to 9.4 too.

> Makes sense to me.

Um ... but we still have 2 live pre-9.4 branches.  If your proposal
doesn't extend to back-porting all of this stuff as far as 9.2,
I don't see what this is really buying.  We'd still need version cutoff
checks in the tests.

(If you *do* propose back-patching all this stuff as far as 9.2, I'm not
quite sure what I'd think about that.  But the proposal as stated seems
like questionable half measures.)

regards, tom lane


-- 
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] TAP backpatching policy

[Stephen Frost]

Craig,

* Craig Ringer (cr...@2ndquadrant.com) wrote:
> I propose that we backpatch all practical TAP enhancements back to the
> last supported back branch.

Thanks for bringing this up.  I tend to agree, as we really want to have
better testing of PostgreSQL in all of our branches and being able to
back-patch tests, and add more tests to all of the branches where those
tests apply, will help a great deal with that.

> At the moment that'd be 9.5, since that's where PostgresNode was
> introduced. But if I can find the time I'd quite like to backport
> PostgresNode to 9.4 too.

Makes sense to me.

> Where needed, PostgresNode could have version-dependent logic so we
> could just copy the Perl module to the different branches.

Agreed.

> This would make it much easier to test for bugs when doing work on
> back branches, run the same test on multiple branches, etc. My
> personal motivation is mainly using it in extensions, but I've
> _frequently_ found myself writing TAP tests when looking into possible
> Pg bugs etc too. Not having the same facilities in the different
> branches makes this way harder.

I also find it really unfortunate to write a set of tests but then not
be able to have them included and run across the buildfarm.  As much as
I'd like to think that testing locally identifies all issues, I've been
proven wrong enough to realize that it's really unlikely to ever be
true.

> If combined with pg_config --version-num (which IMO deserves a
> backpatch especially now Pg 10 makes parsing our versions harder) this
> would make it a lot more practical to do nontrivial tests in
> extensions - which really matters since we introduced bgworkers.

Presumably you mean nontrivial tests of extensions in *back-branches*,
here?  Or am I misunderstanding what you're getting at?

> Thoughts? Backpatch new TAP methods, etc, into back branches routinely?

For my 2c, at least, I'd like to see the project, in general, be much
more accepting of adding tests to all branches, and, really, to
encourage that the focus of the time between feature-freeze and release
be specifically on improving our testing code coverage, including the
coverage in back branches, as well as the to-be-released version.

Perhaps, one day in the far future, we will be able to look back in
amusement at the lack of code coverage we have these days, but we have
to really work to get there and that means setting aside some period of
dedicated time during the year for writing tests, in my opinion, and not
being overly picky about if those tests are adding code coverage for
code committed in Postgres95 or for PG10.  I would have thought that the
extent of the back-patching I've been doing of late for issues found in
pg_dump, back to the initial implementation of those parts, would point
out this need, but I'm afraid that there continues to be a general
feeling that "what we have is good enough" and I find that a very hard
pill to swallow when there are extremely important components of the
system which are, essentially, entirely untested in the buildfarm, such
as:

https://coverage.postgresql.org/src/backend/access/brin/brin_xlog.c.gcov.html
https://coverage.postgresql.org/src/backend/access/common/bufmask.c.gcov.html
https://coverage.postgresql.org/src/backend/access/gin/ginxlog.c.gcov.html
https://coverage.postgresql.org/src/backend/access/gist/gistbuildbuffers.c.gcov.html
https://coverage.postgresql.org/src/backend/access/gist/gistxlog.c.gcov.html
https://coverage.postgresql.org/src/backend/access/hash/hash_xlog.c.gcov.html
https://coverage.postgresql.org/src/backend/access/nbtree/nbtxlog.c.gcov.html
https://coverage.postgresql.org/src/backend/access/spgist/spgxlog.c.gcov.html
https://coverage.postgresql.org/src/backend/access/transam/xlogfuncs.c.gcov.html
https://coverage.postgresql.org/src/backend/executor/nodeCustom.c.gcov.html
https://coverage.postgresql.org/src/backend/executor/tqueue.c.gcov.html

Let's. please. work together to correct this.

Thanks!

Stephen


signature.asc
Description: Digital signature

Re: [HACKERS] TAP backpatching policy

[Michael Paquier]

On Tue, May 30, 2017 at 6:12 PM, Craig Ringer  wrote:
> Thoughts? Backpatch new TAP methods, etc, into back branches routinely?

FWIW, I'd love to see a committer helping into getting this facility
back-patched at least to 9.5. The least I can do is a commitment as a
reviewer of such a patch. So count me in to help out.
-- 
Michael


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] TAP backpatching policy

[Craig Ringer]

Hi all

I propose that we backpatch all practical TAP enhancements back to the
last supported back branch.

At the moment that'd be 9.5, since that's where PostgresNode was
introduced. But if I can find the time I'd quite like to backport
PostgresNode to 9.4 too.

Where needed, PostgresNode could have version-dependent logic so we
could just copy the Perl module to the different branches.

This would make it much easier to test for bugs when doing work on
back branches, run the same test on multiple branches, etc. My
personal motivation is mainly using it in extensions, but I've
_frequently_ found myself writing TAP tests when looking into possible
Pg bugs etc too. Not having the same facilities in the different
branches makes this way harder.

If combined with pg_config --version-num (which IMO deserves a
backpatch especially now Pg 10 makes parsing our versions harder) this
would make it a lot more practical to do nontrivial tests in
extensions - which really matters since we introduced bgworkers.

Thoughts? Backpatch new TAP methods, etc, into back branches routinely?

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, 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