Re: TopoSort() fix
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Andres Freund <andres@anarazel.de>,
Alvaro Herrera <alvherre@2ndquadrant.com>,
PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Date: 2019-07-29T21:55:14Z
Lists: pgsql-hackers
Attachments
- hack-to-test-parallel-locking.patch (text/x-diff) patch
[ removing <ruihaij@gmail.com>, as that mailing address seems to be MIA ] Robert Haas <robertmhaas@gmail.com> writes: > On Mon, Jul 29, 2019 at 10:56 AM Tom Lane <tgl@sss.pgh.pa.us> wrote: >> If Robert doesn't weigh in pretty soon, I'll take responsibility for it. > That's fine, or if you prefer that I commit it, I will. FYI, I just got done inventing a way to reach that code, and I have to suspect that it's impossible to do so in production, because under ordinary circumstances no parallel worker will take any exclusive lock that isn't already held by its leader. (If you happen to know an easy counterexample, let's see it.) The attached heavily-hacked version of deadlock-soft.spec makes it go by forcing duplicate advisory locks to be taken in worker processes, which of course first requires disabling PreventAdvisoryLocksInParallelMode(). I kind of wonder if we should provide some debug-only, here-be-dragons way to disable that restriction so that we could make this an official regression test, because I'm now pretty suspicious that none of this code has ever executed before. Anyway, armed with this, I was able to prove that HEAD just hangs up on this test case; apparently the deadlock checker never detects that the additional holders of the advisory lock need to be rearranged. And removing that "break" fixes it. So I'll go commit the break-ectomy, but what do people think about testing this better? regards, tom lane
Commits
-
Add an isolation test to exercise parallel-worker deadlock resolution.
- da9456d22a76 13.0 landed
-
Mark advisory-lock functions as parallel restricted, not parallel unsafe.
- 4886da832750 13.0 landed
-
Fix busted logic for parallel lock grouping in TopoSort().
- d933816c04c6 12.0 landed
- c3b613e1b0e5 9.6.15 landed
- af41ab50fdc5 10.10 landed
- 3420851a2c2d 13.0 landed
- 0dcb1c6c4874 11.5 landed