On Thu, 24 Aug 2023 09:15:51 +0900 (JST)
Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
> >> 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.
Ok.
> > "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".
Oops. I attached the updated patch.
> >> 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.
>
> 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>