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
Attachments:
0001-Consider-parallel-plans-for-statements-prepared-with.patchapplication/octet-stream; name=0001-Consider-parallel-plans-for-statements-prepared-with.patchDownload+1-2
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]/messages/by-id/CA+TgmoaxyXVr6WPDvPQduQpFhD9VRWExXU7axhDpJ7jZBvqxfQ@mail.gmail.com 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]https://www.postgresql.org/docs/current/static/when-can-parallel-query-be-used.html
Best regards,
Tobias
[1]: /messages/by-id/CA+TgmoaxyXVr6WPDvPQduQpFhD9VRWExXU7axhDpJ7jZBvqxfQ@mail.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 <laurenz.albe@wien.gv.at>:
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
As the patch in [1]/messages/by-id/CAA4eK1KxiYm8F9Pe9xvqzoZocK43w=TRPUNHZpe_iOjF=r+_Vw@mail.gmail.com 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]: /messages/by-id/CAA4eK1KxiYm8F9Pe9xvqzoZocK43w=TRPUNHZpe_iOjF=r+_Vw@mail.gmail.com
Attachments:
prepared_stmt_execute_parallel_query.patchapplication/octet-stream; name=prepared_stmt_execute_parallel_query.patchDownload+18-1
On Wed, Nov 16, 2016 at 2:27 AM, Tobias Bussmann <t.bussmann@gmx.net> 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
On Wed, Nov 16, 2016 at 7:10 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
On Wed, Nov 16, 2016 at 2:27 AM, Tobias Bussmann <t.bussmann@gmx.net> 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
On Tue, Nov 15, 2016 at 10:41 AM, Albe Laurenz <laurenz.albe@wien.gv.at> 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
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
On Tue, Nov 15, 2016 at 3:57 PM, Tobias Bussmann <t.bussmann@gmx.net> 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
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
On Thu, Nov 17, 2016 at 10:07 AM, Tobias Bussmann <t.bussmann@gmx.net> 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
Robert Haas wrote:
On Tue, Nov 15, 2016 at 10:41 AM, Albe Laurenz <laurenz.albe@wien.gv.at> 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
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
On Fri, Nov 18, 2016 at 8:21 AM, Albe Laurenz <laurenz.albe@wien.gv.at> 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
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
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
Am 18.11.2016 um 14:21 schrieb Albe Laurenz <laurenz.albe@wien.gv.at>:
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
There has been a previous discussion about this topic, including an attempted fix by Amit Kapila:
/messages/by-id/CAA4eK1L=tHmmHDK_KW_ja1_dusJxJF+SGQHi=APS4MdNPk7HFQ@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
On Fri, Nov 18, 2016 at 9:01 PM, Albe Laurenz <laurenz.albe@wien.gv.at> 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]/messages/by-id/CAA4eK1L=tHmmHDK_KW_ja1_dusJxJF+SGQHi=APS4MdNPk7HFQ@mail.gmail.com -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com) and both solves the regression test failure. Unless
there is some better approach, I think we can go with one of these.
[1]: /messages/by-id/CAA4eK1L=tHmmHDK_KW_ja1_dusJxJF+SGQHi=APS4MdNPk7HFQ@mail.gmail.com -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.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
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
On Tue, Nov 29, 2016 at 4:40 PM, Albe Laurenz <laurenz.albe@wien.gv.at> 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
