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
