Remove redundant PGPROC.lockGroupLeaderIdentifier field.

We don't really need this field, because it's either zero or redundant with
PGPROC.pid.  The use of zero to mark "not a group leader" is not necessary
since we can just as well test whether lockGroupLeader is NULL.  This does
not save very much, either as to code or data, but the simplification seems
worthwhile anyway.

diff --git a/src/backend/storage/lmgr/README b/src/backend/storage/lmgr/README
index c399618bc77c7aefc5556bc4edfb34acdfc9872d..56b0a12a3e3658d0cc1f4d7c9c3551f94450b510 100644 (file)
--- a/src/backend/storage/lmgr/README
+++ b/src/backend/storage/lmgr/README
@@ -650,27 +650,16 @@ those cases so that they no longer use heavyweight locking in the first place
 (which is not a crazy idea, given that such lock acquisitions are not expected
 to deadlock and that heavyweight lock acquisition is fairly slow anyway).
 
-Group locking adds four new members to each PGPROC: lockGroupLeaderIdentifier,
-lockGroupLeader, lockGroupMembers, and lockGroupLink. The first is simply a
-safety mechanism. A newly started parallel worker has to try to join the
-leader's lock group, but it has no guarantee that the group leader is still
-alive by the time it gets started. We try to ensure that the parallel leader
-dies after all workers in normal cases, but also that the system could survive
-relatively intact if that somehow fails to happen. This is one of the
-precautions against such a scenario: the leader relays its PGPROC and also its
-PID to the worker, and the worker fails to join the lock group unless the
-given PGPROC still has the same PID. We assume that PIDs are not recycled
-quickly enough for this interlock to fail.
-
-A PGPROC's lockGroupLeader is NULL for processes not involved in parallel
-query. When a process wants to cooperate with parallel workers, it becomes a
-lock group leader, which means setting this field to point to its own
-PGPROC. When a parallel worker starts up, it points this field at the leader,
-with the above-mentioned interlock. The lockGroupMembers field is only used in
+Group locking adds three new members to each PGPROC: lockGroupLeader,
+lockGroupMembers, and lockGroupLink. A PGPROC's lockGroupLeader is NULL for
+processes not involved in parallel query. When a process wants to cooperate
+with parallel workers, it becomes a lock group leader, which means setting
+this field to point to its own PGPROC. When a parallel worker starts up, it
+points this field at the leader. The lockGroupMembers field is only used in
 the leader; it is a list of the member PGPROCs of the lock group (the leader
 and all workers). The lockGroupLink field is the list link for this list.
 
-All four of these fields are considered to be protected by a lock manager
+All three of these fields are considered to be protected by a lock manager
 partition lock.  The partition lock that protects these fields within a given
 lock group is chosen by taking the leader's pgprocno modulo the number of lock
 manager partitions.  This unusual arrangement has a major advantage: the
@@ -679,6 +668,18 @@ change while the deadlock detector is running, because it knows that it holds
 all the lock manager locks.  Also, holding this single lock allows safe
 manipulation of the lockGroupMembers list for the lock group.
 
+We need an additional interlock when setting these fields, because a newly
+started parallel worker has to try to join the leader's lock group, but it
+has no guarantee that the group leader is still alive by the time it gets
+started.  We try to ensure that the parallel leader dies after all workers
+in normal cases, but also that the system could survive relatively intact
+if that somehow fails to happen.  This is one of the precautions against
+such a scenario: the leader relays its PGPROC and also its PID to the
+worker, and the worker fails to join the lock group unless the given PGPROC
+still has the same PID and is still a lock group leader.  We assume that
+PIDs are not recycled quickly enough for this interlock to fail.
+
+
 User Locks (Advisory Locks)
 ---------------------------
 

diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 114fba0d384aa556e8a0de43e97da706e79cf035..6453b88a5b8867121c12be43f1da8d877bbd59ff 100644 (file)
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -401,7 +401,6 @@ InitProcess(void)
        pg_atomic_init_u32(&MyProc->procArrayGroupNext, INVALID_PGPROCNO);
 
        /* Check that group locking fields are in a proper initial state. */
-       Assert(MyProc->lockGroupLeaderIdentifier == 0);
        Assert(MyProc->lockGroupLeader == NULL);
        Assert(dlist_is_empty(&MyProc->lockGroupMembers));
 
@@ -565,7 +564,6 @@ InitAuxiliaryProcess(void)
        SwitchToSharedLatch();
 
        /* Check that group locking fields are in a proper initial state. */
-       Assert(MyProc->lockGroupLeaderIdentifier == 0);
        Assert(MyProc->lockGroupLeader == NULL);
        Assert(dlist_is_empty(&MyProc->lockGroupMembers));
 
@@ -822,7 +820,6 @@ ProcKill(int code, Datum arg)
                dlist_delete(&MyProc->lockGroupLink);
                if (dlist_is_empty(&leader->lockGroupMembers))
                {
-                       leader->lockGroupLeaderIdentifier = 0;
                        leader->lockGroupLeader = NULL;
                        if (leader != MyProc)
                        {
@@ -1771,7 +1768,6 @@ BecomeLockGroupLeader(void)
        leader_lwlock = LockHashPartitionLockByProc(MyProc);
        LWLockAcquire(leader_lwlock, LW_EXCLUSIVE);
        MyProc->lockGroupLeader = MyProc;
-       MyProc->lockGroupLeaderIdentifier = MyProcPid;
        dlist_push_head(&MyProc->lockGroupMembers, &MyProc->lockGroupLink);
        LWLockRelease(leader_lwlock);
 }
@@ -1795,6 +1791,9 @@ BecomeLockGroupMember(PGPROC *leader, int pid)
        /* Group leader can't become member of group */
        Assert(MyProc != leader);
 
+       /* Can't already be a member of a group */
+       Assert(MyProc->lockGroupLeader == NULL);
+
        /* PID must be valid. */
        Assert(pid != 0);
 
@@ -1808,9 +1807,10 @@ BecomeLockGroupMember(PGPROC *leader, int pid)
        leader_lwlock = LockHashPartitionLockByProc(leader);
        LWLockAcquire(leader_lwlock, LW_EXCLUSIVE);
 
-       /* Try to join the group */
-       if (leader->lockGroupLeaderIdentifier == pid)
+       /* Is this the leader we're looking for? */
+       if (leader->pid == pid && leader->lockGroupLeader == leader)
        {
+               /* OK, join the group */
                ok = true;
                MyProc->lockGroupLeader = leader;
                dlist_push_tail(&leader->lockGroupMembers, &MyProc->lockGroupLink);

diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index a9405ce13be4e593565002d39a6cbecb40bf3158..dbcdd3f340ea187509cef8a583c01f8cf73c1c15 100644 (file)
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -166,7 +166,6 @@ struct PGPROC
         * Support for lock groups.  Use LockHashPartitionLockByProc on the group
         * leader to get the LWLock protecting these fields.
         */
-       int                     lockGroupLeaderIdentifier;      /* MyProcPid, if I'm a leader */
        PGPROC     *lockGroupLeader;    /* lock group leader, if I'm a member */
        dlist_head      lockGroupMembers;       /* list of members, if I'm a leader */
        dlist_node  lockGroupLink;              /* my member link, if I'm a member */