PATCH: pgbench / int64 instead of int for xact count
Hi,
I've been running a few longer pgbench tests (~week), and I've run into
this:
transaction type: SELECT only
scaling factor: 1250
query mode: simple
number of clients: 32
number of threads: 4
duration: 605000 s
number of transactions actually processed: -1785047856
latency average: -10.846 ms
tps = -2950.492090 (including connections establishing)
tps = -2950.492325 (excluding connections establishing)
The instance was doing ~10k tps for a week, which caused overflow of the
int counter, used to track number of transactions. Hence the negative
values.
I think we've reached the time when hardeare capable of doing this is
pretty common (SSDs, ...), so I think it's time to switch the counter to
int64.
Tomas
Attachments:
pgbench-int64.patchtext/x-diff; name=pgbench-int64.patchDownload
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 6cc06d7..f5b3f60 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -239,7 +239,7 @@ typedef struct
typedef struct
{
instr_time conn_time;
- int xacts;
+ int64 xacts;
int64 latencies;
int64 sqlats;
int64 throttle_lag;
@@ -2180,7 +2180,7 @@ process_builtin(char *tb)
/* print out results */
static void
-printResults(int ttype, int normal_xacts, int nclients,
+printResults(int ttype, int64 normal_xacts, int nclients,
TState *threads, int nthreads,
instr_time total_time, instr_time conn_total_time,
int64 total_latencies, int64 total_sqlats,
@@ -2213,13 +2213,13 @@ printResults(int ttype, int normal_xacts, int nclients,
if (duration <= 0)
{
printf("number of transactions per client: %d\n", nxacts);
- printf("number of transactions actually processed: %d/%d\n",
+ printf("number of transactions actually processed: %ld/%d\n",
normal_xacts, nxacts * nclients);
}
else
{
printf("duration: %d s\n", duration);
- printf("number of transactions actually processed: %d\n",
+ printf("number of transactions actually processed: %ld\n",
normal_xacts);
}
@@ -2359,7 +2359,7 @@ main(int argc, char **argv)
instr_time start_time; /* start up time */
instr_time total_time;
instr_time conn_total_time;
- int total_xacts = 0;
+ int64 total_xacts = 0;
int64 total_latencies = 0;
int64 total_sqlats = 0;
int64 throttle_lag = 0;
Hi,
On 2014-05-25 18:05:03 +0200, Tomas Vondra wrote:
I've been running a few longer pgbench tests (~week), and I've run into
this:
number of transactions actually processed: -1785047856
latency average: -10.846 ms
tps = -2950.492090 (including connections establishing)
tps = -2950.492325 (excluding connections establishing)The instance was doing ~10k tps for a week, which caused overflow of the
int counter, used to track number of transactions. Hence the negative
values.
I think we've reached the time when hardeare capable of doing this is
pretty common (SSDs, ...), so I think it's time to switch the counter to
int64.
Especially when it's perfectly possible to do 500k read only
transactions a second...
printf("number of transactions per client: %d\n", nxacts); - printf("number of transactions actually processed: %d/%d\n", + printf("number of transactions actually processed: %ld/%d\n", normal_xacts, nxacts * nclients);
That's not right though. On windows a long (indicated by the %l) is only
4 bytes wide. Check INT64_FORMAT. That's generated by configure/platform
template files and should always be correct.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 25.5.2014 19:05, Andres Freund wrote:
printf("number of transactions per client: %d\n", nxacts); - printf("number of transactions actually processed: %d/%d\n", + printf("number of transactions actually processed: %ld/%d\n", normal_xacts, nxacts * nclients);That's not right though. On windows a long (indicated by the %l) is only
4 bytes wide. Check INT64_FORMAT. That's generated by configure/platform
template files and should always be correct.
Oh, right. v2 of the patch attached.
Tomas
Attachments:
pgbench-int64-v2.patchtext/x-diff; name=pgbench-int64-v2.patchDownload
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 6cc06d7..2f586da 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -239,7 +239,7 @@ typedef struct
typedef struct
{
instr_time conn_time;
- int xacts;
+ int64 xacts;
int64 latencies;
int64 sqlats;
int64 throttle_lag;
@@ -2180,7 +2180,7 @@ process_builtin(char *tb)
/* print out results */
static void
-printResults(int ttype, int normal_xacts, int nclients,
+printResults(int ttype, int64 normal_xacts, int nclients,
TState *threads, int nthreads,
instr_time total_time, instr_time conn_total_time,
int64 total_latencies, int64 total_sqlats,
@@ -2213,13 +2213,13 @@ printResults(int ttype, int normal_xacts, int nclients,
if (duration <= 0)
{
printf("number of transactions per client: %d\n", nxacts);
- printf("number of transactions actually processed: %d/%d\n",
+ printf("number of transactions actually processed: " INT64_FORMAT "/%d\n",
normal_xacts, nxacts * nclients);
}
else
{
printf("duration: %d s\n", duration);
- printf("number of transactions actually processed: %d\n",
+ printf("number of transactions actually processed: " INT64_FORMAT "\n",
normal_xacts);
}
@@ -2359,7 +2359,7 @@ main(int argc, char **argv)
instr_time start_time; /* start up time */
instr_time total_time;
instr_time conn_total_time;
- int total_xacts = 0;
+ int64 total_xacts = 0;
int64 total_latencies = 0;
int64 total_sqlats = 0;
int64 throttle_lag = 0;
Tomas Vondra <tv@fuzzy.cz> writes:
On 25.5.2014 19:05, Andres Freund wrote:
That's not right though. On windows a long (indicated by the %l) is only
4 bytes wide. Check INT64_FORMAT. That's generated by configure/platform
template files and should always be correct.
Oh, right. v2 of the patch attached.
Hm ... if normal_xacts can overflow an int, shouldn't we expect that
the product nxacts * nclients could?
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 25.5.2014 20:32, Tom Lane wrote:
Tomas Vondra <tv@fuzzy.cz> writes:
On 25.5.2014 19:05, Andres Freund wrote:
That's not right though. On windows a long (indicated by the %l) is only
4 bytes wide. Check INT64_FORMAT. That's generated by configure/platform
template files and should always be correct.Oh, right. v2 of the patch attached.
Hm ... if normal_xacts can overflow an int, shouldn't we expect that
the product nxacts * nclients could?
Maybe, but I saw that as a separate thing, mostly unrelated to the
'hardware got so fast it can overflow' problem. Because nxacts is the
parameter we use to define duration, so if it din't overflow in the
past, it won't overflow today.
OTOH, thanks to hardware improvements we tend to use more clients /
higher number of transactions, so fixing this seems like a good idea.
Should we change the type of nxacts to int64 (thus allowing '-t' with
64-bit integers), or just the overflow in the printf call? I don't find
it very practical to use -t with values not fitting into 32-bits (the -T
seems better to do that), so I'm inclined to just fix the printf. Patch
v3 attached.
Tomas
Attachments:
pgbench-int64-v3.patchtext/x-diff; name=pgbench-int64-v3.patchDownload
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 6cc06d7..cf71c73 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -239,7 +239,7 @@ typedef struct
typedef struct
{
instr_time conn_time;
- int xacts;
+ int64 xacts;
int64 latencies;
int64 sqlats;
int64 throttle_lag;
@@ -2180,7 +2180,7 @@ process_builtin(char *tb)
/* print out results */
static void
-printResults(int ttype, int normal_xacts, int nclients,
+printResults(int ttype, int64 normal_xacts, int nclients,
TState *threads, int nthreads,
instr_time total_time, instr_time conn_total_time,
int64 total_latencies, int64 total_sqlats,
@@ -2213,13 +2213,13 @@ printResults(int ttype, int normal_xacts, int nclients,
if (duration <= 0)
{
printf("number of transactions per client: %d\n", nxacts);
- printf("number of transactions actually processed: %d/%d\n",
- normal_xacts, nxacts * nclients);
+ printf("number of transactions actually processed: " INT64_FORMAT "/" INT64_FORMAT "\n",
+ normal_xacts, (int64)nxacts * nclients);
}
else
{
printf("duration: %d s\n", duration);
- printf("number of transactions actually processed: %d\n",
+ printf("number of transactions actually processed: " INT64_FORMAT "\n",
normal_xacts);
}
@@ -2359,7 +2359,7 @@ main(int argc, char **argv)
instr_time start_time; /* start up time */
instr_time total_time;
instr_time conn_total_time;
- int total_xacts = 0;
+ int64 total_xacts = 0;
int64 total_latencies = 0;
int64 total_sqlats = 0;
int64 throttle_lag = 0;
Tomas Vondra <tv@fuzzy.cz> writes:
Should we change the type of nxacts to int64 (thus allowing '-t' with
64-bit integers), or just the overflow in the printf call? I don't find
it very practical to use -t with values not fitting into 32-bits (the -T
seems better to do that), so I'm inclined to just fix the printf.
I concur: I don't see people setting -t higher than INT_MAX, but a -t
times -c product exceeding INT_MAX seems more plausible.
Patch v3 attached.
Looks good, pushed.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers