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

Commitf3b141c

Browse files
committed
Fix relation leak for subscribers firing triggers in logical replication
Creating a trigger on a relation to which an apply operation istriggered would cause a relation leak once the change gets committed,as the executor would miss that the relation needs to be closedbeforehand. This issue got introduced with the refactoring done in1375422, where it becomes necessary to track relations withines_opened_result_relations to make sure that they are closed.We have discussed using ExecInitResultRelation() coupled withExecCloseResultRelations() for the relations in need of tracking by theapply operations in the subscribers, which would simplify greatly theopening and closing of indexes, but this requires a larger rework andreorganization of the worker code, particularly for the tuple routingpart. And that's not really welcome post feature freeze. So, for now,settle down to the same solution as TRUNCATE which is to fill ines_opened_result_relations with the relation opened, to make sure thatExecGetTriggerResultRel() finds them and that they get closed.The code is lightly refactored so as a relation is not registered threetimes for each DML code path, making the whole a bit easier to follow.Reported-by: Tang Haiying, Shi Yu, Hou ZhijieAuthor: Amit Langote, Masahiko Sawada, Hou ZhijieReviewed-by: Amit Kapila, Michael PaquierDiscussion:https://postgr.es/m/OS0PR01MB611383FA0FE92EB9DE21946AFB769@OS0PR01MB6113.jpnprd01.prod.outlook.com
1 parent1599e7b commitf3b141c

File tree

1 file changed

+47
-26
lines changed

1 file changed

+47
-26
lines changed

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

Lines changed: 47 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -338,10 +338,13 @@ handle_streamed_transaction(LogicalRepMsgType action, StringInfo s)
338338
* Executor state preparation for evaluation of constraint expressions,
339339
* indexes and triggers.
340340
*
341-
* This is based on similar code in copy.c
341+
* resultRelInfo is a ResultRelInfo for the relation to be passed to the
342+
* executor routines. The caller must open and close any indexes to be
343+
* updated independently of the relation registered here.
342344
*/
343345
staticEState*
344-
create_estate_for_relation(LogicalRepRelMapEntry*rel)
346+
create_estate_for_relation(LogicalRepRelMapEntry*rel,
347+
ResultRelInfo**resultRelInfo)
345348
{
346349
EState*estate;
347350
RangeTblEntry*rte;
@@ -355,6 +358,27 @@ create_estate_for_relation(LogicalRepRelMapEntry *rel)
355358
rte->rellockmode=AccessShareLock;
356359
ExecInitRangeTable(estate,list_make1(rte));
357360

361+
*resultRelInfo=makeNode(ResultRelInfo);
362+
363+
/*
364+
* Use Relation opened by logicalrep_rel_open() instead of opening it
365+
* again.
366+
*/
367+
InitResultRelInfo(*resultRelInfo,rel->localrel,1,NULL,0);
368+
369+
/*
370+
* We put the ResultRelInfo in the es_opened_result_relations list, even
371+
* though we don't populate the es_result_relations array. That's a bit
372+
* bogus, but it's enough to make ExecGetTriggerResultRel() find them.
373+
* Also, because we did not open the Relation ourselves here, there is no
374+
* need to worry about closing it.
375+
*
376+
* ExecOpenIndices() is not called here either, each execution path doing
377+
* an apply operation being responsible for that.
378+
*/
379+
estate->es_opened_result_relations=
380+
lappend(estate->es_opened_result_relations,*resultRelInfo);
381+
358382
estate->es_output_cid=GetCurrentCommandId(true);
359383

360384
/* Prepare to catch AFTER triggers. */
@@ -363,6 +387,21 @@ create_estate_for_relation(LogicalRepRelMapEntry *rel)
363387
returnestate;
364388
}
365389

390+
/*
391+
* Finish any operations related to the executor state created by
392+
* create_estate_for_relation().
393+
*/
394+
staticvoid
395+
finish_estate(EState*estate)
396+
{
397+
/* Handle any queued AFTER triggers. */
398+
AfterTriggerEndQuery(estate);
399+
400+
/* Cleanup. */
401+
ExecResetTupleTable(estate->es_tupleTable, false);
402+
FreeExecutorState(estate);
403+
}
404+
366405
/*
367406
* Executes default values for columns for which we can't map to remote
368407
* relation columns.
@@ -1168,12 +1207,10 @@ apply_handle_insert(StringInfo s)
11681207
}
11691208

11701209
/* Initialize the executor state. */
1171-
estate=create_estate_for_relation(rel);
1210+
estate=create_estate_for_relation(rel,&resultRelInfo);
11721211
remoteslot=ExecInitExtraTupleSlot(estate,
11731212
RelationGetDescr(rel->localrel),
11741213
&TTSOpsVirtual);
1175-
resultRelInfo=makeNode(ResultRelInfo);
1176-
InitResultRelInfo(resultRelInfo,rel->localrel,1,NULL,0);
11771214

11781215
/* Input functions may need an active snapshot, so get one */
11791216
PushActiveSnapshot(GetTransactionSnapshot());
@@ -1194,11 +1231,7 @@ apply_handle_insert(StringInfo s)
11941231

11951232
PopActiveSnapshot();
11961233

1197-
/* Handle queued AFTER triggers. */
1198-
AfterTriggerEndQuery(estate);
1199-
1200-
ExecResetTupleTable(estate->es_tupleTable, false);
1201-
FreeExecutorState(estate);
1234+
finish_estate(estate);
12021235

12031236
logicalrep_rel_close(rel,NoLock);
12041237

@@ -1293,12 +1326,10 @@ apply_handle_update(StringInfo s)
12931326
check_relation_updatable(rel);
12941327

12951328
/* Initialize the executor state. */
1296-
estate=create_estate_for_relation(rel);
1329+
estate=create_estate_for_relation(rel,&resultRelInfo);
12971330
remoteslot=ExecInitExtraTupleSlot(estate,
12981331
RelationGetDescr(rel->localrel),
12991332
&TTSOpsVirtual);
1300-
resultRelInfo=makeNode(ResultRelInfo);
1301-
InitResultRelInfo(resultRelInfo,rel->localrel,1,NULL,0);
13021333

13031334
/*
13041335
* Populate updatedCols so that per-column triggers can fire, and so
@@ -1345,11 +1376,7 @@ apply_handle_update(StringInfo s)
13451376

13461377
PopActiveSnapshot();
13471378

1348-
/* Handle queued AFTER triggers. */
1349-
AfterTriggerEndQuery(estate);
1350-
1351-
ExecResetTupleTable(estate->es_tupleTable, false);
1352-
FreeExecutorState(estate);
1379+
finish_estate(estate);
13531380

13541381
logicalrep_rel_close(rel,NoLock);
13551382

@@ -1450,12 +1477,10 @@ apply_handle_delete(StringInfo s)
14501477
check_relation_updatable(rel);
14511478

14521479
/* Initialize the executor state. */
1453-
estate=create_estate_for_relation(rel);
1480+
estate=create_estate_for_relation(rel,&resultRelInfo);
14541481
remoteslot=ExecInitExtraTupleSlot(estate,
14551482
RelationGetDescr(rel->localrel),
14561483
&TTSOpsVirtual);
1457-
resultRelInfo=makeNode(ResultRelInfo);
1458-
InitResultRelInfo(resultRelInfo,rel->localrel,1,NULL,0);
14591484

14601485
PushActiveSnapshot(GetTransactionSnapshot());
14611486

@@ -1474,11 +1499,7 @@ apply_handle_delete(StringInfo s)
14741499

14751500
PopActiveSnapshot();
14761501

1477-
/* Handle queued AFTER triggers. */
1478-
AfterTriggerEndQuery(estate);
1479-
1480-
ExecResetTupleTable(estate->es_tupleTable, false);
1481-
FreeExecutorState(estate);
1502+
finish_estate(estate);
14821503

14831504
logicalrep_rel_close(rel,NoLock);
14841505

0 commit comments

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