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

On Wed, 30 Aug 2023 10:11:10 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:

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

Thank you!

Regards,
Yugo Nagata

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


-- 
Yugo NAGATA <nagata@sraoss.co.jp>