From c7bfa06cb9b959bbb14a219c789d996546815f1e Mon Sep 17 00:00:00 2001 From: Burak Yucesoy Date: Tue, 23 May 2017 10:01:59 +0300 Subject: [PATCH 1/3] Fix incorrect call to CheckInstalledVersion During version update, we indirectly calld CheckInstalledVersion via ChackCitusVersions. This obviously fails because during version update it is expected to have version mismatch between installed version and binary version. Thus, we remove that ChackCitusVersions. We now only call ChackAvailableVersion. --- src/backend/distributed/executor/multi_utility.c | 6 ++---- src/backend/distributed/utils/metadata_cache.c | 11 ++++++----- src/include/distributed/metadata_cache.h | 2 +- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/backend/distributed/executor/multi_utility.c b/src/backend/distributed/executor/multi_utility.c index 2142b90db..9e994f42f 100644 --- a/src/backend/distributed/executor/multi_utility.c +++ b/src/backend/distributed/executor/multi_utility.c @@ -1419,11 +1419,9 @@ ErrorIfUnstableCreateOrAlterExtensionStmt(Node *parsetree) { /* * No version was specified, so PostgreSQL will use the default_version - * from the citus.control file. In case a new default is available, we - * will force a compatibility check of the latest available version. + * from the citus.control file. */ - citusVersionKnownCompatible = false; - CheckCitusVersion(ERROR); + CheckAvailableVersion(ERROR); } } diff --git a/src/backend/distributed/utils/metadata_cache.c b/src/backend/distributed/utils/metadata_cache.c index 128461a6a..12dad443c 100644 --- a/src/backend/distributed/utils/metadata_cache.c +++ b/src/backend/distributed/utils/metadata_cache.c @@ -106,7 +106,7 @@ static Oid workerHashFunctionId = InvalidOid; /* Citus extension version variables */ bool EnableVersionChecks = true; /* version checks are enabled */ -bool citusVersionKnownCompatible = false; +static bool citusVersionKnownCompatible = false; /* Hash table for informations about each partition */ static HTAB *DistTableCacheHash = NULL; @@ -143,7 +143,6 @@ static bool HasUniformHashDistribution(ShardInterval **shardIntervalArray, static bool HasUninitializedShardInterval(ShardInterval **sortedShardIntervalArray, int shardCount); static bool CheckInstalledVersion(int elevel); -static bool CheckAvailableVersion(int elevel); static char * AvailableExtensionVersion(void); static char * InstalledExtensionVersion(void); static bool HasOverlappingShardInterval(ShardInterval **shardIntervalArray, @@ -1181,13 +1180,15 @@ CheckCitusVersion(int elevel) * this function logs an error with the specified elevel and returns false, * otherwise it returns true. */ -static bool +bool CheckAvailableVersion(int elevel) { char *availableVersion = NULL; - Assert(CitusHasBeenLoaded()); - Assert(EnableVersionChecks); + if (!EnableVersionChecks) + { + return true; + } availableVersion = AvailableExtensionVersion(); diff --git a/src/include/distributed/metadata_cache.h b/src/include/distributed/metadata_cache.h index 14b0e86c6..48b03672e 100644 --- a/src/include/distributed/metadata_cache.h +++ b/src/include/distributed/metadata_cache.h @@ -18,7 +18,6 @@ #include "utils/hsearch.h" extern bool EnableVersionChecks; -extern bool citusVersionKnownCompatible; /* * Representation of a table's metadata that is frequently used for @@ -80,6 +79,7 @@ extern void CitusInvalidateRelcacheByShardId(int64 shardId); extern bool CitusHasBeenLoaded(void); extern bool CheckCitusVersion(int elevel); +extern bool CheckAvailableVersion(int elevel); bool MajorVersionsCompatible(char *leftVersion, char *rightVersion); /* access WorkerNodeHash */ From 8c1bbf1417b178e91c6c3dd2f499799072320cc5 Mon Sep 17 00:00:00 2001 From: Burak Yucesoy Date: Tue, 23 May 2017 19:03:48 +0300 Subject: [PATCH 2/3] Register cache invalidation callback before version checks With this commit we start to register InvalidateDistRelationCacheCallback function as cache invalidation callback function before version checks because during version checks we use cache to look up relation ids of some relations like pg_dist_relation or pg_dist_partition_logical_relid_index and we want to know about cache invalidation before accessing them. --- src/backend/distributed/utils/metadata_cache.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/backend/distributed/utils/metadata_cache.c b/src/backend/distributed/utils/metadata_cache.c index 12dad443c..5f4f14f97 100644 --- a/src/backend/distributed/utils/metadata_cache.c +++ b/src/backend/distributed/utils/metadata_cache.c @@ -511,6 +511,11 @@ LookupDistTableCacheEntry(Oid relationId) return NULL; } + if (DistTableCacheHash == NULL) + { + InitializeDistTableCache(); + } + /* * If the version is not known to be compatible, perform thorough check, * unless such checks are disabled. @@ -538,11 +543,6 @@ LookupDistTableCacheEntry(Oid relationId) } } - if (DistTableCacheHash == NULL) - { - InitializeDistTableCache(); - } - cacheEntry = hash_search(DistTableCacheHash, hashKey, HASH_ENTER, &foundInCache); /* return valid matches */ From aff6a3dcc4c46f66d88acf47a2a9eb2f3b69f04d Mon Sep 17 00:00:00 2001 From: Burak Yucesoy Date: Tue, 23 May 2017 19:43:04 +0300 Subject: [PATCH 3/3] Add tests for version check --- src/test/regress/expected/multi_extension.out | 24 +++++++++++++++++++ src/test/regress/sql/multi_extension.sql | 18 ++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/src/test/regress/expected/multi_extension.out b/src/test/regress/expected/multi_extension.out index 9715eb3b5..08762ae0e 100644 --- a/src/test/regress/expected/multi_extension.out +++ b/src/test/regress/expected/multi_extension.out @@ -169,7 +169,31 @@ SET citus.enable_version_checks TO 'true'; DROP FUNCTION citus_table_size(regclass); SET citus.enable_version_checks TO 'false'; ALTER EXTENSION citus UPDATE TO '6.2-2'; +-- Test updating to the latest version without specifying the version number +ALTER EXTENSION citus UPDATE; -- re-create in newest version DROP EXTENSION citus; \c CREATE EXTENSION citus; +-- test cache invalidation in workers +\c - - - :worker_1_port +-- this will initialize the cache +\d + List of relations + Schema | Name | Type | Owner +--------+------+------+------- +(0 rows) + +DROP EXTENSION citus; +SET citus.enable_version_checks TO 'false'; +CREATE EXTENSION citus VERSION '5.2-4'; +SET citus.enable_version_checks TO 'true'; +-- during ALTER EXTENSION, we should invalidate the cache +ALTER EXTENSION citus UPDATE; +-- if cache is invalidated succesfull, this \d should work without any problem +\d + List of relations + Schema | Name | Type | Owner +--------+------+------+------- +(0 rows) + diff --git a/src/test/regress/sql/multi_extension.sql b/src/test/regress/sql/multi_extension.sql index 327644f9c..7c6ed0b7f 100644 --- a/src/test/regress/sql/multi_extension.sql +++ b/src/test/regress/sql/multi_extension.sql @@ -153,7 +153,25 @@ DROP FUNCTION citus_table_size(regclass); SET citus.enable_version_checks TO 'false'; ALTER EXTENSION citus UPDATE TO '6.2-2'; +-- Test updating to the latest version without specifying the version number +ALTER EXTENSION citus UPDATE; + -- re-create in newest version DROP EXTENSION citus; \c CREATE EXTENSION citus; + +-- test cache invalidation in workers +\c - - - :worker_1_port + +-- this will initialize the cache +\d +DROP EXTENSION citus; +SET citus.enable_version_checks TO 'false'; +CREATE EXTENSION citus VERSION '5.2-4'; +SET citus.enable_version_checks TO 'true'; +-- during ALTER EXTENSION, we should invalidate the cache +ALTER EXTENSION citus UPDATE; + +-- if cache is invalidated succesfull, this \d should work without any problem +\d