Re: WIP Patch: Pgbench Serialization and deadlock errors

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Marina Polyakova <m(dot)polyakova(at)postgrespro(dot)ru>
Cc: PostgreSQL Developers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: WIP Patch: Pgbench Serialization and deadlock errors
Date: 2017-07-03 11:37:30
Message-ID: alpine.DEB.2.20.1707031321370.3419@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


Hello Marina,

>> 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?

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.

>> 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.

>> 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.

>> 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.

>> 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.

--
Fabien.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Sharma 2017-07-03 11:44:34 Re: hash index on unlogged tables doesn't behave as expected
Previous Message Kuntal Ghosh 2017-07-03 11:22:47 Error while copying a large file in pg_rewind