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.1809121519590.13887@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,

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


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: [HACKERS] Cutting initdb's runtime (Perl question embedded)
Next
From: Kyle Samson
Date:
Subject: Consistent segfault in complex query