Re: WIP Patch: Pgbench Serialization and deadlock errors

On 12-01-2018 15:47, Fabien COELHO wrote:
> Hello Marina,
>
> A partial answer, to focus on how the feature may be simplified.

Thank you!

> I apologise as it seems that I overlooked one of your mail. Changing
> the thread has not helped.
>
>>> I'm wondering whether the whole feature could be simplified by
>>> considering that one script is one "transaction" (it is from the
>>> report point of view at least), and that any retry is for the full
>>> script only, from its beginning. That would remove the trying to
>>> guess
>>> at transactions begin or end, avoid scanning manually for
>>> subcommands,
>>> and so on.
>>> - Would it make sense?
>>> - Would it be ok for your use case?
>>
>> I'm not sure if this makes sense: if we retry the script from the very
>> beginning including successful transactions, there may be errors..
>
> What I'm suggesting is to simplify the problem by adding some
> restrictions to what kind of case which is handled, so as to simplify
> the code a lot.
>
> I'd start by stating (i.e. documenting) that the features assumes that
> one script is just *one* transaction.
>
> Note that pgbench somehow already assumes that one script is one
> transaction when it reports performance anyway.
>
> If you want 2 transactions, then you have to put them in two scripts,
> which looks fine with me. Different transactions are expected to be
> independent, otherwise they should be merged into one transaction.

Therefore if the script consists of several single statements (= several
transactions), you cannot retry them. For example, the script looked
like this:

UPDATE xy1 SET x = 1 WHERE y = 1;
UPDATE xy2 SET x = 2 WHERE y = 2;
UPDATE xy3 SET x = 3 WHERE y = 3;

If this restriction is ok for you, I'll simplify the code :)

> Under these restrictions, ISTM that a retry is something like:
>
> case ABORTED:
> if (we want to retry) {
> // do necessary stats
> // reset the initial state (random, vars, current command)
> state = START_TX; // loop
> }
> else {
> // count as failed...
> state = FINISHED; // or done.
> }
> break;

If we successfully complete a failed transaction block and process its
end command in CSTATE_END_COMMAND, we may want to make a retry. So do
you think that in this case it is ok to go to CSTATE_ABORTED at the end
of CSTATE_END_COMMAND?..

> Once this works, maybe it could go a step further by restarting at
> savepoints. I'd put restrictions there to ease detecting a savepoint
> so that it cannot occur in a compound command for instance. This would
> probably require a new state. Fine.

We discussed the savepoints earlier in [1]:

>> - 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..
>
> Well, I do not know that. My 0.02 € is that if there was a savepoint
> then
> this is natural the restarting point of a transaction which has some
> recoverable error.

>
> A detail:
>
>>> For file contents, maybe the << 'EOF' here-document syntax would help
>>> instead of using concatenated backslashed strings everywhere.
>>
>> Thanks, but I'm not sure that it is better to open file handlers for
>> printing in variables..
>
> Perl here-document stuff is just a multiline string, no file is
> involved, it is different from the shell.

Oh, now googling was successful, thanks)

[1]
https://www.postgresql.org/message-id/alpine.DEB.2.20.1707141637300.7871%40lancre

--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company