Horton

Thread

Commits

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

  2. Mark advisory-lock functions as parallel restricted, not parallel unsafe.

  3. Fix busted logic for parallel lock grouping in TopoSort().

  1. TopoSort() fix

    Rui Hai Jiang <ruihaij@gmail.com> — 2019-07-02T14:47:32Z

    Hi Hackers,

I think I found an issue in the TopoSort() function.

As the comments say,

                /* .....
                 * ...... If there are any other processes
                 * in the same lock group on the queue, set their number of
                 * beforeConstraints to -1 to indicate that they should be
emitted
                 * with their groupmates rather than considered separately.
                 */

If the line "break;" exists, there is no chance to set beforeConstraints to
-1 for other processes in the same lock group.

So, I think we need delete the line "break;" . See the patch.

I just took a look, and I found all the following versions have this line .


postgresql-12beta2, postgresql-12beta1, postgresql-11.4,
postgresql-11.3,postgresql-11.0,
postgresql-10.9,postgresql-10.5, postgresql-10.0


Thanks,
Ruihai

  2. Re: TopoSort() fix

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-07-02T15:23:30Z

    Rui Hai Jiang <ruihaij@gmail.com> writes:
> I think I found an issue in the TopoSort() function.

This indeed seems like a live bug introduced by a1c1af2a.
Robert?

			regards, tom lane

  3. Re: TopoSort() fix

    Rui Hai Jiang <ruihaij@gmail.com> — 2019-07-03T02:41:59Z

    Could the attached patch fix this issue? Or does any one else plan to fix
it?

If people are busy and have not  time, I can go ahead to fix it.  To fix
this issue, do we need a patch for each official branch?


Regards,
Ruihai

On Tue, Jul 2, 2019 at 11:23 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:

> Rui Hai Jiang <ruihaij@gmail.com> writes:
> > I think I found an issue in the TopoSort() function.
>
> This indeed seems like a live bug introduced by a1c1af2a.
> Robert?
>
>                         regards, tom lane
>

  4. Re: TopoSort() fix

    Michael Paquier <michael@paquier.xyz> — 2019-07-03T07:11:31Z

    On Wed, Jul 03, 2019 at 10:41:59AM +0800, Rui Hai Jiang wrote:
> Could the attached patch fix this issue? Or does any one else plan to fix
> it?
> 
> If people are busy and have not  time, I can go ahead to fix it.  To fix
> this issue, do we need a patch for each official branch?

Only a committer could merge any fix you produce.  What you have sent
looks fine to me, so let's wait for Robert, who has visiblu broken
this part to comment.  Back-patched versions are usually taken care of
by the committer merging the fix, and by experience it is better to
agree about the shape of a patch on HEAD before working on other
branches.  Depending on the review done, the patch's shape may change
slightly...
--
Michael

  5. Re: TopoSort() fix

    Robert Haas <robertmhaas@gmail.com> — 2019-07-03T13:38:16Z

    On Tue, Jul 2, 2019 at 11:23 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Rui Hai Jiang <ruihaij@gmail.com> writes:
> > I think I found an issue in the TopoSort() function.
>
> This indeed seems like a live bug introduced by a1c1af2a.
> Robert?

This is pretty thoroughly swapped out of my head, but it looks like
that analysis might be correct.

Is it practical to come up with a test case that demonstrates the problem?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  6. Re: TopoSort() fix

    Rui Hai Jiang <ruihaij@gmail.com> — 2019-07-04T03:15:50Z

    I'll try to figure out some  scenarios to do the test. A parallel process
group is needed for the test.

Actually I was trying to do some testing against the locking mechanism. I
happened to see this issue.

On Wed, Jul 3, 2019 at 9:38 PM Robert Haas <robertmhaas@gmail.com> wrote:

> On Tue, Jul 2, 2019 at 11:23 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > Rui Hai Jiang <ruihaij@gmail.com> writes:
> > > I think I found an issue in the TopoSort() function.
> >
> > This indeed seems like a live bug introduced by a1c1af2a.
> > Robert?
>
> This is pretty thoroughly swapped out of my head, but it looks like
> that analysis might be correct.
>
> Is it practical to come up with a test case that demonstrates the problem?
>
> --
> Robert Haas
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

  7. Re: TopoSort() fix

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-07-26T22:05:38Z

    On 2019-Jul-04, Rui Hai Jiang wrote:

> I'll try to figure out some  scenarios to do the test. A parallel process
> group is needed for the test.
> 
> Actually I was trying to do some testing against the locking mechanism. I
> happened to see this issue.

Hello, is anybody looking into this issue?

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  8. Re: TopoSort() fix

    Andres Freund <andres@anarazel.de> — 2019-07-26T23:48:35Z

    Hi,

On 2019-07-26 18:05:38 -0400, Alvaro Herrera wrote:
> On 2019-Jul-04, Rui Hai Jiang wrote:
> 
> > I'll try to figure out some  scenarios to do the test. A parallel process
> > group is needed for the test.

Rui, have you made any progress on this?


> > Actually I was trying to do some testing against the locking mechanism. I
> > happened to see this issue.
> 
> Hello, is anybody looking into this issue?

I guess this is on Robert's docket otherwise. He's on vacation till
early next week...

Greetings,

Andres Freund

  9. Re: TopoSort() fix

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-07-27T00:24:16Z

    Andres Freund <andres@anarazel.de> writes:
> On 2019-07-26 18:05:38 -0400, Alvaro Herrera wrote:
>> Hello, is anybody looking into this issue?

> I guess this is on Robert's docket otherwise. He's on vacation till
> early next week...

I think this is a sufficiently obvious bug, and a sufficiently
obvious fix, that we should just fix it and not insist on getting
a reproducible test case first.  I think a test case would soon
bit-rot anyway, and no longer exercise the problem.

I certainly do *not* want to wait so long that we miss the
upcoming minor releases.

			regards, tom lane

  10. Re: TopoSort() fix

    Michael Paquier <michael@paquier.xyz> — 2019-07-29T01:50:59Z

    On Fri, Jul 26, 2019 at 08:24:16PM -0400, Tom Lane wrote:
> I think this is a sufficiently obvious bug, and a sufficiently
> obvious fix, that we should just fix it and not insist on getting
> a reproducible test case first.  I think a test case would soon
> bit-rot anyway, and no longer exercise the problem.
> 
> I certainly do *not* want to wait so long that we miss the
> upcoming minor releases.

+1.  Any volunteers?
--
Michael

  11. Re: TopoSort() fix

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-07-29T14:56:05Z

    Michael Paquier <michael@paquier.xyz> writes:
> On Fri, Jul 26, 2019 at 08:24:16PM -0400, Tom Lane wrote:
>> I think this is a sufficiently obvious bug, and a sufficiently
>> obvious fix, that we should just fix it and not insist on getting
>> a reproducible test case first.  I think a test case would soon
>> bit-rot anyway, and no longer exercise the problem.
>> I certainly do *not* want to wait so long that we miss the
>> upcoming minor releases.

> +1.  Any volunteers?

If Robert doesn't weigh in pretty soon, I'll take responsibility for it.

			regards, tom lane

  12. Re: TopoSort() fix

    Robert Haas <robertmhaas@gmail.com> — 2019-07-29T20:40:13Z

    On Mon, Jul 29, 2019 at 10:56 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Michael Paquier <michael@paquier.xyz> writes:
> > On Fri, Jul 26, 2019 at 08:24:16PM -0400, Tom Lane wrote:
> >> I think this is a sufficiently obvious bug, and a sufficiently
> >> obvious fix, that we should just fix it and not insist on getting
> >> a reproducible test case first.  I think a test case would soon
> >> bit-rot anyway, and no longer exercise the problem.
> >> I certainly do *not* want to wait so long that we miss the
> >> upcoming minor releases.
>
> > +1.  Any volunteers?
>
> 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.

I just got back from a week's vacation and am only very gradually
unburying myself from mounds of email.  (Of course, the way
pgsql-hackers is getting, there's sort of always a mound of email
these days.)

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  13. Re: TopoSort() fix

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-07-29T21:55:14Z

    [ 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

  14. Re: TopoSort() fix

    Robert Haas <robertmhaas@gmail.com> — 2019-07-30T00:57:15Z

    On Mon, Jul 29, 2019 at 5:55 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> 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.)

I think the way you could make that happen would be to run a parallel
query that calls a user-defined function which does LOCK TABLE.

> 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.

Nice!

> So I'll go commit the break-ectomy, but what do people think about
> testing this better?

I think it's a great idea.  I was never very happy with the amount of
exercise I was able to give this code.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  15. Re: TopoSort() fix

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-07-30T01:48:28Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Jul 29, 2019 at 5:55 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> 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.)

> I think the way you could make that happen would be to run a parallel
> query that calls a user-defined function which does LOCK TABLE.

I tried that first.  There are backstops preventing doing LOCK TABLE
in a worker, just like for advisory locks.

I believe the only accessible route to taking any sort of new lock
in a parallel worker is catalog lookups causing AccessShareLock on
a catalog.  In principle, maybe you could make a deadlock situation
by combining parallel workers with something that takes
AccessExclusiveLock on a catalog ... but making that into a reliable
test case sounds about impossible, because AEL on a catalog will
have all sorts of unpleasant side-effects, such as blocking
isolationtester's own queries.  (Getting it to work in a
CLOBBER_CACHE_ALWAYS build seems right out, for instance.)

			regards, tom lane

  16. Re: TopoSort() fix

    Robert Haas <robertmhaas@gmail.com> — 2019-07-30T02:06:09Z

    On Mon, Jul 29, 2019 at 9:48 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I tried that first.  There are backstops preventing doing LOCK TABLE
> in a worker, just like for advisory locks.
>
> I believe the only accessible route to taking any sort of new lock
> in a parallel worker is catalog lookups causing AccessShareLock on
> a catalog.

Can't the worker just query a previously-untouched table, maybe by
constructing a string and then using EXECUTE to execute it?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  17. Re: TopoSort() fix

    Michael Paquier <michael@paquier.xyz> — 2019-07-30T02:31:05Z

    On Mon, Jul 29, 2019 at 10:56:05AM -0400, Tom Lane wrote:
> Michael Paquier <michael@paquier.xyz> writes:
>> +1.  Any volunteers?
> 
> If Robert doesn't weigh in pretty soon, I'll take responsibility for it.

Thanks Tom for taking care of it!
--
Michael

  18. Re: TopoSort() fix

    Alvaro Herrera <alvherre@2ndquadrant.com> — 2019-07-30T03:21:57Z

    On 2019-Jul-29, Tom Lane wrote:

> 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.

Hmm, okay, so this wasn't a bug that would have bit anyone in practice,
yeah?  That's reassuring.

Thanks,

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

  19. Re: TopoSort() fix

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-07-30T04:06:02Z

    Alvaro Herrera <alvherre@2ndquadrant.com> writes:
> On 2019-Jul-29, Tom Lane wrote:
>> 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.

> Hmm, okay, so this wasn't a bug that would have bit anyone in practice,
> yeah?  That's reassuring.

At the least, you'd have to go well out of your way to make it happen.

			regards, tom lane

  20. Re: TopoSort() fix

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-07-30T14:27:24Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Jul 29, 2019 at 9:48 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> I believe the only accessible route to taking any sort of new lock
>> in a parallel worker is catalog lookups causing AccessShareLock on
>> a catalog.

> Can't the worker just query a previously-untouched table, maybe by
> constructing a string and then using EXECUTE to execute it?

Hm, yeah, looks like you could get a new AccessShareLock that way too.
But not any exclusive lock.

I also looked into whether one could use SELECT FOR UPDATE/SHARE to get
stronger locks at a tuple level, but that's been blocked off as well.
You guys really did a pretty good job of locking that down.

After thinking about this for awhile, though, I believe it might be
reasonable to just remove PreventAdvisoryLocksInParallelMode()
altogether.  The "parallel unsafe" markings on the advisory-lock
functions seem like adequate protection against somebody running
them in a parallel worker.  If you defeat that by calling them from
a mislabeled-parallel-safe wrapper (as the proposed test case does),
then any negative consequences are on your own head.  AFAICT the
only actual negative consequence is that the locks disappear the
moment the parallel worker exits, so we'd not be opening any large
holes even to people who rip off the safety cover.

(BTW, why aren't these functions just "parallel restricted"?)

			regards, tom lane

  21. Re: TopoSort() fix

    Robert Haas <robertmhaas@gmail.com> — 2019-07-30T16:46:41Z

    On Tue, Jul 30, 2019 at 10:27 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I also looked into whether one could use SELECT FOR UPDATE/SHARE to get
> stronger locks at a tuple level, but that's been blocked off as well.
> You guys really did a pretty good job of locking that down.

Thanks.  We learned from the master.

> After thinking about this for awhile, though, I believe it might be
> reasonable to just remove PreventAdvisoryLocksInParallelMode()
> altogether.  The "parallel unsafe" markings on the advisory-lock
> functions seem like adequate protection against somebody running
> them in a parallel worker.  If you defeat that by calling them from
> a mislabeled-parallel-safe wrapper (as the proposed test case does),
> then any negative consequences are on your own head.  AFAICT the
> only actual negative consequence is that the locks disappear the
> moment the parallel worker exits, so we'd not be opening any large
> holes even to people who rip off the safety cover.
>
> (BTW, why aren't these functions just "parallel restricted"?)

I don't exactly remember why we installed all of these restrictions
any more.  You might be able to find some discussion of it by
searching the archives.  I believe we may have been concerned about
the fact that group locking would cause advisory locks taken in one
process not to conflict with the same advisory lock taken in some
cooperating process, and maybe that would be unwelcome behavior for
someone.  For example, suppose the user defines a function that takes
an advisory lock on the number 1, does a bunch of stuff that should
never happen multiply at the same time, and then releases the lock.
Without parallel query, that will work.  With parallel query, it
won't, because several workers running the same query might run the
same function simultaneously and their locks won't conflict.

But it is really pretty arguable whether we should feel responsible
for that problem.  We could just decide that if you're doing that, and
you don't want the scenario described above to happen, you oughta mark
the function that contains this logic at least PARALLEL RESTRICTED,
and if you don't, then it's your fault for doing a dumb thing.  I
believe when we were early on in the development of this we wanted to
be very conservative lest, ah, someone accuse us of not locking things
down well enough, but maybe at this point parallel query is a
sufficiently well-established concept that we should lighten up on
some cases where we took an overly-stringent line.  If we take that
view, then I'm not sure why these functions couldn't be just marked
PARALLEL SAFE.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  22. Re: TopoSort() fix

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-07-30T17:36:36Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Jul 30, 2019 at 10:27 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> (BTW, why aren't these functions just "parallel restricted"?)

> ...
> But it is really pretty arguable whether we should feel responsible
> for that problem.  We could just decide that if you're doing that, and
> you don't want the scenario described above to happen, you oughta mark
> the function that contains this logic at least PARALLEL RESTRICTED,
> and if you don't, then it's your fault for doing a dumb thing.  I
> believe when we were early on in the development of this we wanted to
> be very conservative lest, ah, someone accuse us of not locking things
> down well enough, but maybe at this point parallel query is a
> sufficiently well-established concept that we should lighten up on
> some cases where we took an overly-stringent line.  If we take that
> view, then I'm not sure why these functions couldn't be just marked
> PARALLEL SAFE.

No, there's a sufficient reason why we should force advisory locks
to be taken in the leader process, namely that the behavior is totally
different if we don't: they will disappear at the end of the parallel
worker run, not at end of transaction or session as documented.

However, that argument doesn't seem to be a reason why the advisory-lock
functions couldn't be parallel-restricted rather than parallel-unsafe.

In any case, my question at the moment is whether we need the belt-and-
suspenders-too approach of having both non-parallel-safe marking and an
explicit check inside these functions.  We've largely moved away from
hard-wired checks for e.g. superuserness, and surely these things are
less dangerous than most formerly-superuser-only functions.

			regards, tom lane

  23. Re: TopoSort() fix

    Robert Haas <robertmhaas@gmail.com> — 2019-07-30T17:40:06Z

    On Tue, Jul 30, 2019 at 1:36 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> No, there's a sufficient reason why we should force advisory locks
> to be taken in the leader process, namely that the behavior is totally
> different if we don't: they will disappear at the end of the parallel
> worker run, not at end of transaction or session as documented.

Oh, good point.  I forgot about that.

> However, that argument doesn't seem to be a reason why the advisory-lock
> functions couldn't be parallel-restricted rather than parallel-unsafe.

Agreed.

> In any case, my question at the moment is whether we need the belt-and-
> suspenders-too approach of having both non-parallel-safe marking and an
> explicit check inside these functions.  We've largely moved away from
> hard-wired checks for e.g. superuserness, and surely these things are
> less dangerous than most formerly-superuser-only functions.

If we can't think of a way that the lack of these checks could crash
it, then I think it's OK to remove the hardwired checks.  If we can,
I'd favor keeping them.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  24. Re: TopoSort() fix

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-07-30T17:44:17Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Jul 30, 2019 at 1:36 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> In any case, my question at the moment is whether we need the belt-and-
>> suspenders-too approach of having both non-parallel-safe marking and an
>> explicit check inside these functions.  We've largely moved away from
>> hard-wired checks for e.g. superuserness, and surely these things are
>> less dangerous than most formerly-superuser-only functions.

> If we can't think of a way that the lack of these checks could crash
> it, then I think it's OK to remove the hardwired checks.  If we can,
> I'd favor keeping them.

Well, there'd be an actual isolation test that they work ;-), if you
override the marking.  Admittedly, one test case does not prove that
there's no way to crash the system, but that can be said of most
parts of Postgres.

			regards, tom lane

  25. Re: TopoSort() fix

    Robert Haas <robertmhaas@gmail.com> — 2019-07-30T17:45:42Z

    On Tue, Jul 30, 2019 at 1:44 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Well, there'd be an actual isolation test that they work ;-), if you
> override the marking.  Admittedly, one test case does not prove that
> there's no way to crash the system, but that can be said of most
> parts of Postgres.

True.  I'm just talking about what we can foresee.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

  26. Re: TopoSort() fix

    Tom Lane <tgl@sss.pgh.pa.us> — 2019-07-30T18:10:13Z

    Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Jul 30, 2019 at 1:44 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Well, there'd be an actual isolation test that they work ;-), if you
>> override the marking.  Admittedly, one test case does not prove that
>> there's no way to crash the system, but that can be said of most
>> parts of Postgres.

> True.  I'm just talking about what we can foresee.

Sure.  But I think what we can foresee is that if there are any bugs
reachable this way, they'd be reachable and need fixing regardless.
We've already established that parallel workers can take and release locks
that their leader isn't holding.  Apparently, they won't take anything
stronger than RowExclusiveLock; but even AccessShare is enough to let a
process participate in all interesting behaviors of the lock manager,
including blocking, being blocked, and being released early by deadlock
resolution.  And the advisory-lock functions are pretty darn thin wrappers
around the lock manager.  So I'm finding it hard to see where there's
incremental risk, even if a user does intentionally bypass the parallel
safety markings.  And what we get in return is an easier way to add tests
for this area.

			regards, tom lane

  27. Re: TopoSort() fix

    Robert Haas <robertmhaas@gmail.com> — 2019-07-31T15:46:05Z

    On Tue, Jul 30, 2019 at 2:10 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Sure.  But I think what we can foresee is that if there are any bugs
> reachable this way, they'd be reachable and need fixing regardless.
> We've already established that parallel workers can take and release locks
> that their leader isn't holding.  Apparently, they won't take anything
> stronger than RowExclusiveLock; but even AccessShare is enough to let a
> process participate in all interesting behaviors of the lock manager,
> including blocking, being blocked, and being released early by deadlock
> resolution.  And the advisory-lock functions are pretty darn thin wrappers
> around the lock manager.  So I'm finding it hard to see where there's
> incremental risk, even if a user does intentionally bypass the parallel
> safety markings.  And what we get in return is an easier way to add tests
> for this area.

Sure, I was basically just asking whether you could foresee any
crash-risk of the proposed change.  It sounds like the answer is "no,"
so I'm fine with it on that basis.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company