force_parallel_mode is meant to be used to allow us to exercise the
parallel query infrastructure to ensure that it's working as we expect.
It seems some users think this GUC is for forcing the query planner into
picking a parallel plan regardless of the costs. A quick look at the
documentation would have made them realize that they were wrong, but the
GUC is likely too conveniently named which, evidently, seems to often
result in users expecting that it forces the planner into usefully
parallelizing queries.
Here we rename the GUC to something which casual users are less likely to
mistakenly think is what they need to make their query run more quickly.
For now, the old name can still be used. We'll revisit if the old name
mapping can be removed once the buildfarm configs are all updated.
Reviewed-by: John Naylor
Discussion: https://postgr.es/m/CAApHDvrsOi92_uA7PEaHZMH-S4Xv+MGhQWA+GrP8b1kjpS1HjQ@mail.gmail.com
Previously, isolationtester displayed SQL query results using some
ad-hoc code that clearly hadn't had much effort expended on it.
Field values longer than 14 characters weren't separated from
the next field, and usually caused misalignment of the columns
too. Also there was no visual separation of a query's result
from subsequent isolationtester output. This made test result
files confusing and hard to read.
To improve matters, let's use libpq's PQprint() function. Although
that's long since unused by psql, it's still plenty good enough
for the purpose here.
Like 741d7f104, back-patch to all supported branches, so that this
isn't a stumbling block for back-patching isolation test changes.
Discussion: https://postgr.es/m/582362.1623798221@sss.pgh.pa.us
This test failed fairly reproducibly on some CLOBBER_CACHE_ALWAYS
buildfarm animals. The cause seems to be that if a parallel worker
is slow enough to reach its lock wait, it may not be released by
the first deadlock check run, and then later deadlock checks might
decide to unblock the d2 session instead of the d1 session, leaving
us in an undetected deadlock state (since the isolationtester client
is waiting for d1 to complete first).
Fix by introducing an additional lock wait at the end of the d2a1
step, ensuring that the deadlock checker will recognize that d1
has to be unblocked before d2a1 completes.
Also reduce max_parallel_workers_per_gather to 3 in this test. With the
default max_worker_processes value, we were only getting one parallel
worker for the d2a1 step, which is not the case I hoped to test. We
should get 3 for d1a2 and 2 for d2a1, as the code stands; and maybe 3
for d2a1 if somebody figures out why the last parallel worker slot isn't
free already.
Discussion: https://postgr.es/m/22195.1566077308@sss.pgh.pa.us
Commit a1c1af2a1 added logic in the deadlock checker to handle lock
grouping, but it was very poorly tested, as evidenced by the bug
fixed in 3420851a2. Add a test case that exercises that a bit better
(and catches the bug --- if you revert 3420851a2, this will hang).
Since it's pretty hard to get parallel workers to take exclusive
regular locks that their parents don't already have, this test operates
by creating a deadlock among advisory locks taken in parallel workers.
To make that happen, we must override the parallel-safety labeling of
the advisory-lock functions, which we do by putting them in mislabeled,
non-inlinable wrapper functions.
We also have to remove the redundant PreventAdvisoryLocksInParallelMode
checks in lockfuncs.c. That seems fine though; if some user accidentally
does what this test is intentionally doing, not much harm will ensue.
(If there are any remaining bugs that are reachable that way, they're
probably reachable in other ways too.)
Discussion: https://postgr.es/m/3243.1564437314@sss.pgh.pa.us
David Rowley
Tom Lane