Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lbHf6-0008WT-0y for pgsql-hackers@arkaria.postgresql.org; Tue, 27 Apr 2021 06:57:36 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1lbHf4-0000bi-UH for pgsql-hackers@arkaria.postgresql.org; Tue, 27 Apr 2021 06:57:34 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1lbHf4-0000bb-JT for pgsql-hackers@lists.postgresql.org; Tue, 27 Apr 2021 06:57:34 +0000 Received: from mail.postgrespro.ru ([93.174.131.139]) by magus.postgresql.org with esmtp (Exim 4.92) (envelope-from ) id 1lbHf2-0006Wb-Ew for pgsql-hackers@lists.postgresql.org; Tue, 27 Apr 2021 06:57:34 +0000 Received: from [192.168.129.197] (unknown [94.25.177.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) by mail.postgrespro.ru (Postfix) with ESMTPSA id DD67A21C802F; Tue, 27 Apr 2021 09:57:30 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=postgrespro.ru; s=mail; t=1619506651; bh=RkO46s46aDExZZr3i4ApxvPxEvI3ccqYkRqg2nB8B4U=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=a26EPJVS6VsEISyHeZ+kN1YA9kbU4QXMc4TBqjG1ftlefd24wGvXbzkP0x4d5jdxp w8Vn9SSNCRzMIEfCzotvCkkhxIes9nCXKhGw2dd0TNJbXAFqg+0ZL+FMd+jw7JZw/l X8KjL7yC5UV88LLc+wTSzkVGJkyeaywGaFqoQsgM= Subject: Re: Asynchronous Append on postgres_fdw nodes. To: Etsuro Fujita , Kyotaro Horiguchi Cc: Justin Pryzby , PostgreSQL Hackers References: <20210331.101137.148208859959175708.horikyota.ntt@gmail.com> From: "Andrey V. Lepikhov" Organization: Postgres Professional Message-ID: Date: Tue, 27 Apr 2021 11:57:30 +0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: multipart/mixed; boundary="------------4962A865F3ACEE2B45701A77" Content-Language: en-US List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Archived-At: Precedence: bulk This is a multi-part message in MIME format. --------------4962A865F3ACEE2B45701A77 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 4/23/21 8:12 AM, Etsuro Fujita wrote: > On Thu, Apr 22, 2021 at 12:30 PM Etsuro Fujita wrote: > I have committed the patch. One more question. Append choose async plans at the stage of the Append plan creation. Later, the planner performs some optimizations, such as eliminating trivial Subquery nodes. So, AsyncAppend is impossible in some situations, for example: (SELECT * FROM f1 WHERE a < 10) UNION ALL (SELECT * FROM f2 WHERE a < 10); But works for the query: SELECT * FROM (SELECT * FROM f1 UNION ALL SELECT * FROM f2) AS q1 WHERE a < 10; As far as I understand, this is not a hard limit. We can choose async subplans at the beginning of the execution stage. For a demo, I prepared the patch (see in attachment). It solves the problem and passes the regression tests. -- regards, Andrey Lepikhov Postgres Professional --------------4962A865F3ACEE2B45701A77 Content-Type: text/x-patch; charset=UTF-8; name="asyncappend_executor_fix.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="asyncappend_executor_fix.patch" diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index a960ada441..655e743c6e 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -1246,6 +1246,7 @@ postgresGetForeignPlan(PlannerInfo *root, bool has_final_sort = false; bool has_limit = false; ListCell *lc; + ForeignScan *fsplan; /* * Get FDW private data created by postgresGetForeignUpperPaths(), if any. @@ -1430,7 +1431,7 @@ postgresGetForeignPlan(PlannerInfo *root, * field of the finished plan node; we can't keep them in private state * because then they wouldn't be subject to later planner processing. */ - return make_foreignscan(tlist, + fsplan = make_foreignscan(tlist, local_exprs, scan_relid, params_list, @@ -1438,6 +1439,13 @@ postgresGetForeignPlan(PlannerInfo *root, fdw_scan_tlist, fdw_recheck_quals, outer_plan); + + /* If appropriate, consider participation in async operations */ + fsplan->scan.plan.async_capable = (enable_async_append && + best_path->path.pathkeys == NIL && + !fsplan->scan.plan.parallel_safe && + is_async_capable_path((Path *)best_path)); + return fsplan; } /* diff --git a/src/backend/executor/execAmi.c b/src/backend/executor/execAmi.c index b3726a54f3..4e70f4eb54 100644 --- a/src/backend/executor/execAmi.c +++ b/src/backend/executor/execAmi.c @@ -524,6 +524,9 @@ ExecSupportsBackwardScan(Plan *node) if (node->parallel_aware) return false; + if (node->async_capable) + return false; + switch (nodeTag(node)) { case T_Result: @@ -536,10 +539,6 @@ ExecSupportsBackwardScan(Plan *node) { ListCell *l; - /* With async, tuples may be interleaved, so can't back up. */ - if (((Append *) node)->nasyncplans > 0) - return false; - foreach(l, ((Append *) node)->appendplans) { if (!ExecSupportsBackwardScan((Plan *) lfirst(l))) diff --git a/src/backend/executor/nodeAppend.c b/src/backend/executor/nodeAppend.c index 3c1f12adaf..363cf9f4a5 100644 --- a/src/backend/executor/nodeAppend.c +++ b/src/backend/executor/nodeAppend.c @@ -117,6 +117,8 @@ ExecInitAppend(Append *node, EState *estate, int eflags) int firstvalid; int i, j; + ListCell *l; + bool consider_async = false; /* check for unsupported flags */ Assert(!(eflags & EXEC_FLAG_MARK)); @@ -197,6 +199,23 @@ ExecInitAppend(Append *node, EState *estate, int eflags) appendplanstates = (PlanState **) palloc(nplans * sizeof(PlanState *)); + /* If appropriate, consider async append */ + consider_async = (list_length(node->appendplans) > 1); + + if (!consider_async) + { + foreach(l, node->appendplans) + { + Plan *subplan = (Plan *) lfirst(l); + + /* Check to see if subplan can be executed asynchronously */ + if (subplan->async_capable) + { + subplan->async_capable = false; + } + } + } + /* * call ExecInitNode on each of the valid plans to be executed and save * the results into the appendplanstates array. diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 632cc31a04..f7302ccf28 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -242,7 +242,6 @@ _copyAppend(const Append *from) */ COPY_BITMAPSET_FIELD(apprelids); COPY_NODE_FIELD(appendplans); - COPY_SCALAR_FIELD(nasyncplans); COPY_SCALAR_FIELD(first_partial_plan); COPY_NODE_FIELD(part_prune_info); diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index c723f6d635..665cdf3add 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -432,7 +432,6 @@ _outAppend(StringInfo str, const Append *node) WRITE_BITMAPSET_FIELD(apprelids); WRITE_NODE_FIELD(appendplans); - WRITE_INT_FIELD(nasyncplans); WRITE_INT_FIELD(first_partial_plan); WRITE_NODE_FIELD(part_prune_info); } diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index 3746668f52..9e3822f7db 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -1716,7 +1716,6 @@ _readAppend(void) READ_BITMAPSET_FIELD(apprelids); READ_NODE_FIELD(appendplans); - READ_INT_FIELD(nasyncplans); READ_INT_FIELD(first_partial_plan); READ_NODE_FIELD(part_prune_info); diff --git a/src/backend/optimizer/plan/createplan.c b/src/backend/optimizer/plan/createplan.c index a9aff24831..8792eef451 100644 --- a/src/backend/optimizer/plan/createplan.c +++ b/src/backend/optimizer/plan/createplan.c @@ -81,7 +81,6 @@ static List *get_gating_quals(PlannerInfo *root, List *quals); static Plan *create_gating_plan(PlannerInfo *root, Path *path, Plan *plan, List *gating_quals); static Plan *create_join_plan(PlannerInfo *root, JoinPath *best_path); -static bool is_async_capable_path(Path *path); static Plan *create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags); static Plan *create_merge_append_plan(PlannerInfo *root, MergeAppendPath *best_path, @@ -1093,10 +1092,10 @@ create_join_plan(PlannerInfo *root, JoinPath *best_path) } /* - * is_async_capable_path - * Check whether a given Path node is async-capable. + * is_async_capable_plan + * Check whether a given Plan node is async-capable. */ -static bool +bool is_async_capable_path(Path *path) { switch (nodeTag(path)) @@ -1134,7 +1133,6 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags) List *pathkeys = best_path->path.pathkeys; List *subplans = NIL; ListCell *subpaths; - int nasyncplans = 0; RelOptInfo *rel = best_path->path.parent; PartitionPruneInfo *partpruneinfo = NULL; int nodenumsortkeys = 0; @@ -1142,7 +1140,6 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags) Oid *nodeSortOperators = NULL; Oid *nodeCollations = NULL; bool *nodeNullsFirst = NULL; - bool consider_async = false; /* * The subpaths list could be empty, if every child was proven empty by @@ -1206,11 +1203,6 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags) tlist_was_changed = (orig_tlist_length != list_length(plan->plan.targetlist)); } - /* If appropriate, consider async append */ - consider_async = (enable_async_append && pathkeys == NIL && - !best_path->path.parallel_safe && - list_length(best_path->subpaths) > 1); - /* Build the plan for each child */ foreach(subpaths, best_path->subpaths) { @@ -1279,12 +1271,6 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags) subplans = lappend(subplans, subplan); - /* Check to see if subplan can be executed asynchronously */ - if (consider_async && is_async_capable_path(subpath)) - { - subplan->async_capable = true; - ++nasyncplans; - } } /* @@ -1317,7 +1303,6 @@ create_append_plan(PlannerInfo *root, AppendPath *best_path, int flags) } plan->appendplans = subplans; - plan->nasyncplans = nasyncplans; plan->first_partial_plan = best_path->first_partial_path; plan->part_prune_info = partpruneinfo; diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h index d671328dfd..b5ac0a1da2 100644 --- a/src/include/nodes/plannodes.h +++ b/src/include/nodes/plannodes.h @@ -250,7 +250,6 @@ typedef struct Append Plan plan; Bitmapset *apprelids; /* RTIs of appendrel(s) formed by this node */ List *appendplans; - int nasyncplans; /* # of asynchronous plans */ /* * All 'appendplans' preceding this index are non-partial plans. All diff --git a/src/include/optimizer/planmain.h b/src/include/optimizer/planmain.h index bf1adfc52a..8a96a19e5f 100644 --- a/src/include/optimizer/planmain.h +++ b/src/include/optimizer/planmain.h @@ -115,5 +115,6 @@ extern Plan *set_plan_references(PlannerInfo *root, Plan *plan); extern void record_plan_function_dependency(PlannerInfo *root, Oid funcid); extern void record_plan_type_dependency(PlannerInfo *root, Oid typid); extern bool extract_query_dependencies_walker(Node *node, PlannerInfo *root); +extern bool is_async_capable_path(Path *path); #endif /* PLANMAIN_H */ --------------4962A865F3ACEE2B45701A77--