| Age | Commit message (Collapse) | Author |
|---|
|
Buildfarm members skimmer and crake have reported that pg_upgrade
running from v18 fails due to the changes of d52c24b0f808, with the
expectations that the objects removed in the test module
injection_points should still be present post upgrades, but the test
module does not have them anymore.
The origin of the issue is that the following test modules depend on
injection_points, but they do not drop the extension once the tests
finish, leaving its traces in the dumps used for the upgrades:
- gin, down to v17
- typcache, down to v18
- nbtree, HEAD-only
Test modules have no upgrade requirements, as they are used only for..
Tests, so there is no point in keeping them around.
An alternative solution would be to drop the databases created by these
modules in AdjustUpgrade.pm, but the solution of this commit to drop the
extension is simpler. Note that there would be a catch if using a
solution based on AdjustUpgrade.pm as the database name used for the
test runs differs between configure and meson:
- configure relies on USE_MODULE_DB for the database name unicity, that
would build a database name based on the *first* entry of REGRESS, that
lists all the SQL tests.
- meson relies on a "name" field.
For example, for the test module "gin", the regression database is named
"regression_gin" under meson, while it is more complex for configure, as
of "contrib_regression_gin_incomplete_splits". So a AdjustUpgrade.pm
would need a set of DROP DATABASE IF EXISTS to solve this issue, to cope
with each build system.
The failure has been caused by d52c24b0f808, and the problem can happen
with upgrade dumps from v17 and v18 to HEAD. This problem is not
currently reachable in the back-branches, but it could be possible that
a future change in injection_points in stable branches invalidates this
theory, so this commit is applied down to v17 in the test modules that
matter.
Per discussion with Tom Lane and Heikki Linnakangas.
Discussion: https://postgr.es/m/2899652.1765167313@sss.pgh.pa.us
Backpatch-through: 17
|
|
Backpatch-through: 13
|
|
f587338dec87 has introduced in the test module injection_points a SQL
function called injection_points_set_local(), that can be used to make
all the injection points linked to the process where they are attached,
discarded automatically if any remain once the process exits.
e2e3b8ae9ed7 has added a NO_INSTALLCHECK to the test module to prevent
the use of installcheck. Now that there is a way to make the test
concurrent-safe, let's use it and remove the installcheck restriction.
Concurrency issues could be easily reproduced by running in a tight
loop a command like this one, in src/test/modules/gin/ (hardcoding
pg_sleep() after attaching injection points enlarges the race window)
and a second test suite like contrib/btree_gin/:
make installcheck USE_MODULE_DB=1
Reviewed-by: Andrey Borodin
Discussion: https://postgr.es/m/ZhNG4Io9uYOgwv3F@paquier.xyz
|
|
As written, the test would fail when run repeatedly because one of the
injection points attached was not detached. This would not matter if
the test is rewritten to be concurrently safe, but let's be clean and
it is a good practice.
Oversight in 6a1ea02c491d.
Discussion: https://postgr.es/m/ZfP7IDs9TvrKe49x@paquier.xyz
|
|
Per Tom Lane's observation that the test got stuck in infinite loop if
the injection_points module was not loaded. It was supposed to give up
after 10000 iterations, but the backstop was broken.
Discussion: https://www.postgresql.org/message-id/2498595.1710511222%40sss.pgh.pa.us
|
|
The buildfarm script attempts to run all tests marked as
NO_INSTALLCHECK under src/test/modules without paying attention to
whether they are enabled or disabled in the parent Makefile. That
hasn't been a problem so far, because all the tests marked with
NO_INSTALLCHECK ran unconditionally in "make check". But commit
e2e3b8ae9e changed that: the injection fault tests are marked as
NO_INSTALLCHECK, and also depend on --enable-injection-points.
Try to work around that by ensuring that "make check" does nothing in
the those subdirectories. We can hopefully get rid of this hack soon,
after fixing the buildfarm client, or by switching to meson.
Discussion: https://www.postgresql.org/message-id/8e4cf596-dd70-432e-9068-16466ed596ed%40iki.fi
|
|
The 'gin' test injections faults to GIN index build. If another test
running concurrently in the same cluster also tries to create a GIN
index, it will hit the fault, too.
To fix, disable tests using injection points when running against an
existing cluster. A better long-term solution would be to make the
injection points scoped to the database or process, but this will do
for now.
Discussion: https://www.postgresql.org/message-id/CA%2BhUKGJYhcG_o2nwSK6r01eOZJwNWUJUbX%3D%3DAVnW84f-%2B8yamQ@mail.gmail.com
Discussion: https://www.postgresql.org/message-id/10fd6cdd-c5d9-46fe-9fa1-7e661191309e@iki.fi
|
|
This has been forgotten in 6a1ea02c491d.
|
|
ginFinishSplit() expects the caller to hold an exclusive lock on the
buffer, but when finishing an earlier "leftover" incomplete split of
an internal page, the caller held a shared lock. That caused an
assertion failure in MarkBufferDirty(). Without assertions, it could
lead to corruption if two backends tried to complete the split at the
same time.
On master, add a test case using the new injection point facility.
Report and analysis by Fei Changhong. Backpatch the fix to all
supported versions.
Reviewed-by: Fei Changhong, Michael Paquier
Discussion: https://www.postgresql.org/message-id/tencent_A3CE810F59132D8E230475A5F0F7A08C8307@qq.com
|
