opensourcemirror
/
postgresql
mirror of git://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Releases Wiki Activity

Handle interrupts within a transaction context in REINDEX CONCURRENTLY 

Phases 2 (building the new index) and 3 (validating the new index)
checked for interrupts outside a transaction context, having as
consequence to not release session-level locks taken on the parent
relation and the old and new indexes processed.  This could for example
be triggered with statement_timeout and a bad timing, and would issue
confusing error messages when shutting down the session still holding
the locks (note that an assertion failure would be triggered first), on
top of more issues with concurrent sessions trying to take a lock that
would interfere with the SHARE UPDATE EXCLUSIVE locks hold here.

This moves all the interruption checks inside a transaction context.
Note that I have manually tested all interruptions to make sure that
invalid indexes can be cleaned up properly.  Partition indexes still
have issues on their own with some missing dependency handling, which
will be dealt with in a follow-up patch.

Reported-by: Justin Pryzby
Author: Michael Paquier
Discussion: https://postgr.es/m/20191013025145.GC4475@telsasoft.com
Backpatch-through: 12
This commit is contained in:
Michael Paquier 2019-10-25 10:20:08 +09:00
parent 3b0c59ac1c
commit 8270a0d9a9
1 changed files with 25 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
src/backend/commands/indexcmds.c
View File

@ -3057,11 +3057,16 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid newIndexId = lfirst_oid(lc2);
Oid heapId;
CHECK_FOR_INTERRUPTS();
/* Start new transaction for this index's concurrent build */
StartTransactionCommand();
/*
* Check for user-requested abort. This is inside a transaction so as
* xact.c does not issue a useless WARNING, and ensures that
* session-level locks are cleaned up on abort.
*/
CHECK_FOR_INTERRUPTS();
/* Set ActiveSnapshot since functions in the indexes may need it */
PushActiveSnapshot(GetTransactionSnapshot());
@ -3101,10 +3106,15 @@ ReindexRelationConcurrently(Oid relationOid, int options)
TransactionId limitXmin;
Snapshot snapshot;
CHECK_FOR_INTERRUPTS();
StartTransactionCommand();
/*
* Check for user-requested abort. This is inside a transaction so as
* xact.c does not issue a useless WARNING, and ensures that
* session-level locks are cleaned up on abort.
*/
CHECK_FOR_INTERRUPTS();
heapId = IndexGetRelation(newIndexId, false);
/*
@ -3166,6 +3176,11 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid newIndexId = lfirst_oid(lc2);
Oid heapId;
/*
* Check for user-requested abort. This is inside a transaction so as
* xact.c does not issue a useless WARNING, and ensures that
* session-level locks are cleaned up on abort.
*/
CHECK_FOR_INTERRUPTS();
heapId = IndexGetRelation(oldIndexId, false);
@ -3221,7 +3236,13 @@ ReindexRelationConcurrently(Oid relationOid, int options)
Oid oldIndexId = lfirst_oid(lc);
Oid heapId;
/*
* Check for user-requested abort. This is inside a transaction so as
* xact.c does not issue a useless WARNING, and ensures that
* session-level locks are cleaned up on abort.
*/
CHECK_FOR_INTERRUPTS();
heapId = IndexGetRelation(oldIndexId, false);
index_concurrently_set_dead(heapId, oldIndexId);
}