public inbox for [email protected]
help / color / mirror / Atom feedRe: Implement generalized sub routine find_in_log for tap test
15+ messages / 6 participants
[nested] [flat]
* Re: Implement generalized sub routine find_in_log for tap test
@ 2023-05-27 12:01 Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Andrew Dunstan @ 2023-05-27 12:01 UTC (permalink / raw)
To: vignesh C <[email protected]>; Michael Paquier <[email protected]>; +Cc: Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On 2023-05-26 Fr 20:35, vignesh C wrote:
> On Fri, 26 May 2023 at 04:09, Michael Paquier<[email protected]> wrote:
>> On Thu, May 25, 2023 at 06:34:20PM +0100, Dagfinn Ilmari Mannsåker wrote:
>>> However, none of the other functions in ::Utils know anything about node
>>> objects, which makes me think it should be a method on the node itself
>>> (i.e. in PostgreSQL::Test::Cluster) instead. Also, I think log_contains
>>> would be a better name, since it just returns a boolean. The name
>>> find_in_log makes me think it would return the log lines matching the
>>> pattern, or the position of the match in the file.
>>>
>>> In that case, the slurp_file() call would have to be fully qualified,
>>> since ::Cluster uses an empty import list to avoid polluting the method
>>> namespace with imported functions.
>> Hmm. connect_ok() and connect_fails() in Cluster.pm have a similar
>> log comparison logic, feeding from the offset of a log file. Couldn't
>> you use the same code across the board for everything? Note that this
>> stuff is parameterized so as it is possible to check if patterns match
>> or do not match, for multiple patterns. It seems to me that we could
>> use the new log finding routine there as well, so how about extending
>> it a bit more? You would need, at least:
>> - One parameter for log entries matching.
>> - One parameter for log entries not matching.
> I felt adding these to log_contains was making the function slightly
> complex with multiple checks. I was not able to make it simple with
> the approach I tried. How about having a common function
> check_connect_log_contents which has the common log contents check for
> connect_ok and connect_fails function like the v2-0002 patch attached.
+ $offset = 0 unless defined $offset;
This is unnecessary, as slurp_file() handles it appropriately, and in
fact doing this is slightly inefficient, as it will cause slurp_file to
do a redundant seek.
FYI there's a simpler way to say it if we wanted to:
$offset //= 0;
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Implement generalized sub routine find_in_log for tap test
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
@ 2023-05-29 02:19 ` vignesh C <[email protected]>
2023-06-03 22:21 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: vignesh C @ 2023-05-29 02:19 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Michael Paquier <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On Sat, 27 May 2023 at 17:32, Andrew Dunstan <[email protected]> wrote:
>
>
> On 2023-05-26 Fr 20:35, vignesh C wrote:
>
> On Fri, 26 May 2023 at 04:09, Michael Paquier <[email protected]> wrote:
>
> On Thu, May 25, 2023 at 06:34:20PM +0100, Dagfinn Ilmari Mannsåker wrote:
>
> However, none of the other functions in ::Utils know anything about node
> objects, which makes me think it should be a method on the node itself
> (i.e. in PostgreSQL::Test::Cluster) instead. Also, I think log_contains
> would be a better name, since it just returns a boolean. The name
> find_in_log makes me think it would return the log lines matching the
> pattern, or the position of the match in the file.
>
> In that case, the slurp_file() call would have to be fully qualified,
> since ::Cluster uses an empty import list to avoid polluting the method
> namespace with imported functions.
>
> Hmm. connect_ok() and connect_fails() in Cluster.pm have a similar
> log comparison logic, feeding from the offset of a log file. Couldn't
> you use the same code across the board for everything? Note that this
> stuff is parameterized so as it is possible to check if patterns match
> or do not match, for multiple patterns. It seems to me that we could
> use the new log finding routine there as well, so how about extending
> it a bit more? You would need, at least:
> - One parameter for log entries matching.
> - One parameter for log entries not matching.
>
> I felt adding these to log_contains was making the function slightly
> complex with multiple checks. I was not able to make it simple with
> the approach I tried. How about having a common function
> check_connect_log_contents which has the common log contents check for
> connect_ok and connect_fails function like the v2-0002 patch attached.
>
>
> + $offset = 0 unless defined $offset;
>
>
> This is unnecessary, as slurp_file() handles it appropriately, and in fact doing this is slightly inefficient, as it will cause slurp_file to do a redundant seek.
>
> FYI there's a simpler way to say it if we wanted to:
>
> $offset //= 0;
Thanks for the comment, the attached v3 version patch has the changes
for the same.
Regards,
Vignesh
Attachments:
[text/x-patch] v3-0002-Move-common-connection-log-content-verification-c.patch (4.7K, ../../CALDaNm3SWpFDTs1oRRbOAfYJNuSPMCLu4O3mLnU-tKoSFgGckQ@mail.gmail.com/2-v3-0002-Move-common-connection-log-content-verification-c.patch)
download | inline diff:
From 02cda5bc291d2e951b971081981a336e22c74ec1 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Sat, 27 May 2023 05:36:34 +0530
Subject: [PATCH v3 2/2] Move common connection log content verification code
to a common function.
Move common connection log content verification code to a common
function.
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 122 +++++++++++------------
1 file changed, 60 insertions(+), 62 deletions(-)
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 4df7dd4dec..912892e28b 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2168,21 +2168,19 @@ sub pgbench
=pod
-=item $node->connect_ok($connstr, $test_name, %params)
+=item $node->check_connect_log_contents($offset, $test_name, %parameters)
-Attempt a connection with a custom connection string. This is expected
-to succeed.
+Check connection log contents.
=over
-=item sql => B<value>
+=item $test_name
-If this parameter is set, this query is used for the connection attempt
-instead of the default.
+Name of test for error messages.
-=item expected_stdout => B<value>
+=item $offset
-If this regular expression is set, matches it with the output generated.
+Offset of the log file.
=item log_like => [ qr/required message/ ]
@@ -2200,6 +2198,58 @@ passed to C<Test::More::unlike()>.
=cut
+sub check_connect_log_contents
+{
+ my ($self, $test_name, $offset, %params) = @_;
+
+ my (@log_like, @log_unlike);
+ if (defined($params{log_like}))
+ {
+ @log_like = @{ $params{log_like} };
+ }
+ if (defined($params{log_unlike}))
+ {
+ @log_unlike = @{ $params{log_unlike} };
+ }
+
+ if (@log_like or @log_unlike)
+ {
+ my $log_contents =
+ PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset);
+
+ while (my $regex = shift @log_like)
+ {
+ like($log_contents, $regex, "$test_name: log matches");
+ }
+ while (my $regex = shift @log_unlike)
+ {
+ unlike($log_contents, $regex, "$test_name: log does not match");
+ }
+ }
+}
+
+=pod
+
+=item $node->connect_ok($connstr, $test_name, %params)
+
+Attempt a connection with a custom connection string. This is expected
+to succeed.
+
+=over
+
+=item sql => B<value>
+
+If this parameter is set, this query is used for the connection attempt
+instead of the default.
+
+=item expected_stdout => B<value>
+
+If this regular expression is set, matches it with the output generated.
+
+=back
+
+=cut
+
sub connect_ok
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
@@ -2215,16 +2265,6 @@ sub connect_ok
$sql = "SELECT \$\$connected with $connstr\$\$";
}
- my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
-
my $log_location = -s $self->logfile;
# Never prompt for a password, any callers of this routine should
@@ -2245,20 +2285,7 @@ sub connect_ok
is($stderr, "", "$test_name: no stderr");
- if (@log_like or @log_unlike)
- {
- my $log_contents =
- PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
+ $self->check_connect_log_contents($test_name, $log_location, %params);
}
=pod
@@ -2274,12 +2301,6 @@ to fail.
If this regular expression is set, matches it with the output generated.
-=item log_like => [ qr/required message/ ]
-
-=item log_unlike => [ qr/prohibited message/ ]
-
-See C<connect_ok(...)>, above.
-
=back
=cut
@@ -2289,16 +2310,6 @@ sub connect_fails
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($self, $connstr, $test_name, %params) = @_;
- my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
-
my $log_location = -s $self->logfile;
# Never prompt for a password, any callers of this routine should
@@ -2316,20 +2327,7 @@ sub connect_fails
like($stderr, $params{expected_stderr}, "$test_name: matches");
}
- if (@log_like or @log_unlike)
- {
- my $log_contents =
- PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
+ $self->check_connect_log_contents($test_name, $log_location, %params);
}
=pod
--
2.34.1
[text/x-patch] v3-0001-Remove-duplicate-find_in_log-sub-routines-from-ta.patch (7.6K, ../../CALDaNm3SWpFDTs1oRRbOAfYJNuSPMCLu4O3mLnU-tKoSFgGckQ@mail.gmail.com/3-v3-0001-Remove-duplicate-find_in_log-sub-routines-from-ta.patch)
download | inline diff:
From 12dea35bbb20d2ee09a3dfea1f67895d08110096 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Fri, 26 May 2023 20:07:30 +0530
Subject: [PATCH v3 1/2] Remove duplicate find_in_log sub routines from tap
tests.
Remove duplicate find_in_log sub routines from tap tests.
---
src/test/authentication/t/003_peer.pl | 17 ++--------
src/test/perl/PostgreSQL/Test/Cluster.pm | 15 +++++++++
src/test/recovery/t/019_replslot_limit.pl | 33 +++++--------------
src/test/recovery/t/033_replay_tsp_drops.pl | 15 ++-------
.../t/035_standby_logical_decoding.pl | 26 +++------------
5 files changed, 32 insertions(+), 74 deletions(-)
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 3272e52cae..2a035c2d0d 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -69,17 +69,6 @@ sub test_role
}
}
-# Find $pattern in log file of $node.
-sub find_in_log
-{
- my ($node, $offset, $pattern) = @_;
-
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $offset);
- return 0 if (length($log) <= 0);
-
- return $log =~ m/$pattern/;
-}
-
my $node = PostgreSQL::Test::Cluster->new('node');
$node->init;
$node->append_conf('postgresql.conf', "log_connections = on\n");
@@ -91,9 +80,9 @@ reset_pg_hba($node, 'peer');
# Check if peer authentication is supported on this platform.
my $log_offset = -s $node->logfile;
$node->psql('postgres');
-if (find_in_log(
- $node, $log_offset,
- qr/peer authentication is not supported on this platform/))
+if ($node->log_contains(
+ qr/peer authentication is not supported on this platform/),
+ $log_offset,)
{
plan skip_all => 'peer authentication is not supported on this platform';
}
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index baea0fcd1c..4df7dd4dec 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2536,6 +2536,21 @@ sub log_content
}
+=pod
+
+=item log_contains(pattern, offset)
+
+Find pattern in logfile of node after offset byte.
+
+=cut
+
+sub log_contains
+{
+ my ($self, $pattern, $offset) = @_;
+
+ return PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset) =~ m/$pattern/;
+}
+
=pod
=item $node->run_log(...)
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index a1aba16e14..95acf9e357 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -161,8 +161,7 @@ $node_primary->wait_for_catchup($node_standby);
$node_standby->stop;
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed"),
'check that required WAL segments are still available');
@@ -184,8 +183,8 @@ $node_primary->safe_psql('postgres', "CHECKPOINT;");
my $invalidated = 0;
for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
{
- if (find_in_log(
- $node_primary, 'invalidating obsolete replication slot "rep1"',
+ if ($node_primary->log_contains(
+ 'invalidating obsolete replication slot "rep1"',
$logstart))
{
$invalidated = 1;
@@ -207,7 +206,7 @@ is($result, "rep1|f|t|lost|",
my $checkpoint_ended = 0;
for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
{
- if (find_in_log($node_primary, "checkpoint complete: ", $logstart))
+ if ($node_primary->log_contains("checkpoint complete: ", $logstart))
{
$checkpoint_ended = 1;
last;
@@ -237,8 +236,7 @@ $node_standby->start;
my $failed = 0;
for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
{
- if (find_in_log(
- $node_standby,
+ if ($node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed",
$logstart))
{
@@ -381,8 +379,7 @@ my $msg_logged = 0;
my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
while ($max_attempts-- >= 0)
{
- if (find_in_log(
- $node_primary3,
+ if ($node_primary3->log_contains(
"terminating process $senderpid to release replication slot \"rep3\"",
$logstart))
{
@@ -406,8 +403,8 @@ $msg_logged = 0;
$max_attempts = $PostgreSQL::Test::Utils::timeout_default;
while ($max_attempts-- >= 0)
{
- if (find_in_log(
- $node_primary3, 'invalidating obsolete replication slot "rep3"',
+ if ($node_primary3->log_contains(
+ 'invalidating obsolete replication slot "rep3"',
$logstart))
{
$msg_logged = 1;
@@ -446,18 +443,4 @@ sub get_log_size
return (stat $node->logfile)[7];
}
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- $off = 0 unless defined $off;
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
- return 0 if (length($log) <= $off);
-
- $log = substr($log, $off);
-
- return $log =~ m/$pat/;
-}
-
done_testing();
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 0a35a7bda6..307c30bc6b 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -135,22 +135,11 @@ while ($max_attempts-- >= 0)
{
last
if (
- find_in_log(
- $node_standby,
+ $node_standby->log_contains(
qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
$logstart));
usleep(100_000);
}
ok($max_attempts > 0, "invalid directory creation is detected");
-done_testing();
-
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $off);
-
- return $log =~ m/$pat/;
-}
+done_testing();
\ No newline at end of file
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index 64beec4bd3..480e6d6caa 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -28,20 +28,6 @@ my $res;
my $primary_slotname = 'primary_physical';
my $standby_physical_slotname = 'standby_physical';
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- $off = 0 unless defined $off;
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
- return 0 if (length($log) <= $off);
-
- $log = substr($log, $off);
-
- return $log =~ m/$pat/;
-}
-
# Fetch xmin columns from slot's pg_replication_slots row, after waiting for
# given boolean condition to be true to ensure we've reached a quiescent state.
sub wait_for_xmins
@@ -235,14 +221,12 @@ sub check_for_invalidation
my $inactive_slot = $slot_prefix . 'inactiveslot';
# message should be issued
- ok( find_in_log(
- $node_standby,
+ ok( $node_standby->log_contains(
"invalidating obsolete replication slot \"$inactive_slot\"",
$log_start),
"inactiveslot slot invalidation is logged $test_name");
- ok( find_in_log(
- $node_standby,
+ ok( $node_standby->log_contains(
"invalidating obsolete replication slot \"$active_slot\"",
$log_start),
"activeslot slot invalidation is logged $test_name");
@@ -657,14 +641,12 @@ $node_primary->safe_psql(
$node_primary->wait_for_replay_catchup($node_standby);
# message should not be issued
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"invalidating obsolete slot \"no_conflict_inactiveslot\"", $logstart),
'inactiveslot slot invalidation is not logged with vacuum on conflict_test'
);
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"invalidating obsolete slot \"no_conflict_activeslot\"", $logstart),
'activeslot slot invalidation is not logged with vacuum on conflict_test'
);
--
2.34.1
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Implement generalized sub routine find_in_log for tap test
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
@ 2023-06-03 22:21 ` Michael Paquier <[email protected]>
2023-06-05 16:09 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-06 02:35 ` Re: Implement generalized sub routine find_in_log for tap test Amit Kapila <[email protected]>
0 siblings, 2 replies; 15+ messages in thread
From: Michael Paquier @ 2023-06-03 22:21 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, May 29, 2023 at 07:49:52AM +0530, vignesh C wrote:
> Thanks for the comment, the attached v3 version patch has the changes
> for the same.
-if (find_in_log(
- $node, $log_offset,
- qr/peer authentication is not supported on this platform/))
+if ($node->log_contains(
+ qr/peer authentication is not supported on this platform/),
+ $log_offset,)
This looks like a typo to me, the log offset is eaten.
Except of that, I am on board with log_contains().
There are two things that bugged me with the refactoring related to
connect_ok and connect_fails:
- check_connect_log_contents() is a name too complicated. While
looking at that I have settled to a simpler log_check(), as we could
perfectly use that for something else than connections as long as we
want to check multiple patterns at once.
- The refactoring of the documentation for the routines of Cluster.pm
became incorrect. For example, the patch does not list anymore
log_like and log_unlike for connect_ok.
With all that in mind I have hacked a few adjustments in a 0003,
though I agree with the separation between 0001 and 0002.
033_replay_tsp_drops and 019_replslot_limit are not new to v16, but
003_peer.pl and 035_standby_logical_decoding.pl, making the number of
places where find_in_log() exists twice as much. So I would be
tempted to refactor these tests in v16. Perhaps anybody from the RMT
could comment? We've usually been quite flexible with the tests even
in beta.
Thoughts?
--
Michael
From 49473ab523adf43e82fe71d7e33be478287ad2c8 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Fri, 26 May 2023 20:07:30 +0530
Subject: [PATCH v4 1/3] Remove duplicate find_in_log sub routines from tap
tests.
Remove duplicate find_in_log sub routines from tap tests.
---
src/test/authentication/t/003_peer.pl | 17 ++--------
src/test/perl/PostgreSQL/Test/Cluster.pm | 15 +++++++++
src/test/recovery/t/019_replslot_limit.pl | 33 +++++--------------
src/test/recovery/t/033_replay_tsp_drops.pl | 15 ++-------
.../t/035_standby_logical_decoding.pl | 26 +++------------
5 files changed, 32 insertions(+), 74 deletions(-)
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 3272e52cae..2a035c2d0d 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -69,17 +69,6 @@ sub test_role
}
}
-# Find $pattern in log file of $node.
-sub find_in_log
-{
- my ($node, $offset, $pattern) = @_;
-
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $offset);
- return 0 if (length($log) <= 0);
-
- return $log =~ m/$pattern/;
-}
-
my $node = PostgreSQL::Test::Cluster->new('node');
$node->init;
$node->append_conf('postgresql.conf', "log_connections = on\n");
@@ -91,9 +80,9 @@ reset_pg_hba($node, 'peer');
# Check if peer authentication is supported on this platform.
my $log_offset = -s $node->logfile;
$node->psql('postgres');
-if (find_in_log(
- $node, $log_offset,
- qr/peer authentication is not supported on this platform/))
+if ($node->log_contains(
+ qr/peer authentication is not supported on this platform/),
+ $log_offset,)
{
plan skip_all => 'peer authentication is not supported on this platform';
}
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index baea0fcd1c..4df7dd4dec 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2536,6 +2536,21 @@ sub log_content
}
+=pod
+
+=item log_contains(pattern, offset)
+
+Find pattern in logfile of node after offset byte.
+
+=cut
+
+sub log_contains
+{
+ my ($self, $pattern, $offset) = @_;
+
+ return PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset) =~ m/$pattern/;
+}
+
=pod
=item $node->run_log(...)
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index a1aba16e14..95acf9e357 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -161,8 +161,7 @@ $node_primary->wait_for_catchup($node_standby);
$node_standby->stop;
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed"),
'check that required WAL segments are still available');
@@ -184,8 +183,8 @@ $node_primary->safe_psql('postgres', "CHECKPOINT;");
my $invalidated = 0;
for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
{
- if (find_in_log(
- $node_primary, 'invalidating obsolete replication slot "rep1"',
+ if ($node_primary->log_contains(
+ 'invalidating obsolete replication slot "rep1"',
$logstart))
{
$invalidated = 1;
@@ -207,7 +206,7 @@ is($result, "rep1|f|t|lost|",
my $checkpoint_ended = 0;
for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
{
- if (find_in_log($node_primary, "checkpoint complete: ", $logstart))
+ if ($node_primary->log_contains("checkpoint complete: ", $logstart))
{
$checkpoint_ended = 1;
last;
@@ -237,8 +236,7 @@ $node_standby->start;
my $failed = 0;
for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
{
- if (find_in_log(
- $node_standby,
+ if ($node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed",
$logstart))
{
@@ -381,8 +379,7 @@ my $msg_logged = 0;
my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
while ($max_attempts-- >= 0)
{
- if (find_in_log(
- $node_primary3,
+ if ($node_primary3->log_contains(
"terminating process $senderpid to release replication slot \"rep3\"",
$logstart))
{
@@ -406,8 +403,8 @@ $msg_logged = 0;
$max_attempts = $PostgreSQL::Test::Utils::timeout_default;
while ($max_attempts-- >= 0)
{
- if (find_in_log(
- $node_primary3, 'invalidating obsolete replication slot "rep3"',
+ if ($node_primary3->log_contains(
+ 'invalidating obsolete replication slot "rep3"',
$logstart))
{
$msg_logged = 1;
@@ -446,18 +443,4 @@ sub get_log_size
return (stat $node->logfile)[7];
}
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- $off = 0 unless defined $off;
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
- return 0 if (length($log) <= $off);
-
- $log = substr($log, $off);
-
- return $log =~ m/$pat/;
-}
-
done_testing();
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 0a35a7bda6..307c30bc6b 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -135,22 +135,11 @@ while ($max_attempts-- >= 0)
{
last
if (
- find_in_log(
- $node_standby,
+ $node_standby->log_contains(
qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
$logstart));
usleep(100_000);
}
ok($max_attempts > 0, "invalid directory creation is detected");
-done_testing();
-
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $off);
-
- return $log =~ m/$pat/;
-}
+done_testing();
\ No newline at end of file
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index 64beec4bd3..480e6d6caa 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -28,20 +28,6 @@ my $res;
my $primary_slotname = 'primary_physical';
my $standby_physical_slotname = 'standby_physical';
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- $off = 0 unless defined $off;
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
- return 0 if (length($log) <= $off);
-
- $log = substr($log, $off);
-
- return $log =~ m/$pat/;
-}
-
# Fetch xmin columns from slot's pg_replication_slots row, after waiting for
# given boolean condition to be true to ensure we've reached a quiescent state.
sub wait_for_xmins
@@ -235,14 +221,12 @@ sub check_for_invalidation
my $inactive_slot = $slot_prefix . 'inactiveslot';
# message should be issued
- ok( find_in_log(
- $node_standby,
+ ok( $node_standby->log_contains(
"invalidating obsolete replication slot \"$inactive_slot\"",
$log_start),
"inactiveslot slot invalidation is logged $test_name");
- ok( find_in_log(
- $node_standby,
+ ok( $node_standby->log_contains(
"invalidating obsolete replication slot \"$active_slot\"",
$log_start),
"activeslot slot invalidation is logged $test_name");
@@ -657,14 +641,12 @@ $node_primary->safe_psql(
$node_primary->wait_for_replay_catchup($node_standby);
# message should not be issued
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"invalidating obsolete slot \"no_conflict_inactiveslot\"", $logstart),
'inactiveslot slot invalidation is not logged with vacuum on conflict_test'
);
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"invalidating obsolete slot \"no_conflict_activeslot\"", $logstart),
'activeslot slot invalidation is not logged with vacuum on conflict_test'
);
--
2.40.1
From 966ea0c337a187522bcabc877214761d9818b58a Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Sat, 27 May 2023 05:36:34 +0530
Subject: [PATCH v4 2/3] Move common connection log content verification code
to a common function.
Move common connection log content verification code to a common
function.
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 130 +++++++++++------------
1 file changed, 64 insertions(+), 66 deletions(-)
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 4df7dd4dec..912892e28b 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2168,6 +2168,68 @@ sub pgbench
=pod
+=item $node->check_connect_log_contents($offset, $test_name, %parameters)
+
+Check connection log contents.
+
+=over
+
+=item $test_name
+
+Name of test for error messages.
+
+=item $offset
+
+Offset of the log file.
+
+=item log_like => [ qr/required message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must match against the server log, using
+C<Test::More::like()>.
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must NOT match against the server log. They will be
+passed to C<Test::More::unlike()>.
+
+=back
+
+=cut
+
+sub check_connect_log_contents
+{
+ my ($self, $test_name, $offset, %params) = @_;
+
+ my (@log_like, @log_unlike);
+ if (defined($params{log_like}))
+ {
+ @log_like = @{ $params{log_like} };
+ }
+ if (defined($params{log_unlike}))
+ {
+ @log_unlike = @{ $params{log_unlike} };
+ }
+
+ if (@log_like or @log_unlike)
+ {
+ my $log_contents =
+ PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset);
+
+ while (my $regex = shift @log_like)
+ {
+ like($log_contents, $regex, "$test_name: log matches");
+ }
+ while (my $regex = shift @log_unlike)
+ {
+ unlike($log_contents, $regex, "$test_name: log does not match");
+ }
+ }
+}
+
+=pod
+
=item $node->connect_ok($connstr, $test_name, %params)
Attempt a connection with a custom connection string. This is expected
@@ -2184,18 +2246,6 @@ instead of the default.
If this regular expression is set, matches it with the output generated.
-=item log_like => [ qr/required message/ ]
-
-If given, it must be an array reference containing a list of regular
-expressions that must match against the server log, using
-C<Test::More::like()>.
-
-=item log_unlike => [ qr/prohibited message/ ]
-
-If given, it must be an array reference containing a list of regular
-expressions that must NOT match against the server log. They will be
-passed to C<Test::More::unlike()>.
-
=back
=cut
@@ -2215,16 +2265,6 @@ sub connect_ok
$sql = "SELECT \$\$connected with $connstr\$\$";
}
- my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
-
my $log_location = -s $self->logfile;
# Never prompt for a password, any callers of this routine should
@@ -2245,20 +2285,7 @@ sub connect_ok
is($stderr, "", "$test_name: no stderr");
- if (@log_like or @log_unlike)
- {
- my $log_contents =
- PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
+ $self->check_connect_log_contents($test_name, $log_location, %params);
}
=pod
@@ -2274,12 +2301,6 @@ to fail.
If this regular expression is set, matches it with the output generated.
-=item log_like => [ qr/required message/ ]
-
-=item log_unlike => [ qr/prohibited message/ ]
-
-See C<connect_ok(...)>, above.
-
=back
=cut
@@ -2289,16 +2310,6 @@ sub connect_fails
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($self, $connstr, $test_name, %params) = @_;
- my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
-
my $log_location = -s $self->logfile;
# Never prompt for a password, any callers of this routine should
@@ -2316,20 +2327,7 @@ sub connect_fails
like($stderr, $params{expected_stderr}, "$test_name: matches");
}
- if (@log_like or @log_unlike)
- {
- my $log_contents =
- PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
+ $self->check_connect_log_contents($test_name, $log_location, %params);
}
=pod
--
2.40.1
From fce041c5223d14b8bc654461e344f098df95c52a Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Sat, 3 Jun 2023 18:15:42 -0400
Subject: [PATCH v4 3/3] Adjust a bit previous patches ;)
---
src/test/authentication/t/003_peer.pl | 4 +-
src/test/perl/PostgreSQL/Test/Cluster.pm | 139 +++++++++++---------
src/test/recovery/t/033_replay_tsp_drops.pl | 2 +-
3 files changed, 78 insertions(+), 67 deletions(-)
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 2a035c2d0d..d8e4976072 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -81,8 +81,8 @@ reset_pg_hba($node, 'peer');
my $log_offset = -s $node->logfile;
$node->psql('postgres');
if ($node->log_contains(
- qr/peer authentication is not supported on this platform/),
- $log_offset,)
+ qr/peer authentication is not supported on this platform/,
+ $log_offset))
{
plan skip_all => 'peer authentication is not supported on this platform';
}
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 912892e28b..19cbc46390 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2168,68 +2168,6 @@ sub pgbench
=pod
-=item $node->check_connect_log_contents($offset, $test_name, %parameters)
-
-Check connection log contents.
-
-=over
-
-=item $test_name
-
-Name of test for error messages.
-
-=item $offset
-
-Offset of the log file.
-
-=item log_like => [ qr/required message/ ]
-
-If given, it must be an array reference containing a list of regular
-expressions that must match against the server log, using
-C<Test::More::like()>.
-
-=item log_unlike => [ qr/prohibited message/ ]
-
-If given, it must be an array reference containing a list of regular
-expressions that must NOT match against the server log. They will be
-passed to C<Test::More::unlike()>.
-
-=back
-
-=cut
-
-sub check_connect_log_contents
-{
- my ($self, $test_name, $offset, %params) = @_;
-
- my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
-
- if (@log_like or @log_unlike)
- {
- my $log_contents =
- PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
-}
-
-=pod
-
=item $node->connect_ok($connstr, $test_name, %params)
Attempt a connection with a custom connection string. This is expected
@@ -2246,6 +2184,12 @@ instead of the default.
If this regular expression is set, matches it with the output generated.
+=item log_like => [ qr/required message/ ]
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+See C<log_check(...)>.
+
=back
=cut
@@ -2285,7 +2229,7 @@ sub connect_ok
is($stderr, "", "$test_name: no stderr");
- $self->check_connect_log_contents($test_name, $log_location, %params);
+ $self->log_check($test_name, $log_location, %params);
}
=pod
@@ -2301,6 +2245,12 @@ to fail.
If this regular expression is set, matches it with the output generated.
+=item log_like => [ qr/required message/ ]
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+See C<log_check(...)>.
+
=back
=cut
@@ -2327,7 +2277,7 @@ sub connect_fails
like($stderr, $params{expected_stderr}, "$test_name: matches");
}
- $self->check_connect_log_contents($test_name, $log_location, %params);
+ $self->log_check($test_name, $log_location, %params);
}
=pod
@@ -2533,6 +2483,67 @@ sub log_content
return PostgreSQL::Test::Utils::slurp_file($self->logfile);
}
+=pod
+
+=item $node->log_check($offset, $test_name, %parameters)
+
+Check contents of server logs.
+
+=over
+
+=item $test_name
+
+Name of test for error messages.
+
+=item $offset
+
+Offset of the log file.
+
+=item log_like => [ qr/required message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must match against the server log, using
+C<Test::More::like()>.
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must NOT match against the server log. They will be
+passed to C<Test::More::unlike()>.
+
+=back
+
+=cut
+
+sub log_check
+{
+ my ($self, $test_name, $offset, %params) = @_;
+
+ my (@log_like, @log_unlike);
+ if (defined($params{log_like}))
+ {
+ @log_like = @{ $params{log_like} };
+ }
+ if (defined($params{log_unlike}))
+ {
+ @log_unlike = @{ $params{log_unlike} };
+ }
+
+ if (@log_like or @log_unlike)
+ {
+ my $log_contents =
+ PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset);
+
+ while (my $regex = shift @log_like)
+ {
+ like($log_contents, $regex, "$test_name: log matches");
+ }
+ while (my $regex = shift @log_unlike)
+ {
+ unlike($log_contents, $regex, "$test_name: log does not match");
+ }
+ }
+}
=pod
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 307c30bc6b..af97ed9f70 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -142,4 +142,4 @@ while ($max_attempts-- >= 0)
}
ok($max_attempts > 0, "invalid directory creation is detected");
-done_testing();
\ No newline at end of file
+done_testing();
--
2.40.1
Attachments:
[text/plain] v4-0001-Remove-duplicate-find_in_log-sub-routines-from-ta.patch (7.6K, ../../[email protected]/2-v4-0001-Remove-duplicate-find_in_log-sub-routines-from-ta.patch)
download | inline diff:
From 49473ab523adf43e82fe71d7e33be478287ad2c8 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Fri, 26 May 2023 20:07:30 +0530
Subject: [PATCH v4 1/3] Remove duplicate find_in_log sub routines from tap
tests.
Remove duplicate find_in_log sub routines from tap tests.
---
src/test/authentication/t/003_peer.pl | 17 ++--------
src/test/perl/PostgreSQL/Test/Cluster.pm | 15 +++++++++
src/test/recovery/t/019_replslot_limit.pl | 33 +++++--------------
src/test/recovery/t/033_replay_tsp_drops.pl | 15 ++-------
.../t/035_standby_logical_decoding.pl | 26 +++------------
5 files changed, 32 insertions(+), 74 deletions(-)
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 3272e52cae..2a035c2d0d 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -69,17 +69,6 @@ sub test_role
}
}
-# Find $pattern in log file of $node.
-sub find_in_log
-{
- my ($node, $offset, $pattern) = @_;
-
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $offset);
- return 0 if (length($log) <= 0);
-
- return $log =~ m/$pattern/;
-}
-
my $node = PostgreSQL::Test::Cluster->new('node');
$node->init;
$node->append_conf('postgresql.conf', "log_connections = on\n");
@@ -91,9 +80,9 @@ reset_pg_hba($node, 'peer');
# Check if peer authentication is supported on this platform.
my $log_offset = -s $node->logfile;
$node->psql('postgres');
-if (find_in_log(
- $node, $log_offset,
- qr/peer authentication is not supported on this platform/))
+if ($node->log_contains(
+ qr/peer authentication is not supported on this platform/),
+ $log_offset,)
{
plan skip_all => 'peer authentication is not supported on this platform';
}
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index baea0fcd1c..4df7dd4dec 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2536,6 +2536,21 @@ sub log_content
}
+=pod
+
+=item log_contains(pattern, offset)
+
+Find pattern in logfile of node after offset byte.
+
+=cut
+
+sub log_contains
+{
+ my ($self, $pattern, $offset) = @_;
+
+ return PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset) =~ m/$pattern/;
+}
+
=pod
=item $node->run_log(...)
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index a1aba16e14..95acf9e357 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -161,8 +161,7 @@ $node_primary->wait_for_catchup($node_standby);
$node_standby->stop;
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed"),
'check that required WAL segments are still available');
@@ -184,8 +183,8 @@ $node_primary->safe_psql('postgres', "CHECKPOINT;");
my $invalidated = 0;
for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
{
- if (find_in_log(
- $node_primary, 'invalidating obsolete replication slot "rep1"',
+ if ($node_primary->log_contains(
+ 'invalidating obsolete replication slot "rep1"',
$logstart))
{
$invalidated = 1;
@@ -207,7 +206,7 @@ is($result, "rep1|f|t|lost|",
my $checkpoint_ended = 0;
for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
{
- if (find_in_log($node_primary, "checkpoint complete: ", $logstart))
+ if ($node_primary->log_contains("checkpoint complete: ", $logstart))
{
$checkpoint_ended = 1;
last;
@@ -237,8 +236,7 @@ $node_standby->start;
my $failed = 0;
for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
{
- if (find_in_log(
- $node_standby,
+ if ($node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed",
$logstart))
{
@@ -381,8 +379,7 @@ my $msg_logged = 0;
my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
while ($max_attempts-- >= 0)
{
- if (find_in_log(
- $node_primary3,
+ if ($node_primary3->log_contains(
"terminating process $senderpid to release replication slot \"rep3\"",
$logstart))
{
@@ -406,8 +403,8 @@ $msg_logged = 0;
$max_attempts = $PostgreSQL::Test::Utils::timeout_default;
while ($max_attempts-- >= 0)
{
- if (find_in_log(
- $node_primary3, 'invalidating obsolete replication slot "rep3"',
+ if ($node_primary3->log_contains(
+ 'invalidating obsolete replication slot "rep3"',
$logstart))
{
$msg_logged = 1;
@@ -446,18 +443,4 @@ sub get_log_size
return (stat $node->logfile)[7];
}
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- $off = 0 unless defined $off;
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
- return 0 if (length($log) <= $off);
-
- $log = substr($log, $off);
-
- return $log =~ m/$pat/;
-}
-
done_testing();
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 0a35a7bda6..307c30bc6b 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -135,22 +135,11 @@ while ($max_attempts-- >= 0)
{
last
if (
- find_in_log(
- $node_standby,
+ $node_standby->log_contains(
qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
$logstart));
usleep(100_000);
}
ok($max_attempts > 0, "invalid directory creation is detected");
-done_testing();
-
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $off);
-
- return $log =~ m/$pat/;
-}
+done_testing();
\ No newline at end of file
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index 64beec4bd3..480e6d6caa 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -28,20 +28,6 @@ my $res;
my $primary_slotname = 'primary_physical';
my $standby_physical_slotname = 'standby_physical';
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- $off = 0 unless defined $off;
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
- return 0 if (length($log) <= $off);
-
- $log = substr($log, $off);
-
- return $log =~ m/$pat/;
-}
-
# Fetch xmin columns from slot's pg_replication_slots row, after waiting for
# given boolean condition to be true to ensure we've reached a quiescent state.
sub wait_for_xmins
@@ -235,14 +221,12 @@ sub check_for_invalidation
my $inactive_slot = $slot_prefix . 'inactiveslot';
# message should be issued
- ok( find_in_log(
- $node_standby,
+ ok( $node_standby->log_contains(
"invalidating obsolete replication slot \"$inactive_slot\"",
$log_start),
"inactiveslot slot invalidation is logged $test_name");
- ok( find_in_log(
- $node_standby,
+ ok( $node_standby->log_contains(
"invalidating obsolete replication slot \"$active_slot\"",
$log_start),
"activeslot slot invalidation is logged $test_name");
@@ -657,14 +641,12 @@ $node_primary->safe_psql(
$node_primary->wait_for_replay_catchup($node_standby);
# message should not be issued
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"invalidating obsolete slot \"no_conflict_inactiveslot\"", $logstart),
'inactiveslot slot invalidation is not logged with vacuum on conflict_test'
);
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"invalidating obsolete slot \"no_conflict_activeslot\"", $logstart),
'activeslot slot invalidation is not logged with vacuum on conflict_test'
);
--
2.40.1
[text/plain] v4-0002-Move-common-connection-log-content-verification-c.patch (4.9K, ../../[email protected]/3-v4-0002-Move-common-connection-log-content-verification-c.patch)
download | inline diff:
From 966ea0c337a187522bcabc877214761d9818b58a Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Sat, 27 May 2023 05:36:34 +0530
Subject: [PATCH v4 2/3] Move common connection log content verification code
to a common function.
Move common connection log content verification code to a common
function.
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 130 +++++++++++------------
1 file changed, 64 insertions(+), 66 deletions(-)
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 4df7dd4dec..912892e28b 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2168,6 +2168,68 @@ sub pgbench
=pod
+=item $node->check_connect_log_contents($offset, $test_name, %parameters)
+
+Check connection log contents.
+
+=over
+
+=item $test_name
+
+Name of test for error messages.
+
+=item $offset
+
+Offset of the log file.
+
+=item log_like => [ qr/required message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must match against the server log, using
+C<Test::More::like()>.
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must NOT match against the server log. They will be
+passed to C<Test::More::unlike()>.
+
+=back
+
+=cut
+
+sub check_connect_log_contents
+{
+ my ($self, $test_name, $offset, %params) = @_;
+
+ my (@log_like, @log_unlike);
+ if (defined($params{log_like}))
+ {
+ @log_like = @{ $params{log_like} };
+ }
+ if (defined($params{log_unlike}))
+ {
+ @log_unlike = @{ $params{log_unlike} };
+ }
+
+ if (@log_like or @log_unlike)
+ {
+ my $log_contents =
+ PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset);
+
+ while (my $regex = shift @log_like)
+ {
+ like($log_contents, $regex, "$test_name: log matches");
+ }
+ while (my $regex = shift @log_unlike)
+ {
+ unlike($log_contents, $regex, "$test_name: log does not match");
+ }
+ }
+}
+
+=pod
+
=item $node->connect_ok($connstr, $test_name, %params)
Attempt a connection with a custom connection string. This is expected
@@ -2184,18 +2246,6 @@ instead of the default.
If this regular expression is set, matches it with the output generated.
-=item log_like => [ qr/required message/ ]
-
-If given, it must be an array reference containing a list of regular
-expressions that must match against the server log, using
-C<Test::More::like()>.
-
-=item log_unlike => [ qr/prohibited message/ ]
-
-If given, it must be an array reference containing a list of regular
-expressions that must NOT match against the server log. They will be
-passed to C<Test::More::unlike()>.
-
=back
=cut
@@ -2215,16 +2265,6 @@ sub connect_ok
$sql = "SELECT \$\$connected with $connstr\$\$";
}
- my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
-
my $log_location = -s $self->logfile;
# Never prompt for a password, any callers of this routine should
@@ -2245,20 +2285,7 @@ sub connect_ok
is($stderr, "", "$test_name: no stderr");
- if (@log_like or @log_unlike)
- {
- my $log_contents =
- PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
+ $self->check_connect_log_contents($test_name, $log_location, %params);
}
=pod
@@ -2274,12 +2301,6 @@ to fail.
If this regular expression is set, matches it with the output generated.
-=item log_like => [ qr/required message/ ]
-
-=item log_unlike => [ qr/prohibited message/ ]
-
-See C<connect_ok(...)>, above.
-
=back
=cut
@@ -2289,16 +2310,6 @@ sub connect_fails
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($self, $connstr, $test_name, %params) = @_;
- my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
-
my $log_location = -s $self->logfile;
# Never prompt for a password, any callers of this routine should
@@ -2316,20 +2327,7 @@ sub connect_fails
like($stderr, $params{expected_stderr}, "$test_name: matches");
}
- if (@log_like or @log_unlike)
- {
- my $log_contents =
- PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
+ $self->check_connect_log_contents($test_name, $log_location, %params);
}
=pod
--
2.40.1
[text/plain] v4-0003-Adjust-a-bit-previous-patches.patch (5.5K, ../../[email protected]/4-v4-0003-Adjust-a-bit-previous-patches.patch)
download | inline diff:
From fce041c5223d14b8bc654461e344f098df95c52a Mon Sep 17 00:00:00 2001
From: Michael Paquier <[email protected]>
Date: Sat, 3 Jun 2023 18:15:42 -0400
Subject: [PATCH v4 3/3] Adjust a bit previous patches ;)
---
src/test/authentication/t/003_peer.pl | 4 +-
src/test/perl/PostgreSQL/Test/Cluster.pm | 139 +++++++++++---------
src/test/recovery/t/033_replay_tsp_drops.pl | 2 +-
3 files changed, 78 insertions(+), 67 deletions(-)
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 2a035c2d0d..d8e4976072 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -81,8 +81,8 @@ reset_pg_hba($node, 'peer');
my $log_offset = -s $node->logfile;
$node->psql('postgres');
if ($node->log_contains(
- qr/peer authentication is not supported on this platform/),
- $log_offset,)
+ qr/peer authentication is not supported on this platform/,
+ $log_offset))
{
plan skip_all => 'peer authentication is not supported on this platform';
}
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 912892e28b..19cbc46390 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2168,68 +2168,6 @@ sub pgbench
=pod
-=item $node->check_connect_log_contents($offset, $test_name, %parameters)
-
-Check connection log contents.
-
-=over
-
-=item $test_name
-
-Name of test for error messages.
-
-=item $offset
-
-Offset of the log file.
-
-=item log_like => [ qr/required message/ ]
-
-If given, it must be an array reference containing a list of regular
-expressions that must match against the server log, using
-C<Test::More::like()>.
-
-=item log_unlike => [ qr/prohibited message/ ]
-
-If given, it must be an array reference containing a list of regular
-expressions that must NOT match against the server log. They will be
-passed to C<Test::More::unlike()>.
-
-=back
-
-=cut
-
-sub check_connect_log_contents
-{
- my ($self, $test_name, $offset, %params) = @_;
-
- my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
-
- if (@log_like or @log_unlike)
- {
- my $log_contents =
- PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
-}
-
-=pod
-
=item $node->connect_ok($connstr, $test_name, %params)
Attempt a connection with a custom connection string. This is expected
@@ -2246,6 +2184,12 @@ instead of the default.
If this regular expression is set, matches it with the output generated.
+=item log_like => [ qr/required message/ ]
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+See C<log_check(...)>.
+
=back
=cut
@@ -2285,7 +2229,7 @@ sub connect_ok
is($stderr, "", "$test_name: no stderr");
- $self->check_connect_log_contents($test_name, $log_location, %params);
+ $self->log_check($test_name, $log_location, %params);
}
=pod
@@ -2301,6 +2245,12 @@ to fail.
If this regular expression is set, matches it with the output generated.
+=item log_like => [ qr/required message/ ]
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+See C<log_check(...)>.
+
=back
=cut
@@ -2327,7 +2277,7 @@ sub connect_fails
like($stderr, $params{expected_stderr}, "$test_name: matches");
}
- $self->check_connect_log_contents($test_name, $log_location, %params);
+ $self->log_check($test_name, $log_location, %params);
}
=pod
@@ -2533,6 +2483,67 @@ sub log_content
return PostgreSQL::Test::Utils::slurp_file($self->logfile);
}
+=pod
+
+=item $node->log_check($offset, $test_name, %parameters)
+
+Check contents of server logs.
+
+=over
+
+=item $test_name
+
+Name of test for error messages.
+
+=item $offset
+
+Offset of the log file.
+
+=item log_like => [ qr/required message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must match against the server log, using
+C<Test::More::like()>.
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must NOT match against the server log. They will be
+passed to C<Test::More::unlike()>.
+
+=back
+
+=cut
+
+sub log_check
+{
+ my ($self, $test_name, $offset, %params) = @_;
+
+ my (@log_like, @log_unlike);
+ if (defined($params{log_like}))
+ {
+ @log_like = @{ $params{log_like} };
+ }
+ if (defined($params{log_unlike}))
+ {
+ @log_unlike = @{ $params{log_unlike} };
+ }
+
+ if (@log_like or @log_unlike)
+ {
+ my $log_contents =
+ PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset);
+
+ while (my $regex = shift @log_like)
+ {
+ like($log_contents, $regex, "$test_name: log matches");
+ }
+ while (my $regex = shift @log_unlike)
+ {
+ unlike($log_contents, $regex, "$test_name: log does not match");
+ }
+ }
+}
=pod
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 307c30bc6b..af97ed9f70 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -142,4 +142,4 @@ while ($max_attempts-- >= 0)
}
ok($max_attempts > 0, "invalid directory creation is detected");
-done_testing();
\ No newline at end of file
+done_testing();
--
2.40.1
[application/pgp-signature] signature.asc (833B, ../../[email protected]/5-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Implement generalized sub routine find_in_log for tap test
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-03 22:21 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
@ 2023-06-05 16:09 ` vignesh C <[email protected]>
2023-06-06 04:30 ` Re: Implement generalized sub routine find_in_log for tap test Bharath Rupireddy <[email protected]>
1 sibling, 1 reply; 15+ messages in thread
From: vignesh C @ 2023-06-05 16:09 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On Sun, 4 Jun 2023 at 03:51, Michael Paquier <[email protected]> wrote:
>
> On Mon, May 29, 2023 at 07:49:52AM +0530, vignesh C wrote:
> > Thanks for the comment, the attached v3 version patch has the changes
> > for the same.
>
> -if (find_in_log(
> - $node, $log_offset,
> - qr/peer authentication is not supported on this platform/))
> +if ($node->log_contains(
> + qr/peer authentication is not supported on this platform/),
> + $log_offset,)
>
> This looks like a typo to me, the log offset is eaten.
>
> Except of that, I am on board with log_contains().
Thanks for fixing this.
> There are two things that bugged me with the refactoring related to
> connect_ok and connect_fails:
> - check_connect_log_contents() is a name too complicated. While
> looking at that I have settled to a simpler log_check(), as we could
> perfectly use that for something else than connections as long as we
> want to check multiple patterns at once.
> - The refactoring of the documentation for the routines of Cluster.pm
> became incorrect. For example, the patch does not list anymore
> log_like and log_unlike for connect_ok.
This new name suggested by you looks simpler, your documentation of
having it in connect_ok and connect_fails and referring it to
log_check makes it more clearer.
Regards,
Vignesh
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Implement generalized sub routine find_in_log for tap test
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-03 22:21 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
2023-06-05 16:09 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
@ 2023-06-06 04:30 ` Bharath Rupireddy <[email protected]>
2023-06-06 10:41 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Bharath Rupireddy @ 2023-06-06 04:30 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Michael Paquier <[email protected]>; Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On Mon, Jun 5, 2023 at 9:39 PM vignesh C <[email protected]> wrote:
>
> On Sun, 4 Jun 2023 at 03:51, Michael Paquier <[email protected]> wrote:
> >
> > This looks like a typo to me, the log offset is eaten.
> >
> > Except of that, I am on board with log_contains().
>
> Thanks for fixing this.
+1 for deduplicating find_in_log. How about deduplicating advance_wal
too so that 019_replslot_limit.pl, 033_replay_tsp_drops.pl,
035_standby_logical_decoding.pl and 001_stream_rep.pl can benefit
immediately?
FWIW, a previous discussion related to this is here
https://www.postgresql.org/message-id/flat/CALj2ACVUcXtLgHRPbx28ZQQyRM6j%2BeSH3jNUALr2pJ4%2Bf%3DHRGA....
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Implement generalized sub routine find_in_log for tap test
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-03 22:21 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
2023-06-05 16:09 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-06 04:30 ` Re: Implement generalized sub routine find_in_log for tap test Bharath Rupireddy <[email protected]>
@ 2023-06-06 10:41 ` Michael Paquier <[email protected]>
2023-06-06 12:23 ` Re: Implement generalized sub routine find_in_log for tap test Bharath Rupireddy <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2023-06-06 10:41 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: vignesh C <[email protected]>; Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Jun 06, 2023 at 10:00:00AM +0530, Bharath Rupireddy wrote:
> +1 for deduplicating find_in_log. How about deduplicating advance_wal
> too so that 019_replslot_limit.pl, 033_replay_tsp_drops.pl,
> 035_standby_logical_decoding.pl and 001_stream_rep.pl can benefit
> immediately?
As in a small wrapper for pg_switch_wal() that generates N segments at
will? I don't see why we could not do that if it proves useful in the
long run.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Implement generalized sub routine find_in_log for tap test
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-03 22:21 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
2023-06-05 16:09 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-06 04:30 ` Re: Implement generalized sub routine find_in_log for tap test Bharath Rupireddy <[email protected]>
2023-06-06 10:41 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
@ 2023-06-06 12:23 ` Bharath Rupireddy <[email protected]>
2023-06-09 02:59 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Bharath Rupireddy @ 2023-06-06 12:23 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: vignesh C <[email protected]>; Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Jun 6, 2023 at 4:11 PM Michael Paquier <[email protected]> wrote:
>
> On Tue, Jun 06, 2023 at 10:00:00AM +0530, Bharath Rupireddy wrote:
> > +1 for deduplicating find_in_log. How about deduplicating advance_wal
> > too so that 019_replslot_limit.pl, 033_replay_tsp_drops.pl,
> > 035_standby_logical_decoding.pl and 001_stream_rep.pl can benefit
> > immediately?
>
> As in a small wrapper for pg_switch_wal() that generates N segments at
> will?
Yes. A simpler way of doing it would be to move advance_wal() in
019_replslot_limit.pl to Cluster.pm, something like the attached. CI
members don't complain with it
https://github.com/BRupireddy/postgres/tree/add_a_function_in_Cluster.pm_to_generate_WAL.
Perhaps, we can name it better instead of advance_wal, say
generate_wal or some other?
> I don't see why we could not do that if it proves useful in the
> long run.
Besides the beneficiaries listed above, the test case added by
https://commitfest.postgresql.org/43/3663/ can use it. And, the
test_table bits in 020_pg_receivewal.pl can use it (?).
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Attachments:
[application/octet-stream] v1-0001-Add-a-function-in-Cluster.pm-to-generate-WAL (6.4K, ../../CALj2ACVE+vQK9gDKoNqSm+-aJxY=6OY7QJYFoc4oNFqy-BU_bA@mail.gmail.com/2-v1-0001-Add-a-function-in-Cluster.pm-to-generate-WAL)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Implement generalized sub routine find_in_log for tap test
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-03 22:21 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
2023-06-05 16:09 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-06 04:30 ` Re: Implement generalized sub routine find_in_log for tap test Bharath Rupireddy <[email protected]>
2023-06-06 10:41 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
2023-06-06 12:23 ` Re: Implement generalized sub routine find_in_log for tap test Bharath Rupireddy <[email protected]>
@ 2023-06-09 02:59 ` Michael Paquier <[email protected]>
2023-06-11 01:47 ` Re: Implement generalized sub routine find_in_log for tap test Bharath Rupireddy <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2023-06-09 02:59 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: vignesh C <[email protected]>; Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Jun 06, 2023 at 05:53:40PM +0530, Bharath Rupireddy wrote:
> Yes. A simpler way of doing it would be to move advance_wal() in
> 019_replslot_limit.pl to Cluster.pm, something like the attached. CI
> members don't complain with it
> https://github.com/BRupireddy/postgres/tree/add_a_function_in_Cluster.pm_to_generate_WAL.
> Perhaps, we can name it better instead of advance_wal, say
> generate_wal or some other?
Why not discussing that on a separate thread? What you are proposing
is independent of what Vignesh has proposed. Note that the patch
format is octet-stream, causing extra CRs to exist in the patch.
Something happened on your side when you sent your patch, I guess?
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Implement generalized sub routine find_in_log for tap test
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-03 22:21 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
2023-06-05 16:09 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-06 04:30 ` Re: Implement generalized sub routine find_in_log for tap test Bharath Rupireddy <[email protected]>
2023-06-06 10:41 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
2023-06-06 12:23 ` Re: Implement generalized sub routine find_in_log for tap test Bharath Rupireddy <[email protected]>
2023-06-09 02:59 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
@ 2023-06-11 01:47 ` Bharath Rupireddy <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: Bharath Rupireddy @ 2023-06-11 01:47 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: vignesh C <[email protected]>; Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, Jun 9, 2023 at 8:29 AM Michael Paquier <[email protected]> wrote:
>
> On Tue, Jun 06, 2023 at 05:53:40PM +0530, Bharath Rupireddy wrote:
> > Yes. A simpler way of doing it would be to move advance_wal() in
> > 019_replslot_limit.pl to Cluster.pm, something like the attached. CI
> > members don't complain with it
> > https://github.com/BRupireddy/postgres/tree/add_a_function_in_Cluster.pm_to_generate_WAL.
> > Perhaps, we can name it better instead of advance_wal, say
> > generate_wal or some other?
>
> Why not discussing that on a separate thread? What you are proposing
> is independent of what Vignesh has proposed.
Sure. Here it is -
https://www.postgresql.org/message-id/CALj2ACU3R8QFCvDewHCMKjgb2w_-CMCyd6DAK%3DJb-af14da5eg%40mail.g....
> Note that the patch
> format is octet-stream, causing extra CRs to exist in the patch.
> Something happened on your side when you sent your patch, I guess?
Had to attach the patch in .txt format to not block Vignesh's patch
from testing by CF Bot (if at all this thread was added there).
--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Implement generalized sub routine find_in_log for tap test
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-03 22:21 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
@ 2023-06-06 02:35 ` Amit Kapila <[email protected]>
2023-06-06 04:06 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
1 sibling, 1 reply; 15+ messages in thread
From: Amit Kapila @ 2023-06-06 02:35 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: vignesh C <[email protected]>; Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On Sun, Jun 4, 2023 at 3:51 AM Michael Paquier <[email protected]> wrote:
>
> On Mon, May 29, 2023 at 07:49:52AM +0530, vignesh C wrote:
> > Thanks for the comment, the attached v3 version patch has the changes
> > for the same.
>
> -if (find_in_log(
> - $node, $log_offset,
> - qr/peer authentication is not supported on this platform/))
> +if ($node->log_contains(
> + qr/peer authentication is not supported on this platform/),
> + $log_offset,)
>
> This looks like a typo to me, the log offset is eaten.
>
> Except of that, I am on board with log_contains().
>
> There are two things that bugged me with the refactoring related to
> connect_ok and connect_fails:
> - check_connect_log_contents() is a name too complicated. While
> looking at that I have settled to a simpler log_check(), as we could
> perfectly use that for something else than connections as long as we
> want to check multiple patterns at once.
> - The refactoring of the documentation for the routines of Cluster.pm
> became incorrect. For example, the patch does not list anymore
> log_like and log_unlike for connect_ok.
>
> With all that in mind I have hacked a few adjustments in a 0003,
> though I agree with the separation between 0001 and 0002.
>
> 033_replay_tsp_drops and 019_replslot_limit are not new to v16, but
> 003_peer.pl and 035_standby_logical_decoding.pl, making the number of
> places where find_in_log() exists twice as much. So I would be
> tempted to refactor these tests in v16. Perhaps anybody from the RMT
> could comment? We've usually been quite flexible with the tests even
> in beta.
>
Personally, I don't see any problem to do this refactoring for v16.
However, sometimes, we do decide to backpatch refactoring in tests to
avoid backpatch effort. I am not completely sure if that is the case
here.
--
With Regards,
Amit Kapila.
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Implement generalized sub routine find_in_log for tap test
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-03 22:21 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
2023-06-06 02:35 ` Re: Implement generalized sub routine find_in_log for tap test Amit Kapila <[email protected]>
@ 2023-06-06 04:06 ` Michael Paquier <[email protected]>
2023-06-06 13:13 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2023-06-06 04:06 UTC (permalink / raw)
To: Amit Kapila <[email protected]>; +Cc: vignesh C <[email protected]>; Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Jun 06, 2023 at 08:05:49AM +0530, Amit Kapila wrote:
> Personally, I don't see any problem to do this refactoring for v16.
> However, sometimes, we do decide to backpatch refactoring in tests to
> avoid backpatch effort. I am not completely sure if that is the case
> here.
033_replay_tsp_drops.pl has one find_in_log() down to 11, and
019_replslot_limit.pl has four calls down to 14. Making things
consistent everywhere is a rather appealing argument to ease future
backpatching. So I am OK to spend a few extra cycles in adjusting
these routines all the way down where needed. I'll do that tomorrow
once I get back in front of my laptop.
Note that connect_ok() and connect_fails() are new to 14, so this
part has no need to go further down than that.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Implement generalized sub routine find_in_log for tap test
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-03 22:21 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
2023-06-06 02:35 ` Re: Implement generalized sub routine find_in_log for tap test Amit Kapila <[email protected]>
2023-06-06 04:06 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
@ 2023-06-06 13:13 ` vignesh C <[email protected]>
2023-06-09 03:01 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: vignesh C @ 2023-06-06 13:13 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Amit Kapila <[email protected]>; Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, 6 Jun 2023 at 09:36, Michael Paquier <[email protected]> wrote:
>
> On Tue, Jun 06, 2023 at 08:05:49AM +0530, Amit Kapila wrote:
> > Personally, I don't see any problem to do this refactoring for v16.
> > However, sometimes, we do decide to backpatch refactoring in tests to
> > avoid backpatch effort. I am not completely sure if that is the case
> > here.
>
> 033_replay_tsp_drops.pl has one find_in_log() down to 11, and
> 019_replslot_limit.pl has four calls down to 14. Making things
> consistent everywhere is a rather appealing argument to ease future
> backpatching. So I am OK to spend a few extra cycles in adjusting
> these routines all the way down where needed. I'll do that tomorrow
> once I get back in front of my laptop.
>
> Note that connect_ok() and connect_fails() are new to 14, so this
> part has no need to go further down than that.
Please find the attached patches that can be applied on back branches
too. v5*master.patch can be applied on master, v5*PG15.patch can be
applied on PG15, v5*PG14.patch can be applied on PG14, v5*PG13.patch
can be applied on PG13, v5*PG12.patch can be applied on PG12, PG11 and
PG10.
Regards,
Vignesh
Attachments:
[text/x-patch] v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_PG13.patch (3.7K, ../../CALDaNm2TK623jrGQomqwH1JFd8DdFSa9epoZJf279AXb1S2GGQ@mail.gmail.com/2-v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_PG13.patch)
download | inline diff:
From d582676214f3fd5068c343f42116fa256be18958 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Tue, 6 Jun 2023 17:07:37 +0530
Subject: [PATCH v5] Remove duplicate find_in_log sub routines from tap tests.
Remove duplicate find_in_log sub routines from tap tests.
---
src/test/perl/PostgresNode.pm | 15 +++++++++++++
src/test/recovery/t/019_replslot_limit.pl | 25 ++++-----------------
src/test/recovery/t/033_replay_tsp_drops.pl | 14 ++----------
3 files changed, 21 insertions(+), 33 deletions(-)
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index b7e4dbd0e8..ce924de0b2 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -2081,6 +2081,21 @@ sub issues_sql_like
=pod
+=item $node->log_contains(pattern, offset)
+
+Find pattern in logfile of node after offset byte.
+
+=cut
+
+sub log_contains
+{
+ my ($self, $pattern, $offset) = @_;
+
+ return TestLib::slurp_file($self->logfile, $offset) =~ m/$pattern/;
+}
+
+=pod
+
=item $node->run_log(...)
Runs a shell command like TestLib::run_log, but with connection parameters set
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index 7a8954dad2..d36c954588 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -165,8 +165,7 @@ $node_master->wait_for_catchup($node_standby, 'replay', $start_lsn);
$node_standby->stop;
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed"),
'check that required WAL segments are still available');
@@ -188,8 +187,7 @@ $node_master->safe_psql('postgres', "CHECKPOINT;");
my $invalidated = 0;
for (my $i = 0; $i < 10000; $i++)
{
- if (find_in_log(
- $node_master,
+ if ($node_master->log_contains(
"invalidating slot \"rep1\" because its restart_lsn [0-9A-F/]+ exceeds max_slot_wal_keep_size",
$logstart))
{
@@ -212,7 +210,7 @@ is($result, "rep1|f|t|lost|",
my $checkpoint_ended = 0;
for (my $i = 0; $i < 10000; $i++)
{
- if (find_in_log($node_master, "checkpoint complete: ", $logstart))
+ if ($node_master->log_contains("checkpoint complete: ", $logstart))
{
$checkpoint_ended = 1;
last;
@@ -242,8 +240,7 @@ $node_standby->start;
my $failed = 0;
for (my $i = 0; $i < 10000; $i++)
{
- if (find_in_log(
- $node_standby,
+ if ($node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed",
$logstart))
{
@@ -318,17 +315,3 @@ sub get_log_size
return (stat $node->logfile)[7];
}
-
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- $off = 0 unless defined $off;
- my $log = TestLib::slurp_file($node->logfile);
- return 0 if (length($log) <= $off);
-
- $log = substr($log, $off);
-
- return $log =~ m/$pat/;
-}
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 140d94db31..4c8f4e7970 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -132,21 +132,11 @@ while ($max_attempts-- >= 0)
{
last
if (
- find_in_log(
- $node_standby, qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
+ $node_standby->log_contains(
+ qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
$logstart));
usleep(100_000);
}
ok($max_attempts > 0, "invalid directory creation is detected");
done_testing();
-
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $off);
-
- return $log =~ m/$pat/;
-}
--
2.34.1
[text/x-patch] v5-0001-Have-find_in_log-sub-routine-as-a-common-routine_PG12.patch (1.9K, ../../CALDaNm2TK623jrGQomqwH1JFd8DdFSa9epoZJf279AXb1S2GGQ@mail.gmail.com/3-v5-0001-Have-find_in_log-sub-routine-as-a-common-routine_PG12.patch)
download | inline diff:
From 7570e55a9f0121a9e2e0a12f4e8718cc531811c9 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Tue, 6 Jun 2023 17:20:34 +0530
Subject: [PATCH v5] Have find_in_log sub routine as a common routine available
for all tap tests.
Have find_in_log sub routine as a common routine available for all tap
tests.
---
src/test/perl/PostgresNode.pm | 15 +++++++++++++++
src/test/recovery/t/033_replay_tsp_drops.pl | 14 ++------------
2 files changed, 17 insertions(+), 12 deletions(-)
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 6506308ff1..8fae4eb1bb 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -1972,6 +1972,21 @@ sub issues_sql_like
=pod
+=item $node->log_contains(pattern, offset)
+
+Find pattern in logfile of node after offset byte.
+
+=cut
+
+sub log_contains
+{
+ my ($self, $pattern, $offset) = @_;
+
+ return TestLib::slurp_file($self->logfile, $offset) =~ m/$pattern/;
+}
+
+=pod
+
=item $node->run_log(...)
Runs a shell command like TestLib::run_log, but with connection parameters set
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 140d94db31..4c8f4e7970 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -132,21 +132,11 @@ while ($max_attempts-- >= 0)
{
last
if (
- find_in_log(
- $node_standby, qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
+ $node_standby->log_contains(
+ qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
$logstart));
usleep(100_000);
}
ok($max_attempts > 0, "invalid directory creation is detected");
done_testing();
-
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $off);
-
- return $log =~ m/$pat/;
-}
--
2.34.1
[text/x-patch] v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_PG14.patch (4.3K, ../../CALDaNm2TK623jrGQomqwH1JFd8DdFSa9epoZJf279AXb1S2GGQ@mail.gmail.com/4-v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_PG14.patch)
download | inline diff:
From 94a9c3e62df5c4044271bada78af8fa5869f0a26 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Tue, 6 Jun 2023 16:40:17 +0530
Subject: [PATCH v5 1/2] Remove duplicate find_in_log sub routines from tap
tests.
Remove duplicate find_in_log sub routines from tap tests.
---
src/test/perl/PostgresNode.pm | 15 ++++++++++
src/test/recovery/t/019_replslot_limit.pl | 31 ++++-----------------
src/test/recovery/t/033_replay_tsp_drops.pl | 14 ++--------
3 files changed, 23 insertions(+), 37 deletions(-)
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index cce3f621e7..b48bf17a40 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -2459,6 +2459,21 @@ sub issues_sql_like
=pod
+=item $node->log_contains(pattern, offset)
+
+Find pattern in logfile of node after offset byte.
+
+=cut
+
+sub log_contains
+{
+ my ($self, $pattern, $offset) = @_;
+
+ return TestLib::slurp_file($self->logfile, $offset) =~ m/$pattern/;
+}
+
+=pod
+
=item $node->run_log(...)
Runs a shell command like TestLib::run_log, but with connection parameters set
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index f6dcf0af1d..1eec98f2c9 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -168,8 +168,7 @@ $node_primary->wait_for_catchup($node_standby, 'replay', $start_lsn);
$node_standby->stop;
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed"),
'check that required WAL segments are still available');
@@ -191,8 +190,7 @@ $node_primary->safe_psql('postgres', "CHECKPOINT;");
my $invalidated = 0;
for (my $i = 0; $i < 10000; $i++)
{
- if (find_in_log(
- $node_primary,
+ if ($node_primary->log_contains(
"invalidating slot \"rep1\" because its restart_lsn [0-9A-F/]+ exceeds max_slot_wal_keep_size",
$logstart))
{
@@ -215,7 +213,7 @@ is($result, "rep1|f|t|lost|",
my $checkpoint_ended = 0;
for (my $i = 0; $i < 10000; $i++)
{
- if (find_in_log($node_primary, "checkpoint complete: ", $logstart))
+ if ($node_primary->log_contains("checkpoint complete: ", $logstart))
{
$checkpoint_ended = 1;
last;
@@ -245,8 +243,7 @@ $node_standby->start;
my $failed = 0;
for (my $i = 0; $i < 10000; $i++)
{
- if (find_in_log(
- $node_standby,
+ if ($node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed",
$logstart))
{
@@ -351,8 +348,7 @@ advance_wal($node_primary3, 2);
my $max_attempts = $TestLib::timeout_default;
while ($max_attempts-- >= 0)
{
- if (find_in_log(
- $node_primary3,
+ if ($node_primary3->log_contains(
"terminating process $senderpid to release replication slot \"rep3\"",
$logstart))
{
@@ -374,8 +370,7 @@ $node_primary3->poll_query_until('postgres',
$max_attempts = $TestLib::timeout_default;
while ($max_attempts-- >= 0)
{
- if (find_in_log(
- $node_primary3,
+ if ($node_primary3->log_contains(
'invalidating slot "rep3" because its restart_lsn', $logstart))
{
ok(1, "slot invalidation logged");
@@ -412,17 +407,3 @@ sub get_log_size
return (stat $node->logfile)[7];
}
-
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- $off = 0 unless defined $off;
- my $log = TestLib::slurp_file($node->logfile);
- return 0 if (length($log) <= $off);
-
- $log = substr($log, $off);
-
- return $log =~ m/$pat/;
-}
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 140d94db31..4c8f4e7970 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -132,21 +132,11 @@ while ($max_attempts-- >= 0)
{
last
if (
- find_in_log(
- $node_standby, qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
+ $node_standby->log_contains(
+ qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
$logstart));
usleep(100_000);
}
ok($max_attempts > 0, "invalid directory creation is detected");
done_testing();
-
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $off);
-
- return $log =~ m/$pat/;
-}
--
2.34.1
[text/x-patch] v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_master.patch (7.6K, ../../CALDaNm2TK623jrGQomqwH1JFd8DdFSa9epoZJf279AXb1S2GGQ@mail.gmail.com/5-v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_master.patch)
download | inline diff:
From e831e26c06994c656b61a99dee86c4710d5e37a1 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Fri, 26 May 2023 20:07:30 +0530
Subject: [PATCH v5 1/2] Remove duplicate find_in_log sub routines from tap
tests.
Remove duplicate find_in_log sub routines from tap tests.
---
src/test/authentication/t/003_peer.pl | 17 ++--------
src/test/perl/PostgreSQL/Test/Cluster.pm | 15 +++++++++
src/test/recovery/t/019_replslot_limit.pl | 33 +++++--------------
src/test/recovery/t/033_replay_tsp_drops.pl | 13 +-------
.../t/035_standby_logical_decoding.pl | 26 +++------------
5 files changed, 31 insertions(+), 73 deletions(-)
diff --git a/src/test/authentication/t/003_peer.pl b/src/test/authentication/t/003_peer.pl
index 3272e52cae..d8e4976072 100644
--- a/src/test/authentication/t/003_peer.pl
+++ b/src/test/authentication/t/003_peer.pl
@@ -69,17 +69,6 @@ sub test_role
}
}
-# Find $pattern in log file of $node.
-sub find_in_log
-{
- my ($node, $offset, $pattern) = @_;
-
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $offset);
- return 0 if (length($log) <= 0);
-
- return $log =~ m/$pattern/;
-}
-
my $node = PostgreSQL::Test::Cluster->new('node');
$node->init;
$node->append_conf('postgresql.conf', "log_connections = on\n");
@@ -91,9 +80,9 @@ reset_pg_hba($node, 'peer');
# Check if peer authentication is supported on this platform.
my $log_offset = -s $node->logfile;
$node->psql('postgres');
-if (find_in_log(
- $node, $log_offset,
- qr/peer authentication is not supported on this platform/))
+if ($node->log_contains(
+ qr/peer authentication is not supported on this platform/,
+ $log_offset))
{
plan skip_all => 'peer authentication is not supported on this platform';
}
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index baea0fcd1c..4df7dd4dec 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2536,6 +2536,21 @@ sub log_content
}
+=pod
+
+=item log_contains(pattern, offset)
+
+Find pattern in logfile of node after offset byte.
+
+=cut
+
+sub log_contains
+{
+ my ($self, $pattern, $offset) = @_;
+
+ return PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset) =~ m/$pattern/;
+}
+
=pod
=item $node->run_log(...)
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index a1aba16e14..95acf9e357 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -161,8 +161,7 @@ $node_primary->wait_for_catchup($node_standby);
$node_standby->stop;
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed"),
'check that required WAL segments are still available');
@@ -184,8 +183,8 @@ $node_primary->safe_psql('postgres', "CHECKPOINT;");
my $invalidated = 0;
for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
{
- if (find_in_log(
- $node_primary, 'invalidating obsolete replication slot "rep1"',
+ if ($node_primary->log_contains(
+ 'invalidating obsolete replication slot "rep1"',
$logstart))
{
$invalidated = 1;
@@ -207,7 +206,7 @@ is($result, "rep1|f|t|lost|",
my $checkpoint_ended = 0;
for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
{
- if (find_in_log($node_primary, "checkpoint complete: ", $logstart))
+ if ($node_primary->log_contains("checkpoint complete: ", $logstart))
{
$checkpoint_ended = 1;
last;
@@ -237,8 +236,7 @@ $node_standby->start;
my $failed = 0;
for (my $i = 0; $i < 10 * $PostgreSQL::Test::Utils::timeout_default; $i++)
{
- if (find_in_log(
- $node_standby,
+ if ($node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed",
$logstart))
{
@@ -381,8 +379,7 @@ my $msg_logged = 0;
my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
while ($max_attempts-- >= 0)
{
- if (find_in_log(
- $node_primary3,
+ if ($node_primary3->log_contains(
"terminating process $senderpid to release replication slot \"rep3\"",
$logstart))
{
@@ -406,8 +403,8 @@ $msg_logged = 0;
$max_attempts = $PostgreSQL::Test::Utils::timeout_default;
while ($max_attempts-- >= 0)
{
- if (find_in_log(
- $node_primary3, 'invalidating obsolete replication slot "rep3"',
+ if ($node_primary3->log_contains(
+ 'invalidating obsolete replication slot "rep3"',
$logstart))
{
$msg_logged = 1;
@@ -446,18 +443,4 @@ sub get_log_size
return (stat $node->logfile)[7];
}
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- $off = 0 unless defined $off;
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
- return 0 if (length($log) <= $off);
-
- $log = substr($log, $off);
-
- return $log =~ m/$pat/;
-}
-
done_testing();
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 0a35a7bda6..af97ed9f70 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -135,8 +135,7 @@ while ($max_attempts-- >= 0)
{
last
if (
- find_in_log(
- $node_standby,
+ $node_standby->log_contains(
qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
$logstart));
usleep(100_000);
@@ -144,13 +143,3 @@ while ($max_attempts-- >= 0)
ok($max_attempts > 0, "invalid directory creation is detected");
done_testing();
-
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $off);
-
- return $log =~ m/$pat/;
-}
diff --git a/src/test/recovery/t/035_standby_logical_decoding.pl b/src/test/recovery/t/035_standby_logical_decoding.pl
index 64beec4bd3..480e6d6caa 100644
--- a/src/test/recovery/t/035_standby_logical_decoding.pl
+++ b/src/test/recovery/t/035_standby_logical_decoding.pl
@@ -28,20 +28,6 @@ my $res;
my $primary_slotname = 'primary_physical';
my $standby_physical_slotname = 'standby_physical';
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- $off = 0 unless defined $off;
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
- return 0 if (length($log) <= $off);
-
- $log = substr($log, $off);
-
- return $log =~ m/$pat/;
-}
-
# Fetch xmin columns from slot's pg_replication_slots row, after waiting for
# given boolean condition to be true to ensure we've reached a quiescent state.
sub wait_for_xmins
@@ -235,14 +221,12 @@ sub check_for_invalidation
my $inactive_slot = $slot_prefix . 'inactiveslot';
# message should be issued
- ok( find_in_log(
- $node_standby,
+ ok( $node_standby->log_contains(
"invalidating obsolete replication slot \"$inactive_slot\"",
$log_start),
"inactiveslot slot invalidation is logged $test_name");
- ok( find_in_log(
- $node_standby,
+ ok( $node_standby->log_contains(
"invalidating obsolete replication slot \"$active_slot\"",
$log_start),
"activeslot slot invalidation is logged $test_name");
@@ -657,14 +641,12 @@ $node_primary->safe_psql(
$node_primary->wait_for_replay_catchup($node_standby);
# message should not be issued
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"invalidating obsolete slot \"no_conflict_inactiveslot\"", $logstart),
'inactiveslot slot invalidation is not logged with vacuum on conflict_test'
);
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"invalidating obsolete slot \"no_conflict_activeslot\"", $logstart),
'activeslot slot invalidation is not logged with vacuum on conflict_test'
);
--
2.34.1
[text/x-patch] v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_PG15.patch (4.4K, ../../CALDaNm2TK623jrGQomqwH1JFd8DdFSa9epoZJf279AXb1S2GGQ@mail.gmail.com/6-v5-0001-Remove-duplicate-find_in_log-sub-routines-from-ta_PG15.patch)
download | inline diff:
From a91e8b12840dfe9aaea425f1cf0255d5849cbcb0 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Tue, 6 Jun 2023 15:48:50 +0530
Subject: [PATCH v5 1/2] Remove duplicate find_in_log sub routines from tap
tests.
Remove duplicate find_in_log sub routines from tap tests.
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 15 ++++++++++
src/test/recovery/t/019_replslot_limit.pl | 31 ++++-----------------
src/test/recovery/t/033_replay_tsp_drops.pl | 14 ++--------
3 files changed, 23 insertions(+), 37 deletions(-)
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 69a8ff3d74..5063e77368 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2559,6 +2559,21 @@ sub issues_sql_like
=pod
+=item log_contains(pattern, offset)
+
+Find pattern in logfile of node after offset byte.
+
+=cut
+
+sub log_contains
+{
+ my ($self, $pattern, $offset) = @_;
+
+ return PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset) =~ m/$pattern/;
+}
+
+=pod
+
=item $node->run_log(...)
Runs a shell command like PostgreSQL::Test::Utils::run_log, but with connection parameters set
diff --git a/src/test/recovery/t/019_replslot_limit.pl b/src/test/recovery/t/019_replslot_limit.pl
index 84c9c4b0b3..4ec1a9ab33 100644
--- a/src/test/recovery/t/019_replslot_limit.pl
+++ b/src/test/recovery/t/019_replslot_limit.pl
@@ -163,8 +163,7 @@ $node_primary->wait_for_catchup($node_standby);
$node_standby->stop;
-ok( !find_in_log(
- $node_standby,
+ok( !$node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed"),
'check that required WAL segments are still available');
@@ -186,8 +185,7 @@ $node_primary->safe_psql('postgres', "CHECKPOINT;");
my $invalidated = 0;
for (my $i = 0; $i < 10000; $i++)
{
- if (find_in_log(
- $node_primary,
+ if ($node_primary->log_contains(
"invalidating slot \"rep1\" because its restart_lsn [0-9A-F/]+ exceeds max_slot_wal_keep_size",
$logstart))
{
@@ -210,7 +208,7 @@ is($result, "rep1|f|t|lost|",
my $checkpoint_ended = 0;
for (my $i = 0; $i < 10000; $i++)
{
- if (find_in_log($node_primary, "checkpoint complete: ", $logstart))
+ if ($node_primary->log_contains("checkpoint complete: ", $logstart))
{
$checkpoint_ended = 1;
last;
@@ -240,8 +238,7 @@ $node_standby->start;
my $failed = 0;
for (my $i = 0; $i < 10000; $i++)
{
- if (find_in_log(
- $node_standby,
+ if ($node_standby->log_contains(
"requested WAL segment [0-9A-F]+ has already been removed",
$logstart))
{
@@ -384,8 +381,7 @@ advance_wal($node_primary3, 2);
my $max_attempts = $PostgreSQL::Test::Utils::timeout_default;
while ($max_attempts-- >= 0)
{
- if (find_in_log(
- $node_primary3,
+ if ($node_primary3->log_contains(
"terminating process $senderpid to release replication slot \"rep3\"",
$logstart))
{
@@ -407,8 +403,7 @@ $node_primary3->poll_query_until('postgres',
$max_attempts = $PostgreSQL::Test::Utils::timeout_default;
while ($max_attempts-- >= 0)
{
- if (find_in_log(
- $node_primary3,
+ if ($node_primary3->log_contains(
'invalidating slot "rep3" because its restart_lsn', $logstart))
{
ok(1, "slot invalidation logged");
@@ -446,18 +441,4 @@ sub get_log_size
return (stat $node->logfile)[7];
}
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- $off = 0 unless defined $off;
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile);
- return 0 if (length($log) <= $off);
-
- $log = substr($log, $off);
-
- return $log =~ m/$pat/;
-}
-
done_testing();
diff --git a/src/test/recovery/t/033_replay_tsp_drops.pl b/src/test/recovery/t/033_replay_tsp_drops.pl
index 57fee21276..64c3296a70 100644
--- a/src/test/recovery/t/033_replay_tsp_drops.pl
+++ b/src/test/recovery/t/033_replay_tsp_drops.pl
@@ -138,21 +138,11 @@ while ($max_attempts-- >= 0)
{
last
if (
- find_in_log(
- $node_standby, qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
+ $node_standby->log_contains(
+ qr!WARNING: ( [A-Z0-9]+:)? creating missing directory: pg_tblspc/!,
$logstart));
usleep(100_000);
}
ok($max_attempts > 0, "invalid directory creation is detected");
done_testing();
-
-# find $pat in logfile of $node after $off-th byte
-sub find_in_log
-{
- my ($node, $pat, $off) = @_;
-
- my $log = PostgreSQL::Test::Utils::slurp_file($node->logfile, $off);
-
- return $log =~ m/$pat/;
-}
--
2.34.1
[text/x-patch] v5-0002-Move-common-connection-log-content-verification-c_master.patch (4.7K, ../../CALDaNm2TK623jrGQomqwH1JFd8DdFSa9epoZJf279AXb1S2GGQ@mail.gmail.com/7-v5-0002-Move-common-connection-log-content-verification-c_master.patch)
download | inline diff:
From f96a537129a740fe8998e7201ac052d3e63d4129 Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Sat, 27 May 2023 05:36:34 +0530
Subject: [PATCH v5 2/2] Move common connection log content verification code
to a common function.
Move common connection log content verification code to a common
function.
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 121 ++++++++++++-----------
1 file changed, 65 insertions(+), 56 deletions(-)
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 4df7dd4dec..19cbc46390 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2186,15 +2186,9 @@ If this regular expression is set, matches it with the output generated.
=item log_like => [ qr/required message/ ]
-If given, it must be an array reference containing a list of regular
-expressions that must match against the server log, using
-C<Test::More::like()>.
-
=item log_unlike => [ qr/prohibited message/ ]
-If given, it must be an array reference containing a list of regular
-expressions that must NOT match against the server log. They will be
-passed to C<Test::More::unlike()>.
+See C<log_check(...)>.
=back
@@ -2215,16 +2209,6 @@ sub connect_ok
$sql = "SELECT \$\$connected with $connstr\$\$";
}
- my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
-
my $log_location = -s $self->logfile;
# Never prompt for a password, any callers of this routine should
@@ -2245,20 +2229,7 @@ sub connect_ok
is($stderr, "", "$test_name: no stderr");
- if (@log_like or @log_unlike)
- {
- my $log_contents =
- PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
+ $self->log_check($test_name, $log_location, %params);
}
=pod
@@ -2278,7 +2249,7 @@ If this regular expression is set, matches it with the output generated.
=item log_unlike => [ qr/prohibited message/ ]
-See C<connect_ok(...)>, above.
+See C<log_check(...)>.
=back
@@ -2289,16 +2260,6 @@ sub connect_fails
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($self, $connstr, $test_name, %params) = @_;
- my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
-
my $log_location = -s $self->logfile;
# Never prompt for a password, any callers of this routine should
@@ -2316,20 +2277,7 @@ sub connect_fails
like($stderr, $params{expected_stderr}, "$test_name: matches");
}
- if (@log_like or @log_unlike)
- {
- my $log_contents =
- PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
+ $self->log_check($test_name, $log_location, %params);
}
=pod
@@ -2535,6 +2483,67 @@ sub log_content
return PostgreSQL::Test::Utils::slurp_file($self->logfile);
}
+=pod
+
+=item $node->log_check($offset, $test_name, %parameters)
+
+Check contents of server logs.
+
+=over
+
+=item $test_name
+
+Name of test for error messages.
+
+=item $offset
+
+Offset of the log file.
+
+=item log_like => [ qr/required message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must match against the server log, using
+C<Test::More::like()>.
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must NOT match against the server log. They will be
+passed to C<Test::More::unlike()>.
+
+=back
+
+=cut
+
+sub log_check
+{
+ my ($self, $test_name, $offset, %params) = @_;
+
+ my (@log_like, @log_unlike);
+ if (defined($params{log_like}))
+ {
+ @log_like = @{ $params{log_like} };
+ }
+ if (defined($params{log_unlike}))
+ {
+ @log_unlike = @{ $params{log_unlike} };
+ }
+
+ if (@log_like or @log_unlike)
+ {
+ my $log_contents =
+ PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset);
+
+ while (my $regex = shift @log_like)
+ {
+ like($log_contents, $regex, "$test_name: log matches");
+ }
+ while (my $regex = shift @log_unlike)
+ {
+ unlike($log_contents, $regex, "$test_name: log does not match");
+ }
+ }
+}
=pod
--
2.34.1
[text/x-patch] v5-0002-Move-common-connection-log-content-verification-c_PG14.patch (4.5K, ../../CALDaNm2TK623jrGQomqwH1JFd8DdFSa9epoZJf279AXb1S2GGQ@mail.gmail.com/8-v5-0002-Move-common-connection-log-content-verification-c_PG14.patch)
download | inline diff:
From 16cabb908af170b90cf971f7e47a69712aa2720e Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Tue, 6 Jun 2023 16:51:56 +0530
Subject: [PATCH v5 2/2] Move common connection log content verification code
to a common function.
Move common connection log content verification code to a common
function.
---
src/test/perl/PostgresNode.pm | 117 +++++++++++++++++++---------------
1 file changed, 65 insertions(+), 52 deletions(-)
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index b48bf17a40..22baa4cf80 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -2148,15 +2148,9 @@ If this regular expression is set, matches it with the output generated.
=item log_like => [ qr/required message/ ]
-If given, it must be an array reference containing a list of regular
-expressions that must match against the server log, using
-C<Test::More::like()>.
-
=item log_unlike => [ qr/prohibited message/ ]
-If given, it must be an array reference containing a list of regular
-expressions that must NOT match against the server log. They will be
-passed to C<Test::More::unlike()>.
+See C<log_check(...)>.
=back
@@ -2177,16 +2171,6 @@ sub connect_ok
$sql = "SELECT \$\$connected with $connstr\$\$";
}
- my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
-
my $log_location = -s $self->logfile;
# Never prompt for a password, any callers of this routine should
@@ -2204,19 +2188,7 @@ sub connect_ok
{
like($stdout, $params{expected_stdout}, "$test_name: matches");
}
- if (@log_like or @log_unlike)
- {
- my $log_contents = TestLib::slurp_file($self->logfile, $log_location);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
+ $self->log_check($test_name, $log_location, %params);
}
=pod
@@ -2236,7 +2208,7 @@ If this regular expression is set, matches it with the output generated.
=item log_unlike => [ qr/prohibited message/ ]
-See C<connect_ok(...)>, above.
+See C<log_check(...)>.
=back
@@ -2248,14 +2220,6 @@ sub connect_fails
my ($self, $connstr, $test_name, %params) = @_;
my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
my $log_location = -s $self->logfile;
@@ -2274,19 +2238,7 @@ sub connect_fails
like($stderr, $params{expected_stderr}, "$test_name: matches");
}
- if (@log_like or @log_unlike)
- {
- my $log_contents = TestLib::slurp_file($self->logfile, $log_location);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
+ $self->log_check($test_name, $log_location, %params);
}
=pod
@@ -2459,6 +2411,67 @@ sub issues_sql_like
=pod
+=item $node->log_check($offset, $test_name, %parameters)
+
+Check contents of server logs.
+
+=over
+
+=item $test_name
+
+Name of test for error messages.
+
+=item $offset
+
+Offset of the log file.
+
+=item log_like => [ qr/required message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must match against the server log, using
+C<Test::More::like()>.
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must NOT match against the server log. They will be
+passed to C<Test::More::unlike()>.
+
+=back
+
+=cut
+
+sub log_check
+{
+ my ($self, $test_name, $offset, %params) = @_;
+
+ my (@log_like, @log_unlike);
+ if (defined($params{log_like}))
+ {
+ @log_like = @{ $params{log_like} };
+ }
+ if (defined($params{log_unlike}))
+ {
+ @log_unlike = @{ $params{log_unlike} };
+ }
+
+ if (@log_like or @log_unlike)
+ {
+ my $log_contents = TestLib::slurp_file($self->logfile, $offset);
+
+ while (my $regex = shift @log_like)
+ {
+ like($log_contents, $regex, "$test_name: log matches");
+ }
+ while (my $regex = shift @log_unlike)
+ {
+ unlike($log_contents, $regex, "$test_name: log does not match");
+ }
+ }
+}
+
+=pod
+
=item $node->log_contains(pattern, offset)
Find pattern in logfile of node after offset byte.
--
2.34.1
[text/x-patch] v5-0002-Move-common-connection-log-content-verification-c_PG15.patch (4.7K, ../../CALDaNm2TK623jrGQomqwH1JFd8DdFSa9epoZJf279AXb1S2GGQ@mail.gmail.com/9-v5-0002-Move-common-connection-log-content-verification-c_PG15.patch)
download | inline diff:
From 27d621e922717b01171d12a28d96d699596b992a Mon Sep 17 00:00:00 2001
From: Vignesh C <[email protected]>
Date: Tue, 6 Jun 2023 15:55:56 +0530
Subject: [PATCH v5 2/2] Move common connection log content verification code
to a common function.
Move common connection log content verification code to a common
function.
---
src/test/perl/PostgreSQL/Test/Cluster.pm | 122 ++++++++++++-----------
1 file changed, 66 insertions(+), 56 deletions(-)
diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm
index 5063e77368..0d7a1bcfbc 100644
--- a/src/test/perl/PostgreSQL/Test/Cluster.pm
+++ b/src/test/perl/PostgreSQL/Test/Cluster.pm
@@ -2222,15 +2222,9 @@ If this regular expression is set, matches it with the output generated.
=item log_like => [ qr/required message/ ]
-If given, it must be an array reference containing a list of regular
-expressions that must match against the server log, using
-C<Test::More::like()>.
-
=item log_unlike => [ qr/prohibited message/ ]
-If given, it must be an array reference containing a list of regular
-expressions that must NOT match against the server log. They will be
-passed to C<Test::More::unlike()>.
+See C<log_check(...)>.
=back
@@ -2251,16 +2245,6 @@ sub connect_ok
$sql = "SELECT \$\$connected with $connstr\$\$";
}
- my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
-
my $log_location = -s $self->logfile;
# Never prompt for a password, any callers of this routine should
@@ -2281,20 +2265,7 @@ sub connect_ok
is($stderr, "", "$test_name: no stderr");
- if (@log_like or @log_unlike)
- {
- my $log_contents =
- PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
+ $self->log_check($test_name, $log_location, %params);
}
=pod
@@ -2314,7 +2285,7 @@ If this regular expression is set, matches it with the output generated.
=item log_unlike => [ qr/prohibited message/ ]
-See C<connect_ok(...)>, above.
+See C<log_check(...)>.
=back
@@ -2325,16 +2296,6 @@ sub connect_fails
local $Test::Builder::Level = $Test::Builder::Level + 1;
my ($self, $connstr, $test_name, %params) = @_;
- my (@log_like, @log_unlike);
- if (defined($params{log_like}))
- {
- @log_like = @{ $params{log_like} };
- }
- if (defined($params{log_unlike}))
- {
- @log_unlike = @{ $params{log_unlike} };
- }
-
my $log_location = -s $self->logfile;
# Never prompt for a password, any callers of this routine should
@@ -2352,20 +2313,7 @@ sub connect_fails
like($stderr, $params{expected_stderr}, "$test_name: matches");
}
- if (@log_like or @log_unlike)
- {
- my $log_contents =
- PostgreSQL::Test::Utils::slurp_file($self->logfile, $log_location);
-
- while (my $regex = shift @log_like)
- {
- like($log_contents, $regex, "$test_name: log matches");
- }
- while (my $regex = shift @log_unlike)
- {
- unlike($log_contents, $regex, "$test_name: log does not match");
- }
- }
+ $self->log_check($test_name, $log_location, %params);
}
=pod
@@ -2559,6 +2507,68 @@ sub issues_sql_like
=pod
+=item $node->log_check($offset, $test_name, %parameters)
+
+Check contents of server logs.
+
+=over
+
+=item $test_name
+
+Name of test for error messages.
+
+=item $offset
+
+Offset of the log file.
+
+=item log_like => [ qr/required message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must match against the server log, using
+C<Test::More::like()>.
+
+=item log_unlike => [ qr/prohibited message/ ]
+
+If given, it must be an array reference containing a list of regular
+expressions that must NOT match against the server log. They will be
+passed to C<Test::More::unlike()>.
+
+=back
+
+=cut
+
+sub log_check
+{
+ my ($self, $test_name, $offset, %params) = @_;
+
+ my (@log_like, @log_unlike);
+ if (defined($params{log_like}))
+ {
+ @log_like = @{ $params{log_like} };
+ }
+ if (defined($params{log_unlike}))
+ {
+ @log_unlike = @{ $params{log_unlike} };
+ }
+
+ if (@log_like or @log_unlike)
+ {
+ my $log_contents =
+ PostgreSQL::Test::Utils::slurp_file($self->logfile, $offset);
+
+ while (my $regex = shift @log_like)
+ {
+ like($log_contents, $regex, "$test_name: log matches");
+ }
+ while (my $regex = shift @log_unlike)
+ {
+ unlike($log_contents, $regex, "$test_name: log does not match");
+ }
+ }
+}
+
+=pod
+
=item log_contains(pattern, offset)
Find pattern in logfile of node after offset byte.
--
2.34.1
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Implement generalized sub routine find_in_log for tap test
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-03 22:21 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
2023-06-06 02:35 ` Re: Implement generalized sub routine find_in_log for tap test Amit Kapila <[email protected]>
2023-06-06 04:06 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
2023-06-06 13:13 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
@ 2023-06-09 03:01 ` Michael Paquier <[email protected]>
2023-06-11 03:12 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
0 siblings, 1 reply; 15+ messages in thread
From: Michael Paquier @ 2023-06-09 03:01 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Amit Kapila <[email protected]>; Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On Tue, Jun 06, 2023 at 06:43:44PM +0530, vignesh C wrote:
> Please find the attached patches that can be applied on back branches
> too. v5*master.patch can be applied on master, v5*PG15.patch can be
> applied on PG15, v5*PG14.patch can be applied on PG14, v5*PG13.patch
> can be applied on PG13, v5*PG12.patch can be applied on PG12, PG11 and
> PG10.
Thanks. The amount of minimal conflicts across all these branches is
always fun to play with. I have finally got around and applied all
that, after doing a proper split, applying one part down to 14 and the
second back to 11.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 15+ messages in thread
* Re: Implement generalized sub routine find_in_log for tap test
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-03 22:21 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
2023-06-06 02:35 ` Re: Implement generalized sub routine find_in_log for tap test Amit Kapila <[email protected]>
2023-06-06 04:06 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
2023-06-06 13:13 ` Re: Implement generalized sub routine find_in_log for tap test vignesh C <[email protected]>
2023-06-09 03:01 ` Re: Implement generalized sub routine find_in_log for tap test Michael Paquier <[email protected]>
@ 2023-06-11 03:12 ` vignesh C <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: vignesh C @ 2023-06-11 03:12 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Amit Kapila <[email protected]>; Andrew Dunstan <[email protected]>; Dagfinn Ilmari Mannsåker <[email protected]>; PostgreSQL Hackers <[email protected]>
On Fri, 9 Jun 2023 at 08:31, Michael Paquier <[email protected]> wrote:
>
> On Tue, Jun 06, 2023 at 06:43:44PM +0530, vignesh C wrote:
> > Please find the attached patches that can be applied on back branches
> > too. v5*master.patch can be applied on master, v5*PG15.patch can be
> > applied on PG15, v5*PG14.patch can be applied on PG14, v5*PG13.patch
> > can be applied on PG13, v5*PG12.patch can be applied on PG12, PG11 and
> > PG10.
>
> Thanks. The amount of minimal conflicts across all these branches is
> always fun to play with. I have finally got around and applied all
> that, after doing a proper split, applying one part down to 14 and the
> second back to 11.
Thanks for pushing this.
Regards,
Vignesh
^ permalink raw reply [nested|flat] 15+ messages in thread
* [PATCH v1 7/8] convert ParallelBlockTableScanDescData->phs_{start,num}block to atomics
@ 2026-07-09 20:21 Nathan Bossart <[email protected]>
0 siblings, 0 replies; 15+ messages in thread
From: Nathan Bossart @ 2026-07-09 20:21 UTC (permalink / raw)
---
src/backend/access/heap/heapam_handler.c | 2 +-
src/backend/access/table/tableam.c | 59 +++++++++++-------------
src/include/access/relscan.h | 8 ++--
3 files changed, 31 insertions(+), 38 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index bf87430cf01..0f24a132564 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -1965,7 +1965,7 @@ heapam_scan_get_blocks_done(HeapScanDesc hscan)
if (hscan->rs_base.rs_parallel != NULL)
{
bpscan = (ParallelBlockTableScanDesc) hscan->rs_base.rs_parallel;
- startblock = bpscan->phs_startblock;
+ startblock = pg_atomic_read_u32(&bpscan->phs_startblock);
}
else
startblock = hscan->rs_startblock;
diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index 68ff0966f1c..f2038ea9205 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -421,9 +421,8 @@ table_block_parallelscan_initialize(Relation rel, ParallelTableScanDesc pscan)
bpscan->base.phs_syncscan = synchronize_seqscans &&
!RelationUsesLocalBuffers(rel) &&
bpscan->phs_nblocks > NBuffers / 4;
- SpinLockInit(&bpscan->phs_mutex);
- bpscan->phs_startblock = InvalidBlockNumber;
- bpscan->phs_numblock = InvalidBlockNumber;
+ pg_atomic_init_u32(&bpscan->phs_startblock, InvalidBlockNumber);
+ pg_atomic_init_u32(&bpscan->phs_numblock, InvalidBlockNumber);
pg_atomic_init_u64(&bpscan->phs_nallocated, 0);
return sizeof(ParallelBlockTableScanDescData);
@@ -459,25 +458,22 @@ table_block_parallelscan_startblock_init(Relation rel,
StaticAssertDecl(MaxBlockNumber <= 0xFFFFFFFE,
"pg_nextpower2_32 may be too small for non-standard BlockNumber width");
- BlockNumber sync_startpage = InvalidBlockNumber;
BlockNumber scan_nblocks;
/* Reset the state we use for controlling allocation size. */
memset(pbscanwork, 0, sizeof(*pbscanwork));
-retry:
- /* Grab the spinlock. */
- SpinLockAcquire(&pbscan->phs_mutex);
-
/*
* When the caller specified a limit on the number of blocks to scan, set
* that in the ParallelBlockTableScanDesc, if it's not been done by
* another worker already.
*/
- if (numblocks != InvalidBlockNumber &&
- pbscan->phs_numblock == InvalidBlockNumber)
+ if (numblocks != InvalidBlockNumber)
{
- pbscan->phs_numblock = numblocks;
+ uint32 expected = InvalidBlockNumber;
+
+ pg_atomic_compare_exchange_u32(&pbscan->phs_numblock, &expected,
+ numblocks);
}
/*
@@ -485,36 +481,35 @@ retry:
* so now. If a startblock was specified, start there, otherwise if this
* is not a synchronized scan, we just start at block 0, but if it is a
* synchronized scan, we must get the starting position from the
- * synchronized scan machinery. We can't hold the spinlock while doing
- * that, though, so release the spinlock, get the information we need, and
- * retry. If nobody else has initialized the scan in the meantime, we'll
- * fill in the value we fetched on the second time through.
+ * synchronized scan machinery.
+ *
+ * If another worker initializes phs_startblock concurrently, just use
+ * their value.
*/
- if (pbscan->phs_startblock == InvalidBlockNumber)
+ if (pg_atomic_read_u32(&pbscan->phs_startblock) == InvalidBlockNumber)
{
+ BlockNumber newstartblock;
+ uint32 expected = InvalidBlockNumber;
+
if (startblock != InvalidBlockNumber)
- pbscan->phs_startblock = startblock;
+ newstartblock = startblock;
else if (!pbscan->base.phs_syncscan)
- pbscan->phs_startblock = 0;
- else if (sync_startpage != InvalidBlockNumber)
- pbscan->phs_startblock = sync_startpage;
+ newstartblock = 0;
else
- {
- SpinLockRelease(&pbscan->phs_mutex);
- sync_startpage = ss_get_location(rel, pbscan->phs_nblocks);
- goto retry;
- }
+ newstartblock = ss_get_location(rel, pbscan->phs_nblocks);
+
+ pg_atomic_compare_exchange_u32(&pbscan->phs_startblock, &expected,
+ newstartblock);
}
- SpinLockRelease(&pbscan->phs_mutex);
/*
* Figure out how many blocks we're going to scan; either all of them, or
* just phs_numblock's worth, if a limit has been imposed.
*/
- if (pbscan->phs_numblock == InvalidBlockNumber)
+ if (pg_atomic_read_u32(&pbscan->phs_numblock) == InvalidBlockNumber)
scan_nblocks = pbscan->phs_nblocks;
else
- scan_nblocks = pbscan->phs_numblock;
+ scan_nblocks = pg_atomic_read_u32(&pbscan->phs_numblock);
/*
* We determine the chunk size based on scan_nblocks. First we split
@@ -595,10 +590,10 @@ table_block_parallelscan_nextpage(Relation rel,
*/
/* First, figure out how many blocks we're planning on scanning */
- if (pbscan->phs_numblock == InvalidBlockNumber)
+ if (pg_atomic_read_u32(&pbscan->phs_numblock) == InvalidBlockNumber)
scan_nblocks = pbscan->phs_nblocks;
else
- scan_nblocks = pbscan->phs_numblock;
+ scan_nblocks = pg_atomic_read_u32(&pbscan->phs_numblock);
/*
* Now check if we have any remaining blocks in a previous chunk for this
@@ -644,7 +639,7 @@ table_block_parallelscan_nextpage(Relation rel,
if (nallocated >= scan_nblocks)
page = InvalidBlockNumber; /* all blocks have been allocated */
else
- page = (nallocated + pbscan->phs_startblock) % pbscan->phs_nblocks;
+ page = (nallocated + pg_atomic_read_u32(&pbscan->phs_startblock)) % pbscan->phs_nblocks;
/*
* Report scan location. Normally, we report the current page number.
@@ -658,7 +653,7 @@ table_block_parallelscan_nextpage(Relation rel,
if (page != InvalidBlockNumber)
ss_report_location(rel, page);
else if (nallocated == pbscan->phs_nblocks)
- ss_report_location(rel, pbscan->phs_startblock);
+ ss_report_location(rel, pg_atomic_read_u32(&pbscan->phs_startblock));
}
return page;
diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h
index 2ea06a67a63..2305d0159f3 100644
--- a/src/include/access/relscan.h
+++ b/src/include/access/relscan.h
@@ -19,7 +19,6 @@
#include "nodes/tidbitmap.h"
#include "port/atomics.h"
#include "storage/relfilelocator.h"
-#include "storage/spin.h"
#include "utils/relcache.h"
@@ -99,10 +98,9 @@ typedef struct ParallelBlockTableScanDescData
ParallelTableScanDescData base;
BlockNumber phs_nblocks; /* # blocks in relation at start of scan */
- slock_t phs_mutex; /* mutual exclusion for setting startblock */
- BlockNumber phs_startblock; /* starting block number */
- BlockNumber phs_numblock; /* # blocks to scan, or InvalidBlockNumber if
- * no limit */
+ pg_atomic_uint32 phs_startblock; /* starting block number */
+ pg_atomic_uint32 phs_numblock; /* # blocks to scan, or InvalidBlockNumber
+ * if no limit */
pg_atomic_uint64 phs_nallocated; /* number of blocks allocated to
* workers so far. */
} ParallelBlockTableScanDescData;
--
2.50.1 (Apple Git-155)
--Ot5x3ffkWEuxilWO
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
filename=v1-0008-convert-FastPathStrongRelationLocks-to-atomics.patch
^ permalink raw reply [nested|flat] 15+ messages in thread
end of thread, other threads:[~2026-07-09 20:21 UTC | newest]
Thread overview: 15+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-05-27 12:01 Re: Implement generalized sub routine find_in_log for tap test Andrew Dunstan <[email protected]>
2023-05-29 02:19 ` vignesh C <[email protected]>
2023-06-03 22:21 ` Michael Paquier <[email protected]>
2023-06-05 16:09 ` vignesh C <[email protected]>
2023-06-06 04:30 ` Bharath Rupireddy <[email protected]>
2023-06-06 10:41 ` Michael Paquier <[email protected]>
2023-06-06 12:23 ` Bharath Rupireddy <[email protected]>
2023-06-09 02:59 ` Michael Paquier <[email protected]>
2023-06-11 01:47 ` Bharath Rupireddy <[email protected]>
2023-06-06 02:35 ` Amit Kapila <[email protected]>
2023-06-06 04:06 ` Michael Paquier <[email protected]>
2023-06-06 13:13 ` vignesh C <[email protected]>
2023-06-09 03:01 ` Michael Paquier <[email protected]>
2023-06-11 03:12 ` vignesh C <[email protected]>
2026-07-09 20:21 [PATCH v1 7/8] convert ParallelBlockTableScanDescData->phs_{start,num}block to atomics Nathan Bossart <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox