Date: Thu, 13 Jul 2017 18:32:53 +0200 (CEST)
From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Marina Polyakova <m.polyakova@postgrespro.ru>
cc: PostgreSQL Developers <pgsql-hackers@postgresql.org>,
        Kevin Grittner <kgrittn@gmail.com>,
 Andres Freund <andres@anarazel.de>,
        Alvaro Herrera <alvherre@2ndquadrant.com>,
        Thomas Munro <thomas.munro@enterprisedb.com>,
        Robert Haas <robertmhaas@gmail.com>
Subject: Re: WIP Patch: Pgbench Serialization and deadlock errors
In-Reply-To: <6ce8613f061001105654673506348c13@postgrespro.ru>
Message-ID: <alpine.DEB.2.20.1707131818200.20175@lancre>
References: <72a0d590d6ba06f242d75c2e641820ec@postgrespro.ru>
 <alpine.DEB.2.20.1707011607370.6840@lancre>
 <6b4a7fb36c9956595a89a08e5368a9c5@postgrespro.ru>
 <alpine.DEB.2.20.1707031321370.3419@lancre>
 <01df6ca86c78e2c80b4e4d021c99d53a@postgrespro.ru>
 <alpine.DEB.2.20.1707031430380.15247@lancre>
 <f69d534dfada1fdb4767496b7ffc007f@postgrespro.ru>
 <alpine.DEB.2.20.1707121142300.12795@lancre>
 <6ce8613f061001105654673506348c13@postgrespro.ru>
User-Agent: Alpine 2.20 (DEB 67 2015-01-07)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII; format=flowed
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org


Hello,

> [...] I didn't make rollbacks to savepoints after the failure because 
> they cannot help for serialization failures at all: after rollback to 
> savepoint a new attempt will be always unsuccessful.

Not necessarily? It depends on where the locks triggering the issue are 
set, if they are all set after the savepoint it could work on a second 
attempt.

>> "SimpleStats attempts": I disagree with using this floating poiunt 
>> oriented structures to count integers. I would suggest "int64 tries" 
>> instead, which should be enough for the purpose.
>
> I'm not sure that it is enough. Firstly it may be several transactions in 
> script so to count the average attempts number you should know the total 
> number of runned transactions. Secondly I think that stddev for attempts 
> number can be quite interesting and often it is not close to zero.

I would prefer to have a real motivation to add this complexity in the 
report and in the code. Without that, a simple int seems better for now. 
It can be improved later if the need really arises.

>> Some variables, such as "int attempt_number", should be in the client
>> structure, not in the client? Generally, try to use block variables if
>> possible to keep the state clearly disjoints. If there could be NO new
>> variable at the doCustom level that would be great, because that would
>> ensure that there is no machine state mixup hidden in these variables.
>
> Do you mean the code cleanup for doCustom function? Because if I do so there 
> will be two code styles for state blocks and their variables in this 
> function..

I think that any variable shared between state is a recipee for bugs if it 
is not reset properly, so they should be avoided. Maybe there are already 
too many of them, then too bad, not a reason to add more. The status 
before the automaton was a nightmare.

>> I wondering whether the RETRY & FAILURE states could/should be merged:
>
> I divided these states because if there's a failed transaction block you 
> should end it before retrying.

Hmmm. Maybe I'm wrong. I'll think about it.

>> I would suggest a more compact one-line report about failures:
>>
>>   "number of failures: 12 (0.001%, deadlock: 7, serialization: 5)"
>
> I think, there may be a misunderstanding. Because script can contain several 
> transactions and get both failures.

I do not understand. Both failures number are on the compact line I 
suggested.

-- 
Fabien.


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