Horton

Thread

Commits

  1. Add filtering capability for cross-version pg_upgrade tests

  1. Improve TAP tests of pg_upgrade for cross-version tests

    Michael Paquier <michael@paquier.xyz> — 2022-05-24T06:03:28Z

    Hi all,
(Adding Andrew Dunstan in CC.)

I have been toying with $subject, trying to improve the ways to test
pg_upgrade across different major versions as perl makes that easier.
The buildfarm does three things to allow such tests to work (see
TestUpgradeXversion.pm):
- Apply a filter to the dumps generated to make them perfectly equal
as the set of regexps on the SQL dumps (removal of empty lines and
comments in the SQL dumps as arguments of a diff command).
- Apply diffs dumps to remove or modify objects, to avoid
inconsistencies, which is what upgrade_adapt.sql does in the tree.
- Add tweaks to the dump commands used, like --extra-float-digits=0
when testing with a version <= 11 as origin (aka c6f9464b in the
buildfarm client).

Attached is a basic patch to show what can be used to improve the TAP
tests of pg_upgrade in this area, with contents coming mostly from the
buildfarm client.  The end picture would be to allow all those tests
to use the core code, rather than duplicating that in the buildfarm
client.  This reduces a lot the amount of noise that can be seen when
comparing the dumps taken (the tests pass with v14) while remaining
simple, down to v11, so that could be a first step.  There are a
couple of things where I am not sure how the buildfarm handles things,
but perhaps the dumps of installcheck have been tweaked to ignore such
cases?  Here is an exhaustive list:
- multirange_type_name when using PG <= v13 as origin, for CREATE
TYPE.
- CREATE/ALTER PROCEDURE append IN to the list of parameters dumped,
when using PG <= v13 as origin.
- CREATE OPERATOR CLASS and ALTER OPERATOR FAMILY, where FUNCTION 2 is
moved from one command to the other.

Thoughts?
--
Michael

  2. Re: Improve TAP tests of pg_upgrade for cross-version tests

    Michael Paquier <michael@paquier.xyz> — 2022-05-25T01:35:57Z

    On Tue, May 24, 2022 at 03:03:28PM +0900, Michael Paquier wrote:
> (Adding Andrew Dunstan in CC.)
> 
> I have been toying with $subject, trying to improve the ways to test
> pg_upgrade across different major versions as perl makes that easier.
> The buildfarm does three things to allow such tests to work (see
> TestUpgradeXversion.pm):

And with Andrew in CC, that's better ;p
--
Michael

  3. Re: Improve TAP tests of pg_upgrade for cross-version tests

    Michael Paquier <michael@paquier.xyz> — 2022-06-09T07:49:01Z

    On Wed, May 25, 2022 at 10:35:57AM +0900, Michael Paquier wrote:
> On Tue, May 24, 2022 at 03:03:28PM +0900, Michael Paquier wrote:
>> (Adding Andrew Dunstan in CC.)
>> 
>> I have been toying with $subject, trying to improve the ways to test
>> pg_upgrade across different major versions as perl makes that easier.
>> The buildfarm does three things to allow such tests to work (see
>> TestUpgradeXversion.pm):

Rebased to cope with the recent changes in this area.
--
Michael

  4. Re: Improve TAP tests of pg_upgrade for cross-version tests

    Andrew Dunstan <andrew@dunslane.net> — 2022-06-12T14:14:35Z

    On 2022-06-09 Th 03:49, Michael Paquier wrote:
> On Wed, May 25, 2022 at 10:35:57AM +0900, Michael Paquier wrote:
>> On Tue, May 24, 2022 at 03:03:28PM +0900, Michael Paquier wrote:
>>> (Adding Andrew Dunstan in CC.)
>>>
>>> I have been toying with $subject, trying to improve the ways to test
>>> pg_upgrade across different major versions as perl makes that easier.
>>> The buildfarm does three things to allow such tests to work (see
>>> TestUpgradeXversion.pm):
> Rebased to cope with the recent changes in this area.


I tried in fb16d2c658 to avoid littering the mainline code with
version-specific tests, and put that in the methods in the subclasses
that override the mainline functions.

I'll try to rework this along those lines.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  5. Re: Improve TAP tests of pg_upgrade for cross-version tests

    Andrew Dunstan <andrew@dunslane.net> — 2022-06-12T21:58:54Z

    On 2022-06-12 Su 10:14, Andrew Dunstan wrote:
> On 2022-06-09 Th 03:49, Michael Paquier wrote:
>> On Wed, May 25, 2022 at 10:35:57AM +0900, Michael Paquier wrote:
>>> On Tue, May 24, 2022 at 03:03:28PM +0900, Michael Paquier wrote:
>>>> (Adding Andrew Dunstan in CC.)
>>>>
>>>> I have been toying with $subject, trying to improve the ways to test
>>>> pg_upgrade across different major versions as perl makes that easier.
>>>> The buildfarm does three things to allow such tests to work (see
>>>> TestUpgradeXversion.pm):
>> Rebased to cope with the recent changes in this area.
>
> I tried in fb16d2c658 to avoid littering the mainline code with
> version-specific tests, and put that in the methods in the subclasses
> that override the mainline functions.
>
> I'll try to rework this along those lines.
>
>

I think I must have been insufficiently caffeinated when I wrote this,
because clearly I was not reading correctly.


I have had another look and the patch looks fine, although I haven't
tested it.


cheers


andrew

--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  6. Re: Improve TAP tests of pg_upgrade for cross-version tests

    Michael Paquier <michael@paquier.xyz> — 2022-06-13T07:51:30Z

    On Sun, Jun 12, 2022 at 05:58:54PM -0400, Andrew Dunstan wrote:
> On 2022-06-12 Su 10:14, Andrew Dunstan wrote:
>> I tried in fb16d2c658 to avoid littering the mainline code with
>> version-specific tests, and put that in the methods in the subclasses
>> that override the mainline functions.

Except that manipulating the diffs of pg_upgrade is not something that
needs to be internal to the subclasses where we set up the nodes :)

> I think I must have been insufficiently caffeinated when I wrote this,
> because clearly I was not reading correctly.
> 
> I have had another look and the patch looks fine, although I haven't
> tested it.

I have a question about the tests done in the buildfarm though.  Do
the dumps from the older versions drop some of the objects that cause
the diffs in the dumps?  At which extent is that a dump from
installcheck?
--
Michael

  7. Re: Improve TAP tests of pg_upgrade for cross-version tests

    Andrew Dunstan <andrew@dunslane.net> — 2022-06-13T13:59:01Z

    On 2022-06-13 Mo 03:51, Michael Paquier wrote:
> On Sun, Jun 12, 2022 at 05:58:54PM -0400, Andrew Dunstan wrote:
>> On 2022-06-12 Su 10:14, Andrew Dunstan wrote:
>>> I tried in fb16d2c658 to avoid littering the mainline code with
>>> version-specific tests, and put that in the methods in the subclasses
>>> that override the mainline functions.
> Except that manipulating the diffs of pg_upgrade is not something that
> needs to be internal to the subclasses where we set up the nodes :)
>
>> I think I must have been insufficiently caffeinated when I wrote this,
>> because clearly I was not reading correctly.
>>
>> I have had another look and the patch looks fine, although I haven't
>> tested it.
> I have a question about the tests done in the buildfarm though.  Do
> the dumps from the older versions drop some of the objects that cause
> the diffs in the dumps?  At which extent is that a dump from
> installcheck?


See lines 324..347 (save stage) and 426..586 (upgrade stage) of
<https://github.com/PGBuildFarm/client-code/blob/main/PGBuild/Modules/TestUpgradeXversion.pm>

We save the cluster to be upgraded after all the installcheck stages
have run, so on crake here's the list of databases upgraded for HEAD:


postgres
template1
template0
contrib_regression_pgxml
contrib_regression_bool_plperl
contrib_regression_hstore_plperl
contrib_regression_jsonb_plperl
regression
contrib_regression_redis_fdw
contrib_regression_file_textarray_fdw
isolation_regression
pl_regression_plpgsql
contrib_regression_hstore_plpython3
pl_regression_plperl
pl_regression_plpython3
pl_regression_pltcl
contrib_regression_adminpack
contrib_regression_amcheck
contrib_regression_bloom
contrib_regression_btree_gin
contrib_regression_btree_gist
contrib_regression_citext
contrib_regression_cube
contrib_regression_dblink
contrib_regression_dict_int
contrib_regression_dict_xsyn
contrib_regression_earthdistance
contrib_regression_file_fdw
contrib_regression_fuzzystrmatch
contrib_regression_hstore
contrib_regression__int
contrib_regression_isn
contrib_regression_lo
contrib_regression_ltree
contrib_regression_pageinspect
contrib_regression_passwordcheck
contrib_regression_pg_surgery
contrib_regression_pg_trgm
contrib_regression_pgstattuple
contrib_regression_pg_visibility
contrib_regression_postgres_fdw
contrib_regression_seg
contrib_regression_tablefunc
contrib_regression_tsm_system_rows
contrib_regression_tsm_system_time
contrib_regression_unaccent
contrib_regression_pgcrypto
contrib_regression_uuid-ossp
contrib_regression_jsonb_plpython3
contrib_regression_ltree_plpython3
isolation_regression_summarization-and-inprogress-insertion
isolation_regression_delay_execution
contrib_regression_dummy_index_am
contrib_regression_dummy_seclabel
contrib_regression_plsample
contrib_regression_spgist_name_ops
contrib_regression_test_bloomfilter
contrib_regression_test_extensions
contrib_regression_test_ginpostinglist
contrib_regression_test_integerset
contrib_regression_test_parser
contrib_regression_test_pg_dump
contrib_regression_test_predtest
contrib_regression_test_rbtree
contrib_regression_test_regex
contrib_regression_test_rls_hooks
contrib_regression_test_shm_mq
contrib_regression_rolenames


cheers


andrew





--
Andrew Dunstan
EDB: https://www.enterprisedb.com

  8. Re: Improve TAP tests of pg_upgrade for cross-version tests

    Michael Paquier <michael@paquier.xyz> — 2022-07-06T06:27:28Z

    On Thu, Jun 09, 2022 at 04:49:01PM +0900, Michael Paquier wrote:
> Rebased to cope with the recent changes in this area.

Please find attached an updated version of this patch, where I have
extended the support of the upgrade script down to 9.5 as origin
version, as ~9.4 now fail because of cluster_name, so Cluster.pm does
not support that.  FWIW, I have created an extra set of dumps down to
9.4.

This adds proper handling for initdb --wal-segsize and
--allow-group-access when it comes to v11~, as these options are
missing in ~10.
--
Michael

  9. Re: Improve TAP tests of pg_upgrade for cross-version tests

    Justin Pryzby <pryzby@telsasoft.com> — 2022-07-29T21:15:26Z

    On Wed, Jul 06, 2022 at 03:27:28PM +0900, Michael Paquier wrote:
> On Thu, Jun 09, 2022 at 04:49:01PM +0900, Michael Paquier wrote:
> > Rebased to cope with the recent changes in this area.
> 
> Please find attached an updated version of this patch, where I have
> extended the support of the upgrade script down to 9.5 as origin
> version, as ~9.4 now fail because of cluster_name, so Cluster.pm does
> not support that.  FWIW, I have created an extra set of dumps down to
> 9.4.

This was using the old psql rather than the new one.
Before v10, psql didn't have \if.

I think Cluster.pm should be updated to support the upgrades that upgrade.sh
supported.  I guess it ought to be fixed in v15.

-- 
Justin

  10. Re: Improve TAP tests of pg_upgrade for cross-version tests

    Michael Paquier <michael@paquier.xyz> — 2022-07-30T07:29:16Z

    On Fri, Jul 29, 2022 at 04:15:26PM -0500, Justin Pryzby wrote:
> This was using the old psql rather than the new one.
> Before v10, psql didn't have \if.

        # Note that upgrade_adapt.sql from the new version is used, to
	# cope with an upgrade to this version.
-       $oldnode->command_ok(
+       $newnode->command_ok(
                [
-                       'psql', '-X',
+                       "$newbindir/psql", '-X',

Yeah, you are right here that this psql command should use the one
from the new cluster and connect to the old cluster.  There is no
point in adding $newbindir though as Cluster::_get_env would enforce
PATH to find the correct binary.  I'll look at that in details later.
--
Michael

  11. Re: Improve TAP tests of pg_upgrade for cross-version tests

    Anton A. Melnikov <aamelnikov@inbox.ru> — 2022-07-31T15:22:33Z

    Hello!

On 30.07.2022 10:29, Michael Paquier wrote:
>                  [
> -                       'psql', '-X',
> +                       "$newbindir/psql", '-X',
> 

Found that adding $newbindir to psql gives an error when upgrading from 
versions 14 and below to master when the test tries to run 
upgrade_adapt.sql script:

t/002_pg_upgrade.pl .. 1/?
#   Failed test 'ran adapt script'
#   at t/002_pg_upgrade.pl line 141.

in regress_log_002_pg_upgrade:
# Running: <$newbindir>/psql -X -f 
<$srcdir>/src/bin/pg_upgrade/upgrade_adapt.sql regression
<$newbindir>/psql: symbol lookup error: <$newbindir>/psql: undefined 
symbol: PQmblenBounded

Tests from 15-stable and from itself work as expected.

Question about similar error was here: 
https://www.postgresql.org/message-id/flat/BN0PR20MB3912AA107FA6E90FB6B0A034FD9F9%40BN0PR20MB3912.namprd20.prod.outlook.com 


With best regards,

-- 
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company

  12. Re: Improve TAP tests of pg_upgrade for cross-version tests

    Andres Freund <andres@anarazel.de> — 2022-10-02T17:02:37Z

    Hi,

On 2022-07-29 16:15:26 -0500, Justin Pryzby wrote:
> This was using the old psql rather than the new one.
> Before v10, psql didn't have \if.
> 
> I think Cluster.pm should be updated to support the upgrades that upgrade.sh
> supported.  I guess it ought to be fixed in v15.

This fails tests widely, and has so for a while:
https://cirrus-ci.com/build/4862820121575424
https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/39/3649

Note that it causes timeouts, which end up chewing up a cfbot "slot" for an
hour...

Greetings,

Andres Freund

  13. Re: Improve TAP tests of pg_upgrade for cross-version tests

    Michael Paquier <michael@paquier.xyz> — 2022-10-03T02:23:11Z

    On Sun, Oct 02, 2022 at 10:02:37AM -0700, Andres Freund wrote:
> This fails tests widely, and has so for a while:
> https://cirrus-ci.com/build/4862820121575424
> https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/39/3649
> 
> Note that it causes timeouts, which end up chewing up a cfbot "slot" for an
> hour...

Sorry for kicking the can down the road for a too-long time.  Attached
is an updated patch that I have strimmed down to the minimum that
addresses all the issues I want fixed as of the scope of this thread:
- The filtering of the dumps is reduced to a minimum, removing only
comments and empty lines.
- The configuration of the old and new nodes is tweaked so as it is
abvle to handle upgrade from nodes older than 10.
- pg_dumpall is tweaked to use --extra-float-digits=0 for the old
nodes older than 11 to minimize the amount of diffs generated.

That's quite nice in itself, as it becomes possible to use much more
dump patterns loaded as part of the tests.  More filtering rules could
be used, like the part about procedures and functions that I have sent
in the previous versions, but what I have here is enough to make the
test complete with all the versions supported by Cluster.pm.  I am
thinking to get this stuff applied soon before moving on to the PATH
issue.

There is still one issue reported upthread by Justin about the fact
that we can use unexpected commands depending on the node involved.
For example, something like that would build a PATH so as psql from
the old node is used, not from the new node:
$newnode->command_ok(['psql', '-X', '-f', 'blah.sql']);

So with the current Cluster.pm, we could fail upgrade_adapt.sql if the
version upgraded from does not support psql's \if, and I don't think
that we should use a full path to the binary either.  I am not
completely done analyzing that and this deserves a separate thread, as
it impacts all the commands used in TAP tests manipulating nodes from
multiple versions.
--
Michael