Re: [GENERAL] pgbench not setting scale size correctly?

Hello,
I'm trying to perform some benchmarks using pgbench. I'm using the
following rpm package:

postgresql-contrib-8.3.0-2PGDG.rhel5

for x86_64, downloaded from the pgsql yum repository for centos5/amd64.
When I init the pgbench database, the scale factor seems to work,
however when performing the benchmark I get the following output:

sudo -u postgres pgbench -c 10 -t 1000 -s 1000 pgbench
starting vacuum...end.
transaction type: TPC-B (sort of)
scaling factor: 1
number of clients: 10
number of transactions per client: 1000
number of transactions actually processed: 10000/10000
tps = 2034.588824 (including connections establishing)
tps = 2047.924715 (excluding connections establishing)

as you see, the reported scaling factor is 1, but I specified -s 1000,
which seems strange... I'm going to recompile it from the sources now.
Didn't I get anything or there is a bug somewhere?
Thanks for your help,
Enrico

On Fri, Mar 14, 2008 at 2:34 PM, Enrico Sirola <enrico(dot)sirola(at)gmail(dot)com> wrote:
>
> as you see, the reported scaling factor is 1, but I specified -s 1000,
> which seems strange... I'm going to recompile it from the sources now.
> Didn't I get anything or there is a bug somewhere?

You must have initialized pgbench with scale 1. While running the tests,
it will pick up the scale factor with which it was initialized

Thanks,
Pavan

--
Pavan Deolasee
EnterpriseDB http://www.enterprisedb.com

"Pavan Deolasee" <pavan(dot)deolasee(at)gmail(dot)com> writes:
> On Fri, Mar 14, 2008 at 2:34 PM, Enrico Sirola <enrico(dot)sirola(at)gmail(dot)com> wrote:
>> as you see, the reported scaling factor is 1, but I specified -s 1000,

> You must have initialized pgbench with scale 1.

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.

regards, tom lane

On Fri, 14 Mar 2008, Tom Lane wrote:

> "Pavan Deolasee" <pavan(dot)deolasee(at)gmail(dot)com> writes:
>> You must have initialized pgbench with scale 1.
>
> 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.
But if you're running one of the internal scripts, it just looks in the
database instead, overriding whatever you pass.

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. This is somewhat documented as of
8.3 (I think I submitted that); see the notes on "-s" in
http://www.postgresql.org/docs/current/static/pgbench.html#PGBENCH-RUN-OPTIONS

That could be clearer though, it makes it sound like it's a display only
thing where it's actually quite functional if your custom script uses
scale. I have another pgbench doc patch I'm working on, when I get that
done I'll correct this as well.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD

Greg Smith <gsmith(at)gregsmith(dot)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?

regards, tom lane

Tom Lane wrote:
> Greg Smith <gsmith(at)gregsmith(dot)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?
>
> regards, tom lane
>
>
I was wondering why the -s would not rescale the data?

IMHO a warning would be fine

Justin <justin(at)emproshunts(dot)com> writes:
> I was wondering why the -s would not rescale the data?

That would involve re-initializing the table contents.
If that's what you want, use -i.

regards, tom lane

On Fri, 14 Mar 2008, Justin wrote:

> I was wondering why the -s would not rescale the data?

First, you don't know how to rescale the data if someone is passing in a
custom script. More importantly, people don't expect the benchmark tool
to change things in tables unless specifically requested. I once made the
unfortunate mistake of running 'pgbench -i' against the wrong database,
one that just happened to have a table named 'accounts' in it, only to
watch that table get destroyed. It would be unwise to make that behavior
easier to trigger.

pgbench is a fairly flexible tool that supports creating simple scripted
tests for a variety of purpose. It just so happens that most people only
ever use the default tests where it seems things could be simplified.
They could, but it would take some of the flexibility out as well.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD

Tom Lane wrote:
> Justin <justin(at)emproshunts(dot)com> writes:
>
>> I was wondering why the -s would not rescale the data?
>>
>
> That would involve re-initializing the table contents.
> If that's what you want, use -i.
>
> regards, tom lane
>
>
thanks

Greg Smith wrote:
> On Fri, 14 Mar 2008, Justin wrote:
>
>> I was wondering why the -s would not rescale the data?
>
> First, you don't know how to rescale the data if someone is passing in
> a custom script. More importantly, people don't expect the benchmark
> tool to change things in tables unless specifically requested. I once
> made the unfortunate mistake of running 'pgbench -i' against the wrong
> database, one that just happened to have a table named 'accounts' in
> it, only to watch that table get destroyed. It would be unwise to
> make that behavior easier to trigger.
>
> pgbench is a fairly flexible tool that supports creating simple
> scripted tests for a variety of purpose. It just so happens that most
> people only ever use the default tests where it seems things could be
> simplified. They could, but it would take some of the flexibility out
> as well.
>
> --
> * Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD
>
OK, what if it warns on the -s is set when missing -i and when ( -i
and -f is set in the same command) ???

Tom Lane wrote:
> Greg Smith <gsmith(at)gregsmith(dot)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(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

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(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD

On Wed, 7 May 2008, Bruce Momjian wrote:

> Tom Lane wrote:
>> Greg Smith <gsmith(at)gregsmith(dot)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(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD

Greg Smith <gsmith(at)gregsmith(dot)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(at)gregsmith(dot)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);
}
}
}