Move I/O before the index_update_stats() buffer lock region.

Commit a07e03fd8fa7daf4d1356f7cb501ffe784ea6257 enlarged the work done
here under the pg_class heap buffer lock.  Two preexisting actions are
best done before holding that lock.  Both RelationGetNumberOfBlocks()
and visibilitymap_count() do I/O, and the latter might exclusive-lock a
visibility map buffer.  Moving these reduces contention and risk of
undetected LWLock deadlock.  Back-patch to v12, like that commit.

Discussion: https://postgr.es/m/20241031200139.b4@rfd.leadboat.com

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index 547769d227f4d6e03fd5b2eb571ec631af503c47..24d6356c3d1663d49e7d9c77faa22022520e49ee 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -2787,6 +2787,9 @@ index_update_stats(Relation rel,
                   bool hasindex,
                   double reltuples)
 {
+   bool        update_stats;
+   BlockNumber relpages;
+   BlockNumber relallvisible;
    Oid         relid = RelationGetRelid(rel);
    Relation    pg_class;
    ScanKeyData key[1];
@@ -2795,6 +2798,38 @@ index_update_stats(Relation rel,
    Form_pg_class rd_rel;
    bool        dirty;
 
+   /*
+    * As a special hack, if we are dealing with an empty table and the
+    * existing reltuples is -1, we leave that alone.  This ensures that
+    * creating an index as part of CREATE TABLE doesn't cause the table to
+    * prematurely look like it's been vacuumed.  The rd_rel we modify may
+    * differ from rel->rd_rel due to e.g. commit of concurrent GRANT, but the
+    * commands that change reltuples take locks conflicting with ours.  (Even
+    * if a command changed reltuples under a weaker lock, this affects only
+    * statistics for an empty table.)
+    */
+   if (reltuples == 0 && rel->rd_rel->reltuples < 0)
+       reltuples = -1;
+
+   update_stats = reltuples >= 0;
+
+   /*
+    * Finish I/O and visibility map buffer locks before
+    * systable_inplace_update_begin() locks the pg_class buffer.  The rd_rel
+    * we modify may differ from rel->rd_rel due to e.g. commit of concurrent
+    * GRANT, but no command changes a relkind from non-index to index.  (Even
+    * if one did, relallvisible doesn't break functionality.)
+    */
+   if (update_stats)
+   {
+       relpages = RelationGetNumberOfBlocks(rel);
+
+       if (rel->rd_rel->relkind != RELKIND_INDEX)
+           visibilitymap_count(rel, &relallvisible, NULL);
+       else                    /* don't bother for indexes */
+           relallvisible = 0;
+   }
+
    /*
     * We always update the pg_class row using a non-transactional,
     * overwrite-in-place update.  There are several reasons for this:
@@ -2848,16 +2883,8 @@ index_update_stats(Relation rel,
        dirty = true;
    }
 
-   if (reltuples >= 0)
+   if (update_stats)
    {
-       BlockNumber relpages = RelationGetNumberOfBlocks(rel);
-       BlockNumber relallvisible;
-
-       if (rd_rel->relkind != RELKIND_INDEX)
-           visibilitymap_count(rel, &relallvisible, NULL);
-       else                    /* don't bother for indexes */
-           relallvisible = 0;
-
        if (rd_rel->relpages != (int32) relpages)
        {
            rd_rel->relpages = (int32) relpages;