Suggested change to pgbench - Mailing list 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@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@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)
          {