Re: [PATCH] pgbench - refactor some connection finish/null into common function

[Rady, Doug]

On 1/30/18, 03:41, "Fabien COELHO"  wrote:
Hello Doug,
Hi Fabien,

> This patch refactors all of the connection state PQfinish() and NULL’ing 
into a single function.
> Excludes PQfinish() in doConnect().

My 0.02€:

The argument could be "PGconn **" instead of a "CState *"?
If so, it may be used in a few more places. What is your opinion?

I should have named  finishCon()  as  finishCStateCon()  since it was specific 
to that use pattern.
I'll resubmit with that change if you think it helps.

I'm fine with this kind of factorization which takes out a three-line 
pattern, but I'm wondering whether it would please committers.

Guess we'll find out ...

-- 
Fabien.
Thanks!
doug

Re: PATCH: pgbench - break out timing data for initialization phases

[Rady, Doug]

On 1/29/18, 23:52, "Fabien COELHO"  wrote:

Hello Doug,
Hi Fabien,

> With patch and ‘-I dtgvpf’ options:
> pgrun pgbench -i -s 2000 -F 90 -q -I dtgvpf
> dropping old tables...
> creating tables...
> generating data...
> …
> 2 of 2 tuples (100%) done (elapsed 168.76 s, remaining 
0.00 s)
> vacuuming...
> creating primary keys...
> creating foreign keys...
> total time: 353.52 s (drop 1.67 s, tables 0.11 s, insert 168.82 s, commit 
0.46 s, primary 92.32 s, foreign 40.11 s, vacuum 50.03 s)
> done.

I'm in favor of such a feature.

However, I think that the durations should be shown in the order in which 
the initialization is performed.

Agreed.

I would suggest to:

- move the time measure in the initialization loop, instead of doing it
  in each function, so that it is done just in one place.

I will do this.

- maybe store the actions in some array/list data structure, eg:
   "{ char * phase; double duration; }", so that they can be kept
   in order and eventually repeated.

In order to extract the commit time, I'd say that explicit begin and 
commit should be separate instructions triggerred by '(' and ')'.

Also, I'm not sure of the one line display, maybe it could be done while 

The one line display was for ease of parsing it from the output flow.

it is in progress, i.e. something like:
   dropping table...
   table drop: 1.67 s
   creating table...
   table creation: 0.11 s
   ...
In which case there is no need for storing the actions and their 
durations, only the running total is needed.

Doing the "in progress" way suffers from everything before 'generating data' 
possibly scrolling off the screen/window.
For me, it is much handier to look at one set of duration times all reported 
together after all of the initialize phases are done.

-- 
Fabien.

Thanks!
doug