Re: [HACKERS] Parallel execution and prepared statements
On Wed, Dec 7, 2016 at 9:23 PM, Amit Kapila wrote: > Attached patch changes the comment based on above suggestions. Thanks. Committed and back-patched to 9.6. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Wed, Dec 7, 2016 at 12:30 AM, Robert Haas wrote: > On Tue, Dec 6, 2016 at 1:07 PM, Tom Lane wrote: >> Robert Haas writes: >>> Done. >> >> The comment seems quite confused now: >> >> * If a tuple count was supplied or data is being written to relation, we >> * must force the plan to run without parallelism, because we might exit >> * early. >> >> Exit early is exactly what we *won't* do if writing to an INTO rel, so >> I think this will confuse future readers. I think it should be more like >> >> * If a tuple count was supplied, we must force the plan to run without >> * parallelism, because we might exit early. Also disable parallelism >> * when writing into a relation, because [ uh, why exactly? ] >> >> Considering that the writing would happen at top level of the executor, >> and hence in the parent process, it's not actually clear to me why the >> second restriction is there at all: can't we write tuples to a rel even >> though they came from a parallel worker? In any case, the current wording >> of the comment is a complete fail at explaining this. > > Oops. You're right. [ uh, why exactly? ] -> no database changes > whatsoever are allowed while in parallel mode. (This restriction > might be lifted someday, but right now we're stuck with it.) > Attached patch changes the comment based on above suggestions. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com fix_comment_parallel_mode_v1.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Tue, Dec 6, 2016 at 1:07 PM, Tom Lane wrote: > Robert Haas writes: >> Done. > > The comment seems quite confused now: > > * If a tuple count was supplied or data is being written to relation, we > * must force the plan to run without parallelism, because we might exit > * early. > > Exit early is exactly what we *won't* do if writing to an INTO rel, so > I think this will confuse future readers. I think it should be more like > > * If a tuple count was supplied, we must force the plan to run without > * parallelism, because we might exit early. Also disable parallelism > * when writing into a relation, because [ uh, why exactly? ] > > Considering that the writing would happen at top level of the executor, > and hence in the parent process, it's not actually clear to me why the > second restriction is there at all: can't we write tuples to a rel even > though they came from a parallel worker? In any case, the current wording > of the comment is a complete fail at explaining this. Oops. You're right. [ uh, why exactly? ] -> no database changes whatsoever are allowed while in parallel mode. (This restriction might be lifted someday, but right now we're stuck with it.) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
Robert Haas writes: > Done. The comment seems quite confused now: * If a tuple count was supplied or data is being written to relation, we * must force the plan to run without parallelism, because we might exit * early. Exit early is exactly what we *won't* do if writing to an INTO rel, so I think this will confuse future readers. I think it should be more like * If a tuple count was supplied, we must force the plan to run without * parallelism, because we might exit early. Also disable parallelism * when writing into a relation, because [ uh, why exactly? ] Considering that the writing would happen at top level of the executor, and hence in the parent process, it's not actually clear to me why the second restriction is there at all: can't we write tuples to a rel even though they came from a parallel worker? In any case, the current wording of the comment is a complete fail at explaining this. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Mon, Dec 5, 2016 at 5:18 AM, Albe Laurenz wrote: > Tobias Bussmann wrote: >>> I think if we don't see any impact then we should backpatch this >>> patch. However, if we decide to go some other way, then I can provide >>> a separate patch for back branches. BTW, what is your opinion? >> >> I could not find anything on backporting guidelines in the wiki but my >> opinion would be to backpatch >> the patch in total. With a different behaviour between the simple and >> extended query protocol it would >> be hard to debug query performance issue in user applications that uses >> PQprepare. If the user tries >> to replicate a query with a PREPARE in psql and tries to EXPLAIN EXECUTE it, >> the results would be >> different then what happens within the application. That behaviour could be >> confusing, like >> differences between EXPLAIN SELECT and EXPLAIN EXECUTE can be to less >> experienced users. > > +1 Done. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
Tobias Bussmann wrote: >> I think if we don't see any impact then we should backpatch this >> patch. However, if we decide to go some other way, then I can provide >> a separate patch for back branches. BTW, what is your opinion? > > I could not find anything on backporting guidelines in the wiki but my > opinion would be to backpatch > the patch in total. With a different behaviour between the simple and > extended query protocol it would > be hard to debug query performance issue in user applications that uses > PQprepare. If the user tries > to replicate a query with a PREPARE in psql and tries to EXPLAIN EXECUTE it, > the results would be > different then what happens within the application. That behaviour could be > confusing, like > differences between EXPLAIN SELECT and EXPLAIN EXECUTE can be to less > experienced users. +1 Yours, Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
> I think if we don't see any impact then we should backpatch this > patch. However, if we decide to go some other way, then I can provide > a separate patch for back branches. BTW, what is your opinion? I could not find anything on backporting guidelines in the wiki but my opinion would be to backpatch the patch in total. With a different behaviour between the simple and extended query protocol it would be hard to debug query performance issue in user applications that uses PQprepare. If the user tries to replicate a query with a PREPARE in psql and tries to EXPLAIN EXECUTE it, the results would be different then what happens within the application. That behaviour could be confusing, like differences between EXPLAIN SELECT and EXPLAIN EXECUTE can be to less experienced users. Best regards Tobias -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Fri, Dec 2, 2016 at 3:31 PM, Tobias Bussmann wrote: > >> On Thu, Dec 1, 2016 at 9:40 PM, Robert Haas wrote: >>> >>> OK, then my vote is to do it that way for now. > > Thanks for your opinion. That's fine with me. > >> Am 02.12.2016 um 07:22 schrieb Amit Kapila : >> Done that way in attached patch. > > Did a quick review: > Thanks for the review. > You should however include a sentence in the documentation on that parallel > plan w/o workers corner-case behaviour. Feel free to take that from my patch > or phase a better wording. > I have taken it from your patch. > And again my question regarding back patching to 9.6: > - 9.6 is currently broken as Laurenz showed in [1] > - 9.6 does not have documented that SQL PREPARE prepared statements cannot > not use parallel query > > The former could be fixed by back patching the full patch which would void > the latter. > I think if we don't see any impact then we should backpatch this patch. However, if we decide to go some other way, then I can provide a separate patch for back branches. BTW, what is your opinion? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com prepared_stmt_parallel_query_v4.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
> On Thu, Dec 1, 2016 at 9:40 PM, Robert Haas wrote: >> >> OK, then my vote is to do it that way for now. Thanks for your opinion. That's fine with me. > Am 02.12.2016 um 07:22 schrieb Amit Kapila : > Done that way in attached patch. Did a quick review: The patch applies cleanly against current head. make installcheck with force_parallel_mode = regress passes all tests. My manual tests show that parallel query is working for prepared statements in SQL with PREPARE and EXECUTE. CREATE TABLE AS EXECUTE is working, EXPLAIN on that shows a parallel plan, EXPLAIN ANALZE indicates 0 launched workers for that. Looks fine so far! You should however include a sentence in the documentation on that parallel plan w/o workers corner-case behaviour. Feel free to take that from my patch or phase a better wording. And again my question regarding back patching to 9.6: - 9.6 is currently broken as Laurenz showed in [1] - 9.6 does not have documented that SQL PREPARE prepared statements cannot not use parallel query The former could be fixed by back patching the full patch which would void the latter. Or it could be fixed by disabling generation of parallel plans in extended query protocol prepare. Alternatively only the change in execMain.c could be back patched. In these cases we would need to have the a separate wording for the 9.6 docs. Best regards, Tobias [1] a737b7a37273e048b164557adef4a58b53999...@ntex2010i.host.magwien.gv.at -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Thu, Dec 1, 2016 at 9:40 PM, Robert Haas wrote: > On Thu, Dec 1, 2016 at 7:57 AM, Amit Kapila wrote: >> >> I have tried to restrict all the non-readonly operation modes or modes >> where parallelism might not make sense like DestTupleStore. If we >> want to just prohibit the cases where it can fail now, then I think >> prohibiting only DestIntoRel should be sufficient because that is a >> case where the user is allowed to do DDL for an already prepared read >> only statement like Create Table AS .. EXECUTE. > > OK, then my vote is to do it that way for now. > Done that way in attached patch. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com prepared_stmt_parallel_query_v3.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Thu, Dec 1, 2016 at 7:57 AM, Amit Kapila wrote: > On Wed, Nov 30, 2016 at 11:57 PM, Robert Haas wrote: >> On Tue, Nov 29, 2016 at 6:24 AM, Amit Kapila wrote: >>> >>> Robert, do you have any better ideas for this problem? >> >> Not really. I think your prepared_stmt_parallel_query_v2.patch is >> probably the best approach proposed so far, but I wonder why we need >> to include DestCopyOut and DestTupleStore. DestIntoRel and >> DestTransientRel both write to an actual relation, which is a problem >> for parallel mode, but I think the others don't. >> > > I have tried to restrict all the non-readonly operation modes or modes > where parallelism might not make sense like DestTupleStore. If we > want to just prohibit the cases where it can fail now, then I think > prohibiting only DestIntoRel should be sufficient because that is a > case where the user is allowed to do DDL for an already prepared read > only statement like Create Table AS .. EXECUTE. OK, then my vote is to do it that way for now. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Wed, Nov 30, 2016 at 11:57 PM, Robert Haas wrote: > On Tue, Nov 29, 2016 at 6:24 AM, Amit Kapila wrote: >> >> Robert, do you have any better ideas for this problem? > > Not really. I think your prepared_stmt_parallel_query_v2.patch is > probably the best approach proposed so far, but I wonder why we need > to include DestCopyOut and DestTupleStore. DestIntoRel and > DestTransientRel both write to an actual relation, which is a problem > for parallel mode, but I think the others don't. > I have tried to restrict all the non-readonly operation modes or modes where parallelism might not make sense like DestTupleStore. If we want to just prohibit the cases where it can fail now, then I think prohibiting only DestIntoRel should be sufficient because that is a case where the user is allowed to do DDL for an already prepared read only statement like Create Table AS .. EXECUTE. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Tue, Nov 29, 2016 at 6:24 AM, Amit Kapila wrote: > On Tue, Nov 29, 2016 at 4:40 PM, Albe Laurenz wrote: >> Amit Kapila wrote: But with Tobias' complete patch "make installcheck-world" succeeds. >>> >>> Okay. I have looked into patch proposed and found that it will >>> resolve the regression test problem (Create Table .. AS Execute). I >>> think it is slightly prone to breakage if tomorrow we implement usage >>> of EXECUTE with statements other Create Table (like Copy). Have you >>> registered this patch in CF to avoid loosing track? >>> >>> We have two patches (one proposed in this thread and one proposed by >>> me earlier [1]) and both solves the regression test failure. Unless >>> there is some better approach, I think we can go with one of these. >> >> Tobias did that here: https://commitfest.postgresql.org/12/890/ >> >> He added your patch as well. >> > > Okay, Thanks. > > Robert, do you have any better ideas for this problem? Not really. I think your prepared_stmt_parallel_query_v2.patch is probably the best approach proposed so far, but I wonder why we need to include DestCopyOut and DestTupleStore. DestIntoRel and DestTransientRel both write to an actual relation, which is a problem for parallel mode, but I think the others don't. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Tue, Nov 29, 2016 at 4:40 PM, Albe Laurenz wrote: > Amit Kapila wrote: >>> But with Tobias' complete patch "make installcheck-world" succeeds. >> >> Okay. I have looked into patch proposed and found that it will >> resolve the regression test problem (Create Table .. AS Execute). I >> think it is slightly prone to breakage if tomorrow we implement usage >> of EXECUTE with statements other Create Table (like Copy). Have you >> registered this patch in CF to avoid loosing track? >> >> We have two patches (one proposed in this thread and one proposed by >> me earlier [1]) and both solves the regression test failure. Unless >> there is some better approach, I think we can go with one of these. > > Tobias did that here: https://commitfest.postgresql.org/12/890/ > > He added your patch as well. > Okay, Thanks. Robert, do you have any better ideas for this problem? -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
Amit Kapila wrote: >> But with Tobias' complete patch "make installcheck-world" succeeds. > > Okay. I have looked into patch proposed and found that it will > resolve the regression test problem (Create Table .. AS Execute). I > think it is slightly prone to breakage if tomorrow we implement usage > of EXECUTE with statements other Create Table (like Copy). Have you > registered this patch in CF to avoid loosing track? > > We have two patches (one proposed in this thread and one proposed by > me earlier [1]) and both solves the regression test failure. Unless > there is some better approach, I think we can go with one of these. Tobias did that here: https://commitfest.postgresql.org/12/890/ He added your patch as well. Yours, Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Fri, Nov 18, 2016 at 9:01 PM, Albe Laurenz wrote: > Amit Kapila wrote: >> Can you once test by just passing CURSOR_OPT_PARALLEL_OK in >> PrepareQuery() and run the tests by having forc_parallel_mode=regress? >> It seems to me that the code in the planner is changed for setting a >> parallelModeNeeded flag, so it might work as it is. > > No, that doesn't work. > > But with Tobias' complete patch "make installcheck-world" succeeds. > Okay. I have looked into patch proposed and found that it will resolve the regression test problem (Create Table .. AS Execute). I think it is slightly prone to breakage if tomorrow we implement usage of EXECUTE with statements other Create Table (like Copy). Have you registered this patch in CF to avoid loosing track? We have two patches (one proposed in this thread and one proposed by me earlier [1]) and both solves the regression test failure. Unless there is some better approach, I think we can go with one of these. [1] - https://www.postgresql.org/message-id/CAA4eK1L%3DtHmmHDK_KW_ja1_dusJxJF%2BSGQHi%3DAPS4MdNPk7HFQ%40mail.gmail.com -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
There has been a previous discussion about this topic, including an attempted fix by Amit Kapila: http://www.postgresql.org/message-id/flat/CAA4eK1L=tHmmHDK_KW_ja1_dusJxJF+SGQHi=aps4mdnpk7...@mail.gmail.com/ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
Am 18.11.2016 um 14:21 schrieb Albe Laurenz :
> But then the change to use CURSOR_OPT_PARALLEL_OK in tcop/postgres.c should
> be reverted as well, because it breaks things just as bad:
>
> /* creates a parallel-enabled plan */
> PQprepare(conn, "pstmt", "SELECT id FROM l WHERE val = $1", 1, NULL);
> /* blows up with "cannot insert tuples during a parallel operation" */
> PQexec(conn, "CREATE TABLE bad(id) AS EXECUTE pstmt('abcd')");
Great example of mixing a v3 prepare with an simple query execute. I didn't
think about that while even the docs state clearly: "Named prepared statements
can also be created and accessed at the SQL command level, using PREPARE and
EXECUTE." Sticking with either protocol version does not trigger the error.
> I think we should either apply something like that patch or disable parallel
> execution for prepared statements altogether and document that.
So we likely need to backpatch something more then a doc-fix for 9.6. Given the
patch proposals around, this would either disable a feature (in extended query
protocol) or add a new one (in simple query protocol/SQL). Or would it be more
appropriate to split the patch and use CURSOR_OPT_PARALLEL_OK in prepare.c on
master only? I'm asking in case there is a necessity to prepare a proposal for
an documentation patch targeting 9.6 specifically.
Best regards
Tobias
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
> True, but we also try to avoid it whenever possible, because it's > likely to lead to poor performance. This non-readonly case should be way less often hit compared to other uses of prepared statements. But sure, it depends on the individual use case and a likely performance regession in these edge cases is nothing to decide for easily. > I think it would be a good idea to come up with a way for a query to > produce both a parallel and a non-parallel plan and pick between them > at execution time. However, that's more work than I've been willing > to undertake. Wouldn't the precautionary generation of two plans always increase the planning overhead, which precisely is what one want to reduce by using prepared statements? Best regards Tobias -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
Robert Haas wrote:
>>/* creates a parallel-enabled plan */
>>PQprepare(conn, "pstmt", "SELECT id FROM l WHERE val = $1", 1, NULL);
>>/* blows up with "cannot insert tuples during a parallel operation" */
>>PQexec(conn, "CREATE TABLE bad(id) AS EXECUTE pstmt('abcd')");
>
> Ouch. I missed that.
>
>> With Tobias' patch, this does not fail.
>>
>> I think we should either apply something like that patch or disable parallel
>> execution for prepared statements altogether and document that.
>
> I agree. I think probably the first option is better, but I might
> have to commit with one hand so I can hold my nose with the other.
Is there a better, more consistent approach for that?
Yours,
Laurenz Albe
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Fri, Nov 18, 2016 at 8:21 AM, Albe Laurenz wrote:
> I didn't notice the CREATE TABLE ... AS EXECUTE problem.
>
> But then the change to use CURSOR_OPT_PARALLEL_OK in tcop/postgres.c should
> be reverted as well, because it breaks things just as bad:
>
>/* creates a parallel-enabled plan */
>PQprepare(conn, "pstmt", "SELECT id FROM l WHERE val = $1", 1, NULL);
>/* blows up with "cannot insert tuples during a parallel operation" */
>PQexec(conn, "CREATE TABLE bad(id) AS EXECUTE pstmt('abcd')");
Ouch. I missed that.
> With Tobias' patch, this does not fail.
>
> I think we should either apply something like that patch or disable parallel
> execution for prepared statements altogether and document that.
I agree. I think probably the first option is better, but I might
have to commit with one hand so I can hold my nose with the other.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
Amit Kapila wrote: > Can you once test by just passing CURSOR_OPT_PARALLEL_OK in > PrepareQuery() and run the tests by having forc_parallel_mode=regress? > It seems to me that the code in the planner is changed for setting a > parallelModeNeeded flag, so it might work as it is. No, that doesn't work. But with Tobias' complete patch "make installcheck-world" succeeds. Yours, Laurenz Albe -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
Robert Haas wrote:
> On Tue, Nov 15, 2016 at 10:41 AM, Albe Laurenz
> wrote:
>> Tobias Bussmann has discovered an oddity with prepared statements.
>>
>> Parallel scan is used with prepared statements, but only if they have
>> been created with protocol V3 "Parse".
>> If a prepared statement has been prepared with the SQL statement PREPARE,
>> it will never use a parallel scan.
>>
>> I guess that is an oversight in commit 57a6a72b, right?
>> PrepareQuery in commands/prepare.c should call CompleteCachedPlan
>> with cursor options CURSOR_OPT_PARALLEL_OK, just like
>> exec_prepare_message in tcop/postgres.c does.
>>
>> The attached patch fixes the problem for me.
>
> Actually, commit 57a6a72b made this change, and then 7bea19d0 backed
> it out again because it turned out to break things.
I didn't notice the CREATE TABLE ... AS EXECUTE problem.
But then the change to use CURSOR_OPT_PARALLEL_OK in tcop/postgres.c should
be reverted as well, because it breaks things just as bad:
/* creates a parallel-enabled plan */
PQprepare(conn, "pstmt", "SELECT id FROM l WHERE val = $1", 1, NULL);
/* blows up with "cannot insert tuples during a parallel operation" */
PQexec(conn, "CREATE TABLE bad(id) AS EXECUTE pstmt('abcd')");
With Tobias' patch, this does not fail.
I think we should either apply something like that patch or disable parallel
execution for prepared statements altogether and document that.
Yours,
Laurenz Albe
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Thu, Nov 17, 2016 at 10:07 AM, Tobias Bussmann wrote: >> Yeah, we could do something like this, perhaps not in exactly this >> way, but I'm not sure it's a good idea to just execute the parallel >> plan without workers. > > sure, executing parallel plans w/o workers seems a bit of a hack. But: > - we already do it this way in some other situations True, but we also try to avoid it whenever possible, because it's likely to lead to poor performance. > - the alternative in this special situation would be to _force_ replanning > without the CURSOR_OPT_PARALLEL_OK. The decision for replanning is hidden > deep within plancache.c and while we could influence it with > CURSOR_OPT_CUSTOM_PLAN this wouldn't have an effect if the prepared statement > doesn't have any parameters. Additionally, influencing the decision and > generating a non-parallel plan would shift the avg cost calculation used to > choose custom or generic plans. I think it would be a good idea to come up with a way for a query to produce both a parallel and a non-parallel plan and pick between them at execution time. However, that's more work than I've been willing to undertake. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
> Yeah, we could do something like this, perhaps not in exactly this > way, but I'm not sure it's a good idea to just execute the parallel > plan without workers. sure, executing parallel plans w/o workers seems a bit of a hack. But: - we already do it this way in some other situations - the alternative in this special situation would be to _force_ replanning without the CURSOR_OPT_PARALLEL_OK. The decision for replanning is hidden deep within plancache.c and while we could influence it with CURSOR_OPT_CUSTOM_PLAN this wouldn't have an effect if the prepared statement doesn't have any parameters. Additionally, influencing the decision and generating a non-parallel plan would shift the avg cost calculation used to choose custom or generic plans. Maybe someone can come up with a better idea for a solution. These three approaches are all I see so far. Best regards, Tobias Bussmann -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Tue, Nov 15, 2016 at 3:57 PM, Tobias Bussmann wrote: > As the patch in [1] targeting the execution of the plan in ExecutePlan > depending on the destination was declined, I hacked around a bit to find > another way to use parallel mode with SQL prepared statements while disabling > the parallel execution in case of an non read-only execution. For this I used > the already present test for an existing intoClause in ExecuteQuery to set > the parallelModeNeeded flag of the prepared statement. This results in a non > parallel execution of the parallel plan, as we see with a non-zero fetch > count used with the extended query protocol. Despite this patch seem to work > in my tests, I'm by no means confident this being a proper way of handling > the situation in question. Yeah, we could do something like this, perhaps not in exactly this way, but I'm not sure it's a good idea to just execute the parallel plan without workers. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
> Can you once test by just passing CURSOR_OPT_PARALLEL_OK in > PrepareQuery() and run the tests by having forc_parallel_mode=regress? > It seems to me that the code in the planner is changed for setting a > parallelModeNeeded flag, so it might work as it is. Do you mean running a "make installcheck" with "force_parallel_mode=regress" in postgresql.conf? I did so with just CURSOR_OPT_PARALLEL_OK in PrepareQuery (like the original commit 57a6a72b) and still got 3 failed tests, all with CREATE TABLE .. AS EXECUTE .. . With my patch applied, all tests were successful. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Tue, Nov 15, 2016 at 10:41 AM, Albe Laurenz wrote: > Tobias Bussmann has discovered an oddity with prepared statements. > > Parallel scan is used with prepared statements, but only if they have > been created with protocol V3 "Parse". > If a prepared statement has been prepared with the SQL statement PREPARE, > it will never use a parallel scan. > > I guess that is an oversight in commit 57a6a72b, right? > PrepareQuery in commands/prepare.c should call CompleteCachedPlan > with cursor options CURSOR_OPT_PARALLEL_OK, just like > exec_prepare_message in tcop/postgres.c does. > > The attached patch fixes the problem for me. Actually, commit 57a6a72b made this change, and then 7bea19d0 backed it out again because it turned out to break things. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Wed, Nov 16, 2016 at 7:10 PM, Amit Kapila wrote: > On Wed, Nov 16, 2016 at 2:27 AM, Tobias Bussmann wrote: >> As the patch in [1] targeting the execution of the plan in ExecutePlan >> depending on the destination was declined, I hacked around a bit to find >> another way to use parallel mode with SQL prepared statements while >> disabling the parallel execution in case of an non read-only execution. For >> this I used the already present test for an existing intoClause in >> ExecuteQuery to set the parallelModeNeeded flag of the prepared statement. >> This results in a non parallel execution of the parallel plan, as we see >> with a non-zero fetch count used with the extended query protocol. Despite >> this patch seem to work in my tests, I'm by no means confident this being a >> proper way of handling the situation in question. >> > > Can you once test by just passing CURSOR_OPT_PARALLEL_OK in > PrepareQuery() and run the tests by having forc_parallel_mode=regress? Typo. /forc_parallel_mode/force_parallel_mode -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
On Wed, Nov 16, 2016 at 2:27 AM, Tobias Bussmann wrote: > As the patch in [1] targeting the execution of the plan in ExecutePlan > depending on the destination was declined, I hacked around a bit to find > another way to use parallel mode with SQL prepared statements while disabling > the parallel execution in case of an non read-only execution. For this I used > the already present test for an existing intoClause in ExecuteQuery to set > the parallelModeNeeded flag of the prepared statement. This results in a non > parallel execution of the parallel plan, as we see with a non-zero fetch > count used with the extended query protocol. Despite this patch seem to work > in my tests, I'm by no means confident this being a proper way of handling > the situation in question. > Can you once test by just passing CURSOR_OPT_PARALLEL_OK in PrepareQuery() and run the tests by having forc_parallel_mode=regress? It seems to me that the code in the planner is changed for setting a parallelModeNeeded flag, so it might work as it is. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
As the patch in [1] targeting the execution of the plan in ExecutePlan depending on the destination was declined, I hacked around a bit to find another way to use parallel mode with SQL prepared statements while disabling the parallel execution in case of an non read-only execution. For this I used the already present test for an existing intoClause in ExecuteQuery to set the parallelModeNeeded flag of the prepared statement. This results in a non parallel execution of the parallel plan, as we see with a non-zero fetch count used with the extended query protocol. Despite this patch seem to work in my tests, I'm by no means confident this being a proper way of handling the situation in question. Best Tobias [1] https://www.postgresql.org/message-id/CAA4eK1KxiYm8F9Pe9xvqzoZocK43w%3DTRPUNHZpe_iOjF%3Dr%2B_Vw%40mail.gmail.com prepared_stmt_execute_parallel_query.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel execution and prepared statements
Thanks Laurenz, for your help with debugging on this topic. I was preparing a message to the list myself and found an earlier discussion [1] on the topic. If I understand it correctly, the issue is with CREATE TABLE ... AS EXECUTE So it seems parallel execution of prepared statements was intentionally disabled in 7bea19d. However, what is missing is at least a mention of that current limitation in the 9.6 docs at "When Can Parallel Query Be Used?" [2] Best regards, Tobias [1] https://www.postgresql.org/message-id/CA%2BTgmoaxyXVr6WPDvPQduQpFhD9VRWExXU7axhDpJ7jZBvqxfQ%40mail.gmail.com [2] https://www.postgresql.org/docs/current/static/when-can-parallel-query-be-used.html > Am 15.11.2016 um 16:41 schrieb Albe Laurenz : > > Tobias Bussmann has discovered an oddity with prepared statements. > > Parallel scan is used with prepared statements, but only if they have > been created with protocol V3 "Parse". > If a prepared statement has been prepared with the SQL statement PREPARE, > it will never use a parallel scan. > > I guess that is an oversight in commit 57a6a72b, right? > PrepareQuery in commands/prepare.c should call CompleteCachedPlan > with cursor options CURSOR_OPT_PARALLEL_OK, just like > exec_prepare_message in tcop/postgres.c does. > > The attached patch fixes the problem for me. > > Yours, > Laurenz Albe > <0001-Consider-parallel-plans-for-statements-prepared-with.patch> > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Parallel execution and prepared statements
Tobias Bussmann has discovered an oddity with prepared statements. Parallel scan is used with prepared statements, but only if they have been created with protocol V3 "Parse". If a prepared statement has been prepared with the SQL statement PREPARE, it will never use a parallel scan. I guess that is an oversight in commit 57a6a72b, right? PrepareQuery in commands/prepare.c should call CompleteCachedPlan with cursor options CURSOR_OPT_PARALLEL_OK, just like exec_prepare_message in tcop/postgres.c does. The attached patch fixes the problem for me. Yours, Laurenz Albe 0001-Consider-parallel-plans-for-statements-prepared-with.patch Description: 0001-Consider-parallel-plans-for-statements-prepared-with.patch -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
