Commit 73bf8715 authored by Tom Lane's avatar Tom Lane

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.
parent ea56b06c
...@@ -650,27 +650,16 @@ those cases so that they no longer use heavyweight locking in the first place ...@@ -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 (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). to deadlock and that heavyweight lock acquisition is fairly slow anyway).
Group locking adds four new members to each PGPROC: lockGroupLeaderIdentifier, Group locking adds three new members to each PGPROC: lockGroupLeader,
lockGroupLeader, lockGroupMembers, and lockGroupLink. The first is simply a lockGroupMembers, and lockGroupLink. A PGPROC's lockGroupLeader is NULL for
safety mechanism. A newly started parallel worker has to try to join the processes not involved in parallel query. When a process wants to cooperate
leader's lock group, but it has no guarantee that the group leader is still with parallel workers, it becomes a lock group leader, which means setting
alive by the time it gets started. We try to ensure that the parallel leader this field to point to its own PGPROC. When a parallel worker starts up, it
dies after all workers in normal cases, but also that the system could survive points this field at the leader. The lockGroupMembers field is only used in
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
the leader; it is a list of the member PGPROCs of the lock group (the leader 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. 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 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 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 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 ...@@ -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 all the lock manager locks. Also, holding this single lock allows safe
manipulation of the lockGroupMembers list for the lock group. 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) User Locks (Advisory Locks)
--------------------------- ---------------------------
......
...@@ -401,7 +401,6 @@ InitProcess(void) ...@@ -401,7 +401,6 @@ InitProcess(void)
pg_atomic_init_u32(&MyProc->procArrayGroupNext, INVALID_PGPROCNO); pg_atomic_init_u32(&MyProc->procArrayGroupNext, INVALID_PGPROCNO);
/* Check that group locking fields are in a proper initial state. */ /* Check that group locking fields are in a proper initial state. */
Assert(MyProc->lockGroupLeaderIdentifier == 0);
Assert(MyProc->lockGroupLeader == NULL); Assert(MyProc->lockGroupLeader == NULL);
Assert(dlist_is_empty(&MyProc->lockGroupMembers)); Assert(dlist_is_empty(&MyProc->lockGroupMembers));
...@@ -565,7 +564,6 @@ InitAuxiliaryProcess(void) ...@@ -565,7 +564,6 @@ InitAuxiliaryProcess(void)
SwitchToSharedLatch(); SwitchToSharedLatch();
/* Check that group locking fields are in a proper initial state. */ /* Check that group locking fields are in a proper initial state. */
Assert(MyProc->lockGroupLeaderIdentifier == 0);
Assert(MyProc->lockGroupLeader == NULL); Assert(MyProc->lockGroupLeader == NULL);
Assert(dlist_is_empty(&MyProc->lockGroupMembers)); Assert(dlist_is_empty(&MyProc->lockGroupMembers));
...@@ -822,7 +820,6 @@ ProcKill(int code, Datum arg) ...@@ -822,7 +820,6 @@ ProcKill(int code, Datum arg)
dlist_delete(&MyProc->lockGroupLink); dlist_delete(&MyProc->lockGroupLink);
if (dlist_is_empty(&leader->lockGroupMembers)) if (dlist_is_empty(&leader->lockGroupMembers))
{ {
leader->lockGroupLeaderIdentifier = 0;
leader->lockGroupLeader = NULL; leader->lockGroupLeader = NULL;
if (leader != MyProc) if (leader != MyProc)
{ {
...@@ -1771,7 +1768,6 @@ BecomeLockGroupLeader(void) ...@@ -1771,7 +1768,6 @@ BecomeLockGroupLeader(void)
leader_lwlock = LockHashPartitionLockByProc(MyProc); leader_lwlock = LockHashPartitionLockByProc(MyProc);
LWLockAcquire(leader_lwlock, LW_EXCLUSIVE); LWLockAcquire(leader_lwlock, LW_EXCLUSIVE);
MyProc->lockGroupLeader = MyProc; MyProc->lockGroupLeader = MyProc;
MyProc->lockGroupLeaderIdentifier = MyProcPid;
dlist_push_head(&MyProc->lockGroupMembers, &MyProc->lockGroupLink); dlist_push_head(&MyProc->lockGroupMembers, &MyProc->lockGroupLink);
LWLockRelease(leader_lwlock); LWLockRelease(leader_lwlock);
} }
...@@ -1795,6 +1791,9 @@ BecomeLockGroupMember(PGPROC *leader, int pid) ...@@ -1795,6 +1791,9 @@ BecomeLockGroupMember(PGPROC *leader, int pid)
/* Group leader can't become member of group */ /* Group leader can't become member of group */
Assert(MyProc != leader); Assert(MyProc != leader);
/* Can't already be a member of a group */
Assert(MyProc->lockGroupLeader == NULL);
/* PID must be valid. */ /* PID must be valid. */
Assert(pid != 0); Assert(pid != 0);
...@@ -1808,9 +1807,10 @@ BecomeLockGroupMember(PGPROC *leader, int pid) ...@@ -1808,9 +1807,10 @@ BecomeLockGroupMember(PGPROC *leader, int pid)
leader_lwlock = LockHashPartitionLockByProc(leader); leader_lwlock = LockHashPartitionLockByProc(leader);
LWLockAcquire(leader_lwlock, LW_EXCLUSIVE); LWLockAcquire(leader_lwlock, LW_EXCLUSIVE);
/* Try to join the group */ /* Is this the leader we're looking for? */
if (leader->lockGroupLeaderIdentifier == pid) if (leader->pid == pid && leader->lockGroupLeader == leader)
{ {
/* OK, join the group */
ok = true; ok = true;
MyProc->lockGroupLeader = leader; MyProc->lockGroupLeader = leader;
dlist_push_tail(&leader->lockGroupMembers, &MyProc->lockGroupLink); dlist_push_tail(&leader->lockGroupMembers, &MyProc->lockGroupLink);
......
...@@ -166,7 +166,6 @@ struct PGPROC ...@@ -166,7 +166,6 @@ struct PGPROC
* Support for lock groups. Use LockHashPartitionLockByProc on the group * Support for lock groups. Use LockHashPartitionLockByProc on the group
* leader to get the LWLock protecting these fields. * 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 */ PGPROC *lockGroupLeader; /* lock group leader, if I'm a member */
dlist_head lockGroupMembers; /* list of members, if I'm a leader */ dlist_head lockGroupMembers; /* list of members, if I'm a leader */
dlist_node lockGroupLink; /* my member link, if I'm a member */ dlist_node lockGroupLink; /* my member link, if I'm a member */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment