On Tue, Nov 26, 2019 at 11:37 AM Amit Kapila <amit.kapil...@gmail.com> wrote: > > On Mon, Nov 25, 2019 at 11:22 PM vignesh C <vignes...@gmail.com> wrote: > > > > On Sun, Nov 24, 2019 at 5:06 PM Pavel Stehule <pavel.steh...@gmail.com> > > wrote: > > > > > > > > > > > > ne 24. 11. 2019 v 11:25 odesílatel vignesh C <vignes...@gmail.com> napsal: > > >> > > >> On Sat, Nov 23, 2019 at 4:42 PM Amit Kapila <amit.kapil...@gmail.com> > > >> wrote: > > >> > > > >> > On Fri, Nov 22, 2019 at 3:16 PM vignesh C <vignes...@gmail.com> wrote: > > >> > > > > >> > > Thanks for fixing the comments. The changes looks fine to me. I have > > >> > > fixed the first comment, attached patch has the changes for the same. > > >> > > > > >> > > > >> > Few comments: > > >> > -------------------------- > > >> > 1. There is a lot of duplicate code in this test. Basically, almost > > >> > the same code is used once to test Drop Database command and dropdb. > > >> > I think we can create a few sub-functions to avoid duplicate code, but > > >> > do we really need to test the same thing once via command and then by > > >> > dropdb utility? I don't think it is required, but even if we want to > > >> > do so, then I am not sure if this is the right test script. I suggest > > >> > removing the command related test. > > >> > > > >> > > >> Pavel: What is your opinion on this? > > > > > > > > > I have not any problem with this reduction. > > > > > > We will see in future years what is benefit of this test. > > > > > > > Fixed, removed dropdb utility test. > > > > Hmm, you have done the opposite of what I have asked. This test file > is in rc/bin/scripts/t/ where we generally keep the tests for > utilities. So, retaining the dropdb utility test in that file seems > more sensible. >
Fixed. Retained the test of dropdb utility and removed drop database sql command test. > +ok( TestLib::pump_until( > + $killme, $psql_timeout, \$killme_stdout, qr/[[:digit:]]+[\r\n]$/m), > + 'acquired pid'); > > How about changing 'acquired pid' to 'acquired pid for SIGTERM'? > Fixed. Changed as suggested. > > >> > > > >> > > >> I have verified by running perltidy. > > >> > > I think we don't need to run perltidy on the existing code. So, I am > not sure if it is a good idea to run it for file 013_crash_restart.pl > as it changes some existing code formatting. > I have retained the format same as old format, one additional change I added was to break the line if the line is lengthy in the modified code. Attached patch has the fixes for the above comments. Regards, Vignesh EnterpriseDB: http://www.enterprisedb.com
From ef60824cd15b3c4cab1fc7fb26f79e5243d8cb7b Mon Sep 17 00:00:00 2001 From: Pavel Stehule <pavel.steh...@gmail.com> Date: Wed, 27 Nov 2019 09:18:26 +0530 Subject: [PATCH 2/2] Add tests for the support of '-f' option in dropdb utility. Add tests for the support of '-f' option in dropdb utility. --- src/bin/scripts/t/060_dropdb_force.pl | 84 +++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 src/bin/scripts/t/060_dropdb_force.pl diff --git a/src/bin/scripts/t/060_dropdb_force.pl b/src/bin/scripts/t/060_dropdb_force.pl new file mode 100644 index 0000000..0a1f612 --- /dev/null +++ b/src/bin/scripts/t/060_dropdb_force.pl @@ -0,0 +1,84 @@ +# +# Tests drop database with force option. +# +use strict; +use warnings; +use PostgresNode; +use TestLib; +use Test::More tests => 6; + +# To avoid hanging while expecting some specific input from a psql +# instance being driven by us, add a timeout high enough that it +# should never trigger even on very slow machines, unless something +# is really wrong. +my $psql_timeout = IPC::Run::timer(60); + +my $node = get_new_node('master'); +$node->init; +$node->start; + +# Create database that will be dropped. +$node->safe_psql('postgres', 'CREATE DATABASE foobar1'); + +# Run psql, keeping session alive, so we have an alive backend to kill. +my ($killme_stdin, $killme_stdout, $killme_stderr) = ('', '', ''); +my $killme = IPC::Run::start( + [ + 'psql', '-X', '-qAt', '-v', 'ON_ERROR_STOP=1', '-f', '-', '-d', + $node->connstr('foobar1') + ], + '<', + \$killme_stdin, + '>', + \$killme_stdout, + '2>', + \$killme_stderr, + $psql_timeout); + +# Ensure killme process is active. +$killme_stdin .= q[ +SELECT pg_backend_pid(); +]; +ok( TestLib::pump_until( + $killme, $psql_timeout, \$killme_stdout, qr/[[:digit:]]+[\r\n]$/m), + 'acquired pid for SIGTERM'); +my $pid = $killme_stdout; +chomp($pid); +$killme_stdout = ''; +$killme_stderr = ''; + +# Check the connections on foobar1 database. +is( $node->safe_psql( + 'postgres', + qq[SELECT pid FROM pg_stat_activity WHERE datname='foobar1' AND pid = $pid;] + ), + $pid, + 'database foobar1 is used'); + +# Now drop database with dropdb --force command. +$node->issues_sql_like( + [ 'dropdb', '--force', 'foobar1' ], + qr/statement: DROP DATABASE foobar1 WITH \(FORCE\);/, + 'SQL DROP DATABASE (FORCE) run'); + +# Check that psql sees the killed backend as having been terminated. +$killme_stdin .= q[ +SELECT 1; +]; +ok( TestLib::pump_until( + $killme, $psql_timeout, \$killme_stderr, + qr/FATAL: terminating connection due to administrator command/m), + "psql query died successfully after SIGTERM"); +$killme_stderr = ''; +$killme_stdout = ''; +$killme->finish; + +# Check database foobar1 does not exist. +is( $node->safe_psql( + 'postgres', + qq[SELECT EXISTS(SELECT * FROM pg_database WHERE datname='foobar1');] + ), + 'f', + 'database foobar1 was removed'); + +$node->stop(); -- 1.8.3.1
From 6bfb40e665de267bac73808c9610a6722aef09ac Mon Sep 17 00:00:00 2001 From: vignesh <vignesh@localhost.localdomain> Date: Wed, 27 Nov 2019 09:15:05 +0530 Subject: [PATCH 1/2] Made pump_until usable as a common subroutine. Patch includes movement of pump_until subroutine from 013_crash_restart to TestLib so that it can be used as a common sub from 013_crash_restart and 060_dropdb_force. --- src/test/perl/TestLib.pm | 37 +++++++++++++++++++ src/test/recovery/t/013_crash_restart.pl | 63 ++++++++++++-------------------- 2 files changed, 60 insertions(+), 40 deletions(-) diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm index 18b4ce3..e235550 100644 --- a/src/test/perl/TestLib.pm +++ b/src/test/perl/TestLib.pm @@ -877,6 +877,43 @@ sub command_checks_all =pod +=item pump_until(proc, timeout, stream, untl) + +# Pump until string is matched, or timeout occurs + +=cut + +sub pump_until +{ + my ($proc, $timeout, $stream, $untl) = @_; + $proc->pump_nb(); + while (1) + { + last if $$stream =~ /$untl/; + if ($timeout->is_expired) + { + diag("aborting wait: program timed out"); + diag("stream contents: >>", $$stream, "<<"); + diag("pattern searched for: ", $untl); + + return 0; + } + if (not $proc->pumpable()) + { + diag("aborting wait: program died"); + diag("stream contents: >>", $$stream, "<<"); + diag("pattern searched for: ", $untl); + + return 0; + } + $proc->pump(); + } + return 1; + +} + +=pod + =back =cut diff --git a/src/test/recovery/t/013_crash_restart.pl b/src/test/recovery/t/013_crash_restart.pl index 2c47797..5957619 100644 --- a/src/test/recovery/t/013_crash_restart.pl +++ b/src/test/recovery/t/013_crash_restart.pl @@ -72,7 +72,9 @@ CREATE TABLE alive(status text); INSERT INTO alive VALUES($$committed-before-sigquit$$); SELECT pg_backend_pid(); ]; -ok(pump_until($killme, \$killme_stdout, qr/[[:digit:]]+[\r\n]$/m), +ok(TestLib::pump_until( + $killme, $psql_timeout, \$killme_stdout, + qr/[[:digit:]]+[\r\n]$/m), 'acquired pid for SIGQUIT'); my $pid = $killme_stdout; chomp($pid); @@ -84,7 +86,9 @@ $killme_stdin .= q[ BEGIN; INSERT INTO alive VALUES($$in-progress-before-sigquit$$) RETURNING status; ]; -ok(pump_until($killme, \$killme_stdout, qr/in-progress-before-sigquit/m), +ok(TestLib::pump_until( + $killme, $psql_timeout, \$killme_stdout, + qr/in-progress-before-sigquit/m), 'inserted in-progress-before-sigquit'); $killme_stdout = ''; $killme_stderr = ''; @@ -97,7 +101,8 @@ $monitor_stdin .= q[ SELECT $$psql-connected$$; SELECT pg_sleep(3600); ]; -ok(pump_until($monitor, \$monitor_stdout, qr/psql-connected/m), +ok(TestLib::pump_until( + $monitor, $psql_timeout, \$monitor_stdout, qr/psql-connected/m), 'monitor connected'); $monitor_stdout = ''; $monitor_stderr = ''; @@ -112,8 +117,9 @@ is($ret, 0, "killed process with SIGQUIT"); $killme_stdin .= q[ SELECT 1; ]; -ok( pump_until( +ok( TestLib::pump_until( $killme, + $psql_timeout, \$killme_stderr, qr/WARNING: terminating connection because of crash of another server process|server closed the connection unexpectedly|connection to server was lost/m ), @@ -125,8 +131,9 @@ $killme->finish; # Wait till server restarts - we should get the WARNING here, but # sometimes the server is unable to send that, if interrupted while # sending. -ok( pump_until( +ok( TestLib::pump_until( $monitor, + $psql_timeout, \$monitor_stderr, qr/WARNING: terminating connection because of crash of another server process|server closed the connection unexpectedly|connection to server was lost/m ), @@ -153,7 +160,8 @@ $monitor->run(); $killme_stdin .= q[ SELECT pg_backend_pid(); ]; -ok(pump_until($killme, \$killme_stdout, qr/[[:digit:]]+[\r\n]$/m), +ok(TestLib::pump_until( + $killme, $psql_timeout, \$killme_stdout, qr/[[:digit:]]+[\r\n]$/m), "acquired pid for SIGKILL"); $pid = $killme_stdout; chomp($pid); @@ -166,7 +174,9 @@ INSERT INTO alive VALUES($$committed-before-sigkill$$) RETURNING status; BEGIN; INSERT INTO alive VALUES($$in-progress-before-sigkill$$) RETURNING status; ]; -ok(pump_until($killme, \$killme_stdout, qr/in-progress-before-sigkill/m), +ok(TestLib::pump_until( + $killme, $psql_timeout, \$killme_stdout, + qr/in-progress-before-sigkill/m), 'inserted in-progress-before-sigkill'); $killme_stdout = ''; $killme_stderr = ''; @@ -178,7 +188,8 @@ $monitor_stdin .= q[ SELECT $$psql-connected$$; SELECT pg_sleep(3600); ]; -ok(pump_until($monitor, \$monitor_stdout, qr/psql-connected/m), +ok(TestLib::pump_until( + $monitor, $psql_timeout, \$monitor_stdout, qr/psql-connected/m), 'monitor connected'); $monitor_stdout = ''; $monitor_stderr = ''; @@ -194,8 +205,9 @@ is($ret, 0, "killed process with KILL"); $killme_stdin .= q[ SELECT 1; ]; -ok( pump_until( +ok( TestLib::pump_until( $killme, + $psql_timeout, \$killme_stderr, qr/server closed the connection unexpectedly|connection to server was lost/m ), @@ -205,8 +217,9 @@ $killme->finish; # Wait till server restarts - we should get the WARNING here, but # sometimes the server is unable to send that, if interrupted while # sending. -ok( pump_until( +ok( TestLib::pump_until( $monitor, + $psql_timeout, \$monitor_stderr, qr/WARNING: terminating connection because of crash of another server process|server closed the connection unexpectedly|connection to server was lost/m ), @@ -244,33 +257,3 @@ is( $node->safe_psql( 'can still write after orderly restart'); $node->stop(); - -# Pump until string is matched, or timeout occurs -sub pump_until -{ - my ($proc, $stream, $untl) = @_; - $proc->pump_nb(); - while (1) - { - last if $$stream =~ /$untl/; - if ($psql_timeout->is_expired) - { - diag("aborting wait: program timed out"); - diag("stream contents: >>", $$stream, "<<"); - diag("pattern searched for: ", $untl); - - return 0; - } - if (not $proc->pumpable()) - { - diag("aborting wait: program died"); - diag("stream contents: >>", $$stream, "<<"); - diag("pattern searched for: ", $untl); - - return 0; - } - $proc->pump(); - } - return 1; - -} -- 1.8.3.1