Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors

Hello!

Fabien and Alvaro, thank you very much! And sorry for such a late reply 
(I was a bit busy and making of ereport took some time..) :-( Below is a 
rebased version of the patch (commit 
9effb63e0dd12b0704cd8e11106fe08ff5c9d685) divided into several smaller 
patches:

v9-0001-Pgbench-errors-use-the-RandomState-structure-for-.patch
- a patch for the RandomState structure (this is used to reset a 
client's random seed during the repeating of transactions after 
serialization/deadlock failures).

v9-0002-Pgbench-errors-use-the-Variables-structure-for-cl.patch
- a patch for the Variables structure (this is used to reset client 
variables during the repeating of transactions after 
serialization/deadlock failures).

v9-0003-Pgbench-errors-use-the-ereport-macro-to-report-de.patch
- a patch for the ereport() macro (this is used to report client 
failures that do not cause an aborts and this depends on the level of 
debugging).
- implementation: if possible, use the local ErrorData structure during 
the errstart()/errmsg()/errfinish() calls. Otherwise use a static 
variable protected by a mutex if necessary. To do all of this export the 
function appendPQExpBufferVA from libpq.

v9-0004-Pgbench-errors-and-serialization-deadlock-retries.patch
- the main patch for handling client errors and repetition of 
transactions with serialization/deadlock failures (see the detailed 
description in the file).

Any suggestions are welcome!

On 08-05-2018 9:00, Fabien COELHO wrote:
> Hello Marina,
> 
> FYI the v8 patch does not apply anymore, mostly because of a recent
> perl reindentation.
> 
> I think that I'll have time for a round of review in the first half of
> July. Providing a rebased patch before then would be nice.

They are attached, but a little delayed due to testing..

On 08-05-2018 13:58, Alvaro Herrera wrote:
> Looking over the diff, I find that this patch tries to do too much and
> needs to be split up.  At a minimum there is a preliminary patch that
> introduces the error reporting stuff (errstart etc); there are other
> thread-related changes (for example to the random generation functions)
> that probably belong in a separate one too.  Not sure if there are 
> other
> smaller patches hidden inside the rest.

Here is a try to do it..

> On elog/errstart: we already have a convention for what ereport() calls
> look like; I suggest to use that instead of inventing your own.  With
> that, is there a need for elog()?  In the backend we have it because
> $HISTORY but there's no need for that here -- I propose to lose elog()
> and use only ereport everywhere.  Also, I don't see that you need
> errmsg_internal() at all; let's lose it too.

I agree, done. But there're some changes to make such a design 
thread-safe..

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