Re: TopoSort() fix

[ removing <ruihaij(at)gmail(dot)com>, as that mailing address seems to be MIA ]

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Mon, Jul 29, 2019 at 10:56 AM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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