From: Marina Polyakova <m.polyakova@postgrespro.ru>
To: Fabien COELHO <coelho@cri.ensmp.fr>
Cc: pgsql-hackers@postgresql.org, Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Ildus Kurbangaliev <i.kurbangaliev@postgrespro.ru>
Cc: Teodor Sigaev <teodor@sigaev.ru>
Subject: Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
Date: Mon, 13 Aug 2018 17:21:35 +0300
Message-ID: <ef0864e65bdc26dda26484e045366c7f@postgrespro.ru> (raw)
In-Reply-To: <alpine.DEB.2.21.1808121116400.6189@lancre>
References: <20180508193621.dphlgwrd5yz67o2z@alvherre.pgsql>
<0569c5309cf536a839e2a3a1433bd113@postgrespro.ru>
<alpine.DEB.2.21.1807091451520.17811@lancre>
<a6299efa310e1d106144f761259f4fd6@postgrespro.ru>
<alpine.DEB.2.21.1807111435250.27883@lancre>
<976a25a46420c93866ada4051976b1be@postgrespro.ru>
<alpine.DEB.2.21.1807112124210.27883@lancre>
<f153ffd55f481d714ee44afc99ecc67e@postgrespro.ru>
<alpine.DEB.2.21.1807121801250.27883@lancre>
<25d7b1f518c218a8c335bdfc6c858ea1@postgrespro.ru>
<alpine.DEB.2.21.1808121057540.6189@lancre>
<alpine.DEB.2.21.1808121116400.6189@lancre>
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
swap.
> 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.
Ok!
--
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The 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: m.polyakova@postgrespro.ru, coelho@cri.ensmp.fr, alvherre@2ndquadrant.com, i.kurbangaliev@postgrespro.ru, teodor@sigaev.ru
Subject: Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
In-Reply-To: <ef0864e65bdc26dda26484e045366c7f@postgrespro.ru>
* 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