Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitd12bdba

Browse files
committed
Fix possible crash during FATAL exit from reindexing.
index.c supposed that it could just use a PG_TRY block to clean up thestate associated with an active REINDEX operation. However, that codedoesn't run if we do a FATAL exit --- for example, due to a SIGTERMshutdown signal --- while the REINDEX is happening. And that state doesget consulted during catalog accesses, which makes it problematic if wedo any catalog accesses during shutdown --- for example, to clean up anytemp tables created in the session.If this combination of circumstances occurred, we could find ourselvestrying to access already-freed memory. In debug builds that'd fairlyreliably cause an assertion failure. In production we might oftenget away with it, but with some bad luck it could cause a core dump.Another possible bad outcome is an erroneous conclusion that anindex-to-be-accessed is being reindexed; but it looks like that wouldbe unlikely to have any consequences worse than failing to drop temptables right away. (They'd still get dropped by the next session thatuses that temp schema.)To fix, get rid of the use of PG_TRY here, and instead hook intothe transaction abort mechanisms to clean up reindex state.Per bug #16378 from Alexander Lakhin. This has been wrong for avery long time, so back-patch to all supported branches.Discussion:https://postgr.es/m/16378-7a70ca41b3ec2009@postgresql.org
1 parent5836d32 commitd12bdba

File tree

3 files changed

+104
-91
lines changed

3 files changed

+104
-91
lines changed

‎src/backend/access/transam/xact.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include"access/xlog.h"
3131
#include"access/xloginsert.h"
3232
#include"access/xlogutils.h"
33+
#include"catalog/index.h"
3334
#include"catalog/namespace.h"
3435
#include"catalog/pg_enum.h"
3536
#include"catalog/storage.h"
@@ -2662,6 +2663,9 @@ AbortTransaction(void)
26622663
*/
26632664
SetUserIdAndSecContext(s->prevUser,s->prevSecContext);
26642665

2666+
/* Forget about any active REINDEX. */
2667+
ResetReindexState(s->nestingLevel);
2668+
26652669
/* If in parallel mode, clean up workers and exit parallel mode. */
26662670
if (IsInParallelMode())
26672671
{
@@ -4961,6 +4965,9 @@ AbortSubTransaction(void)
49614965
*/
49624966
SetUserIdAndSecContext(s->prevUser,s->prevSecContext);
49634967

4968+
/* Forget about any active REINDEX. */
4969+
ResetReindexState(s->nestingLevel);
4970+
49644971
/* Exit from parallel mode, if necessary. */
49654972
if (IsInParallelMode())
49664973
{

‎src/backend/catalog/index.c

Lines changed: 94 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,6 @@ static void SetReindexProcessing(Oid heapOid, Oid indexOid);
131131
staticvoidResetReindexProcessing(void);
132132
staticvoidSetReindexPending(List*indexes);
133133
staticvoidRemoveReindexPending(OidindexOid);
134-
staticvoidResetReindexPending(void);
135134

136135

137136
/*
@@ -1543,8 +1542,8 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, const char *oldName)
15431542
newIndexForm->indisclustered=oldIndexForm->indisclustered;
15441543

15451544
/*
1546-
* Mark the new index as valid, and the old index as invalid similarly
1547-
*towhat index_set_state_flags() does.
1545+
* Mark the new index as valid, and the old index as invalid similarly to
1546+
* what index_set_state_flags() does.
15481547
*/
15491548
newIndexForm->indisvalid= true;
15501549
oldIndexForm->indisvalid= false;
@@ -3525,25 +3524,18 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
35253524
indexInfo->ii_ExclusionStrats=NULL;
35263525
}
35273526

3528-
/* ensure SetReindexProcessing state isn't leaked */
3529-
PG_TRY();
3530-
{
3531-
/* Suppress use of the target index while rebuilding it */
3532-
SetReindexProcessing(heapId,indexId);
3527+
/* Suppress use of the target index while rebuilding it */
3528+
SetReindexProcessing(heapId,indexId);
35333529

3534-
/* Create a new physical relation for the index */
3535-
RelationSetNewRelfilenode(iRel,persistence);
3530+
/* Create a new physical relation for the index */
3531+
RelationSetNewRelfilenode(iRel,persistence);
35363532

3537-
/* Initialize the index and rebuild */
3538-
/* Note: we do not need to re-establish pkey setting */
3539-
index_build(heapRelation,iRel,indexInfo, true, true);
3540-
}
3541-
PG_FINALLY();
3542-
{
3543-
/* Make sure flag gets cleared on error exit */
3544-
ResetReindexProcessing();
3545-
}
3546-
PG_END_TRY();
3533+
/* Initialize the index and rebuild */
3534+
/* Note: we do not need to re-establish pkey setting */
3535+
index_build(heapRelation,iRel,indexInfo, true, true);
3536+
3537+
/* Re-allow use of target index */
3538+
ResetReindexProcessing();
35473539

35483540
/*
35493541
* If the index is marked invalid/not-ready/dead (ie, it's from a failed
@@ -3680,7 +3672,9 @@ reindex_relation(Oid relid, int flags, int options)
36803672
Relationrel;
36813673
Oidtoast_relid;
36823674
List*indexIds;
3675+
charpersistence;
36833676
boolresult;
3677+
ListCell*indexId;
36843678
inti;
36853679

36863680
/*
@@ -3715,77 +3709,65 @@ reindex_relation(Oid relid, int flags, int options)
37153709
*/
37163710
indexIds=RelationGetIndexList(rel);
37173711

3718-
PG_TRY();
3712+
if (flags&REINDEX_REL_SUPPRESS_INDEX_USE)
37193713
{
3720-
ListCell*indexId;
3721-
charpersistence;
3714+
/* Suppress use of all the indexes until they are rebuilt */
3715+
SetReindexPending(indexIds);
37223716

3723-
if (flags&REINDEX_REL_SUPPRESS_INDEX_USE)
3724-
{
3725-
/* Suppress use of all the indexes until they are rebuilt */
3726-
SetReindexPending(indexIds);
3717+
/*
3718+
* Make the new heap contents visible --- now things might be
3719+
* inconsistent!
3720+
*/
3721+
CommandCounterIncrement();
3722+
}
37273723

3728-
/*
3729-
* Make the new heap contents visible --- now things might be
3730-
* inconsistent!
3731-
*/
3732-
CommandCounterIncrement();
3733-
}
3724+
/*
3725+
* Compute persistence of indexes: same as that of owning rel, unless
3726+
* caller specified otherwise.
3727+
*/
3728+
if (flags&REINDEX_REL_FORCE_INDEXES_UNLOGGED)
3729+
persistence=RELPERSISTENCE_UNLOGGED;
3730+
elseif (flags&REINDEX_REL_FORCE_INDEXES_PERMANENT)
3731+
persistence=RELPERSISTENCE_PERMANENT;
3732+
else
3733+
persistence=rel->rd_rel->relpersistence;
3734+
3735+
/* Reindex all the indexes. */
3736+
i=1;
3737+
foreach(indexId,indexIds)
3738+
{
3739+
OidindexOid=lfirst_oid(indexId);
3740+
OidindexNamespaceId=get_rel_namespace(indexOid);
37343741

37353742
/*
3736-
* Compute persistence of indexes: same as that of owning rel, unless
3737-
* caller specified otherwise.
3743+
* Skip any invalid indexes on a TOAST table. These can only be
3744+
* duplicate leftovers from a failed REINDEX CONCURRENTLY, and if
3745+
* rebuilt it would not be possible to drop them anymore.
37383746
*/
3739-
if (flags&REINDEX_REL_FORCE_INDEXES_UNLOGGED)
3740-
persistence=RELPERSISTENCE_UNLOGGED;
3741-
elseif (flags&REINDEX_REL_FORCE_INDEXES_PERMANENT)
3742-
persistence=RELPERSISTENCE_PERMANENT;
3743-
else
3744-
persistence=rel->rd_rel->relpersistence;
3745-
3746-
/* Reindex all the indexes. */
3747-
i=1;
3748-
foreach(indexId,indexIds)
3747+
if (IsToastNamespace(indexNamespaceId)&&
3748+
!get_index_isvalid(indexOid))
37493749
{
3750-
OidindexOid=lfirst_oid(indexId);
3751-
OidindexNamespaceId=get_rel_namespace(indexOid);
3752-
3753-
/*
3754-
* Skip any invalid indexes on a TOAST table. These can only be
3755-
* duplicate leftovers from a failed REINDEX CONCURRENTLY, and if
3756-
* rebuilt it would not be possible to drop them anymore.
3757-
*/
3758-
if (IsToastNamespace(indexNamespaceId)&&
3759-
!get_index_isvalid(indexOid))
3760-
{
3761-
ereport(WARNING,
3762-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
3763-
errmsg("cannot reindex invalid index \"%s.%s\" on TOAST table, skipping",
3764-
get_namespace_name(indexNamespaceId),
3765-
get_rel_name(indexOid))));
3766-
continue;
3767-
}
3750+
ereport(WARNING,
3751+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
3752+
errmsg("cannot reindex invalid index \"%s.%s\" on TOAST table, skipping",
3753+
get_namespace_name(indexNamespaceId),
3754+
get_rel_name(indexOid))));
3755+
continue;
3756+
}
37683757

3769-
reindex_index(indexOid, !(flags&REINDEX_REL_CHECK_CONSTRAINTS),
3770-
persistence,options);
3758+
reindex_index(indexOid, !(flags&REINDEX_REL_CHECK_CONSTRAINTS),
3759+
persistence,options);
37713760

3772-
CommandCounterIncrement();
3761+
CommandCounterIncrement();
37733762

3774-
/* Index should no longer be in the pending list */
3775-
Assert(!ReindexIsProcessingIndex(indexOid));
3763+
/* Index should no longer be in the pending list */
3764+
Assert(!ReindexIsProcessingIndex(indexOid));
37763765

3777-
/* Set index rebuild count */
3778-
pgstat_progress_update_param(PROGRESS_CLUSTER_INDEX_REBUILD_COUNT,
3779-
i);
3780-
i++;
3781-
}
3766+
/* Set index rebuild count */
3767+
pgstat_progress_update_param(PROGRESS_CLUSTER_INDEX_REBUILD_COUNT,
3768+
i);
3769+
i++;
37823770
}
3783-
PG_FINALLY();
3784-
{
3785-
/* Make sure list gets cleared on error exit */
3786-
ResetReindexPending();
3787-
}
3788-
PG_END_TRY();
37893771

37903772
/*
37913773
* Close rel, but continue to hold the lock.
@@ -3819,6 +3801,7 @@ reindex_relation(Oid relid, int flags, int options)
38193801
staticOidcurrentlyReindexedHeap=InvalidOid;
38203802
staticOidcurrentlyReindexedIndex=InvalidOid;
38213803
staticList*pendingReindexedIndexes=NIL;
3804+
staticintreindexingNestLevel=0;
38223805

38233806
/*
38243807
* ReindexIsProcessingHeap
@@ -3855,8 +3838,6 @@ ReindexIsProcessingIndex(Oid indexOid)
38553838
/*
38563839
* SetReindexProcessing
38573840
*Set flag that specified heap/index are being reindexed.
3858-
*
3859-
* NB: caller must use a PG_TRY block to ensure ResetReindexProcessing is done.
38603841
*/
38613842
staticvoid
38623843
SetReindexProcessing(OidheapOid,OidindexOid)
@@ -3869,6 +3850,8 @@ SetReindexProcessing(Oid heapOid, Oid indexOid)
38693850
currentlyReindexedIndex=indexOid;
38703851
/* Index is no longer "pending" reindex. */
38713852
RemoveReindexPending(indexOid);
3853+
/* This may have been set already, but in case it isn't, do so now. */
3854+
reindexingNestLevel=GetCurrentTransactionNestLevel();
38723855
}
38733856

38743857
/*
@@ -3878,17 +3861,16 @@ SetReindexProcessing(Oid heapOid, Oid indexOid)
38783861
staticvoid
38793862
ResetReindexProcessing(void)
38803863
{
3881-
/* This may be called in leader error path */
38823864
currentlyReindexedHeap=InvalidOid;
38833865
currentlyReindexedIndex=InvalidOid;
3866+
/* reindexingNestLevel remains set till end of (sub)transaction */
38843867
}
38853868

38863869
/*
38873870
* SetReindexPending
38883871
*Mark the given indexes as pending reindex.
38893872
*
3890-
* NB: caller must use a PG_TRY block to ensure ResetReindexPending is done.
3891-
* Also, we assume that the current memory context stays valid throughout.
3873+
* NB: we assume that the current memory context stays valid throughout.
38923874
*/
38933875
staticvoid
38943876
SetReindexPending(List*indexes)
@@ -3899,6 +3881,7 @@ SetReindexPending(List *indexes)
38993881
if (IsInParallelMode())
39003882
elog(ERROR,"cannot modify reindex state during a parallel operation");
39013883
pendingReindexedIndexes=list_copy(indexes);
3884+
reindexingNestLevel=GetCurrentTransactionNestLevel();
39023885
}
39033886

39043887
/*
@@ -3915,14 +3898,32 @@ RemoveReindexPending(Oid indexOid)
39153898
}
39163899

39173900
/*
3918-
*ResetReindexPending
3919-
*Unset reindex-pending status.
3901+
*ResetReindexState
3902+
*Clear all reindexing state during (sub)transaction abort.
39203903
*/
3921-
staticvoid
3922-
ResetReindexPending(void)
3904+
void
3905+
ResetReindexState(intnestLevel)
39233906
{
3924-
/* This may be called in leader error path */
3925-
pendingReindexedIndexes=NIL;
3907+
/*
3908+
* Because reindexing is not re-entrant, we don't need to cope with nested
3909+
* reindexing states. We just need to avoid messing up the outer-level
3910+
* state in case a subtransaction fails within a REINDEX. So checking the
3911+
* current nest level against that of the reindex operation is sufficient.
3912+
*/
3913+
if (reindexingNestLevel >=nestLevel)
3914+
{
3915+
currentlyReindexedHeap=InvalidOid;
3916+
currentlyReindexedIndex=InvalidOid;
3917+
3918+
/*
3919+
* We needn't try to release the contents of pendingReindexedIndexes;
3920+
* that list should be in a transaction-lifespan context, so it will
3921+
* go away automatically.
3922+
*/
3923+
pendingReindexedIndexes=NIL;
3924+
3925+
reindexingNestLevel=0;
3926+
}
39263927
}
39273928

39283929
/*
@@ -3975,4 +3976,7 @@ RestoreReindexState(void *reindexstate)
39753976
lappend_oid(pendingReindexedIndexes,
39763977
sistate->pendingReindexedIndexes[c]);
39773978
MemoryContextSwitchTo(oldcontext);
3979+
3980+
/* Note the worker has its own transaction nesting level */
3981+
reindexingNestLevel=GetCurrentTransactionNestLevel();
39783982
}

‎src/include/catalog/index.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,8 @@ extern void validate_index(Oid heapId, Oid indexId, Snapshot snapshot);
131131

132132
externvoidindex_set_state_flags(OidindexId,IndexStateFlagsActionaction);
133133

134+
externOidIndexGetRelation(OidindexId,boolmissing_ok);
135+
134136
externvoidreindex_index(OidindexId,boolskip_constraint_checks,
135137
charrelpersistence,intoptions);
136138

@@ -145,8 +147,8 @@ extern bool reindex_relation(Oid relid, int flags, int options);
145147

146148
externboolReindexIsProcessingHeap(OidheapOid);
147149
externboolReindexIsProcessingIndex(OidindexOid);
148-
externOidIndexGetRelation(OidindexId,boolmissing_ok);
149150

151+
externvoidResetReindexState(intnestLevel);
150152
externSizeEstimateReindexStateSpace(void);
151153
externvoidSerializeReindexState(Sizemaxsize,char*start_address);
152154
externvoidRestoreReindexState(void*reindexstate);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp