Dear Fabien, Andres
I think your idea is good, hence I put some comments as a reviewer.
I focus on only the linux code because I'm not familiar with the Windows
system. Sorry.
[For patch A]
Please complete fixes for the documentation. At least the following sentence
should be fixed:
```
The last two lines report the number of transactions per second, figured with
and without counting the time to start database sessions.
```
> -starting vacuum...end.
I think any other options should be disabled in the example, therefore please
leave this line.
> + /* explicitly initialize the state machines */
> + for (int i = 0; i < nstate; i++)
> + {
> + state[i].state = CSTATE_CHOOSE_SCRIPT;
> + }
I'm not sure but I think braces should be removed in our coding conventions.
Other changes are being reviewed now.
[For patch B]
> + /* GO */
> + pthread_barrier_wait(&barrier);
The current implementation is too simple. If nthreads >= 2 and connection fails
in the one thread,
the other one will wait forever.
Some special treatments are needed in the `done` code block and here.
[others]
> > (that is, it can be disabled)
>
> On reflection, I'm not sure I see a use case for not running the barrier
> if it is available.
If the start point changes and there is no way to disable this feature,
the backward compatibility will be completely violated.
It means that tps cannot be compared to older versions under the same
conditions,
and It may hide performance-related issues.
I think it's not good.
Best regards,
Hayato Kuroda
FUJITSU LIMITED
-----Original Message-----
From: Fabien COELHO <coe...@cri.ensmp.fr>
Sent: Saturday, July 4, 2020 3:34 PM
To: Daniel Gustafsson <dan...@yesql.se>
Cc: Andres Freund <and...@anarazel.de>; pgsql-hack...@postgresql.org
Subject: Re: pgbench: option delaying queries till connections establishment?
>> * First patch reworks time measurements in pgbench.
>> * Second patch adds a barrier before starting the bench
>>
>> It applies on top of the previous one. The initial imbalance due to
>> thread creation times is smoothed.
>
> The usecs patch fails to apply to HEAD, can you please submit a rebased
> version
> of this patchset. Also, when doing so, can you please rename the patches such
> that sort alphabetically in the order in which they are intended to be
> applied.
> The CFBot patch tester will otherwise try to apply them out of order which
> cause errors.
Ok. Attached.
--
Fabien.
