Re: [PATCHES] [GENERAL] pgbench not setting scale size correctly?
On Wed, 7 May 2008, Bruce Momjian wrote: Tom Lane wrote: Greg Smith [EMAIL PROTECTED] 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... 1 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 [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MDIndex: contrib/pgbench/pgbench.c === RCS file: /home/gsmith/cvsrepo/pgsql/contrib/pgbench/pgbench.c,v retrieving revision 1.79 diff -u -r1.79 pgbench.c --- contrib/pgbench/pgbench.c 19 Mar 2008 03:33:21 - 1.79 +++ contrib/pgbench/pgbench.c 9 May 2008 07:12:21 - @@ -1645,6 +1645,9 @@ exit(0); } + if (scale (ttype != 3)) + fprintf(stderr,Scale setting ignored by standard tests, using database branch count\n); + remains = nclients; if (getVariable(state[0], scale) == NULL) -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [GENERAL] pgbench not setting scale size correctly?
Greg Smith [EMAIL PROTECTED] 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 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [GENERAL] pgbench not setting scale size correctly?
Greg Smith [EMAIL PROTECTED] 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 - 1.79
--- pgbench.c 9 May 2008 15:49:47 -
***
*** 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;
+ boolscale_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);
}
}
}
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make
Re: [PATCHES] [GENERAL] pgbench not setting scale size correctly?
Tom Lane wrote: Greg Smith [EMAIL PROTECTED] 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 [EMAIL PROTECTED]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 - 1.79 --- contrib/pgbench/pgbench.c 7 May 2008 21:36:42 - *** *** 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 -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [GENERAL] pgbench not setting scale size correctly?
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 [EMAIL PROTECTED] http://www.gregsmith.com Baltimore, MD -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
