]> granicus.if.org Git - postgresql/commitdiff
Second try at fixing O(N^2) problem in foreign key references.
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 25 Sep 2015 17:16:30 +0000 (13:16 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 25 Sep 2015 17:16:30 +0000 (13:16 -0400)
This replaces ill-fated commit 5ddc72887a012f6a8b85707ef27d85c274faf53d,
which was reverted because it broke active uses of FK cache entries.  In
this patch, we still do nothing more to invalidatable cache entries than
mark them as needing revalidation, so we won't break active uses.  To keep
down the overhead of InvalidateConstraintCacheCallBack(), keep a list of
just the currently-valid cache entries.  (The entries are large enough that
some added space for list links doesn't seem like a big problem.)  This
would still be O(N^2) when there are many valid entries, though, so when
the list gets too long, just force the "sinval reset" behavior to remove
everything from the list.  I set the threshold at 1000 entries, somewhat
arbitrarily.  Possibly that could be fine-tuned later.  Another item for
future study is whether it's worth adding reference counting so that we
could safely remove invalidated entries.  As-is, problem cases are likely
to end up with large and mostly invalid FK caches.

Like the previous attempt, backpatch to 9.3.

Jan Wieck and Tom Lane

src/backend/utils/adt/ri_triggers.c

index fc1c77f072f2f6f4d0222da36f8bab3c4b23ac88..018cb99e8cdef488f8a903eb3f4dadf1bc7d8987 100644 (file)
@@ -40,6 +40,7 @@
 #include "commands/trigger.h"
 #include "executor/executor.h"
 #include "executor/spi.h"
+#include "lib/ilist.h"
 #include "parser/parse_coerce.h"
 #include "parser/parse_relation.h"
 #include "miscadmin.h"
@@ -125,6 +126,7 @@ typedef struct RI_ConstraintInfo
                                                                                                 * PK) */
        Oid                     ff_eq_oprs[RI_MAX_NUMKEYS];             /* equality operators (FK =
                                                                                                 * FK) */
+       dlist_node      valid_link;             /* Link in list of valid entries */
 } RI_ConstraintInfo;
 
 
@@ -185,6 +187,8 @@ typedef struct RI_CompareHashEntry
 static HTAB *ri_constraint_cache = NULL;
 static HTAB *ri_query_cache = NULL;
 static HTAB *ri_compare_cache = NULL;
+static dlist_head ri_constraint_cache_valid_list;
+static int     ri_constraint_cache_valid_count = 0;
 
 
 /* ----------
@@ -2924,6 +2928,13 @@ ri_LoadConstraintInfo(Oid constraintOid)
 
        ReleaseSysCache(tup);
 
+       /*
+        * For efficient processing of invalidation messages below, we keep a
+        * doubly-linked list, and a count, of all currently valid entries.
+        */
+       dlist_push_tail(&ri_constraint_cache_valid_list, &riinfo->valid_link);
+       ri_constraint_cache_valid_count++;
+
        riinfo->valid = true;
 
        return riinfo;
@@ -2936,21 +2947,41 @@ ri_LoadConstraintInfo(Oid constraintOid)
  * gets enough update traffic that it's probably worth being smarter.
  * Invalidate any ri_constraint_cache entry associated with the syscache
  * entry with the specified hash value, or all entries if hashvalue == 0.
+ *
+ * Note: at the time a cache invalidation message is processed there may be
+ * active references to the cache.  Because of this we never remove entries
+ * from the cache, but only mark them invalid, which is harmless to active
+ * uses.  (Any query using an entry should hold a lock sufficient to keep that
+ * data from changing under it --- but we may get cache flushes anyway.)
  */
 static void
 InvalidateConstraintCacheCallBack(Datum arg, int cacheid, uint32 hashvalue)
 {
-       HASH_SEQ_STATUS status;
-       RI_ConstraintInfo *hentry;
+       dlist_mutable_iter iter;
 
        Assert(ri_constraint_cache != NULL);
 
-       hash_seq_init(&status, ri_constraint_cache);
-       while ((hentry = (RI_ConstraintInfo *) hash_seq_search(&status)) != NULL)
+       /*
+        * If the list of currently valid entries gets excessively large, we mark
+        * them all invalid so we can empty the list.  This arrangement avoids
+        * O(N^2) behavior in situations where a session touches many foreign keys
+        * and also does many ALTER TABLEs, such as a restore from pg_dump.
+        */
+       if (ri_constraint_cache_valid_count > 1000)
+               hashvalue = 0;                  /* pretend it's a cache reset */
+
+       dlist_foreach_modify(iter, &ri_constraint_cache_valid_list)
        {
-               if (hentry->valid &&
-                       (hashvalue == 0 || hentry->oidHashValue == hashvalue))
-                       hentry->valid = false;
+               RI_ConstraintInfo *riinfo = dlist_container(RI_ConstraintInfo,
+                                                                                                       valid_link, iter.cur);
+
+               if (hashvalue == 0 || riinfo->oidHashValue == hashvalue)
+               {
+                       riinfo->valid = false;
+                       /* Remove invalidated entries from the list, too */
+                       dlist_delete(iter.cur);
+                       ri_constraint_cache_valid_count--;
+               }
        }
 }