Re: pgbench bug / limitation

Hello Tom,

My 0.02 €, which might not convince you:

>> ISTM that the patch your propose, a full reimplementation of the abstract
>> poll/select fd set interface for windows, is overkill because it adds
>> about 80 lines of code. ISTM that just redefining some macros is enough to
>> get the same performance benefit without adding much to the code base, but
>> maybe I'm missing something. At least, I do not think that getting rid of
>> the FD_SETSIZE redefinition for windows is worth all that trouble.
>
> I think it's worth doing because it puts the Windows implementation at
> full feature parity with Unixen, ie, there's no hard-wired limit on the
> number of connections.

That is a point, although I do not find it desirable: In most pgbench 
settings ISTM that one thread should handle a few clients, which means 
waiting at most for a few handles, so that performance are not limited by 
pgbench itself.

> I'm also not that thrilled with trying to make one code path work with 
> two fundamentally different implementations.

Yep, but with the same interface.

> We've found out that just because Microsoft claims their implementation
> is compatible with POSIX doesn't make it so,

Sure!

> and I'd rather not have warts in our source code from trying to paper 
> over that.

Hmmm.

> Lastly, I think it's entirely likely that someone will come up with a
> Windows-native implementation at some point, so that we'd have the extra
> code lines anyway.

When reading WaitForMultipleObjects doc, the suggestion seems to create 
more threads to do the waiting, by chunks of 64, and then you wait for 
these threads, possibly recursively. In pgbench context, it would make 
much more sense to advise the user to create more threads for benchmarking 
from the beginning, instead of having one thread for benchmarking and 
possibly creating more just to wait for events, so I cannot see any 
windows implementation that would make sense and would be worth having, at 
least based on this interface.

So it seems to me that we would be better off with a minimal patch to work 
around the FD_SET check, not bother with the FD_SET overhead, not fiddle 
with FD_SETSIZE at all (which given the underlying n² handling seems like 
a bad idea anyway), and advise the user to create more threads when the 
limit is reached, as the attached version does.

I cannot foresee a setting where over 60 clients per thread (ISTM there is 
at most one wait per client) would make much sense, so let's not help the 
user run ineffective benchmarks. Maybe we should even issue a warning when 
#client/#thread is high. Even if there is a bench where a thread could 
really handle 1000 clients (implying 1000 pg processes server side), 
probably the user could suffer to run that bench with 16 threads.

-- 
Fabien.