(2014/01/11 3:11), Robert Haas wrote:
On Mon, Jan 6, 2014 at 5:50 PM, Robert Haas <robertmh...@gmail.com> wrote:
This is only part of the solution, of course: a complete solution will
involve making the hash table key something other than the lock ID.
What I'm thinking we can do is making the lock ID consist of two
unsigned 32-bit integers.  One of these will be stored in the lwlock
itself, which if my calculations are correct won't increase the size
of LWLockPadded on any common platforms (a 64-bit integer would).
Let's call this the "tranch id".  The other will be derived from the
LWLock address.  Let's call this the "instance ID".  We'll keep a
table of tranch IDs, which will be assigned consecutively starting
with 0.  We'll keep an array of metadata for tranches, indexed by
tranch ID, and each entry will have three associated pieces of
information: an array base, a stride length, and a printable name.
When we need to identify an lwlock in the log or to dtrace, we'll
fetch the tranch ID from the lwlock itself and use that to index into
the tranch metadata array.  We'll then take the address of the lwlock,
subtract the array base address for the tranch, and divide by the
stride length; the result is the instance ID.  When reporting the
user, we can report either the tranch ID directly or the associated
name for that tranch; in either case, we'll also report the instance
ID.

So initially we'll probably just have tranch 0: the main LWLock array.
  If we move buffer content and I/O locks to the buffer headers, we'll
define tranch 1 and tranch 2 with the same base address: the start of
the buffer descriptor array, and the same stride length, the size of a
buffer descriptor.  One will have the associated name "buffer content
lock" and the other "buffer I/O lock".  If we want, we can define
split the main LWLock array into several tranches so that we can more
easily identify lock manager locks, predicate lock manager locks, and
buffer mapping locks.

OK, I've implemented this: here's what I believe to be a complete
patch, based on the previous lwlock-pointers.patch but now handling
LOCK_DEBUG and TRACE_LWLOCKS and dtrace and a bunch of other loose
ends.  I think this should be adequate for allowing lwlocks to be
stored elsewhere in the main shared memory segment as well as in
dynamic shared memory segments.

Thoughts?

I briefly checked the patch. Most of lines are mechanical replacement
from LWLockId to LWLock *, and compiler didn't claim anything with
-Wall -Werror option.

My concern is around LWLockTranche mechanism. Isn't it too complicated
towards the purpose?
For example, it may make sense to add "char lockname[NAMEDATALEN];" at
the tail of LWLock structure if LOCK_DEBUG is enabled. Probably, it
also adds an argument of LWLockAssign() to gives the human readable
name. Is the additional 64bytes (= NAMEDATALEN) per lock too large
for recent hardware?

Below is minor comments.

It seems to me this newer form increased direct reference to
the MainLWLockArray. Is it really graceful?
My recommendation is to define a macro that wraps the reference to
MainLWLockArray.

For example:
  #define PartitionLock(i) \
        (&MainLWLockArray[LOCK_MANAGER_LWLOCK_OFFSET + (i)].lock)

instead of the following manner.

@@ -3400,7 +3406,12 @@ GetLockStatusData(void)
     * Must grab LWLocks in partition-number order to avoid LWLock deadlock.
     */
    for (i = 0; i < NUM_LOCK_PARTITIONS; i++)
-       LWLockAcquire(FirstLockMgrLock + i, LW_SHARED);
+   {
+       LWLock *partitionLock;
+
+       partitionLock = &MainLWLockArray[LOCK_MANAGER_LWLOCK_OFFSET + i].lock;
+       LWLockAcquire(partitionLock, LW_SHARED);
+   }


This chunk is enclosed by #ifdef EXEC_BACKEND, the legacy LWLockArray
was not removed.

@@ -5633,6 +5630,7 @@ save_backend_variables(BackendParameters *param, Port 
*port,
    param->SpinlockSemaArray = SpinlockSemaArray;
 #endif
    param->LWLockArray = LWLockArray;
+   param->MainLWLockArray = MainLWLockArray;
    param->ProcStructLock = ProcStructLock;
    param->ProcGlobal = ProcGlobal;
    param->AuxiliaryProcs = AuxiliaryProcs;

Thanks,
--
OSS Promotion Center / The PG-Strom Project
KaiGai Kohei <kai...@ak.jp.nec.com>


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to