On Wed, 30 Aug 2023 10:11:10 +0900 (JST) Tatsuo Ishii <is...@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 <nag...@sraoss.co.jp>
