pgsql: Switch pg_verify_checksums back to a blacklist

Started by Michael Paquierabout 7 years ago5 messages
#1Michael Paquier
michael@paquier.xyz

Switch pg_verify_checksums back to a blacklist

This basically reverts commit d55241af705667d4503638e3f77d3689fd6be31,
leaving around a portion of the regression tests still adapted with
empty relation files, and corrupted cases. This is also proving to be
failing to check properly relation files located in a non-default
tablespace path.

Per discussion with various folks, including Stephen Frost, David
Steele, Andres Freund, Michael Banck and myself.

Reported-by: Michael Banck
Discussion: /messages/by-id/20181021134206.GA14282@paquier.xyz
Backpatch-through: 11

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/a1c91dd1108c2e6536935619341c79c238734f77

Modified Files
--------------
src/bin/pg_verify_checksums/pg_verify_checksums.c | 79 ++++++-----------------
src/bin/pg_verify_checksums/t/002_actions.pl | 17 -----
2 files changed, 18 insertions(+), 78 deletions(-)

#2Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#1)
1 attachment(s)
Re: pgsql: Switch pg_verify_checksums back to a blacklist

On Fri, Nov 30, 2018 at 01:36:43AM +0000, Michael Paquier wrote:

Switch pg_verify_checksums back to a blacklist

This basically reverts commit d55241af705667d4503638e3f77d3689fd6be31,
leaving around a portion of the regression tests still adapted with
empty relation files, and corrupted cases. This is also proving to be
failing to check properly relation files located in a non-default
tablespace path.

So, jacana is not in love with this commit for the tests:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&dt=2018-11-30%2003%3A34%3A23

The error is here:
server started
# Postmaster PID for node "node_checksum" is 14820
error running SQL: 'psql:<stdin>:1: ERROR: directory
"/home/pgrunner/bf/root/HEAD/pgsql.build/src/bin/pg_verify_checksums/tmp_check/t_002_actions_node_checksum_data/ts_corrupt_dir"
does not exist'

What visibly happens is that we create a path for a tablespace which
will include a corrupted table. However psql tries to run CREATE
TABLESPACE but gets back a complain about the path not existing. The
thing is that mkdir has been used beforehand so the path should be
here.

I have tested this set of TAP tests on my Windows 10 VM with MSVC 2015
before commit and those tests passed. Could it be that jacana is using
msys, so a trick similar to what is used in 014_unlogged_reinit.pl is
necessary?

Andrew, could you confirm if the patch attached works? I would prefer
not doing a blind fix if possible..
--
Michael

Attachments:

verify-checksum-tap-msys.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/pg_verify_checksums/t/002_actions.pl b/src/bin/pg_verify_checksums/t/002_actions.pl
index 12cca604e6..181f58f67c 100644
--- a/src/bin/pg_verify_checksums/t/002_actions.pl
+++ b/src/bin/pg_verify_checksums/t/002_actions.pl
@@ -113,11 +113,10 @@ command_fails(['pg_verify_checksums',  '-D', $pgdata],
 check_relation_corruption($node, 'corrupt1', 'pg_default');
 
 # Create tablespace to check corruptions in a non-default tablespace.
-my $basedir = $node->basedir;
-my $tablespace_dir = "$basedir/ts_corrupt_dir";
-mkdir ($tablespace_dir);
+my $tablespaceDir = TestLib::tempdir;
+my $realTSDir = TestLib::real_dir($tablespaceDir);
 $node->safe_psql('postgres',
-	"CREATE TABLESPACE ts_corrupt LOCATION '$tablespace_dir';");
+	"CREATE TABLESPACE ts_corrupt LOCATION '$realTSDir';");
 check_relation_corruption($node, 'corrupt2', 'ts_corrupt');
 
 # Utility routine to check that pg_verify_checksums is able to detect
#3Andrew Dunstan
andrew.dunstan@2ndquadrant.com
In reply to: Michael Paquier (#2)
Re: pgsql: Switch pg_verify_checksums back to a blacklist

On 11/30/18 12:35 AM, Michael Paquier wrote:

On Fri, Nov 30, 2018 at 01:36:43AM +0000, Michael Paquier wrote:

Switch pg_verify_checksums back to a blacklist

This basically reverts commit d55241af705667d4503638e3f77d3689fd6be31,
leaving around a portion of the regression tests still adapted with
empty relation files, and corrupted cases. This is also proving to be
failing to check properly relation files located in a non-default
tablespace path.

So, jacana is not in love with this commit for the tests:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&amp;dt=2018-11-30%2003%3A34%3A23

The error is here:
server started
# Postmaster PID for node "node_checksum" is 14820
error running SQL: 'psql:<stdin>:1: ERROR: directory
"/home/pgrunner/bf/root/HEAD/pgsql.build/src/bin/pg_verify_checksums/tmp_check/t_002_actions_node_checksum_data/ts_corrupt_dir"
does not exist'

What visibly happens is that we create a path for a tablespace which
will include a corrupted table. However psql tries to run CREATE
TABLESPACE but gets back a complain about the path not existing. The
thing is that mkdir has been used beforehand so the path should be
here.

I have tested this set of TAP tests on my Windows 10 VM with MSVC 2015
before commit and those tests passed. Could it be that jacana is using
msys, so a trick similar to what is used in 014_unlogged_reinit.pl is
necessary?

Andrew, could you confirm if the patch attached works? I would prefer
not doing a blind fix if possible..

All it actually needs is this additional line after the mkdir:

$tablespace_dir = TestLib::real_dir($tablespace_dir);

Explanation: TAP tests on msys need to run with the DTK perl, which
understands msys virtualized paths. Postgres, however, does not
understand such paths, so before you use such a value in, say, CREATE
TABLESPACE, you need to translate it into a path on the underlying file
system.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#4Michael Paquier
michael@paquier.xyz
In reply to: Andrew Dunstan (#3)
Re: pgsql: Switch pg_verify_checksums back to a blacklist

On Fri, Nov 30, 2018 at 02:18:18PM -0500, Andrew Dunstan wrote:

All it actually needs is this additional line after the mkdir:

$tablespace_dir = TestLib::real_dir($tablespace_dir);

Explanation: TAP tests on msys need to run with the DTK perl, which
understands msys virtualized paths. Postgres, however, does not understand
such paths, so before you use such a value in, say, CREATE TABLESPACE, you
need to translate it into a path on the underlying file system.

Thanks Andrew for confirming. I have committed your suggestion, which
should calm down jacana.
--
Michael

#5Michael Paquier
michael@paquier.xyz
In reply to: Michael Paquier (#4)
Re: pgsql: Switch pg_verify_checksums back to a blacklist

On Sat, Dec 01, 2018 at 08:00:16AM +0900, Michael Paquier wrote:

Thanks Andrew for confirming. I have committed your suggestion, which
should calm down jacana.

And it is now green:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=jacana&amp;dt=2018-12-01%2004%3A02%3A53
--
Michael