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

Commit5a4efd1

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 parent90abbba commit5a4efd1

File tree

3 files changed

+103
-94
lines changed

3 files changed

+103
-94
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"
@@ -2646,6 +2647,9 @@ AbortTransaction(void)
26462647
*/
26472648
SetUserIdAndSecContext(s->prevUser,s->prevSecContext);
26482649

2650+
/* Forget about any active REINDEX. */
2651+
ResetReindexState(s->nestingLevel);
2652+
26492653
/* If in parallel mode, clean up workers and exit parallel mode. */
26502654
if (IsInParallelMode())
26512655
{
@@ -4946,6 +4950,9 @@ AbortSubTransaction(void)
49464950
*/
49474951
SetUserIdAndSecContext(s->prevUser,s->prevSecContext);
49484952

4953+
/* Forget about any active REINDEX. */
4954+
ResetReindexState(s->nestingLevel);
4955+
49494956
/* Exit from parallel mode, if necessary. */
49504957
if (IsInParallelMode())
49514958
{

‎src/backend/catalog/index.c

Lines changed: 93 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,6 @@ static void SetReindexProcessing(Oid heapOid, Oid indexOid);
130130
staticvoidResetReindexProcessing(void);
131131
staticvoidSetReindexPending(List*indexes);
132132
staticvoidRemoveReindexPending(OidindexOid);
133-
staticvoidResetReindexPending(void);
134133

135134

136135
/*
@@ -1532,8 +1531,8 @@ index_concurrently_swap(Oid newIndexId, Oid oldIndexId, const char *oldName)
15321531
newIndexForm->indisclustered=oldIndexForm->indisclustered;
15331532

15341533
/*
1535-
* Mark the new index as valid, and the old index as invalid similarly
1536-
*towhat index_set_state_flags() does.
1534+
* Mark the new index as valid, and the old index as invalid similarly to
1535+
* what index_set_state_flags() does.
15371536
*/
15381537
newIndexForm->indisvalid= true;
15391538
oldIndexForm->indisvalid= false;
@@ -3534,26 +3533,17 @@ reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
35343533
indexInfo->ii_ExclusionStrats=NULL;
35353534
}
35363535

3537-
/* ensure SetReindexProcessing state isn't leaked */
3538-
PG_TRY();
3539-
{
3540-
/* Suppress use of the target index while rebuilding it */
3541-
SetReindexProcessing(heapId,indexId);
3536+
/* Suppress use of the target index while rebuilding it */
3537+
SetReindexProcessing(heapId,indexId);
35423538

3543-
/* Create a new physical relation for the index */
3544-
RelationSetNewRelfilenode(iRel,persistence);
3539+
/* Create a new physical relation for the index */
3540+
RelationSetNewRelfilenode(iRel,persistence);
35453541

3546-
/* Initialize the index and rebuild */
3547-
/* Note: we do not need to re-establish pkey setting */
3548-
index_build(heapRelation,iRel,indexInfo, true, true);
3549-
}
3550-
PG_CATCH();
3551-
{
3552-
/* Make sure flag gets cleared on error exit */
3553-
ResetReindexProcessing();
3554-
PG_RE_THROW();
3555-
}
3556-
PG_END_TRY();
3542+
/* Initialize the index and rebuild */
3543+
/* Note: we do not need to re-establish pkey setting */
3544+
index_build(heapRelation,iRel,indexInfo, true, true);
3545+
3546+
/* Re-allow use of target index */
35573547
ResetReindexProcessing();
35583548

35593549
/*
@@ -3691,7 +3681,9 @@ reindex_relation(Oid relid, int flags, int options)
36913681
Relationrel;
36923682
Oidtoast_relid;
36933683
List*indexIds;
3684+
charpersistence;
36943685
boolresult;
3686+
ListCell*indexId;
36953687
inti;
36963688

36973689
/*
@@ -3726,79 +3718,65 @@ reindex_relation(Oid relid, int flags, int options)
37263718
*/
37273719
indexIds=RelationGetIndexList(rel);
37283720

3729-
PG_TRY();
3721+
if (flags&REINDEX_REL_SUPPRESS_INDEX_USE)
37303722
{
3731-
ListCell*indexId;
3732-
charpersistence;
3723+
/* Suppress use of all the indexes until they are rebuilt */
3724+
SetReindexPending(indexIds);
37333725

3734-
if (flags&REINDEX_REL_SUPPRESS_INDEX_USE)
3735-
{
3736-
/* Suppress use of all the indexes until they are rebuilt */
3737-
SetReindexPending(indexIds);
3726+
/*
3727+
* Make the new heap contents visible --- now things might be
3728+
* inconsistent!
3729+
*/
3730+
CommandCounterIncrement();
3731+
}
37383732

3739-
/*
3740-
* Make the new heap contents visible --- now things might be
3741-
* inconsistent!
3742-
*/
3743-
CommandCounterIncrement();
3744-
}
3733+
/*
3734+
* Compute persistence of indexes: same as that of owning rel, unless
3735+
* caller specified otherwise.
3736+
*/
3737+
if (flags&REINDEX_REL_FORCE_INDEXES_UNLOGGED)
3738+
persistence=RELPERSISTENCE_UNLOGGED;
3739+
elseif (flags&REINDEX_REL_FORCE_INDEXES_PERMANENT)
3740+
persistence=RELPERSISTENCE_PERMANENT;
3741+
else
3742+
persistence=rel->rd_rel->relpersistence;
3743+
3744+
/* Reindex all the indexes. */
3745+
i=1;
3746+
foreach(indexId,indexIds)
3747+
{
3748+
OidindexOid=lfirst_oid(indexId);
3749+
OidindexNamespaceId=get_rel_namespace(indexOid);
37453750

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

3780-
reindex_index(indexOid, !(flags&REINDEX_REL_CHECK_CONSTRAINTS),
3781-
persistence,options);
3767+
reindex_index(indexOid, !(flags&REINDEX_REL_CHECK_CONSTRAINTS),
3768+
persistence,options);
37823769

3783-
CommandCounterIncrement();
3770+
CommandCounterIncrement();
37843771

3785-
/* Index should no longer be in the pending list */
3786-
Assert(!ReindexIsProcessingIndex(indexOid));
3772+
/* Index should no longer be in the pending list */
3773+
Assert(!ReindexIsProcessingIndex(indexOid));
37873774

3788-
/* Set index rebuild count */
3789-
pgstat_progress_update_param(PROGRESS_CLUSTER_INDEX_REBUILD_COUNT,
3790-
i);
3791-
i++;
3792-
}
3775+
/* Set index rebuild count */
3776+
pgstat_progress_update_param(PROGRESS_CLUSTER_INDEX_REBUILD_COUNT,
3777+
i);
3778+
i++;
37933779
}
3794-
PG_CATCH();
3795-
{
3796-
/* Make sure list gets cleared on error exit */
3797-
ResetReindexPending();
3798-
PG_RE_THROW();
3799-
}
3800-
PG_END_TRY();
3801-
ResetReindexPending();
38023780

38033781
/*
38043782
* Close rel, but continue to hold the lock.
@@ -3832,6 +3810,7 @@ reindex_relation(Oid relid, int flags, int options)
38323810
staticOidcurrentlyReindexedHeap=InvalidOid;
38333811
staticOidcurrentlyReindexedIndex=InvalidOid;
38343812
staticList*pendingReindexedIndexes=NIL;
3813+
staticintreindexingNestLevel=0;
38353814

38363815
/*
38373816
* ReindexIsProcessingHeap
@@ -3868,8 +3847,6 @@ ReindexIsProcessingIndex(Oid indexOid)
38683847
/*
38693848
* SetReindexProcessing
38703849
*Set flag that specified heap/index are being reindexed.
3871-
*
3872-
* NB: caller must use a PG_TRY block to ensure ResetReindexProcessing is done.
38733850
*/
38743851
staticvoid
38753852
SetReindexProcessing(OidheapOid,OidindexOid)
@@ -3882,6 +3859,8 @@ SetReindexProcessing(Oid heapOid, Oid indexOid)
38823859
currentlyReindexedIndex=indexOid;
38833860
/* Index is no longer "pending" reindex. */
38843861
RemoveReindexPending(indexOid);
3862+
/* This may have been set already, but in case it isn't, do so now. */
3863+
reindexingNestLevel=GetCurrentTransactionNestLevel();
38853864
}
38863865

38873866
/*
@@ -3891,17 +3870,16 @@ SetReindexProcessing(Oid heapOid, Oid indexOid)
38913870
staticvoid
38923871
ResetReindexProcessing(void)
38933872
{
3894-
/* This may be called in leader error path */
38953873
currentlyReindexedHeap=InvalidOid;
38963874
currentlyReindexedIndex=InvalidOid;
3875+
/* reindexingNestLevel remains set till end of (sub)transaction */
38973876
}
38983877

38993878
/*
39003879
* SetReindexPending
39013880
*Mark the given indexes as pending reindex.
39023881
*
3903-
* NB: caller must use a PG_TRY block to ensure ResetReindexPending is done.
3904-
* Also, we assume that the current memory context stays valid throughout.
3882+
* NB: we assume that the current memory context stays valid throughout.
39053883
*/
39063884
staticvoid
39073885
SetReindexPending(List*indexes)
@@ -3912,6 +3890,7 @@ SetReindexPending(List *indexes)
39123890
if (IsInParallelMode())
39133891
elog(ERROR,"cannot modify reindex state during a parallel operation");
39143892
pendingReindexedIndexes=list_copy(indexes);
3893+
reindexingNestLevel=GetCurrentTransactionNestLevel();
39153894
}
39163895

39173896
/*
@@ -3928,14 +3907,32 @@ RemoveReindexPending(Oid indexOid)
39283907
}
39293908

39303909
/*
3931-
*ResetReindexPending
3932-
*Unset reindex-pending status.
3910+
*ResetReindexState
3911+
*Clear all reindexing state during (sub)transaction abort.
39333912
*/
3934-
staticvoid
3935-
ResetReindexPending(void)
3913+
void
3914+
ResetReindexState(intnestLevel)
39363915
{
3937-
/* This may be called in leader error path */
3938-
pendingReindexedIndexes=NIL;
3916+
/*
3917+
* Because reindexing is not re-entrant, we don't need to cope with nested
3918+
* reindexing states. We just need to avoid messing up the outer-level
3919+
* state in case a subtransaction fails within a REINDEX. So checking the
3920+
* current nest level against that of the reindex operation is sufficient.
3921+
*/
3922+
if (reindexingNestLevel >=nestLevel)
3923+
{
3924+
currentlyReindexedHeap=InvalidOid;
3925+
currentlyReindexedIndex=InvalidOid;
3926+
3927+
/*
3928+
* We needn't try to release the contents of pendingReindexedIndexes;
3929+
* that list should be in a transaction-lifespan context, so it will
3930+
* go away automatically.
3931+
*/
3932+
pendingReindexedIndexes=NIL;
3933+
3934+
reindexingNestLevel=0;
3935+
}
39393936
}
39403937

39413938
/*
@@ -3988,4 +3985,7 @@ RestoreReindexState(void *reindexstate)
39883985
lappend_oid(pendingReindexedIndexes,
39893986
sistate->pendingReindexedIndexes[c]);
39903987
MemoryContextSwitchTo(oldcontext);
3988+
3989+
/* Note the worker has its own transaction nesting level */
3990+
reindexingNestLevel=GetCurrentTransactionNestLevel();
39913991
}

‎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