Add an isolation test to exercise parallel-worker deadlock resolution.

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

diff --git a/src/backend/utils/adt/lockfuncs.c b/src/backend/utils/adt/lockfuncs.c
index ffd1970f589bc95bd432a2dff3a5680f63b1ba49..bc62c6eef75d0a17c627980079c499d37c88c300 100644 (file)
--- a/src/backend/utils/adt/lockfuncs.c
+++ b/src/backend/utils/adt/lockfuncs.c
@@ -655,15 +655,6 @@ pg_isolation_test_session_is_blocked(PG_FUNCTION_ARGS)
 #define SET_LOCKTAG_INT32(tag, key1, key2) \
    SET_LOCKTAG_ADVISORY(tag, MyDatabaseId, key1, key2, 2)
 
-static void
-PreventAdvisoryLocksInParallelMode(void)
-{
-   if (IsInParallelMode())
-       ereport(ERROR,
-               (errcode(ERRCODE_INVALID_TRANSACTION_STATE),
-                errmsg("cannot use advisory locks during a parallel operation")));
-}
-
 /*
  * pg_advisory_lock(int8) - acquire exclusive lock on an int8 key
  */
@@ -673,7 +664,6 @@ pg_advisory_lock_int8(PG_FUNCTION_ARGS)
    int64       key = PG_GETARG_INT64(0);
    LOCKTAG     tag;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT64(tag, key);
 
    (void) LockAcquire(&tag, ExclusiveLock, true, false);
@@ -691,7 +681,6 @@ pg_advisory_xact_lock_int8(PG_FUNCTION_ARGS)
    int64       key = PG_GETARG_INT64(0);
    LOCKTAG     tag;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT64(tag, key);
 
    (void) LockAcquire(&tag, ExclusiveLock, false, false);
@@ -708,7 +697,6 @@ pg_advisory_lock_shared_int8(PG_FUNCTION_ARGS)
    int64       key = PG_GETARG_INT64(0);
    LOCKTAG     tag;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT64(tag, key);
 
    (void) LockAcquire(&tag, ShareLock, true, false);
@@ -726,7 +714,6 @@ pg_advisory_xact_lock_shared_int8(PG_FUNCTION_ARGS)
    int64       key = PG_GETARG_INT64(0);
    LOCKTAG     tag;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT64(tag, key);
 
    (void) LockAcquire(&tag, ShareLock, false, false);
@@ -746,7 +733,6 @@ pg_try_advisory_lock_int8(PG_FUNCTION_ARGS)
    LOCKTAG     tag;
    LockAcquireResult res;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT64(tag, key);
 
    res = LockAcquire(&tag, ExclusiveLock, true, true);
@@ -767,7 +753,6 @@ pg_try_advisory_xact_lock_int8(PG_FUNCTION_ARGS)
    LOCKTAG     tag;
    LockAcquireResult res;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT64(tag, key);
 
    res = LockAcquire(&tag, ExclusiveLock, false, true);
@@ -787,7 +772,6 @@ pg_try_advisory_lock_shared_int8(PG_FUNCTION_ARGS)
    LOCKTAG     tag;
    LockAcquireResult res;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT64(tag, key);
 
    res = LockAcquire(&tag, ShareLock, true, true);
@@ -808,7 +792,6 @@ pg_try_advisory_xact_lock_shared_int8(PG_FUNCTION_ARGS)
    LOCKTAG     tag;
    LockAcquireResult res;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT64(tag, key);
 
    res = LockAcquire(&tag, ShareLock, false, true);
@@ -828,7 +811,6 @@ pg_advisory_unlock_int8(PG_FUNCTION_ARGS)
    LOCKTAG     tag;
    bool        res;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT64(tag, key);
 
    res = LockRelease(&tag, ExclusiveLock, true);
@@ -848,7 +830,6 @@ pg_advisory_unlock_shared_int8(PG_FUNCTION_ARGS)
    LOCKTAG     tag;
    bool        res;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT64(tag, key);
 
    res = LockRelease(&tag, ShareLock, true);
@@ -866,7 +847,6 @@ pg_advisory_lock_int4(PG_FUNCTION_ARGS)
    int32       key2 = PG_GETARG_INT32(1);
    LOCKTAG     tag;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT32(tag, key1, key2);
 
    (void) LockAcquire(&tag, ExclusiveLock, true, false);
@@ -885,7 +865,6 @@ pg_advisory_xact_lock_int4(PG_FUNCTION_ARGS)
    int32       key2 = PG_GETARG_INT32(1);
    LOCKTAG     tag;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT32(tag, key1, key2);
 
    (void) LockAcquire(&tag, ExclusiveLock, false, false);
@@ -903,7 +882,6 @@ pg_advisory_lock_shared_int4(PG_FUNCTION_ARGS)
    int32       key2 = PG_GETARG_INT32(1);
    LOCKTAG     tag;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT32(tag, key1, key2);
 
    (void) LockAcquire(&tag, ShareLock, true, false);
@@ -922,7 +900,6 @@ pg_advisory_xact_lock_shared_int4(PG_FUNCTION_ARGS)
    int32       key2 = PG_GETARG_INT32(1);
    LOCKTAG     tag;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT32(tag, key1, key2);
 
    (void) LockAcquire(&tag, ShareLock, false, false);
@@ -943,7 +920,6 @@ pg_try_advisory_lock_int4(PG_FUNCTION_ARGS)
    LOCKTAG     tag;
    LockAcquireResult res;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT32(tag, key1, key2);
 
    res = LockAcquire(&tag, ExclusiveLock, true, true);
@@ -965,7 +941,6 @@ pg_try_advisory_xact_lock_int4(PG_FUNCTION_ARGS)
    LOCKTAG     tag;
    LockAcquireResult res;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT32(tag, key1, key2);
 
    res = LockAcquire(&tag, ExclusiveLock, false, true);
@@ -986,7 +961,6 @@ pg_try_advisory_lock_shared_int4(PG_FUNCTION_ARGS)
    LOCKTAG     tag;
    LockAcquireResult res;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT32(tag, key1, key2);
 
    res = LockAcquire(&tag, ShareLock, true, true);
@@ -1008,7 +982,6 @@ pg_try_advisory_xact_lock_shared_int4(PG_FUNCTION_ARGS)
    LOCKTAG     tag;
    LockAcquireResult res;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT32(tag, key1, key2);
 
    res = LockAcquire(&tag, ShareLock, false, true);
@@ -1029,7 +1002,6 @@ pg_advisory_unlock_int4(PG_FUNCTION_ARGS)
    LOCKTAG     tag;
    bool        res;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT32(tag, key1, key2);
 
    res = LockRelease(&tag, ExclusiveLock, true);
@@ -1050,7 +1022,6 @@ pg_advisory_unlock_shared_int4(PG_FUNCTION_ARGS)
    LOCKTAG     tag;
    bool        res;
 
-   PreventAdvisoryLocksInParallelMode();
    SET_LOCKTAG_INT32(tag, key1, key2);
 
    res = LockRelease(&tag, ShareLock, true);

diff --git a/src/test/isolation/expected/deadlock-parallel.out b/src/test/isolation/expected/deadlock-parallel.out
new file mode 100644 (file)
index 0000000..871a80c
--- /dev/null
+++ b/src/test/isolation/expected/deadlock-parallel.out
@@ -0,0 +1,47 @@
+Parsed test spec with 4 sessions
+
+starting permutation: d1a1 d2a2 e1l e2l d1a2 d2a1 d1c e1c d2c e2c
+step d1a1: SELECT lock_share(1,x) FROM bigt LIMIT 1;
+lock_share     
+
+1              
+step d2a2: select lock_share(2,x) FROM bigt LIMIT 1;
+lock_share     
+
+1              
+step e1l: SELECT lock_excl(1,x) FROM bigt LIMIT 1; <waiting ...>
+step e2l: SELECT lock_excl(2,x) FROM bigt LIMIT 1; <waiting ...>
+step d1a2: SET force_parallel_mode = on;
+             SET parallel_setup_cost = 0;
+             SET parallel_tuple_cost = 0;
+             SET min_parallel_table_scan_size = 0;
+             SET parallel_leader_participation = off;
+             SET max_parallel_workers_per_gather = 4;
+             SELECT sum(lock_share(2,x)) FROM bigt; <waiting ...>
+step d2a1: SET force_parallel_mode = on;
+             SET parallel_setup_cost = 0;
+             SET parallel_tuple_cost = 0;
+             SET min_parallel_table_scan_size = 0;
+             SET parallel_leader_participation = off;
+             SET max_parallel_workers_per_gather = 4;
+             SELECT sum(lock_share(1,x)) FROM bigt; <waiting ...>
+step d1a2: <... completed>
+sum            
+
+10000          
+step d1c: COMMIT;
+step e1l: <... completed>
+lock_excl      
+
+1              
+step d2a1: <... completed>
+sum            
+
+10000          
+step e1c: COMMIT;
+step d2c: COMMIT;
+step e2l: <... completed>
+lock_excl      
+
+1              
+step e2c: COMMIT;

diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 74b50779e2560e9f14cccf654ab1234fd9a91f5c..009c2ec54bdb4619ccbf0def56038776fb578c71 100644 (file)
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -21,6 +21,7 @@ test: deadlock-simple
 test: deadlock-hard
 test: deadlock-soft
 test: deadlock-soft-2
+test: deadlock-parallel
 test: fk-contention
 test: fk-deadlock
 test: fk-deadlock2

diff --git a/src/test/isolation/specs/deadlock-parallel.spec b/src/test/isolation/specs/deadlock-parallel.spec
new file mode 100644 (file)
index 0000000..aa4a084
--- /dev/null
+++ b/src/test/isolation/specs/deadlock-parallel.spec
@@ -0,0 +1,89 @@
+# Test deadlock resolution with parallel process groups.
+
+# It's fairly hard to get parallel worker processes to block on locks,
+# since generally they don't want any locks their leader didn't already
+# take.  We cheat like mad here by making a function that takes a lock,
+# and is incorrectly marked parallel-safe so that it can execute in a worker.
+
+# Note that we explicitly override any global settings of isolation level
+# or force_parallel_mode, to ensure we're testing what we intend to.
+
+# Otherwise, this is morally equivalent to deadlock-soft.spec:
+# Four-process deadlock with two hard edges and two soft edges.
+# d2 waits for e1 (soft edge), e1 waits for d1 (hard edge),
+# d1 waits for e2 (soft edge), e2 waits for d2 (hard edge).
+# The deadlock detector resolves the deadlock by reversing the d1-e2 edge,
+# unblocking d1.
+
+setup
+{
+  create function lock_share(int,int) returns int language sql as
+  'select pg_advisory_xact_lock_shared($1); select 1;' parallel safe;
+
+  create function lock_excl(int,int) returns int language sql as
+  'select pg_advisory_xact_lock($1); select 1;' parallel safe;
+
+  create table bigt as select x from generate_series(1, 10000) x;
+  analyze bigt;
+}
+
+teardown
+{
+  drop function lock_share(int,int);
+  drop function lock_excl(int,int);
+  drop table bigt;
+}
+
+session "d1"
+setup      { BEGIN isolation level repeatable read;
+             SET force_parallel_mode = off;
+             SET deadlock_timeout = '10s';
+}
+# this lock will be taken in the leader, so it will persist:
+step "d1a1"    { SELECT lock_share(1,x) FROM bigt LIMIT 1; }
+# this causes all the parallel workers to take locks:
+step "d1a2"    { SET force_parallel_mode = on;
+             SET parallel_setup_cost = 0;
+             SET parallel_tuple_cost = 0;
+             SET min_parallel_table_scan_size = 0;
+             SET parallel_leader_participation = off;
+             SET max_parallel_workers_per_gather = 4;
+             SELECT sum(lock_share(2,x)) FROM bigt; }
+step "d1c" { COMMIT; }
+
+session "d2"
+setup      { BEGIN isolation level repeatable read;
+             SET force_parallel_mode = off;
+             SET deadlock_timeout = '10ms';
+}
+# this lock will be taken in the leader, so it will persist:
+step "d2a2"    { select lock_share(2,x) FROM bigt LIMIT 1; }
+# this causes all the parallel workers to take locks:
+step "d2a1"    { SET force_parallel_mode = on;
+             SET parallel_setup_cost = 0;
+             SET parallel_tuple_cost = 0;
+             SET min_parallel_table_scan_size = 0;
+             SET parallel_leader_participation = off;
+             SET max_parallel_workers_per_gather = 4;
+             SELECT sum(lock_share(1,x)) FROM bigt; }
+step "d2c" { COMMIT; }
+
+session "e1"
+setup      { BEGIN isolation level repeatable read;
+             SET force_parallel_mode = on;
+             SET deadlock_timeout = '10s';
+}
+# this lock will be taken in a parallel worker, but we don't need it to persist
+step "e1l" { SELECT lock_excl(1,x) FROM bigt LIMIT 1; }
+step "e1c" { COMMIT; }
+
+session "e2"
+setup      { BEGIN isolation level repeatable read;
+             SET force_parallel_mode = on;
+             SET deadlock_timeout = '10s';
+}
+# this lock will be taken in a parallel worker, but we don't need it to persist
+step "e2l" { SELECT lock_excl(2,x) FROM bigt LIMIT 1; }
+step "e2c" { COMMIT; }
+
+permutation "d1a1" "d2a2" "e1l" "e2l" "d1a2" "d2a1" "d1c" "e1c" "d2c" "e2c"