From 83158f74d3ab8dd48aed92e84b6207562992d13d Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 14 Sep 2020 13:56:41 +0900 Subject: [PATCH] Make index_set_state_flags() transactional 3c84046 is the original commit that introduced index_set_state_flags(), where the presence of SnapshotNow made necessary the use of an in-place update. SnapshotNow has been removed in 813fb03, so there is no actual reasons to not make this operation transactional. Note that while making the operation more robust, using a transactional operation in this routine was not strictly necessary as there was no use case for it yet. However, some future features are going to need a transactional behavior, like support for CREATE/DROP INDEX CONCURRENTLY with partitioned tables, where indexes in a partition tree need to have all their pg_index.indis* flags updated in the same transaction to make the operation stable to the end-user by keeping partition trees consistent, even with a failure mid-flight. REINDEX CONCURRENTLY uses already transactional updates when swapping the old and new indexes, making this change more consistent with the index-swapping logic. Author: Michael Paquier Reviewed-by: Anastasia Lubennikova Discussion: https://postgr.es/m/20200903080440.GA8559@paquier.xyz --- src/backend/catalog/index.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 117e3fdef7..0974f3e23a 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -3311,18 +3311,10 @@ validate_index_callback(ItemPointer itemptr, void *opaque) * index_set_state_flags - adjust pg_index state flags * * This is used during CREATE/DROP INDEX CONCURRENTLY to adjust the pg_index - * flags that denote the index's state. Because the update is not - * transactional and will not roll back on error, this must only be used as - * the last step in a transaction that has not made any transactional catalog - * updates! + * flags that denote the index's state. * - * Note that heap_inplace_update does send a cache inval message for the + * Note that CatalogTupleUpdate() sends a cache invalidation message for the * tuple, so other sessions will hear about the update as soon as we commit. - * - * NB: In releases prior to PostgreSQL 9.4, the use of a non-transactional - * update here would have been unsafe; now that MVCC rules apply even for - * system catalog scans, we could potentially use a transactional update here - * instead. */ void index_set_state_flags(Oid indexId, IndexStateFlagsAction action) @@ -3331,9 +3323,6 @@ index_set_state_flags(Oid indexId, IndexStateFlagsAction action) HeapTuple indexTuple; Form_pg_index indexForm; - /* Assert that current xact hasn't done any transactional updates */ - Assert(GetTopTransactionIdIfAny() == InvalidTransactionId); - /* Open pg_index and fetch a writable copy of the index's tuple */ pg_index = table_open(IndexRelationId, RowExclusiveLock); @@ -3397,8 +3386,8 @@ index_set_state_flags(Oid indexId, IndexStateFlagsAction action) break; } - /* ... and write it back in-place */ - heap_inplace_update(pg_index, indexTuple); + /* ... and update it */ + CatalogTupleUpdate(pg_index, &indexTuple->t_self, indexTuple); table_close(pg_index, RowExclusiveLock); }