Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
Date
Msg-id alpine.DEB.2.21.1808121057540.6189@lancre
Whole thread Raw
In response to Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors  (Marina Polyakova <m.polyakova@postgrespro.ru>)
Responses Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
List 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.

-- 
Fabien.


pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: [HACKERS] Optional message to user when terminating/cancelling backend
Next
From: Fabien COELHO
Date:
Subject: Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors