Invalidate cached tenant tracker pointers per hypertable

The relcache callback used to hash_destroy the whole resolved-tracker
cache. Callbacks run from AcceptInvalidationMessages(), which fires
inside resolve_tenant_tracker()'s own catalog scans, so the function
dereferenced the table it had just created and crashed. It was also
tied to the hypertable proxy, so every hypertable catalog change in the
database made every backend re-resolve every tracker.

Disabling granular refresh now invalidates the raw hypertable's own
relcache entry. The callback receives that relid and only marks the
matching cache entry stale; the next resolve treats a stale entry as a
miss and overwrites it in place. Nothing is freed in a callback. The
freeing backend marks its own entry at commit, since it may never
execute its own message locally. The drain keeps using cached pointers
regardless of the flag: every hypertable in the buffer was resolved
under a lock this backend holds to transaction end.

Add the debug_discard_caches reproducer and a cross-backend stale
pointer permutation. Filter the tracker map by database in TEST 7 so
concurrent test databases no longer change its count.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Author: gayyappan

src/cache_invalidate.c

@@ -84,31 +84,34 @@ cache_invalidate_relcache_callback(Datum arg, Oid relid)
 	if (!OidIsValid(relid))
 	{
 		cache_invalidate_relcache_all();
-		ts_cm_functions->tenant_tracker_cache_invalidate();
+		ts_cm_functions->tenant_tracker_cache_invalidate(InvalidOid);
 	}
 	else if (ts_extension_is_proxy_table_relid(relid))
 	{
 		ts_extension_invalidate();
 		cache_invalidate_relcache_all();
-		ts_cm_functions->tenant_tracker_cache_invalidate();
+		ts_cm_functions->tenant_tracker_cache_invalidate(InvalidOid);
 		ts_cache_invalidate_set_proxy_tables(InvalidOid, InvalidOid);
 	}
 	else if (relid == hypertable_proxy_table_oid)
 	{
 		ts_hypertable_cache_invalidate_callback();
-
-		/*
-		 * Deleting a hypertable's granular refresh settings frees its tenant
-		 * tracker at commit, so drop any tracker pointer this backend cached.
-		 * Deliberately not hooked into cache_invalidate_relcache_all(): that
-		 * also runs on every aborted transaction, where nothing was freed.
-		 */
-		ts_cm_functions->tenant_tracker_cache_invalidate();
 	}
 	else if (relid == bgw_proxy_table_oid)
 	{
 		ts_bgw_job_cache_invalidate_callback();
 	}
+	else
+	{
+		/*
+		 * Disabling granular refresh on a hypertable frees its tenant tracker
+		 * at commit and invalidates the hypertable's own relcache entry, so a
+		 * backend that cached a pointer to that tracker can drop just that one.
+		 * Not routed through cache_invalidate_relcache_all(): an aborted
+		 * transaction frees nothing.
+		 */
+		ts_cm_functions->tenant_tracker_cache_invalidate(relid);
+	}
 }
 
 TS_FUNCTION_INFO_V1(ts_timescaledb_invalidate_cache);

src/cross_module_fn.c

@@ -287,7 +287,7 @@ preprocess_query_tsl_default_fn_community(Query *parse, int *cursor_opts)
 }
 
 static void
-tenant_tracker_cache_invalidate_default(void)
+tenant_tracker_cache_invalidate_default(Oid relid)
 {
 	/* No op in community licensed code */
 }

src/cross_module_fn.h

@@ -106,7 +106,7 @@ typedef struct CrossModuleFunctions
 	void (*continuous_agg_dml_invalidate)(int32 hypertable_id, Relation chunk_rel,
 										  HeapTuple chunk_tuple, HeapTuple chunk_newtuple,
 										  bool update);
-	void (*tenant_tracker_cache_invalidate)(void);
+	void (*tenant_tracker_cache_invalidate)(Oid relid);
 	void (*continuous_agg_update_options)(ContinuousAgg *cagg,
 										  WithClauseResult *with_clause_options);
 	void (*continuous_agg_add_column)(ContinuousAgg *cagg, AlterTableStmt *stmt);

src/ts_catalog/catalog.c

@@ -800,7 +800,6 @@ ts_catalog_invalidate_cache(Oid catalog_relid, CmdType operation)
 		case DIMENSION:
 		case CONTINUOUS_AGG:
 		case CHUNK_COLUMN_STATS:
-		case HYPERTABLE_CAGG_SETTINGS:
 			relid = ts_catalog_get_cache_proxy_id(catalog, CACHE_TYPE_HYPERTABLE);
 			CacheInvalidateRelcacheByRelid(relid);
 			break;

tsl/src/continuous_aggs/insert.c

@@ -88,7 +88,8 @@ typedef struct TenantLocalKey
 
 typedef struct TenantLocalEntry
 {
-	TenantLocalKey key; /* must be first: dynahash key */
+	TenantLocalKey key;	  /* must be first: dynahash key */
+	Oid hypertable_relid; /* key into the resolved-tracker cache at drain */
 	int64 min_ts;
 	int64 max_ts;
 } TenantLocalEntry;
@@ -115,14 +116,21 @@ typedef struct ContinuousAggsCacheHyperInvalThresholdEntry
  * the DML path and avoids any additional lookups.
  * tracking == NULL means DISABLED, tracking = non NULL means we have a valid
  * entry in shared mem.
- * Both outcomes are stable for the backend's life: a tracker is
- * never moved or freed and its mapping is pinned, and a DISABLED marker persists
- * until restart . So entries never need invalidation.  MyDatabaseId is fixed
- * per backend, so hypertable_id alone is a sufficient key.
+ *
+ * A tracker is freed only when granular refresh is disabled on its hypertable,
+ * which invalidates the hypertable's relcache entry (see granular_refresh_disable).
+ * The cache is keyed by the hypertable's relid so the relcache callback can find
+ * the one entry the message is about and mark it stale; the next resolve treats
+ * a stale entry as a miss and overwrites it in place.  Entries are never removed
+ * and the table is never destroyed, so the callback can run at any
+ * AcceptInvalidationMessages() without pulling memory out from under a caller.
+ * MyDatabaseId is fixed per backend, so the relid alone is a sufficient key.
  */
 typedef struct TenantTrackerCacheEntry
 {
-	int32 hypertable_id;
+	Oid main_table_relid; /* must be first: dynahash key; what relcache messages carry */
+	int32 hypertable_id;  /* what the shared tracker map is keyed by */
+	bool stale;			  /* set by the relcache callback; re-resolve on next use */
 	TenantTracking *tracking;
 } TenantTrackerCacheEntry;
 
@@ -160,7 +168,7 @@ static inline ContinuousAggsCacheInvalEntry *get_cache_inval_entry(int32 hyperta
 static void cache_inval_cleanup(void);
 static void cache_inval_htab_write(List *hypertable_seqnums);
 static HTAB *get_tenant_local_htab(void);
-static TenantTracking *resolve_tenant_tracker(int32 hypertable_id);
+static TenantTracking *resolve_tenant_tracker(int32 hypertable_id, Oid main_table_relid);
 static List *tenant_local_htab_write(void);
 static void continuous_agg_xact_invalidation_callback(XactEvent event, void *arg);
 static ScanTupleResult invalidation_tuple_found(TupleInfo *ti, void *min);
@@ -365,10 +373,10 @@ get_tenant_local_htab(void)
  *
  * Returns the tracker, or NULL when tracking is disabled for this hypertable
  * (negative-cache marker, loader absent, or a contained OOM).  Both outcomes are
- * cached and stable for the backend's life.
+ * cached until a relcache invalidation for the hypertable marks the entry stale.
  */
 static TenantTracking *
-resolve_tenant_tracker(int32 hypertable_id)
+resolve_tenant_tracker(int32 hypertable_id, Oid main_table_relid)
 {
 	TenantTrackerCacheEntry *ce;
 	TenantTracking *volatile tracking = NULL;
@@ -380,7 +388,7 @@ resolve_tenant_tracker(int32 hypertable_id)
 		HASHCTL ctl;
 
 		memset(&ctl, 0, sizeof(ctl));
-		ctl.keysize = sizeof(int32);
+		ctl.keysize = sizeof(Oid);
 		ctl.entrysize = sizeof(TenantTrackerCacheEntry);
 		ctl.hcxt = TopMemoryContext; /* backend lifetime: entries never expire */
 		tenant_tracker_resolved_htab = hash_create("TS Tenant Tracker Resolved",
@@ -389,14 +397,15 @@ resolve_tenant_tracker(int32 hypertable_id)
 												   HASH_ELEM | HASH_BLOBS | HASH_CONTEXT);
 	}
 
-	ce = hash_search(tenant_tracker_resolved_htab, &hypertable_id, HASH_FIND, NULL);
-	if (ce != NULL)
+	ce = hash_search(tenant_tracker_resolved_htab, &main_table_relid, HASH_FIND, NULL);
+	if (ce != NULL && !ce->stale)
 	{
 		return ce->tracking; /* cached FOUND (ptr) or DISABLED (NULL) */
 	}
 
 	/*
-	 * First time this backend touches this hypertable.  Park here so the
+	 * First time this backend touches this hypertable, or the cached entry was
+	 * invalidated.  Park here so the
 	 * first-touch race isolation test can line up two backends before the
 	 * get_or_attach below (which serializes on the dshash partition lock).
 	 */
@@ -468,39 +477,60 @@ resolve_tenant_tracker(int32 hypertable_id)
 	}
 	PG_END_TRY();
 
-	ce = hash_search(tenant_tracker_resolved_htab, &hypertable_id, HASH_ENTER, NULL);
+	/* Re-find: the catalog scans above can run relcache callbacks, but those only
+	 * flip the stale flag; the table itself is never freed under us. */
+	ce = hash_search(tenant_tracker_resolved_htab, &main_table_relid, HASH_ENTER, NULL);
+	ce->hypertable_id = hypertable_id;
 	ce->tracking = tracking;
+	ce->stale = false;
 	return tracking;
 }
 
 /*
- * Drop this backend's resolved-tracker cache.
+ * Relcache invalidation callback: mark this backend's cached tracker entry for
+ * the relation stale (all entries for InvalidOid, i.e. a sinval reset).
  *
- * Called from the hypertable-proxy relcache invalidation callback, which is how
- * a backend learns that some hypertable's granular refresh settings changed --
- * and therefore that its tenant tracker may have been freed at that
- * transaction's commit.  The cached pointers are raw DSA addresses with no way
- * to tell a live tracker from a freed one, so the whole table goes; a re-resolve
- * is one dshash lookup.
+ * Disabling granular refresh frees the hypertable's tracker at commit and
+ * invalidates the hypertable's relcache entry, so this is how a backend learns
+ * that its cached pointer may now dangle.  The relation's ordinary relcache
+ * events (ANALYZE, ALTER TABLE) land here too; they just cost one dshash
+ * lookup on the next resolve.
  *
- * The proxy carries no hypertable id, so this cannot be selective.  That also
- * clears the cached DISABLED (NULL) entries, which is harmless: the
- * negative-cache marker lives in the shared map entry, so a re-resolve for a
- * hypertable whose allocation once failed still returns NULL without retrying
- * the allocation.
+ * Also called directly by the commit callback that frees a tracker: the freeing
+ * backend's own sinval messages are only executed locally at
+ * CommandCounterIncrement, so it cannot count on receiving the message it sent.
  *
- * Safe to run mid-transaction.  A backend that has already buffered tenants
- * this transaction will find no entry in tenant_local_htab_write() and record
- * seqnum 0 for that hypertable, which makes the refresh fall back to the full
- * invalidation log -- an over-refresh, never a missed one.
+ * Runs from AcceptInvalidationMessages(), possibly in the middle of
+ * resolve_tenant_tracker(): it must not allocate, free or throw.  Setting a
+ * flag is all it does.  The drain (tenant_local_htab_write) ignores the flag on
+ * purpose -- see the comment there.
  */
 void
-continuous_agg_tenant_tracker_cache_invalidate(void)
+continuous_agg_tenant_tracker_cache_invalidate(Oid relid)
 {
-	if (tenant_tracker_resolved_htab != NULL)
+	TenantTrackerCacheEntry *ce;
+
+	if (tenant_tracker_resolved_htab == NULL)
 	{
-		hash_destroy(tenant_tracker_resolved_htab);
-		tenant_tracker_resolved_htab = NULL;
+		return;
+	}
+
+	if (!OidIsValid(relid))
+	{
+		HASH_SEQ_STATUS status;
+
+		hash_seq_init(&status, tenant_tracker_resolved_htab);
+		while ((ce = hash_seq_search(&status)) != NULL)
+		{
+			ce->stale = true;
+		}
+		return;
+	}
+
+	ce = hash_search(tenant_tracker_resolved_htab, &relid, HASH_FIND, NULL);
+	if (ce != NULL)
+	{
+		ce->stale = true;
 	}
 }
 
@@ -582,11 +612,13 @@ record_tenant_invalidation_values(const ContinuousAggsCacheInvalEntry *cache_ent
 	// TODO: try to see if there's a way to avoid allocate and free memory for every key.
 	pfree(key);
 
-	resolve_tenant_tracker(cache_entry->hypertable_id);
+	resolve_tenant_tracker(cache_entry->hypertable_id,
+						   cache_entry->hypertable_open_dimension.main_table_relid);
 
 	entry = (TenantLocalEntry *) hash_search(get_tenant_local_htab(), &lookup, HASH_ENTER, &found);
 	if (!found)
 	{
+		entry->hypertable_relid = cache_entry->hypertable_open_dimension.main_table_relid;
 		entry->min_ts = timeval;
 		entry->max_ts = timeval;
 	}
@@ -695,26 +727,33 @@ tenant_local_htab_write(void)
 	HASH_SEQ_STATUS hash_seq;
 	TenantLocalEntry *entry;
 	List *hypertable_ids = NIL;
+	List *hypertable_relids = NIL;
 	List *hypertable_seqnums = NIL;
-	ListCell *lc;
+	ListCell *lc, *lc_relid;
 
 	if (tenant_local_htab == NULL)
 	{
 		return NIL;
 	}
 
-	/* Distinct hypertables present in the buffer (usually just one). */
+	/* Distinct hypertables present in the buffer (usually just one), with the
+	 * relid each was resolved under -- that is the resolved cache's key. */
 	hash_seq_init(&hash_seq, tenant_local_htab);
 	while ((entry = hash_seq_search(&hash_seq)) != NULL)
 	{
-		hypertable_ids = list_append_unique_int(hypertable_ids, entry->key.hypertable_id);
+		if (!list_member_int(hypertable_ids, entry->key.hypertable_id))
+		{
+			hypertable_ids = lappend_int(hypertable_ids, entry->key.hypertable_id);
+			hypertable_relids = lappend_oid(hypertable_relids, entry->hypertable_relid);
+		}
 	}
 
 	/* Drain each hypertable's tenants into its own tracker (one generation pin
 	 * per hypertable). */
-	foreach (lc, hypertable_ids)
+	forboth (lc, hypertable_ids, lc_relid, hypertable_relids)
 	{
 		int32 hypertable_id = lfirst_int(lc);
+		Oid hypertable_relid = lfirst_oid(lc_relid);
 		TenantTracking *tracking;
 		TenantGeneration *generation;
 		int32 seqnum = 0;
@@ -743,9 +782,17 @@ tenant_local_htab_write(void)
 		 * relying on this staying throw-free.  A NULL entry means tracking is
 		 * disabled for this hypertable; a missing entry means nothing was buffered
 		 * for it (shouldn't happen), both -> skip (untracked, seqnum 0).
+		 *
+		 * The stale flag is deliberately ignored here.  Every hypertable in the
+		 * buffer was resolved in this transaction while this backend held a lock
+		 * on it, and that lock is held to transaction end, so the free (which
+		 * needs AccessExclusiveLock) cannot have happened.  A stale mark seen
+		 * here comes from a non-conflicting relcache event on the same relation
+		 * (e.g. ANALYZE); honoring it would only force a needless seqnum-0
+		 * fallback, and re-resolving would put dshash locks on this path.
 		 */
 		ce = (tenant_tracker_resolved_htab != NULL) ?
-				 hash_search(tenant_tracker_resolved_htab, &hypertable_id, HASH_FIND, NULL) :
+				 hash_search(tenant_tracker_resolved_htab, &hypertable_relid, HASH_FIND, NULL) :
 				 NULL;
 		tracking = (ce != NULL) ? ce->tracking : NULL;
 

tsl/src/continuous_aggs/insert.h

@@ -7,6 +7,7 @@
 
 #include <postgres.h>
 #include <executor/tuptable.h>
+#include <utils/rel.h>
 
 extern void _continuous_aggs_cache_inval_init(void);
 extern void _continuous_aggs_cache_inval_fini(void);
@@ -17,4 +18,4 @@ extern void continuous_agg_dml_invalidate(int32 hypertable_id, Relation chunk_re
 										  bool update);
 extern void continuous_agg_record_tenant_from_slot(int32 hypertable_id, Oid chunk_relid,
 												   TupleTableSlot *slot);
-extern void continuous_agg_tenant_tracker_cache_invalidate(void);
+extern void continuous_agg_tenant_tracker_cache_invalidate(Oid relid);