make finish_delayed_invalidation() more lazy

Author: funbringer

expected/pathman_calamity.out

@@ -867,7 +867,7 @@ SELECT context, entries FROM pathman_cache_stats ORDER BY context;	/* OK */
  maintenance              |       0
  partition bounds cache   |       0
  partition dispatch cache |       1
- partition parents cache  |      10
+ partition parents cache  |       0
 (4 rows)
 
 SELECT drop_partitions('calamity.test_pathman_cache_stats');
@@ -910,9 +910,9 @@ SELECT context, entries FROM pathman_cache_stats ORDER BY context;	/* OK */
          context          | entries 
 --------------------------+---------
  maintenance              |       0
- partition bounds cache   |      10
+ partition bounds cache   |       0
  partition dispatch cache |       1
- partition parents cache  |      10
+ partition parents cache  |       0
 (4 rows)
 
 SELECT drop_partitions('calamity.test_pathman_cache_stats');

src/hooks.c

@@ -746,7 +746,7 @@ pathman_relcache_hook(Datum arg, Oid relid)
 
 				/* Which means that 'relid' might be parent */
 				if (relid != InvalidOid)
-					delay_invalidation_parent_rel(relid);
+					delay_invalidation_vague_rel(relid);
 #ifdef NOT_USED
 				elog(DEBUG2, "Invalidation message for relation %u [%u]",
 					 relid, MyProcPid);

src/include/init.h

@@ -225,6 +225,8 @@ bool read_pathman_params(Oid relid,
 						 Datum *values,
 						 bool *isnull);
 
+Oid *read_parent_oids(int *nelems);
+
 
 bool validate_range_constraint(const Expr *expr,
 							   const PartRelationInfo *prel,

src/include/utils.h

@@ -31,6 +31,7 @@ bool match_expr_to_operand(Node *expr, Node *operand);
  */
 Oid get_pathman_schema(void);
 List * list_reverse(List *l);
+int oid_cmp(const void *p1, const void *p2);
 
 /*
  * Useful functions for relations.

src/init.c

@@ -70,7 +70,15 @@ static bool init_pathman_relation_oids(void);
 static void fini_pathman_relation_oids(void);
 static void init_local_cache(void);
 static void fini_local_cache(void);
-static void read_pathman_config(void);
+
+/* Special handlers for read_pathman_config() */
+static void add_partrel_to_array(Datum *values, bool *isnull, void *context);
+static void startup_invalidate_parent(Datum *values, bool *isnull, void *context);
+
+static void read_pathman_config(void (*per_row_cb)(Datum *values,
+												   bool *isnull,
+												   void *context),
+								void *context);
 
 static bool validate_range_opexpr(const Expr *expr,
 								  const PartRelationInfo *prel,
@@ -82,8 +90,6 @@ static bool read_opexpr_const(const OpExpr *opexpr,
 							  const PartRelationInfo *prel,
 							  Datum *value);
 
-static int oid_cmp(const void *p1, const void *p2);
-
 
 /* Validate SQL facade */
 static uint32 build_sql_facade_version(char *version_cstr);
@@ -202,8 +208,11 @@ load_config(void)
 	/* Validate pg_pathman's Pl/PgSQL facade (might be outdated) */
 	validate_sql_facade_version(get_sql_facade_version());
 
-	init_local_cache();		/* create various hash tables (caches) */
-	read_pathman_config();	/* read PATHMAN_CONFIG table & fill cache */
+	/* Create various hash tables (caches) */
+	init_local_cache();
+
+	/* Read PATHMAN_CONFIG table & fill cache */
+	read_pathman_config(startup_invalidate_parent, NULL);
 
 	/* Register pathman_relcache_hook(), currently we can't unregister it */
 	if (relcache_callback_needed)
@@ -781,11 +790,83 @@ read_pathman_params(Oid relid, Datum *values, bool *isnull)
 }
 
 
+typedef struct
+{
+	Oid	   *array;
+	int		nelems;
+	int		capacity;
+} read_parent_oids_cxt;
+
+/*
+ * Get a sorted array of partitioned tables' Oids.
+ */
+Oid *
+read_parent_oids(int *nelems)
+{
+	read_parent_oids_cxt context = { NULL, 0, 0 };
+
+	read_pathman_config(add_partrel_to_array, &context);
+
+	/* Perform sorting */
+	qsort(context.array, context.nelems, sizeof(Oid), oid_cmp);
+
+	/* Return values */
+	*nelems = context.nelems;
+	return context.array;
+}
+
+
+/* read_pathman_config(): add parent to array of Oids */
+static void
+add_partrel_to_array(Datum *values, bool *isnull, void *context)
+{
+	Oid relid = DatumGetObjectId(values[Anum_pathman_config_partrel - 1]);
+	read_parent_oids_cxt *result = (read_parent_oids_cxt *) context;
+
+	if (result->array == NULL)
+	{
+		result->capacity = PART_RELS_SIZE;
+		result->array = palloc(result->capacity * sizeof(Oid));
+	}
+
+	if (result->nelems >= result->capacity)
+	{
+		result->capacity = result->capacity * 2 + 1;
+		result->array = repalloc(result->array, result->capacity * sizeof(Oid));
+	}
+
+	/* Append current relid */
+	result->array[result->nelems++] = relid;
+}
+
+/* read_pathman_config(): create dummy cache entry for parent */
+static void
+startup_invalidate_parent(Datum *values, bool *isnull, void *context)
+{
+	Oid relid = DatumGetObjectId(values[Anum_pathman_config_partrel - 1]);
+
+	/* Check that relation 'relid' exists */
+	if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(relid)))
+	{
+		DisablePathman(); /* disable pg_pathman since config is broken */
+		ereport(ERROR,
+				(errmsg("table \"%s\" contains nonexistent relation %u",
+						PATHMAN_CONFIG, relid),
+				 errhint(INIT_ERROR_HINT)));
+	}
+
+	/* get_pathman_relation_info() will refresh this entry */
+	invalidate_pathman_relation_info(relid, NULL);
+}
+
 /*
  * Go through the PATHMAN_CONFIG table and create PartRelationInfo entries.
  */
 static void
-read_pathman_config(void)
+read_pathman_config(void (*per_row_cb)(Datum *values,
+									   bool *isnull,
+									   void *context),
+					void *context)
 {
 	Relation		rel;
 	HeapScanDesc	scan;
@@ -811,7 +892,6 @@ read_pathman_config(void)
 	{
 		Datum		values[Natts_pathman_config];
 		bool		isnull[Natts_pathman_config];
-		Oid			relid; /* partitioned table */
 
 		/* Extract Datums from tuple 'htup' */
 		heap_deform_tuple(htup, RelationGetDescr(rel), values, isnull);
@@ -821,21 +901,8 @@ read_pathman_config(void)
 		Assert(!isnull[Anum_pathman_config_parttype - 1]);
 		Assert(!isnull[Anum_pathman_config_expr - 1]);
 
-		/* Extract values from Datums */
-		relid = DatumGetObjectId(values[Anum_pathman_config_partrel - 1]);
-
-		/* Check that relation 'relid' exists */
-		if (!SearchSysCacheExists1(RELOID, ObjectIdGetDatum(relid)))
-		{
-			DisablePathman(); /* disable pg_pathman since config is broken */
-			ereport(ERROR,
-					(errmsg("table \"%s\" contains nonexistent relation %u",
-							PATHMAN_CONFIG, relid),
-					 errhint(INIT_ERROR_HINT)));
-		}
-
-		/* get_pathman_relation_info() will refresh this entry */
-		invalidate_pathman_relation_info(relid, NULL);
+		/* Execute per row callback */
+		per_row_cb(values, isnull, context);
 	}
 
 	/* Clean resources */
@@ -1127,20 +1194,6 @@ validate_hash_constraint(const Expr *expr,
 	return false;
 }
 
-/* needed for find_inheritance_children_array() function */
-static int
-oid_cmp(const void *p1, const void *p2)
-{
-	Oid			v1 = *((const Oid *) p1);
-	Oid			v2 = *((const Oid *) p2);
-
-	if (v1 < v2)
-		return -1;
-	if (v1 > v2)
-		return 1;
-	return 0;
-}
-
 
 /* Parse cstring and build uint32 representing the version */
 static uint32

src/relation_info.c

@@ -88,8 +88,12 @@ static bool		delayed_shutdown = false; /* pathman was dropped */
 		list = NIL; \
 	} while (0)
 
+/* Handy wrappers for Oids */
+#define bsearch_oid(key, array, array_size) \
+	bsearch((const void *) &(key), (array), (array_size), sizeof(Oid), oid_cmp)
 
-static bool try_perform_parent_refresh(Oid parent);
+
+static bool try_invalidate_parent(Oid relid, Oid *parents, int parents_count);
 static Oid try_syscache_parent_search(Oid partition, PartParentSearch *status);
 static Oid get_parent_of_partition_internal(Oid partition,
 											PartParentSearch *status,
@@ -857,6 +861,9 @@ finish_delayed_invalidation(void)
 	/* Check that current state is transactional */
 	if (IsTransactionState())
 	{
+		Oid		   *parents = NULL;
+		int			parents_count;
+		bool		parents_fetched = false;
 		ListCell   *lc;
 
 		/* Handle the probable 'DROP EXTENSION' case */
@@ -896,11 +903,19 @@ finish_delayed_invalidation(void)
 			if (IsToastNamespace(get_rel_namespace(parent)))
 				continue;
 
-			if (!pathman_config_contains_relation(parent, NULL, NULL, NULL, NULL))
-				remove_pathman_relation_info(parent);
-			else
+			/* Fetch all partitioned tables */
+			if (!parents_fetched)
+			{
+				parents = read_parent_oids(&parents_count);
+				parents_fetched = true;
+			}
+
+			/* Check if parent still exists */
+			if (bsearch_oid(parent, parents, parents_count))
 				/* get_pathman_relation_info() will refresh this entry */
 				invalidate_pathman_relation_info(parent, NULL);
+			else
+				remove_pathman_relation_info(parent);
 		}
 
 		/* Process all other vague cases */
@@ -912,8 +927,15 @@ finish_delayed_invalidation(void)
 			if (IsToastNamespace(get_rel_namespace(vague_rel)))
 				continue;
 
+			/* Fetch all partitioned tables */
+			if (!parents_fetched)
+			{
+				parents = read_parent_oids(&parents_count);
+				parents_fetched = true;
+			}
+
 			/* It might be a partitioned table or a partition */
-			if (!try_perform_parent_refresh(vague_rel))
+			if (!try_invalidate_parent(vague_rel, parents, parents_count))
 			{
 				PartParentSearch	search;
 				Oid					parent;
@@ -923,21 +945,17 @@ finish_delayed_invalidation(void)
 
 				switch (search)
 				{
-					/* It's still parent */
+					/*
+					 * Two main cases:
+					 *	- It's *still* parent (in PATHMAN_CONFIG)
+					 *	- It *might have been* parent before (not in PATHMAN_CONFIG)
+					 */
 					case PPS_ENTRY_PART_PARENT:
-						{
-							/* Skip if we've already refreshed this parent */
-							if (!list_member_oid(fresh_rels, parent))
-								try_perform_parent_refresh(parent);
-						}
-						break;
-
-					/* It *might have been* parent before (not in PATHMAN_CONFIG) */
 					case PPS_ENTRY_PARENT:
 						{
 							/* Skip if we've already refreshed this parent */
 							if (!list_member_oid(fresh_rels, parent))
-								try_perform_parent_refresh(parent);
+								try_invalidate_parent(parent, parents, parents_count);
 						}
 						break;
 
@@ -954,6 +972,9 @@ finish_delayed_invalidation(void)
 
 		free_invalidation_list(delayed_invalidation_parent_rels);
 		free_invalidation_list(delayed_invalidation_vague_rels);
+
+		if (parents)
+			pfree(parents);
 	}
 }
 
@@ -1113,34 +1134,25 @@ try_syscache_parent_search(Oid partition, PartParentSearch *status)
 	}
 }
 
-/*
- * Try to refresh cache entry for relation 'parent'.
- *
- * Return true on success.
- */
+/* Try to invalidate cache entry for relation 'parent' */
 static bool
-try_perform_parent_refresh(Oid parent)
+try_invalidate_parent(Oid relid, Oid *parents, int parents_count)
 {
-	ItemPointerData		iptr;
-	Datum				values[Natts_pathman_config];
-	bool				isnull[Natts_pathman_config];
-
-	if (pathman_config_contains_relation(parent, values, isnull, NULL, &iptr))
+	/* Check if this is a partitioned table */
+	if (bsearch_oid(relid, parents, parents_count))
 	{
-		bool should_update_expr = isnull[Anum_pathman_config_cooked_expr - 1];
-
-		if (should_update_expr)
-			pathman_config_refresh_parsed_expression(parent, values, isnull, &iptr);
+		/* get_pathman_relation_info() will refresh this entry */
+		invalidate_pathman_relation_info(relid, NULL);
 
-		/* If anything went wrong, return false (actually, it might emit ERROR) */
-		refresh_pathman_relation_info(parent,
-									  values,
-									  true); /* allow lazy */
+		/* Success */
+		return true;
 	}
-	/* Not a partitioned relation */
-	else return false;
 
-	return true;
+	/* Clear remaining cache entry */
+	remove_pathman_relation_info(relid);
+
+	/* Not a partitioned relation */
+	return false;
 }
 
 

src/utils.c

@@ -179,6 +179,21 @@ list_reverse(List *l)
 	return result;
 }
 
+int
+oid_cmp(const void *p1, const void *p2)
+{
+	Oid		v1 = *((const Oid *) p1);
+	Oid		v2 = *((const Oid *) p2);
+
+	if (v1 < v2)
+		return -1;
+
+	if (v1 > v2)
+		return 1;
+
+	return 0;
+}
+
 
 
 /*