Thread: Re: [GENERAL] pgbench not setting scale size correctly?
Tom Lane wrote: > Greg Smith <gsmith@gregsmith.com> writes: > > On Fri, 14 Mar 2008, Tom Lane wrote: > >> Yeah, -s is only meaningful when given with -i. Maybe someday we ought > >> to fix pgbench to complain if you try to set it at other times. > > > You have to pass -s in to the actual run if you're specifying your own > > custom script(s) using -f and you want the :scale variable to be defined. > > Right, I knew that at one time ;-) > > > The way the option parsing code is done would make complaining in the case > > where your parameter is ignored a bit of a contortion. The part that > > detects based on the database is after all the other parsing because the > > connection has to be brought up first. > > Yeah. But couldn't we have that part issue a warning if -s had been set > on the command line? Patch attached that issues a warning. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + Index: contrib/pgbench/pgbench.c =================================================================== RCS file: /cvsroot/pgsql/contrib/pgbench/pgbench.c,v retrieving revision 1.79 diff -c -c -r1.79 pgbench.c *** contrib/pgbench/pgbench.c 19 Mar 2008 03:33:21 -0000 1.79 --- contrib/pgbench/pgbench.c 7 May 2008 21:36:42 -0000 *************** *** 1627,1632 **** --- 1627,1635 ---- } } + if (!is_init_mode && scale) + fprintf(stderr, "Scale specification ignored because init mode (-i) not specified\n"); + if (argc > optind) dbName = argv[optind]; else
On Wed, 7 May 2008, Bruce Momjian wrote: > Patch attached that issues a warning. This doesn't take into account the -F case and the warning isn't quite right because of that as well. When I get a break later today I'll create a new patch that handles that correctly, I see where it should go now that I look at this again. -- * Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD
On Wed, 7 May 2008, Bruce Momjian wrote: > Tom Lane wrote: >> Greg Smith <gsmith@gregsmith.com> writes: >>> The way the option parsing code is done would make complaining in the case >>> where your parameter is ignored a bit of a contortion. >> >> Yeah. But couldn't we have that part issue a warning if -s had been set >> on the command line? > > Patch attached that issues a warning. Turns out it wasn't so contorted. Updated patch attached that only warns in the exact cases where the setting is ignored, and the warning says how it's actually setting the scale. I tested all the run types and it correctly complains only when warranted, samples: $ ./pgbench -s 200 -i pgbench creating tables... 10000 tuples done. ... $ ./pgbench -s 100 pgbench Scale setting ignored by standard tests, using database branch count starting vacuum...end. transaction type: TPC-B (sort of) scaling factor: 200 ... $ ./pgbench -s 100 -f select.sql pgbench starting vacuum...end. transaction type: Custom query scaling factor: 100 ... -- * Greg Smith gsmith@gregsmith.com http://www.gregsmith.com Baltimore, MD
Attachment
Greg Smith <gsmith@gregsmith.com> writes:
> Turns out it wasn't so contorted. Updated patch attached that only warns
> in the exact cases where the setting is ignored, and the warning says how
> it's actually setting the scale.
It looks like the code could do with some refactoring. AFAICS
scale is stored into all the :scale variables at lines 1671-1691
(although not if you only have one client !?) only to be done over
again at lines 1746-1763 (though not if ttype != 3). Wasteful,
confusing, and there's a case where it fails to be done at all.
Sigh ...
regards, tom lane
Greg Smith <gsmith@gregsmith.com> writes:
> Turns out it wasn't so contorted. Updated patch attached that only warns
> in the exact cases where the setting is ignored, and the warning says how
> it's actually setting the scale. I tested all the run types and it
> correctly complains only when warranted, samples:
Actually that didn't work, because scale defaults to 1, so it would
*always* warn ... I applied the attached instead.
regards, tom lane
Index: pgbench.c
===================================================================
RCS file: /cvsroot/pgsql/contrib/pgbench/pgbench.c,v
retrieving revision 1.79
diff -c -r1.79 pgbench.c
*** pgbench.c 19 Mar 2008 03:33:21 -0000 1.79
--- pgbench.c 9 May 2008 15:49:47 -0000
***************
*** 1449,1454 ****
--- 1449,1455 ----
int ttype = 0; /* transaction type. 0: TPC-B, 1: SELECT only,
* 2: skip update of branches and tellers */
char *filename = NULL;
+ bool scale_given = false;
CState *state; /* status of clients */
***************
*** 1552,1557 ****
--- 1553,1559 ----
is_connect = 1;
break;
case 's':
+ scale_given = true;
scale = atoi(optarg);
if (scale <= 0)
{
***************
*** 1647,1662 ****
remains = nclients;
- if (getVariable(&state[0], "scale") == NULL)
- {
- snprintf(val, sizeof(val), "%d", scale);
- if (putVariable(&state[0], "scale", val) == false)
- {
- fprintf(stderr, "Couldn't allocate memory for variable\n");
- exit(1);
- }
- }
-
if (nclients > 1)
{
state = (CState *) realloc(state, sizeof(CState) * nclients);
--- 1649,1654 ----
***************
*** 1668,1675 ****
memset(state + 1, 0, sizeof(*state) * (nclients - 1));
! snprintf(val, sizeof(val), "%d", scale);
!
for (i = 1; i < nclients; i++)
{
int j;
--- 1660,1666 ----
memset(state + 1, 0, sizeof(*state) * (nclients - 1));
! /* copy any -D switch values to all clients */
for (i = 1; i < nclients; i++)
{
int j;
***************
*** 1682,1693 ****
exit(1);
}
}
-
- if (putVariable(&state[i], "scale", val) == false)
- {
- fprintf(stderr, "Couldn't allocate memory for variable\n");
- exit(1);
- }
}
}
--- 1673,1678 ----
***************
*** 1743,1764 ****
}
PQclear(res);
! snprintf(val, sizeof(val), "%d", scale);
! if (putVariable(&state[0], "scale", val) == false)
! {
! fprintf(stderr, "Couldn't allocate memory for variable\n");
! exit(1);
! }
! if (nclients > 1)
{
! for (i = 1; i < nclients; i++)
{
! if (putVariable(&state[i], "scale", val) == false)
! {
! fprintf(stderr, "Couldn't allocate memory for variable\n");
! exit(1);
! }
}
}
}
--- 1728,1753 ----
}
PQclear(res);
! /* warn if we override user-given -s switch */
! if (scale_given)
! fprintf(stderr,
! "Scale option ignored, using branches table count = %d\n",
! scale);
! }
! /*
! * :scale variables normally get -s or database scale, but don't override
! * an explicit -D switch
! */
! if (getVariable(&state[0], "scale") == NULL)
! {
! snprintf(val, sizeof(val), "%d", scale);
! for (i = 0; i < nclients; i++)
{
! if (putVariable(&state[i], "scale", val) == false)
{
! fprintf(stderr, "Couldn't allocate memory for variable\n");
! exit(1);
}
}
}