Re: pgbench: allow to exit immediately when any client is aborted

From: Tatsuo Ishii <ishii(at)sraoss(dot)co(dot)jp>
To: nagata(at)sraoss(dot)co(dot)jp
Cc: pgsql-hackers(at)postgresql(dot)org, coelho(at)cri(dot)ensmp(dot)fr
Subject: Re: pgbench: allow to exit immediately when any client is aborted
Date: 2023-08-30 01:11:10
Message-ID: 20230830.101110.349241303555372232.t-ishii@sranhm.sra.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Yugo,
Fabien,

>>> I start to think this behavior is ok and consistent with previous
>>> behavior of pgbench because serialization (and dealock) errors have
>>> been treated specially from other types of errors, such as accessing
>>> non existing tables. However, I suggest to add more sentences to the
>>> explanation of this option so that users are not confused by this.
>>>
>>> + <varlistentry id="pgbench-option-exit-on-abort">
>>> + <term><option>--exit-on-abort</option></term>
>>> + <listitem>
>>> + <para>
>>> + Exit immediately when any client is aborted due to some error. Without
>>> + this option, even when a client is aborted, other clients could continue
>>> + their run as specified by <option>-t</option> or <option>-T</option> option,
>>> + and <application>pgbench</application> will print an incomplete results
>>> + in this case.
>>> + </para>
>>> + </listitem>
>>> + </varlistentry>
>>> +
>>>
>>> What about inserting "Note that serialization failures or deadlock
>>> failures will not abort client. See <xref
>>> linkend="failures-and-retries"/> for more information." into the end
>>> of this paragraph?
>>
>> --exit-on-abort is related to "abort" of a client instead of error or
>> failure itself, so rather I wonder a bit that mentioning serialization/deadlock
>> failures might be confusing. However, if users may think of such failures from
>> "abort", it could be beneficial to add the sentences with some modification as
>> below.
>
> I myself confused by this and believe that adding extra paragraph is
> beneficial to users.
>
>> "Note that serialization failures or deadlock failures does not abort the
>> client, so they are not affected by this option.
>> See <xref linkend="failures-and-retries"/> for more information."
>
> "does not" --> "do not".
>
>>> BTW, I think:
>>> Exit immediately when any client is aborted due to some error. Without
>>>
>>> should be:
>>> Exit immediately when any client is aborted due to some errors. Without
>>>
>>> (error vs. erros)
>>
>> Well, I chose "some" to mean "unknown or unspecified", not "an unspecified amount
>> or number of", so singular form "error" is used.
>
> Ok.
>
>> Instead, should we use "due to a error"?
>
> I don't think so.
>
>>> Also:
>>> + <option>--exit-on-abort</option> is specified . Otherwise in the worst
>>>
>>> There is an extra space between "specified" and ".".
>>
>> Fixed.
>>
>> Also, I fixed the place of the description in the documentation
>> to alphabetical order
>>
>> Attached is the updated patch.
>
> Looks good to me. If there's no objection, I will commit this next week.

I have pushed the patch. Thank you for the conributions!

Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2023-08-30 01:14:08 Re: should frontend tools use syncfs() ?
Previous Message Erik Wienhold 2023-08-30 00:59:14 Re: Restoring default privileges on objects