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

From: Marina Polyakova <m(dot)polyakova(at)postgrespro(dot)ru>
To: Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>
Cc: pgsql-hackers(at)postgresql(dot)org, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Ildus Kurbangaliev <i(dot)kurbangaliev(at)postgrespro(dot)ru>, Teodor Sigaev <teodor(at)sigaev(dot)ru>
Subject: Re: [HACKERS] WIP Patch: Pgbench Serialization and deadlock errors
Date: 2018-09-12 15:12:29
Message-ID: 0c59e59d5c665a5fbcec4d27ba57ffb7@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2018-09-12 15:23:48 Re: executor relation handling
Previous Message Andrew Gierth 2018-09-12 14:55:47 Re: Consistent segfault in complex query