pg_waldump: add test for coverage
Started by Dong Wook Leeover 3 years ago7 messages
Hi Hackers,
I wrote a test for coverage.
Unfortunately, it seems to take quite a while to run the test.
I want to improve these execution times, but I don't know exactly what to do.
Therefore, I want to hear feedback from many people.
---
Regards,
Dong Wook Lee
Attachments:
v1_add_test_pg_waldump.patchapplication/octet-stream; name=v1_add_test_pg_waldump.patchDownload
From f2d9a4c447f4b7a1e51c672aa8269cac6de6a500 Mon Sep 17 00:00:00 2001
From: Lee Dong Wook <sh95119@gmail.com>
Date: Tue, 23 Aug 2022 10:18:58 +0900
Subject: [PATCH] pg_waldump: add test for coverage
---
src/bin/pg_waldump/t/001_basic.pl | 39 ++++++
src/bin/pg_waldump/t/002_wal_type.pl | 180 +++++++++++++++++++++++++++
2 files changed, 219 insertions(+)
create mode 100644 src/bin/pg_waldump/t/002_wal_type.pl
diff --git a/src/bin/pg_waldump/t/001_basic.pl b/src/bin/pg_waldump/t/001_basic.pl
index 883adff8fa13..72f124bfb1f0 100644
--- a/src/bin/pg_waldump/t/001_basic.pl
+++ b/src/bin/pg_waldump/t/001_basic.pl
@@ -3,6 +3,7 @@
use strict;
use warnings;
+use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
@@ -10,4 +11,42 @@
program_version_ok('pg_waldump');
program_options_handling_ok('pg_waldump');
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+
+my $wal_dump_path = $node->data_dir . "/pg_wal/000000010000000000000001";
+my ($stdout, $stderr);
+
+# test pg_waldump
+IPC::Run::run [ 'pg_waldump', "$wal_dump_path" ], '>', \$stdout, '2>', \$stderr;
+isnt($stdout, '', "");
+
+# test pg_waldump with -p (path), -s (start), -e (end) options
+IPC::Run::run [ 'pg_waldump', "$wal_dump_path", '-s', '0/0132C160', '-e', '0/018C41A0' ], '>', \$stdout, '2>', \$stderr;
+isnt($stdout, '', "");
+
+# test pg_waldump with -p (path), -s (start), -e (end) options -z (stats)
+IPC::Run::run [ 'pg_waldump', "$wal_dump_path", '-s', '0/0132C160', '-e', '0/018C41A0', '-z' ], '>', \$stdout, '2>', \$stderr;
+isnt($stdout, '', "");
+
+# test pg_waldump with -F (main)
+IPC::Run::run [ 'pg_waldump', "$wal_dump_path", '-F', 'main' ], '>', \$stdout, '2>', \$stderr;
+isnt($stdout, '', "");
+
+# test pg_waldump with -F (fsm)
+IPC::Run::run [ 'pg_waldump', "$wal_dump_path", '-F', 'fsm' ], '>', \$stdout, '2>', \$stderr;
+is($stdout, '', "");
+
+# test pg_waldump with -F (vm)
+IPC::Run::run [ 'pg_waldump', "$wal_dump_path", '-F', 'vm' ], '>', \$stdout, '2>', \$stderr;
+isnt($stdout, '', "");
+
+# test pg_waldump with -F (init)
+IPC::Run::run [ 'pg_waldump', "$wal_dump_path", '-F', 'init' ], '>', \$stdout, '2>', \$stderr;
+is($stdout, '', "");
+
+# test pg_waldump with -q
+IPC::Run::run [ 'pg_waldump', "$wal_dump_path", '-F', 'init' ], '>', \$stdout, '2>', \$stderr;
+is($stdout, '', "");
done_testing();
+
diff --git a/src/bin/pg_waldump/t/002_wal_type.pl b/src/bin/pg_waldump/t/002_wal_type.pl
new file mode 100644
index 000000000000..d69ddf3a39d0
--- /dev/null
+++ b/src/bin/pg_waldump/t/002_wal_type.pl
@@ -0,0 +1,180 @@
+
+# Copyright (c) 2021-2022, PostgreSQL Global Development Group
+
+use strict;
+use warnings;
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+use Test::More;
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->append_conf('postgresql.conf',
+ "track_commit_timestamp = on\n"
+ . "archive_mode=on\n"
+ . "archive_command='/bin/false'\n"
+ . "wal_level=logical");
+
+# test pg_waldump with xlog files.
+my $source_ts_path = PostgreSQL::Test::Utils::tempdir_short();
+my $pg_wal_dir = $node->data_dir . "/pg_wal/";
+my ($stdout, $stderr);
+
+# test pg_waldump with hash index
+$node->start;
+$node->safe_psql('postgres',
+ 'BEGIN;'
+ . 'CREATE TABLE hash_idx_tbl (
+ id int
+ );'
+ . 'CREATE INDEX ON hash_idx_tbl USING hash (id);'
+ . 'COMMIT;'
+ . 'BEGIN;'
+ . 'INSERT INTO hash_idx_tbl
+ SELECT generate_series(1, 1000);'
+ . 'COMMIT;'
+ . 'select pg_switch_wal();'
+);
+$node->stop;
+
+# track_commit_timestamp off
+$node->append_conf('postgresql.conf',
+ "track_commit_timestamp = off");
+
+# test pg_waldump with gin index
+$node->start;
+$node->safe_psql('postgres',
+ 'BEGIN;'
+ . 'CREATE TABLE gin_idx_tbl (
+ id bigserial PRIMARY KEY,
+ data jsonb
+ );'
+ . 'CREATE INDEX ON gin_idx_tbl USING gin (data);'
+ . 'COMMIT;'
+ . 'BEGIN;'
+ . "INSERT INTO gin_idx_tbl
+ WITH random_json AS (
+ SELECT json_object_agg(key, trunc(random() * 10)) as json_data
+ FROM unnest(array['a', 'b', 'c']) as u(key))
+ SELECT generate_series(1,500), json_data FROM random_json;"
+ . 'COMMIT;'
+ . 'select pg_switch_wal();'
+);
+$node->stop;
+
+# track_commit_timestamp on
+$node->append_conf('postgresql.conf',
+ "track_commit_timestamp = on");
+
+## test pg_waldump with gist index
+$node->start;
+$node->safe_psql('postgres',
+ 'BEGIN;'
+ . 'CREATE TABLE gist_idx_tbl (
+ p point
+ );'
+ . 'CREATE INDEX ON gist_idx_tbl USING gist (p);'
+ . 'COMMIT;'
+ . 'BEGIN;'
+ . 'INSERT INTO gist_idx_tbl(p) values (point \'(1,1)\');'
+ . 'INSERT INTO gist_idx_tbl(p) values (point \'(3,2)\');'
+ . 'INSERT INTO gist_idx_tbl(p) values (point \'(6,3)\');'
+ . 'INSERT INTO gist_idx_tbl(p) values (point \'(5,5)\');'
+ . 'INSERT INTO gist_idx_tbl(p) values (point \'(7,8)\');'
+ . 'INSERT INTO gist_idx_tbl(p) values (point \'(8,6)\');'
+ . 'INSERT INTO gist_idx_tbl(p) values (point \'(9,19)\');'
+ . 'COMMIT;'
+ . 'select pg_switch_wal();'
+);
+$node->stop;
+
+
+## test pg_waldump with spgist index
+$node->start;
+$node->safe_psql('postgres',
+ 'BEGIN;'
+ . 'CREATE TABLE spgist_idx_tbl (
+ p point
+ );'
+ . 'CREATE INDEX ON spgist_idx_tbl USING spgist (p);'
+ . 'COMMIT;'
+ . 'BEGIN;'
+ . 'INSERT INTO spgist_idx_tbl(p) values (point \'(1,1)\');'
+ . 'INSERT INTO spgist_idx_tbl(p) values (point \'(3,2)\');'
+ . 'INSERT INTO spgist_idx_tbl(p) values (point \'(6,3)\');'
+ . 'INSERT INTO spgist_idx_tbl(p) values (point \'(5,5)\');'
+ . 'INSERT INTO spgist_idx_tbl(p) values (point \'(7,8)\');'
+ . 'INSERT INTO spgist_idx_tbl(p) values (point \'(8,6)\');'
+ . 'INSERT INTO spgist_idx_tbl(p) values (point \'(9,19)\');'
+ . 'COMMIT;'
+);
+$node->stop;
+
+# test pg_waldump with brin index
+$node->start;
+$node->safe_psql('postgres',
+ 'BEGIN;'
+ . 'CREATE TABLE brin_idx_tbl (
+ col1 int,
+ col2 text,
+ col3 text
+ );'
+ . 'COMMIT;'
+ . 'BEGIN;'
+ . 'CREATE INDEX brin_idx ON brin_idx_tbl USING brin (col1, col2, col3) WITH (autosummarize=on);'
+ . 'COMMIT;'
+ . 'BEGIN;'
+ . "INSERT INTO brin_idx_tbl
+ SELECT generate_series(1, 10000), 'dummy', 'dummy';"
+ . 'COMMIT;'
+ . 'BEGIN;'
+ . "UPDATE brin_idx_tbl
+ SET col2 = 'updated'
+ WHERE col1 BETWEEN 1 AND 5000;"
+ . 'COMMIT;'
+ . "SELECT brin_summarize_range('brin_idx', 0);"
+ . "SELECT brin_desummarize_range('brin_idx', 0);"
+);
+$node->stop;
+
+# test pg_waldump with tablespace
+$node->start;
+$node->safe_psql('postgres',
+ "CREATE TABLESPACE tbl_space LOCATION '$source_ts_path';"
+ . 'CREATE TABLE table_space_tbl (
+ col1 int
+ ) TABLESPACE tbl_space;'
+);
+$node->stop;
+
+# test pg_waldump with sequence
+$node->start;
+$node->safe_psql('postgres',
+ "CREATE SEQUENCE sequence;"
+);
+$node->stop;
+
+# test local message
+$node->start;
+$node->safe_psql('postgres',
+ 'BEGIN;'
+ . "SELECT pg_logical_emit_message(true, 'text', 'text');"
+ . 'COMMIT;'
+);
+$node->stop;
+
+# test relmap
+$node->start;
+$node->safe_psql('postgres',
+ "SELECT relfilenode FROM pg_class WHERE relname = 'pg_authid';"
+ . 'VACUUM FULL pg_authid;'
+);
+$node->stop;
+
+chdir $pg_wal_dir;
+foreach my $file (glob '000*') {
+ IPC::Run::run [ 'pg_waldump', "$pg_wal_dir/$file" ], '>', \$stdout, '2>', \$stderr;
+ isnt($stdout, '');
+}
+
+done_testing();
Re: pg_waldump: add test for coverage
On 23.08.22 03:50, Dong Wook Lee wrote:
Hi Hackers,
I wrote a test for coverage.
Unfortunately, it seems to take quite a while to run the test.
I want to improve these execution times, but I don't know exactly what to do.
Therefore, I want to hear feedback from many people.
I don't find these tests to be particularly slow. How long do they take
for you to run?
A couple of tips:
- You should give each test a name. That's why each test function has a
(usually) last argument that takes a string.
- You could use command_like() to run a command and check that it exits
successfully and check its standard out. For example, instead of
# test pg_waldump with -F (main)
IPC::Run::run [ 'pg_waldump', "$wal_dump_path", '-F', 'main' ], '>',
\$stdout, '2>', \$stderr;
isnt($stdout, '', "");
it is better to write
command_like([ 'pg_waldump', "$wal_dump_path", '-F', 'main' ],
qr/TODO/, 'test -F (main)');
- It would be useful to test the actual output (that is, fill in the
TODO above). I don't know what the best way to do that is -- that is
part of designing these tests.
Also,
- Your patch introduces a spurious blank line at the end of the test file.
- For portability, options must be before non-option arguments. So
instead of
[ 'pg_waldump', "$wal_dump_path", '-F', 'main' ]
it should be
[ 'pg_waldump', '-F', 'main', "$wal_dump_path" ]
I think having some more test coverage for pg_waldump would be good, so
I encourage you to continue working on this.
Re: pg_waldump: add test for coverage
Hi,
On 2022-08-23 10:50:08 +0900, Dong Wook Lee wrote:
I wrote a test for coverage.
Unfortunately the test doesn't seem to pass on windows, and hasn't ever done so:
https://cirrus-ci.com/github/postgresql-cfbot/postgresql/commitfest/39/3834
Due to the merge of the meson patchset, you should also add 001_basic.pl to
the list of tests in meson.build
Greetings,
Andres Freund
Re: pg_waldump: add test for coverage
On 06.09.22 07:57, Peter Eisentraut wrote:
I wrote a test for coverage.
Unfortunately, it seems to take quite a while to run the test.
I want to improve these execution times, but I don't know exactly what
to do.
Therefore, I want to hear feedback from many people.
I think having some more test coverage for pg_waldump would be good, so
I encourage you to continue working on this.
I made an updated patch that incorporates many of your ideas and code,
just made it a bit more compact, and added more tests for various
command-line options. This moves the test coverage of pg_waldump from
"bloodbath" to "mixed fruit salad", which I think is pretty good
progress. And now there is room for additional patches if someone wants
to figure out, e.g., how to get more complete coverage in gindesc.c or
whatever.
Attachments:
v2-0001-Add-more-pg_waldump-tests.patchtext/plain; charset=UTF-8; name=v2-0001-Add-more-pg_waldump-tests.patchDownload
From 67d63a87cf9ea8de69801127607101b3a515fb42 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 14 Jun 2023 08:46:47 +0200
Subject: [PATCH v2] Add more pg_waldump tests
This adds tests for most command-line options and tests for most
rmgrdesc routines.
Discussion: https://www.postgresql.org/message-id/flat/CAAcByaKM7zyJSDkPWv6_%2BapupY4fXXM3q3SRXas9MMNVPUGcsQ%40mail.gmail.com
---
src/bin/pg_waldump/t/001_basic.pl | 191 ++++++++++++++++++++++++++++++
1 file changed, 191 insertions(+)
diff --git a/src/bin/pg_waldump/t/001_basic.pl b/src/bin/pg_waldump/t/001_basic.pl
index 492a8cadd8..d4056e1b95 100644
--- a/src/bin/pg_waldump/t/001_basic.pl
+++ b/src/bin/pg_waldump/t/001_basic.pl
@@ -3,6 +3,7 @@
use strict;
use warnings;
+use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
@@ -10,4 +11,194 @@
program_version_ok('pg_waldump');
program_options_handling_ok('pg_waldump');
+# wrong number of arguments
+command_fails_like([ 'pg_waldump', ], qr/error: no arguments/, 'no arguments');
+command_fails_like([ 'pg_waldump', 'foo', 'bar', 'baz' ], qr/error: too many command-line arguments/, 'too many arguments');
+
+# invalid option arguments
+command_fails_like([ 'pg_waldump', '--block', 'bad' ], qr/error: invalid block number/, 'invalid block number');
+command_fails_like([ 'pg_waldump', '--fork', 'bad' ], qr/error: invalid fork name/, 'invalid fork name');
+command_fails_like([ 'pg_waldump', '--limit', 'bad' ], qr/error: invalid value/, 'invalid limit');
+command_fails_like([ 'pg_waldump', '--relation', 'bad' ], qr/error: invalid relation/, 'invalid relation specification');
+command_fails_like([ 'pg_waldump', '--rmgr', 'bad' ], qr/error: resource manager .* does not exist/, 'invalid rmgr name');
+command_fails_like([ 'pg_waldump', '--start', 'bad' ], qr/error: invalid WAL location/, 'invalid start LSN');
+command_fails_like([ 'pg_waldump', '--end', 'bad' ], qr/error: invalid WAL location/, 'invalid end LSN');
+
+# rmgr list: If you add one to the list, consider also adding a test
+# case exercising the new rmgr below.
+command_like([ 'pg_waldump', '--rmgr=list'], qr/^XLOG
+Transaction
+Storage
+CLOG
+Database
+Tablespace
+MultiXact
+RelMap
+Standby
+Heap2
+Heap
+Btree
+Hash
+Gin
+Gist
+Sequence
+SPGist
+BRIN
+CommitTs
+ReplicationOrigin
+Generic
+LogicalMessage$/,
+ 'rmgr list');
+
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->append_conf('postgresql.conf', q{
+autovacuum = off
+checkpoint_timeout = 1h
+
+# for standbydesc
+archive_mode=on
+archive_command=''
+
+# for XLOG_HEAP_TRUNCATE
+wal_level=logical
+});
+$node->start;
+
+my ($start_lsn, $start_walfile) = split /\|/, $node->safe_psql('postgres', q{SELECT pg_current_wal_insert_lsn(), pg_walfile_name(pg_current_wal_insert_lsn())});
+
+$node->safe_psql('postgres', q{
+-- heap, btree, hash, sequence
+CREATE TABLE t1 (a int GENERATED ALWAYS AS IDENTITY, b text);
+CREATE INDEX i1a ON t1 USING btree (a);
+CREATE INDEX i1b ON t1 USING hash (b);
+INSERT INTO t1 VALUES (default, 'one'), (default, 'two');
+DELETE FROM t1 WHERE b = 'one';
+TRUNCATE t1;
+
+-- abort
+START TRANSACTION;
+INSERT INTO t1 VALUES (default, 'three');
+ROLLBACK;
+
+-- unlogged/init fork
+CREATE UNLOGGED TABLE t2 (x int);
+CREATE INDEX i2 ON t2 USING btree (x);
+INSERT INTO t2 SELECT generate_series(1, 10);
+
+-- gin
+CREATE TABLE gin_idx_tbl (id bigserial PRIMARY KEY, data jsonb);
+CREATE INDEX gin_idx ON gin_idx_tbl USING gin (data);
+INSERT INTO gin_idx_tbl
+ WITH random_json AS (
+ SELECT json_object_agg(key, trunc(random() * 10)) as json_data
+ FROM unnest(array['a', 'b', 'c']) as u(key))
+ SELECT generate_series(1,500), json_data FROM random_json;
+
+-- gist, spgist
+CREATE TABLE gist_idx_tbl (p point);
+CREATE INDEX gist_idx ON gist_idx_tbl USING gist (p);
+CREATE INDEX spgist_idx ON gist_idx_tbl USING spgist (p);
+INSERT INTO gist_idx_tbl (p) VALUES (point '(1, 1)'), (point '(3, 2)'), (point '(6, 3)');
+
+-- brin
+CREATE TABLE brin_idx_tbl (col1 int, col2 text, col3 text );
+CREATE INDEX brin_idx ON brin_idx_tbl USING brin (col1, col2, col3) WITH (autosummarize=on);
+INSERT INTO brin_idx_tbl SELECT generate_series(1, 10000), 'dummy', 'dummy';
+UPDATE brin_idx_tbl SET col2 = 'updated' WHERE col1 BETWEEN 1 AND 5000;
+SELECT brin_summarize_range('brin_idx', 0);
+SELECT brin_desummarize_range('brin_idx', 0);
+
+VACUUM;
+
+-- logical message
+SELECT pg_logical_emit_message(true, 'foo', 'bar');
+
+-- relmap
+VACUUM FULL pg_authid;
+
+-- database
+CREATE DATABASE d1;
+DROP DATABASE d1;
+});
+
+my $tblspc_path = PostgreSQL::Test::Utils::tempdir_short();
+
+$node->safe_psql('postgres', qq{
+CREATE TABLESPACE ts1 LOCATION '$tblspc_path';
+DROP TABLESPACE ts1;
+});
+
+my ($end_lsn, $end_walfile) = split /\|/, $node->safe_psql('postgres', q{SELECT pg_current_wal_insert_lsn(), pg_walfile_name(pg_current_wal_insert_lsn())});
+
+my $default_ts_oid = $node->safe_psql('postgres', q{SELECT oid FROM pg_tablespace WHERE spcname = 'pg_default'});
+my $postgres_db_oid = $node->safe_psql('postgres', q{SELECT oid FROM pg_database WHERE datname = 'postgres'});
+my $rel_t1_oid = $node->safe_psql('postgres', q{SELECT oid FROM pg_class WHERE relname = 't1'});
+my $rel_i1a_oid = $node->safe_psql('postgres', q{SELECT oid FROM pg_class WHERE relname = 'i1a'});
+
+$node->stop;
+
+
+# various ways of specifying WAL range
+command_fails_like([ 'pg_waldump', 'foo', 'bar' ], qr/error: could not locate WAL file "foo"/, 'start file not found');
+command_ok([ 'pg_waldump', $node->data_dir . '/pg_wal/' . $start_walfile ], 'runs with start segment specified');
+command_fails_like([ 'pg_waldump', $node->data_dir . '/pg_wal/' . $start_walfile, 'bar' ], qr/error: could not open file "bar"/, 'end file not found');
+command_ok([ 'pg_waldump', $node->data_dir . '/pg_wal/' . $start_walfile, $node->data_dir . '/pg_wal/' . $end_walfile ], 'runs with start and end segment specified');
+command_fails_like([ 'pg_waldump', '-p', $node->data_dir ], qr/error: no start WAL location given/, 'path option requires start location');
+command_ok([ 'pg_waldump', '-p', $node->data_dir, '--start', $start_lsn, '--end', $end_lsn ], 'runs with path option and start and end locations');
+command_fails_like([ 'pg_waldump', '-p', $node->data_dir, '--start', $start_lsn ], qr/error: error in WAL record at/, 'falling off the end of the WAL results in an error');
+
+
+# Helper function to test various options. Pass options as arguments.
+# Output lines are returned as array.
+sub test_pg_waldump
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+ my @opts = @_;
+
+ my (@cmd, $stdout, $stderr, $result, @lines);
+
+ @cmd = ('pg_waldump', '-p', $node->data_dir, '--start', $start_lsn, '--end', $end_lsn);
+ push @cmd, @opts;
+ $result = IPC::Run::run \@cmd, '>', \$stdout, '2>', \$stderr;
+ ok($result, "pg_waldump @opts: runs ok");
+ is($stderr, '', "pg_waldump @opts: no stderr");
+ @lines = split /\n/, $stdout;
+ ok(@lines > 0, "pg_waldump @opts: some lines are output");
+ return @lines;
+}
+
+my @lines;
+
+@lines = test_pg_waldump;
+is(grep(!/^rmgr: \w/, @lines), 0, 'all output lines are rmgr lines');
+
+@lines = test_pg_waldump('--limit', 6);
+is(@lines, 6, 'limit option observed');
+
+@lines = test_pg_waldump('--fullpage');
+is(grep(!/^rmgr:.*\bFPW\b/, @lines), 0, 'all output lines are FPW');
+
+@lines = test_pg_waldump('--stats');
+like($lines[0], qr/WAL statistics/, "statistics on stdout");
+is(grep(/^rmgr:/, @lines), 0, 'no rmgr lines output');
+
+@lines = test_pg_waldump('--stats=record');
+like($lines[0], qr/WAL statistics/, "statistics on stdout");
+is(grep(/^rmgr:/, @lines), 0, 'no rmgr lines output');
+
+@lines = test_pg_waldump('--rmgr', 'Btree');
+is(grep(!/^rmgr: Btree/, @lines), 0, 'only Btree lines');
+
+@lines = test_pg_waldump('--fork', 'init');
+is(grep(!/fork init/, @lines), 0, 'only init fork lines');
+
+@lines = test_pg_waldump('--relation', "$default_ts_oid/$postgres_db_oid/$rel_t1_oid");
+is(grep(!/rel $default_ts_oid\/$postgres_db_oid\/$rel_t1_oid/, @lines), 0, 'only lines for selected relation');
+
+@lines = test_pg_waldump('--relation', "$default_ts_oid/$postgres_db_oid/$rel_i1a_oid", '--block', 1);
+is(grep(!/\bblk 1\b/, @lines), 0, 'only lines for selected block');
+
+
done_testing();
base-commit: 0f8cfaf8921fed35f0b92d918ce95eec7b46ff05
--
2.41.0
Re: pg_waldump: add test for coverage
On 14.06.23 09:16, Peter Eisentraut wrote:
On 06.09.22 07:57, Peter Eisentraut wrote:
I wrote a test for coverage.
Unfortunately, it seems to take quite a while to run the test.
I want to improve these execution times, but I don't know exactly
what to do.
Therefore, I want to hear feedback from many people.I think having some more test coverage for pg_waldump would be good,
so I encourage you to continue working on this.I made an updated patch that incorporates many of your ideas and code,
just made it a bit more compact, and added more tests for various
command-line options. This moves the test coverage of pg_waldump from
"bloodbath" to "mixed fruit salad", which I think is pretty good
progress. And now there is room for additional patches if someone wants
to figure out, e.g., how to get more complete coverage in gindesc.c or
whatever.
Here is an updated patch set. I added a test case for the "first record
is after" message. Also, I think this message should really go to
stderr, since it's more of a notice or warning, so I changed it to use
pg_log_info.
Attachments:
v3-0002-pg_waldump-Add-test-case-for-notice-message.patchtext/plain; charset=UTF-8; name=v3-0002-pg_waldump-Add-test-case-for-notice-message.patchDownload
From 86a7f0ca489b61907973a266f0b5855869b30692 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 28 Jun 2023 07:28:14 +0200
Subject: [PATCH v3 2/2] pg_waldump: Add test case for notice message
Add a test case for the "first record is after" message. Also, this
message should really go to stderr, so change to use pg_log_info.
---
src/bin/pg_waldump/pg_waldump.c | 12 ++++++------
src/bin/pg_waldump/t/001_basic.pl | 18 ++++++++++++++++++
2 files changed, 24 insertions(+), 6 deletions(-)
diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c
index c6d3ae6344..6e53b94219 100644
--- a/src/bin/pg_waldump/pg_waldump.c
+++ b/src/bin/pg_waldump/pg_waldump.c
@@ -1219,12 +1219,12 @@ main(int argc, char **argv)
*/
if (first_record != private.startptr &&
XLogSegmentOffset(private.startptr, WalSegSz) != 0)
- printf(ngettext("first record is after %X/%X, at %X/%X, skipping over %u byte\n",
- "first record is after %X/%X, at %X/%X, skipping over %u bytes\n",
- (first_record - private.startptr)),
- LSN_FORMAT_ARGS(private.startptr),
- LSN_FORMAT_ARGS(first_record),
- (uint32) (first_record - private.startptr));
+ pg_log_info(ngettext("first record is after %X/%X, at %X/%X, skipping over %u byte",
+ "first record is after %X/%X, at %X/%X, skipping over %u bytes",
+ (first_record - private.startptr)),
+ LSN_FORMAT_ARGS(private.startptr),
+ LSN_FORMAT_ARGS(first_record),
+ (uint32) (first_record - private.startptr));
if (config.stats == true && !config.quiet)
stats.startptr = first_record;
diff --git a/src/bin/pg_waldump/t/001_basic.pl b/src/bin/pg_waldump/t/001_basic.pl
index d4056e1b95..4bf73b2e34 100644
--- a/src/bin/pg_waldump/t/001_basic.pl
+++ b/src/bin/pg_waldump/t/001_basic.pl
@@ -149,6 +149,24 @@
command_ok([ 'pg_waldump', '-p', $node->data_dir, '--start', $start_lsn, '--end', $end_lsn ], 'runs with path option and start and end locations');
command_fails_like([ 'pg_waldump', '-p', $node->data_dir, '--start', $start_lsn ], qr/error: error in WAL record at/, 'falling off the end of the WAL results in an error');
+# Test for: Display a message that we're skipping data if `from`
+# wasn't a pointer to the start of a record.
+{
+ # Construct a new LSN that is one byte past the original
+ # start_lsn.
+ my ($part1, $part2) = split qr{/}, $start_lsn;
+ my $lsn2 = hex $part2;
+ $lsn2++;
+ my $new_start = sprintf("%s/%X", $part1, $lsn2);
+
+ my (@cmd, $stdout, $stderr, $result);
+
+ @cmd = ( 'pg_waldump', '--start', $new_start, $node->data_dir . '/pg_wal/' . $start_walfile );
+ $result = IPC::Run::run \@cmd, '>', \$stdout, '2>', \$stderr;
+ ok($result, "runs with start segment and start LSN specified");
+ like($stderr, qr/first record is after/, 'info message printed');
+}
+
# Helper function to test various options. Pass options as arguments.
# Output lines are returned as array.
--
2.41.0
v3-0001-Add-more-pg_waldump-tests.patchtext/plain; charset=UTF-8; name=v3-0001-Add-more-pg_waldump-tests.patchDownload
From e5d13b7891a9c0fcc3d28f9bc2756c5372b2fa40 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 14 Jun 2023 08:46:47 +0200
Subject: [PATCH v3 1/2] Add more pg_waldump tests
This adds tests for most command-line options and tests for most
rmgrdesc routines.
Discussion: https://www.postgresql.org/message-id/flat/CAAcByaKM7zyJSDkPWv6_%2BapupY4fXXM3q3SRXas9MMNVPUGcsQ%40mail.gmail.com
---
src/bin/pg_waldump/t/001_basic.pl | 191 ++++++++++++++++++++++++++++++
1 file changed, 191 insertions(+)
diff --git a/src/bin/pg_waldump/t/001_basic.pl b/src/bin/pg_waldump/t/001_basic.pl
index 492a8cadd8..d4056e1b95 100644
--- a/src/bin/pg_waldump/t/001_basic.pl
+++ b/src/bin/pg_waldump/t/001_basic.pl
@@ -3,6 +3,7 @@
use strict;
use warnings;
+use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils;
use Test::More;
@@ -10,4 +11,194 @@
program_version_ok('pg_waldump');
program_options_handling_ok('pg_waldump');
+# wrong number of arguments
+command_fails_like([ 'pg_waldump', ], qr/error: no arguments/, 'no arguments');
+command_fails_like([ 'pg_waldump', 'foo', 'bar', 'baz' ], qr/error: too many command-line arguments/, 'too many arguments');
+
+# invalid option arguments
+command_fails_like([ 'pg_waldump', '--block', 'bad' ], qr/error: invalid block number/, 'invalid block number');
+command_fails_like([ 'pg_waldump', '--fork', 'bad' ], qr/error: invalid fork name/, 'invalid fork name');
+command_fails_like([ 'pg_waldump', '--limit', 'bad' ], qr/error: invalid value/, 'invalid limit');
+command_fails_like([ 'pg_waldump', '--relation', 'bad' ], qr/error: invalid relation/, 'invalid relation specification');
+command_fails_like([ 'pg_waldump', '--rmgr', 'bad' ], qr/error: resource manager .* does not exist/, 'invalid rmgr name');
+command_fails_like([ 'pg_waldump', '--start', 'bad' ], qr/error: invalid WAL location/, 'invalid start LSN');
+command_fails_like([ 'pg_waldump', '--end', 'bad' ], qr/error: invalid WAL location/, 'invalid end LSN');
+
+# rmgr list: If you add one to the list, consider also adding a test
+# case exercising the new rmgr below.
+command_like([ 'pg_waldump', '--rmgr=list'], qr/^XLOG
+Transaction
+Storage
+CLOG
+Database
+Tablespace
+MultiXact
+RelMap
+Standby
+Heap2
+Heap
+Btree
+Hash
+Gin
+Gist
+Sequence
+SPGist
+BRIN
+CommitTs
+ReplicationOrigin
+Generic
+LogicalMessage$/,
+ 'rmgr list');
+
+
+my $node = PostgreSQL::Test::Cluster->new('main');
+$node->init;
+$node->append_conf('postgresql.conf', q{
+autovacuum = off
+checkpoint_timeout = 1h
+
+# for standbydesc
+archive_mode=on
+archive_command=''
+
+# for XLOG_HEAP_TRUNCATE
+wal_level=logical
+});
+$node->start;
+
+my ($start_lsn, $start_walfile) = split /\|/, $node->safe_psql('postgres', q{SELECT pg_current_wal_insert_lsn(), pg_walfile_name(pg_current_wal_insert_lsn())});
+
+$node->safe_psql('postgres', q{
+-- heap, btree, hash, sequence
+CREATE TABLE t1 (a int GENERATED ALWAYS AS IDENTITY, b text);
+CREATE INDEX i1a ON t1 USING btree (a);
+CREATE INDEX i1b ON t1 USING hash (b);
+INSERT INTO t1 VALUES (default, 'one'), (default, 'two');
+DELETE FROM t1 WHERE b = 'one';
+TRUNCATE t1;
+
+-- abort
+START TRANSACTION;
+INSERT INTO t1 VALUES (default, 'three');
+ROLLBACK;
+
+-- unlogged/init fork
+CREATE UNLOGGED TABLE t2 (x int);
+CREATE INDEX i2 ON t2 USING btree (x);
+INSERT INTO t2 SELECT generate_series(1, 10);
+
+-- gin
+CREATE TABLE gin_idx_tbl (id bigserial PRIMARY KEY, data jsonb);
+CREATE INDEX gin_idx ON gin_idx_tbl USING gin (data);
+INSERT INTO gin_idx_tbl
+ WITH random_json AS (
+ SELECT json_object_agg(key, trunc(random() * 10)) as json_data
+ FROM unnest(array['a', 'b', 'c']) as u(key))
+ SELECT generate_series(1,500), json_data FROM random_json;
+
+-- gist, spgist
+CREATE TABLE gist_idx_tbl (p point);
+CREATE INDEX gist_idx ON gist_idx_tbl USING gist (p);
+CREATE INDEX spgist_idx ON gist_idx_tbl USING spgist (p);
+INSERT INTO gist_idx_tbl (p) VALUES (point '(1, 1)'), (point '(3, 2)'), (point '(6, 3)');
+
+-- brin
+CREATE TABLE brin_idx_tbl (col1 int, col2 text, col3 text );
+CREATE INDEX brin_idx ON brin_idx_tbl USING brin (col1, col2, col3) WITH (autosummarize=on);
+INSERT INTO brin_idx_tbl SELECT generate_series(1, 10000), 'dummy', 'dummy';
+UPDATE brin_idx_tbl SET col2 = 'updated' WHERE col1 BETWEEN 1 AND 5000;
+SELECT brin_summarize_range('brin_idx', 0);
+SELECT brin_desummarize_range('brin_idx', 0);
+
+VACUUM;
+
+-- logical message
+SELECT pg_logical_emit_message(true, 'foo', 'bar');
+
+-- relmap
+VACUUM FULL pg_authid;
+
+-- database
+CREATE DATABASE d1;
+DROP DATABASE d1;
+});
+
+my $tblspc_path = PostgreSQL::Test::Utils::tempdir_short();
+
+$node->safe_psql('postgres', qq{
+CREATE TABLESPACE ts1 LOCATION '$tblspc_path';
+DROP TABLESPACE ts1;
+});
+
+my ($end_lsn, $end_walfile) = split /\|/, $node->safe_psql('postgres', q{SELECT pg_current_wal_insert_lsn(), pg_walfile_name(pg_current_wal_insert_lsn())});
+
+my $default_ts_oid = $node->safe_psql('postgres', q{SELECT oid FROM pg_tablespace WHERE spcname = 'pg_default'});
+my $postgres_db_oid = $node->safe_psql('postgres', q{SELECT oid FROM pg_database WHERE datname = 'postgres'});
+my $rel_t1_oid = $node->safe_psql('postgres', q{SELECT oid FROM pg_class WHERE relname = 't1'});
+my $rel_i1a_oid = $node->safe_psql('postgres', q{SELECT oid FROM pg_class WHERE relname = 'i1a'});
+
+$node->stop;
+
+
+# various ways of specifying WAL range
+command_fails_like([ 'pg_waldump', 'foo', 'bar' ], qr/error: could not locate WAL file "foo"/, 'start file not found');
+command_ok([ 'pg_waldump', $node->data_dir . '/pg_wal/' . $start_walfile ], 'runs with start segment specified');
+command_fails_like([ 'pg_waldump', $node->data_dir . '/pg_wal/' . $start_walfile, 'bar' ], qr/error: could not open file "bar"/, 'end file not found');
+command_ok([ 'pg_waldump', $node->data_dir . '/pg_wal/' . $start_walfile, $node->data_dir . '/pg_wal/' . $end_walfile ], 'runs with start and end segment specified');
+command_fails_like([ 'pg_waldump', '-p', $node->data_dir ], qr/error: no start WAL location given/, 'path option requires start location');
+command_ok([ 'pg_waldump', '-p', $node->data_dir, '--start', $start_lsn, '--end', $end_lsn ], 'runs with path option and start and end locations');
+command_fails_like([ 'pg_waldump', '-p', $node->data_dir, '--start', $start_lsn ], qr/error: error in WAL record at/, 'falling off the end of the WAL results in an error');
+
+
+# Helper function to test various options. Pass options as arguments.
+# Output lines are returned as array.
+sub test_pg_waldump
+{
+ local $Test::Builder::Level = $Test::Builder::Level + 1;
+ my @opts = @_;
+
+ my (@cmd, $stdout, $stderr, $result, @lines);
+
+ @cmd = ('pg_waldump', '-p', $node->data_dir, '--start', $start_lsn, '--end', $end_lsn);
+ push @cmd, @opts;
+ $result = IPC::Run::run \@cmd, '>', \$stdout, '2>', \$stderr;
+ ok($result, "pg_waldump @opts: runs ok");
+ is($stderr, '', "pg_waldump @opts: no stderr");
+ @lines = split /\n/, $stdout;
+ ok(@lines > 0, "pg_waldump @opts: some lines are output");
+ return @lines;
+}
+
+my @lines;
+
+@lines = test_pg_waldump;
+is(grep(!/^rmgr: \w/, @lines), 0, 'all output lines are rmgr lines');
+
+@lines = test_pg_waldump('--limit', 6);
+is(@lines, 6, 'limit option observed');
+
+@lines = test_pg_waldump('--fullpage');
+is(grep(!/^rmgr:.*\bFPW\b/, @lines), 0, 'all output lines are FPW');
+
+@lines = test_pg_waldump('--stats');
+like($lines[0], qr/WAL statistics/, "statistics on stdout");
+is(grep(/^rmgr:/, @lines), 0, 'no rmgr lines output');
+
+@lines = test_pg_waldump('--stats=record');
+like($lines[0], qr/WAL statistics/, "statistics on stdout");
+is(grep(/^rmgr:/, @lines), 0, 'no rmgr lines output');
+
+@lines = test_pg_waldump('--rmgr', 'Btree');
+is(grep(!/^rmgr: Btree/, @lines), 0, 'only Btree lines');
+
+@lines = test_pg_waldump('--fork', 'init');
+is(grep(!/fork init/, @lines), 0, 'only init fork lines');
+
+@lines = test_pg_waldump('--relation', "$default_ts_oid/$postgres_db_oid/$rel_t1_oid");
+is(grep(!/rel $default_ts_oid\/$postgres_db_oid\/$rel_t1_oid/, @lines), 0, 'only lines for selected relation');
+
+@lines = test_pg_waldump('--relation', "$default_ts_oid/$postgres_db_oid/$rel_i1a_oid", '--block', 1);
+is(grep(!/\bblk 1\b/, @lines), 0, 'only lines for selected block');
+
+
done_testing();
--
2.41.0
Re: pg_waldump: add test for coverage
The following review has been posted through the commitfest application:
make installcheck-world: tested, passed
Implements feature: tested, passed
Spec compliant: not tested
Documentation: not tested
Hello,
I've reviewed your latest v3 patches on Ubuntu 23.04. Both patches apply correctly and all the tests run and pass as they should. Execution time was normal for me, I didn't notice any significant latency when compared to other tests. The only other feedback I can provide would be to add test coverage to some of the other options that aren't currently covered (ie. --bkp-details, --end, --follow, --path, etc.) for completeness. Other than that, this looks like a great patch.
Kind regards,
Tristen
Re: pg_waldump: add test for coverage
On 29.06.23 21:16, Tristen Raab wrote:
I've reviewed your latest v3 patches on Ubuntu 23.04. Both patches apply correctly and all the tests run and pass as they should. Execution time was normal for me, I didn't notice any significant latency when compared to other tests. The only other feedback I can provide would be to add test coverage to some of the other options that aren't currently covered (ie. --bkp-details, --end, --follow, --path, etc.) for completeness. Other than that, this looks like a great patch.
Committed.
I added a test for the --quiet option. --end and --path are covered.
The only options not covered now are
-b, --bkp-details output detailed information about backup blocks
-f, --follow keep retrying after reaching end of WAL
-t, --timeline=TLI timeline from which to read WAL records
-x, --xid=XID only show records with transaction ID XID
--follow is a bit tricky to test because you need to leave pg_waldump
running in the background for a while, or something like that.
--timeline and --xid can be tested but would need some work on the
underlying test data (such as creating more than one timeline). I don't
know much about --bkp-details, so I don't have a good idea how to test
it. So I'll leave those as projects for the future.