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

Commit5b4791b

Browse files
committed
Fix access to no-longer-open relcache entry in logical-rep worker.
If we redirected a replicated tuple operation into a partition childtable, and then tried to fire AFTER triggers for that event, therelation cache entry for the child table was already closed. This hasno visible ill effects as long as the entry is still there and stillvalid, but an unluckily-timed cache flush could result in a crash orother misbehavior.To fix, postpone the ExecCleanupTupleRouting call (which is whatcloses the child table) until after we've fired triggers. Thisrequires a bit of refactoring so that the cleanup function canhave access to the necessary state.In HEAD, I took the opportunity to simplify some of worker.c'sfunction APIs based on use of the new ApplyExecutionData struct.However, it doesn't seem safe/practical to back-patch that aspect,at least not without a lot of analysis of possible interactionswitha04daa9.In passing, add an Assert to afterTriggerInvokeEvents to catchsuch cases. This seems worthwhile because we've grown a numberof fairly unstructured ways of calling AfterTriggerEndQuery.Back-patch to v13, where worker.c grew the ability to deal withpartitioned target tables.Discussion:https://postgr.es/m/3382681.1621381328@sss.pgh.pa.us
1 parent849c797 commit5b4791b

File tree

2 files changed

+80
-35
lines changed

2 files changed

+80
-35
lines changed

‎src/backend/commands/trigger.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4140,6 +4140,8 @@ afterTriggerInvokeEvents(AfterTriggerEventList *events,
41404140
{
41414141
rInfo=ExecGetTriggerResultRel(estate,evtshared->ats_relid);
41424142
rel=rInfo->ri_RelationDesc;
4143+
/* Catch calls with insufficient relcache refcounting */
4144+
Assert(!RelationHasReferenceCountZero(rel));
41434145
trigdesc=rInfo->ri_TrigDesc;
41444146
finfo=rInfo->ri_TrigFunctions;
41454147
instr=rInfo->ri_TrigInstrument;

‎src/backend/replication/logical/worker.c

Lines changed: 78 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,18 @@ typedef struct SlotErrCallbackArg
9797
intremote_attnum;
9898
}SlotErrCallbackArg;
9999

100+
typedefstructApplyExecutionData
101+
{
102+
EState*estate;/* executor state, used to track resources */
103+
104+
LogicalRepRelMapEntry*targetRel;/* replication target rel */
105+
ResultRelInfo*targetRelInfo;/* ResultRelInfo for same */
106+
107+
/* These fields are used when the target relation is partitioned: */
108+
ModifyTableState*mtstate;/* dummy ModifyTable state */
109+
PartitionTupleRouting*proute;/* partition routing info */
110+
}ApplyExecutionData;
111+
100112
staticMemoryContextApplyMessageContext=NULL;
101113
MemoryContextApplyContext=NULL;
102114

@@ -127,11 +139,9 @@ static bool FindReplTupleInLocalRel(EState *estate, Relation localrel,
127139
LogicalRepRelation*remoterel,
128140
TupleTableSlot*remoteslot,
129141
TupleTableSlot**localslot);
130-
staticvoidapply_handle_tuple_routing(ResultRelInfo*relinfo,
131-
EState*estate,
142+
staticvoidapply_handle_tuple_routing(ApplyExecutionData*edata,
132143
TupleTableSlot*remoteslot,
133144
LogicalRepTupleData*newtup,
134-
LogicalRepRelMapEntry*relmapentry,
135145
CmdTypeoperation);
136146

137147
/*
@@ -188,25 +198,29 @@ ensure_transaction(void)
188198

189199
/*
190200
* Executor state preparation for evaluation of constraint expressions,
191-
* indexes and triggers.
201+
* indexes and triggers for the specified relation.
192202
*
193-
*This is based on similar code in copy.c
203+
*Note that the caller must open and close any indexes to be updated.
194204
*/
195-
staticEState*
196-
create_estate_for_relation(LogicalRepRelMapEntry*rel)
205+
staticApplyExecutionData*
206+
create_edata_for_relation(LogicalRepRelMapEntry*rel)
197207
{
208+
ApplyExecutionData*edata;
198209
EState*estate;
199210
ResultRelInfo*resultRelInfo;
200211
RangeTblEntry*rte;
201212

202213
/*
203214
* Input functions may need an active snapshot, as may AFTER triggers
204-
* invoked duringfinish_estate. For safety, ensure an active snapshot
215+
* invoked duringfinish_edata. For safety, ensure an active snapshot
205216
* exists throughout all our usage of the executor.
206217
*/
207218
PushActiveSnapshot(GetTransactionSnapshot());
208219

209-
estate=CreateExecutorState();
220+
edata= (ApplyExecutionData*)palloc0(sizeof(ApplyExecutionData));
221+
edata->targetRel=rel;
222+
223+
edata->estate=estate=CreateExecutorState();
210224

211225
rte=makeNode(RangeTblEntry);
212226
rte->rtekind=RTE_RELATION;
@@ -215,7 +229,12 @@ create_estate_for_relation(LogicalRepRelMapEntry *rel)
215229
rte->rellockmode=AccessShareLock;
216230
ExecInitRangeTable(estate,list_make1(rte));
217231

218-
resultRelInfo=makeNode(ResultRelInfo);
232+
edata->targetRelInfo=resultRelInfo=makeNode(ResultRelInfo);
233+
234+
/*
235+
* Use Relation opened by logicalrep_rel_open() instead of opening it
236+
* again.
237+
*/
219238
InitResultRelInfo(resultRelInfo,rel->localrel,1,NULL,0);
220239

221240
estate->es_result_relations=resultRelInfo;
@@ -227,22 +246,38 @@ create_estate_for_relation(LogicalRepRelMapEntry *rel)
227246
/* Prepare to catch AFTER triggers. */
228247
AfterTriggerBeginQuery();
229248

230-
returnestate;
249+
/* other fields of edata remain NULL for now */
250+
251+
returnedata;
231252
}
232253

233254
/*
234255
* Finish any operations related to the executor state created by
235-
*create_estate_for_relation().
256+
*create_edata_for_relation().
236257
*/
237258
staticvoid
238-
finish_estate(EState*estate)
259+
finish_edata(ApplyExecutionData*edata)
239260
{
261+
EState*estate=edata->estate;
262+
240263
/* Handle any queued AFTER triggers. */
241264
AfterTriggerEndQuery(estate);
242265

243-
/* Cleanup. */
266+
/* Shut down tuple routing, if any was done. */
267+
if (edata->proute)
268+
ExecCleanupTupleRouting(edata->mtstate,edata->proute);
269+
270+
/*
271+
* Cleanup. It might seem that we should call ExecCloseResultRelations()
272+
* here, but we intentionally don't. It would close the rel we added to
273+
* the estate above, which is wrong because we took no corresponding
274+
* refcount. We rely on ExecCleanupTupleRouting() to close any other
275+
* relations opened during execution.
276+
*/
244277
ExecResetTupleTable(estate->es_tupleTable, false);
245278
FreeExecutorState(estate);
279+
pfree(edata);
280+
246281
PopActiveSnapshot();
247282
}
248283

@@ -633,6 +668,7 @@ apply_handle_insert(StringInfo s)
633668
LogicalRepRelMapEntry*rel;
634669
LogicalRepTupleDatanewtup;
635670
LogicalRepRelIdrelid;
671+
ApplyExecutionData*edata;
636672
EState*estate;
637673
TupleTableSlot*remoteslot;
638674
MemoryContextoldctx;
@@ -652,7 +688,8 @@ apply_handle_insert(StringInfo s)
652688
}
653689

654690
/* Initialize the executor state. */
655-
estate=create_estate_for_relation(rel);
691+
edata=create_edata_for_relation(rel);
692+
estate=edata->estate;
656693
remoteslot=ExecInitExtraTupleSlot(estate,
657694
RelationGetDescr(rel->localrel),
658695
&TTSOpsVirtual);
@@ -665,13 +702,13 @@ apply_handle_insert(StringInfo s)
665702

666703
/* For a partitioned table, insert the tuple into a partition. */
667704
if (rel->localrel->rd_rel->relkind==RELKIND_PARTITIONED_TABLE)
668-
apply_handle_tuple_routing(estate->es_result_relation_info,estate,
669-
remoteslot,NULL,rel,CMD_INSERT);
705+
apply_handle_tuple_routing(edata,
706+
remoteslot,NULL,CMD_INSERT);
670707
else
671708
apply_handle_insert_internal(estate->es_result_relation_info,estate,
672709
remoteslot);
673710

674-
finish_estate(estate);
711+
finish_edata(edata);
675712

676713
logicalrep_rel_close(rel,NoLock);
677714

@@ -735,6 +772,7 @@ apply_handle_update(StringInfo s)
735772
{
736773
LogicalRepRelMapEntry*rel;
737774
LogicalRepRelIdrelid;
775+
ApplyExecutionData*edata;
738776
EState*estate;
739777
LogicalRepTupleDataoldtup;
740778
LogicalRepTupleDatanewtup;
@@ -762,7 +800,8 @@ apply_handle_update(StringInfo s)
762800
check_relation_updatable(rel);
763801

764802
/* Initialize the executor state. */
765-
estate=create_estate_for_relation(rel);
803+
edata=create_edata_for_relation(rel);
804+
estate=edata->estate;
766805
remoteslot=ExecInitExtraTupleSlot(estate,
767806
RelationGetDescr(rel->localrel),
768807
&TTSOpsVirtual);
@@ -800,13 +839,13 @@ apply_handle_update(StringInfo s)
800839

801840
/* For a partitioned table, apply update to correct partition. */
802841
if (rel->localrel->rd_rel->relkind==RELKIND_PARTITIONED_TABLE)
803-
apply_handle_tuple_routing(estate->es_result_relation_info,estate,
804-
remoteslot,&newtup,rel,CMD_UPDATE);
842+
apply_handle_tuple_routing(edata,
843+
remoteslot,&newtup,CMD_UPDATE);
805844
else
806845
apply_handle_update_internal(estate->es_result_relation_info,estate,
807846
remoteslot,&newtup,rel);
808847

809-
finish_estate(estate);
848+
finish_edata(edata);
810849

811850
logicalrep_rel_close(rel,NoLock);
812851

@@ -881,6 +920,7 @@ apply_handle_delete(StringInfo s)
881920
LogicalRepRelMapEntry*rel;
882921
LogicalRepTupleDataoldtup;
883922
LogicalRepRelIdrelid;
923+
ApplyExecutionData*edata;
884924
EState*estate;
885925
TupleTableSlot*remoteslot;
886926
MemoryContextoldctx;
@@ -903,7 +943,8 @@ apply_handle_delete(StringInfo s)
903943
check_relation_updatable(rel);
904944

905945
/* Initialize the executor state. */
906-
estate=create_estate_for_relation(rel);
946+
edata=create_edata_for_relation(rel);
947+
estate=edata->estate;
907948
remoteslot=ExecInitExtraTupleSlot(estate,
908949
RelationGetDescr(rel->localrel),
909950
&TTSOpsVirtual);
@@ -915,13 +956,13 @@ apply_handle_delete(StringInfo s)
915956

916957
/* For a partitioned table, apply delete to correct partition. */
917958
if (rel->localrel->rd_rel->relkind==RELKIND_PARTITIONED_TABLE)
918-
apply_handle_tuple_routing(estate->es_result_relation_info,estate,
919-
remoteslot,NULL,rel,CMD_DELETE);
959+
apply_handle_tuple_routing(edata,
960+
remoteslot,NULL,CMD_DELETE);
920961
else
921962
apply_handle_delete_internal(estate->es_result_relation_info,estate,
922963
remoteslot,&rel->remoterel);
923964

924-
finish_estate(estate);
965+
finish_edata(edata);
925966

926967
logicalrep_rel_close(rel,NoLock);
927968

@@ -1004,16 +1045,17 @@ FindReplTupleInLocalRel(EState *estate, Relation localrel,
10041045
* This handles insert, update, delete on a partitioned table.
10051046
*/
10061047
staticvoid
1007-
apply_handle_tuple_routing(ResultRelInfo*relinfo,
1008-
EState*estate,
1048+
apply_handle_tuple_routing(ApplyExecutionData*edata,
10091049
TupleTableSlot*remoteslot,
10101050
LogicalRepTupleData*newtup,
1011-
LogicalRepRelMapEntry*relmapentry,
10121051
CmdTypeoperation)
10131052
{
1053+
EState*estate=edata->estate;
1054+
LogicalRepRelMapEntry*relmapentry=edata->targetRel;
1055+
ResultRelInfo*relinfo=edata->targetRelInfo;
10141056
Relationparentrel=relinfo->ri_RelationDesc;
1015-
ModifyTableState*mtstate=NULL;
1016-
PartitionTupleRouting*proute=NULL;
1057+
ModifyTableState*mtstate;
1058+
PartitionTupleRouting*proute;
10171059
ResultRelInfo*partrelinfo;
10181060
Relationpartrel;
10191061
TupleTableSlot*remoteslot_part;
@@ -1022,12 +1064,15 @@ apply_handle_tuple_routing(ResultRelInfo *relinfo,
10221064
MemoryContextoldctx;
10231065

10241066
/* ModifyTableState is needed for ExecFindPartition(). */
1025-
mtstate=makeNode(ModifyTableState);
1067+
edata->mtstate=mtstate=makeNode(ModifyTableState);
10261068
mtstate->ps.plan=NULL;
10271069
mtstate->ps.state=estate;
10281070
mtstate->operation=operation;
10291071
mtstate->resultRelInfo=relinfo;
1030-
proute=ExecSetupPartitionTupleRouting(estate,mtstate,parentrel);
1072+
1073+
/* ... as is PartitionTupleRouting. */
1074+
edata->proute=proute=ExecSetupPartitionTupleRouting(estate,mtstate,
1075+
parentrel);
10311076

10321077
/*
10331078
* Find the partition to which the "search tuple" belongs.
@@ -1225,8 +1270,6 @@ apply_handle_tuple_routing(ResultRelInfo *relinfo,
12251270
elog(ERROR,"unrecognized CmdType: %d", (int)operation);
12261271
break;
12271272
}
1228-
1229-
ExecCleanupTupleRouting(mtstate,proute);
12301273
}
12311274

12321275
/*

0 commit comments

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