From 3d9d3d0ce4a998ce00ad30dc678ab93ff7fc2136 Mon Sep 17 00:00:00 2001 From: Hannu Krosing Date: Sun, 4 Oct 2026 11:24:36 +0000 Subject: [PATCH v4 01/12] Refactor pgbench: extract statistics structures and functions to stats.c/h Move the statistics data structures (SimpleStats, pg_time_usec_t, StatsData, EStatus), timing helpers (pg_time_now, pg_time_now_lazy, PG_TIME_GET_DOUBLE), and statistics accumulation/reporting functions (initSimpleStats, addToSimpleStats, mergeSimpleStats, initStats, accumStats, getFailures, getResultString, printSimpleStats) out of pgbench.c into a dedicated stats.c and stats.h module. No function bodies, parameter lists, expressions, or comments are modified in this commit. Why function/variable linkage changes are strictly necessary: 1. Removal of `static` on 8 functions moved to stats.c: In C, `static` functions have internal linkage and are visible only within the translation unit where they are defined. Because these 8 functions are defined in stats.c and called from pgbench.c (and later script.c and engine.c), `static` must be removed and `extern` prototypes declared in stats.h: - initSimpleStats() (called by postprocess_sql_command) - addToSimpleStats() (called by processXactStats) - mergeSimpleStats() (called by printProgressReport and main) - initStats() (called by ParseScript, threadRun, and main) - accumStats() (called by doLog and processXactStats) - getFailures() (called by printProgressReport and printResults) - getResultString() (called by doLog) - printSimpleStats() (called by printResults) 2. Moving `static inline` helpers pg_time_now() and pg_time_now_lazy() from pgbench.c to stats.h (unchanged): Per C99 6.7.4, a `static inline` function definition must be visible in every translation unit that calls it. Placing both functions verbatim in stats.h allows pgbench.c (and subsequent modules) to inline them without altering their qualifiers or bodies. 3. Removal of `static` on global variables `throttle_delay` and `failures_detailed` in pgbench.c: In master, accumStats() reads `throttle_delay` directly and getResultString() reads `failures_detailed` directly. Exporting those two globals via `extern` in stats.h allows accumStats() and getResultString() to be moved to stats.c with zero changes to their parameter lists or function bodies (decoupling them via explicit parameters is deferred to commit 0008). --- src/bin/pgbench/Makefile | 1 + src/bin/pgbench/meson.build | 1 + src/bin/pgbench/pgbench.c | 299 +----------------------------------- src/bin/pgbench/stats.c | 182 ++++++++++++++++++++++ src/bin/pgbench/stats.h | 166 ++++++++++++++++++++ 5 files changed, 353 insertions(+), 296 deletions(-) create mode 100644 src/bin/pgbench/stats.c create mode 100644 src/bin/pgbench/stats.h diff --git a/src/bin/pgbench/Makefile b/src/bin/pgbench/Makefile index 987bf64df9d..839d470e91e 100644 --- a/src/bin/pgbench/Makefile +++ b/src/bin/pgbench/Makefile @@ -11,6 +11,7 @@ OBJS = \ $(WIN32RES) \ exprparse.o \ exprscan.o \ + stats.o \ pgbench.o override CPPFLAGS := -I. -I$(srcdir) -I$(libpq_srcdir) $(CPPFLAGS) diff --git a/src/bin/pgbench/meson.build b/src/bin/pgbench/meson.build index 12e895796c1..4a2b97fa4ac 100644 --- a/src/bin/pgbench/meson.build +++ b/src/bin/pgbench/meson.build @@ -2,6 +2,7 @@ pgbench_sources = files( 'pgbench.c', + 'stats.c', ) exprscan = custom_target('exprscan', diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c index 181f0d892ef..bfc9d71eb46 100644 --- a/src/bin/pgbench/pgbench.c +++ b/src/bin/pgbench/pgbench.c @@ -66,8 +66,8 @@ #include "getopt_long.h" #include "libpq-fe.h" #include "pgbench.h" +#include "stats.h" #include "port/pg_bitutils.h" -#include "portability/instr_time.h" /* X/Open (XSI) requires to provide M_PI, but core POSIX does not */ #ifndef M_PI @@ -201,7 +201,7 @@ static double sample_rate = 0.0; * When threads are throttled to a given rate limit, this is the target delay * to reach that rate in usec. 0 is the default and means no throttling. */ -static double throttle_delay = 0; +double throttle_delay = 0; /* * Transactions which take longer than this limit (in usec) are counted as @@ -290,7 +290,7 @@ static int main_pid; /* main process id used in log filename */ */ static uint32 max_tries = 1; -static bool failures_detailed = false; /* whether to group failures in +bool failures_detailed = false; /* whether to group failures in * reports or logs by basic types */ static const char *pghost = NULL; @@ -349,106 +349,6 @@ typedef struct #define MAX_SCRIPTS 128 /* max number of SQL scripts allowed */ #define SHELL_COMMAND_SIZE 256 /* maximum size allowed for shell command */ -/* - * Simple data structure to keep stats about something. - * - * XXX probably the first value should be kept and used as an offset for - * better numerical stability... - */ -typedef struct SimpleStats -{ - int64 count; /* how many values were encountered */ - double min; /* the minimum seen */ - double max; /* the maximum seen */ - double sum; /* sum of values */ - double sum2; /* sum of squared values */ -} SimpleStats; - -/* - * The instr_time type is expensive when dealing with time arithmetic. Define - * a type to hold microseconds instead. Type int64 is good enough for about - * 584500 years. - */ -typedef int64 pg_time_usec_t; - -/* - * Data structure to hold various statistics: per-thread and per-script stats - * are maintained and merged together. - */ -typedef struct StatsData -{ - pg_time_usec_t start_time; /* interval start time, for aggregates */ - - /*---------- - * Transactions are counted depending on their execution and outcome. - * First a transaction may have started or not: skipped transactions occur - * under --rate and --latency-limit when the client is too late to execute - * them. Secondly, a started transaction may ultimately succeed or fail, - * possibly after some retries when --max-tries is not one. Thus - * - * the number of all transactions = - * 'cnt_skipped' (it was too late to execute them) + - * 'cnt' (the number of successful transactions) + - * 'failed' (the number of failed transactions). - * - * A successful transaction can have several unsuccessful tries before a - * successful run. Thus - * - * 'cnt' (the number of successful transactions) = - * successfully retried transactions (they got a serialization or a - * deadlock error(s), but were - * successfully retried from the very - * beginning) + - * directly successful transactions (they were successfully completed on - * the first try). - * - * 'failed' (the number of failed transactions) = - * 'serialization_failures' (they got a serialization error and were not - * successfully retried) + - * 'deadlock_failures' (they got a deadlock error and were not - * successfully retried) + - * 'other_sql_failures' (they failed on the first try or after retries - * due to a SQL error other than serialization or - * deadlock; they are counted as a failed transaction - * only when --continue-on-error is specified). - * - * If the transaction was retried after a serialization or a deadlock - * error this does not guarantee that this retry was successful. Thus - * - * 'retries' (number of retries) = - * number of retries in all retried transactions = - * number of retries in (successfully retried transactions + - * failed transactions); - * - * 'retried' (number of all retried transactions) = - * successfully retried transactions + - * unsuccessful retried transactions. - *---------- - */ - int64 cnt; /* number of successful transactions, not - * including 'cnt_skipped' */ - int64 cnt_skipped; /* number of transactions skipped under --rate - * and --latency-limit */ - int64 retries; /* number of retries after a serialization or - * a deadlock error in all the transactions */ - int64 retried; /* number of all transactions that were - * retried after a serialization or a deadlock - * error (perhaps the last try was - * unsuccessful) */ - int64 serialization_failures; /* number of transactions that were - * not successfully retried after a - * serialization error */ - int64 deadlock_failures; /* number of transactions that were not - * successfully retried after a deadlock - * error */ - int64 other_sql_failures; /* number of failed transactions for - * reasons other than - * serialization/deadlock failure, which - * is counted if --continue-on-error is - * specified */ - SimpleStats latency; - SimpleStats lag; -} StatsData; /* * For displaying Unix epoch timestamps, as some time functions may have @@ -456,20 +356,6 @@ typedef struct StatsData */ static pg_time_usec_t epoch_shift; -/* - * Error status for errors during script execution. - */ -typedef enum EStatus -{ - ESTATUS_NO_ERROR = 0, - ESTATUS_META_COMMAND_ERROR, - ESTATUS_CONN_ERROR, - - /* SQL errors */ - ESTATUS_SERIALIZATION_ERROR, - ESTATUS_DEADLOCK_ERROR, - ESTATUS_OTHER_SQL_ERROR, -} EStatus; /* * Transaction status at the end of a command. @@ -882,24 +768,6 @@ get_table_relkind(PGconn *con, const char *table) return relkind; } -static inline pg_time_usec_t -pg_time_now(void) -{ - instr_time now; - - INSTR_TIME_SET_CURRENT(now); - - return (pg_time_usec_t) INSTR_TIME_GET_MICROSEC(now); -} - -static inline void -pg_time_now_lazy(pg_time_usec_t *now) -{ - if ((*now) == 0) - (*now) = pg_time_now(); -} - -#define PG_TIME_GET_DOUBLE(t) (0.000001 * (t)) static void usage(void) @@ -1378,117 +1246,6 @@ permute(const int64 val, const int64 isize, const int64 seed) return (int64) v; } -/* - * Initialize the given SimpleStats struct to all zeroes - */ -static void -initSimpleStats(SimpleStats *ss) -{ - memset(ss, 0, sizeof(SimpleStats)); -} - -/* - * Accumulate one value into a SimpleStats struct. - */ -static void -addToSimpleStats(SimpleStats *ss, double val) -{ - if (ss->count == 0 || val < ss->min) - ss->min = val; - if (ss->count == 0 || val > ss->max) - ss->max = val; - ss->count++; - ss->sum += val; - ss->sum2 += val * val; -} - -/* - * Merge two SimpleStats objects - */ -static void -mergeSimpleStats(SimpleStats *acc, SimpleStats *ss) -{ - if (acc->count == 0 || ss->min < acc->min) - acc->min = ss->min; - if (acc->count == 0 || ss->max > acc->max) - acc->max = ss->max; - acc->count += ss->count; - acc->sum += ss->sum; - acc->sum2 += ss->sum2; -} - -/* - * Initialize a StatsData struct to mostly zeroes, with its start time set to - * the given value. - */ -static void -initStats(StatsData *sd, pg_time_usec_t start) -{ - sd->start_time = start; - sd->cnt = 0; - sd->cnt_skipped = 0; - sd->retries = 0; - sd->retried = 0; - sd->serialization_failures = 0; - sd->deadlock_failures = 0; - sd->other_sql_failures = 0; - initSimpleStats(&sd->latency); - initSimpleStats(&sd->lag); -} - -/* - * Accumulate one additional item into the given stats object. - */ -static void -accumStats(StatsData *stats, bool tx_skipped, double lat, double lag, - EStatus estatus, int64 tries) -{ - /* Record the skipped transaction */ - if (tx_skipped) - { - /* no latency to record on skipped transactions */ - stats->cnt_skipped++; - return; - } - - /* - * Record the number of retries regardless of whether the transaction was - * successful or failed. - */ - if (tries > 1) - { - stats->retries += (tries - 1); - stats->retried++; - } - - switch (estatus) - { - /* Record the successful transaction */ - case ESTATUS_NO_ERROR: - stats->cnt++; - - addToSimpleStats(&stats->latency, lat); - - /* and possibly the same for schedule lag */ - if (throttle_delay) - addToSimpleStats(&stats->lag, lag); - break; - - /* Record the failed transaction */ - case ESTATUS_SERIALIZATION_ERROR: - stats->serialization_failures++; - break; - case ESTATUS_DEADLOCK_ERROR: - stats->deadlock_failures++; - break; - case ESTATUS_OTHER_SQL_ERROR: - stats->other_sql_failures++; - break; - default: - /* internal error which should never occur */ - pg_fatal("unexpected error status: %d", estatus); - } -} /* call PQexec() and exit() on failure */ static void @@ -4591,44 +4348,6 @@ executeMetaCommand(CState *st, pg_time_usec_t *now) return CSTATE_END_COMMAND; } -/* - * Return the number of failed transactions. - */ -static int64 -getFailures(const StatsData *stats) -{ - return (stats->serialization_failures + - stats->deadlock_failures + - stats->other_sql_failures); -} - -/* - * Return a string constant representing the result of a transaction - * that is not successfully processed. - */ -static const char * -getResultString(bool tx_skipped, EStatus estatus) -{ - if (tx_skipped) - return "skipped"; - else if (failures_detailed) - { - switch (estatus) - { - case ESTATUS_SERIALIZATION_ERROR: - return "serialization"; - case ESTATUS_DEADLOCK_ERROR: - return "deadlock"; - case ESTATUS_OTHER_SQL_ERROR: - return "other"; - default: - /* internal error which should never occur */ - pg_fatal("unexpected error status: %d", estatus); - } - } - else - return "failed"; -} /* * Print log entry after completing one transaction. @@ -6410,18 +6129,6 @@ printProgressReport(TState *threads, int64 test_start, pg_time_usec_t now, *last_report = now; } -static void -printSimpleStats(const char *prefix, SimpleStats *ss) -{ - if (ss->count > 0) - { - double latency = ss->sum / ss->count; - double stddev = sqrt(ss->sum2 / ss->count - latency * latency); - - printf("%s average = %.3f ms\n", prefix, 0.001 * latency); - printf("%s stddev = %.3f ms\n", prefix, 0.001 * stddev); - } -} /* print version banner */ static void diff --git a/src/bin/pgbench/stats.c b/src/bin/pgbench/stats.c new file mode 100644 index 00000000000..5370be2eb26 --- /dev/null +++ b/src/bin/pgbench/stats.c @@ -0,0 +1,182 @@ +/*------------------------------------------------------------------------- + * + * stats.c + * Statistics functions for pgbench + * + * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/bin/pgbench/stats.c + * + *------------------------------------------------------------------------- + */ +#include "postgres_fe.h" + +#include + +#include "common/logging.h" +#include "stats.h" + +/* + * Initialize the given SimpleStats struct to all zeroes + */ +void +initSimpleStats(SimpleStats *ss) +{ + memset(ss, 0, sizeof(SimpleStats)); +} + +/* + * Accumulate one value into a SimpleStats struct. + */ +void +addToSimpleStats(SimpleStats *ss, double val) +{ + if (ss->count == 0 || val < ss->min) + ss->min = val; + if (ss->count == 0 || val > ss->max) + ss->max = val; + ss->count++; + ss->sum += val; + ss->sum2 += val * val; +} + +/* + * Merge two SimpleStats objects + */ +void +mergeSimpleStats(SimpleStats *acc, SimpleStats *ss) +{ + if (acc->count == 0 || ss->min < acc->min) + acc->min = ss->min; + if (acc->count == 0 || ss->max > acc->max) + acc->max = ss->max; + acc->count += ss->count; + acc->sum += ss->sum; + acc->sum2 += ss->sum2; +} + +/* + * Initialize a StatsData struct to mostly zeroes, with its start time set to + * the given value. + */ +void +initStats(StatsData *sd, pg_time_usec_t start) +{ + sd->start_time = start; + sd->cnt = 0; + sd->cnt_skipped = 0; + sd->retries = 0; + sd->retried = 0; + sd->serialization_failures = 0; + sd->deadlock_failures = 0; + sd->other_sql_failures = 0; + initSimpleStats(&sd->latency); + initSimpleStats(&sd->lag); +} + +/* + * Accumulate one additional item into the given stats object. + */ +void +accumStats(StatsData *stats, bool tx_skipped, double lat, double lag, + EStatus estatus, int64 tries) +{ + /* Record the skipped transaction */ + if (tx_skipped) + { + /* no latency to record on skipped transactions */ + stats->cnt_skipped++; + return; + } + + /* + * Record the number of retries regardless of whether the transaction was + * successful or failed. + */ + if (tries > 1) + { + stats->retries += (tries - 1); + stats->retried++; + } + + switch (estatus) + { + /* Record the successful transaction */ + case ESTATUS_NO_ERROR: + stats->cnt++; + + addToSimpleStats(&stats->latency, lat); + + /* and possibly the same for schedule lag */ + if (throttle_delay) + addToSimpleStats(&stats->lag, lag); + break; + + /* Record the failed transaction */ + case ESTATUS_SERIALIZATION_ERROR: + stats->serialization_failures++; + break; + case ESTATUS_DEADLOCK_ERROR: + stats->deadlock_failures++; + break; + case ESTATUS_OTHER_SQL_ERROR: + stats->other_sql_failures++; + break; + default: + /* internal error which should never occur */ + pg_fatal("unexpected error status: %d", estatus); + } +} + +/* + * Return the number of failed transactions. + */ +int64 +getFailures(const StatsData *stats) +{ + return (stats->serialization_failures + + stats->deadlock_failures + + stats->other_sql_failures); +} + +/* + * Return a string constant representing the result of a transaction + * that is not successfully processed. + */ +const char * +getResultString(bool tx_skipped, EStatus estatus) +{ + if (tx_skipped) + return "skipped"; + else if (failures_detailed) + { + switch (estatus) + { + case ESTATUS_SERIALIZATION_ERROR: + return "serialization"; + case ESTATUS_DEADLOCK_ERROR: + return "deadlock"; + case ESTATUS_OTHER_SQL_ERROR: + return "other"; + default: + /* internal error which should never occur */ + pg_fatal("unexpected error status: %d", estatus); + } + } + else + return "failed"; +} + +void +printSimpleStats(const char *prefix, SimpleStats *ss) +{ + if (ss->count > 0) + { + double latency = ss->sum / ss->count; + double stddev = sqrt(ss->sum2 / ss->count - latency * latency); + + printf("%s average = %.3f ms\n", prefix, 0.001 * latency); + printf("%s stddev = %.3f ms\n", prefix, 0.001 * stddev); + } +} diff --git a/src/bin/pgbench/stats.h b/src/bin/pgbench/stats.h new file mode 100644 index 00000000000..4009bcfd0ab --- /dev/null +++ b/src/bin/pgbench/stats.h @@ -0,0 +1,166 @@ +/*------------------------------------------------------------------------- + * + * stats.h + * Statistics structures and functions for pgbench + * + * Portions Copyright (c) 1996-2026, PostgreSQL Global Development Group + * Portions Copyright (c) 1994, Regents of the University of California + * + * src/bin/pgbench/stats.h + * + *------------------------------------------------------------------------- + */ +#ifndef STATS_H +#define STATS_H + +#include "portability/instr_time.h" + +/* + * Simple data structure to keep stats about something. + * + * XXX probably the first value should be kept and used as an offset for + * better numerical stability... + */ +typedef struct SimpleStats +{ + int64 count; /* how many values were encountered */ + double min; /* the minimum seen */ + double max; /* the maximum seen */ + double sum; /* sum of values */ + double sum2; /* sum of squared values */ +} SimpleStats; + +/* + * The instr_time type is expensive when dealing with time arithmetic. Define + * a type to hold microseconds instead. Type int64 is good enough for about + * 584500 years. + */ +typedef int64 pg_time_usec_t; + +/* + * Data structure to hold various statistics: per-thread and per-script stats + * are maintained and merged together. + */ +typedef struct StatsData +{ + pg_time_usec_t start_time; /* interval start time, for aggregates */ + + /*---------- + * Transactions are counted depending on their execution and outcome. + * First a transaction may have started or not: skipped transactions occur + * under --rate and --latency-limit when the client is too late to execute + * them. Secondly, a started transaction may ultimately succeed or fail, + * possibly after some retries when --max-tries is not one. Thus + * + * the number of all transactions = + * 'cnt_skipped' (it was too late to execute them) + + * 'cnt' (the number of successful transactions) + + * 'failed' (the number of failed transactions). + * + * A successful transaction can have several unsuccessful tries before a + * successful run. Thus + * + * 'cnt' (the number of successful transactions) = + * successfully retried transactions (they got a serialization or a + * deadlock error(s), but were + * successfully retried from the very + * beginning) + + * directly successful transactions (they were successfully completed on + * the first try). + * + * 'failed' (the number of failed transactions) = + * 'serialization_failures' (they got a serialization error and were not + * successfully retried) + + * 'deadlock_failures' (they got a deadlock error and were not + * successfully retried) + + * 'other_sql_failures' (they failed on the first try or after retries + * due to a SQL error other than serialization or + * deadlock; they are counted as a failed transaction + * only when --continue-on-error is specified). + * + * If the transaction was retried after a serialization or a deadlock + * error this does not guarantee that this retry was successful. Thus + * + * 'retries' (number of retries) = + * number of retries in all retried transactions = + * number of retries in (successfully retried transactions + + * failed transactions); + * + * 'retried' (number of all retried transactions) = + * successfully retried transactions + + * unsuccessful retried transactions. + *---------- + */ + int64 cnt; /* number of successful transactions, not + * including 'cnt_skipped' */ + int64 cnt_skipped; /* number of transactions skipped under --rate + * and --latency-limit */ + int64 retries; /* number of retries after a serialization or + * a deadlock error in all the transactions */ + int64 retried; /* number of all transactions that were + * retried after a serialization or a deadlock + * error (perhaps the last try was + * unsuccessful) */ + int64 serialization_failures; /* number of transactions that were + * not successfully retried after a + * serialization error */ + int64 deadlock_failures; /* number of transactions that were not + * successfully retried after a deadlock + * error */ + int64 other_sql_failures; /* number of failed transactions for + * reasons other than + * serialization/deadlock failure, which + * is counted if --continue-on-error is + * specified */ + SimpleStats latency; + SimpleStats lag; +} StatsData; + +/* + * Error status for errors during script execution. + */ +typedef enum EStatus +{ + ESTATUS_NO_ERROR = 0, + ESTATUS_META_COMMAND_ERROR, + ESTATUS_CONN_ERROR, + + /* SQL errors */ + ESTATUS_SERIALIZATION_ERROR, + ESTATUS_DEADLOCK_ERROR, + ESTATUS_OTHER_SQL_ERROR, +} EStatus; + +static inline pg_time_usec_t +pg_time_now(void) +{ + instr_time now; + + INSTR_TIME_SET_CURRENT(now); + + return (pg_time_usec_t) INSTR_TIME_GET_MICROSEC(now); +} + +static inline void +pg_time_now_lazy(pg_time_usec_t *now) +{ + if ((*now) == 0) + (*now) = pg_time_now(); +} + +#define PG_TIME_GET_DOUBLE(t) (0.000001 * (t)) + +extern double throttle_delay; +extern bool failures_detailed; + +extern void initSimpleStats(SimpleStats *ss); +extern void addToSimpleStats(SimpleStats *ss, double val); +extern void mergeSimpleStats(SimpleStats *acc, SimpleStats *ss); +extern void initStats(StatsData *sd, pg_time_usec_t start); +extern void accumStats(StatsData *stats, bool tx_skipped, double lat, double lag, + EStatus estatus, int64 tries); +extern int64 getFailures(const StatsData *stats); +extern const char *getResultString(bool tx_skipped, EStatus estatus); +extern void printSimpleStats(const char *prefix, SimpleStats *ss); + +#endif /* STATS_H */ -- 2.56.0.385.gd3acb90ef8-goog