Re: Extend compatibility of PostgreSQL::Test::Cluster
On 3/30/22 01:55, Michael Paquier wrote: > On Tue, Mar 29, 2022 at 05:56:02PM -0400, Andrew Dunstan wrote: >> I'm not sure why this item has been moved to the next CF without any >> discussion I could see on the mailing list. It was always my intention >> to commit it this time, and I propose to do so tomorrow with the comment >> Michael has requested above. The cfbot is still happy with it. > Thanks for taking care of it! Committed. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: Extend compatibility of PostgreSQL::Test::Cluster
On Tue, Mar 29, 2022 at 05:56:02PM -0400, Andrew Dunstan wrote: > I'm not sure why this item has been moved to the next CF without any > discussion I could see on the mailing list. It was always my intention > to commit it this time, and I propose to do so tomorrow with the comment > Michael has requested above. The cfbot is still happy with it. Thanks for taking care of it! -- Michael signature.asc Description: PGP signature
Re: Extend compatibility of PostgreSQL::Test::Cluster
On 1/21/22 09:59, Andrew Dunstan wrote: > On 1/21/22 02:47, Michael Paquier wrote: >> On Tue, Jan 18, 2022 at 06:35:39PM -0500, Andrew Dunstan wrote: >>> Here's a version that does that and removes some recent bitrot. >> I have been looking at the full set of features of Cluster.pm and the >> requirements behind v10 as minimal version supported, and nothing >> really stands out. >> >> + # old versions of walreceiver just set the application name to >> + # `walreceiver' >> >> Perhaps this should mention to which older versions this sentence >> applies? > > > Will do in the next version. FTR it's versions older than 12. > > I'm not sure why this item has been moved to the next CF without any discussion I could see on the mailing list. It was always my intention to commit it this time, and I propose to do so tomorrow with the comment Michael has requested above. The cfbot is still happy with it. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: Extend compatibility of PostgreSQL::Test::Cluster
On 1/21/22 02:47, Michael Paquier wrote: > On Tue, Jan 18, 2022 at 06:35:39PM -0500, Andrew Dunstan wrote: >> Here's a version that does that and removes some recent bitrot. > I have been looking at the full set of features of Cluster.pm and the > requirements behind v10 as minimal version supported, and nothing > really stands out. > > + # old versions of walreceiver just set the application name to > + # `walreceiver' > > Perhaps this should mention to which older versions this sentence > applies? Will do in the next version. FTR it's versions older than 12. cheers andrew -- Andrew Dunstan EDB: https://www.enterprisedb.com
Re: Extend compatibility of PostgreSQL::Test::Cluster
On Tue, Jan 18, 2022 at 06:35:39PM -0500, Andrew Dunstan wrote: > Here's a version that does that and removes some recent bitrot. I have been looking at the full set of features of Cluster.pm and the requirements behind v10 as minimal version supported, and nothing really stands out. + # old versions of walreceiver just set the application name to + # `walreceiver' Perhaps this should mention to which older versions this sentence applies? -- Michael signature.asc Description: PGP signature
Re: Extend compatibility of PostgreSQL::Test::Cluster
On 12/31/21 11:22, Andrew Dunstan wrote:
> On 12/31/21 11:20, Dagfinn Ilmari Mannsåker wrote:
>> Andrew Dunstan writes:
>>
>>> + my $subclass = __PACKAGE__ . "::V_$maj";
>>> + bless $node, $subclass;
>>> + unless ($node->isa(__PACKAGE__))
>>> + {
>>> + # It's not a subclass, so re-bless back into the main
>>> package
>>> + bless($node, __PACKAGE__);
>>> + carp "PostgreSQL::Test::Cluster isn't fully compatible
>>> with version $ver";
>>> + }
>> The ->isa() method works on package names as well as blessed objects, so
>> the back-and-forth blessing can be avoided.
>>
>> my $subclass = __PACKAGE__ . "::V_$maj";
>> if ($subclass->isa(__PACKAGE__))
>> {
>> bless($node, $subclass);
>> }
>> else
>> {
>> carp "PostgreSQL::Test::Cluster isn't fully compatible with
>> version $ver";
>> }
>>
> OK, thanks, will fix in next version.
>
>
Here's a version that does that and removes some recent bitrot.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 7af0f8db13..a5e3d2f159 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -111,6 +111,10 @@ use Scalar::Util qw(blessed);
our ($use_tcp, $test_localhost, $test_pghost, $last_host_assigned,
$last_port_assigned, @all_nodes, $died);
+# the minimum version we believe to be compatible with this package without
+# subclassing.
+our $min_compat = 12;
+
INIT
{
@@ -1018,7 +1022,7 @@ sub enable_streaming
print "### Enabling streaming replication for node \"$name\"\n";
$self->append_conf(
- 'postgresql.conf', qq(
+ $self->_recovery_file, qq(
primary_conninfo='$root_connstr'
));
$self->set_standby_mode();
@@ -1047,7 +1051,7 @@ sub enable_restoring
: qq{cp "$path/%f" "%p"};
$self->append_conf(
- 'postgresql.conf', qq(
+ $self->_recovery_file, qq(
restore_command = '$copy_command'
));
if ($standby)
@@ -1061,6 +1065,8 @@ restore_command = '$copy_command'
return;
}
+sub _recovery_file { return "postgresql.conf"; }
+
=pod
=item $node->set_recovery_mode()
@@ -1246,15 +1252,29 @@ sub new
$node->dump_info;
- # Add node to list of nodes
- push(@all_nodes, $node);
-
$node->_set_pg_version;
- my $v = $node->{_pg_version};
+ my $ver = $node->{_pg_version};
- carp("PostgreSQL::Test::Cluster isn't fully compatible with version " . $v)
- if $v < 12;
+ # Use a subclass as defined below (or elsewhere) if this version
+ # isn't fully compatible. Warn if the version is too old and thus we don't
+ # have a subclass of this class.
+ if (ref $ver && $ver < $min_compat)
+{
+ my $maj = $ver->major(separator => '_');
+ my $subclass = $class . "::V_$maj";
+ if ($subclass->isa($class))
+ {
+ bless $node, $subclass;
+ }
+ else
+ {
+ carp "PostgreSQL::Test::Cluster isn't fully compatible with version $ver";
+ }
+}
+
+ # Add node to list of nodes
+ push(@all_nodes, $node);
return $node;
}
@@ -2546,8 +2566,12 @@ sub wait_for_catchup
. "_lsn to pass "
. $target_lsn . " on "
. $self->name . "\n";
+ # old versions of walreceiver just set the application name to
+ # `walreceiver'
my $query =
- qq[SELECT '$target_lsn' <= ${mode}_lsn AND state = 'streaming' FROM pg_catalog.pg_stat_replication WHERE application_name = '$standby_name';];
+ qq[SELECT '$target_lsn' <= ${mode}_lsn AND state = 'streaming'
+ FROM pg_catalog.pg_stat_replication
+ WHERE application_name in ('$standby_name', 'walreceiver');];
$self->poll_query_until('postgres', $query)
or croak "timed out waiting for catchup";
print "done\n";
@@ -2807,4 +2831,41 @@ sub pg_recvlogical_upto
=cut
+##
+
+package PostgreSQL::Test::Cluster::V_11; ## no critic (ProhibitMultiplePackages)
+
+use parent -norequire, qw(PostgreSQL::Test::Cluster);
+
+# https://www.postgresql.org/docs/11/release-11.html
+
+# max_wal_senders + superuser_reserved_connections must be < max_connections
+# uses recovery.conf
+
+sub _recovery_file { return "recovery.conf"; }
+
+sub set_standby_mode
+{
+my $self = shift;
+$self->append_conf("recovery.conf", "standby_mode = on\n");
+}
+
+sub init
+{
+my ($self, %params) = @_;
+$self->SUPER::init(%params);
+$self->adjust_conf('postgresql.conf', 'max_wal_senders',
+ $params{allows_streaming} ? 5 : 0);
+}
+
+##
+
+package PostgreSQL::Test::Cluster::V_10; ## no critic (ProhibitMultiplePackages)
+
+use parent -norequire, qw(PostgreSQL::Test::Cluster::V_11);
+
+# https://www.postgresql.org/docs/10/release-10.html
+
+
+
1;
Re: Extend compatibility of PostgreSQL::Test::Cluster
On 12/31/21 11:20, Dagfinn Ilmari Mannsåker wrote:
> Andrew Dunstan writes:
>
>> +my $subclass = __PACKAGE__ . "::V_$maj";
>> +bless $node, $subclass;
>> +unless ($node->isa(__PACKAGE__))
>> +{
>> +# It's not a subclass, so re-bless back into the main
>> package
>> +bless($node, __PACKAGE__);
>> +carp "PostgreSQL::Test::Cluster isn't fully compatible
>> with version $ver";
>> +}
> The ->isa() method works on package names as well as blessed objects, so
> the back-and-forth blessing can be avoided.
>
> my $subclass = __PACKAGE__ . "::V_$maj";
> if ($subclass->isa(__PACKAGE__))
> {
> bless($node, $subclass);
> }
> else
> {
> carp "PostgreSQL::Test::Cluster isn't fully compatible with
> version $ver";
> }
>
OK, thanks, will fix in next version.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
Re: Extend compatibility of PostgreSQL::Test::Cluster
Andrew Dunstan writes:
> + my $subclass = __PACKAGE__ . "::V_$maj";
> + bless $node, $subclass;
> + unless ($node->isa(__PACKAGE__))
> + {
> + # It's not a subclass, so re-bless back into the main
> package
> + bless($node, __PACKAGE__);
> + carp "PostgreSQL::Test::Cluster isn't fully compatible
> with version $ver";
> + }
The ->isa() method works on package names as well as blessed objects, so
the back-and-forth blessing can be avoided.
my $subclass = __PACKAGE__ . "::V_$maj";
if ($subclass->isa(__PACKAGE__))
{
bless($node, $subclass);
}
else
{
carp "PostgreSQL::Test::Cluster isn't fully compatible with
version $ver";
}
- ilmari
Re: Extend compatibility of PostgreSQL::Test::Cluster
On 12/28/21 09:30, Andrew Dunstan wrote:
> PFA a patch to extend the compatibility of PostgreSQL::Test::Cluster to
> all live branches. It does this by introducing a couple of subclasses
> which override a few things. The required class is automatically
> detected and used, so users don't need to specify a subclass. Although
> this is my work it draws some inspiration from work by Jehan-Guillaume
> de Rorthais. The aim here is to provide minimal disruption to the
> mainline code, and also to have very small override subroutines.
>
> My hope is to take this further, down to 9.2, which we recently decided
> to give limited build support to. However I think the present patch is a
> good stake to put into the ground.
This version handles older versions for which we have no subclass more
gracefully.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index c061e850fb..89504128ec 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -111,6 +111,10 @@ use Scalar::Util qw(blessed);
our ($use_tcp, $test_localhost, $test_pghost, $last_host_assigned,
$last_port_assigned, @all_nodes, $died);
+# the minimum version we believe to be compatible with this package without
+# subclassing.
+our $min_compat = 12;
+
INIT
{
@@ -1018,7 +1022,7 @@ sub enable_streaming
print "### Enabling streaming replication for node \"$name\"\n";
$self->append_conf(
- 'postgresql.conf', qq(
+ $self->_recovery_file, qq(
primary_conninfo='$root_connstr'
));
$self->set_standby_mode();
@@ -1047,7 +1051,7 @@ sub enable_restoring
: qq{cp "$path/%f" "%p"};
$self->append_conf(
- 'postgresql.conf', qq(
+ $self->_recovery_file, qq(
restore_command = '$copy_command'
));
if ($standby)
@@ -1061,6 +1065,8 @@ restore_command = '$copy_command'
return;
}
+sub _recovery_file { return "postgresql.conf"; }
+
=pod
=item $node->set_recovery_mode()
@@ -1246,15 +1252,28 @@ sub new
$node->dump_info;
- # Add node to list of nodes
- push(@all_nodes, $node);
-
$node->_set_pg_version;
- my $v = $node->{_pg_version};
+ my $ver = $node->{_pg_version};
+
+ # Use a subclass as defined below (or elsewhere) if this version
+ # isn't fully compatible. Warn if the version is too old and thus we don't
+ # have a subclass of this class.
+ if (ref $ver && $ver < $min_compat)
+{
+ my $maj = $ver->major(separator => '_');
+ my $subclass = __PACKAGE__ . "::V_$maj";
+ bless $node, $subclass;
+ unless ($node->isa(__PACKAGE__))
+ {
+ # It's not a subclass, so re-bless back into the main package
+ bless($node, __PACKAGE__);
+ carp "PostgreSQL::Test::Cluster isn't fully compatible with version $ver";
+ }
+}
- carp("PostgreSQL::Test::Cluster isn't fully compatible with version " . $v)
- if $v < 12;
+ # Add node to list of nodes
+ push(@all_nodes, $node);
return $node;
}
@@ -2546,8 +2565,12 @@ sub wait_for_catchup
. "_lsn to pass "
. $lsn_expr . " on "
. $self->name . "\n";
+# old versions of walreceiver just set the application name to
+# `walreceiver'
my $query =
- qq[SELECT $lsn_expr <= ${mode}_lsn AND state = 'streaming' FROM pg_catalog.pg_stat_replication WHERE application_name = '$standby_name';];
+ qq[SELECT $lsn_expr <= ${mode}_lsn AND state = 'streaming'
+ FROM pg_catalog.pg_stat_replication
+ WHERE application_name in ('$standby_name','walreceiver');];
$self->poll_query_until('postgres', $query)
or croak "timed out waiting for catchup";
print "done\n";
@@ -2771,4 +2794,41 @@ sub pg_recvlogical_upto
=cut
+##
+
+package PostgreSQL::Test::Cluster::V_11; ## no critic (ProhibitMultiplePackages)
+
+use parent -norequire, qw(PostgreSQL::Test::Cluster);
+
+# https://www.postgresql.org/docs/11/release-11.html
+
+# max_wal_senders + superuser_reserved_connections must be < max_connections
+# uses recovery.conf
+
+sub _recovery_file { return "recovery.conf"; }
+
+sub set_standby_mode
+{
+my $self = shift;
+$self->append_conf("recovery.conf", "standby_mode = on\n");
+}
+
+sub init
+{
+my ($self, %params) = @_;
+$self->SUPER::init(%params);
+$self->adjust_conf('postgresql.conf', 'max_wal_senders',
+ $params{allows_streaming} ? 5 : 0);
+}
+
+##
+
+package PostgreSQL::Test::Cluster::V_10; ## no critic (ProhibitMultiplePackages)
+
+use parent -norequire, qw(PostgreSQL::Test::Cluster::V_11);
+
+# https://www.postgresql.org/docs/10/release-10.html
+
+
+
1;
Extend compatibility of PostgreSQL::Test::Cluster
PFA a patch to extend the compatibility of PostgreSQL::Test::Cluster to
all live branches. It does this by introducing a couple of subclasses
which override a few things. The required class is automatically
detected and used, so users don't need to specify a subclass. Although
this is my work it draws some inspiration from work by Jehan-Guillaume
de Rorthais. The aim here is to provide minimal disruption to the
mainline code, and also to have very small override subroutines.
My hope is to take this further, down to 9.2, which we recently decided
to give limited build support to. However I think the present patch is a
good stake to put into the ground.
cheers
andrew
--
Andrew Dunstan
EDB: https://www.enterprisedb.com
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index c061e850fb..0e0bf0ecfc 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -111,6 +111,10 @@ use Scalar::Util qw(blessed);
our ($use_tcp, $test_localhost, $test_pghost, $last_host_assigned,
$last_port_assigned, @all_nodes, $died);
+# the minimum version we believe to be compatible with this package without
+# subclassing.
+our $min_compat = 12;
+
INIT
{
@@ -1018,7 +1022,7 @@ sub enable_streaming
print "### Enabling streaming replication for node \"$name\"\n";
$self->append_conf(
- 'postgresql.conf', qq(
+ $self->_recovery_file, qq(
primary_conninfo='$root_connstr'
));
$self->set_standby_mode();
@@ -1047,7 +1051,7 @@ sub enable_restoring
: qq{cp "$path/%f" "%p"};
$self->append_conf(
- 'postgresql.conf', qq(
+ $self->_recovery_file, qq(
restore_command = '$copy_command'
));
if ($standby)
@@ -1061,6 +1065,8 @@ restore_command = '$copy_command'
return;
}
+sub _recovery_file { return "postgresql.conf"; }
+
=pod
=item $node->set_recovery_mode()
@@ -1246,15 +1252,24 @@ sub new
$node->dump_info;
- # Add node to list of nodes
- push(@all_nodes, $node);
-
$node->_set_pg_version;
- my $v = $node->{_pg_version};
+ my $ver = $node->{_pg_version};
- carp("PostgreSQL::Test::Cluster isn't fully compatible with version " . $v)
- if $v < 12;
+ # Use a subclass as defined below (or elsewhere) if this version
+ # isn't fully compatible. If the subclass doesn't exist then bad things
+ # will happen when you try to use the node. However, there isn't a simple
+ # test to see if the class exists, and we don't want to create another
+ # instance in order to check.
+ if (ref $ver && $ver < $min_compat)
+{
+my $maj = $ver->major(separator => '_');
+my $subclass = __PACKAGE__ . "::V_$maj";
+bless $node, $subclass;
+}
+
+ # Add node to list of nodes
+ push(@all_nodes, $node);
return $node;
}
@@ -2546,8 +2561,12 @@ sub wait_for_catchup
. "_lsn to pass "
. $lsn_expr . " on "
. $self->name . "\n";
+# old versions of walreceiver just set the application name to
+# `walreceiver'
my $query =
- qq[SELECT $lsn_expr <= ${mode}_lsn AND state = 'streaming' FROM pg_catalog.pg_stat_replication WHERE application_name = '$standby_name';];
+ qq[SELECT $lsn_expr <= ${mode}_lsn AND state = 'streaming'
+ FROM pg_catalog.pg_stat_replication
+ WHERE application_name in ('$standby_name','walreceiver');];
$self->poll_query_until('postgres', $query)
or croak "timed out waiting for catchup";
print "done\n";
@@ -2771,4 +2790,41 @@ sub pg_recvlogical_upto
=cut
+##
+
+package PostgreSQL::Test::Cluster::V_11; ## no critic (ProhibitMultiplePackages)
+
+use parent -norequire, qw(PostgreSQL::Test::Cluster);
+
+# https://www.postgresql.org/docs/11/release-11.html
+
+# max_wal_senders + superuser_reserved_connections must be < max_connections
+# uses recovery.conf
+
+sub _recovery_file { return "recovery.conf"; }
+
+sub set_standby_mode
+{
+my $self = shift;
+$self->append_conf("recovery.conf", "standby_mode = on\n");
+}
+
+sub init
+{
+my ($self, %params) = @_;
+$self->SUPER::init(%params);
+$self->adjust_conf('postgresql.conf', 'max_wal_senders',
+ $params{allows_streaming} ? 5 : 0);
+}
+
+##
+
+package PostgreSQL::Test::Cluster::V_10; ## no critic (ProhibitMultiplePackages)
+
+use parent -norequire, qw(PostgreSQL::Test::Cluster::V_11);
+
+# https://www.postgresql.org/docs/10/release-10.html
+
+
+
1;
