From: Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru>
To: Marina Polyakova <m.polyakova@postgrespro.ru>
Cc: pgsql-hackers@postgresql.org, Teodor Sigaev <teodor@sigaev.ru>
Cc: Fabien COELHO <coelho@cri.ensmp.fr>
Subject: Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
Date: Thu, 5 Apr 2018 18:08:07 +0300
Message-ID: <20180405180807.0bc1114f@wp.localdomain> (raw)
In-Reply-To: <453fa52de88477df2c4a2d82e09e461c@postgrespro.ru>
References: <72a0d590d6ba06f242d75c2e641820ec@postgrespro.ru>
<72ca61a8-b515-1bac-b082-b77952c78b64@sigaev.ru>
<alpine.DEB.2.20.1803251417220.8109@lancre>
<ae993ae02a7578f70b8012b9f2ca1866@postgrespro.ru>
<alpine.DEB.2.20.1803261748030.23514@lancre>
<9c5bf620e050e0803d2a8828a9e9bdb4@postgrespro.ru>
<8ab4a743-321a-1714-86ef-19fee348451d@sigaev.ru>
<alpine.DEB.2.20.1803292134380.16472@lancre>
<fc2d3f13e4c2e4ebe061fb2e26f9f68b@postgrespro.ru>
<453fa52de88477df2c4a2d82e09e461c@postgrespro.ru>
On Wed, 04 Apr 2018 16:07:25 +0300
Marina Polyakova <m.polyakova@postgrespro.ru> wrote:
> Hello, hackers!
>
> Here there's a seventh version of the patch for error handling and
> retrying of transactions with serialization/deadlock failures in
> pgbench (based on the commit
> a08dc711952081d63577fc182fcf955958f70add). I added the option
> --max-tries-time which is an implemetation of Fabien Coelho's
> proposal in [1]: the transaction with serialization or deadlock
> failure can be retried if the total time of all its tries is less
> than this limit (in ms). This option can be combined with the option
> --max-tries. But if none of them are used, failed transactions are
> not retried at all.
>
> Also:
> * Now when the first failure occurs in the transaction it is always
> reported as a failure since only after the remaining commands of this
> transaction are executed we find out whether we can try again or not.
> Therefore add the messages about retrying or ending the failed
> transaction to the "fails" debugging level so you can distinguish
> failures (which are retried) and errors (which are not retried).
> * Fix a report on the latency average because the total time includes
> time for both errors and successful transactions.
> * Code cleanup (including tests).
>
> [1]
> https://www.postgresql.org/message-id/alpine.DEB.2.20.1803292134380.16472%40lancre
>
> > Maybe the max retry should rather be expressed in time rather than
> > number
> > of attempts, or both approach could be implemented?
>
Hi, I did a little review of your patch. It seems to work as
expected, documentation and tests are there. Still I have few comments.
There is a lot of checks like "if (debug_level >= DEBUG_FAILS)" with
corresponding fprintf(stderr..) I think it's time to do it like in the
main code, wrap with some function like log(level, msg).
In CSTATE_RETRY state used_time is used only in printing but calculated
more than needed.
In my opinion Debuglevel should be renamed to DebugLevel that looks
nicer, also there DEBUGLEVEl (where last letter is in lower case) which
is very confusing.
I have checked overall functionality of this patch, but haven't checked
any special cases yet.
--
---
Ildus Kurbangaliev
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: i.kurbangaliev@postgrespro.ru, m.polyakova@postgrespro.ru, teodor@sigaev.ru, coelho@cri.ensmp.fr
Subject: Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
In-Reply-To: <20180405180807.0bc1114f@wp.localdomain>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox