Re: LWLocks by LockManager slowing large DB - Mailing list pgsql-performance

I wrote:
> Andres Freund <andres@anarazel.de> writes:
>> Cool. And damn: I can't immediately think of a way to optimize this to
>> not require this kind of hack in the future.

> Maybe the same thing we do with user tables, ie not give up the lock
> when we close a toast rel?  As long as the internal lock counters
> are 64-bit, we'd not have to worry about overflowing them.

Like this?  This passes check-world, modulo the one very-unsurprising
regression test change.  I've not tried to do any performance testing.

            regards, tom lane

diff --git a/src/backend/access/common/detoast.c b/src/backend/access/common/detoast.c
index 545a6b8867..3826b4d93e 100644
--- a/src/backend/access/common/detoast.c
+++ b/src/backend/access/common/detoast.c
@@ -367,7 +367,7 @@ toast_fetch_datum(struct varlena *attr)
                                  * case. */

     /*
-     * Open the toast relation and its indexes
+     * Open the toast relation (but its indexes are dealt with by the AM)
      */
     toastrel = table_open(toast_pointer.va_toastrelid, AccessShareLock);

@@ -376,7 +376,7 @@ toast_fetch_datum(struct varlena *attr)
                                      attrsize, 0, attrsize, result);

     /* Close toast table */
-    table_close(toastrel, AccessShareLock);
+    table_close(toastrel, NoLock);

     return result;
 }
@@ -457,7 +457,7 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset,
                                      result);

     /* Close toast table */
-    table_close(toastrel, AccessShareLock);
+    table_close(toastrel, NoLock);

     return result;
 }
diff --git a/src/backend/access/common/toast_internals.c b/src/backend/access/common/toast_internals.c
index 730cd04a2d..f0d0251ce0 100644
--- a/src/backend/access/common/toast_internals.c
+++ b/src/backend/access/common/toast_internals.c
@@ -27,7 +27,7 @@
 #include "utils/rel.h"
 #include "utils/snapmgr.h"

-static bool toastrel_valueid_exists(Relation toastrel, Oid valueid);
+static bool toastrel_valueid_exists(Relation toastrel, Oid valueid, LOCKMODE lock);
 static bool toastid_valueid_exists(Oid toastrelid, Oid valueid);

 /* ----------
@@ -264,7 +264,8 @@ toast_save_datum(Relation rel, Datum value,
                  * be reclaimed by VACUUM.
                  */
                 if (toastrel_valueid_exists(toastrel,
-                                            toast_pointer.va_valueid))
+                                            toast_pointer.va_valueid,
+                                            RowExclusiveLock))
                 {
                     /* Match, so short-circuit the data storage loop below */
                     data_todo = 0;
@@ -359,8 +360,8 @@ toast_save_datum(Relation rel, Datum value,
     /*
      * Done - close toast relation and its indexes
      */
-    toast_close_indexes(toastidxs, num_indexes, RowExclusiveLock);
-    table_close(toastrel, RowExclusiveLock);
+    toast_close_indexes(toastidxs, num_indexes, NoLock);
+    table_close(toastrel, NoLock);

     /*
      * Create the TOAST pointer value that we'll return
@@ -440,8 +441,8 @@ toast_delete_datum(Relation rel, Datum value, bool is_speculative)
      * End scan and close relations
      */
     systable_endscan_ordered(toastscan);
-    toast_close_indexes(toastidxs, num_indexes, RowExclusiveLock);
-    table_close(toastrel, RowExclusiveLock);
+    toast_close_indexes(toastidxs, num_indexes, NoLock);
+    table_close(toastrel, NoLock);
 }

 /* ----------
@@ -453,7 +454,7 @@ toast_delete_datum(Relation rel, Datum value, bool is_speculative)
  * ----------
  */
 static bool
-toastrel_valueid_exists(Relation toastrel, Oid valueid)
+toastrel_valueid_exists(Relation toastrel, Oid valueid, LOCKMODE lock)
 {
     bool        result = false;
     ScanKeyData toastkey;
@@ -464,7 +465,7 @@ toastrel_valueid_exists(Relation toastrel, Oid valueid)

     /* Fetch a valid index relation */
     validIndex = toast_open_indexes(toastrel,
-                                    RowExclusiveLock,
+                                    lock,
                                     &toastidxs,
                                     &num_indexes);

@@ -489,7 +490,7 @@ toastrel_valueid_exists(Relation toastrel, Oid valueid)
     systable_endscan(toastscan);

     /* Clean up */
-    toast_close_indexes(toastidxs, num_indexes, RowExclusiveLock);
+    toast_close_indexes(toastidxs, num_indexes, NoLock);

     return result;
 }
@@ -508,9 +509,9 @@ toastid_valueid_exists(Oid toastrelid, Oid valueid)

     toastrel = table_open(toastrelid, AccessShareLock);

-    result = toastrel_valueid_exists(toastrel, valueid);
+    result = toastrel_valueid_exists(toastrel, valueid, AccessShareLock);

-    table_close(toastrel, AccessShareLock);
+    table_close(toastrel, NoLock);

     return result;
 }
diff --git a/src/backend/access/heap/heaptoast.c b/src/backend/access/heap/heaptoast.c
index 55bbe1d584..c3bfb97c62 100644
--- a/src/backend/access/heap/heaptoast.c
+++ b/src/backend/access/heap/heaptoast.c
@@ -789,5 +789,5 @@ heap_fetch_toast_slice(Relation toastrel, Oid valueid, int32 attrsize,

     /* End scan and close indexes. */
     systable_endscan_ordered(toastscan);
-    toast_close_indexes(toastidxs, num_indexes, AccessShareLock);
+    toast_close_indexes(toastidxs, num_indexes, NoLock);
 }
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index ec14b060a6..b7a007904e 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2610,7 +2610,9 @@ select * from my_locks order by 1;
   relname  |       max_lockmode
 -----------+--------------------------
  alterlock | ShareUpdateExclusiveLock
-(1 row)
+ pg_toast  | AccessShareLock
+ pg_toast  | AccessShareLock
+(3 rows)

 rollback;
 begin; alter table alterlock cluster on alterlock_pkey;