Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
Hi, Hello! Just had a need for this feature, and took this to a short test drive. So some comments: - it'd be useful to display a retry percentage of all transactions, similar to what's displayed for failed transactions. - it'd be useful to also conveniently display the number of retried transactions, rather than the total number of retries. Ok! - it appears that we now unconditionally do not disregard a connection after a serialization / deadlock failure. Good. But that's useful far beyond just deadlocks / serialization errors, and should probably be exposed. I agree that it will be useful. But how do you propose to print the results if there are many types of errors? I'm afraid that the progress report can be very long although it is expected that it will be rather short [1]. The per statement report can also be very long.. Nice feature! Thanks and thank you for your comments :) [1] https://www.postgresql.org/message-id/alpine.DEB.2.20.1707121142300.12795%40lancre -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
Hello again! Here is the third version of the patch for pgbench thanks to Fabien Coelho comments. As in the previous one, transactions with serialization and deadlock failures are rolled back and retried until they end successfully or their number of tries reaches maximum. Differences from the previous version: * Some code cleanup :) In particular, the Variables structure for managing client variables and only one new tap tests file (as they were recommended here [1] and here [2]). * There's no error if the last transaction in the script is not completed. But the transactions started in the previous scripts and/or not ending in the current script, are not rolled back and retried after the failure. Such script try is reported as failed because it contains a failure that was not rolled back and retried. * Usually the retries and/or failures are printed if they are not equal to zeros. In transaction/aggregation logs the failures are always printed and the retries are printed if max_tries is greater than 1. It is done for the general format of the log during the execution of the program. Patch is attached. Any suggestions are welcome! [1] https://www.postgresql.org/message-id/alpine.DEB.2.20.1707121338090.12795%40lancre [2] https://www.postgresql.org/message-id/alpine.DEB.2.20.1707121142300.12795%40lancre -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres CompanyFrom 0ee372e93b8d7017563d2f2f55357c39c08a Mon Sep 17 00:00:00 2001 From: Marina Polyakova <m.polyak...@postgrespro.ru> Date: Fri, 21 Jul 2017 17:57:58 +0300 Subject: [PATCH v3] Pgbench Retry transactions with serialization or deadlock errors Now transactions with serialization or deadlock failures can be rolled back and retried again and again until they end successfully or their number of tries reaches maximum. You can set the maximum number of tries by using the appropriate benchmarking option (--max-tries). The default value is 1. If there're retries and/or failures their statistics are printed in the progress, in the transaction / aggregation logs and in the end with other results (all and for each script). A transaction failure is reported here only if the last try of this transaction fails. Also retries and/or failures are printed per-command with average latencies if you use the appropriate benchmarking option (--report-per-command, -r) and the total number of retries and/or failures is not zero. Note that the transactions started in the previous scripts and/or not ending in the current script, are not rolled back and retried after the failure. Such script try is reported as failed because it contains a failure that was not rolled back and retried. --- doc/src/sgml/ref/pgbench.sgml | 240 +- src/bin/pgbench/pgbench.c | 872 + .../t/002_serialization_and_deadlock_failures.pl | 459 +++ 3 files changed, 1412 insertions(+), 159 deletions(-) create mode 100644 src/bin/pgbench/t/002_serialization_and_deadlock_failures.pl diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index 64b043b..3bbeec5 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -49,6 +49,7 @@ transaction type: builtin: TPC-B (sort of) +transaction maximum tries number: 1 scaling factor: 10 query mode: simple number of clients: 10 @@ -59,7 +60,7 @@ tps = 85.184871 (including connections establishing) tps = 85.296346 (excluding connections establishing) - The first six lines report some of the most important parameter + The first seven lines report some of the most important parameter settings. The next line reports the number of transactions completed and intended (the latter being just the product of number of clients and number of transactions per client); these will be equal unless the run @@ -436,22 +437,33 @@ pgbench options dbname Show progress report every sec seconds. The report includes the time since the beginning of the run, the tps since the last report, and the transaction latency average and standard -deviation since the last report. Under throttling (-R), -the latency is computed with respect to the transaction scheduled -start time, not the actual transaction beginning time, thus it also -includes the average schedule lag time. +deviation since the last report. If since the last report there are +transactions that ended with serialization/deadlock failures they are +also reported here as failed (see + for more information). Under +throttling (-R), the latency is computed with respect to the +transaction scheduled start time, not the actual transaction beginning +time, thus it also includes the average schedule lag time. If since the +last report there're transactions that have been rolled back and r
Re: [HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1
Like for the previous patches it seems that there is
no obvious performance degradation too on regular queries (according
to
pgbench).
pgbench probably isn't a very good test for this sort of thing - it
only issues very short-running queries where the cost of evaluating
expressions is a relatively small part of the total cost. Even if
things get worse, I'm not sure if you'd see it.
If there's a mistake, for example, more than 1 try to replace cached
expressions in the whole query tree, results of "in buffer test" or
"mostly cache test" can different a little..
I'm not sure exactly
how you could construct a test case that could be harmed by this patch
- I guess you'd want to initialize lots of CacheExprs but never make
use of the caching usefully?
As I mentioned in the first letter about this feature it will be useful
for such text search queries [1]:
SELECT COUNT(*) FROM messages WHERE body_tsvector @@
to_tsquery('postgres');
And I'm not sure that it is logical to precalculate stable and immutable
functions themselves, but not to precalculate expressions that behave
like stable/immutable functions; precalculate some types of operators
and not to precalculate others (ScalarArrayOpExpr, RowCompareExpr). My
patch solves the problem that not all nodes are simplified in
eval_const_expressions_mutator (for example, ScalarArrayOpExpr) and
consts of other types now behave more like ordinary consts (for example,
composite types, coerce expressions, ConvertRowtypeExpr).
It could also be useful to test things like TPC-H to see if you get an
improvement.
I saw the examples of queries in TPC-H tests. If I'm not wrong they are
not the target tests for this functionality (nothing will be
precalculated). But it's a good idea to check that there's no a
performance degradation on them too.
[1]
https://www.postgresql.org/message-id/ba261b9fc25dea4069d8ba9a8fcadf35%40postgrespro.ru
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
Well, the short version may be to only do a full transaction retry and to document that for now savepoints are not handled, and to let that for future work if need arises. I agree with you. For progress the output must be short and readable, and probably we do not care about whether retries came from this or that, so I would let that out. For log and aggregated log possibly that would make more sense, but it must stay easy to parse. Ok! -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
Ok, fine. My point was just to check before proceeding. And I'm very grateful for that :) -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
Given that the number of variables of a pgbench script is expected to be pretty small, I'm not sure that the sorting stuff is worth the effort. I think it is a good insurance if there're many variables.. My suggestion is really to look at both implementations and to answer the question "should pgbench share its variable implementation with psql?". If the answer is yes, then the relevant part of the implementation should be moved to fe_utils, and that's it. If the answer is no, then implement something in pgbench directly. The structure of variables is different, the container structure of the variables is different, so I think that the answer is no. -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
Not necessarily? It depends on where the locks triggering the issue are set, if they are all set after the savepoint it could work on a second attempt. Don't you mean the deadlock failures where can really help rollback to Yes, I mean deadlock failures can rollback to a savepoint and work on a second attempt. And could you, please, give an example where a rollback to savepoint can help to end its subtransaction successfully after a serialization failure? I do not know whether this is possible with about serialization failures. It might be if the stuff before and after the savepoint are somehow unrelated... If you mean, for example, the updates of different tables - a rollback to savepoint doesn't help. And I'm not sure that we should do all the stuff for savepoints rollbacks because: - as I see it now it only makes sense for the deadlock failures; - if there's a failure what savepoint we should rollback to and start the execution again? Maybe to go to the last one, if it is not successful go to the previous one etc. Retrying the entire transaction may take less time.. [...] I mean that the sum of transactions with serialization failure and transactions with deadlock failure can be greater then the totally sum of transactions with failures. Hmmm. Ok. A "failure" is a transaction (in the sense of pgbench) that could not made it to the end, even after retries. If there is a rollback and the a retry which works, it is not a failure. Now deadlock or serialization errors, which trigger retries, are worth counting as well, although they are not "failures". So my format proposal was over optimistic, and the number of deadlocks and serializations should better be on a retry count line. Maybe something like: ... number of failures: 12 (0.004%) number of retries: 64 (deadlocks: 29, serialization: 35) Ok! How to you like the idea to use the same format (the total number of transactions with failures and the number of retries for each failure type) in other places (log, aggregation log, progress) if the values are not "default" (= no failures and no retries)? -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
I was not convinced by the overall memory management around variables to begin with, and it is even less so with their new copy management. Maybe having a clean "Variables" data structure could help improve the situation. Note that there is something for psql (src/bin/psql/variable.c) which may or may not be shared. It should be checked before recoding eventually the same thing. Thank you very much for pointing this file! As I checked this is another structure: here there's a simple list, while in pgbench we should know if the list is sorted and the number of elements in the list. How do you think, is it a good idea to name a variables structure in pgbench in the same way (VariableSpace) or it should be different not to be confused (Variables, for example)? -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
On 13-07-2017 19:32, Fabien COELHO wrote: Hello, Hi! [...] I didn't make rollbacks to savepoints after the failure because they cannot help for serialization failures at all: after rollback to savepoint a new attempt will be always unsuccessful. Not necessarily? It depends on where the locks triggering the issue are set, if they are all set after the savepoint it could work on a second attempt. Don't you mean the deadlock failures where can really help rollback to savepoint? And could you, please, give an example where a rollback to savepoint can help to end its subtransaction successfully after a serialization failure? "SimpleStats attempts": I disagree with using this floating poiunt oriented structures to count integers. I would suggest "int64 tries" instead, which should be enough for the purpose. I'm not sure that it is enough. Firstly it may be several transactions in script so to count the average attempts number you should know the total number of runned transactions. Secondly I think that stddev for attempts number can be quite interesting and often it is not close to zero. I would prefer to have a real motivation to add this complexity in the report and in the code. Without that, a simple int seems better for now. It can be improved later if the need really arises. Ok! Some variables, such as "int attempt_number", should be in the client structure, not in the client? Generally, try to use block variables if possible to keep the state clearly disjoints. If there could be NO new variable at the doCustom level that would be great, because that would ensure that there is no machine state mixup hidden in these variables. Do you mean the code cleanup for doCustom function? Because if I do so there will be two code styles for state blocks and their variables in this function.. I think that any variable shared between state is a recipee for bugs if it is not reset properly, so they should be avoided. Maybe there are already too many of them, then too bad, not a reason to add more. The status before the automaton was a nightmare. Ok! I would suggest a more compact one-line report about failures: "number of failures: 12 (0.001%, deadlock: 7, serialization: 5)" I think, there may be a misunderstanding. Because script can contain several transactions and get both failures. I do not understand. Both failures number are on the compact line I suggested. I mean that the sum of transactions with serialization failure and transactions with deadlock failure can be greater then the totally sum of transactions with failures. But if you think it's ok I'll change it and write the appropriate note in documentation. -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
Another detail I forgot about this point: there may be a memory leak on variables copies, ISTM that the "variables" array is never freed. I was not convinced by the overall memory management around variables to begin with, and it is even less so with their new copy management. Maybe having a clean "Variables" data structure could help improve the situation. Ok! -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
ld end it before retrying. It means to go to states CSTATE_START_COMMAND -> CSTATE_WAIT_RESULT -> CSTATE_END_COMMAND with the appropriate command. How do you propose not to go to these states? About malloc - I agree with you that it should be done without malloc/free. About savepoints - as I wrote you earlier I didn't make rollbacks to savepoints after the failure. Because they cannot help for serialization failures at all: after rollback to savepoint a new attempt will be always unsuccessful. I disagree about exit in ParseScript if the transaction block is not completed, especially as it misses out on combined statements/queries (BEGIN \; stuff... \; COMMIT") and would break an existing feature. Thanks, I'll fix it for usual transaction blocks that don't end in the scripts. There are strange characters things in comments, eg "??ontinuous". Oh, I'm sorry. I'll fix it too. Option "max-attempt-number" -> "max-tries" I would put the client random state initialization with the state intialization, not with the connection. * About tracing Progress is expected to be short, not detailed. Only add the number of failures and retries if max retry is not 1. Ok! * About reporting I think that too much is reported. I advised to do that, but nevertheless it is a little bit steep. At least, it should not report the number of tries/attempts when the max number is one. Ok! Simple counting should be reported for failures, not floats... I would suggest a more compact one-line report about failures: "number of failures: 12 (0.001%, deadlock: 7, serialization: 5)" I think, there may be a misunderstanding. Because script can contain several transactions and get both failures. * About the TAP tests They are too expensive, with 3 initdb. I think that they should be integrated in the existing tests, as a patch has been submitted to rework the whole pgbench tap test infrastructure. For now, at most one initdb and several small tests inside. Ok! * About the documentation I'm not sure that the feature needs pre-emminence in the documentation, because most of the time there is no retry as none is needed, there is no failure, so this rather a special (although useful) case for people playing with serializable and other advanced features. Smaller updates, without dedicated examples, should be enough. Maybe there should be some examples to prepare people what they can see in the output of the program? Of course now failures are special cases because they disconnect its clients to the end of the program and ruin all the results. I hope that if this patch is committed there will be much more cases with retried failures. If a transaction is skipped, there was no tries, so the corresponding number of attempts is 0, not one. Oh, I'm sorry, it is a typo in the documentation. -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
Hello everyone! There's the second version of my patch for pgbench. Now transactions with serialization and deadlock failures are rolled back and retried until they end successfully or their number of attempts reaches maximum. In details: - You can set the maximum number of attempts by the appropriate benchmarking option (--max-attempts-number). Its default value is 1 partly because retrying of shell commands can produce new errors. - Statistics of attempts and failures is printed in progress, in transaction / aggregation logs and in the end with other results (all and for each script). The transaction failure is reported here only if the last retry of this transaction fails. - Also failures and average numbers of transactions attempts are printed per-command with average latencies if you use the appropriate benchmarking option (--report-per-command, -r) (it replaces the option --report-latencies as I was advised here [1]). Average numbers of transactions attempts are printed only for commands which start transactions. As usual: TAP tests for new functionality and changed documentation with new examples. Patch is attached. Any suggestions are welcome! [1] https://www.postgresql.org/message-id/alpine.DEB.2.20.1707031321370.3419%40lancre -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres CompanyFrom 58f51cdc896af801bcd35e495406655ca03aa6ce Mon Sep 17 00:00:00 2001 From: Marina Polyakova <m.polyak...@postgrespro.ru> Date: Mon, 10 Jul 2017 13:33:41 +0300 Subject: [PATCH v2] Pgbench Retry transactions with serialization or deadlock errors Now transactions with serialization or deadlock failures can be rolled back and retried again and again until they end successfully or their number of attempts reaches maximum. You can set the maximum number of attempts by the appropriate benchmarking option (--max-attempts-number). Its default value is 1. Statistics of attempts and failures is printed in progress, in transaction / aggregation logs and in the end with other results (all and for each script). The transaction failure is reported here only if the last retry of this transaction fails. Also failures and average numbers of transactions attempts are printed per-command with average latencies if you use the appropriate benchmarking option (--report-per-command, -r). Average numbers of transactions attempts are printed only for commands which start transactions. --- doc/src/sgml/ref/pgbench.sgml | 277 ++-- src/bin/pgbench/pgbench.c | 751 ++--- src/bin/pgbench/t/002_serialization_errors.pl | 121 src/bin/pgbench/t/003_deadlock_errors.pl | 130 src/bin/pgbench/t/004_retry_failed_transactions.pl | 280 5 files changed, 1421 insertions(+), 138 deletions(-) create mode 100644 src/bin/pgbench/t/002_serialization_errors.pl create mode 100644 src/bin/pgbench/t/003_deadlock_errors.pl create mode 100644 src/bin/pgbench/t/004_retry_failed_transactions.pl diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml index 64b043b..dc1daa9 100644 --- a/doc/src/sgml/ref/pgbench.sgml +++ b/doc/src/sgml/ref/pgbench.sgml @@ -49,22 +49,34 @@ transaction type: builtin: TPC-B (sort of) +transaction maximum attempts number: 1 scaling factor: 10 query mode: simple number of clients: 10 number of threads: 1 number of transactions per client: 1000 number of transactions actually processed: 1/1 +number of transactions with serialization failures: 0 (0.000 %) +number of transactions with deadlock failures: 0 (0.000 %) +attempts number average = 1.00 +attempts number stddev = 0.00 tps = 85.184871 (including connections establishing) tps = 85.296346 (excluding connections establishing) - The first six lines report some of the most important parameter + The first seven lines report some of the most important parameter settings. The next line reports the number of transactions completed and intended (the latter being just the product of number of clients and number of transactions per client); these will be equal unless the run failed before completion. (In -T mode, only the actual number of transactions is printed.) + The next four lines report the number of transactions with serialization and + deadlock failures, and also the statistics of transactions attempts. With + such errors, transactions are rolled back and are repeated again and again + until they end sucessufully or their number of attempts reaches maximum (to + change this maximum see the appropriate benchmarking option + --max-attempts-number). The transaction failure is reported here + only if the last retry of this transaction fails. The last two lines report the number of transactions per second, figured with and without counting the time to start database sessions. @@ -434,24 +446,28 @@ pgbench options
Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
The current error handling is either "close connection" or maybe in some cases even "exit". If this is changed, then the client may continue execution in some unforseen state and behave unexpectedly. We'll see. Thanks, now I understand this. ISTM that the retry implementation should be implemented somehow in the automaton, restarting the same script for the beginning. If there are several transactions in this script - don't you think that we should restart only the failed transaction?.. On some transaction failures based on their status. My point is that the retry process must be implemented clearly with a new state in the client automaton. Exactly when the transition to this new state must be taken is another issue. About it, I agree with you that it should be done in this way. The number of retries and maybe failures should be counted, maybe with some adjustable maximum, as suggested. If we fix the maximum number of attempts the maximum number of failures for one script execution will be bounded above (number_of_transactions_in_script * maximum_number_of_attempts). Do you think we should make the option in program to limit this number much more? Probably not. I think that there should be a configurable maximum of retries on a transaction, which may be 0 by default if we want to be upward compatible with the current behavior, or maybe something else. I propose the option --max-attempts-number=NUM which NUM cannot be less than 1. I propose it because I think that, for example, --max-attempts-number=100 is better than --max-retries-number=99. And maybe it's better to set its default value to 1 too because retrying of shell commands can produce new errors.. In doLog, added columns should be at the end of the format. I have inserted it earlier because these columns are not optional. Do you think they should be optional? I think that new non-optional columns it should be at the end of the existing non-optional columns so that existing scripts which may process the output may not need to be updated. Thanks, I agree with you :) I'm not sure that there should be an new option to report failures, the information when relevant should be integrated in a clean format into the existing reports... Maybe the "per command latency" report/option should be renamed if it becomes more general. I have tried do not change other parts of program as much as possible. But if you think that it will be more useful to change the option I'll do it. I think that the option should change if its naming becomes less relevant, which is to be determined. AFAICS, ISTM that new measures should be added to the various existing reports unconditionnaly (i.e. without a new option), so maybe no new option would be needed. Thanks! I didn't think about it in this way.. -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
Hello Marina, Hello, Fabien! A few comments about the submitted patches. Thank you very much for them! I agree that improving the error handling ability of pgbench is a good thing, although I'm not sure about the implications... Could you tell a little bit more exactly.. What implications are you worried about? About the "retry" discussion: I agree that retry is the relevant option from an application point of view. I'm glad to hear it! ISTM that the retry implementation should be implemented somehow in the automaton, restarting the same script for the beginning. If there are several transactions in this script - don't you think that we should restart only the failed transaction?.. As pointed out in the discussion, the same values/commands should be executed, which suggests that random generated values should be the same on the retry runs, so that for a simple script the same operations are attempted. This means that the random generator state must be kept & reinstated for a client on retries. Currently the random state is in the thread, which is not convenient for this purpose, so it should be moved in the client so that it can be saved at transaction start and reinstated on retries. I think about it in the same way =) The number of retries and maybe failures should be counted, maybe with some adjustable maximum, as suggested. If we fix the maximum number of attempts the maximum number of failures for one script execution will be bounded above (number_of_transactions_in_script * maximum_number_of_attempts). Do you think we should make the option in program to limit this number much more? About 0001: In accumStats, just use one level if, the two levels bring nothing. Thanks, I agree =[ In doLog, added columns should be at the end of the format. I have inserted it earlier because these columns are not optional. Do you think they should be optional? The number of column MUST NOT change when different issues arise, so that it works well with cut/... unix commands, so inserting a sentence such as "serialization and deadlock failures" is a bad idea. Thanks, I agree again. threadRun: the point of the progress format is to fit on one not too wide line on a terminal and to allow some simple automatic processing. Adding a verbose sentence in the middle of it is not the way to go. I was thinking about it.. Thanks, I'll try to make it shorter. About tests: I do not understand why test 003 includes 2 transactions. It would seem more logical to have two scripts. Ok! About 0003: I'm not sure that there should be an new option to report failures, the information when relevant should be integrated in a clean format into the existing reports... Maybe the "per command latency" report/option should be renamed if it becomes more general. I have tried do not change other parts of program as much as possible. But if you think that it will be more useful to change the option I'll do it. About 0004: The documentation must not be in a separate patch, but in the same patch as their corresponding code. Ok! -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
> To be clear, part of "retrying from the beginning" means that if a> result from one statement is used to determine the content (or> whether to run) a subsequent statement, that first statement must be> run in the new transaction and the results evaluated again to> determine what to use for the later statement. You can't simply> replay the statements that were run during the first try. For> examples, to help get a feel of why that is, see:> > https://wiki.postgresql.org/wiki/SSIThank you again! :))-- Marina PolyakovaPostgres Professional: http://www.postgrespro.comThe Russian Postgres Company
Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
>> P.S. Does this use case (do not retry transaction with serialization or >> deadlock failure) is most interesting or failed transactions should be >> retried (and how much times if there seems to be no hope of success...)? > > I can't quite parse that sentence, could you restate? The way I read it was that the most interesting solution would retry a transaction from the beginning on a serialization failure or deadlock failure. As far as I understand her proposal, it is exactly the opposite -- if a transaction fails, it is discarded. And this P.S. note is asking whether this is a good idea, or would we prefer that failing transactions are retried. Yes, I have meant this, thank you! I think it's pretty obvious that transactions that failed with some serializability problem should be retried. Thank you voted :) -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
P.S. Does this use case (do not retry transaction with serialization or deadlock failure) is most interesting or failed transactions should be retried (and how much times if there seems to be no hope of success...)? I can't quite parse that sentence, could you restate? The way I read it was that the most interesting solution would retry a transaction from the beginning on a serialization failure or deadlock failure. Most people who use serializable transactions (at least in my experience) run though a framework that does that automatically, regardless of what client code initiated the transaction. These retries are generally hidden from the client code -- it just looks like the transaction took a bit longer. Sometimes people will have a limit on the number of retries. I never used such a limit and never had a problem, because our implementation of serializable transactions will not throw a serialization failure error until one of the transactions involved in causing it has successfully committed -- meaning that the retry can only hit this again on a *new* set of transactions. Essentially, the transaction should only count toward the TPS rate when it eventually completes without a serialization failure. Marina, did I understand you correctly? Álvaro Herrera in next message of this thread has understood my text right: As far as I understand her proposal, it is exactly the opposite -- if a transaction fails, it is discarded. And this P.S. note is asking whether this is a good idea, or would we prefer that failing transactions are retried. And thank you very much for your explanation how and why transactions with failures should be retried! I'll try to implement all of it. -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
Hi, Hello! I think that's a good idea and sorely needed. Thanks, I'm very glad to hear it! - if there were these failures during script execution this "transaction" is marked appropriately in logs; - numbers of "transactions" with these failures are printed in progress, in aggregation logs and in the end with other results (all and for each script); I guess that'll include a "rolled-back %' or 'retried %' somewhere? Not exactly, see documentation: + If transaction has serialization / deadlock failure or them both (last thing + is possible if used script contains several transactions; see +for more information), its + time will be reported as serialization failure / + deadlock failure / + serialization and deadlock failures appropriately. + Example with serialization, deadlock and both these failures: + +1 128 24968 0 1496759158 426984 +0 129 serialization failure 0 1496759158 427023 +3 129 serialization failure 0 1496759158 432662 +2 128 serialization failure 0 1496759158 432765 +0 130 deadlock failure 0 1496759159 460070 +1 129 serialization failure 0 1496759160 485188 +2 129 serialization and deadlock failures 0 1496759160 485339 +4 130 serialization failure 0 1496759160 485465 + I have understood proposals in next messages of this thread that the most interesting case is to retry failed transaction. Do you think it's better to write for example "rolled-back after % retries (serialization failure)' or "time (retried % times, serialization and deadlock failures)'? Advanced options: - mostly for testing built-in scripts: you can set the default transaction isolation level by the appropriate benchmarking option (-I); I'm less convinced of the need of htat, you can already set arbitrary connection options with PGOPTIONS='-c default_transaction_isolation=serializable' pgbench Oh, thanks, I forgot about it =[ P.S. Does this use case (do not retry transaction with serialization or deadlock failure) is most interesting or failed transactions should be retried (and how much times if there seems to be no hope of success...)? I can't quite parse that sentence, could you restate? Álvaro Herrera later in this thread has understood my text right: As far as I understand her proposal, it is exactly the opposite -- if a transaction fails, it is discarded. And this P.S. note is asking whether this is a good idea, or would we prefer that failing transactions are retried. With his explanation has my text become clearer? -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Pgbench Serialization and deadlock errors
Sounds like a good idea. Thank you! Please add to the next CommitFest Done: https://commitfest.postgresql.org/14/1170/ and review somebody else's patch in exchange for having your own patch reviewed. Of course, I remember about it. -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Precalculate stable functions, infrastructure v1
Hi Marina, Hello again! I still don't see anything particularly wrong with your patch. It applies, passes all test, it is well test-covered and even documented. Also I've run `make installcheck` under Valgrind and didn't find any memory-related errors. Thank you very much as usual! Is there anything that you would like to change before we call it more or less final? I would like to add some primitive nodes for precalculation if their behaviour allows to do it and their arguments/inputs are constant or precalculated too; and regression tests for it, of course. Also I would like to add some notes about precalculation of stable functions in documentation, for example, here [1] and here [2]. Also I would advice to add your branch to our internal buildfarm just to make sure everything is OK on exotic platforms like Windows ;) Thanks! Done) [1] https://www.postgresql.org/docs/10/static/xfunc-volatility.html [2] https://www.postgresql.org/docs/10/static/sql-createfunction.html -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Precalculate stable functions, infrastructure v1
Here's v2 of the patches. Changes from v1:
And here there's v3 of planning and execution: common executor steps for
all types of cached expression.
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres CompanyFrom 5e89221251670526eb2b5750868ac73eee48f10b Mon Sep 17 00:00:00 2001
From: Marina Polyakova <m.polyak...@postgrespro.ru>
Date: Mon, 15 May 2017 15:31:21 +0300
Subject: [PATCH 2/3] Precalculate stable functions, planning and execution v3
Now in Postgresql only immutable functions are precalculated; stable functions
are calculated for every row so in fact they don't differ from volatile
functions.
This patch includes:
- replacement nonvolatile functions and operators by appropriate cached
expressions
- planning and execution cached expressions
- regression tests
---
src/backend/executor/execExpr.c| 37 +
src/backend/executor/execExprInterp.c | 37 +
src/backend/optimizer/path/allpaths.c |9 +-
src/backend/optimizer/path/clausesel.c | 13 +
src/backend/optimizer/plan/planagg.c |1 +
src/backend/optimizer/plan/planner.c | 28 +
src/backend/optimizer/util/clauses.c | 55 +
src/backend/utils/adt/ruleutils.c |5 +
src/include/executor/execExpr.h| 19 +
src/include/optimizer/planner.h|3 +
src/include/optimizer/tlist.h |8 +-
src/pl/plpgsql/src/pl_exec.c | 10 +
.../expected/precalculate_stable_functions.out | 2625
src/test/regress/serial_schedule |1 +
.../regress/sql/precalculate_stable_functions.sql | 949 +++
15 files changed, 3797 insertions(+), 3 deletions(-)
create mode 100644 src/test/regress/expected/precalculate_stable_functions.out
create mode 100644 src/test/regress/sql/precalculate_stable_functions.sql
diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c
index 5a34a46..3c2068d 100644
--- a/src/backend/executor/execExpr.c
+++ b/src/backend/executor/execExpr.c
@@ -865,6 +865,43 @@ ExecInitExprRec(Expr *node, PlanState *parent, ExprState *state,
break;
}
+ case T_CachedExpr:
+ {
+int adjust_jump;
+
+/*
+ * Allocate and fill scratch memory used by all steps of
+ * CachedExpr evaluation.
+ */
+scratch.d.cachedexpr.isExecuted = (bool *) palloc(sizeof(bool));
+scratch.d.cachedexpr.resnull = (bool *) palloc(sizeof(bool));
+scratch.d.cachedexpr.resvalue = (Datum *) palloc(sizeof(Datum));
+
+*scratch.d.cachedexpr.isExecuted = false;
+*scratch.d.cachedexpr.resnull = false;
+*scratch.d.cachedexpr.resvalue = (Datum) 0;
+scratch.d.cachedexpr.jumpdone = -1;
+
+/* add EEOP_CACHEDEXPR_IF_CACHED step */
+scratch.opcode = EEOP_CACHEDEXPR_IF_CACHED;
+ExprEvalPushStep(state, );
+adjust_jump = state->steps_len - 1;
+
+/* add subexpression steps */
+ExecInitExprRec((Expr *) get_subexpr((CachedExpr *) node),
+parent, state, resv, resnull);
+
+/* add EEOP_CACHEDEXPR_SUBEXPR_END step */
+scratch.opcode = EEOP_CACHEDEXPR_SUBEXPR_END;
+ExprEvalPushStep(state, );
+
+/* adjust jump target */
+state->steps[adjust_jump].d.cachedexpr.jumpdone =
+ state->steps_len;
+
+break;
+ }
+
case T_ScalarArrayOpExpr:
{
ScalarArrayOpExpr *opexpr = (ScalarArrayOpExpr *) node;
diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c
index fed0052..ac7b7f8 100644
--- a/src/backend/executor/execExprInterp.c
+++ b/src/backend/executor/execExprInterp.c
@@ -279,6 +279,7 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
TupleTableSlot *innerslot;
TupleTableSlot *outerslot;
TupleTableSlot *scanslot;
+ MemoryContext oldContext; /* for EEOP_CACHEDEXPR_* */
/*
* This array has to be in the same order as enum ExprEvalOp.
@@ -309,6 +310,8 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
&_EEOP_FUNCEXPR_STRICT,
&_EEOP_FUNCEXPR_FUSAGE,
&_EEOP_FUNCEXPR_STRICT_FUSAGE,
+ &_EEOP_CACHEDEXPR_IF_CACHED,
+ &_EEOP_CACHEDEXPR_SUBEXPR_END,
&_EEOP_BOOL_AND_STEP_FIRST,
&_EEOP_BOOL_AND_STEP,
&_EEOP_BOOL_AND_STEP_LAST,
@@ -721,6 +724,40 @@ ExecInterpExpr(ExprState *state, ExprContext *econtext, bool *isnull)
EEO_NEXT();
}
+ EEO_CASE(EEOP_CACHEDEXPR_IF_CACHED)
+ {
+ if (*op->d.cachedexpr.isExecuted)
+ {
+/* use saved result and skip subexpression evaluation */
+*op->resnull = *op->d.cachedexpr.resnull;
+if (!(*op->resnull))
+ *op->resvalue = *op->d.cachedexpr.resvalue;
+
+EEO_JUMP(op->d.cachedexpr.jumpdone);
+ }
+
+ /*
+ * Switch per-query memory context for subexpression evaluation.
+ * It is necessary to save result between all tuples.
+ */
+
Re: [HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1
Hello, Aleksander! I've noticed that this patch needs a review and decided to take a look. Thank you very much! There are a trailing whitespaces, though. Oh, sorry, I'll check them. I see 8-10% performance improvement on full text search queries. Glad to hear it =) It seems that there is no obvious performance degradation on regular queries (according to pgbench). Thanks for testing it, I'll try not to forget about it next time =[ In short, it looks very promising. And thanks again! -- Marina Polyakova Postgres Professional: http://www.postgrespro.com The Russian Postgres 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] WIP Patch: Precalculate stable functions, infrastructure v1
and here I send infrastructure patch which includes <...>
Next 2 patches:
Patch 'planning and execution', which includes:
- replacement nonvolatile functions and operators by appropriate cached
expressions;
- planning and execution cached expressions;
- regression tests.
Patch 'costs', which includes cost changes for cached expressions
(according to their behaviour).
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres CompanyFrom cf446cbfc8625701f9e3f32d1870b47de869802a Mon Sep 17 00:00:00 2001
From: Marina Polyakova <m.polyak...@postgrespro.ru>
Date: Thu, 4 May 2017 19:36:05 +0300
Subject: [PATCH 3/3] Precalculate stable functions, costs v1
Now in Postgresql only immutable functions are precalculated; stable functions
are calculated for every row so in fact they don't differ from volatile
functions.
This patch includes:
- cost changes for cached expressions (according to their behaviour)
---
src/backend/optimizer/path/costsize.c | 58 ---
1 file changed, 53 insertions(+), 5 deletions(-)
diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c
index 52643d0..34707fa 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -140,6 +140,7 @@ static MergeScanSelCache *cached_scansel(PlannerInfo *root,
PathKey *pathkey);
static void cost_rescan(PlannerInfo *root, Path *path,
Cost *rescan_startup_cost, Cost *rescan_total_cost);
+static double cost_eval_cacheable_expr_per_tuple(Node *node);
static bool cost_qual_eval_walker(Node *node, cost_qual_eval_context *context);
static void get_restriction_qual_cost(PlannerInfo *root, RelOptInfo *baserel,
ParamPathInfo *param_info,
@@ -3464,6 +3465,44 @@ cost_qual_eval_node(QualCost *cost, Node *qual, PlannerInfo *root)
*cost = context.total;
}
+/*
+ * cost_eval_cacheable_expr_per_tuple
+ * Evaluate per tuple cost for expressions that can be cacheable.
+ *
+ * This function was created to not duplicate code for some expression and
+ * cached some expression.
+ */
+static double
+cost_eval_cacheable_expr_per_tuple(Node *node)
+{
+ double result;
+
+ /*
+ * For each operator or function node in the given tree, we charge the
+ * estimated execution cost given by pg_proc.procost (remember to multiply
+ * this by cpu_operator_cost).
+ */
+ if (IsA(node, FuncExpr))
+ {
+ result = get_func_cost(((FuncExpr *) node)->funcid) * cpu_operator_cost;
+ }
+ else if (IsA(node, OpExpr))
+ {
+ OpExpr *opexpr = (OpExpr *) node;
+
+ /* rely on struct equivalence to treat these all alike */
+ set_opfuncid(opexpr);
+
+ result = get_func_cost(opexpr->opfuncid) * cpu_operator_cost;
+ }
+ else
+ {
+ elog(ERROR, "non cacheable expression node type: %d", (int) nodeTag(node));
+ }
+
+ return result;
+}
+
static bool
cost_qual_eval_walker(Node *node, cost_qual_eval_context *context)
{
@@ -3537,13 +3576,22 @@ cost_qual_eval_walker(Node *node, cost_qual_eval_context *context)
* moreover, since our rowcount estimates for functions tend to be pretty
* phony, the results would also be pretty phony.
*/
- if (IsA(node, FuncExpr))
+ if (IsA(node, FuncExpr) ||
+ IsA(node, OpExpr))
{
- context->total.per_tuple +=
- get_func_cost(((FuncExpr *) node)->funcid) * cpu_operator_cost;
+ context->total.per_tuple += cost_eval_cacheable_expr_per_tuple(node);
+ }
+ else if (IsA(node, CachedExpr))
+ {
+ /*
+ * Calculate subexpression cost per tuple as usual and add it to startup
+ * cost (because subexpression will be executed only once for all
+ * tuples).
+ */
+ context->total.startup += cost_eval_cacheable_expr_per_tuple(
+ get_subexpr((CachedExpr *) node));
}
- else if (IsA(node, OpExpr) ||
- IsA(node, DistinctExpr) ||
+ else if (IsA(node, DistinctExpr) ||
IsA(node, NullIfExpr))
{
/* rely on struct equivalence to treat these all alike */
--
1.9.1
From 508f8b959ff9d1ab78dfc79ab4657b4c10a11690 Mon Sep 17 00:00:00 2001
From: Marina Polyakova <m.polyak...@postgrespro.ru>
Date: Thu, 4 May 2017 19:09:51 +0300
Subject: [PATCH 2/3] Precalculate stable functions, planning and execution v1
Now in Postgresql only immutable functions are precalculated; stable functions
are calculated for every row so in fact they don't differ from volatile
functions.
This patch includes:
- replacement nonvolatile functions and operators by appropriate cached
expressions
- planning and execution cached expressions
- regression tests
---
src/backend/executor/execExpr.c| 70 ++
src/backend/executor/execExprInterp.c | 191 +
src/backend/optimizer/path/allpaths.c | 9 +-
src/backend/optimizer/path/clausesel.c | 13 +
src/backend/optimizer/plan/planagg.c | 1 +
src/backend/optimizer/plan/planner.c | 28 +
src/backend/optimizer/util/clauses.c
[HACKERS] WIP Patch: Precalculate stable functions, infrastructure v1
Hello everyone again!
This is the continuation of my previous patch on the same topic; here
there are changes made thanks to Tom Lane comments (see thread here
[1]). To not send big patch I have split it (that's why version starts
with the first again) and here I send infrastructure patch which
includes:
- creation of CachedExpr node
- usual node functions for it
- mutator to replace nonovolatile functions' and operators' expressions
by appropriate cached expressions.
Any suggestions are welcome!
[1]
https://www.postgresql.org/message-id/flat/98c77534fa51aa4bf84a5b39931c42ea%40postgrespro.ru#98c77534fa51aa4bf84a5b39931c4...@postgrespro.ru
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres CompanyFrom d7871c9aaf64210130b591a93c13b18c74ebb2b4 Mon Sep 17 00:00:00 2001
From: Marina Polyakova <m.polyak...@postgrespro.ru>
Date: Wed, 3 May 2017 18:09:16 +0300
Subject: [PATCH] Precalculate stable functions, infrastructure v1
Now in Postgresql only immutable functions are precalculated; stable functions
are calculated for every row so in fact they don't differ from volatile
functions.
This patch includes:
- creation of CachedExpr node
- usual node functions for it
- mutator to replace nonovolatile functions' and operators' expressions by
appropriate cached expressions.
---
src/backend/nodes/copyfuncs.c| 22 +++
src/backend/nodes/equalfuncs.c | 22 +++
src/backend/nodes/nodeFuncs.c| 121 ++
src/backend/nodes/outfuncs.c | 32 +
src/backend/nodes/readfuncs.c| 33 ++
src/backend/optimizer/plan/planner.c | 124 +++
src/include/nodes/nodeFuncs.h| 2 +
src/include/nodes/nodes.h| 1 +
src/include/nodes/primnodes.h| 32 +
9 files changed, 389 insertions(+)
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 35a237a..1a16e3a 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -1529,6 +1529,25 @@ _copyNullIfExpr(const NullIfExpr *from)
return newnode;
}
+static CachedExpr *
+_copyCachedExpr(const CachedExpr *from)
+{
+ CachedExpr *newnode = makeNode(CachedExpr);
+
+ COPY_SCALAR_FIELD(subexprtype);
+ switch(from->subexprtype)
+ {
+ case CACHED_FUNCEXPR:
+ COPY_NODE_FIELD(subexpr.funcexpr);
+ break;
+ case CACHED_OPEXPR:
+ COPY_NODE_FIELD(subexpr.opexpr);
+ break;
+ }
+
+ return newnode;
+}
+
/*
* _copyScalarArrayOpExpr
*/
@@ -4869,6 +4888,9 @@ copyObjectImpl(const void *from)
case T_NullIfExpr:
retval = _copyNullIfExpr(from);
break;
+ case T_CachedExpr:
+ retval = _copyCachedExpr(from);
+ break;
case T_ScalarArrayOpExpr:
retval = _copyScalarArrayOpExpr(from);
break;
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 21dfbb0..5a0929a 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -384,6 +384,25 @@ _equalNullIfExpr(const NullIfExpr *a, const NullIfExpr *b)
}
static bool
+_equalCachedExpr(const CachedExpr *a, const CachedExpr *b)
+{
+ COMPARE_SCALAR_FIELD(subexprtype);
+
+ /* the same subexprtype for b because we have already compared it */
+ switch(a->subexprtype)
+ {
+ case CACHED_FUNCEXPR:
+ COMPARE_NODE_FIELD(subexpr.funcexpr);
+ break;
+ case CACHED_OPEXPR:
+ COMPARE_NODE_FIELD(subexpr.opexpr);
+ break;
+ }
+
+ return true;
+ }
+
+static bool
_equalScalarArrayOpExpr(const ScalarArrayOpExpr *a, const ScalarArrayOpExpr *b)
{
COMPARE_SCALAR_FIELD(opno);
@@ -3031,6 +3050,9 @@ equal(const void *a, const void *b)
case T_NullIfExpr:
retval = _equalNullIfExpr(a, b);
break;
+ case T_CachedExpr:
+ retval = _equalCachedExpr(a, b);
+ break;
case T_ScalarArrayOpExpr:
retval = _equalScalarArrayOpExpr(a, b);
break;
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 3e8189c..9621511 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -32,6 +32,7 @@ static bool planstate_walk_subplans(List *plans, bool (*walker) (),
void *context);
static bool planstate_walk_members(List *plans, PlanState **planstates,
bool (*walker) (), void *context);
+static const Node *get_const_subexpr(const CachedExpr *cachedexpr);
/*
@@ -92,6 +93,9 @@ exprType(const Node *expr)
case T_NullIfExpr:
type = ((const NullIfExpr *) expr)->opresulttype;
break;
+ case T_CachedExpr:
+ type = exprType(get_const_subexpr((const CachedExpr *) expr));
+ break;
case T_ScalarArrayOpExpr:
type = BOOLOID;
break;
@@ -311,6 +315,8 @@ exprTypmod(const Node *expr)
return exprTypmod((Node *) linitial(nexpr->args));
}
break;
+ case T_CachedExpr:
+ return exprTypmod(get_const_subexpr((const CachedExpr *) expr));
case T_SubLink:
{
const SubLink *sublink = (const SubLink *) expr;
[HACKERS] Fwd: WIP Patch: Precalculate stable functions
Sorry, attached patch.
Исходное сообщение
Тема: WIP Patch: Precalculate stable functions
Дата: 20-04-2017 19:56
От: Marina Polyakova <m.polyak...@postgrespro.ru>
Кому: pgsql-hackers@postgresql.org
Hello everyone!
Now in Postgresql only immutable functions are precalculated; stable
functions are calculated for every row so in fact they don't differ from
volatile functions.
There's a proposal to precalculate stable and immutable functions (=
calculate once for all output rows, but as many times as function is
mentioned in query), if they don't return a set and their arguments are
constants or recursively precalculated functions. The same for
operators' functions, strict functions, tracking functions. It can be
very effective, for example, there's a comparison for full text search
in messages (Intel® Core™ i5-6500 CPU @ 3.20GHz × 4, RAM 8Gb):
Without precalculation:
EXPLAIN (ANALYZE TRUE, BUFFERS TRUE) SELECT COUNT(*) FROM messages WHERE
body_tsvector @@ to_tsquery('postgres');
QUERY
PLAN
--
Aggregate (cost=18714.82..18714.83 rows=1 width=8) (actual
time=2275.334..2275.334 rows=1 loops=1)
Buffers: shared hit=309234 read=184261
-> Bitmap Heap Scan on messages (cost=66.93..18702.34 rows=4991
width=0) (actual time=70.661..224
7.462 rows=151967 loops=1)
Recheck Cond: (body_tsvector @@ to_tsquery('postgres'::text))
Rows Removed by Index Recheck: 118531
Heap Blocks: exact=56726 lossy=33286
Buffers: shared hit=309234 read=184261
-> Bitmap Index Scan on message_body_idx (cost=0.00..65.68
rows=4991 width=0) (actual time=
54.599..54.599 rows=151967 loops=1)
Index Cond: (body_tsvector @@
to_tsquery('postgres'::text))
Buffers: shared hit=1 read=37
Planning time: 0.493 ms
Execution time: 2276.412 ms
(12 rows)
With precalculation:
EXPLAIN (ANALYZE TRUE, BUFFERS TRUE) SELECT COUNT(*) FROM messages WHERE
body_tsvector @@ to_tsquery('postgres');
QUERY
PLAN
--
Aggregate (cost=192269.70..192269.71 rows=1 width=8) (actual
time=1458.679..1458.680 rows=1 loops=1)
Buffers: shared hit=309234 read=184261
-> Bitmap Heap Scan on messages (cost=1445.68..191883.51
rows=154474 width=0) (actual time=70.069
..1433.999 rows=151967 loops=1)
Recheck Cond: (body_tsvector @@ to_tsquery('postgres'::text))
Rows Removed by Index Recheck: 118531
Heap Blocks: exact=56726 lossy=33286
Buffers: shared hit=309234 read=184261
-> Bitmap Index Scan on message_body_idx (cost=0.00..1406.81
rows=154474 width=0) (actual t
ime=56.149..56.149 rows=151967 loops=1)
Index Cond: (body_tsvector @@
to_tsquery('postgres'::text))
Buffers: shared hit=1 read=37
Planning time: 1.644 ms
Execution time: 1459.836 ms
(12 rows)
Patch is attached. It isn't done yet:
- changing documentation (partly because of next lines);
- precalculation of expressions IS DISTINCT FROM and NULLIF which use
nonvolatile equality operators;
- precalculation of expressions "scalar op ANY/ALL (array)" which use
nonvolatile operators;
- precalculation of row compare expressions which use nonvolatile
operators.
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
+7 926 92 00 265From 46d590281129083d524751805797ef0a3c386df0 Mon Sep 17 00:00:00 2001
From: Marina Polyakova <m.polyak...@postgrespro.ru>
Date: Thu, 20 Apr 2017 19:23:05 +0300
Subject: [PATCH 2/2] Precalculate stable functions
Now in Postgresql only immutable functions are precalculated; stable functions
are calculated for every row so in fact they don't differ from volatile
functions.
In this patch function / operator is precalculated if:
1) it doesn't return set,
2) it's not volatile itself,
3) its arguments are constants or nonvolatile too (functions or operators).
Costs are changed to reflect the changed behaviour.
---
src/backend/executor/execExpr.c| 41 ++
src/backend/executor/execExprInterp.c | 196 +-
src/backend/optimizer/path/costsize.c | 84 ++-
src/include/fmgr.h | 4 +
src/include/nodes/primnodes.h | 2 +
.../expected/precalculate_stable_functions.out | 784 +
src/test/regress/serial_schedule | 1 +
.../regress/sql/precalculate_stable_functions.sql | 240 +++
8 files changed, 1321 insertions(+), 31 deletions(-)
create mode 100644 src/test/regress/expected/precalculate_stable_fun
[HACKERS] WIP Patch: Precalculate stable functions
Hello everyone!
Now in Postgresql only immutable functions are precalculated; stable
functions are calculated for every row so in fact they don't differ from
volatile functions.
There's a proposal to precalculate stable and immutable functions (=
calculate once for all output rows, but as many times as function is
mentioned in query), if they don't return a set and their arguments are
constants or recursively precalculated functions. The same for
operators' functions, strict functions, tracking functions. It can be
very effective, for example, there's a comparison for full text search
in messages (Intel® Core™ i5-6500 CPU @ 3.20GHz × 4, RAM 8Gb):
Without precalculation:
EXPLAIN (ANALYZE TRUE, BUFFERS TRUE) SELECT COUNT(*) FROM messages WHERE
body_tsvector @@ to_tsquery('postgres');
QUERY
PLAN
--
Aggregate (cost=18714.82..18714.83 rows=1 width=8) (actual
time=2275.334..2275.334 rows=1 loops=1)
Buffers: shared hit=309234 read=184261
-> Bitmap Heap Scan on messages (cost=66.93..18702.34 rows=4991
width=0) (actual time=70.661..224
7.462 rows=151967 loops=1)
Recheck Cond: (body_tsvector @@ to_tsquery('postgres'::text))
Rows Removed by Index Recheck: 118531
Heap Blocks: exact=56726 lossy=33286
Buffers: shared hit=309234 read=184261
-> Bitmap Index Scan on message_body_idx (cost=0.00..65.68
rows=4991 width=0) (actual time=
54.599..54.599 rows=151967 loops=1)
Index Cond: (body_tsvector @@
to_tsquery('postgres'::text))
Buffers: shared hit=1 read=37
Planning time: 0.493 ms
Execution time: 2276.412 ms
(12 rows)
With precalculation:
EXPLAIN (ANALYZE TRUE, BUFFERS TRUE) SELECT COUNT(*) FROM messages WHERE
body_tsvector @@ to_tsquery('postgres');
QUERY
PLAN
--
Aggregate (cost=192269.70..192269.71 rows=1 width=8) (actual
time=1458.679..1458.680 rows=1 loops=1)
Buffers: shared hit=309234 read=184261
-> Bitmap Heap Scan on messages (cost=1445.68..191883.51
rows=154474 width=0) (actual time=70.069
..1433.999 rows=151967 loops=1)
Recheck Cond: (body_tsvector @@ to_tsquery('postgres'::text))
Rows Removed by Index Recheck: 118531
Heap Blocks: exact=56726 lossy=33286
Buffers: shared hit=309234 read=184261
-> Bitmap Index Scan on message_body_idx (cost=0.00..1406.81
rows=154474 width=0) (actual t
ime=56.149..56.149 rows=151967 loops=1)
Index Cond: (body_tsvector @@
to_tsquery('postgres'::text))
Buffers: shared hit=1 read=37
Planning time: 1.644 ms
Execution time: 1459.836 ms
(12 rows)
Patch is attached. It isn't done yet:
- changing documentation (partly because of next lines);
- precalculation of expressions IS DISTINCT FROM and NULLIF which use
nonvolatile equality operators;
- precalculation of expressions "scalar op ANY/ALL (array)" which use
nonvolatile operators;
- precalculation of row compare expressions which use nonvolatile
operators.
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
+7 926 92 00 265
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
