Re: Speed up transaction completion faster after many relations are accessed in a transaction

On 10/02/2023 04:51, David Rowley wrote:
> I've attached another set of patches. I do need to spend longer
> looking at this. I'm mainly attaching these as CI seems to be
> highlighting a problem that I'm unable to recreate locally and I
> wanted to see if the attached fixes it.

I like this patch's approach.

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

> @@ -3224,14 +3206,6 @@ PostPrepare_Locks(TransactionId xid)
>                         Assert(lock->nGranted <= lock->nRequested);
>                         Assert((proclock->holdMask & ~lock->grantMask) == 0);
>  
> -                       /* Ignore it if nothing to release (must be a session lock) */
> -                       if (proclock->releaseMask == 0)
> -                               continue;
> -
> -                       /* Else we should be releasing all locks */
> -                       if (proclock->releaseMask != proclock->holdMask)
> -                               elog(PANIC, "we seem to have dropped a bit somewhere");
> -
>                         /*
>                          * We cannot simply modify proclock->tag.myProc to reassign
>                          * ownership of the lock, because that's part of the hash key and

This looks wrong. If you prepare a transaction that is holding any 
session locks, we will now transfer them to the prepared transaction. 
And its locallock entry will be out of sync. To fix, I think we could 
keep around the hash table that CheckForSessionAndXactLocks() builds, 
and use that here.

-- 
Heikki Linnakangas
Neon (https://neon.tech)