Re: pgbench stats per script & other stuff

Fabien COELHO <coelho@cri.ensmp.fr>

From: Fabien COELHO <coelho@cri.ensmp.fr>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Michael Paquier <michael.paquier@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2016-02-04T15:53:55Z
Lists: pgsql-hackers

Attachments

Hello Alvaro,

> Something is wrong with patch d.  I noticed two things,
>
> 1. the total_weight stuff can overflow,

It can generate an error on overflow by checking the total_weight while 
it is being computed. I've switched total_weight to int64 so it is now 
really impossible to overflow with the 32 bit int_max limit on weight.

> 2. the chooseScript stuff is broken, or something.

Sorry, probably a <=/< error. I think I've fixed it and I've simplified 
the code a little bit.

> Another thing is that the "transaction type" output really deserves some
> more work.  I think "multiple scripts" really doesn't cut it; we should
> have some YAML-like as in the latency reports, which lists all scripts
> in use and their weights.

For me the current output is clear for the reader, which does not 
mean that it cannot be improve, but how is rather a matter of taste.

I've tried to improve it further, see attached patch.

If you want something else, it would help to provide a sample of what you 
expect.

> Also, while I have your attention regarding accumulated "technical
> debt", please have a look at the "desc" argument used in addScript etc.
> It's pretty ridiculous currently.  Maybe findBuiltin / process_builtin /
> process_file should return a struct containing Command ** and the
> "desc" string, rather than passing desc as a separate argument.

Ok, it can return a pointer to the builtin script.

> Attached is my version of the patch.  While you're messing with it, it'd
> be nice if you added comments on top of your recently added functions
> such as findBuiltin, process_builtin, chooseScript.

Why not.

Find attached a 18-d which addresses these concerns, and a actualized 18-e 
for the prefix.

-- 
Fabien.

Commits

  1. pgbench: Allow changing weights for scripts