Subject: Test 041_checkpoint_at_promote.pl faild in installcheck due to missing injection_points

From: Maxim Orlov <[email-masked]>
Date: Mon, 19 Aug 2024 18:00:32 +0300

Hi!

After rebasing one of my old patches, I'm hit to a problem with the
installcheck test for 041_checkpoint_at_promote.pl.
At first, I thought it was something wrong with my patch, although it
doesn't relate to this part of the Postgres.
Then I decided to do the same run but on current master and got the same
result.

Here is my configure:
SRC="../postgres"
TRG=`pwd`

LINUX_CONFIGURE_FEATURES="
        --without-llvm
        --with-tcl --with-tclconfig=/usr/lib/tcl8.6/ --with-perl
        --with-python --with-gssapi --with-pam --with-ldap --with-selinux
        --with-systemd --with-uuid=ossp --with-libxml --with-libxslt
--with-zstd
        --with-ssl=openssl
"

$SRC/configure \
        -C \
        --prefix=$TRG/"pgsql" \
        --enable-debug --enable-tap-tests --enable-depend --enable-cassert \
        --enable-injection-points --enable-nls \
        $LINUX_CONFIGURE_FEATURES \
        CC="ccache clang" CXX="ccache clang++" \
        CFLAGS="-Og -ggdb -fsanitize-recover=all" \
        CXXFLAGS="-Og -ggdb -fsanitize-recover=all"

And here is my run:
$ time make PROVE_TESTS=t/041_checkpoint_at_promote.pl installcheck -C
src/test/recovery
...
# Postmaster PID for node "standby1" is 820439
error running SQL: 'psql:<stdin>:1: ERROR:  extension "injection_points" is
not available
DETAIL:  Could not open extension control file
"/home/omg/proj/build/pgsql/share/extension/injection_points.control": No
such file or directory.
HINT:  The extension must first be installed on the system where PostgreSQL
is running.'
while running 'psql -XAtq -d port=17154 host=/tmp/xkTLcw1tDb
dbname='postgres' -f - -v ON_ERROR_STOP=1' with sql 'CREATE EXTENSION
injection_points;' at
/home/omg/proj/build/../postgres/src/test/perl/PostgreSQL/Test/Cluster.pm
line 2140.
# Postmaster PID for node "master" is 820423
...

Cleary, Postgres can't find injection_points extension.
Am I doing something wrong, or it is a problem with injection points extension
itself?

-- 
Best regards,
Maxim Orlov.

---

From: Tom Lane <[email-masked]>
Date: Mon, 19 Aug 2024 12:10:17 -0400

> installcheck test for 041_checkpoint_at_promote.pl.

src/test/recovery/README points out that

  Also, to use "make installcheck", you must have built and installed
  contrib/pg_prewarm, contrib/pg_stat_statements and contrib/test_decoding
  in addition to the core code.

I suspect this needs some additional verbiage about also installing
src/test/modules/injection_points if you've enabled injection points.

(I think we haven't noticed because most people just use "make check"
instead.)

			regards, tom lane

---

From: Maxim Orlov <[email-masked]>
Date: Tue, 20 Aug 2024 10:29:42 +0300

> (I think we haven't noticed because most people just use "make check"

OK, many thanks for a comprehensive explanation!

-- 
Best regards,
Maxim Orlov.

---

From: Maxim Orlov <[email-masked]>
Date: Tue, 20 Aug 2024 14:40:10 +0300

Shame on me, I didn't mention one thing in the original email.  Actually,
the problem starts for me with "make installcheck-world".
And only then I've run a specific test 041_checkpoint_at_promote.pl. I.e.
the whole sequence of the commands are:
configure --enable_injection_points ...
make world
make install-world
initdb ...
pg_ctl ... start
make installcheck-world

And all the tests passes perfectly, for example, in REL_15_STABLE because
there is no opt like enable_injection_points.
But this is not the case if we are dealing with the current master branch.
So, my point here: installcheck-world doesn't
work on the current master branch until I explicitly install
injection_points extension.  In my view, it's a bit wired, since
neither test_decoding, pg_stat_statements or pg_prewarm demand it.

-- 
Best regards,
Maxim Orlov.

---

From: Tom Lane <[email-masked]>
Date: Tue, 20 Aug 2024 11:04:07 -0400

> neither test_decoding, pg_stat_statements or pg_prewarm demand it.

Ugh.  The basic issue here is that "make install-world" doesn't
install anything from underneath src/test/modules, which I recall
as being an intentional decision.  Rather than poking a hole in
that policy for injection_points, I wonder if we should move it
to contrib.

			regards, tom lane

---

From: Tom Lane <[email-masked]>
Date: Tue, 20 Aug 2024 12:10:08 -0400

> that policy for injection_points, I wonder if we should move it

... which would also imply writing documentation and so forth,
and it'd mean that injection_points starts to show up in end-user
installations.  (That would happen with the alternative choice of
hacking install-world to include src/test/modules/injection_points,
too.)  While you could argue that that'd be helpful for extension
authors who'd like to use injection_points in their own tests, I'm
not sure that it's where we want to go with that module.  It's only
meant as test scaffolding, and I don't think we've analyzed the
implications of some naive user installing it.

We do, however, need to preserve the property that installcheck
works after install-world.  I'm starting to think that maybe
the 041 test should be hacked to silently skip if it doesn't
find injection_points available.  (We could then remove some of
the makefile hackery that's supporting the current behavior.)
Probably the same needs to happen in each other test script
that's using injection_points --- I imagine that Maxim's
test is simply failing here first.

			regards, tom lane

---

From: Michael Paquier <[email-masked]>
Date: Wed, 21 Aug 2024 08:25:20 +0900

> implications of some naive user installing it.

The original line of thoughts here is that if I were to write a test
that relies on injection points for a critical bug fix, then I'd
rather not have to worry about the hassle of maintaining user-facing
documentation to get the work done.  The second line is that we should
be able to tweak the module or even break its ABI as much as we want
in stable branches to accomodate to the tests, which is what a test
module is good for.  The same ABI argument kind of stands as well for
the backend portion, but we'll see where it goes.

> test is simply failing here first.

Yeah, we could do that.
--
Michael