pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedThe case when AsyncAppend exists also in the qual of Async ForeignScan
20+ messages / 3 participants
[nested] [flat]
Commits from this thread:
2021-07-30 1ec7fca859 1cf7fb376a postgres_fdw: Fix handling of pending asynchronous requests.
* The case when AsyncAppend exists also in the qual of Async ForeignScan
@ 2021-07-02 13:24 Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-02 23:35 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-22 11:14 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 2 replies; 20+ messages in thread
From: Andrey Lepikhov @ 2021-07-02 13:24 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org, Etsuro Fujita <etsuro.fujita@gmail.com>
Hi,
I found one bug. Look at the case:
CREATE TABLE test (x int) PARTITION BY HASH (x);
CREATE TABLE test_1 (x int);
CREATE TABLE test_2 (x int);
CREATE FOREIGN TABLE ftest_1 PARTITION OF test
FOR VALUES WITH (modulus 2, remainder 0)
SERVER loopback OPTIONS (table_name 'test_1');
CREATE FOREIGN TABLE ftest_2 PARTITION OF test
FOR VALUES WITH (modulus 2, remainder 1)
SERVER loopback2 OPTIONS (table_name 'test_2');
INSERT INTO test (SELECT * FROM generate_series(1,10));
ANALYZE test,test_1,test_2,ftest_1,ftest_2;
EXPLAIN (ANALYZE)
SELECT * FROM test WHERE x NOT IN (
SELECT avg(x) FROM test WHERE x > 10
);
ERROR: InstrEndLoop called on running node
When I added ', COSTS OFF, SUMMARY OFF, TIMING OFF' this example fall
down into the SEGFAULT.
I prepared quick fix for this problem (see patch in attachment). Maybe
it is'nt a best solution but it can speedup search of it.
--
regards,
Andrey Lepikhov
Postgres Professional
Attachments:
0001-Do-not-take-a-tuple-immediately-after-finishing-of-a.patch (text/plain, 8.0K) download
From ce7a68c951344aa2163fb590ecdf3bb0649b83ae Mon Sep 17 00:00:00 2001
From: "Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru>
Date: Fri, 2 Jul 2021 16:20:19 +0300
Subject: [PATCH] Do not take a tuple immediately after finishing of async
request if this request was initialized by another append node.
---
contrib/postgres_fdw/connection.c | 4 +--
.../postgres_fdw/expected/postgres_fdw.out | 36 +++++++++++++++++++
contrib/postgres_fdw/postgres_fdw.c | 25 ++++++++++---
contrib/postgres_fdw/postgres_fdw.h | 2 +-
contrib/postgres_fdw/sql/postgres_fdw.sql | 22 ++++++++++++
5 files changed, 81 insertions(+), 8 deletions(-)
diff --git a/contrib/postgres_fdw/connection.c b/contrib/postgres_fdw/connection.c
index 82aa14a65d..163825f4d7 100644
--- a/contrib/postgres_fdw/connection.c
+++ b/contrib/postgres_fdw/connection.c
@@ -203,7 +203,7 @@ GetConnection(UserMapping *user, bool will_prep_stmt, PgFdwConnState **state)
{
/* Process a pending asynchronous request if any. */
if (entry->state.pendingAreq)
- process_pending_request(entry->state.pendingAreq);
+ process_pending_request(entry->state.pendingAreq, true);
/* Start a new transaction or subtransaction if needed. */
begin_remote_xact(entry);
}
@@ -686,7 +686,7 @@ pgfdw_exec_query(PGconn *conn, const char *query, PgFdwConnState *state)
{
/* First, process a pending asynchronous request, if any. */
if (state && state->pendingAreq)
- process_pending_request(state->pendingAreq);
+ process_pending_request(state->pendingAreq, true);
/*
* Submit a query. Since we don't use non-blocking mode, this also can
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 31b5de91ad..9386453e60 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -10470,7 +10470,43 @@ SELECT * FROM async_pt;
-> Seq Scan on async_p3 async_pt_3 (actual rows=0 loops=1)
(4 rows)
+--
+-- The case when we have the Async Append in the
+-- qual of the Async ForeignScan node.
+--
+CREATE TABLE test (x int) PARTITION BY HASH (x);
+CREATE TABLE test_1 (x int);
+CREATE TABLE test_2 (x int);
+CREATE FOREIGN TABLE ftest_1 PARTITION OF test
+ FOR VALUES WITH (modulus 2, remainder 0)
+ SERVER loopback OPTIONS (table_name 'test_1');
+CREATE FOREIGN TABLE ftest_2 PARTITION OF test
+ FOR VALUES WITH (modulus 2, remainder 1)
+ SERVER loopback2 OPTIONS (table_name 'test_2');
+INSERT INTO test (SELECT * FROM generate_series(1,10));
+ANALYZE test,test_1,test_2,ftest_1,ftest_2;
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+SELECT * FROM test WHERE x NOT IN (
+ SELECT avg(x) FROM test WHERE x > 10
+);
+ QUERY PLAN
+----------------------------------------------------------------------------------------
+ Append (actual rows=0 loops=1)
+ -> Async Foreign Scan on ftest_1 test_1 (actual rows=0 loops=1)
+ Filter: (NOT (hashed SubPlan 1))
+ Rows Removed by Filter: 2
+ SubPlan 1
+ -> Aggregate (actual rows=1 loops=2)
+ -> Append (actual rows=0 loops=2)
+ -> Async Foreign Scan on ftest_1 test_4 (actual rows=0 loops=2)
+ -> Async Foreign Scan on ftest_2 test_5 (actual rows=0 loops=2)
+ -> Async Foreign Scan on ftest_2 test_2 (actual rows=0 loops=1)
+ Filter: (NOT (hashed SubPlan 1))
+ Rows Removed by Filter: 8
+(12 rows)
+
-- Clean up
+DROP TABLE test CASCADE;
DROP TABLE async_pt;
DROP TABLE base_tbl1;
DROP TABLE base_tbl2;
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index fafbab6b02..1c4ae328d0 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -3696,7 +3696,7 @@ create_cursor(ForeignScanState *node)
/* First, process a pending asynchronous request, if any. */
if (fsstate->conn_state->pendingAreq)
- process_pending_request(fsstate->conn_state->pendingAreq);
+ process_pending_request(fsstate->conn_state->pendingAreq, true);
/*
* Construct array of query parameter values in text format. We do the
@@ -4066,7 +4066,7 @@ execute_foreign_modify(EState *estate,
/* First, process a pending asynchronous request, if any. */
if (fmstate->conn_state->pendingAreq)
- process_pending_request(fmstate->conn_state->pendingAreq);
+ process_pending_request(fmstate->conn_state->pendingAreq, true);
/*
* If the existing query was deparsed and prepared for a different number
@@ -4509,7 +4509,7 @@ execute_dml_stmt(ForeignScanState *node)
/* First, process a pending asynchronous request, if any. */
if (dmstate->conn_state->pendingAreq)
- process_pending_request(dmstate->conn_state->pendingAreq);
+ process_pending_request(dmstate->conn_state->pendingAreq, true);
/*
* Construct array of query parameter values in text format.
@@ -6850,7 +6850,7 @@ postgresForeignAsyncConfigureWait(AsyncRequest *areq)
*/
if (GetNumRegisteredWaitEvents(set) > 1)
return;
- process_pending_request(pendingAreq);
+ process_pending_request(pendingAreq, false);
fetch_more_data_begin(areq);
}
else if (pendingAreq->requestee != areq->requestee)
@@ -6995,7 +6995,7 @@ fetch_more_data_begin(AsyncRequest *areq)
* Process a pending asynchronous request.
*/
void
-process_pending_request(AsyncRequest *areq)
+process_pending_request(AsyncRequest *areq, bool fetch)
{
ForeignScanState *node = (ForeignScanState *) areq->requestee;
PgFdwScanState *fsstate PG_USED_FOR_ASSERTS_ONLY = (PgFdwScanState *) node->fdw_state;
@@ -7015,6 +7015,21 @@ process_pending_request(AsyncRequest *areq)
fetch_more_data(node);
+ /*
+ * If the request are made by another append we will only prepare connection
+ * for the next query and don't take a tuple immediately. It is needed to
+ * prevent possible recursion into a qual subplan.
+ */
+ if (!fetch)
+ {
+ AppendState *node = (AppendState *) areq->requestor;
+
+ ExecAsyncRequestDone(areq, NULL);
+ node->as_needrequest = bms_add_member(node->as_needrequest,
+ areq->request_index);
+ return;
+ }
+
/* We need to send a new query afterwards; don't fetch */
produce_tuple_asynchronously(areq, false);
diff --git a/contrib/postgres_fdw/postgres_fdw.h b/contrib/postgres_fdw/postgres_fdw.h
index 9591c0f6c2..02c9c7bfa6 100644
--- a/contrib/postgres_fdw/postgres_fdw.h
+++ b/contrib/postgres_fdw/postgres_fdw.h
@@ -137,7 +137,7 @@ typedef struct PgFdwConnState
/* in postgres_fdw.c */
extern int set_transmission_modes(void);
extern void reset_transmission_modes(int nestlevel);
-extern void process_pending_request(AsyncRequest *areq);
+extern void process_pending_request(AsyncRequest *areq, bool fetch);
/* in connection.c */
extern PGconn *GetConnection(UserMapping *user, bool will_prep_stmt,
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 286dd99573..38caf96cc8 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3328,7 +3328,29 @@ DELETE FROM async_p3;
EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
SELECT * FROM async_pt;
+--
+-- The case when we have the Async Append in the
+-- qual of the Async ForeignScan node.
+--
+CREATE TABLE test (x int) PARTITION BY HASH (x);
+CREATE TABLE test_1 (x int);
+CREATE TABLE test_2 (x int);
+CREATE FOREIGN TABLE ftest_1 PARTITION OF test
+ FOR VALUES WITH (modulus 2, remainder 0)
+ SERVER loopback OPTIONS (table_name 'test_1');
+CREATE FOREIGN TABLE ftest_2 PARTITION OF test
+ FOR VALUES WITH (modulus 2, remainder 1)
+ SERVER loopback2 OPTIONS (table_name 'test_2');
+INSERT INTO test (SELECT * FROM generate_series(1,10));
+ANALYZE test,test_1,test_2,ftest_1,ftest_2;
+
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+SELECT * FROM test WHERE x NOT IN (
+ SELECT avg(x) FROM test WHERE x > 10
+);
+
-- Clean up
+DROP TABLE test CASCADE;
DROP TABLE async_pt;
DROP TABLE base_tbl1;
DROP TABLE base_tbl2;
--
2.31.1
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
@ 2021-07-02 23:35 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-08 07:49 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
1 sibling, 1 reply; 20+ messages in thread
From: Etsuro Fujita @ 2021-07-02 23:35 UTC (permalink / raw)
To: Andrey Lepikhov <a.lepikhov@postgrespro.ru>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Hi Andrey,
On Fri, Jul 2, 2021 at 10:24 PM Andrey Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
> I found one bug. Look at the case:
>
> CREATE TABLE test (x int) PARTITION BY HASH (x);
> CREATE TABLE test_1 (x int);
> CREATE TABLE test_2 (x int);
> CREATE FOREIGN TABLE ftest_1 PARTITION OF test
> FOR VALUES WITH (modulus 2, remainder 0)
> SERVER loopback OPTIONS (table_name 'test_1');
> CREATE FOREIGN TABLE ftest_2 PARTITION OF test
> FOR VALUES WITH (modulus 2, remainder 1)
> SERVER loopback2 OPTIONS (table_name 'test_2');
> INSERT INTO test (SELECT * FROM generate_series(1,10));
> ANALYZE test,test_1,test_2,ftest_1,ftest_2;
>
> EXPLAIN (ANALYZE)
> SELECT * FROM test WHERE x NOT IN (
> SELECT avg(x) FROM test WHERE x > 10
> );
> ERROR: InstrEndLoop called on running node
>
> When I added ', COSTS OFF, SUMMARY OFF, TIMING OFF' this example fall
> down into the SEGFAULT.
>
> I prepared quick fix for this problem (see patch in attachment). Maybe
> it is'nt a best solution but it can speedup search of it.
I’ll looking into this. Thanks for the report and patch!
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-02 23:35 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
@ 2021-07-08 07:49 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-08 08:06 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-08 10:18 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-08 10:32 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
0 siblings, 3 replies; 20+ messages in thread
From: Etsuro Fujita @ 2021-07-08 07:49 UTC (permalink / raw)
To: Andrey Lepikhov <a.lepikhov@postgrespro.ru>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Sat, Jul 3, 2021 at 8:35 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> On Fri, Jul 2, 2021 at 10:24 PM Andrey Lepikhov
> <a.lepikhov@postgrespro.ru> wrote:
> > I found one bug. Look at the case:
> >
> > CREATE TABLE test (x int) PARTITION BY HASH (x);
> > CREATE TABLE test_1 (x int);
> > CREATE TABLE test_2 (x int);
> > CREATE FOREIGN TABLE ftest_1 PARTITION OF test
> > FOR VALUES WITH (modulus 2, remainder 0)
> > SERVER loopback OPTIONS (table_name 'test_1');
> > CREATE FOREIGN TABLE ftest_2 PARTITION OF test
> > FOR VALUES WITH (modulus 2, remainder 1)
> > SERVER loopback2 OPTIONS (table_name 'test_2');
> > INSERT INTO test (SELECT * FROM generate_series(1,10));
> > ANALYZE test,test_1,test_2,ftest_1,ftest_2;
> >
> > EXPLAIN (ANALYZE)
> > SELECT * FROM test WHERE x NOT IN (
> > SELECT avg(x) FROM test WHERE x > 10
> > );
> > ERROR: InstrEndLoop called on running node
> >
> > When I added ', COSTS OFF, SUMMARY OFF, TIMING OFF' this example fall
> > down into the SEGFAULT.
> >
> > I prepared quick fix for this problem (see patch in attachment). Maybe
> > it is'nt a best solution but it can speedup search of it.
>
> I’ll looking into this. Thanks for the report and patch!
I tried to reproduce this in my environment using HEAD and PG14, but I
couldn't. Could you elaborate a bit more on how to reproduce this?
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-02 23:35 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-08 07:49 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
@ 2021-07-08 08:06 ` Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-08 08:32 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2 siblings, 1 reply; 20+ messages in thread
From: Andrey Lepikhov @ 2021-07-08 08:06 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On 8/7/21 10:49, Etsuro Fujita wrote:
> On Sat, Jul 3, 2021 at 8:35 AM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>> On Fri, Jul 2, 2021 at 10:24 PM Andrey Lepikhov
>> <a.lepikhov@postgrespro.ru> wrote:
>>> I found one bug. Look at the case:
>>>
>>> CREATE TABLE test (x int) PARTITION BY HASH (x);
>>> CREATE TABLE test_1 (x int);
>>> CREATE TABLE test_2 (x int);
>>> CREATE FOREIGN TABLE ftest_1 PARTITION OF test
>>> FOR VALUES WITH (modulus 2, remainder 0)
>>> SERVER loopback OPTIONS (table_name 'test_1');
>>> CREATE FOREIGN TABLE ftest_2 PARTITION OF test
>>> FOR VALUES WITH (modulus 2, remainder 1)
>>> SERVER loopback2 OPTIONS (table_name 'test_2');
>>> INSERT INTO test (SELECT * FROM generate_series(1,10));
>>> ANALYZE test,test_1,test_2,ftest_1,ftest_2;
>>>
>>> EXPLAIN (ANALYZE)
>>> SELECT * FROM test WHERE x NOT IN (
>>> SELECT avg(x) FROM test WHERE x > 10
>>> );
>>> ERROR: InstrEndLoop called on running node
>>>
>>> When I added ', COSTS OFF, SUMMARY OFF, TIMING OFF' this example fall
>>> down into the SEGFAULT.
>>>
>>> I prepared quick fix for this problem (see patch in attachment). Maybe
>>> it is'nt a best solution but it can speedup search of it.
>>
>> I’ll looking into this. Thanks for the report and patch!
>
> I tried to reproduce this in my environment using HEAD and PG14, but I
> couldn't. Could you elaborate a bit more on how to reproduce this?
>
> Best regards,
> Etsuro Fujita
>
Initially I cought this bug during TPC-H benchmarking, query Q16, right
after start of execution.
Key thing here is using a hashed subplan.
--
regards,
Andrey Lepikhov
Postgres Professional
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-02 23:35 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-08 07:49 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-08 08:06 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
@ 2021-07-08 08:32 ` Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 0 replies; 20+ messages in thread
From: Etsuro Fujita @ 2021-07-08 08:32 UTC (permalink / raw)
To: Andrey Lepikhov <a.lepikhov@postgrespro.ru>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Thu, Jul 8, 2021 at 5:06 PM Andrey Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
> Initially I cought this bug during TPC-H benchmarking, query Q16, right
> after start of execution.
> Key thing here is using a hashed subplan.
The plan I got also uses a hashed SubPlan; moreover, the plan is the
same as the one shown in your patch.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-02 23:35 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-08 07:49 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
@ 2021-07-08 10:18 ` Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-08 17:10 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2 siblings, 1 reply; 20+ messages in thread
From: Andrey Lepikhov @ 2021-07-08 10:18 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On 8/7/21 10:49, Etsuro Fujita wrote:
> I tried to reproduce this in my environment using HEAD and PG14, but I
> couldn't. Could you elaborate a bit more on how to reproduce this?
Interesting...
I use the only script (see t.sql) right after the instance has started.
Output see in t.out
All my specific preferences:
autovacuum = 'off'
max_worker_processes = 64
max_parallel_workers_per_gather = 16
max_parallel_workers = 32
parallel_setup_cost = 100
parallel_tuple_cost = 0.05
min_parallel_table_scan_size = 0
--
regards,
Andrey Lepikhov
Postgres Professional
Attachments:
t.sql (application/sql, 954B) download
t.out (text/plain, 517B) download
psql -f t.sql
DROP TABLE
psql:t.sql:2: NOTICE: drop cascades to 4 other objects
DETAIL: drop cascades to server lb1
drop cascades to user mapping for public on server lb1
drop cascades to server lb2
drop cascades to user mapping for public on server lb2
DROP EXTENSION
CREATE EXTENSION
CREATE SERVER
CREATE SERVER
CREATE USER MAPPING
CREATE USER MAPPING
CREATE TABLE
CREATE TABLE
CREATE TABLE
CREATE FOREIGN TABLE
CREATE FOREIGN TABLE
INSERT 0 10
ANALYZE
psql:t.sql:26: ERROR: InstrEndLoop called on running node
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-02 23:35 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-08 07:49 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-08 10:18 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
@ 2021-07-08 17:10 ` Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 0 replies; 20+ messages in thread
From: Etsuro Fujita @ 2021-07-08 17:10 UTC (permalink / raw)
To: Andrey Lepikhov <a.lepikhov@postgrespro.ru>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Thu, Jul 8, 2021 at 7:18 PM Andrey Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
> On 8/7/21 10:49, Etsuro Fujita wrote:
> > I tried to reproduce this in my environment using HEAD and PG14, but I
> > couldn't. Could you elaborate a bit more on how to reproduce this?
> Interesting...
> I use the only script (see t.sql) right after the instance has started.
Reproduced here. I’ll continue to investigate this.
The reason I couldn’t reproduce this is my setting where partitions
were created on the same foreign server. :-(
Thanks!
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-02 23:35 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-08 07:49 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
@ 2021-07-08 10:32 ` Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2 siblings, 0 replies; 20+ messages in thread
From: Andrey Lepikhov @ 2021-07-08 10:32 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On 8/7/21 10:49, Etsuro Fujita wrote:
Important. If you want to get the InstrEndLoop error, you need to enable
timing in the EXPLAIN operation.
--
regards,
Andrey Lepikhov
Postgres Professional
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
@ 2021-07-22 11:14 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-23 06:09 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-23 11:26 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-27 06:22 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
1 sibling, 3 replies; 20+ messages in thread
From: Etsuro Fujita @ 2021-07-22 11:14 UTC (permalink / raw)
To: Andrey Lepikhov <a.lepikhov@postgrespro.ru>; +Cc: pgsql-bugs@lists.postgresql.org
On Fri, Jul 2, 2021 at 10:24 PM Andrey Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
> I found one bug. Look at the case:
>
> CREATE TABLE test (x int) PARTITION BY HASH (x);
> CREATE TABLE test_1 (x int);
> CREATE TABLE test_2 (x int);
> CREATE FOREIGN TABLE ftest_1 PARTITION OF test
> FOR VALUES WITH (modulus 2, remainder 0)
> SERVER loopback OPTIONS (table_name 'test_1');
> CREATE FOREIGN TABLE ftest_2 PARTITION OF test
> FOR VALUES WITH (modulus 2, remainder 1)
> SERVER loopback2 OPTIONS (table_name 'test_2');
> INSERT INTO test (SELECT * FROM generate_series(1,10));
> ANALYZE test,test_1,test_2,ftest_1,ftest_2;
>
> EXPLAIN (ANALYZE)
> SELECT * FROM test WHERE x NOT IN (
> SELECT avg(x) FROM test WHERE x > 10
> );
> ERROR: InstrEndLoop called on running node
>
> When I added ', COSTS OFF, SUMMARY OFF, TIMING OFF' this example fall
> down into the SEGFAULT.
>
> I prepared quick fix for this problem (see patch in attachment). Maybe
> it is'nt a best solution but it can speedup search of it.
@@ -7015,6 +7015,21 @@ process_pending_request(AsyncRequest *areq)
fetch_more_data(node);
+ /*
+ * If the request are made by another append we will only prepare connection
+ * for the next query and don't take a tuple immediately. It is needed to
+ * prevent possible recursion into a qual subplan.
+ */
+ if (!fetch)
+ {
+ AppendState *node = (AppendState *) areq->requestor;
+
+ ExecAsyncRequestDone(areq, NULL);
+ node->as_needrequest = bms_add_member(node->as_needrequest,
+ areq->request_index);
+ return;
+ }
I don’t think this is a good idea, because it is pretty inconsistent,
as doing ExecAsyncRequestDone(areq, NULL) means that there are no more
tuples while changing as_needrequest like that means that there is at
least one tuple to return. This would happen to work, but for
example, if we add to the core more sanity checks on AsyncRequests,
this would not work well anymore. I tried to devise a consistent
solution for this issue, but I couldn’t. So I feel inclined to
disable async execution in cases where async-capable nodes access to
subplans (or initplans), for now.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-22 11:14 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
@ 2021-07-23 06:09 ` Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-25 08:55 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2 siblings, 1 reply; 20+ messages in thread
From: Andrey V. Lepikhov @ 2021-07-23 06:09 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org
On 7/22/21 4:14 PM, Etsuro Fujita wrote:
> On Fri, Jul 2, 2021 at 10:24 PM Andrey Lepikhov
> @@ -7015,6 +7015,21 @@ process_pending_request(AsyncRequest *areq)
>
> fetch_more_data(node);
>
> + /*
> + * If the request are made by another append we will only prepare connection
> + * for the next query and don't take a tuple immediately. It is needed to
> + * prevent possible recursion into a qual subplan.
> + */
> + if (!fetch)
> + {
> + AppendState *node = (AppendState *) areq->requestor;
> +
> + ExecAsyncRequestDone(areq, NULL);
> + node->as_needrequest = bms_add_member(node->as_needrequest,
> + areq->request_index);
> + return;
> + }
>
> I don’t think this is a good idea, because it is pretty inconsistent,
> as doing ExecAsyncRequestDone(areq, NULL) means that there are no more
> tuples while changing as_needrequest like that means that there is at
> least one tuple to return. This would happen to work, but for
> example, if we add to the core more sanity checks on AsyncRequests,
> this would not work well anymore.
I agree.
> I tried to devise a consistent
> solution for this issue, but I couldn’t. So I feel inclined to
> disable async execution in cases where async-capable nodes access to
> subplans (or initplans), for now.
I think it can be done, but only as a temporary solution. InitPlan is a
common planning utility.
Maybe we can split async logic into:
- receiving stage, when we only fetch and store tuples,
- evaluating stage, when we form resulting tuple and return by a scan node.
I will think about such solution more.
Also, may be you tell your opinion about an additional optimization of
Async Append [1]?
[1]
https://www.postgresql.org/message-id/edb1331c-e861-0c53-9fdb-f7ca7dfd884d%40postgrespro.ru
--
regards,
Andrey Lepikhov
Postgres Professional
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-22 11:14 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-23 06:09 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
@ 2021-07-25 08:55 ` Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 0 replies; 20+ messages in thread
From: Etsuro Fujita @ 2021-07-25 08:55 UTC (permalink / raw)
To: Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Fri, Jul 23, 2021 at 3:09 PM Andrey V. Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
> On 7/22/21 4:14 PM, Etsuro Fujita wrote:
> > On Fri, Jul 2, 2021 at 10:24 PM Andrey Lepikhov
> > @@ -7015,6 +7015,21 @@ process_pending_request(AsyncRequest *areq)
> >
> > fetch_more_data(node);
> >
> > + /*
> > + * If the request are made by another append we will only prepare connection
> > + * for the next query and don't take a tuple immediately. It is needed to
> > + * prevent possible recursion into a qual subplan.
> > + */
> > + if (!fetch)
> > + {
> > + AppendState *node = (AppendState *) areq->requestor;
> > +
> > + ExecAsyncRequestDone(areq, NULL);
> > + node->as_needrequest = bms_add_member(node->as_needrequest,
> > + areq->request_index);
> > + return;
> > + }
> >
> > I don’t think this is a good idea, because it is pretty inconsistent,
> > as doing ExecAsyncRequestDone(areq, NULL) means that there are no more
> > tuples while changing as_needrequest like that means that there is at
> > least one tuple to return. This would happen to work, but for
> > example, if we add to the core more sanity checks on AsyncRequests,
> > this would not work well anymore.
> > So I feel inclined to
> > disable async execution in cases where async-capable nodes access to
> > subplans (or initplans), for now.
> I think it can be done, but only as a temporary solution.
My concern about that is that such an inconsistency would make the
code complicated, and thus make the maintenance hard.
> Maybe we can split async logic into:
> - receiving stage, when we only fetch and store tuples,
> - evaluating stage, when we form resulting tuple and return by a scan node.
> I will think about such solution more.
One simple solution along this line I came up with, which is not the
rewrite, is to 1) split process_pending_request() into the two steps,
and 2) postpone the second step until we are called from
postgresForeignAsyncConfigureWait(), like the attached, which I think
would be much consistent with the existing logic.
> Also, may be you tell your opinion about an additional optimization of
> Async Append [1]?
Is the optimization related to this issue? (Sorry, I didn’t have time
for reviewing the patch in [1] than expected. I plan to do so next
month.)
Best regards,
Etsuro Fujita
Attachments:
reconsider-process_pending_request.patch (application/octet-stream, 8.8K) download
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index ed25e7a743..931b941e9f 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -10341,9 +10341,60 @@ SELECT * FROM async_pt t1 WHERE t1.b === 505 LIMIT 1;
3505 | 505 | 0505
(1 row)
--- Check with foreign modify
CREATE TABLE local_tbl (a int, b int, c text);
INSERT INTO local_tbl VALUES (1505, 505, 'foo');
+ANALYZE local_tbl;
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT * FROM local_tbl t1 LEFT JOIN (SELECT *, (SELECT count(*) FROM async_pt WHERE a < 3000) FROM async_pt WHERE a < 3000) t2 ON t1.a = t2.a;
+ QUERY PLAN
+----------------------------------------------------------------------------------------
+ Nested Loop Left Join
+ Output: t1.a, t1.b, t1.c, async_pt.a, async_pt.b, async_pt.c, ($0)
+ Join Filter: (t1.a = async_pt.a)
+ InitPlan 1 (returns $0)
+ -> Aggregate
+ Output: count(*)
+ -> Append
+ -> Async Foreign Scan on public.async_p1 async_pt_4
+ Remote SQL: SELECT NULL FROM public.base_tbl1 WHERE ((a < 3000))
+ -> Async Foreign Scan on public.async_p2 async_pt_5
+ Remote SQL: SELECT NULL FROM public.base_tbl2 WHERE ((a < 3000))
+ -> Seq Scan on public.local_tbl t1
+ Output: t1.a, t1.b, t1.c
+ -> Append
+ -> Async Foreign Scan on public.async_p1 async_pt_1
+ Output: async_pt_1.a, async_pt_1.b, async_pt_1.c, $0
+ Remote SQL: SELECT a, b, c FROM public.base_tbl1 WHERE ((a < 3000))
+ -> Async Foreign Scan on public.async_p2 async_pt_2
+ Output: async_pt_2.a, async_pt_2.b, async_pt_2.c, $0
+ Remote SQL: SELECT a, b, c FROM public.base_tbl2 WHERE ((a < 3000))
+(20 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+SELECT * FROM local_tbl t1 LEFT JOIN (SELECT *, (SELECT count(*) FROM async_pt WHERE a < 3000) FROM async_pt WHERE a < 3000) t2 ON t1.a = t2.a;
+ QUERY PLAN
+-----------------------------------------------------------------------------------------
+ Nested Loop Left Join (actual rows=1 loops=1)
+ Join Filter: (t1.a = async_pt.a)
+ Rows Removed by Join Filter: 399
+ InitPlan 1 (returns $0)
+ -> Aggregate (actual rows=1 loops=1)
+ -> Append (actual rows=400 loops=1)
+ -> Async Foreign Scan on async_p1 async_pt_4 (actual rows=200 loops=1)
+ -> Async Foreign Scan on async_p2 async_pt_5 (actual rows=200 loops=1)
+ -> Seq Scan on local_tbl t1 (actual rows=1 loops=1)
+ -> Append (actual rows=400 loops=1)
+ -> Async Foreign Scan on async_p1 async_pt_1 (actual rows=200 loops=1)
+ -> Async Foreign Scan on async_p2 async_pt_2 (actual rows=200 loops=1)
+(12 rows)
+
+SELECT * FROM local_tbl t1 LEFT JOIN (SELECT *, (SELECT count(*) FROM async_pt WHERE a < 3000) FROM async_pt WHERE a < 3000) t2 ON t1.a = t2.a;
+ a | b | c | a | b | c | count
+------+-----+-----+------+-----+------+-------
+ 1505 | 505 | foo | 1505 | 505 | 0505 | 400
+(1 row)
+
+-- Check with foreign modify
CREATE TABLE base_tbl3 (a int, b int, c text);
CREATE FOREIGN TABLE remote_tbl (a int, b int, c text)
SERVER loopback OPTIONS (table_name 'base_tbl3');
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index f15c97ad7a..0443c4e090 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -503,6 +503,7 @@ static void analyze_row_processor(PGresult *res, int row,
PgFdwAnalyzeState *astate);
static void produce_tuple_asynchronously(AsyncRequest *areq, bool fetch);
static void fetch_more_data_begin(AsyncRequest *areq);
+static void complete_pending_request(AsyncRequest *areq);
static HeapTuple make_tuple_from_result_row(PGresult *res,
int row,
Relation rel,
@@ -6829,6 +6830,20 @@ postgresForeignAsyncConfigureWait(AsyncRequest *areq)
/* The core code would have registered postmaster death event */
Assert(GetNumRegisteredWaitEvents(set) >= 1);
+ /*
+ * If process_pending_request() has been invoked on the given request
+ * before we get here, we might have some tuples already; in which case
+ * complete the request
+ */
+ if (fsstate->next_tuple < fsstate->num_tuples)
+ {
+ complete_pending_request(areq);
+ if (areq->request_complete)
+ return;
+ Assert(areq->callback_pending);
+ }
+ Assert(fsstate->next_tuple >= fsstate->num_tuples);
+
/* Begin an asynchronous data fetch if not already done */
if (!pendingAreq)
fetch_more_data_begin(areq);
@@ -6844,6 +6859,8 @@ postgresForeignAsyncConfigureWait(AsyncRequest *areq)
* we might end up with no configured events other than the postmaster
* death event.
*/
+ if (!bms_is_empty(requestor->as_needrequest))
+ return;
if (GetNumRegisteredWaitEvents(set) > 1)
return;
process_pending_request(pendingAreq);
@@ -6995,23 +7012,40 @@ process_pending_request(AsyncRequest *areq)
{
ForeignScanState *node = (ForeignScanState *) areq->requestee;
PgFdwScanState *fsstate PG_USED_FOR_ASSERTS_ONLY = (PgFdwScanState *) node->fdw_state;
- EState *estate = node->ss.ps.state;
- MemoryContext oldcontext;
+
+ /* The request would have been pending for a callback */
+ Assert(areq->callback_pending);
/* The request should be currently in-process */
Assert(fsstate->conn_state->pendingAreq == areq);
- oldcontext = MemoryContextSwitchTo(estate->es_query_cxt);
+ fetch_more_data(node);
+
+ /* If we didn't get any tuples, must be end of data; nothing more to do */
+ if (fsstate->next_tuple >= fsstate->num_tuples)
+ {
+ /* Unlike AsyncNotify, we unset callback_pending ourselves */
+ areq->callback_pending = false;
+ /* Mark the request as complete */
+ ExecAsyncRequestDone(areq, NULL);
+ /* Unlike AsyncNotify, we call ExecAsyncResponse ourselves */
+ ExecAsyncResponse(areq);
+ }
+}
+/*
+ * Complete a pending asynchronous request.
+ */
+static void
+complete_pending_request(AsyncRequest *areq)
+{
/* The request would have been pending for a callback */
Assert(areq->callback_pending);
/* Unlike AsyncNotify, we unset callback_pending ourselves */
areq->callback_pending = false;
- fetch_more_data(node);
-
- /* We need to send a new query afterwards; don't fetch */
+ /* We begin a fetch afterwards if necessary; don't fetch */
produce_tuple_asynchronously(areq, false);
/* Unlike AsyncNotify, we call ExecAsyncResponse ourselves */
@@ -7021,8 +7055,6 @@ process_pending_request(AsyncRequest *areq)
if (areq->requestee->instrument)
InstrUpdateTupleCount(areq->requestee->instrument,
TupIsNull(areq->result) ? 0.0 : 1.0);
-
- MemoryContextSwitchTo(oldcontext);
}
/*
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 02a6b15a13..a4bc15186e 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3280,10 +3280,17 @@ EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
SELECT * FROM async_pt t1 WHERE t1.b === 505 LIMIT 1;
SELECT * FROM async_pt t1 WHERE t1.b === 505 LIMIT 1;
--- Check with foreign modify
CREATE TABLE local_tbl (a int, b int, c text);
INSERT INTO local_tbl VALUES (1505, 505, 'foo');
+ANALYZE local_tbl;
+
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT * FROM local_tbl t1 LEFT JOIN (SELECT *, (SELECT count(*) FROM async_pt WHERE a < 3000) FROM async_pt WHERE a < 3000) t2 ON t1.a = t2.a;
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+SELECT * FROM local_tbl t1 LEFT JOIN (SELECT *, (SELECT count(*) FROM async_pt WHERE a < 3000) FROM async_pt WHERE a < 3000) t2 ON t1.a = t2.a;
+SELECT * FROM local_tbl t1 LEFT JOIN (SELECT *, (SELECT count(*) FROM async_pt WHERE a < 3000) FROM async_pt WHERE a < 3000) t2 ON t1.a = t2.a;
+-- Check with foreign modify
CREATE TABLE base_tbl3 (a int, b int, c text);
CREATE FOREIGN TABLE remote_tbl (a int, b int, c text)
SERVER loopback OPTIONS (table_name 'base_tbl3');
diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index 755c1392f0..a4eef19d7f 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -1043,6 +1043,17 @@ ExecAppendAsyncEventWait(AppendState *node)
ExecAsyncConfigureWait(areq);
}
+ /*
+ * No need for further processing if there are no configured events other
+ * than the postmaster death event.
+ */
+ if (GetNumRegisteredWaitEvents(node->as_eventset) == 1)
+ {
+ FreeWaitEventSet(node->as_eventset);
+ node->as_eventset = NULL;
+ return;
+ }
+
/* We wait on at most EVENT_BUFFER_SIZE events. */
if (nevents > EVENT_BUFFER_SIZE)
nevents = EVENT_BUFFER_SIZE;
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-22 11:14 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
@ 2021-07-23 11:26 ` Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2 siblings, 0 replies; 20+ messages in thread
From: Andrey V. Lepikhov @ 2021-07-23 11:26 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org
On 7/22/21 4:14 PM, Etsuro Fujita wrote:
> On Fri, Jul 2, 2021 at 10:24 PM Andrey Lepikhov
> @@ -7015,6 +7015,21 @@ process_pending_request(AsyncRequest *areq)
>
> fetch_more_data(node);
>
> + /*
> + * If the request are made by another append we will only prepare connection
> + * for the next query and don't take a tuple immediately. It is needed to
> + * prevent possible recursion into a qual subplan.
> + */
> + if (!fetch)
> + {
> + AppendState *node = (AppendState *) areq->requestor;
> +
> + ExecAsyncRequestDone(areq, NULL);
> + node->as_needrequest = bms_add_member(node->as_needrequest,
> + areq->request_index);
> + return;
> + }
>
> I don’t think this is a good idea, because it is pretty inconsistent,
> as doing ExecAsyncRequestDone(areq, NULL) means that there are no more
> tuples while changing as_needrequest like that means that there is at
> least one tuple to return. This would happen to work, but for
> example, if we add to the core more sanity checks on AsyncRequests,
> this would not work well anymore. I tried to devise a consistent
> solution for this issue, but I couldn’t. So I feel inclined to
> disable async execution in cases where async-capable nodes access to
> subplans (or initplans), for now.
That do you think, if (in addition to the patch) we will give
ExecAsyncAppendResponse a chance to fill the result slot? Code:
inline patch 1: src/backend/executor/nodeAppend.c (+2/-0 lines) download inline-1.patch
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -1108,6 +1108,9 @@ ExecAsyncAppendResponse(AsyncRequest *areq)
return;
}
+ if (areq->result == NULL)
+ areq->result =
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-22 11:14 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
@ 2021-07-27 06:22 ` Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-27 10:50 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2 siblings, 1 reply; 20+ messages in thread
From: Andrey V. Lepikhov @ 2021-07-27 06:22 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org
>> I think it can be done, but only as a temporary solution.
> My concern about that is that such an inconsistency would make the
> code complicated, and thus make the maintenance hard.
Agree, but your new patch is quite understandable.
>> Maybe we can split async logic into:
>> - receiving stage, when we only fetch and store tuples,
>> - evaluating stage, when we form resulting tuple and return by a
scan >> node.
>> I will think about such solution more.
> One simple solution along this line I came up with, which is not the
> rewrite, is to 1) split process_pending_request() into the two steps,
> and 2) postpone the second step until we are called from
> postgresForeignAsyncConfigureWait(), like the attached, which I think
> would be much consistent with the existing logic.
Good idea. Are you planning to commit this patch?
>> Also, may be you tell your opinion about an additional optimization
>> of Async Append [1]?
> Is the optimization related to this issue? (Sorry, I didn’t have time
> for reviewing the patch in [1] than expected. I plan to do so next
> month.)
This optimization tries to postpone choice of async subplans. It allows
us to make a decision on async capable subplans after all plan
flattening operations.
--
regards,
Andrey Lepikhov
Postgres Professional
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-22 11:14 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-27 06:22 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
@ 2021-07-27 10:50 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-27 11:00 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-28 06:39 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-30 08:11 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 3 replies; 20+ messages in thread
From: Etsuro Fujita @ 2021-07-27 10:50 UTC (permalink / raw)
To: CAPmGK15eY_P5AEC6-HjZusx2euftT0KD94ywaeJ=f3NGTLJEFw@mail.gmail.com; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Tue, Jul 27, 2021 at 3:22 PM Andrey V. Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
> >> Maybe we can split async logic into:
> >> - receiving stage, when we only fetch and store tuples,
> >> - evaluating stage, when we form resulting tuple and return by a
> >> scan node.
> >> I will think about such solution more.
> > One simple solution along this line I came up with, which is not the
> > rewrite, is to 1) split process_pending_request() into the two steps,
> > and 2) postpone the second step until we are called from
> > postgresForeignAsyncConfigureWait(), like the attached, which I think
> > would be much consistent with the existing logic.
> Good idea. Are you planning to commit this patch?
Cool!
Here is an updated version of the patch, in which I added/tweaked
comments a bit further. I'm planning to push this version if there
are no objections from others.
> >> Also, may be you tell your opinion about an additional optimization
> >> of Async Append [1]?
> > Is the optimization related to this issue?
> This optimization tries to postpone choice of async subplans. It allows
> us to make a decision on async capable subplans after all plan
> flattening operations.
Thanks for the explanation! My understanding is that the optimization
isn’t related to this issue. Right?
Best regards,
Etsuro Fujita
Attachments:
reconsider-process_pending_request-2.patch (application/octet-stream, 10.5K) download
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index ed25e7a743..78c05ee5ea 100644
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -10300,6 +10300,59 @@ SELECT * FROM async_pt t1, async_p2 t2 WHERE t1.a = t2.a AND t1.b === 505;
2505 | 505 | 0505 | 2505 | 505 | 0505
(1 row)
+CREATE TABLE local_tbl (a int, b int, c text);
+INSERT INTO local_tbl VALUES (1505, 505, 'foo');
+ANALYZE local_tbl;
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT * FROM local_tbl t1 LEFT JOIN (SELECT *, (SELECT count(*) FROM async_pt WHERE a < 3000) FROM async_pt WHERE a < 3000) t2 ON t1.a = t2.a;
+ QUERY PLAN
+----------------------------------------------------------------------------------------
+ Nested Loop Left Join
+ Output: t1.a, t1.b, t1.c, async_pt.a, async_pt.b, async_pt.c, ($0)
+ Join Filter: (t1.a = async_pt.a)
+ InitPlan 1 (returns $0)
+ -> Aggregate
+ Output: count(*)
+ -> Append
+ -> Async Foreign Scan on public.async_p1 async_pt_4
+ Remote SQL: SELECT NULL FROM public.base_tbl1 WHERE ((a < 3000))
+ -> Async Foreign Scan on public.async_p2 async_pt_5
+ Remote SQL: SELECT NULL FROM public.base_tbl2 WHERE ((a < 3000))
+ -> Seq Scan on public.local_tbl t1
+ Output: t1.a, t1.b, t1.c
+ -> Append
+ -> Async Foreign Scan on public.async_p1 async_pt_1
+ Output: async_pt_1.a, async_pt_1.b, async_pt_1.c, $0
+ Remote SQL: SELECT a, b, c FROM public.base_tbl1 WHERE ((a < 3000))
+ -> Async Foreign Scan on public.async_p2 async_pt_2
+ Output: async_pt_2.a, async_pt_2.b, async_pt_2.c, $0
+ Remote SQL: SELECT a, b, c FROM public.base_tbl2 WHERE ((a < 3000))
+(20 rows)
+
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+SELECT * FROM local_tbl t1 LEFT JOIN (SELECT *, (SELECT count(*) FROM async_pt WHERE a < 3000) FROM async_pt WHERE a < 3000) t2 ON t1.a = t2.a;
+ QUERY PLAN
+-----------------------------------------------------------------------------------------
+ Nested Loop Left Join (actual rows=1 loops=1)
+ Join Filter: (t1.a = async_pt.a)
+ Rows Removed by Join Filter: 399
+ InitPlan 1 (returns $0)
+ -> Aggregate (actual rows=1 loops=1)
+ -> Append (actual rows=400 loops=1)
+ -> Async Foreign Scan on async_p1 async_pt_4 (actual rows=200 loops=1)
+ -> Async Foreign Scan on async_p2 async_pt_5 (actual rows=200 loops=1)
+ -> Seq Scan on local_tbl t1 (actual rows=1 loops=1)
+ -> Append (actual rows=400 loops=1)
+ -> Async Foreign Scan on async_p1 async_pt_1 (actual rows=200 loops=1)
+ -> Async Foreign Scan on async_p2 async_pt_2 (actual rows=200 loops=1)
+(12 rows)
+
+SELECT * FROM local_tbl t1 LEFT JOIN (SELECT *, (SELECT count(*) FROM async_pt WHERE a < 3000) FROM async_pt WHERE a < 3000) t2 ON t1.a = t2.a;
+ a | b | c | a | b | c | count
+------+-----+-----+------+-----+------+-------
+ 1505 | 505 | foo | 1505 | 505 | 0505 | 400
+(1 row)
+
EXPLAIN (VERBOSE, COSTS OFF)
SELECT * FROM async_pt t1 WHERE t1.b === 505 LIMIT 1;
QUERY PLAN
@@ -10342,8 +10395,6 @@ SELECT * FROM async_pt t1 WHERE t1.b === 505 LIMIT 1;
(1 row)
-- Check with foreign modify
-CREATE TABLE local_tbl (a int, b int, c text);
-INSERT INTO local_tbl VALUES (1505, 505, 'foo');
CREATE TABLE base_tbl3 (a int, b int, c text);
CREATE FOREIGN TABLE remote_tbl (a int, b int, c text)
SERVER loopback OPTIONS (table_name 'base_tbl3');
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index f15c97ad7a..bd8fda9e91 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -503,6 +503,7 @@ static void analyze_row_processor(PGresult *res, int row,
PgFdwAnalyzeState *astate);
static void produce_tuple_asynchronously(AsyncRequest *areq, bool fetch);
static void fetch_more_data_begin(AsyncRequest *areq);
+static void complete_pending_request(AsyncRequest *areq);
static HeapTuple make_tuple_from_result_row(PGresult *res,
int row,
Relation rel,
@@ -6826,6 +6827,22 @@ postgresForeignAsyncConfigureWait(AsyncRequest *areq)
/* This should not be called unless callback_pending */
Assert(areq->callback_pending);
+ /*
+ * If process_pending_request() has been invoked on the given request
+ * before we get here, we might have some tuples already; in which case
+ * complete the request
+ */
+ if (fsstate->next_tuple < fsstate->num_tuples)
+ {
+ complete_pending_request(areq);
+ if (areq->request_complete)
+ return;
+ Assert(areq->callback_pending);
+ }
+
+ /* We must have run out of tuples */
+ Assert(fsstate->next_tuple >= fsstate->num_tuples);
+
/* The core code would have registered postmaster death event */
Assert(GetNumRegisteredWaitEvents(set) >= 1);
@@ -6838,12 +6855,15 @@ postgresForeignAsyncConfigureWait(AsyncRequest *areq)
* This is the case when the in-process request was made by another
* Append. Note that it might be useless to process the request,
* because the query might not need tuples from that Append anymore.
- * Skip the given request if there are any configured events other
- * than the postmaster death event; otherwise process the request,
- * then begin a fetch to configure the event below, because otherwise
- * we might end up with no configured events other than the postmaster
- * death event.
+ * If there are any child subplans of the same parent that are ready
+ * for new requests, skip the given request. Likewise, if there are
+ * any configured events other than the postmaster death event, skip
+ * it. Otherwise, process the in-process request, then begin a fetch
+ * to configure the event below, because we might otherwise end up
+ * with no configured events other than the postmaster death event.
*/
+ if (!bms_is_empty(requestor->as_needrequest))
+ return;
if (GetNumRegisteredWaitEvents(set) > 1)
return;
process_pending_request(pendingAreq);
@@ -6995,23 +7015,44 @@ process_pending_request(AsyncRequest *areq)
{
ForeignScanState *node = (ForeignScanState *) areq->requestee;
PgFdwScanState *fsstate PG_USED_FOR_ASSERTS_ONLY = (PgFdwScanState *) node->fdw_state;
- EState *estate = node->ss.ps.state;
- MemoryContext oldcontext;
+
+ /* The request would have been pending for a callback */
+ Assert(areq->callback_pending);
/* The request should be currently in-process */
Assert(fsstate->conn_state->pendingAreq == areq);
- oldcontext = MemoryContextSwitchTo(estate->es_query_cxt);
+ fetch_more_data(node);
+ /*
+ * If we didn't get any tuples, must be end of data; complete the request
+ * now. Otherwise, we postpone completing the request until we are called
+ * from postgresForeignAsyncConfigureWait().
+ */
+ if (fsstate->next_tuple >= fsstate->num_tuples)
+ {
+ /* Unlike AsyncNotify, we unset callback_pending ourselves */
+ areq->callback_pending = false;
+ /* Mark the request as complete */
+ ExecAsyncRequestDone(areq, NULL);
+ /* Unlike AsyncNotify, we call ExecAsyncResponse ourselves */
+ ExecAsyncResponse(areq);
+ }
+}
+
+/*
+ * Complete a pending asynchronous request.
+ */
+static void
+complete_pending_request(AsyncRequest *areq)
+{
/* The request would have been pending for a callback */
Assert(areq->callback_pending);
/* Unlike AsyncNotify, we unset callback_pending ourselves */
areq->callback_pending = false;
- fetch_more_data(node);
-
- /* We need to send a new query afterwards; don't fetch */
+ /* We begin a fetch afterwards if necessary; don't fetch */
produce_tuple_asynchronously(areq, false);
/* Unlike AsyncNotify, we call ExecAsyncResponse ourselves */
@@ -7021,8 +7062,6 @@ process_pending_request(AsyncRequest *areq)
if (areq->requestee->instrument)
InstrUpdateTupleCount(areq->requestee->instrument,
TupIsNull(areq->result) ? 0.0 : 1.0);
-
- MemoryContextSwitchTo(oldcontext);
}
/*
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 02a6b15a13..75fff9bad0 100644
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3274,6 +3274,16 @@ EXPLAIN (VERBOSE, COSTS OFF)
SELECT * FROM async_pt t1, async_p2 t2 WHERE t1.a = t2.a AND t1.b === 505;
SELECT * FROM async_pt t1, async_p2 t2 WHERE t1.a = t2.a AND t1.b === 505;
+CREATE TABLE local_tbl (a int, b int, c text);
+INSERT INTO local_tbl VALUES (1505, 505, 'foo');
+ANALYZE local_tbl;
+
+EXPLAIN (VERBOSE, COSTS OFF)
+SELECT * FROM local_tbl t1 LEFT JOIN (SELECT *, (SELECT count(*) FROM async_pt WHERE a < 3000) FROM async_pt WHERE a < 3000) t2 ON t1.a = t2.a;
+EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
+SELECT * FROM local_tbl t1 LEFT JOIN (SELECT *, (SELECT count(*) FROM async_pt WHERE a < 3000) FROM async_pt WHERE a < 3000) t2 ON t1.a = t2.a;
+SELECT * FROM local_tbl t1 LEFT JOIN (SELECT *, (SELECT count(*) FROM async_pt WHERE a < 3000) FROM async_pt WHERE a < 3000) t2 ON t1.a = t2.a;
+
EXPLAIN (VERBOSE, COSTS OFF)
SELECT * FROM async_pt t1 WHERE t1.b === 505 LIMIT 1;
EXPLAIN (ANALYZE, COSTS OFF, SUMMARY OFF, TIMING OFF)
@@ -3281,9 +3291,6 @@ SELECT * FROM async_pt t1 WHERE t1.b === 505 LIMIT 1;
SELECT * FROM async_pt t1 WHERE t1.b === 505 LIMIT 1;
-- Check with foreign modify
-CREATE TABLE local_tbl (a int, b int, c text);
-INSERT INTO local_tbl VALUES (1505, 505, 'foo');
-
CREATE TABLE base_tbl3 (a int, b int, c text);
CREATE FOREIGN TABLE remote_tbl (a int, b int, c text)
SERVER loopback OPTIONS (table_name 'base_tbl3');
diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c
index 755c1392f0..a4eef19d7f 100644
--- a/src/backend/executor/nodeAppend.c
+++ b/src/backend/executor/nodeAppend.c
@@ -1043,6 +1043,17 @@ ExecAppendAsyncEventWait(AppendState *node)
ExecAsyncConfigureWait(areq);
}
+ /*
+ * No need for further processing if there are no configured events other
+ * than the postmaster death event.
+ */
+ if (GetNumRegisteredWaitEvents(node->as_eventset) == 1)
+ {
+ FreeWaitEventSet(node->as_eventset);
+ node->as_eventset = NULL;
+ return;
+ }
+
/* We wait on at most EVENT_BUFFER_SIZE events. */
if (nevents > EVENT_BUFFER_SIZE)
nevents = EVENT_BUFFER_SIZE;
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-22 11:14 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-27 06:22 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-27 10:50 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
@ 2021-07-27 11:00 ` Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-28 03:35 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2 siblings, 1 reply; 20+ messages in thread
From: Andrey V. Lepikhov @ 2021-07-27 11:00 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org
On 7/27/21 3:50 PM, Etsuro Fujita wrote:
> On Tue, Jul 27, 2021 at 3:22 PM Andrey V. Lepikhov
> <a.lepikhov@postgrespro.ru> wrote:
>> >> Maybe we can split async logic into:
>> >> - receiving stage, when we only fetch and store tuples,
>> >> - evaluating stage, when we form resulting tuple and return by a
>> >> scan node.
>> >> I will think about such solution more.
>
>> > One simple solution along this line I came up with, which is not the
>> > rewrite, is to 1) split process_pending_request() into the two steps,
>> > and 2) postpone the second step until we are called from
>> > postgresForeignAsyncConfigureWait(), like the attached, which I think
>> > would be much consistent with the existing logic.
>
>> Good idea. Are you planning to commit this patch?
>
> Cool!
>
> Here is an updated version of the patch, in which I added/tweaked
> comments a bit further. I'm planning to push this version if there
> are no objections from others.
Thanks. In addition, I will use this patch in TPC-H benchmark tomorrow.
>
>> >> Also, may be you tell your opinion about an additional optimization
>> >> of Async Append [1]?
>
>> > Is the optimization related to this issue?
>
>> This optimization tries to postpone choice of async subplans. It allows
>> us to make a decision on async capable subplans after all plan
>> flattening operations.
>
> Thanks for the explanation! My understanding is that the optimization
> isn’t related to this issue. Right?
Right
--
regards,
Andrey Lepikhov
Postgres Professional
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-22 11:14 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-27 06:22 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-27 10:50 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-27 11:00 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
@ 2021-07-28 03:35 ` Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 0 replies; 20+ messages in thread
From: Etsuro Fujita @ 2021-07-28 03:35 UTC (permalink / raw)
To: Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Tue, Jul 27, 2021 at 8:01 PM Andrey V. Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
> On 7/27/21 3:50 PM, Etsuro Fujita wrote:
> > On Tue, Jul 27, 2021 at 3:22 PM Andrey V. Lepikhov
> > <a.lepikhov@postgrespro.ru> wrote:
> >> >> Also, may be you tell your opinion about an additional optimization
> >> >> of Async Append [1]?
> >
> >> > Is the optimization related to this issue?
> >
> >> This optimization tries to postpone choice of async subplans. It allows
> >> us to make a decision on async capable subplans after all plan
> >> flattening operations.
> >
> > Thanks for the explanation! My understanding is that the optimization
> > isn’t related to this issue. Right?
> Right
OK
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-22 11:14 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-27 06:22 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-27 10:50 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
@ 2021-07-28 06:39 ` Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-28 08:28 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2 siblings, 1 reply; 20+ messages in thread
From: Andrey V. Lepikhov @ 2021-07-28 06:39 UTC (permalink / raw)
To: Etsuro Fujita <etsuro.fujita@gmail.com>; CAPmGK15eY_P5AEC6-HjZusx2euftT0KD94ywaeJ=f3NGTLJEFw@mail.gmail.com; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On 7/27/21 3:50 PM, Etsuro Fujita wrote:
> On Tue, Jul 27, 2021 at 3:22 PM Andrey V. Lepikhov
> <a.lepikhov@postgrespro.ru> wrote:
> Here is an updated version of the patch, in which I added/tweaked
> comments a bit further. I'm planning to push this version if there
> are no objections from others.
I used this patch in TPC-H benchmark (that caused initial error) and it
finished without problems.
Queries and their explains are attached. You could look for situations
when async append was not used.
--
regards,
Andrey Lepikhov
Postgres Professional
Attachments:
res.txt (text/plain, 91.6K) download
tpchq_rewritten.sql (application/sql, 14.5K) download
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-22 11:14 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-27 06:22 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-27 10:50 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-28 06:39 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
@ 2021-07-28 08:28 ` Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 0 replies; 20+ messages in thread
From: Etsuro Fujita @ 2021-07-28 08:28 UTC (permalink / raw)
To: Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>; +Cc: CAPmGK15eY_P5AEC6-HjZusx2euftT0KD94ywaeJ=f3NGTLJEFw@mail.gmail.com, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Wed, Jul 28, 2021 at 3:39 PM Andrey V. Lepikhov
<a.lepikhov@postgrespro.ru> wrote:
> On 7/27/21 3:50 PM, Etsuro Fujita wrote:
> > On Tue, Jul 27, 2021 at 3:22 PM Andrey V. Lepikhov
> > <a.lepikhov@postgrespro.ru> wrote:
> > Here is an updated version of the patch, in which I added/tweaked
> > comments a bit further. I'm planning to push this version if there
> > are no objections from others.
> I used this patch in TPC-H benchmark (that caused initial error) and it
> finished without problems.
Good to know!
> Queries and their explains are attached. You could look for situations
> when async append was not used.
Interesting! Yeah, async exection doesn't apply to Merge Append, so
let's improve to support async-aware Merge Append.
Thanks for the testing!
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-22 11:14 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-27 06:22 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-27 10:50 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
@ 2021-07-30 08:11 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-30 12:25 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2 siblings, 1 reply; 20+ messages in thread
From: Etsuro Fujita @ 2021-07-30 08:11 UTC (permalink / raw)
To: CAPmGK15eY_P5AEC6-HjZusx2euftT0KD94ywaeJ=f3NGTLJEFw@mail.gmail.com; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Tue, Jul 27, 2021 at 7:50 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> Here is an updated version of the patch, in which I added/tweaked
> comments a bit further. I'm planning to push this version if there
> are no objections from others.
Done.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 20+ messages in thread
* Re: The case when AsyncAppend exists also in the qual of Async ForeignScan
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-22 11:14 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-27 06:22 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-27 10:50 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-30 08:11 ` Re: The case when AsyncAppend exists also in the qual of Async ForeignScan Etsuro Fujita <etsuro.fujita@gmail.com>
@ 2021-07-30 12:25 ` Etsuro Fujita <etsuro.fujita@gmail.com>
0 siblings, 0 replies; 20+ messages in thread
From: Etsuro Fujita @ 2021-07-30 12:25 UTC (permalink / raw)
To: CAPmGK15eY_P5AEC6-HjZusx2euftT0KD94ywaeJ=f3NGTLJEFw@mail.gmail.com; +Cc: PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
On Fri, Jul 30, 2021 at 5:11 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> On Tue, Jul 27, 2021 at 7:50 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> > Here is an updated version of the patch, in which I added/tweaked
> > comments a bit further. I'm planning to push this version if there
> > are no objections from others.
>
> Done.
I noticed that some buildfarm members cause this assertion failure:
TRAP: FailedAssertion("fsstate->conn_state->pendingAreq == areq",
File: "postgres_fdw.c", Line: 6900, PID: 25840)
I will look into this.
Best regards,
Etsuro Fujita
^ permalink raw reply [nested|flat] 20+ messages in thread
end of thread, other threads:[~2021-07-30 12:25 UTC | newest]
Thread overview: 20+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-07-02 13:24 The case when AsyncAppend exists also in the qual of Async ForeignScan Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-02 23:35 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-08 07:49 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-08 08:06 ` Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-08 08:32 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-08 10:18 ` Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-08 17:10 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-08 10:32 ` Andrey Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-22 11:14 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-23 06:09 ` Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-25 08:55 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-23 11:26 ` Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-27 06:22 ` Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-27 10:50 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-27 11:00 ` Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-28 03:35 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-28 06:39 ` Andrey V. Lepikhov <a.lepikhov@postgrespro.ru>
2021-07-28 08:28 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-30 08:11 ` Etsuro Fujita <etsuro.fujita@gmail.com>
2021-07-30 12:25 ` Etsuro Fujita <etsuro.fujita@gmail.com>
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