Skip to content

Commit

Permalink
Fix error for exported_uuid in pg_restore
Browse files Browse the repository at this point in the history
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 timescale#1409.
  • Loading branch information
cevian committed Oct 30, 2019
1 parent 0cf0e11 commit 3ee9eb6
Show file tree
Hide file tree
Showing 8 changed files with 138 additions and 16 deletions.
27 changes: 18 additions & 9 deletions sql/restoring.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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;
$BODY$
LANGUAGE PLPGSQL;
17 changes: 17 additions & 0 deletions test/expected/pg_dump.out
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions test/sql/pg_dump.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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');
Expand Down
2 changes: 1 addition & 1 deletion test/sql/utils/pg_dump_aux_restore.sh
Original file line number Diff line number Diff line change
@@ -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
60 changes: 60 additions & 0 deletions tsl/test/expected/compression_hypertable.out
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,16 @@ 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.
timescaledb_pre_restore
-------------------------
t
(1 row)

timescaledb_post_restore
--------------------------
t
(1 row)

?column? | count
-----------------------------------------------------------------------------------+-------
Number of rows different between original and query on compressed data (expect 0) | 0
Expand Down Expand Up @@ -227,6 +237,16 @@ 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.
timescaledb_pre_restore
-------------------------
t
(1 row)

timescaledb_post_restore
--------------------------
t
(1 row)

?column? | count
-----------------------------------------------------------------------------------+-------
Number of rows different between original and query on compressed data (expect 0) | 0
Expand Down Expand Up @@ -338,6 +358,16 @@ 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.
timescaledb_pre_restore
-------------------------
t
(1 row)

timescaledb_post_restore
--------------------------
t
(1 row)

?column? | count
-----------------------------------------------------------------------------------+-------
Number of rows different between original and query on compressed data (expect 0) | 0
Expand Down Expand Up @@ -449,6 +479,16 @@ 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.
timescaledb_pre_restore
-------------------------
t
(1 row)

timescaledb_post_restore
--------------------------
t
(1 row)

?column? | count
-----------------------------------------------------------------------------------+-------
Number of rows different between original and query on compressed data (expect 0) | 0
Expand Down Expand Up @@ -528,6 +568,16 @@ 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.
timescaledb_pre_restore
-------------------------
t
(1 row)

timescaledb_post_restore
--------------------------
t
(1 row)

?column? | count
-----------------------------------------------------------------------------------+-------
Number of rows different between original and query on compressed data (expect 0) | 0
Expand Down Expand Up @@ -602,6 +652,16 @@ 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.
timescaledb_pre_restore
-------------------------
t
(1 row)

timescaledb_post_restore
--------------------------
t
(1 row)

?column? | count
-----------------------------------------------------------------------------------+-------
Number of rows different between original and query on compressed data (expect 0) | 0
Expand Down
18 changes: 16 additions & 2 deletions tsl/test/expected/continuous_aggs_dump.out
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 9 additions & 2 deletions tsl/test/sql/continuous_aggs_dump.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
10 changes: 8 additions & 2 deletions tsl/test/sql/include/compression_test_hypertable.sql
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,16 @@ WHERE hypertable.table_name like :'HYPERTABLE_NAME' and chunk.compressed_chunk_i
\c postgres :ROLE_SUPERUSER
SET client_min_messages = ERROR;
\! 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

with original AS (
Expand Down

0 comments on commit 3ee9eb6

Please sign in to comment.