From 42b23bda942674f0ac1dfd587a982debc0a89dc8 Mon Sep 17 00:00:00 2001 From: Artem Gavrilov Date: Tue, 8 Oct 2024 15:04:05 +0200 Subject: [PATCH] PG-1093 Fix pending deletes for subtransactions (#302) * PG-1093 Fix pending deletes for subtransactions * PG-1093 Add dedicated test cases for tde_heap and tde_heap_basic --- Makefile.in | 1 + expected/subtransaction.out | 32 ++++++++++++++++ expected/subtransaction_basic.out | 32 ++++++++++++++++ sql/subtransaction.inc | 25 ++++++++++++ sql/subtransaction.sql | 2 + sql/subtransaction_basic.sql | 2 + src/access/pg_tde_tdemap.c | 1 + src/transam/pg_tde_xact_handler.c | 64 ++++++++++++++++++++++--------- 8 files changed, 141 insertions(+), 18 deletions(-) create mode 100644 expected/subtransaction.out create mode 100644 expected/subtransaction_basic.out create mode 100644 sql/subtransaction.inc create mode 100644 sql/subtransaction.sql create mode 100644 sql/subtransaction_basic.sql diff --git a/Makefile.in b/Makefile.in index c5c818f0..96366c83 100644 --- a/Makefile.in +++ b/Makefile.in @@ -14,6 +14,7 @@ update_compare_indexes_basic \ pg_tde_is_encrypted_basic \ test_issue_153_fix_basic \ multi_insert_basic \ +subtransaction_basic \ trigger_on_view_basic \ change_access_method_basic \ insert_update_delete_basic \ diff --git a/expected/subtransaction.out b/expected/subtransaction.out new file mode 100644 index 00000000..731382a2 --- /dev/null +++ b/expected/subtransaction.out @@ -0,0 +1,32 @@ +\set tde_am tde_heap +\i sql/subtransaction.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) + +BEGIN; -- Nesting level 1 +SAVEPOINT sp; +CREATE TABLE foo(s TEXT); -- Nesting level 2 +RELEASE SAVEPOINT sp; +SAVEPOINT sp; +CREATE TABLE bar(s TEXT); -- Nesting level 2 +ROLLBACK TO sp; -- Rollback should not affect first subtransaction +COMMIT; +BEGIN; -- Nesting level 1 +SAVEPOINT sp; +DROP TABLE foo; -- Nesting level 2 +RELEASE SAVEPOINT sp; +SAVEPOINT sp; +CREATE TABLE bar(s TEXT); -- Nesting level 2 +ROLLBACK TO sp; -- Rollback should not affect first subtransaction +COMMIT; +DROP EXTENSION pg_tde; diff --git a/expected/subtransaction_basic.out b/expected/subtransaction_basic.out new file mode 100644 index 00000000..d84651ef --- /dev/null +++ b/expected/subtransaction_basic.out @@ -0,0 +1,32 @@ +\set tde_am tde_heap_basic +\i sql/subtransaction.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) + +BEGIN; -- Nesting level 1 +SAVEPOINT sp; +CREATE TABLE foo(s TEXT); -- Nesting level 2 +RELEASE SAVEPOINT sp; +SAVEPOINT sp; +CREATE TABLE bar(s TEXT); -- Nesting level 2 +ROLLBACK TO sp; -- Rollback should not affect first subtransaction +COMMIT; +BEGIN; -- Nesting level 1 +SAVEPOINT sp; +DROP TABLE foo; -- Nesting level 2 +RELEASE SAVEPOINT sp; +SAVEPOINT sp; +CREATE TABLE bar(s TEXT); -- Nesting level 2 +ROLLBACK TO sp; -- Rollback should not affect first subtransaction +COMMIT; +DROP EXTENSION pg_tde; diff --git a/sql/subtransaction.inc b/sql/subtransaction.inc new file mode 100644 index 00000000..3446d44e --- /dev/null +++ b/sql/subtransaction.inc @@ -0,0 +1,25 @@ +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'); + + +BEGIN; -- Nesting level 1 +SAVEPOINT sp; +CREATE TABLE foo(s TEXT); -- Nesting level 2 +RELEASE SAVEPOINT sp; +SAVEPOINT sp; +CREATE TABLE bar(s TEXT); -- Nesting level 2 +ROLLBACK TO sp; -- Rollback should not affect first subtransaction +COMMIT; + +BEGIN; -- Nesting level 1 +SAVEPOINT sp; +DROP TABLE foo; -- Nesting level 2 +RELEASE SAVEPOINT sp; +SAVEPOINT sp; +CREATE TABLE bar(s TEXT); -- Nesting level 2 +ROLLBACK TO sp; -- Rollback should not affect first subtransaction +COMMIT; + +DROP EXTENSION pg_tde; \ No newline at end of file diff --git a/sql/subtransaction.sql b/sql/subtransaction.sql new file mode 100644 index 00000000..ce4a442e --- /dev/null +++ b/sql/subtransaction.sql @@ -0,0 +1,2 @@ +\set tde_am tde_heap +\i sql/subtransaction.inc \ No newline at end of file diff --git a/sql/subtransaction_basic.sql b/sql/subtransaction_basic.sql new file mode 100644 index 00000000..c7800c44 --- /dev/null +++ b/sql/subtransaction_basic.sql @@ -0,0 +1,2 @@ +\set tde_am tde_heap_basic +\i sql/subtransaction.inc \ No newline at end of file diff --git a/src/access/pg_tde_tdemap.c b/src/access/pg_tde_tdemap.c index 9a9b79d5..abe2bff9 100644 --- a/src/access/pg_tde_tdemap.c +++ b/src/access/pg_tde_tdemap.c @@ -530,6 +530,7 @@ pg_tde_delete_key_map_entry(const RelFileLocator *rlocator) /* Get the file paths */ pg_tde_set_db_file_paths(rlocator, db_map_path, db_keydata_path); + errno = 0; /* Remove the map entry if found */ LWLockAcquire(lock_files, LW_EXCLUSIVE); key_index = pg_tde_process_map_entry(rlocator, db_map_path, &offset, false); diff --git a/src/transam/pg_tde_xact_handler.c b/src/transam/pg_tde_xact_handler.c index 4b0576a0..2d2ce758 100644 --- a/src/transam/pg_tde_xact_handler.c +++ b/src/transam/pg_tde_xact_handler.c @@ -30,6 +30,7 @@ typedef struct PendingMapEntryDelete static PendingMapEntryDelete *pendingDeletes = NULL; /* head of linked list */ static void do_pending_deletes(bool isCommit); +static void reassign_pending_deletes_to_parent_xact(void); static void pending_delete_cleanup(void); /* Transaction Callbacks from Backend*/ @@ -64,6 +65,11 @@ pg_tde_subxact_callback(SubXactEvent event, SubTransactionId mySubid, ereport(DEBUG2, (errmsg("pg_tde_subxact_callback: aborting subtransaction"))); do_pending_deletes(false); + } else if (event == SUBXACT_EVENT_COMMIT_SUB) + { + ereport(DEBUG2, + (errmsg("pg_tde_subxact_callback: committing subtransaction"))); + reassign_pending_deletes_to_parent_xact(); } } @@ -99,31 +105,53 @@ do_pending_deletes(bool isCommit) for (pending = pendingDeletes; pending != NULL; pending = next) { next = pending->next; - if (pending->nestLevel < nestLevel) + if (pending->nestLevel != nestLevel) { /* outer-level entries should not be processed yet */ prev = pending; + continue; } + + /* unlink list entry first, so we don't retry on failure */ + if (prev) + prev->next = next; else + pendingDeletes = next; + /* do deletion if called for */ + if (pending->atCommit == isCommit) { - /* unlink list entry first, so we don't retry on failure */ - if (prev) - prev->next = next; - else - pendingDeletes = next; - /* do deletion if called for */ - if (pending->atCommit == isCommit) - { - ereport(LOG, - (errmsg("pg_tde_xact_callback: deleting entry at offset %d", - (int)(pending->map_entry_offset)))); - - pg_tde_free_key_map_entry(&pending->rlocator, pending->map_entry_offset); - } - - pfree(pending); - /* prev does not change */ + ereport(LOG, + (errmsg("pg_tde_xact_callback: deleting entry at offset %d", + (int)(pending->map_entry_offset)))); + pg_tde_free_key_map_entry(&pending->rlocator, pending->map_entry_offset); } + pfree(pending); + /* prev does not change */ + + } +} + + +/* + * reassign_pending_deletes_to_parent_xact() -- Adjust nesting level of pending deletes. + * + * There are several cases to consider: + * 1. Only top level transaction can perform on-commit deletes. + * 2. Subtransaction and top level transaction can perform on-abort deletes. + * So we have to decrement the nesting level of pending deletes to reassing them to the parent transaction + * if subtransaction was not self aborted. In other words if subtransaction state is commited all its pending + * deletes are reassigned to the parent transaction. + */ +static void +reassign_pending_deletes_to_parent_xact(void) +{ + PendingMapEntryDelete *pending; + int nestLevel = GetCurrentTransactionNestLevel(); + + for (pending = pendingDeletes; pending != NULL; pending = pending->next) + { + if (pending->nestLevel == nestLevel) + pending->nestLevel--; } }