WIP Patch: Pgbench Serialization and deadlock errors

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
80 messages Options
1234
Reply | Threaded
Open this post in threaded view
|

WIP Patch: Pgbench Serialization and deadlock errors

Marina Polyakova
Hello, hackers!

Now in pgbench we can test only transactions with Read Committed
isolation level because client sessions are disconnected forever on
serialization failures. There were some proposals and discussions about
it (see message here [1] and thread here [2]).

I suggest a patch where pgbench client sessions are not disconnected
because of serialization or deadlock failures and these failures are
mentioned in reports. In details:
- transaction with one of these failures continue run normally, but its
result is rolled back;
- 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);

Advanced options:
- mostly for testing built-in scripts: you can set the default
transaction isolation level by the appropriate benchmarking option (-I);
- for more detailed reports: to know per-statement serialization and
deadlock failures you can use the appropriate benchmarking option
(--report-failures).

Also: TAP tests for new functionality and changed documentation with new
examples.

Patches are attached. Any suggestions are welcome!

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

[1]
https://www.postgresql.org/message-id/4EC65830020000250004323F%40gw.wicourts.gov
[2]
https://www.postgresql.org/message-id/flat/alpine.DEB.2.02.1305182259550.1473%40localhost6.localdomain6#alpine.DEB.2.02.1305182259550.1473@localhost6.localdomain6

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

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

v1-0001-Pgbench-Serialization-and-deadlock-errors.patch (26K) Download Attachment
v1-0002-Pgbench-Set-default-transaction-isolation-level.patch (11K) Download Attachment
v1-0003-Pgbench-Report-per-statement-serialization-and-de.patch (8K) Download Attachment
v1-0004-Pgbench-Fix-documentation.patch (21K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Robert Haas
On Wed, Jun 14, 2017 at 4:48 AM, Marina Polyakova
<[hidden email]> wrote:

> Now in pgbench we can test only transactions with Read Committed isolation
> level because client sessions are disconnected forever on serialization
> failures. There were some proposals and discussions about it (see message
> here [1] and thread here [2]).
>
> I suggest a patch where pgbench client sessions are not disconnected because
> of serialization or deadlock failures and these failures are mentioned in
> reports. In details:
> - transaction with one of these failures continue run normally, but its
> result is rolled back;
> - 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);
>
> Advanced options:
> - mostly for testing built-in scripts: you can set the default transaction
> isolation level by the appropriate benchmarking option (-I);
> - for more detailed reports: to know per-statement serialization and
> deadlock failures you can use the appropriate benchmarking option
> (--report-failures).
>
> Also: TAP tests for new functionality and changed documentation with new
> examples.
>
> Patches are attached. Any suggestions are welcome!

Sounds like a good idea.  Please add to the next CommitFest and review
somebody else's patch in exchange for having your own patch reviewed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Marina Polyakova
> 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 ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Andres Freund
In reply to this post by Marina Polyakova
Hi,

On 2017-06-14 11:48:25 +0300, Marina Polyakova wrote:
> Now in pgbench we can test only transactions with Read Committed isolation
> level because client sessions are disconnected forever on serialization
> failures. There were some proposals and discussions about it (see message
> here [1] and thread here [2]).

> I suggest a patch where pgbench client sessions are not disconnected because
> of serialization or deadlock failures and these failures are mentioned in
> reports.

I think that's a good idea and sorely needed.


In details:


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


> 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


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

- Andres


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Kevin Grittner-7
On Thu, Jun 15, 2017 at 2:16 PM, Andres Freund <[hidden email]> wrote:
> On 2017-06-14 11:48:25 +0300, Marina Polyakova wrote:

>> I suggest a patch where pgbench client sessions are not disconnected because
>> of serialization or deadlock failures and these failures are mentioned in
>> reports.
>
> I think that's a good idea and sorely needed.

+1

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

--
Kevin Grittner
VMware vCenter Server
https://www.vmware.com/


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Alvaro Herrera-9
Kevin Grittner wrote:
> On Thu, Jun 15, 2017 at 2:16 PM, Andres Freund <[hidden email]> wrote:
> > On 2017-06-14 11:48:25 +0300, Marina Polyakova wrote:

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

I think it's pretty obvious that transactions that failed with
some serializability problem should be retried.

--
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Thomas Munro-3
On Fri, Jun 16, 2017 at 9:18 AM, Alvaro Herrera
<[hidden email]> wrote:

> Kevin Grittner wrote:
>> On Thu, Jun 15, 2017 at 2:16 PM, Andres Freund <[hidden email]> wrote:
>> > On 2017-06-14 11:48:25 +0300, Marina Polyakova wrote:
>
>> >> 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.
>
> I think it's pretty obvious that transactions that failed with
> some serializability problem should be retried.

+1 for retry with reporting of retry rates

--
Thomas Munro
http://www.enterprisedb.com


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Kevin Grittner-7
In reply to this post by Alvaro Herrera-9
On Thu, Jun 15, 2017 at 4:18 PM, Alvaro Herrera
<[hidden email]> wrote:
> Kevin Grittner wrote:

> 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.
>
> I think it's pretty obvious that transactions that failed with
> some serializability problem should be retried.

Agreed all around.

--
Kevin Grittner
VMware vCenter Server
https://www.vmware.com/


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Marina Polyakova
In reply to this post by Andres Freund
> 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
+   <xref linkend="transactions-and-scripts"
+   endterm="transactions-and-scripts-title"> for more information), its
+   <replaceable>time</> will be reported as <literal>serialization
failure</> /
+   <literal>deadlock failure</> /
+   <literal>serialization and deadlock failures</> appropriately.

+   Example with serialization, deadlock and both these failures:
+<screen>
+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
+</screen>

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 ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Marina Polyakova
In reply to this post by Kevin Grittner-7
>>> 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 ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Marina Polyakova
In reply to this post by Alvaro Herrera-9
>> >> 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 ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Kevin Grittner-7
In reply to this post by Marina Polyakova
On Fri, Jun 16, 2017 at 5:31 AM, Marina Polyakova
<[hidden email]> wrote:

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

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/SSI

--
Kevin Grittner
VMware vCenter Server
https://www.vmware.com/


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Marina Polyakova
In reply to this post by Marina Polyakova
> 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:


Thank you again! :))

-- 
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Fabien COELHO-3
In reply to this post by Marina Polyakova

Hello Marina,

A few comments about the submitted patches.

I agree that improving the error handling ability of pgbench is a good
thing, although I'm not sure about the implications...

About the "retry" discussion: I agree that retry is the relevant option
from an application point of view.

ISTM that the retry implementation should be implemented somehow in the
automaton, restarting the same script for the beginning.

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.

The number of retries and maybe failures should be counted, maybe with
some adjustable maximum, as suggested.

About 0001:

In accumStats, just use one level if, the two levels bring nothing.

In doLog, added columns should be at the end of the format. 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.

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.

About tests: I do not understand why test 003 includes 2 transactions.
It would seem more logical to have two scripts.

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.

About 0004:

The documentation must not be in a separate patch, but in the same patch
as their corresponding code.

--
Fabien.


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Marina Polyakova
> 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 ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Fabien COELHO-3

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.


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Marina Polyakova
> 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 ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Fabien COELHO-3

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

Personnaly, I like counting retries because it also counts the number of
time the transaction actually failed for some reason. But this is a
marginal preference, and one can be switchted to the other easily.

--
Fabien.


--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Alexander Korotkov
In reply to this post by Andres Freund
On Thu, Jun 15, 2017 at 10:16 PM, Andres Freund <[hidden email]> wrote:
On 2017-06-14 11:48:25 +0300, Marina Polyakova wrote:
> 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

Right, there is already way to specify default isolation level using environment variables.
However, once we make pgbench work with various isolation levels, users may want to run pgbench multiple times in a row with different isolation levels.  Command line option would be very convenient in this case.
In addition, isolation level is vital parameter to interpret benchmark results correctly.  Often, graphs with pgbench results are entitled with pgbench command line.  Having, isolation level specified in command line would naturally fit into this entitling scheme.
Of course, this is solely usability question and it's fair enough to live without such a command line option.  But I'm +1 to add this option.

------
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company 
Reply | Threaded
Open this post in threaded view
|

Re: WIP Patch: Pgbench Serialization and deadlock errors

Marina Polyakova
In reply to this post by Fabien COELHO-3
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 Company

--
Sent via pgsql-hackers mailing list ([hidden email])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

v2-0001-Pgbench-Retry-transactions-with-serialization-or-.patch (110K) Download Attachment
1234
Previous Thread Next Thread