On 11/09/2023 15:00, David Rowley wrote:
> On Wed, 5 Jul 2023 at 21:44, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>>
>>> index 296dc82d2ee..edb8b6026e5 100644
>>> --- a/src/backend/commands/discard.c
>>> +++ b/src/backend/commands/discard.c
>>> @@ -71,7 +71,7 @@ DiscardAll(bool isTopLevel)
>>> Async_UnlistenAll();
>>> - LockReleaseAll(USER_LOCKMETHOD, true);
>>> + LockReleaseSession(USER_LOCKMETHOD);
>>> ResetPlanCache();
>>
>> This assumes that there are no transaction-level advisory locks. I think
>> that's OK. It took me a while to convince myself of that, though. I
>> think we need a high level comment somewhere that explains what
>> assumptions we make on which locks can be held in session mode and which
>> in transaction mode.
>
> Isn't it ok because DISCARD ALL cannot run inside a transaction block,
> so there should be no locks taken apart from possibly session-level
> locks?
Hmm, sounds valid. I think I convinced myself that it's OK through some
other reasoning, but I don't remember it now.
> I've added a call to LockAssertNoneHeld(false) in there.
I don't see it in the patch?
--
Heikki Linnakangas
Neon (https://neon.tech)