Re: Our naming of wait events is a disaster.

Поиск
Список
Период
Сортировка
От Tom Lane
Тема Re: Our naming of wait events is a disaster.
Дата
Msg-id 9056.1589419765@sss.pgh.pa.us
обсуждение исходный текст
Ответ на Re: Our naming of wait events is a disaster.  (Tom Lane <tgl@sss.pgh.pa.us>)
Список pgsql-hackers
The attached patch doesn't actually change any LWLock names, but it
is a useful start on that project.  What it does is to get rid of the
current scheme of dynamically registering the names of built-in
LWLocks in favor of having a constant array of those names.  It's
completely foolish to expend process-startup cycles on constructing
an array of constant data; moreover, the way things are done now
results in the tranche names being defined all over creation.  I draw
a short straight line between that technique and the lack of consistency
in the tranche names.  Given that we have an enum in lwlock.h enumerating
the built-in tranches, there's certainly no expectation that somebody's
going to create a new one without letting the lwlock module know about
it, so this gives up no flexibility.  In fact, it adds some, because
we can now name an SLRU's buffer-locks tranche whatever we want ---
it's not hard-wired as being the same as the SLRU's base name.

The dynamic registration mechanism is still there, but it's now
*only* used if you load an extension that creates dynamic LWLocks.

At some point it might be interesting to generate the enum
BuiltinTrancheIds and the BuiltinTrancheNames array from a common
source file, as we do for lwlocknames.h/.c.  I didn't feel a need
to make that happen today, though.

            regards, tom lane

diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 3ba9fc9..3572b01 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -235,9 +235,6 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
     else
         Assert(found);

-    /* Register SLRU tranche in the main tranches array */
-    LWLockRegisterTranche(tranche_id, name);
-
     /*
      * Initialize the unshared control struct, including directory path. We
      * assume caller set PagePrecedes.
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index a53e6d9..4284659 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -5116,10 +5116,8 @@ XLOGShmemInit(void)
         /* both should be present or neither */
         Assert(foundCFile && foundXLog);

-        /* Initialize local copy of WALInsertLocks and register the tranche */
+        /* Initialize local copy of WALInsertLocks */
         WALInsertLocks = XLogCtl->Insert.WALInsertLocks;
-        LWLockRegisterTranche(LWTRANCHE_WAL_INSERT,
-                              "wal_insert");

         if (localControlFile)
             pfree(localControlFile);
@@ -5155,7 +5153,6 @@ XLOGShmemInit(void)
         (WALInsertLockPadded *) allocptr;
     allocptr += sizeof(WALInsertLockPadded) * NUM_XLOGINSERT_LOCKS;

-    LWLockRegisterTranche(LWTRANCHE_WAL_INSERT, "wal_insert");
     for (i = 0; i < NUM_XLOGINSERT_LOCKS; i++)
     {
         LWLockInitialize(&WALInsertLocks[i].l.lock, LWTRANCHE_WAL_INSERT);
diff --git a/src/backend/replication/logical/origin.c b/src/backend/replication/logical/origin.c
index d0d2b46..923ea3f 100644
--- a/src/backend/replication/logical/origin.c
+++ b/src/backend/replication/logical/origin.c
@@ -517,9 +517,6 @@ ReplicationOriginShmemInit(void)
             ConditionVariableInit(&replication_states[i].origin_cv);
         }
     }
-
-    LWLockRegisterTranche(replication_states_ctl->tranche_id,
-                          "replication_origin");
 }

 /* ---------------------------------------------------------------------------
diff --git a/src/backend/replication/slot.c b/src/backend/replication/slot.c
index abae74c..d3d1033 100644
--- a/src/backend/replication/slot.c
+++ b/src/backend/replication/slot.c
@@ -140,9 +140,6 @@ ReplicationSlotsShmemInit(void)
         ShmemInitStruct("ReplicationSlot Ctl", ReplicationSlotsShmemSize(),
                         &found);

-    LWLockRegisterTranche(LWTRANCHE_REPLICATION_SLOT_IO_IN_PROGRESS,
-                          "replication_slot_io");
-
     if (!found)
     {
         int            i;
diff --git a/src/backend/storage/buffer/buf_init.c b/src/backend/storage/buffer/buf_init.c
index af62d48..8954856 100644
--- a/src/backend/storage/buffer/buf_init.c
+++ b/src/backend/storage/buffer/buf_init.c
@@ -87,9 +87,6 @@ InitBufferPool(void)
                         NBuffers * (Size) sizeof(LWLockMinimallyPadded),
                         &foundIOLocks);

-    LWLockRegisterTranche(LWTRANCHE_BUFFER_IO_IN_PROGRESS, "buffer_io");
-    LWLockRegisterTranche(LWTRANCHE_BUFFER_CONTENT, "buffer_content");
-
     /*
      * The array used to sort to-be-checkpointed buffer ids is located in
      * shared memory, to avoid having to allocate significant amounts of
diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index 3630006..6a94448 100644
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -267,8 +267,6 @@ CreateSharedProcArray(void)
                             mul_size(sizeof(bool), TOTAL_MAX_CACHED_SUBXIDS),
                             &found);
     }
-
-    LWLockRegisterTranche(LWTRANCHE_PROC, "proc");
 }

 /*
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 4c14e51..a4bc47a 100644
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -108,14 +108,85 @@ extern slock_t *ShmemLock;
 #define LW_SHARED_MASK                ((uint32) ((1 << 24)-1))

 /*
- * This is indexed by tranche ID and stores the names of all tranches known
- * to the current backend.
+ * There are three sorts of LWLock "tranches":
+ *
+ * 1. The individually-named locks defined in lwlocknames.h each have their
+ * own tranche.  The names of these tranches appear in MainLWLockNames[]
+ * in lwlocknames.c.
+ *
+ * 2. There are some predefined tranches for built-in groups of locks.
+ * These are listed in enum BuiltinTrancheIds in lwlock.h, and their names
+ * appear in BuiltinTrancheNames[] below.
+ *
+ * 3. Extensions can create new tranches, via either RequestNamedLWLockTranche
+ * or LWLockRegisterTranche.  The names of these that are known in the current
+ * process appear in LWLockTrancheNames[].
  */
-static const char **LWLockTrancheArray = NULL;
-static int    LWLockTranchesAllocated = 0;

-#define T_NAME(lock) \
-    (LWLockTrancheArray[(lock)->tranche])
+static const char *const BuiltinTrancheNames[] = {
+    /* LWTRANCHE_CLOG_BUFFERS: */
+    "clog",
+    /* LWTRANCHE_COMMITTS_BUFFERS: */
+    "commit_timestamp",
+    /* LWTRANCHE_SUBTRANS_BUFFERS: */
+    "subtrans",
+    /* LWTRANCHE_MXACTOFFSET_BUFFERS: */
+    "multixact_offset",
+    /* LWTRANCHE_MXACTMEMBER_BUFFERS: */
+    "multixact_member",
+    /* LWTRANCHE_ASYNC_BUFFERS: */
+    "async",
+    /* LWTRANCHE_OLDSERXID_BUFFERS: */
+    "oldserxid",
+    /* LWTRANCHE_WAL_INSERT: */
+    "wal_insert",
+    /* LWTRANCHE_BUFFER_CONTENT: */
+    "buffer_content",
+    /* LWTRANCHE_BUFFER_IO_IN_PROGRESS: */
+    "buffer_io",
+    /* LWTRANCHE_REPLICATION_ORIGIN: */
+    "replication_origin",
+    /* LWTRANCHE_REPLICATION_SLOT_IO_IN_PROGRESS: */
+    "replication_slot_io",
+    /* LWTRANCHE_PROC: */
+    "proc",
+    /* LWTRANCHE_BUFFER_MAPPING: */
+    "buffer_mapping",
+    /* LWTRANCHE_LOCK_MANAGER: */
+    "lock_manager",
+    /* LWTRANCHE_PREDICATE_LOCK_MANAGER: */
+    "predicate_lock_manager",
+    /* LWTRANCHE_PARALLEL_HASH_JOIN: */
+    "parallel_hash_join",
+    /* LWTRANCHE_PARALLEL_QUERY_DSA: */
+    "parallel_query_dsa",
+    /* LWTRANCHE_SESSION_DSA: */
+    "session_dsa",
+    /* LWTRANCHE_SESSION_RECORD_TABLE: */
+    "session_record_table",
+    /* LWTRANCHE_SESSION_TYPMOD_TABLE: */
+    "session_typmod_table",
+    /* LWTRANCHE_SHARED_TUPLESTORE: */
+    "shared_tuplestore",
+    /* LWTRANCHE_TBM: */
+    "tbm",
+    /* LWTRANCHE_PARALLEL_APPEND: */
+    "parallel_append",
+    /* LWTRANCHE_SXACT: */
+    "serializable_xact"
+};
+
+StaticAssertDecl(lengthof(BuiltinTrancheNames) ==
+                 LWTRANCHE_FIRST_USER_DEFINED - NUM_INDIVIDUAL_LWLOCKS,
+                 "missing entries in BuiltinTrancheNames[]");
+
+/*
+ * This is indexed by tranche ID minus LWTRANCHE_FIRST_USER_DEFINED, and
+ * stores the names of all dynamically-created tranches known to the current
+ * process.  Any unused entries in the array will contain NULL.
+ */
+static const char **LWLockTrancheNames = NULL;
+static int    LWLockTrancheNamesAllocated = 0;

 /*
  * This points to the main array of LWLocks in shared memory.  Backends inherit
@@ -158,10 +229,13 @@ NamedLWLockTranche *NamedLWLockTrancheArray = NULL;
 static bool lock_named_request_allowed = true;

 static void InitializeLWLocks(void);
-static void RegisterLWLockTranches(void);

 static inline void LWLockReportWaitStart(LWLock *lock);
 static inline void LWLockReportWaitEnd(void);
+static const char *GetLWTrancheName(uint16 trancheId);
+
+#define T_NAME(lock) \
+    GetLWTrancheName((lock)->tranche)

 #ifdef LWLOCK_STATS
 typedef struct lwlock_stats_key
@@ -332,7 +406,7 @@ get_lwlock_stats_entry(LWLock *lock)
  * allocated in the main array.
  */
 static int
-NumLWLocksByNamedTranches(void)
+NumLWLocksForNamedTranches(void)
 {
     int            numLocks = 0;
     int            i;
@@ -353,7 +427,8 @@ LWLockShmemSize(void)
     int            i;
     int            numLocks = NUM_FIXED_LWLOCKS;

-    numLocks += NumLWLocksByNamedTranches();
+    /* Calculate total number of locks needed in the main array. */
+    numLocks += NumLWLocksForNamedTranches();

     /* Space for the LWLock array. */
     size = mul_size(numLocks, sizeof(LWLockPadded));
@@ -368,7 +443,7 @@ LWLockShmemSize(void)
     for (i = 0; i < NamedLWLockTrancheRequests; i++)
         size = add_size(size, strlen(NamedLWLockTrancheRequestArray[i].tranche_name) + 1);

-    /* Disallow named LWLocks' requests after startup */
+    /* Disallow adding any more named tranches. */
     lock_named_request_allowed = false;

     return size;
@@ -376,7 +451,7 @@ LWLockShmemSize(void)

 /*
  * Allocate shmem space for the main LWLock array and all tranches and
- * initialize it.  We also register all the LWLock tranches here.
+ * initialize it.  We also register extension LWLock tranches here.
  */
 void
 CreateLWLocks(void)
@@ -416,8 +491,10 @@ CreateLWLocks(void)
         InitializeLWLocks();
     }

-    /* Register all LWLock tranches */
-    RegisterLWLockTranches();
+    /* Register named extension LWLock tranches in the current process. */
+    for (int i = 0; i < NamedLWLockTrancheRequests; i++)
+        LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId,
+                              NamedLWLockTrancheArray[i].trancheName);
 }

 /*
@@ -426,7 +503,7 @@ CreateLWLocks(void)
 static void
 InitializeLWLocks(void)
 {
-    int            numNamedLocks = NumLWLocksByNamedTranches();
+    int            numNamedLocks = NumLWLocksForNamedTranches();
     int            id;
     int            i;
     int            j;
@@ -452,7 +529,10 @@ InitializeLWLocks(void)
     for (id = 0; id < NUM_PREDICATELOCK_PARTITIONS; id++, lock++)
         LWLockInitialize(&lock->lock, LWTRANCHE_PREDICATE_LOCK_MANAGER);

-    /* Initialize named tranches. */
+    /*
+     * Copy the info about any named tranches into shared memory (so that
+     * other processes can see it), and initialize the requested LWLocks.
+     */
     if (NamedLWLockTrancheRequests > 0)
     {
         char       *trancheNames;
@@ -486,51 +566,6 @@ InitializeLWLocks(void)
 }

 /*
- * Register named tranches and tranches for fixed LWLocks.
- */
-static void
-RegisterLWLockTranches(void)
-{
-    int            i;
-
-    if (LWLockTrancheArray == NULL)
-    {
-        LWLockTranchesAllocated = 128;
-        LWLockTrancheArray = (const char **)
-            MemoryContextAllocZero(TopMemoryContext,
-                                   LWLockTranchesAllocated * sizeof(char *));
-        Assert(LWLockTranchesAllocated >= LWTRANCHE_FIRST_USER_DEFINED);
-    }
-
-    for (i = 0; i < NUM_INDIVIDUAL_LWLOCKS; ++i)
-        LWLockRegisterTranche(i, MainLWLockNames[i]);
-
-    LWLockRegisterTranche(LWTRANCHE_BUFFER_MAPPING, "buffer_mapping");
-    LWLockRegisterTranche(LWTRANCHE_LOCK_MANAGER, "lock_manager");
-    LWLockRegisterTranche(LWTRANCHE_PREDICATE_LOCK_MANAGER,
-                          "predicate_lock_manager");
-    LWLockRegisterTranche(LWTRANCHE_PARALLEL_QUERY_DSA,
-                          "parallel_query_dsa");
-    LWLockRegisterTranche(LWTRANCHE_SESSION_DSA,
-                          "session_dsa");
-    LWLockRegisterTranche(LWTRANCHE_SESSION_RECORD_TABLE,
-                          "session_record_table");
-    LWLockRegisterTranche(LWTRANCHE_SESSION_TYPMOD_TABLE,
-                          "session_typmod_table");
-    LWLockRegisterTranche(LWTRANCHE_SHARED_TUPLESTORE,
-                          "shared_tuplestore");
-    LWLockRegisterTranche(LWTRANCHE_TBM, "tbm");
-    LWLockRegisterTranche(LWTRANCHE_PARALLEL_APPEND, "parallel_append");
-    LWLockRegisterTranche(LWTRANCHE_PARALLEL_HASH_JOIN, "parallel_hash_join");
-    LWLockRegisterTranche(LWTRANCHE_SXACT, "serializable_xact");
-
-    /* Register named tranches. */
-    for (i = 0; i < NamedLWLockTrancheRequests; i++)
-        LWLockRegisterTranche(NamedLWLockTrancheArray[i].trancheId,
-                              NamedLWLockTrancheArray[i].trancheName);
-}
-
-/*
  * InitLWLockAccess - initialize backend-local state needed to hold LWLocks
  */
 void
@@ -595,32 +630,47 @@ LWLockNewTrancheId(void)
 }

 /*
- * Register a tranche ID in the lookup table for the current process.  This
- * routine will save a pointer to the tranche name passed as an argument,
+ * Register a dynamic tranche name in the lookup table of the current process.
+ *
+ * This routine will save a pointer to the tranche name passed as an argument,
  * so the name should be allocated in a backend-lifetime context
- * (TopMemoryContext, static variable, or similar).
+ * (TopMemoryContext, static constant, or similar).
  */
 void
 LWLockRegisterTranche(int tranche_id, const char *tranche_name)
 {
-    Assert(LWLockTrancheArray != NULL);
+    /* This should only be called for user-defined tranches. */
+    if (tranche_id < LWTRANCHE_FIRST_USER_DEFINED)
+        return;
+
+    /* Convert to array index. */
+    tranche_id -= LWTRANCHE_FIRST_USER_DEFINED;

-    if (tranche_id >= LWLockTranchesAllocated)
+    /* If necessary, create or enlarge array. */
+    if (tranche_id >= LWLockTrancheNamesAllocated)
     {
-        int            i = LWLockTranchesAllocated;
-        int            j = LWLockTranchesAllocated;
+        int            newalloc;

-        while (i <= tranche_id)
-            i *= 2;
+        newalloc = Max(LWLockTrancheNamesAllocated, 8);
+        while (newalloc <= tranche_id)
+            newalloc *= 2;

-        LWLockTrancheArray = (const char **)
-            repalloc(LWLockTrancheArray, i * sizeof(char *));
-        LWLockTranchesAllocated = i;
-        while (j < LWLockTranchesAllocated)
-            LWLockTrancheArray[j++] = NULL;
+        if (LWLockTrancheNames == NULL)
+            LWLockTrancheNames = (const char **)
+                MemoryContextAllocZero(TopMemoryContext,
+                                       newalloc * sizeof(char *));
+        else
+        {
+            LWLockTrancheNames = (const char **)
+                repalloc(LWLockTrancheNames, newalloc * sizeof(char *));
+            memset(LWLockTrancheNames + LWLockTrancheNamesAllocated,
+                   0,
+                   (newalloc - LWLockTrancheNamesAllocated) * sizeof(char *));
+        }
+        LWLockTrancheNamesAllocated = newalloc;
     }

-    LWLockTrancheArray[tranche_id] = tranche_name;
+    LWLockTrancheNames[tranche_id] = tranche_name;
 }

 /*
@@ -667,7 +717,7 @@ RequestNamedLWLockTranche(const char *tranche_name, int num_lwlocks)

     request = &NamedLWLockTrancheRequestArray[NamedLWLockTrancheRequests];
     Assert(strlen(tranche_name) + 1 < NAMEDATALEN);
-    StrNCpy(request->tranche_name, tranche_name, NAMEDATALEN);
+    strlcpy(request->tranche_name, tranche_name, NAMEDATALEN);
     request->num_lwlocks = num_lwlocks;
     NamedLWLockTrancheRequests++;
 }
@@ -709,23 +759,42 @@ LWLockReportWaitEnd(void)
 }

 /*
- * Return an identifier for an LWLock based on the wait class and event.
+ * Return the name of an LWLock tranche.
  */
-const char *
-GetLWLockIdentifier(uint32 classId, uint16 eventId)
+static const char *
+GetLWTrancheName(uint16 trancheId)
 {
-    Assert(classId == PG_WAIT_LWLOCK);
+    /* Individual LWLock? */
+    if (trancheId < NUM_INDIVIDUAL_LWLOCKS)
+        return MainLWLockNames[trancheId];
+
+    /* Built-in tranche? */
+    if (trancheId < LWTRANCHE_FIRST_USER_DEFINED)
+        return BuiltinTrancheNames[trancheId - NUM_INDIVIDUAL_LWLOCKS];

     /*
-     * It is quite possible that user has registered tranche in one of the
-     * backends (e.g. by allocating lwlocks in dynamic shared memory) but not
-     * all of them, so we can't assume the tranche is registered here.
+     * It's an extension tranche, so look in LWLockTrancheNames[].  However,
+     * it's possible that the tranche has never been registered in the current
+     * process, in which case give up and return "extension".
      */
-    if (eventId >= LWLockTranchesAllocated ||
-        LWLockTrancheArray[eventId] == NULL)
+    trancheId -= LWTRANCHE_FIRST_USER_DEFINED;
+
+    if (trancheId >= LWLockTrancheNamesAllocated ||
+        LWLockTrancheNames[trancheId] == NULL)
         return "extension";

-    return LWLockTrancheArray[eventId];
+    return LWLockTrancheNames[trancheId];
+}
+
+/*
+ * Return an identifier for an LWLock based on the wait class and event.
+ */
+const char *
+GetLWLockIdentifier(uint32 classId, uint16 eventId)
+{
+    Assert(classId == PG_WAIT_LWLOCK);
+    /* The event IDs are just tranche numbers. */
+    return GetLWTrancheName(eventId);
 }

 /*

В списке pgsql-hackers по дате отправления:

Предыдущее
От: Kyotaro Horiguchi
Дата:
Сообщение: Re: MultiXact\SLRU buffers configuration
Следующее
От: godjan •
Дата:
Сообщение: Re: Strange decreasing value of pg_last_wal_receive_lsn()