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

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. I think I was
arguing only about cnt in StatsData.

--
Fabien.