Suggested change to pgbench

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Tatsuo Ishii <t-ishii(at)sra(dot)co(dot)jp>
Cc: pgsql-patches(at)postgreSQL(dot)org
Subject: Suggested change to pgbench
Date: 2002-10-06 16:37:27
Message-ID: 10915.1033922247@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

I think pgbench is not dealing with asynchronous input quite right.
As written, if the backend sends a response that doesn't fit into
a single TCP packet, pgbench will go into a tight loop of
PQisBusy/PQconsumeInput, which will not exit until the rest of the
response arrives. This would degrade the reported performance, first
because of the wasted CPU cycles, and second because other connections
wouldn't get serviced during that interval.

I am not sure how likely this is to happen, but I can report that
correcting it via the attached patch made a visible difference:

[tgl(at)rh1 pgbench]$ ./pgbench -c 100 -t 100 bench1
starting vacuum...end.
transaction type: TPC-B (sort of)
scaling factor: 100
number of clients: 100
number of transactions per client: 100
number of transactions actually processed: 10000/10000
tps = 34.748295 (including connections establishing)
tps = 34.803249 (excluding connections establishing)

[tgl(at)rh1 pgbench]$ ./mybench -c 100 -t 100 bench1 -- patched
starting vacuum...end.
transaction type: TPC-B (sort of)
scaling factor: 100
number of clients: 100
number of transactions per client: 100
number of transactions actually processed: 10000/10000
tps = 36.849122 (including connections establishing)
tps = 36.909993 (excluding connections establishing)

I haven't done enough tests to be sure that that isn't just a chance
variation, but I recommend changing the code as below anyway.

regards, tom lane

*** pgbench.c~ Sun Oct 6 12:01:44 2002
--- pgbench.c Sun Oct 6 12:11:02 2002
***************
*** 184,200 ****
{ /* are we receiver? */
if (debug)
fprintf(stderr, "client %d receiving\n", n);
! while (PQisBusy(st->con) == TRUE)
! {
! if (!PQconsumeInput(st->con))
! { /* there's something wrong */
! fprintf(stderr, "Client %d aborted in state %d. Probably the backend died while processing.\n", n, st->state);
! remains--; /* I've aborted */
! PQfinish(st->con);
! st->con = NULL;
! return;
! }
}

switch (st->state)
{
--- 184,199 ----
{ /* are we receiver? */
if (debug)
fprintf(stderr, "client %d receiving\n", n);
! if (!PQconsumeInput(st->con))
! { /* there's something wrong */
! fprintf(stderr, "Client %d aborted in state %d. Probably the backend died while processing.\n", n, st->state);
! remains--; /* I've aborted */
! PQfinish(st->con);
! st->con = NULL;
! return;
}
+ if (PQisBusy(st->con))
+ return; /* don't have the whole result yet */

switch (st->state)
{
***************
*** 367,383 ****
{ /* are we receiver? */
if (debug)
fprintf(stderr, "client %d receiving\n", n);
! while (PQisBusy(st->con) == TRUE)
! {
! if (!PQconsumeInput(st->con))
! { /* there's something wrong */
! fprintf(stderr, "Client %d aborted in state %d. Probably the backend died while processing.\n", n, st->state);
! remains--; /* I've aborted */
! PQfinish(st->con);
! st->con = NULL;
! return;
! }
}

switch (st->state)
{
--- 366,381 ----
{ /* are we receiver? */
if (debug)
fprintf(stderr, "client %d receiving\n", n);
! if (!PQconsumeInput(st->con))
! { /* there's something wrong */
! fprintf(stderr, "Client %d aborted in state %d. Probably the backend died while processing.\n", n, st->state);
! remains--; /* I've aborted */
! PQfinish(st->con);
! st->con = NULL;
! return;
}
+ if (PQisBusy(st->con))
+ return; /* don't have the whole result yet */

switch (st->state)
{

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Aaron Mulder 2002-10-07 01:26:02 DBMD Patch
Previous Message Bruce Momjian 2002-10-06 04:36:07 Allow SET to not start a transaction