pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedSEGFAULT on a concurrent UPDATE of mix of local and foreign partitions
4+ messages / 3 participants
[nested] [flat]
* SEGFAULT on a concurrent UPDATE of mix of local and foreign partitions
@ 2021-08-06 05:34 Andrey Lepikhov <a.lepikhov@postgrespro.ru>
0 siblings, 1 reply; 4+ messages in thread
From: Andrey Lepikhov @ 2021-08-06 05:34 UTC (permalink / raw)
To: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; +Cc: Heikki Linnakangas <hlinnaka@iki.fi>
Hi,
Postgres SEGFAULT'ed on the UPDATE of mix of local and foreign partitions.
Initialization - see t.sql
For replaying this segfault just execute in parallel:
UPDATE test SET x = x - 1;
The problem was introduced by commit 1375422.
ExecUpdate has found a concurrently updated tuples and starts subplan
evaluation. This operation creates new EState for EPQState and sets
es_result_relations in NULL value. Next, ExecInitNode(subplan) is
launched and underlying ExecInitForeignScan tries to access to an
element of es_result_relations. This causes SEGFAULT.
I studied this problem shortly. I think, EPQState can use
es_result_relations of a parent EState. Patch in attachment fixes this.
check-world passed clearly.
--
regards,
Andrey Lepikhov
Postgres Professional
From ab067bdeb6de84db80504d2fb05da4385fa02ebc Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru>
Date: Fri, 6 Aug 2021 10:11:52 +0500
Subject: [PATCH] Share es_result_relations of parent EState in EPQState
because FDW direct-modify nodes expect it available.
---
src/backend/executor/execMain.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index b3ce4bae53..a030fec86a 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -2714,10 +2714,12 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
rcestate->es_output_cid = parentestate->es_output_cid;
/*
- * ResultRelInfos needed by subplans are initialized from scratch when the
- * subplans themselves are initialized.
+ * Share the parent's copy of ResultRelInfos needed by subplans.
+ * We must do this before initializing the subplan, because direct-modify
+ * FDWs expect their ResultRelInfos to be available.
*/
- rcestate->es_result_relations = NULL;
+ rcestate->es_result_relations = parentestate->es_result_relations;
+
/* es_trig_target_relations must NOT be copied */
rcestate->es_top_eflags = parentestate->es_top_eflags;
rcestate->es_instrument = parentestate->es_instrument;
--
2.31.1
* frame #0: 0x000000010a21df66 postgres`ExecInitForeignScan(node=0x00007f8e6b823ba8, estate=0x00007f8e6d809b20, eflags=0) at nodeForeignscan.c:230:30
frame #1: 0x000000010a1fc3c3 postgres`ExecInitNode(node=0x00007f8e6b823ba8, estate=0x00007f8e6d809b20, eflags=0) at execProcnode.c:284:27
frame #2: 0x000000010a218265 postgres`ExecInitAppend(node=0x00007f8e6b80e150, estate=0x00007f8e6d809b20, eflags=0) at nodeAppend.c:232:27
frame #3: 0x000000010a1fc193 postgres`ExecInitNode(node=0x00007f8e6b80e150, estate=0x00007f8e6d809b20, eflags=0) at execProcnode.c:181:27
frame #4: 0x000000010a1f4be1 postgres`EvalPlanQualStart(epqstate=0x00007f8e6b817490, planTree=0x00007f8e6b80e150) at execMain.c:2817:31
frame #5: 0x000000010a1f399f postgres`EvalPlanQualBegin(epqstate=0x00007f8e6b817490) at execMain.c:2630:3
frame #6: 0x000000010a1f38de postgres`EvalPlanQual(epqstate=0x00007f8e6b817490, relation=0x00007f8e6b59e160, rti=2, inputslot=0x00007f8e6b830688) at execMain.c:2370:2
frame #7: 0x000000010a24085c postgres`ExecUpdate(mtstate=0x00007f8e6b8173a8, resultRelInfo=0x00007f8e6b8175c0, tupleid=0x00007ffee5d76858, oldtuple=0x0000000000000000, slot=0x00007f8e6b82f9f0, planSlot=0x00007f8e6b8194f0, epqstate=0x00007f8e6b817490, estate=0x00007f8e6b817120, canSetTag=true) at nodeModifyTable.c:1861:18
frame #8: 0x000000010a23e964 postgres`ExecModifyTable(pstate=0x00007f8e6b8173a8) at nodeModifyTable.c:2592:12
frame #9: 0x000000010a1fc8c2 postgres`ExecProcNodeFirst(node=0x00007f8e6b8173a8) at execProcnode.c:463:9
frame #10: 0x000000010a1f4762 postgres`ExecProcNode(node=0x00007f8e6b8173a8) at executor.h:257:9
frame #11: 0x000000010a1efe01 postgres`ExecutePlan(estate=0x00007f8e6b817120, planstate=0x00007f8e6b8173a8, use_parallel_mode=false, operation=CMD_UPDATE, sendTuples=false, numberTuples=0, direction=ForwardScanDirection, dest=0x00007f8e6b8240d0, execute_once=true) at execMain.c:1551:10
frame #12: 0x000000010a1efcd0 postgres`standard_ExecutorRun(queryDesc=0x00007f8e6b815b20, direction=ForwardScanDirection, count=0, execute_once=true) at execMain.c:361:3
frame #13: 0x000000010a1efa61 postgres`ExecutorRun(queryDesc=0x00007f8e6b815b20, direction=ForwardScanDirection, count=0, execute_once=true) at execMain.c:305:3
frame #14: 0x000000010a4aff59 postgres`ProcessQuery(plan=0x00007f8e6b824038, sourceText="UPDATE test SET x = x - 1;", params=0x0000000000000000, queryEnv=0x0000000000000000, dest=0x00007f8e6b8240d0, qc=0x00007ffee5d76d30) at pquery.c:160:2
frame #15: 0x000000010a4aede3 postgres`PortalRunMulti(portal=0x00007f8e6e036120, isTopLevel=true, setHoldSnapshot=false, dest=0x00007f8e6b8240d0, altdest=0x00007f8e6b8240d0, qc=0x00007ffee5d76d30) at pquery.c:1266:5
frame #16: 0x000000010a4ae3a8 postgres`PortalRun(portal=0x00007f8e6e036120, count=9223372036854775807, isTopLevel=true, run_once=true, dest=0x00007f8e6b8240d0, altdest=0x00007f8e6b8240d0, qc=0x00007ffee5d76d30) at pquery.c:786:5
frame #17: 0x000000010a4a9393 postgres`exec_simple_query(query_string="UPDATE test SET x = x - 1;") at postgres.c:1214:10
Attachments:
[text/plain] 0001-Share-es_result_relations-of-parent-EState-in-EPQSta.patch (1.3K, ../../cb2b808d-cbaa-4772-76ee-c8809bafcf3d@postgrespro.ru/2-0001-Share-es_result_relations-of-parent-EState-in-EPQSta.patch)
download | inline diff:
From ab067bdeb6de84db80504d2fb05da4385fa02ebc Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru>
Date: Fri, 6 Aug 2021 10:11:52 +0500
Subject: [PATCH] Share es_result_relations of parent EState in EPQState
because FDW direct-modify nodes expect it available.
---
src/backend/executor/execMain.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index b3ce4bae53..a030fec86a 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -2714,10 +2714,12 @@ EvalPlanQualStart(EPQState *epqstate, Plan *planTree)
rcestate->es_output_cid = parentestate->es_output_cid;
/*
- * ResultRelInfos needed by subplans are initialized from scratch when the
- * subplans themselves are initialized.
+ * Share the parent's copy of ResultRelInfos needed by subplans.
+ * We must do this before initializing the subplan, because direct-modify
+ * FDWs expect their ResultRelInfos to be available.
*/
- rcestate->es_result_relations = NULL;
+ rcestate->es_result_relations = parentestate->es_result_relations;
+
/* es_trig_target_relations must NOT be copied */
rcestate->es_top_eflags = parentestate->es_top_eflags;
rcestate->es_instrument = parentestate->es_instrument;
--
2.31.1
[application/sql] t.sql (552B, ../../cb2b808d-cbaa-4772-76ee-c8809bafcf3d@postgrespro.ru/3-t.sql)
download
[text/plain] backtrace.txt (3.0K, ../../cb2b808d-cbaa-4772-76ee-c8809bafcf3d@postgrespro.ru/4-backtrace.txt)
download | inline:
* frame #0: 0x000000010a21df66 postgres`ExecInitForeignScan(node=0x00007f8e6b823ba8, estate=0x00007f8e6d809b20, eflags=0) at nodeForeignscan.c:230:30
frame #1: 0x000000010a1fc3c3 postgres`ExecInitNode(node=0x00007f8e6b823ba8, estate=0x00007f8e6d809b20, eflags=0) at execProcnode.c:284:27
frame #2: 0x000000010a218265 postgres`ExecInitAppend(node=0x00007f8e6b80e150, estate=0x00007f8e6d809b20, eflags=0) at nodeAppend.c:232:27
frame #3: 0x000000010a1fc193 postgres`ExecInitNode(node=0x00007f8e6b80e150, estate=0x00007f8e6d809b20, eflags=0) at execProcnode.c:181:27
frame #4: 0x000000010a1f4be1 postgres`EvalPlanQualStart(epqstate=0x00007f8e6b817490, planTree=0x00007f8e6b80e150) at execMain.c:2817:31
frame #5: 0x000000010a1f399f postgres`EvalPlanQualBegin(epqstate=0x00007f8e6b817490) at execMain.c:2630:3
frame #6: 0x000000010a1f38de postgres`EvalPlanQual(epqstate=0x00007f8e6b817490, relation=0x00007f8e6b59e160, rti=2, inputslot=0x00007f8e6b830688) at execMain.c:2370:2
frame #7: 0x000000010a24085c postgres`ExecUpdate(mtstate=0x00007f8e6b8173a8, resultRelInfo=0x00007f8e6b8175c0, tupleid=0x00007ffee5d76858, oldtuple=0x0000000000000000, slot=0x00007f8e6b82f9f0, planSlot=0x00007f8e6b8194f0, epqstate=0x00007f8e6b817490, estate=0x00007f8e6b817120, canSetTag=true) at nodeModifyTable.c:1861:18
frame #8: 0x000000010a23e964 postgres`ExecModifyTable(pstate=0x00007f8e6b8173a8) at nodeModifyTable.c:2592:12
frame #9: 0x000000010a1fc8c2 postgres`ExecProcNodeFirst(node=0x00007f8e6b8173a8) at execProcnode.c:463:9
frame #10: 0x000000010a1f4762 postgres`ExecProcNode(node=0x00007f8e6b8173a8) at executor.h:257:9
frame #11: 0x000000010a1efe01 postgres`ExecutePlan(estate=0x00007f8e6b817120, planstate=0x00007f8e6b8173a8, use_parallel_mode=false, operation=CMD_UPDATE, sendTuples=false, numberTuples=0, direction=ForwardScanDirection, dest=0x00007f8e6b8240d0, execute_once=true) at execMain.c:1551:10
frame #12: 0x000000010a1efcd0 postgres`standard_ExecutorRun(queryDesc=0x00007f8e6b815b20, direction=ForwardScanDirection, count=0, execute_once=true) at execMain.c:361:3
frame #13: 0x000000010a1efa61 postgres`ExecutorRun(queryDesc=0x00007f8e6b815b20, direction=ForwardScanDirection, count=0, execute_once=true) at execMain.c:305:3
frame #14: 0x000000010a4aff59 postgres`ProcessQuery(plan=0x00007f8e6b824038, sourceText="UPDATE test SET x = x - 1;", params=0x0000000000000000, queryEnv=0x0000000000000000, dest=0x00007f8e6b8240d0, qc=0x00007ffee5d76d30) at pquery.c:160:2
frame #15: 0x000000010a4aede3 postgres`PortalRunMulti(portal=0x00007f8e6e036120, isTopLevel=true, setHoldSnapshot=false, dest=0x00007f8e6b8240d0, altdest=0x00007f8e6b8240d0, qc=0x00007ffee5d76d30) at pquery.c:1266:5
frame #16: 0x000000010a4ae3a8 postgres`PortalRun(portal=0x00007f8e6e036120, count=9223372036854775807, isTopLevel=true, run_once=true, dest=0x00007f8e6b8240d0, altdest=0x00007f8e6b8240d0, qc=0x00007ffee5d76d30) at pquery.c:786:5
frame #17: 0x000000010a4a9393 postgres`exec_simple_query(query_string="UPDATE test SET x = x - 1;") at postgres.c:1214:10
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: SEGFAULT on a concurrent UPDATE of mix of local and foreign partitions
@ 2021-08-09 20:29 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Andrey Lepikhov <a.lepikhov@postgrespro.ru>
0 siblings, 1 reply; 4+ messages in thread
From: Heikki Linnakangas @ 2021-08-09 20:29 UTC (permalink / raw)
To: Andrey Lepikhov <a.lepikhov@postgrespro.ru>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Amit Langote <amitlangote09@gmail.com>
On 06/08/2021 08:34, Andrey Lepikhov wrote:
> Hi,
> Postgres SEGFAULT'ed on the UPDATE of mix of local and foreign partitions.
> Initialization - see t.sql
> For replaying this segfault just execute in parallel:
> UPDATE test SET x = x - 1;
>
> The problem was introduced by commit 1375422.
> ExecUpdate has found a concurrently updated tuples and starts subplan
> evaluation. This operation creates new EState for EPQState and sets
> es_result_relations in NULL value. Next, ExecInitNode(subplan) is
> launched and underlying ExecInitForeignScan tries to access to an
> element of es_result_relations. This causes SEGFAULT.
Thanks for the report!
> I studied this problem shortly. I think, EPQState can use
> es_result_relations of a parent EState. Patch in attachment fixes this.
> check-world passed clearly.
Hmm, that seems to work, but I have a few questions:
Can we be sure that the es_result_relations entry of the result relation
has been initialized when the EPQState is constructed? The general model
now is that you initialize them lazily, when needed.
Does it ever make sense to re-evaluate a direct Foreign Update as part
of EvalPlanQual? It doesn't seem sane to me, you cannot just run an
UPDATE statement again and expect it to be idempotent. So I think it
would make sense skip this during EvalPlanQual processing altogether.
And no need to call the FDW's Begin/EndDirectModify functions either.
It's not clear to me what guarantee there is that a direct-modify
Foreign Update/Delete node is never re-evaluated as part of
EvalPlanQual. Is it because the a ModifyTable node is always at the top
of the plan tree? Or is it because the FDW never uses direct-modify in
queries where EvalPlanQual might be needed? Or because we use
ROW_MARK_COPY in those cases?
In any case, re-evaluating a direct-modify statement doesn't seem sane,
so I propose the attached patch to add some runtime checks for that and
to avoid the original segfault.
- Heikki
Attachments:
[text/x-patch] 0001-Fix-segfault-during-EvalPlanQual-with-mix-of-local-a.patch (0B, ../../1a6c8eab-87e5-2dce-590f-e4aef85d6e27@iki.fi/2-0001-Fix-segfault-during-EvalPlanQual-with-mix-of-local-a.patch)
download
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: SEGFAULT on a concurrent UPDATE of mix of local and foreign partitions
@ 2021-08-10 09:18 Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
parent: Heikki Linnakangas <hlinnaka@iki.fi>
0 siblings, 1 reply; 4+ messages in thread
From: Andrey V. Lepikhov @ 2021-08-10 09:18 UTC (permalink / raw)
To: Heikki Linnakangas <hlinnaka@iki.fi>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Amit Langote <amitlangote09@gmail.com>
On 8/10/21 1:29 AM, Heikki Linnakangas wrote:
> On 06/08/2021 08:34, Andrey Lepikhov wrote:
>> Hi,
>> Postgres SEGFAULT'ed on the UPDATE of mix of local and foreign
>> partitions.
>> Initialization - see t.sql
>> For replaying this segfault just execute in parallel:
>> UPDATE test SET x = x - 1;
>>
>> The problem was introduced by commit 1375422.
>> ExecUpdate has found a concurrently updated tuples and starts subplan
>> evaluation. This operation creates new EState for EPQState and sets
>> es_result_relations in NULL value. Next, ExecInitNode(subplan) is
>> launched and underlying ExecInitForeignScan tries to access to an
>> element of es_result_relations. This causes SEGFAULT.
>
> Thanks for the report!
>
>> I studied this problem shortly. I think, EPQState can use
>> es_result_relations of a parent EState. Patch in attachment fixes this.
>> check-world passed clearly.
>
> Hmm, that seems to work, but I have a few questions:
>
> Can we be sure that the es_result_relations entry of the result relation
> has been initialized when the EPQState is constructed? The general model
> now is that you initialize them lazily, when needed.
As I understand the code correctly - yes, because a ModifyTable node
higher and will initialize this field earlier. But in many cases it is
not needed.
>
> Does it ever make sense to re-evaluate a direct Foreign Update as part
> of EvalPlanQual? It doesn't seem sane to me, you cannot just run an
> UPDATE statement again and expect it to be idempotent. So I think it
> would make sense skip this during EvalPlanQual processing altogether.
> And no need to call the FDW's Begin/EndDirectModify functions either.
Agree. It is my first dive into the EPQ code. After studying i think
this makes no sense.
>
> It's not clear to me what guarantee there is that a direct-modify
> Foreign Update/Delete node is never re-evaluated as part of
> EvalPlanQual. Is it because the a ModifyTable node is always at the top
> of the plan tree? Or is it because the FDW never uses direct-modify in
> queries where EvalPlanQual might be needed? Or because we use
> ROW_MARK_COPY in those cases?
ModifyTable node may be found in an underlying CTE, for example. And,
right now, it's not clear for me too.
>
> In any case, re-evaluating a direct-modify statement doesn't seem sane,
> so I propose the attached patch to add some runtime checks for that and
> to avoid the original segfaultYour patch looks much better.
--
regards,
Andrey Lepikhov
Postgres Professional
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: SEGFAULT on a concurrent UPDATE of mix of local and foreign partitions
@ 2021-08-12 08:38 Heikki Linnakangas <hlinnaka@iki.fi>
parent: Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
0 siblings, 0 replies; 4+ messages in thread
From: Heikki Linnakangas @ 2021-08-12 08:38 UTC (permalink / raw)
To: Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>; PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>; Amit Langote <amitlangote09@gmail.com>
On 10/08/2021 12:18, Andrey V. Lepikhov wrote:
> On 8/10/21 1:29 AM, Heikki Linnakangas wrote:
>> Does it ever make sense to re-evaluate a direct Foreign Update as part
>> of EvalPlanQual? It doesn't seem sane to me, you cannot just run an
>> UPDATE statement again and expect it to be idempotent. So I think it
>> would make sense skip this during EvalPlanQual processing altogether.
>> And no need to call the FDW's Begin/EndDirectModify functions either.
> Agree. It is my first dive into the EPQ code. After studying i think
> this makes no sense.
Pushed the fix to master and v14. Thanks for the report!
- Heikki
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2021-08-12 08:38 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-08-06 05:34 SEGFAULT on a concurrent UPDATE of mix of local and foreign partitions Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-08-09 20:29 ` Heikki Linnakangas <hlinnaka@iki.fi>
2021-08-10 09:18 ` Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-08-12 08:38 ` Heikki Linnakangas <hlinnaka@iki.fi>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox