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

From: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
To: Marina Polyakova <m(dot)polyakova(at)postgrespro(dot)ru>
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-12 09:14:58
Message-ID: alpine.DEB.2.21.1808121057540.6189@lancre
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

HEllo Marina,

> 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,

* typo in comments: "varaibles"

* 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.

ISTM that if something is amiss it will fail in pg_realloc anyway. Also I
do not like the ExpBuf stuff, as usual.

I'm not sure that the size_t cast here and there are useful for any
practical values likely to be encountered by pgbench.

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.


In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Fabien COELHO 2018-08-12 09:17:11 Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
Previous Message Andres Freund 2018-08-12 09:01:13 Re: [HACKERS] Optional message to user when terminating/cancelling backend