> On Jul 10, 2026, at 16:36, Xuneng Zhou <[email protected]> wrote: > > Hi Evan, > > On Mon, Jun 22, 2026 at 4:11 PM Chao Li <[email protected]> wrote: >> >> >> >>> On Jun 18, 2026, at 20:45, Chao Li <[email protected]> wrote: >>> >>> >>> >>>> On Jun 18, 2026, at 17:40, Fujii Masao <[email protected]> wrote: >>>> >>>> On Thu, Jun 18, 2026 at 3:25 PM Chao Li <[email protected]> wrote: >>>>> Thanks for confirming the doc issue. The here comes a small doc patch. >>>>> >>>>> I also noticed there is a later paragraph about “latency” that also needs >>>>> an update, it’s included in the patch as well. >>>> >>>> Thanks for the patch! The TPS-related changes look good to me. >>>> >>>> The latency of a successful transaction includes the entire time of >>>> - transaction execution with rollbacks and retries. The latency is >>>> measured >>>> - only for successful transactions and commands but not for failed >>>> transactions >>>> - or commands. >>>> + transaction execution with rollbacks and retries. The latency is >>>> measured >>>> + only for successful transactions and commands, except that failed >>>> + transactions are included in the simple average latency when the >>>> + <option>--continue-on-error</option> option is used. >>>> >>>> It doesn't seem that the behavior of the latency average in the main >>>> report depends on --continue-on-error itself. Instead, when neither >>>> --progress, --rate, nor --latency-limit is specified, the main >>>> report shows a simple latency average computed as the total benchmark >>>> duration divided by the number of successful and failed transactions, >>>> so failed transactions are included. >>>> >>>> On the other hand, when any of those options is specified, the >>>> latency average and latency stddev in the main report are based >>>> only on successful transactions. Likewise, the per-script and >>>> per-command latency statistics are measured only for successful >>>> transactions and commands. >>>> >>>> So how about something like this instead? >>>> >>>> ------------------------------- >>>> <para> >>>> The latency of a successful transaction includes the entire time of >>>> - transaction execution with rollbacks and retries. The simple >>>> - <literal>latency average</literal> shown in the main report is >>>> computed from >>>> - the total benchmark duration divided by both successful and failed >>>> - transactions, and therefore includes failed transactions. In contrast, >>>> - detailed latency statistics, including the per-script and per-command >>>> - reports, are measured only for successful transactions and commands, >>>> not >>>> - for failed transactions or commands. >>>> + transaction execution with rollbacks and retries. In the main report, >>>> when >>>> + neither <option>--progress</option>, <option>--rate</option>, nor >>>> + <option>--latency-limit</option> is specified, the >>>> + <literal>latency average</literal> is computed from the total benchmark >>>> + duration divided by both successful and failed transactions, and >>>> therefore >>>> + includes failed transactions. Otherwise, the <literal>latency >>>> average</literal> >>>> + and <literal>latency stddev</literal> shown in the main report are >>>> measured >>>> + only for successful transactions. Detailed latency statistics, >>>> including >>>> + the per-script and per-command reports, are also measured only for >>>> + successful transactions and commands, not for failed transactions or >>>> + commands. >>>> </para> >>>> ------------------------------- >>>> >>> >>> I feel that might be too detailed. Listing these three option names here >>> could make the doc harder to read and maintain. When failures are included, >>> the output line already says so explicitly: >>> ``` >>> latency average = 0.387 ms (including failures) >>> ``` >>> >>> Can we simply say something like this? >>> ``` >>> <para> >>> The latency of a successful transaction includes the entire time of >>> transaction execution with rollbacks and retries. The latency is measured >>> only for successful transactions and commands, not for failed transactions >>> or commands, unless the report explicitly says that it includes failures. >>> </para> >>> ``` >>> >>> Best regards, >>> -- >>> Chao Li (Evan) >>> HighGo Software Co., Ltd. >>> https://www.highgo.com/ >> >> Thinking it over, I came up with a version that combines Fujii-san’s >> suggestion and my previous version: >> ``` >> <para> >> The latency of a successful transaction includes the entire time of >> transaction execution with rollbacks and retries. In the main report, >> when none of <option>--progress</option>, <option>--rate</option>, and >> <option>--latency-limit</option> is specified, <literal>latency >> average</literal> >> is computed from both successful and failed transactions. In such cases, >> this is explicitly indicated by <literal>(including failures)</literal>. >> Other latency statistics, including the per-script and per-command reports, >> are measured only for successful transactions and commands, not for failed >> transactions or commands. >> </para> >> ``` > > <para> > This option is useful when your custom script may raise errors > such as unique constraint violations, but you want the benchmark > - to continue and measure performance including those failures. > + to continue despite individual statement failures. Failed > + transactions are reported separately, but are not counted as > + transactions actually processed, nor are they included in TPS. > > "Nor are they included in TPS" seems misleading to me. Failed > transactions do not enter the TPS accumulator, but their execution > time remains in bench_duration, so they can reduce the overall TPS. > > tps = total->cnt / bench_duration; >
Yes, makes sense. I enhanced this paragraph in v3. Also, after thinking about it more, I integrated Fujii-san’s previous suggestion into v3 as well. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
v3-0001-doc-Clarify-pgbench-reporting-with-continue-on-er.patch
Description: Binary data
