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;
