On 17-08-2018 10:49, Fabien COELHO wrote:
Hello Marina,
Detailed -r report. I understand from the doc that the retry number
on the detailed per-statement report is to identify at what point
errors occur? Probably this is more or less always at the same point
on a given script, so that the most interesting feature is to report
the number of retries at the script level.
This may depend on various factors.. for example:
[...]
21.239 5 36 UPDATE xy3 SET y = y + :delta
WHERE x = :x3;
21.360 5 39 UPDATE xy2 SET y = y + :delta
WHERE x = :x2;
Ok, not always the same point, and you confirm that it identifies
where the error is raised which leads to a retry.
Yes, I confirm this. I'll try to write more clearly about this in the
documentation...
So we can write something like this:
All the transactions are divided into several types depending on their
execution. Firstly, they can be divided into transactions that we
started to execute, and transactions which were skipped (it was too
late to execute them). Secondly, running transactions fall into 2 main
types: is there any command that got a failure during the last
execution of the transaction script or not? Thus
Here is an attempt at having a more precise and shorter version, not
sure it is much better than yours, though:
"""
Transactions are counted depending on their execution and outcome.
First
a transaction may have started or not: skipped transactions occur
under --rate and --latency-limit when the client is too late to
execute them. Secondly, a started transaction may ultimately succeed
or fail on some error, possibly after some retries when --max-tries is
not one. Thus
"""
Thank you!
I'd suggest to put "ANOTHER_SQL_FAILURE" as the last option,
otherwise "another" does not make sense yet.
Maybe firstly put a general group, and then special cases?...
I understand it more as a catch all default "none of the above" case.
Ok!
commandFailed: I'm not thrilled by the added boolean, which is
partially
redundant with the second argument.
Do you mean that it is partially redundant with the argument "cmd"
and, for example, the meta commands errors always do not cause the
abortions of the client?
Yes. And also I'm not sure we should want this boolean at all.
Perhaps we can use a separate function to print the messages about
client's abortion, something like this (it is assumed that all abortions
happen when processing SQL commands):
static void
clientAborted(CState *st, const char *message)
{
pgbench_error(...,
"client %d aborted in command %d (SQL) of script
%d; %s\n",
st->id, st->command, st->use_file, message);
}
Or perhaps we can use a more detailed failure status so for each type of
failure we always know the command name (argument "cmd") and whether the
client is aborted. Something like this (but in comparison with the first
variant ISTM overly complicated):
/*
* For the failures during script execution.
*/
typedef enum FailureStatus
{
NO_FAILURE = 0,
/*
* Failures in meta commands. In these cases the failed transaction is
* terminated.
*/
META_SET_FAILURE,
META_SETSHELL_FAILURE,
META_SHELL_FAILURE,
META_SLEEP_FAILURE,
META_IF_FAILURE,
META_ELIF_FAILURE,
/*
* Failures in SQL commands. In cases of serialization/deadlock
failures a
* failed transaction is re-executed from the very beginning if
possible;
* otherwise the failed transaction is terminated.
*/
SERIALIZATION_FAILURE,
DEADLOCK_FAILURE,
OTHER_SQL_FAILURE, /* other failures in SQL
commands that are not
* listed by
themselves above */
/*
* Failures while processing SQL commands. In this case the client is
* aborted.
*/
SQL_CONNECTION_FAILURE
} FailureStatus;
[...]
If in such cases one command is placed on several lines, ISTM that the
code is more understandable if curly brackets are used...
Hmmm. Such basic style changes are avoided because they break
backpatching, so we try to avoid gratuitous changes unless there is a
strong added value, which does not seem to be the case here.
Ok!
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
