From 7caa4b58fbfa1fc0cc38f79a016f0664335cb1d2 Mon Sep 17 00:00:00 2001 From: Andrew Pogrebnoy Date: Wed, 2 Oct 2024 20:13:50 +0300 Subject: [PATCH] Remove empty TDE files from tablespace ... and rebase tests --- expected/tablespace.out | 42 ++++++++++++++++++++++++++++++- expected/tablespace_basic.out | 41 ++++++++++++++++++++++++++++++ meson.build | 1 + sql/tablespace.inc | 26 +++++++++++++++++++ sql/tablespace.sql | 28 ++------------------- sql/tablespace_basic.sql | 2 ++ src/access/pg_tde_tdemap.c | 35 ++++++++++++++++++-------- src/transam/pg_tde_xact_handler.c | 6 +++++ src16/access/pg_tdeam_handler.c | 2 +- src17/access/pg_tdeam_handler.c | 2 +- 10 files changed, 145 insertions(+), 40 deletions(-) create mode 100644 expected/tablespace_basic.out create mode 100644 sql/tablespace.inc create mode 100644 sql/tablespace_basic.sql diff --git a/expected/tablespace.out b/expected/tablespace.out index 0ffdd02f..52448e20 100644 --- a/expected/tablespace.out +++ b/expected/tablespace.out @@ -1 +1,41 @@ -// TODO \ No newline at end of file +\set tde_am tde_heap +\i sql/tablespace.inc +CREATE EXTENSION pg_tde; +SELECT pg_tde_add_key_provider_file('file-vault','/tmp/pg_tde_test_keyring.per'); + pg_tde_add_key_provider_file +------------------------------ + 1 +(1 row) + +SELECT pg_tde_set_principal_key('test-db-principal-key','file-vault'); + pg_tde_set_principal_key +-------------------------- + t +(1 row) + +CREATE TABLE test(num1 bigint, num2 double precision, t text) USING :tde_am; +INSERT INTO test(num1, num2, t) + SELECT round(random()*100), random(), 'text' + FROM generate_series(1, 10) s(i); +CREATE INDEX test_idx ON test(num1); +SET allow_in_place_tablespaces = true; +CREATE TABLESPACE test_tblspace LOCATION ''; +ALTER TABLE test SET TABLESPACE test_tblspace; +SELECT count(*) FROM test; + count +------- + 10 +(1 row) + +ALTER TABLE test SET TABLESPACE pg_default; +REINDEX (TABLESPACE test_tblspace, CONCURRENTLY) TABLE test; +INSERT INTO test VALUES (110, 2); +SELECT * FROM test WHERE num1=110; + num1 | num2 | t +------+------+--- + 110 | 2 | +(1 row) + +DROP TABLE test; +DROP TABLESPACE test_tblspace; +DROP EXTENSION pg_tde; diff --git a/expected/tablespace_basic.out b/expected/tablespace_basic.out new file mode 100644 index 00000000..6718dc5a --- /dev/null +++ b/expected/tablespace_basic.out @@ -0,0 +1,41 @@ +\set tde_am tde_heap_basic +\i sql/tablespace.inc +CREATE EXTENSION pg_tde; +SELECT pg_tde_add_key_provider_file('file-vault','/tmp/pg_tde_test_keyring.per'); + pg_tde_add_key_provider_file +------------------------------ + 1 +(1 row) + +SELECT pg_tde_set_principal_key('test-db-principal-key','file-vault'); + pg_tde_set_principal_key +-------------------------- + t +(1 row) + +CREATE TABLE test(num1 bigint, num2 double precision, t text) USING :tde_am; +INSERT INTO test(num1, num2, t) + SELECT round(random()*100), random(), 'text' + FROM generate_series(1, 10) s(i); +CREATE INDEX test_idx ON test(num1); +SET allow_in_place_tablespaces = true; +CREATE TABLESPACE test_tblspace LOCATION ''; +ALTER TABLE test SET TABLESPACE test_tblspace; +SELECT count(*) FROM test; + count +------- + 10 +(1 row) + +ALTER TABLE test SET TABLESPACE pg_default; +REINDEX (TABLESPACE test_tblspace, CONCURRENTLY) TABLE test; +INSERT INTO test VALUES (110, 2); +SELECT * FROM test WHERE num1=110; + num1 | num2 | t +------+------+--- + 110 | 2 | +(1 row) + +DROP TABLE test; +DROP TABLESPACE test_tblspace; +DROP EXTENSION pg_tde; diff --git a/meson.build b/meson.build index eff9cd13..f365eff0 100644 --- a/meson.build +++ b/meson.build @@ -96,6 +96,7 @@ sql_tests = [ 'trigger_on_view_basic', 'change_access_method_basic', 'insert_update_delete_basic', + 'tablespace_basic', 'vault_v2_test_basic', ] diff --git a/sql/tablespace.inc b/sql/tablespace.inc new file mode 100644 index 00000000..b2f7ebdd --- /dev/null +++ b/sql/tablespace.inc @@ -0,0 +1,26 @@ +CREATE EXTENSION pg_tde; + +SELECT pg_tde_add_key_provider_file('file-vault','/tmp/pg_tde_test_keyring.per'); +SELECT pg_tde_set_principal_key('test-db-principal-key','file-vault'); + +CREATE TABLE test(num1 bigint, num2 double precision, t text) USING :tde_am; +INSERT INTO test(num1, num2, t) + SELECT round(random()*100), random(), 'text' + FROM generate_series(1, 10) s(i); +CREATE INDEX test_idx ON test(num1); + +SET allow_in_place_tablespaces = true; +CREATE TABLESPACE test_tblspace LOCATION ''; + +ALTER TABLE test SET TABLESPACE test_tblspace; +SELECT count(*) FROM test; +ALTER TABLE test SET TABLESPACE pg_default; + +REINDEX (TABLESPACE test_tblspace, CONCURRENTLY) TABLE test; +INSERT INTO test VALUES (110, 2); + +SELECT * FROM test WHERE num1=110; + +DROP TABLE test; +DROP TABLESPACE test_tblspace; +DROP EXTENSION pg_tde; diff --git a/sql/tablespace.sql b/sql/tablespace.sql index b7517991..0d3aa620 100644 --- a/sql/tablespace.sql +++ b/sql/tablespace.sql @@ -1,26 +1,2 @@ -CREATE EXTENSION pg_tde; - -SELECT pg_tde_add_key_provider_file('file-vault','/tmp/pg_tde_test_keyring.per'); -SELECT pg_tde_set_principal_key('test-db-principal-key','file-vault'); - -CREATE TABLE test(num1 bigint, num2 double precision, t text) using tde_heap_basic;; -INSERT INTO test(num1, num2, t) - SELECT round(random()*100), random(), 'text' - FROM generate_series(1, 10) s(i); -CREATE INDEX test_idx ON test(num1); - -SET allow_in_place_tablespaces = true; -CREATE TABLESPACE test_tblspace LOCATION ''; - -ALTER TABLE test SET TABLESPACE test_tblspace; -SELECT count(*) FROM test; -ALTER TABLE test SET TABLESPACE pg_default; - -REINDEX (TABLESPACE test_tblspace, CONCURRENTLY) TABLE test; -INSERT INTO test VALUES (110, 2); - -SELECT * FROM test WHERE num1=110; - -DROP TABLE test; -DROP TABLESPACE test_tblspace; -DROP EXTENSION pg_tde; +\set tde_am tde_heap +\i sql/tablespace.inc \ No newline at end of file diff --git a/sql/tablespace_basic.sql b/sql/tablespace_basic.sql new file mode 100644 index 00000000..0e358723 --- /dev/null +++ b/sql/tablespace_basic.sql @@ -0,0 +1,2 @@ +\set tde_am tde_heap_basic +\i sql/tablespace.inc \ No newline at end of file diff --git a/src/access/pg_tde_tdemap.c b/src/access/pg_tde_tdemap.c index ec4f0635..68cb4750 100644 --- a/src/access/pg_tde_tdemap.c +++ b/src/access/pg_tde_tdemap.c @@ -561,24 +561,23 @@ pg_tde_delete_key_map_entry(const RelFileLocator *rlocator) * * The offset allows us to simply seek to the desired location and mark the entry * as MAP_ENTRY_FREE without needing any further processing. + * + * A caller should hold an EXCLUSIVE tde_lwlock_enc_keys lock. */ void pg_tde_free_key_map_entry(const RelFileLocator *rlocator, off_t offset) { int32 key_index = 0; - LWLock *lock_files = tde_lwlock_enc_keys(); char db_map_path[MAXPGPATH] = {0}; - char db_keydata_path[MAXPGPATH] = {0}; + off_t start = 0; Assert(rlocator); /* Get the file paths */ - pg_tde_set_db_file_paths(rlocator, db_map_path, db_keydata_path); + pg_tde_set_db_file_paths(rlocator, db_map_path, NULL); /* Remove the map entry if found */ - LWLockAcquire(lock_files, LW_EXCLUSIVE); key_index = pg_tde_process_map_entry(rlocator, db_map_path, &offset, true); - LWLockRelease(lock_files); if (key_index == -1) { @@ -588,7 +587,16 @@ pg_tde_free_key_map_entry(const RelFileLocator *rlocator, off_t offset) rlocator->relNumber, db_map_path))); - return; + } + /* + * Remove TDE files it was the last TDE relation in a custom tablespace. + * DROP TABLESPACE needs an empty dir. + */ + if (rlocator->spcOid != GLOBALTABLESPACE_OID && + rlocator->spcOid != DEFAULTTABLESPACE_OID && + pg_tde_process_map_entry(NULL, db_map_path, &start, false) == -1) + { + pg_tde_delete_tde_files(rlocator->dbOid, rlocator->spcOid); } } @@ -843,16 +851,23 @@ pg_tde_move_rel_key(const RelFileLocator *newrlocator, const RelFileLocator *old RelKeyData *rel_key; RelKeyData *enc_key; TDEPrincipalKey *principal_key; - XLogRelKey xlrec; - LWLock *lock_pk = tde_lwlock_enc_keys(); + XLogRelKey xlrec; + char db_map_path[MAXPGPATH] = {0}; + off_t offset = 0; - LWLockAcquire(lock_pk, LW_EXCLUSIVE); + LWLockAcquire(tde_lwlock_enc_keys(), LW_EXCLUSIVE); + principal_key = GetPrincipalKey(oldrlocator->dbOid, oldrlocator->spcOid, LW_EXCLUSIVE); rel_key = GetRelationKey(*oldrlocator); Assert(rel_key); enc_key = tde_encrypt_rel_key(principal_key, rel_key, newrlocator); pg_tde_write_key_map_entry(newrlocator, enc_key, &principal_key->keyInfo); pg_tde_put_key_into_cache(newrlocator->relNumber, rel_key); + + pg_tde_free_key_map_entry(oldrlocator, offset); + + LWLockRelease(tde_lwlock_enc_keys()); + xlrec.rlocator = *newrlocator; xlrec.relKey = *enc_key; @@ -860,7 +875,6 @@ pg_tde_move_rel_key(const RelFileLocator *newrlocator, const RelFileLocator *old XLogRegisterData((char *) &xlrec, sizeof(xlrec)); XLogInsert(RM_TDERMGR_ID, XLOG_TDE_ADD_RELATION_KEY); - LWLockRelease(lock_pk); pfree(enc_key); } @@ -1209,7 +1223,6 @@ pg_tde_read_one_map_entry(File map_file, const RelFileLocator *rlocator, int fla return found; } - /* * Reads a single keydata from the file. */ diff --git a/src/transam/pg_tde_xact_handler.c b/src/transam/pg_tde_xact_handler.c index 2d2ce758..5afb9b39 100644 --- a/src/transam/pg_tde_xact_handler.c +++ b/src/transam/pg_tde_xact_handler.c @@ -101,6 +101,8 @@ do_pending_deletes(bool isCommit) PendingMapEntryDelete *prev; PendingMapEntryDelete *next; + LWLockAcquire(tde_lwlock_enc_keys(), LW_EXCLUSIVE); + prev = NULL; for (pending = pendingDeletes; pending != NULL; pending = next) { @@ -129,6 +131,8 @@ do_pending_deletes(bool isCommit) /* prev does not change */ } + + LWLockRelease(tde_lwlock_enc_keys()); } @@ -153,6 +157,8 @@ reassign_pending_deletes_to_parent_xact(void) if (pending->nestLevel == nestLevel) pending->nestLevel--; } + + LWLockRelease(tde_lwlock_enc_keys()); } /* diff --git a/src16/access/pg_tdeam_handler.c b/src16/access/pg_tdeam_handler.c index 14803347..fc084f3e 100644 --- a/src16/access/pg_tdeam_handler.c +++ b/src16/access/pg_tdeam_handler.c @@ -707,7 +707,7 @@ pg_tdeam_relation_copy_data(Relation rel, const RelFileLocator *newrlocator) } } - pg_tde_move_rel_key(newrlocator, rel->rd_locator); + pg_tde_move_rel_key(newrlocator, &rel->rd_locator); /* drop old relation, and close new one */ RelationDropStorage(rel); diff --git a/src17/access/pg_tdeam_handler.c b/src17/access/pg_tdeam_handler.c index d8f7c278..2ba95b3e 100644 --- a/src17/access/pg_tdeam_handler.c +++ b/src17/access/pg_tdeam_handler.c @@ -702,7 +702,7 @@ pg_tdeam_relation_copy_data(Relation rel, const RelFileLocator *newrlocator) } } - pg_tde_move_rel_key(newrlocator, rel->rd_locator); + pg_tde_move_rel_key(newrlocator, &rel->rd_locator); /* drop old relation, and close new one */ RelationDropStorage(rel);