From 75b71750c96c3a26a97266d4bb64fac433ba6361 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Nordstr=C3=B6m?= Date: Thu, 24 Mar 2022 09:31:52 +0100 Subject: [PATCH 1/2] Fix relcache callback handling causing crashes Fix a crash that could corrupt indexes when running VACUUM FULL pg_class. The crash happens when caches are queried/updated within a cache invalidation function, which can lead to corruption and recursive cache invalidations. To solve the issue, make sure the relcache invalidation callback is simple and never invokes the relcache or syscache directly or indirectly. Some background: The extension is preloaded and thus have planner hooks installed irrespective of whether the extension is actually installed or not in the current database. However, the hooks need to be disabled as long as the extension is not installed. To avoid always having to dynamically check for the presence of the extension, the state is cached in the session. However, the cached state needs to be updated if the extension changes (altered/dropped/created). Therefore, the relcache invalidation callback mechanism is (ab)used in TimescaleDB to signal updates to the extension state across all active backends. The signaling is implemented by installing a dummy table as part of the extension and any invalidation on the relid for that table signals a change in the extension state. However, as of this change, the actual state is no longer determined in the callback itself, since it requires use of the relcache and causes the bad behavior. Therefore, the only thing that remains in the callback after this change is to reset the extension state. The actual state is instead resolved on-demand, but can still be cached when the extension is in the installed state and the dummy table is present with a known relid. However, if the extension is not installed, the extension state can no longer be cached as there is no way to signal other backends that the state should be reset when they don't know the dummy table's relid, and cannot resolve it from within the callback itself. Fixes #3924 --- src/cache_invalidate.c | 60 +++++++++++----------- src/cache_invalidate.h | 13 +++++ src/extension.c | 106 +++++++++++++++++++-------------------- src/extension.h | 11 ++-- src/extension_utils.c | 23 ++++++--- src/planner.c | 6 +-- src/ts_catalog/catalog.c | 7 +++ src/ts_catalog/catalog.h | 1 + test/src/loader/init.c | 7 ++- tsl/src/fdw/relinfo.c | 3 +- 10 files changed, 132 insertions(+), 105 deletions(-) create mode 100644 src/cache_invalidate.h diff --git a/src/cache_invalidate.c b/src/cache_invalidate.c index 7b6c8eb4357..2ceead70e0b 100644 --- a/src/cache_invalidate.c +++ b/src/cache_invalidate.c @@ -20,6 +20,7 @@ #include "bgw/scheduler.h" #include "cross_module_fn.h" +#include "cache_invalidate.h" /* * Notes on the way cache invalidation works. @@ -59,45 +60,44 @@ cache_invalidate_relcache_all(void) ts_bgw_job_cache_invalidate_callback(); } +static Oid hypertable_proxy_table_oid = InvalidOid; +static Oid bgw_proxy_table_oid = InvalidOid; + +void +ts_cache_invalidate_set_proxy_tables(Oid hypertable_proxy_oid, Oid bgw_proxy_oid) +{ + hypertable_proxy_table_oid = hypertable_proxy_oid; + bgw_proxy_table_oid = bgw_proxy_oid; +} + /* * This function is called when any relcache is invalidated. * Should route the invalidation to the correct cache. + * + * NOTE that the callback should not call any functions that could invoke the + * relcache or syscache to query information during the invalidation. That + * might lead to bad things happening. */ static void -cache_invalidate_callback(Datum arg, Oid relid) +cache_invalidate_relcache_callback(Datum arg, Oid relid) { - static bool in_recursion = false; - - if (ts_extension_invalidate(relid)) + if (!OidIsValid(relid)) { cache_invalidate_relcache_all(); - return; } - - if (!ts_extension_is_loaded()) - return; - - /* The cache invalidation can be called indirectly further down in the - * call chain by calling `get_namespace_oid`, which can trigger a - * recursive cache invalidation callback. To prevent infinite recursion, - * we stop it here and instead consider the cache as invalidated, which - * will allow the `get_namespace_oid` to retrieve the OID from the heap - * table and return it properly. */ - if (!in_recursion) + else if (ts_extension_is_proxy_table_relid(relid)) { - Catalog *catalog; - in_recursion = true; - catalog = ts_catalog_get(); - in_recursion = false; - - if (relid == ts_catalog_get_cache_proxy_id(catalog, CACHE_TYPE_HYPERTABLE)) - ts_hypertable_cache_invalidate_callback(); - - if (relid == ts_catalog_get_cache_proxy_id(catalog, CACHE_TYPE_BGW_JOB)) - ts_bgw_job_cache_invalidate_callback(); - - if (relid == InvalidOid) - cache_invalidate_relcache_all(); + ts_extension_invalidate(); + cache_invalidate_relcache_all(); + ts_cache_invalidate_set_proxy_tables(InvalidOid, InvalidOid); + } + else if (relid == hypertable_proxy_table_oid) + { + ts_hypertable_cache_invalidate_callback(); + } + else if (relid == bgw_proxy_table_oid) + { + ts_bgw_job_cache_invalidate_callback(); } } @@ -182,7 +182,7 @@ _cache_invalidate_init(void) { RegisterXactCallback(cache_invalidate_xact_end, NULL); RegisterSubXactCallback(cache_invalidate_subxact_end, NULL); - CacheRegisterRelcacheCallback(cache_invalidate_callback, PointerGetDatum(NULL)); + CacheRegisterRelcacheCallback(cache_invalidate_relcache_callback, PointerGetDatum(NULL)); /* Specific syscache callbacks */ CacheRegisterSyscacheCallback(FOREIGNSERVEROID, diff --git a/src/cache_invalidate.h b/src/cache_invalidate.h new file mode 100644 index 00000000000..d1696696e2d --- /dev/null +++ b/src/cache_invalidate.h @@ -0,0 +1,13 @@ +/* + * This file and its contents are licensed under the Apache License 2.0. + * Please see the included NOTICE for copyright information and + * LICENSE-APACHE for a copy of the license. + */ +#ifndef TIMESCALEDB_CACHE_INVALIDATE_H +#define TIMESCALEDB_CACHE_INVALIDATE_H + +#include + +extern void ts_cache_invalidate_set_proxy_tables(Oid hypertable_proxy_oid, Oid bgw_proxy_oid); + +#endif /* TIMESCALEDB_CACHE_INVALIDATE_H */ diff --git a/src/extension.c b/src/extension.c index e8730243fdb..bd0986d8c3f 100644 --- a/src/extension.c +++ b/src/extension.c @@ -8,8 +8,10 @@ #include #include #include +#include #include #include +#include #include "compat/compat-msvc-enter.h" /* To label externs in extension.h and * miscadmin.h correctly */ @@ -62,7 +64,20 @@ static enum ExtensionState extstate = EXTENSION_STATE_UNKNOWN; * often need it during the planning, so we cache it here. We update it when * the extension status is updated. */ -Oid ts_extension_oid = InvalidOid; +static Oid ts_extension_oid = InvalidOid; + +Oid +ts_extension_get_oid(void) +{ + if (OidIsValid(ts_extension_oid)) + { + Assert(ts_extension_oid == get_extension_oid(EXTENSION_NAME, true)); + return ts_extension_oid; + } + + ts_extension_oid = get_extension_oid(EXTENSION_NAME, false); + return ts_extension_oid; +} static bool extension_loader_present() @@ -139,8 +154,7 @@ extension_set_state(enum ExtensionState newstate) break; case EXTENSION_STATE_CREATED: ts_extension_check_version(TIMESCALEDB_VERSION_MOD); - extension_proxy_oid = get_relname_relid(EXTENSION_PROXY_TABLE, - get_namespace_oid(CACHE_SCHEMA_NAME, false)); + extension_proxy_oid = get_proxy_table_relid(); ts_catalog_reset(); break; case EXTENSION_STATE_NOT_INSTALLED: @@ -156,16 +170,23 @@ extension_set_state(enum ExtensionState newstate) static void extension_update_state() { - static bool in_recursion = false; - /* Since the state of the extension is determined by the snapshot of the transaction there - * is no point processing recursive calls as the outer call will always set the correct state. - * This also prevents deep recursion during `AcceptInvalidationMessages`. + enum ExtensionState new_state = extension_current_state(); + + /* Never actually set the state to "not installed" since there is no good + * way to get out of it in case the extension is installed again in + * another backend. After the extension has been dropped, the proxy table + * no longer exists and when the extension is reinstalled, the proxy table + * will have a different relid. Therefore, there is no way to identify the + * invalidation on the proxy table when CREATE EXTENSION is issued in + * another backend. Nor is it allowed to lookup the new relid in the + * invalidation callback, since that may lead to bad behavior. + * + * Instead, set the state to "unknown" so that a "slow path" lookup of the + * actual state has to be made next time the state is queried. */ - if (in_recursion) - return; + if (new_state == EXTENSION_STATE_NOT_INSTALLED) + new_state = EXTENSION_STATE_UNKNOWN; - in_recursion = true; - enum ExtensionState new_state = extension_current_state(); extension_set_state(new_state); /* * Update the extension oid. Note that it is only safe to run @@ -182,7 +203,6 @@ extension_update_state() { ts_extension_oid = InvalidOid; } - in_recursion = false; } Oid @@ -239,49 +259,21 @@ ts_experimental_schema_name(void) } /* - * Called upon all Relcache invalidate events. - * Returns whether or not to invalidate the entire extension. + * Invalidate the state of the extension (i.e., whether the extension is + * installed or not in the current database). + * + * Since this function is called from a relcache invalidation callback, it + * must not, directly or indirectly, call functions that use the cache. This + * includes, e.g., table scans. + * + * Instead, the function just invalidates the state so that the true state is + * resolved lazily when needed. */ -bool -ts_extension_invalidate(Oid relid) +void +ts_extension_invalidate(void) { - bool invalidate_all = false; - - switch (extstate) - { - case EXTENSION_STATE_NOT_INSTALLED: - /* This event may mean we just added the proxy table */ - case EXTENSION_STATE_UNKNOWN: - /* Can we recompute the state now? */ - case EXTENSION_STATE_TRANSITIONING: - /* Has the create/drop extension finished? */ - extension_update_state(); - break; - case EXTENSION_STATE_CREATED: - - /* - * Here we know the proxy table oid so only listen to potential - * drops on that oid. Note that an invalid oid passed in the - * invalidation event applies to all tables. - */ - if (extension_proxy_oid == relid || !OidIsValid(relid)) - { - extension_update_state(); - if (EXTENSION_STATE_CREATED != extstate) - { - /* - * note this state may be UNKNOWN but should be - * conservative - */ - invalidate_all = true; - } - } - break; - default: - elog(ERROR, "unknown state: %d", extstate); - break; - } - return invalidate_all; + extstate = EXTENSION_STATE_UNKNOWN; + extension_proxy_oid = InvalidOid; } bool @@ -307,6 +299,8 @@ ts_extension_is_loaded(void) switch (extstate) { case EXTENSION_STATE_CREATED: + Assert(OidIsValid(ts_extension_oid)); + Assert(OidIsValid(extension_proxy_oid)); return true; case EXTENSION_STATE_NOT_INSTALLED: case EXTENSION_STATE_UNKNOWN: @@ -353,3 +347,9 @@ ts_extension_get_version(void) { return extension_version(); } + +bool +ts_extension_is_proxy_table_relid(Oid relid) +{ + return relid == extension_proxy_oid; +} diff --git a/src/extension.h b/src/extension.h index 3966589b246..cc7a5622ee1 100644 --- a/src/extension.h +++ b/src/extension.h @@ -6,10 +6,12 @@ #ifndef TIMESCALEDB_EXTENSION_H #define TIMESCALEDB_EXTENSION_H #include +#include + #include "extension_constants.h" #include "export.h" -extern bool ts_extension_invalidate(Oid relid); +extern void ts_extension_invalidate(void); extern TSDLLEXPORT bool ts_extension_is_loaded(void); extern void ts_extension_check_version(const char *so_version); extern void ts_extension_check_server_version(void); @@ -18,7 +20,10 @@ extern TSDLLEXPORT char *ts_extension_schema_name(void); extern const char *ts_experimental_schema_name(void); extern const char *ts_extension_get_so_name(void); extern TSDLLEXPORT const char *ts_extension_get_version(void); - -extern TSDLLEXPORT Oid ts_extension_oid; +extern bool ts_drop_owned_statement_drops_timescaledb_extension(const DropOwnedStmt *stmt); +extern bool ts_extension_is_proxy_table_relid(Oid relid); +extern TSDLLEXPORT Oid ts_extension_get_oid(void); +extern void _ts_extension_init(void); +extern void _ts_extension_fini(void); #endif /* TIMESCALEDB_EXTENSION_H */ diff --git a/src/extension_utils.c b/src/extension_utils.c index ca2c28b1baa..7a3ab68caeb 100644 --- a/src/extension_utils.c +++ b/src/extension_utils.c @@ -37,10 +37,11 @@ enum ExtensionState { /* * NOT_INSTALLED means that this backend knows that the extension is not - * present. In this state we know that the proxy table is not present. - * Thus, the only way to get out of this state is a RelCacheInvalidation - * indicating that the proxy table was added. This is the state returned - * during DROP EXTENSION. + * present. In this state we know that the proxy table is not present. + * This state is never saved since there is no real way to get out of it + * since we cannot signal via the proxy table as its relid is not known + * post installation without a full lookup, which is not allowed in the + * relcache calllback. */ EXTENSION_STATE_NOT_INSTALLED, @@ -110,14 +111,15 @@ extension_version(void) return sql_version; } -static bool inline proxy_table_exists() +static Oid +get_proxy_table_relid() { Oid nsid = get_namespace_oid(CACHE_SCHEMA_NAME, true); if (!OidIsValid(nsid)) - return false; + return InvalidOid; - return OidIsValid(get_relname_relid(EXTENSION_PROXY_TABLE, nsid)); + return get_relname_relid(EXTENSION_PROXY_TABLE, nsid); } static bool inline extension_exists() @@ -142,6 +144,8 @@ static bool inline extension_is_transitioning() static enum ExtensionState extension_current_state() { + Oid proxy_relid; + /* * NormalProcessingMode necessary to avoid accessing cache before its * ready (which may result in an infinite loop). More concretely we need @@ -162,7 +166,10 @@ extension_current_state() * EXTENSION proxy_table_exists() will return false right away, while * extension_exists will return true until the end of the command */ - if (proxy_table_exists()) + + proxy_relid = get_proxy_table_relid(); + + if (OidIsValid(proxy_relid)) { Assert(extension_exists()); return EXTENSION_STATE_CREATED; diff --git a/src/planner.c b/src/planner.c index 9a36d920840..6879e81bb06 100644 --- a/src/planner.c +++ b/src/planner.c @@ -400,13 +400,9 @@ timescaledb_planner(Query *parse, int cursor_opts, ParamListInfo bound_params) { PreprocessQueryContext context = { 0 }; context.rootquery = parse; + if (ts_extension_is_loaded()) { - /* - * Some debug checks. - */ - Assert(ts_extension_oid == get_extension_oid(EXTENSION_NAME, /* missing_ok = */ false)); - /* * Preprocess the hypertables in the query and warm up the caches. */ diff --git a/src/ts_catalog/catalog.c b/src/ts_catalog/catalog.c index 1008b50c3e5..2f775ca74f5 100644 --- a/src/ts_catalog/catalog.c +++ b/src/ts_catalog/catalog.c @@ -21,6 +21,7 @@ #include "compat/compat.h" #include "ts_catalog/catalog.h" #include "extension.h" +#include "cache_invalidate.h" static const TableInfoDef catalog_table_names[_MAX_CATALOG_TABLES + 1] = { [HYPERTABLE] = { @@ -311,6 +312,7 @@ const static InternalFunctionDef internal_function_definitions[_MAX_INTERNAL_FUN static const char *cache_proxy_table_names[_MAX_CACHE_TYPES] = { [CACHE_TYPE_HYPERTABLE] = "cache_inval_hypertable", [CACHE_TYPE_BGW_JOB] = "cache_inval_bgw_job", + [CACHE_TYPE_EXTENSION] = "cache_inval_extension", }; /* Catalog information for the current database. */ @@ -474,6 +476,9 @@ ts_catalog_get(void) get_relname_relid(cache_proxy_table_names[i], s_catalog.extension_schema_id[TS_CACHE_SCHEMA]); + ts_cache_invalidate_set_proxy_tables(s_catalog.caches[CACHE_TYPE_HYPERTABLE].inval_proxy_id, + s_catalog.caches[CACHE_TYPE_BGW_JOB].inval_proxy_id); + for (i = 0; i < _MAX_INTERNAL_FUNCTIONS; i++) { InternalFunctionDef def = internal_function_definitions[i]; @@ -507,6 +512,8 @@ ts_catalog_reset(void) { s_catalog.initialized = false; database_info.database_id = InvalidOid; + + ts_cache_invalidate_set_proxy_tables(InvalidOid, InvalidOid); } static CatalogTable diff --git a/src/ts_catalog/catalog.h b/src/ts_catalog/catalog.h index 0c5a00ebf54..6991172a149 100644 --- a/src/ts_catalog/catalog.h +++ b/src/ts_catalog/catalog.h @@ -1272,6 +1272,7 @@ typedef enum CacheType { CACHE_TYPE_HYPERTABLE, CACHE_TYPE_BGW_JOB, + CACHE_TYPE_EXTENSION, _MAX_CACHE_TYPES } CacheType; diff --git a/test/src/loader/init.c b/test/src/loader/init.c index e0704cadc22..a2e196154bc 100644 --- a/test/src/loader/init.c +++ b/test/src/loader/init.c @@ -17,6 +17,7 @@ #include #include "compat/compat.h" #include "export.h" +#include "extension.h" #define STR_EXPAND(x) #x #define STR(x) STR_EXPAND(x) @@ -30,9 +31,6 @@ extern void PGDLLEXPORT _PG_fini(void); static post_parse_analyze_hook_type prev_post_parse_analyze_hook; -bool ts_extension_invalidate(Oid relid); -bool ts_extension_is_loaded(void); -void ts_extension_check_version(const char *actual_version); bool ts_license_guc_check_hook(char **newval, void **extra, GucSource source); void ts_license_guc_assign_hook(const char *newval, void *extra); @@ -41,7 +39,8 @@ TS_FUNCTION_INFO_V1(ts_post_load_init); static void cache_invalidate_callback(Datum arg, Oid relid) { - ts_extension_invalidate(relid); + if (ts_extension_is_proxy_table_relid(relid)) + ts_extension_invalidate(); } static void diff --git a/tsl/src/fdw/relinfo.c b/tsl/src/fdw/relinfo.c index 84c6c0e0c20..bcaacafce66 100644 --- a/tsl/src/fdw/relinfo.c +++ b/tsl/src/fdw/relinfo.c @@ -425,8 +425,7 @@ fdw_relinfo_create(PlannerInfo *root, RelOptInfo *rel, Oid server_oid, Oid local */ fpinfo->fdw_startup_cost = DEFAULT_FDW_STARTUP_COST; fpinfo->fdw_tuple_cost = DEFAULT_FDW_TUPLE_COST; - Assert(ts_extension_oid != InvalidOid); - fpinfo->shippable_extensions = list_make1_oid(ts_extension_oid); + fpinfo->shippable_extensions = list_make1_oid(ts_extension_get_oid()); fpinfo->fetch_size = DEFAULT_FDW_FETCH_SIZE; apply_fdw_and_server_options(fpinfo); From 43bedf35eb7f090f38d7c6f4d19e3281b7475423 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Nordstr=C3=B6m?= Date: Fri, 25 Mar 2022 14:48:35 +0100 Subject: [PATCH 2/2] Add TAP tests for extension state Add a TAP test that checks that the extensions state is updated across concurrent sessions/backends when the extension is "dropped" or "created". --- .../workflows/linux-32bit-build-and-test.yaml | 10 ++ .github/workflows/linux-build-and-test.yaml | 18 +++- src/extension.c | 17 ++++ test/CMakeLists.txt | 19 ++++ test/pg_prove.sh | 9 +- test/t/001_extension.pl | 99 +++++++++++++++++++ test/t/CMakeLists.txt | 12 +++ test/t/functions.sql.in | 6 ++ tsl/test/CMakeLists.txt | 4 +- 9 files changed, 187 insertions(+), 7 deletions(-) create mode 100644 test/t/001_extension.pl create mode 100644 test/t/CMakeLists.txt create mode 100644 test/t/functions.sql.in diff --git a/.github/workflows/linux-32bit-build-and-test.yaml b/.github/workflows/linux-32bit-build-and-test.yaml index 89ab41c2c9d..64db97baa0c 100644 --- a/.github/workflows/linux-32bit-build-and-test.yaml +++ b/.github/workflows/linux-32bit-build-and-test.yaml @@ -116,3 +116,13 @@ jobs: with: name: PostgreSQL log linux-i386 PG${{ matrix.pg }} path: postgres.log + + - name: Save TAP test logs + if: always() + uses: actions/upload-artifact@v3 + with: + name: TAP test logs ${{ matrix.os }} ${{ matrix.name }} ${{ matrix.pg }} + path: | + build/test/tmp_check/log + build/tsl/test/tmp_check/log + diff --git a/.github/workflows/linux-build-and-test.yaml b/.github/workflows/linux-build-and-test.yaml index 62dadf08714..c38ec4f02f7 100644 --- a/.github/workflows/linux-build-and-test.yaml +++ b/.github/workflows/linux-build-and-test.yaml @@ -151,18 +151,18 @@ jobs: - name: Save regression diffs if: always() && steps.collectlogs.outputs.regression_diff == 'true' - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: Regression diff ${{ matrix.os }} ${{ matrix.name }} ${{ matrix.pg }} path: regression.log - name: Save postmaster.log if: always() - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: PostgreSQL log ${{ matrix.os }} ${{ matrix.name }} ${{ matrix.pg }} path: postgres.log - + - name: Stack trace if: always() && steps.collectlogs.outputs.coredumps == 'true' run: | @@ -172,7 +172,17 @@ jobs: - name: Coredumps if: always() && steps.collectlogs.outputs.coredumps == 'true' - uses: actions/upload-artifact@v2 + uses: actions/upload-artifact@v3 with: name: Coredumps ${{ matrix.os }} ${{ matrix.name }} ${{ matrix.pg }} path: coredumps + + - name: Save TAP test logs + if: always() + uses: actions/upload-artifact@v3 + with: + name: TAP test logs ${{ matrix.os }} ${{ matrix.name }} ${{ matrix.pg }} + path: | + build/test/tmp_check/log + build/tsl/test/tmp_check/log + diff --git a/src/extension.c b/src/extension.c index bd0986d8c3f..6767a4c283f 100644 --- a/src/extension.c +++ b/src/extension.c @@ -353,3 +353,20 @@ ts_extension_is_proxy_table_relid(Oid relid) { return relid == extension_proxy_oid; } + +#ifdef TS_DEBUG +static const char *extstate_str[] = { + [EXTENSION_STATE_UNKNOWN] = "unknown", + [EXTENSION_STATE_TRANSITIONING] = "transitioning", + [EXTENSION_STATE_CREATED] = "created", + [EXTENSION_STATE_NOT_INSTALLED] = "not installed", +}; + +TS_FUNCTION_INFO_V1(ts_extension_get_state); + +Datum +ts_extension_get_state(PG_FUNCTION_ARGS) +{ + PG_RETURN_TEXT_P(cstring_to_text(extstate_str[extstate])); +} +#endif diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e4bd594eab0..55725a74079 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -75,6 +75,24 @@ elseif(REQUIRE_ALL_TESTS) "All tests were required but 'pg_isolation_regress' could not be found") endif() +if(TAP_CHECKS) + add_custom_target( + provecheck + COMMAND rm -rf ${CMAKE_CURRENT_BINARY_DIR}/tmp_check + COMMAND + CONFDIR=${CMAKE_BINARY_DIR}/test PATH="${PG_BINDIR}:$ENV{PATH}" + PG_REGRESS=${PG_REGRESS} SRC_DIR=${PG_SOURCE_DIR} + CM_SRC_DIR=${CMAKE_SOURCE_DIR} PG_LIBDIR=${PG_LIBDIR} + PG_PKGLIBDIR=${PG_PKGLIBDIR} ${PRIMARY_TEST_DIR}/pg_prove.sh + USES_TERMINAL) + list(APPEND _install_checks provecheck) +elseif(REQUIRE_ALL_TESTS) + message( + FATAL_ERROR + "All tests were required but TAP_CHECKS was off (see previous messages why)" + ) +endif() + # We add the installcheck target even when _install_checks is empty as tsl code # might add dependencies to it even when regress checks are disabled. add_custom_target(installcheck DEPENDS ${_install_checks}) @@ -94,6 +112,7 @@ add_custom_target(installchecklocal DEPENDS ${_local_install_checks}) add_subdirectory(sql) add_subdirectory(isolation) +add_subdirectory(t) if(PG_SOURCE_DIR) add_subdirectory(pgtest) diff --git a/test/pg_prove.sh b/test/pg_prove.sh index 8044e84a213..1376f868ee5 100755 --- a/test/pg_prove.sh +++ b/test/pg_prove.sh @@ -25,14 +25,21 @@ then then exit 0 fi - FINAL_TESTS="t/*.pl" + FINAL_TESTS=$(ls -1 t/*.pl 2>/dev/null) else FINAL_TESTS=$PROVE_TESTS fi +if [ -z "$FINAL_TESTS" ] +then + echo "No TAP tests to run for the current configuration, skipping..." + exit 0; +fi + ${PROVE} \ -I "${SRC_DIR}/src/test/perl" \ -I "${CM_SRC_DIR}/test/perl" \ -I "${PG_LIBDIR}/pgxs/src/test/perl" \ + -I "${PG_PKGLIBDIR}/pgxs/src/test/perl" \ -I "${PG_LIBDIR}/postgresql/pgxs/src/test/perl" \ $FINAL_TESTS diff --git a/test/t/001_extension.pl b/test/t/001_extension.pl new file mode 100644 index 00000000000..b1fae97522b --- /dev/null +++ b/test/t/001_extension.pl @@ -0,0 +1,99 @@ +# This file and its contents are licensed under the Timescale License. +# Please see the included NOTICE for copyright information and +# LICENSE-TIMESCALE for a copy of the license. + +use strict; +use warnings; +use TimescaleNode; +use TestLib; +use Data::Dumper; +use Test::More tests => 6; + +# This test checks that the extension state is handled correctly +# across multiple sessions. Specifically, if the extension state +# changes in one session (e.g., the extension is created or dropped), +# this should be reflected in other concurrent sessions. +# +# To test this, we start one background psql session that stays open +# for the duration of the tests and then change the extension state +# from other sessions. +my $node = TimescaleNode->create('extension'); +my $in = ''; +my $out = ''; +my $timer = IPC::Run::timeout(180); +my $h = + $node->background_psql('postgres', \$in, \$out, $timer, on_error_stop => 0); + +sub check_extension_state +{ + my ($pattern, $annotation) = @_; + + # report test failures from caller location + local $Test::Builder::Level = $Test::Builder::Level + 1; + + # reset output collector + $out = ""; + # restart per-command timer + $timer->start(5); + # send the data to be sent + $in .= "SELECT * FROM extension_state();\n"; + # wait ... + + pump $h until ($out =~ $pattern || $timer->is_expired); + my $okay = ($out =~ $pattern && !$timer->is_expired); + ok($okay, $annotation); + # for debugging, log actual output if it didn't match + local $Data::Dumper::Terse = 1; + local $Data::Dumper::Useqq = 1; + diag 'Actual output was ' . Dumper($out) . "Did not match \"$pattern\"\n" + if !$okay; + return; +} + +# Create extension_state function and check initial state +my $result = $node->safe_psql( + 'postgres', + q{ + \ir t/functions.sql + SELECT * FROM extension_state(); +}); + +# Initially, the state should be "created" in both sessions +is($result, qq/created/, "initial state is \"created\""); +check_extension_state(qr/created/, "initial state is \"created\""); + +# Drop the extension in one session, and the new state should be +# reflected in the other backend. +$result = $node->safe_psql( + 'postgres', q{ + DROP EXTENSION timescaledb; + SELECT * FROM extension_state(); +}); + +# After dropping the extension, the new state in both sessions should +# be "unknown" +is($result, qq/unknown/, "state after dropped is \"unknown\""); +check_extension_state(qr/unknown/, + "state is \"unknown\" after extension is dropped in other backend"); + +# Create the extension again, which should be reflected in both +# sessions. +$result = $node->safe_psql( + 'postgres', q{ + CREATE EXTENSION timescaledb; + SELECT * FROM extension_state(); +}); + +# After creating the extension again in one session, the other session +# should go back to "created" state. +is($result, qq/created/, "state after creating extension is \"created\""); +check_extension_state(qr/created/, + "state is \"created\" after extension is created in other backend"); + +# Quit the interactive psql session +$in .= q{ + \q +}; + +$h->finish or die "psql returned $?"; +$node->stop; diff --git a/test/t/CMakeLists.txt b/test/t/CMakeLists.txt new file mode 100644 index 00000000000..fbaae295cd1 --- /dev/null +++ b/test/t/CMakeLists.txt @@ -0,0 +1,12 @@ +set(PROVE_DEBUG_TEST_FILES 001_extension.pl) + +if(CMAKE_BUILD_TYPE MATCHES Debug) + list(APPEND PROVE_TEST_FILES ${PROVE_DEBUG_TEST_FILES}) +endif(CMAKE_BUILD_TYPE MATCHES Debug) + +foreach(P_FILE ${PROVE_TEST_FILES}) + configure_file(${P_FILE} ${CMAKE_CURRENT_BINARY_DIR}/${P_FILE} COPYONLY) +endforeach(P_FILE) + +set(MODULE_PATHNAME "$libdir/timescaledb-${PROJECT_VERSION_MOD}") +configure_file(functions.sql.in functions.sql) diff --git a/test/t/functions.sql.in b/test/t/functions.sql.in new file mode 100644 index 00000000000..892fdc5bc45 --- /dev/null +++ b/test/t/functions.sql.in @@ -0,0 +1,6 @@ +-- This file and its contents are licensed under the Apache License 2.0. +-- Please see the included NOTICE for copyright information and +-- LICENSE-APACHE for a copy of the license. + +CREATE FUNCTION extension_state() RETURNS TEXT +AS '@MODULE_PATHNAME@', 'ts_extension_get_state' LANGUAGE C; diff --git a/tsl/test/CMakeLists.txt b/tsl/test/CMakeLists.txt index e8dc3884cf8..b9d763e71f1 100644 --- a/tsl/test/CMakeLists.txt +++ b/tsl/test/CMakeLists.txt @@ -76,7 +76,7 @@ endif() if(TAP_CHECKS) add_custom_target( - provecheck + provecheck-t COMMAND rm -rf ${CMAKE_CURRENT_BINARY_DIR}/tmp_check COMMAND CONFDIR=${CMAKE_BINARY_DIR}/tsl/test PATH="${PG_BINDIR}:$ENV{PATH}" @@ -84,7 +84,7 @@ if(TAP_CHECKS) CM_SRC_DIR=${CMAKE_SOURCE_DIR} PG_LIBDIR=${PG_LIBDIR} ${PRIMARY_TEST_DIR}/pg_prove.sh USES_TERMINAL) - list(APPEND _install_checks provecheck) + list(APPEND _install_checks provecheck-t) elseif(REQUIRE_ALL_TESTS) message( FATAL_ERROR