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

From: Marina Polyakova <m(dot)polyakova(at)postgrespro(dot)ru>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: pgsql-hackers(at)postgresql(dot)org, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>, Teodor Sigaev <teodor(at)sigaev(dot)ru>
Subject: Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
Date: 2018-08-13 14:21:35
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 12-08-2018 12:14, Fabien COELHO wrote:
> HEllo Marina,

Hello, Fabien!

>> v10-0003-Pgbench-errors-use-the-Variables-structure-for-c.patch
>> - a patch for the Variables structure (this is used to reset client
>> variables during the repeating of transactions after
>> serialization/deadlock failures).
> This patch adds an explicit structure to manage Variables, which is
> useful to reset these on pgbench script retries, which is the purpose
> of the whole patch series.
> About part 3:
> Patch applies cleanly,

On 12-08-2018 12:17, Fabien COELHO wrote:
>> About part 3:
>> Patch applies cleanly,
> I forgot: compiles, global & local "make check" are ok.

I'm glad to hear it :-)

> * typo in comments: "varaibles"

I'm sorry, I'll fix it.

> * About enlargeVariables:
> multiple INT_MAX error handling looks strange, especially as this code
> can never be triggered because pgbench would be dead long before
> having allocated INT_MAX variables. So I would not bother to add such
> checks.
> ...
> I'm not sure that the size_t cast here and there are useful for any
> practical values likely to be encountered by pgbench.

Looking at the code of the functions, for example, ParseScript and
psql_scan_setup, where the integer variable is used for the size of the
entire script - ISTM that you are right.. Therefore size_t casts will
also be removed.

> ISTM that if something is amiss it will fail in pg_realloc anyway.

IIUC and physical RAM is not enough, this may depend on the size of the

> Also I do not like the ExpBuf stuff, as usual.

> The exponential allocation seems overkill. I'd simply add a constant
> number of slots, with a simple rule:
> /* reallocated with a margin */
> if (max_vars < needed) max_vars = needed + 8;
> So in the end the function should be much simpler.


Marina Polyakova
Postgres Professional:
The Russian Postgres Company

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2018-08-13 14:39:55 Re: libpq should not look up all host addresses at once
Previous Message Etsuro Fujita 2018-08-13 12:32:59 Re: Expression errors with "FOR UPDATE" and postgres_fdw with partition wise join enabled.