Re: [bugfix] DISCARD ALL does not release advisory locks

On Mon, Nov 24, 2008 at 10:25 AM, Marko Kreen <markokr(at)gmail(dot)com> wrote:
> On 11/24/08, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> "Marko Kreen" <markokr(at)gmail(dot)com> writes:
>> > It was brought to my attention that DISCARD ALL
>> > does not release advisory locks:
>>
>> What is the argument that it should?
>
> DISCARD ALL is supposed to be used by poolers to reset connection
> back to startup state to reuse server connection after client
> disconnect. New client should see no difference between fresh
> backend and old backend where DISCARD ALL was issued.
>
> IOW, DISCARD ALL should be functionally equivalent to backend exit.
>
> If user want more explicit control over what resources are released,
> he should avoid use of DISCARD ALL, instead he should manually pick
> individual components from the command sequence DISCARD ALL
> is equivalent to. Eg. when user wants to keep old plans or
> advisory locks around, he should manually construct command list
> that resets everything except those.
>
> But DISCARD ALL should release everything possible, never should additional
> commands be needed in addition to it to do full reset.

Having done a lot of work with advisory locks, I support this change.
Advisory locks are essentially session scoped objects like prepared
statements or notifies. It's only natural to clean them up in the
same way.

That said, I don't think this should be backpatched to 8.3. I'm aware
of at least one project that makes heavy use of advisory locks
(openads). Since this project and possibly others are probably used
in bouncing web environments, you have to be careful with behavior
changes like that. People need time...unexpected advisory lock issues
can get nasty. If you need the behavior now, just install the patch
yourself :-)

merlin