RE: pgbench: option delaying queries till connections establishment?

Hello,

> 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
databasesessions.
 
> ```

Indeed. I scanned the file but did not find other places that needed 
updating.

>> -starting vacuum...end.
>
> I think any other options should be disabled in the example, therefore please leave this line.

Yes.

>> +       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.

Not sure either. I'm not for having too many braces anyway, so I removed 
them.

>> +       /* 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.

Indeed. I took your next patch with an added explanation. I'm unclear 
whether proceeding makes much sense though, that is some thread would run 
the test and other would have aborted. Hmmm.

>>> (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.

ISTM that there is another patch in the queue which needs barriers to 
delay some initialization so as to fix a corner case bug, in which case 
the behavior would be mandatory. The current submission could add an 
option to skip the barrier synchronization, but I'm not enthousiastic to 
add an option and remove it shortly later.

Moreover, the "compatibility" is with nothing which does not make much 
sense. When testing with many threads and clients, the current 
implementation make threads start when they are ready, which means that 
you can have threads issuing transactions while others are not yet 
connected or not even started, so that the actually measured performance 
is quite awkward for a short bench. ISTM that allowing such a backward 
compatible strange behavior does not serve pg users. If the user want the 
old unreliable behavior, they can use a old pgbench, and obtain unreliable 
figures.

For these two reasons, I'm inclined not to add an option to skip these 
barriers, but this can be debatted further.

Attached 2 updated patches.

-- 
Fabien.