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)
