Re: Suggested change to pgbench

From: Justin Clift <justin(at)postgresql(dot)org>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Tatsuo Ishii <t-ishii(at)sra(dot)co(dot)jp>, pgsql-patches(at)postgreSQL(dot)org
Subject: Re: Suggested change to pgbench
Date: 2002-10-07 10:21:23
Message-ID: 3DA16023.22EF90F5@postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches

Hi Tom,

Have applied this change to the pg_autotune code, and am running tests
against a remote database with that.

Seems a *lot* more consistent results now.

None of the wide swings in variation, etc.

This will help in the case of pg_autotune quite a lot!

:-)

Regards and best wishes,

Justin Clift

Tom Lane wrote:
>
> 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)
> {
>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: subscribe and unsubscribe commands go to majordomo(at)postgresql(dot)org

--
"My grandfather once told me that there are two kinds of people: those
who work and those who take the credit. He told me to try to be in the
first group; there was less competition there."
- Indira Gandhi

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2002-10-07 14:13:10 Re: Suggested change to pgbench
Previous Message Tatsuo Ishii 2002-10-07 05:04:25 Re: Suggested change to pgbench