Skip to content

Commit 837cc73

Browse files
committed
Fix performance issue in deadlock-parallel isolation test.
With debug_discard_caches = 1, the runtime of this test script increased by about a factor of 10 after commit 0dca5d6. That's causing some of our buildfarm animals to fail with a timeout. The reason for the increased time is that now we are re-planning some intentionally-non-inlineable SQL functions on every execution, where the previous coding held onto the original plans throughout the outer query. The previous behavior was arguably quite buggy, so I don't think 0dca5d6 deserves blame here. But we would like this test script to not take so long. To fix, instead of forcing a "parallel safe" label via a non-inlineable SQL function, apply it directly to the advisory-lock functions by making internal-language aliases for them. A small problem is that the advisory-lock functions return void but this test would really like them to return integer 1. I cheated here by declaring the aliases as returning "int". That's perhaps undue familiarity with the implementation of PG_RETURN_VOID(), but that hasn't changed in twenty years and is unlikely to do so in the next twenty. That gets us an integer 0 result, and then an inline-able wrapper to convert that to an integer 1 allows the rest of the script to remain unchanged. For me, this reduces the runtime with debug_discard_caches = 1 by about 100x, making the test comfortably faster than before instead of slower. Discussion: https://postgr.es/m/136163.1744179562@sss.pgh.pa.us
1 parent 5bbc596 commit 837cc73

File tree

1 file changed

+15
-4
lines changed

1 file changed

+15
-4
lines changed

src/test/isolation/specs/deadlock-parallel.spec

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22

33
# It's fairly hard to get parallel worker processes to block on locks,
44
# since generally they don't want any locks their leader didn't already
5-
# take. We cheat like mad here by making a function that takes a lock,
6-
# and is incorrectly marked parallel-safe so that it can execute in a worker.
5+
# take. We cheat like mad here by creating aliases for advisory-lock
6+
# functions that are incorrectly marked parallel-safe so that they can
7+
# execute in a worker.
78

89
# Note that we explicitly override any global settings of isolation level
910
# or debug_parallel_query, to ensure we're testing what we intend to.
@@ -36,11 +37,21 @@
3637

3738
setup
3839
{
40+
-- The alias functions themselves. Really these return "void", but
41+
-- the implementation is such that we can declare them to return "int",
42+
-- and we will get a zero result.
43+
create function lock_share(bigint) returns int language internal as
44+
'pg_advisory_xact_lock_shared_int8' strict parallel safe;
45+
46+
create function lock_excl(bigint) returns int language internal as
47+
'pg_advisory_xact_lock_int8' strict parallel safe;
48+
49+
-- Inline-able wrappers that will produce an integer "1" result:
3950
create function lock_share(int,int) returns int language sql as
40-
'select pg_advisory_xact_lock_shared($1); select 1;' parallel safe;
51+
'select 1 - lock_share($1)' parallel safe;
4152

4253
create function lock_excl(int,int) returns int language sql as
43-
'select pg_advisory_xact_lock($1); select 1;' parallel safe;
54+
'select 1 - lock_excl($1)' parallel safe;
4455

4556
create table bigt as select x from generate_series(1, 10000) x;
4657
analyze bigt;

0 commit comments

Comments
 (0)