On Tue, 24 Sep 2024 19:00:04 +0900 (JST)
Tatsuo Ishii <is...@postgresql.org> wrote:
> > I overlooked the "NaN% of total" in per-script results.
> > I think this NaN also should be avoided.
> >
> > I fixed the number of transactions in per-script results to include
> > skipped and failed transactions. It prevents to print "total of NaN%"
> > when any transactions are not successfully processed.
>
> Thanks for the fix. Here is the new run with the v2 patch. The result
> looks good to me.
>
> src/bin/pgbench/pgbench -p 11002 -c1 -t 1 -f c.sql -f d.sql
> --failures-detailed -r test
> pgbench (18devel)
> starting vacuum...end.
> transaction type: multiple scripts
> scaling factor: 1
> query mode: simple
> number of clients: 1
> number of threads: 1
> maximum number of tries: 1
> number of transactions per client: 1
> number of transactions actually processed: 1/1
> number of failed transactions: 0 (0.000%)
> number of serialization failures: 0 (0.000%)
> number of deadlock failures: 0 (0.000%)
> latency average = 2.434 ms
> initial connection time = 2.117 ms
> tps = 410.846343 (without initial connection time)
> SQL script 1: c.sql
> - weight: 1 (targets 50.0% of total)
> - 1 transactions (100.0% of total)
> - number of transactions actually pocessed: 1 (tps = 410.846343)
> - number of failed transactions: 0 (0.000%)
> - number of serialization failures: 0 (0.000%)
> - number of deadlock failures: 0 (0.000%)
> - latency average = 2.419 ms
> - latency stddev = 0.000 ms
> - statement latencies in milliseconds and failures:
> 0.187 0 begin;
> 0.153 0 set transaction isolation level serializable;
> 0.977 0 insert into t1 select max(i)+1,2 from t1;
> 1.102 0 end;
> SQL script 2: d.sql
> - weight: 1 (targets 50.0% of total)
> - 0 transactions (0.0% of total)
> - statement latencies in milliseconds and failures:
> 0.000 0 begin;
> 0.000 0 set transaction isolation level serializable;
> 0.000 0 insert into t1 select max(i)+1,2 from t1;
> 0.000 0 end;
>
> > Although it breaks the back-compatibility, this seems reasonable
> > modification because not only succeeded transactions but also skips and
> > failures ones are now handled and reported for each script. Also, the
> > number of transactions actually processed per-script and TPS based on
> > it are now output explicitly in a separate line.
>
> Okay for me as long as the patch is pushed to master branch.
>
> A small comment on the comments in the patch: pgindent dislikes some
> of the comment indentation styles. See attached pgindent.txt. Although
> such a small defect would be fixed by committers when a patch gets
> committed anyway, you might want to help committers beforehand.
Thank you for your comments.
I've attached a updated patch that I applied pgindent.
Regards,
Yugo Nagata
> Best reagards,
> --
> Tatsuo Ishii
> SRA OSS K.K.
> English: http://www.sraoss.co.jp/index_en/
> Japanese:http://www.sraoss.co.jp
--
Yugo NAGATA <nag...@sraoss.co.jp>
>From 7ecc6ceb56ed10c1cf77bc83f393dbe3515e517e Mon Sep 17 00:00:00 2001
From: Yugo Nagata <nag...@sraoss.co.jp>
Date: Thu, 19 Sep 2024 01:37:54 +0900
Subject: [PATCH v3] pgbench: Improve result outputs related to failed
transactions
Previously, per-script statistics were never output when all
transactions are failed due to serialization or deadlock errors.
However, it is reasonable to report such information if there
are ones even when there are no successful transaction since
these failed transactions are now objects to be reported.
Meanwhile, if the total number of successful, skipped, and
failed transactions is zero, we don't have to report the number
of failed transactions as similar to the number of skipped
transactions, which avoids to print "NaN%" in lines on failed
transaction reports.
Also, the number of transactions in per-script results now
includes skipped and failed transactions. It prevents to print
"total of NaN%" when any transactions are not successfully
processed. The number of transactions actually processed per-script
and TPS based on it are now output explicitly in a separate line.
---
src/bin/pgbench/pgbench.c | 81 ++++++++++++++++++++++-----------------
1 file changed, 46 insertions(+), 35 deletions(-)
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 61618f2e18..7d0d729e86 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -6391,6 +6391,13 @@ printResults(StatsData *total,
total->cnt);
}
+ /*
+ * Remaining stats are nonsensical if we failed to execute any xacts due
+ * to others than serialization or deadlock errors
+ */
+ if (total_cnt <= 0)
+ return;
+
printf("number of failed transactions: " INT64_FORMAT " (%.3f%%)\n",
failures, 100.0 * failures / total_cnt);
@@ -6412,10 +6419,6 @@ printResults(StatsData *total,
printf("total number of retries: " INT64_FORMAT "\n", total->retries);
}
- /* Remaining stats are nonsensical if we failed to execute any xacts */
- if (total->cnt + total->skipped <= 0)
- return;
-
if (throttle_delay && latency_limit)
printf("number of transactions skipped: " INT64_FORMAT " (%.3f%%)\n",
total->skipped, 100.0 * total->skipped / total_cnt);
@@ -6483,45 +6486,53 @@ printResults(StatsData *total,
printf("SQL script %d: %s\n"
" - weight: %d (targets %.1f%% of total)\n"
- " - " INT64_FORMAT " transactions (%.1f%% of total, tps = %f)\n",
+ " - " INT64_FORMAT " transactions (%.1f%% of total)\n",
i + 1, sql_script[i].desc,
sql_script[i].weight,
100.0 * sql_script[i].weight / total_weight,
- sstats->cnt,
- 100.0 * sstats->cnt / total->cnt,
- sstats->cnt / bench_duration);
-
- printf(" - number of failed transactions: " INT64_FORMAT " (%.3f%%)\n",
- script_failures,
- 100.0 * script_failures / script_total_cnt);
+ script_total_cnt,
+ 100.0 * script_total_cnt / total_cnt);
- if (failures_detailed)
+ if (script_total_cnt > 0)
{
- printf(" - number of serialization failures: " INT64_FORMAT " (%.3f%%)\n",
- sstats->serialization_failures,
- (100.0 * sstats->serialization_failures /
- script_total_cnt));
- printf(" - number of deadlock failures: " INT64_FORMAT " (%.3f%%)\n",
- sstats->deadlock_failures,
- (100.0 * sstats->deadlock_failures /
- script_total_cnt));
- }
+ printf(" - number of transactions actually pocessed: " INT64_FORMAT " (tps = %f)\n",
+ sstats->cnt, sstats->cnt / bench_duration);
- /* it can be non-zero only if max_tries is not equal to one */
- if (max_tries != 1)
- {
- printf(" - number of transactions retried: " INT64_FORMAT " (%.3f%%)\n",
- sstats->retried,
- 100.0 * sstats->retried / script_total_cnt);
- printf(" - total number of retries: " INT64_FORMAT "\n",
- sstats->retries);
- }
+ printf(" - number of failed transactions: " INT64_FORMAT " (%.3f%%)\n",
+ script_failures,
+ 100.0 * script_failures / script_total_cnt);
- if (throttle_delay && latency_limit && script_total_cnt > 0)
- printf(" - number of transactions skipped: " INT64_FORMAT " (%.3f%%)\n",
- sstats->skipped,
- 100.0 * sstats->skipped / script_total_cnt);
+ if (failures_detailed)
+ {
+ printf(" - number of serialization failures: " INT64_FORMAT " (%.3f%%)\n",
+ sstats->serialization_failures,
+ (100.0 * sstats->serialization_failures /
+ script_total_cnt));
+ printf(" - number of deadlock failures: " INT64_FORMAT " (%.3f%%)\n",
+ sstats->deadlock_failures,
+ (100.0 * sstats->deadlock_failures /
+ script_total_cnt));
+ }
+ /*
+ * it can be non-zero only if max_tries is not equal to
+ * one
+ */
+ if (max_tries != 1)
+ {
+ printf(" - number of transactions retried: " INT64_FORMAT " (%.3f%%)\n",
+ sstats->retried,
+ 100.0 * sstats->retried / script_total_cnt);
+ printf(" - total number of retries: " INT64_FORMAT "\n",
+ sstats->retries);
+ }
+
+ if (throttle_delay && latency_limit)
+ printf(" - number of transactions skipped: " INT64_FORMAT " (%.3f%%)\n",
+ sstats->skipped,
+ 100.0 * sstats->skipped / script_total_cnt);
+
+ }
printSimpleStats(" - latency", &sstats->latency);
}
--
2.34.1
