Hi,

I would like to improve the following two points on the result outputs
of pgbench related to faild transaction. The patch is attached.

(1) Output per-script statistics even when there are no successful
transaction if there is any failed transactions due to serialization
or deadlock errors.

Previously, per-script statistics were never output when any transactions
are failed. However, it is reasonable to report per-script failed transactions
if they are due to serialization or deadlock errors, since these kinds of
failures are now objects to be reported.
 
This is fixed by modifying the following condition to use "total_cnt <= 0".

       /* Remaining stats are nonsensical if we failed to execute any xacts */
       if (total->cnt + total->skipped <= 0)
               return;

(2) Avoid to print "NaN%" in lines on failed transaction reports.
   
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 that
the number of skipped transactions is not reported in this case.

So, I moved the check of total_cnt mentioned above before reporting the number
of faild transactions. Also, I added a check of script_total_cnt before
reporting per-script number of failed transactions.

Regards,
Yugo Nagata

-- 
Yugo Nagata <nag...@sraoss.co.jp>
>From 000340660f2c567164d7f33cf622f8225a046262 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] pgbench: Improve result outputs related to failed
 transactinos

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.
---
 src/bin/pgbench/pgbench.c | 68 +++++++++++++++++++++------------------
 1 file changed, 37 insertions(+), 31 deletions(-)

diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 61618f2e18..b841bd28b2 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);
@@ -6491,37 +6494,40 @@ printResults(StatsData *total,
 					   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);
-
-				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 failed transactions: " INT64_FORMAT " (%.3f%%)\n",
+						   script_failures,
+						   100.0 * script_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 (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));
+					}
 
-				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);
+					/* 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

Reply via email to