From: Tom Lane Date: Mon, 22 Feb 2016 16:20:35 +0000 (-0500) Subject: Remove redundant PGPROC.lockGroupLeaderIdentifier field. X-Git-Tag: REL9_6_BETA1~658 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=73bf8715aa7430bd003516bde448507fbe789c05;p=postgresql 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 c399618bc7..56b0a12a3e 100644 --- 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 114fba0d38..6453b88a5b 100644 --- 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 a9405ce13b..dbcdd3f340 100644 --- 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 */