From 547c9d33018706a928fb0f5024e3d83c8c5f8e69 Mon Sep 17 00:00:00 2001 From: Hadi Moshayedi Date: Mon, 15 Oct 2018 14:06:56 -0400 Subject: [PATCH] Keep track of cached entries in case of interruption. (#2433) * Keep track of cached entries in case of interruption. Previously we set DistTableCacheEntry->sortedShardIntervalArray and DistTableCacheEntry->shardIntervalArrayLength after we entered all related shard entries into DistShardCacheHash. The drawback was that if populating DistShardCacheHash was interrupted, ResetDistTableCacheEntry() didn't see the shard hash entries created, so was unable to clean them up. This patch fixes that by setting sortedShardIntervalArray earlier, and incrementing shardIntervalArrayLength as we enter shards into the cache. --- .../distributed/utils/metadata_cache.c | 31 +++++++++++++++++-- 1 file changed, 28 insertions(+), 3 deletions(-) diff --git a/src/backend/distributed/utils/metadata_cache.c b/src/backend/distributed/utils/metadata_cache.c index 6393c1372..beb4c303e 100644 --- a/src/backend/distributed/utils/metadata_cache.c +++ b/src/backend/distributed/utils/metadata_cache.c @@ -858,12 +858,22 @@ LookupDistTableCacheEntry(Oid relationId) memset(((char *) cacheEntry) + sizeof(Oid), 0, sizeof(DistTableCacheEntry) - sizeof(Oid)); + /* + * We disable interrupts while creating the cache entry because loading + * shard metadata can take a while, and if statement_timeout is too low, + * this will get canceled on each call and we won't be able to run any + * queries on the table. + */ + HOLD_INTERRUPTS(); + /* actually fill out entry */ BuildDistTableCacheEntry(cacheEntry); /* and finally mark as valid */ cacheEntry->isValid = true; + RESUME_INTERRUPTS(); + return cacheEntry; } @@ -1168,6 +1178,13 @@ BuildCachedShardList(DistTableCacheEntry *cacheEntry) } } + /* + * We set these here, so ResetDistTableCacheEntry() can see what has been + * entered into DistShardCacheHash even if the following loop is interrupted + * by throwing errors, etc. + */ + cacheEntry->sortedShardIntervalArray = sortedShardIntervalArray; + cacheEntry->shardIntervalArrayLength = 0; /* maintain shardId->(table,ShardInterval) cache */ for (shardIndex = 0; shardIndex < shardIntervalArrayLength; shardIndex++) @@ -1192,6 +1209,13 @@ BuildCachedShardList(DistTableCacheEntry *cacheEntry) errhint("Reconnect and try again."))); } + /* + * We should increment this only after we are sure this hasn't already + * been assigned to any other relations. ResetDistTableCacheEntry() + * depends on this. + */ + cacheEntry->shardIntervalArrayLength++; + shardEntry->shardIndex = shardIndex; shardEntry->tableEntry = cacheEntry; @@ -1220,8 +1244,6 @@ BuildCachedShardList(DistTableCacheEntry *cacheEntry) shardInterval->shardIndex = shardIndex; } - cacheEntry->shardIntervalArrayLength = shardIntervalArrayLength; - cacheEntry->sortedShardIntervalArray = sortedShardIntervalArray; cacheEntry->shardColumnCompareFunction = shardColumnCompareFunction; cacheEntry->shardIntervalCompareFunction = shardIntervalCompareFunction; } @@ -2896,7 +2918,10 @@ ResetDistTableCacheEntry(DistTableCacheEntry *cacheEntry) bool foundInCache = false; /* delete the shard's placements */ - pfree(placementArray); + if (placementArray != NULL) + { + pfree(placementArray); + } /* delete per-shard cache-entry */ hash_search(DistShardCacheHash, &shardInterval->shardId, HASH_REMOVE,