Thread: pgsql: Avoid spurious deadlocks when upgrading a tuple lock
Avoid spurious deadlocks when upgrading a tuple lock When two (or more) transactions are waiting for transaction T1 to release a tuple-level lock, and transaction T1 upgrades its lock to a higher level, a spurious deadlock can be reported among the waiting transactions when T1 finishes. The simplest example case seems to be: T1: select id from job where name = 'a' for key share; Y: select id from job where name = 'a' for update; -- starts waiting for X Z: select id from job where name = 'a' for key share; T1: update job set name = 'b' where id = 1; Z: update job set name = 'c' where id = 1; -- starts waiting for X T1: rollback; At this point, transaction Y is rolled back on account of a deadlock: Y holds the heavyweight tuple lock and is waiting for the Xmax to be released, while Z holds part of the multixact and tries to acquire the heavyweight lock (per protocol) and goes to sleep; once X releases its part of the multixact, Z is awakened only to be put back to sleep on the heavyweight lock that Y is holding while sleeping. Kaboom. This can be avoided by having Z skip the heavyweight lock acquisition. As far as I can see, the biggest downside is that if there are multiple Z transactions, the order in which they resume after X finishes is not guaranteed. Backpatch to 9.6. The patch applies cleanly on 9.5, but the new tests don't work there (because isolationtester is not smart enough), so I'm not going to risk it. Author: Oleksii Kliukin Discussion: https://postgr.es/m/B9C9D7CD-EB94-4635-91B6-E558ACEC0EC3@hintbits.com Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/de87a084c0a5ac927017cd0834b33a932651cfc9 Modified Files -------------- src/backend/access/heap/README.tuplock | 10 ++ src/backend/access/heap/heapam.c | 84 +++++++++--- .../expected/tuplelock-upgrade-no-deadlock.out | 150 +++++++++++++++++++++ src/test/isolation/isolation_schedule | 1 + .../specs/tuplelock-upgrade-no-deadlock.spec | 57 ++++++++ 5 files changed, 281 insertions(+), 21 deletions(-)
Alvaro Herrera <alvherre@alvh.no-ip.org> writes: > Avoid spurious deadlocks when upgrading a tuple lock I'm now getting heapam.c: In function 'heap_lock_tuple': heapam.c:4041: warning: 'skip_tuple_lock' may be used uninitialized in this function Please fix. regards, tom lane
On 2019-Jun-14, Tom Lane wrote: > Alvaro Herrera <alvherre@alvh.no-ip.org> writes: > > Avoid spurious deadlocks when upgrading a tuple lock > > I'm now getting > > heapam.c: In function 'heap_lock_tuple': > heapam.c:4041: warning: 'skip_tuple_lock' may be used uninitialized in this function Hm, I don't get that warning. Does this patch silence it, please? -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Alvaro Herrera <alvherre@2ndquadrant.com> writes: > On 2019-Jun-14, Tom Lane wrote: >> I'm now getting >> heapam.c: In function 'heap_lock_tuple': >> heapam.c:4041: warning: 'skip_tuple_lock' may be used uninitialized in this function > Hm, I don't get that warning. Does this patch silence it, please? Uh, no patch attached? But initializing the variable where it's declared would certainly silence it. regards, tom lane
I wrote: >> Hm, I don't get that warning. Does this patch silence it, please? > Uh, no patch attached? But initializing the variable where it's > declared would certainly silence it. BTW, after looking around a bit I wonder if this complaint isn't exposing an actual logic bug. Shouldn't skip_tuple_lock have a lifetime similar to first_time? regards, tom lane
On 2019-Jun-14, Tom Lane wrote: > I wrote: > >> Hm, I don't get that warning. Does this patch silence it, please? > > > Uh, no patch attached? But initializing the variable where it's > > declared would certainly silence it. > > BTW, after looking around a bit I wonder if this complaint isn't > exposing an actual logic bug. Shouldn't skip_tuple_lock have > a lifetime similar to first_time? I think you're right. I should come up with a test case that exercises that case. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services