From 4330aa963ac9ac17c895d92210ae756cb81eb45f Mon Sep 17 00:00:00 2001 From: Matvey Arye Date: Sun, 27 Oct 2019 17:02:20 -0400 Subject: [PATCH] Fix error for exported_uuid in pg_restore When restoring a database, people would encounter errors if the restore happened after telemetry has run. This is because a 'exported_uuid' field would then exist and people would encounter a "duplicate key value" when the restore tried to overwrite it. We fix this by moving this metadata to a different key in pre_restore and trying to move it back in post_restore. If the restore create an exported_uuid, that restored value is used and the moved version is simply deleted We also remove the error redirection in restore so that errors will show up in tests in the future. Fixes #1409. --- sql/restoring.sql | 27 ++++++++++++++-------- test/expected/pg_dump.out | 17 ++++++++++++++ test/sql/pg_dump.sql | 9 ++++++++ test/sql/utils/pg_dump_aux_restore.sh | 2 +- tsl/test/expected/continuous_aggs_dump.out | 18 +++++++++++++-- tsl/test/sql/continuous_aggs_dump.sql | 11 +++++++-- 6 files changed, 70 insertions(+), 14 deletions(-) diff --git a/sql/restoring.sql b/sql/restoring.sql index faad0175c56..70c0b442c5b 100644 --- a/sql/restoring.sql +++ b/sql/restoring.sql @@ -2,31 +2,40 @@ -- Please see the included NOTICE for copyright information and -- LICENSE-APACHE for a copy of the license. -CREATE OR REPLACE FUNCTION timescaledb_pre_restore() RETURNS BOOL AS +CREATE OR REPLACE FUNCTION timescaledb_pre_restore() RETURNS BOOL AS $BODY$ -DECLARE - db text; +DECLARE + db text; BEGIN SELECT current_database() INTO db; EXECUTE format($$ALTER DATABASE %I SET timescaledb.restoring ='on'$$, db); SET SESSION timescaledb.restoring = 'on'; PERFORM _timescaledb_internal.stop_background_workers(); + --exported uuid may be included in the dump so backup the version + UPDATE _timescaledb_catalog.metadata SET key='exported_uuid_bak' WHERE key='exported_uuid'; RETURN true; END -$BODY$ +$BODY$ LANGUAGE PLPGSQL; -CREATE OR REPLACE FUNCTION timescaledb_post_restore() RETURNS BOOL AS +CREATE OR REPLACE FUNCTION timescaledb_post_restore() RETURNS BOOL AS $BODY$ -DECLARE - db text; +DECLARE + db text; BEGIN SELECT current_database() INTO db; EXECUTE format($$ALTER DATABASE %I SET timescaledb.restoring ='off'$$, db); SET SESSION timescaledb.restoring='off'; PERFORM _timescaledb_internal.start_background_workers(); + + --try to restore the backed up uuid, if the restore did not set one + INSERT INTO _timescaledb_catalog.metadata + SELECT 'exported_uuid', value, include_in_telemetry FROM _timescaledb_catalog.metadata WHERE key='exported_uuid_bak' + ON CONFLICT DO NOTHING; + DELETE FROM _timescaledb_catalog.metadata WHERE key='exported_uuid_bak'; + RETURN true; END -$BODY$ -LANGUAGE PLPGSQL; \ No newline at end of file +$BODY$ +LANGUAGE PLPGSQL; diff --git a/test/expected/pg_dump.out b/test/expected/pg_dump.out index 1908f2f25b7..e6855eb6512 100644 --- a/test/expected/pg_dump.out +++ b/test/expected/pg_dump.out @@ -257,6 +257,8 @@ SELECT * FROM _timescaledb_catalog.chunk_constraint; 4 | | 4_4_timecustom_device_id_series_2_key | timecustom_device_id_series_2_key (12 rows) +--force a value to exist for exported_uuid +INSERT INTO _timescaledb_catalog.metadata VALUES ('exported_uuid', 'original_uuid', true); \c postgres :ROLE_SUPERUSER -- We shell out to a script in order to grab the correct hostname from the -- environmental variables that originally called this psql command. Sadly @@ -273,6 +275,8 @@ pg_dump: Consider using a full dump instead of a --data-only dump to avoid this \c :TEST_DBNAME SET client_min_messages = ERROR; CREATE EXTENSION timescaledb CASCADE; +--create a exported uuid before restoring (mocks telemetry running before restore) +INSERT INTO _timescaledb_catalog.metadata VALUES ('exported_uuid', 'new_db_uuid', true); RESET client_min_messages; SELECT timescaledb_pre_restore(); timescaledb_pre_restore @@ -318,6 +322,19 @@ SELECT count(*) = :num_dependent_objects as dependent_objects_match t (1 row) +--we should have the original uuid from the backed up db set as the exported_uuid +SELECT value = 'original_uuid' FROM _timescaledb_catalog.metadata WHERE key='exported_uuid'; + ?column? +---------- + t +(1 row) + +SELECT count(*) = 1 FROM _timescaledb_catalog.metadata WHERE key LIKE 'exported%'; + ?column? +---------- + t +(1 row) + --main table and chunk schemas should be the same SELECT * FROM test.show_columns('"test_schema"."two_Partitions"'); Column | Type | NotNull diff --git a/test/sql/pg_dump.sql b/test/sql/pg_dump.sql index ed308cbe63e..4731d0fbc46 100644 --- a/test/sql/pg_dump.sql +++ b/test/sql/pg_dump.sql @@ -73,6 +73,9 @@ SELECT * FROM _timescaledb_internal._hyper_1_2_chunk ORDER BY "timeCustom", devi SELECT * FROM _timescaledb_catalog.chunk_index; SELECT * FROM _timescaledb_catalog.chunk_constraint; +--force a value to exist for exported_uuid +INSERT INTO _timescaledb_catalog.metadata VALUES ('exported_uuid', 'original_uuid', true); + \c postgres :ROLE_SUPERUSER -- We shell out to a script in order to grab the correct hostname from the @@ -83,6 +86,8 @@ SELECT * FROM _timescaledb_catalog.chunk_constraint; \c :TEST_DBNAME SET client_min_messages = ERROR; CREATE EXTENSION timescaledb CASCADE; +--create a exported uuid before restoring (mocks telemetry running before restore) +INSERT INTO _timescaledb_catalog.metadata VALUES ('exported_uuid', 'new_db_uuid', true); RESET client_min_messages; SELECT timescaledb_pre_restore(); SHOW timescaledb.restoring; @@ -108,6 +113,10 @@ SELECT count(*) = :num_dependent_objects as dependent_objects_match WHERE refclassid = 'pg_extension'::regclass AND refobjid = (SELECT oid FROM pg_extension WHERE extname = 'timescaledb'); +--we should have the original uuid from the backed up db set as the exported_uuid +SELECT value = 'original_uuid' FROM _timescaledb_catalog.metadata WHERE key='exported_uuid'; +SELECT count(*) = 1 FROM _timescaledb_catalog.metadata WHERE key LIKE 'exported%'; + --main table and chunk schemas should be the same SELECT * FROM test.show_columns('"test_schema"."two_Partitions"'); SELECT * FROM test.show_columns('_timescaledb_internal._hyper_1_1_chunk'); diff --git a/test/sql/utils/pg_dump_aux_restore.sh b/test/sql/utils/pg_dump_aux_restore.sh index 074cebebd73..513fee78cc5 100755 --- a/test/sql/utils/pg_dump_aux_restore.sh +++ b/test/sql/utils/pg_dump_aux_restore.sh @@ -1,4 +1,4 @@ DUMPFILE=$1 # Redirect output to /dev/null to suppress NOTICE -${PG_BINDIR}/pg_restore -h ${PGHOST} -U ${TEST_ROLE_SUPERUSER} -d ${TEST_DBNAME} ${DUMPFILE} > /dev/null 2>&1 +${PG_BINDIR}/pg_restore -h ${PGHOST} -U ${TEST_ROLE_SUPERUSER} -d ${TEST_DBNAME} ${DUMPFILE} > /dev/null diff --git a/tsl/test/expected/continuous_aggs_dump.out b/tsl/test/expected/continuous_aggs_dump.out index 5491df4ebfe..231b5a9ba68 100644 --- a/tsl/test/expected/continuous_aggs_dump.out +++ b/tsl/test/expected/continuous_aggs_dump.out @@ -153,10 +153,24 @@ pg_dump: NOTICE: there are circular foreign-key constraints on this table: pg_dump: chunk pg_dump: You might not be able to restore the dump without using --disable-triggers or temporarily dropping the constraints. pg_dump: Consider using a full dump instead of a --data-only dump to avoid this problem. +\c :TEST_DBNAME +SET client_min_messages = ERROR; +CREATE EXTENSION timescaledb CASCADE; +RESET client_min_messages; --\! cp dump/pg_dump.sql /tmp/dump.sql -ALTER DATABASE :TEST_DBNAME SET timescaledb.restoring='on'; +SELECT timescaledb_pre_restore(); + timescaledb_pre_restore +------------------------- + t +(1 row) + \! utils/pg_dump_aux_restore.sh dump/pg_dump.sql -ALTER DATABASE :TEST_DBNAME SET timescaledb.restoring='off'; +SELECT timescaledb_post_restore(); + timescaledb_post_restore +-------------------------- + t +(1 row) + \c :TEST_DBNAME :ROLE_DEFAULT_PERM_USER --make sure the appropriate DROP are still blocked. \set ON_ERROR_STOP 0 diff --git a/tsl/test/sql/continuous_aggs_dump.sql b/tsl/test/sql/continuous_aggs_dump.sql index 934bece4a03..0b554e1fb0c 100644 --- a/tsl/test/sql/continuous_aggs_dump.sql +++ b/tsl/test/sql/continuous_aggs_dump.sql @@ -123,10 +123,17 @@ SELECT count(*) FROM conditions_after; --dump & restore \c postgres :ROLE_SUPERUSER \! utils/pg_dump_aux_dump.sh dump/pg_dump.sql + +\c :TEST_DBNAME +SET client_min_messages = ERROR; +CREATE EXTENSION timescaledb CASCADE; +RESET client_min_messages; + --\! cp dump/pg_dump.sql /tmp/dump.sql -ALTER DATABASE :TEST_DBNAME SET timescaledb.restoring='on'; +SELECT timescaledb_pre_restore(); \! utils/pg_dump_aux_restore.sh dump/pg_dump.sql -ALTER DATABASE :TEST_DBNAME SET timescaledb.restoring='off'; +SELECT timescaledb_post_restore(); + \c :TEST_DBNAME :ROLE_DEFAULT_PERM_USER --make sure the appropriate DROP are still blocked.