On Wed, Apr 12, 2023 at 3:04 PM Thomas Munro <thomas.mu...@gmail.com> wrote:
> On Wed, Apr 12, 2023 at 2:56 PM Christoph Berg <m...@debian.org> wrote:
> > I'm hitting a panic in t_004_io_direct. The build is running on
> > overlayfs on tmpfs/ext4 (upper/lower) which is probably a weird
> > combination but has worked well for building everything over the last
> > decade. On Debian unstable:
> >
> > PANIC:  could not open file "pg_wal/000000010000000000000001": Invalid 
> > argument

> ... I have a new idea:  perhaps it is possible to try
> to open a file with O_DIRECT from perl, and if it fails like that,
> skip the test.  Looking into that now.

I think I have that working OK.  Any Perl hackers want to comment on
my use of IO::File (copied from examples on the internet that showed
how to use O_DIRECT)?  I am not much of a perl hacker but according to
my package manager, IO/File.pm came with perl itself.  And the Fcntl
eval trick that I copied from File::stat, and the perl-critic
suppression that requires?

I tested this on OpenBSD, which has no O_DIRECT, so that tests the
first reason to skip.

Does it skip OK on your system, for the second reason?  Should we be
more specific about the errno?

As far as I know, the only systems on the build farm that should be
affected by this change are the illumos boxen (they have O_DIRECT,
unlike Solaris, but perl's $^O couldn't tell the difference between
Solaris and illumos, so they didn't previously run this test).

One thing I resisted the urge to do is invent PG_TEST_SKIP, a sort of
anti-PG_TEST_EXTRA.  I think I'd rather hear about it if there is a
system out there that passes the pre-flight check, but fails later on,
because we'd better investigate why.  That's basically the point of
shipping this "developer only" feature long before serious use of it.
From 72e865835bcf1c9dce2090de0da66839908133c6 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.mu...@gmail.com>
Date: Wed, 12 Apr 2023 16:26:54 +1200
Subject: [PATCH] Skip the 004_io_direct.pl test if a pre-flight check fails.

Previously the test had a list of OSes that direct I/O was expected to
work on, with the idea that we could easily adjust that if problems
showed up on less common systems.  That didn't take account of the
possibility of running the tests on an OS where it works on typical
filesystems (eg all the systems our build farm), but doesn't work for
unusual systems like overlayfs, as Christoph discovered.

The new approach is to try to create a file with O_DIRECT from perl.  If
that fails, we'll log the errno and skip the whole test.

Reported-by: Christoph Berg <m...@debian.org>
Discussion: https://postgr.es/m/ZDYd4A78cT2ULxZZ%40msg.df7cb.de

diff --git a/src/test/modules/test_misc/t/004_io_direct.pl b/src/test/modules/test_misc/t/004_io_direct.pl
index 5a2dd0d288..467b3bfc02 100644
--- a/src/test/modules/test_misc/t/004_io_direct.pl
+++ b/src/test/modules/test_misc/t/004_io_direct.pl
@@ -2,19 +2,44 @@
 
 use strict;
 use warnings;
+use Fcntl;
+use IO::File;
 use PostgreSQL::Test::Cluster;
 use PostgreSQL::Test::Utils;
 use Test::More;
 
-# Systems that we know to have direct I/O support, and whose typical local
-# filesystems support it or at least won't fail with an error.  (illumos should
-# probably be in this list, but perl reports it as solaris.  Solaris should not
-# be in the list because we don't support its way of turning on direct I/O, and
-# even if we did, its version of ZFS rejects it, and OpenBSD just doesn't have
-# it.)
-if (!grep { $^O eq $_ } qw(aix darwin dragonfly freebsd linux MSWin32 netbsd))
+# We know that macOS has F_NOCACHE, and we know that Windows has
+# FILE_FLAG_NO_BUFFERING, and we assume that their typical file systems will
+# accept those flags.  For every other system, we'll probe for O_DIRECT
+# support.
+
+if ($^O ne 'darwin' && $^O ne 'MSWin32')
 {
-	plan skip_all => "no direct I/O support";
+	# Perl's Fcntl module knows if this system's <fcntl.h> has O_DIRECT but can
+	# only tell us by reporting an error, so we copy a trick from File/stat.pm
+	# and probe for the definition with eval.
+	no strict 'refs';    ## no critic (ProhibitNoStrict)
+	my $val = eval { &{'Fcntl::O_DIRECT'} };
+	if (defined $val)
+	{
+		use Fcntl qw(O_DIRECT);
+
+		# Next we want to find out if we can successfully open a file using
+		# that on the present filesystem.
+		my $f = IO::File->new(
+			"${PostgreSQL::Test::Utils::tmp_check}/test_o_direct_file",
+			O_RDWR | O_DIRECT | O_CREAT);
+		if (!$f)
+		{
+			plan skip_all =>
+			  "pre-flight attempt to open file with O_DIRECT fails with errno $!";
+		}
+		$f->close;
+	}
+	else
+	{
+		plan skip_all => "no O_DIRECT";
+	}
 }
 
 my $node = PostgreSQL::Test::Cluster->new('main');
-- 
2.39.2

Reply via email to