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

On 12-09-2018 17:04, Fabien COELHO wrote:
> Hello Marina,
> 
>> You can get other errors that cannot happen for only one client if you 
>> use shell commands in meta commands:
> 
>> Or if you use untrusted procedural languages in SQL expressions (see 
>> the used file in the attachments):
> 
>> Or if you try to create a function and perhaps replace an existing 
>> one:
> 
> Sure. Indeed there can be shell errors, perl errors, create functions
> conflicts... I do not understand what is your point wrt these.
> 
> I'm mostly saying that your patch should focus on implementing the
> retry feature when appropriate, and avoid changing the behavior (error
> displayed, abort or not) on features unrelated to serialization &
> deadlock errors.
> 
> Maybe there are inconsistencies, and "bug"/"feature" worth fixing, but
> if so that should be a separate patch, if possible, and if these are
> bugs they could be backpatched.
> 
> For now I'm still convinced that pgbench should keep on aborting on
> "\set" or SQL syntax errors, and show clear error messages on these,
> and your examples have not changed my mind on that point.
> 
>>> I'm fine with renaming the field if it makes thinks clearer. They are
>>> all counters, so naming them "cnt" or "total_cnt" does not help much.
>>> Maybe "succeeded" or "success" to show what is really counted?
>> 
>> Perhaps renaming of StatsData.cnt is better than just adding a comment 
>> to this field. But IMO we have the same problem (They are all 
>> counters, so naming them "cnt" or "total_cnt" does not help much.) for 
>> CState.cnt which cannot be named in the same way because it also 
>> includes skipped and failed transactions.
> 
> Hmmm. CState's cnt seems only used to implement -t anyway? I'm okay if
> it has a different name, esp if it has a different semantics.

Ok!

> I think
> I was arguing only about cnt in StatsData.

The discussion about this has become entangled from the beginning, 
because as I wrote in [1] at first I misread your original proposal...

[1] 
https://www.postgresql.org/message-id/d318cdee8f96de6b1caf2ce684ffe4db%40postgrespro.ru

-- 
Marina Polyakova
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company