Re: [BUGS] Problem in using pgbench's --connect(-C) and --rate=rate(-R rate) options together.
Repost from bugs.
--
Fabien.
---------- Forwarded message ----------
Date: Wed, 25 Jan 2017 18:59:45 +0100 (CET)
From: Fabien COELHO <coelho@cri.ensmp.fr>
To: nuko yokohama <nuko.yokohama@gmail.com>
Cc: PostgreSQL Bugs List <pgsql-bugs@postgresql.org>
Subject: Re: [BUGS] Problem in using pgbench's --connect(-C) and --rate=rate(-R
rate) options together.
It operates normally when only the -C option or only the -R option is
specified.In the PostgreSQL document, It is not described that "these two options can
not be specified at the same time ". Is this a problem of pgbench?Yes, indeed there is. Thanks for the report. Option -C is seldom used and
tested.
The problem is already fixed in head. Looking at git log, it was unclear to
guess which change fixed that... After another reading, I got it in one, it has
been fixed by Heikki restructuring patch
12788ae49e1933f463bc59a6efe46c4a01701b76 which has no vocation to be
backpatched to prior versions...
The bug is that prior to --rate doCustom was always disconnect/reconnect
without exiting, but with rate it returns if it has to wait. However threadRun
test whether there is a connection before recalling doCustom, so it was never
called.
This is exactly the kind of unmanageable state combination that refactoring has
cleaned up.
Attached a small patch which fixes the issue, I think, in 9.6.
Fixing it raised another issue wrt to some stats under -C, that I fixed as
well.
--
Fabien.
Attachments:
pgbench-CR-bug-1.patchtext/x-diff; charset=us-ascii; name=pgbench-CR-bug-1.patchDownload
diff --git a/src/bin/pgbench/pgbench.c b/src/bin/pgbench/pgbench.c
index 531671a..1f1b7bf 100644
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -1967,7 +1967,6 @@ top:
st->listen = false;
st->sleeping = false;
st->throttling = false;
- st->is_throttled = false;
memset(st->prepared, 0, sizeof(st->prepared));
}
@@ -4345,6 +4344,12 @@ threadRun(void *arg)
remains--; /* I've aborted */
}
}
+ else if (is_connect && st->sleeping)
+ {
+ /* it is sleeping for throttling, maybe it is done, let us try */
+ if (!doCustom(thread, st, &aggs))
+ remains--;
+ }
if (st->ecnt > prev_ecnt && commands[st->state]->type == META_COMMAND)
{
On 1/25/17 2:58 PM, Fabien COELHO wrote:
Repost from bugs.
This patch does not apply at cccbdde:
$ patch -p1 < ../other/pgbench-CR-bug-1.patch
(Stripping trailing CRs from patch.)
patching file src/bin/pgbench/pgbench.c
Hunk #1 FAILED at 1967.
Hunk #2 succeeded at 4180 with fuzz 2 (offset -164 lines).
Marked as "Waiting for Author".
--
-David
david@pgmasters.net
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello David,
Repost from bugs.
This patch does not apply at cccbdde:
Indeed. It should not. The fix is for the 9.6 branch. The issue has been
fixed by some heavy but very welcome restructuring in master.
Marked as "Waiting for Author".
I put it back to "Needs review".
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 3/17/17 2:08 AM, Fabien COELHO wrote:
Hello David,
Repost from bugs.
This patch does not apply at cccbdde:
Indeed. It should not. The fix is for the 9.6 branch. The issue has been
fixed by some heavy but very welcome restructuring in master.
Whoops, sorry about that!
Marked as "Waiting for Author".
I put it back to "Needs review".
Beena, do you know when you'll have time to review?
Thanks,
--
-David
david@pgmasters.net
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hi, the patch looks good except why do you remove initialization of is_throttled?
Suppose, just a typo?
--- a/src/bin/pgbench/pgbench.c
+++ b/src/bin/pgbench/pgbench.c
@@ -1967,7 +1967,6 @@ top:
st->listen = false;
st->sleeping = false;
st->throttling = false;
- st->is_throttled = false;
memset(st->prepared, 0, sizeof(st->prepared));
}--
Teodor Sigaev E-mail: teodor@sigaev.ru
WWW: http://www.sigaev.ru/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Hello Teodor,
Hi, the patch looks good except why do you remove initialization of
is_throttled? Suppose, just a typo?
No, it is really needed so that the lag measure is correct.
Without the is_throttled change:
sh> ./pgbench -T 3 -R 10 -C -S -P 1
starting vacuum...end.
progress: 1.0 s, 9.0 tps, lat 8.278 ms stddev 9.134, lag 2049638230412146.250 ms
progress: 2.0 s, 12.0 tps, lat 4.897 ms stddev 3.961, lag 2.219 ms
transaction type: <builtin: select only>
scaling factor: 1
query mode: simple
number of clients: 1
number of threads: 1
duration: 3 s
number of transactions actually processed: 33
latency average = 5.602 ms
latency stddev = 5.820 ms
rate limit schedule lag: avg 558992244657859.500 (max 18446744073709264.000) ms
tps = 11.024559 (including connections establishing)
tps = 12.183456 (excluding connections establishing)
With the is_throttled change:
./pgbench -T 3 -R 10 -C -S -P 1
starting vacuum...end.
progress: 1.0 s, 11.0 tps, lat 3.742 ms stddev 2.161, lag 1.658 ms
progress: 2.0 s, 7.0 tps, lat 2.985 ms stddev 0.496, lag 0.276 ms
transaction type: <builtin: select only>
scaling factor: 1
query mode: simple
number of clients: 1
number of threads: 1
duration: 3 s
number of transactions actually processed: 25
latency average = 3.312 ms
latency stddev = 1.518 ms
rate limit schedule lag: avg 0.894 (max 7.031) ms
tps = 8.398353 (including connections establishing)
tps = 9.069456 (excluding connections establishing)
--
Fabien.
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
No, it is really needed so that the lag measure is correct.
Thank you, pushed
--
Teodor Sigaev E-mail: teodor@sigaev.ru
WWW: http://www.sigaev.ru/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers