Re: pgbench bug / limitation

On Tue, 2 Jun 2020 at 12:21, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> > Shouldn't: if (fd < 0 || fd >= FD_SETSIZE) just become: if (idx > FD_SETSIZE) ?
>
> Certainly not, because it's the fd not the idx that is being added
> into the fd_set. I am not too sure about the underlying implementation
> on Windows, but on Unix-like OSes, FD_SETSIZE *is* the size of that bit
> array. What you suggest would allow memory stomps.

Looks like I didn't dig deep enough. The struct for the fs_set is defined as:

typedef struct fd_set {
u_int fd_count; /* how many are SET? */
SOCKET fd_array[FD_SETSIZE]; /* an array of SOCKETs */
} fd_set;

So, of course, we need to ensure we don't index beyond whatever FD_SET
is set to.

The implementation of FD_SET that I see in WinSock2.h is:

#define FD_SET(fd, set) do { \
u_int __i; \
for (__i = 0; __i < ((fd_set FAR *)(set))->fd_count; __i++) { \
if (((fd_set FAR *)(set))->fd_array[__i] == (fd)) { \
break; \
} \
} \
if (__i == ((fd_set FAR *)(set))->fd_count) { \
if (((fd_set FAR *)(set))->fd_count < FD_SETSIZE) { \
((fd_set FAR *)(set))->fd_array[__i] = (fd); \
((fd_set FAR *)(set))->fd_count++; \
} \
} \
} while(0, 0)

Which is not so great. I guess the POSIX description of using the
lowest available fd is due it expecting implementations to use a
bitmask, rather than an array, like the above, of which would be more
suited for any arbitrary large integer.

> Given your results, I'm guessing that we are indeed managing to increase
> the fd_set size to 1024, but that's not enough to allow order-of-1000
> connections because there are other things competing for FD identifiers.
> Maybe we should just crank up the forced value of FD_SETSIZE (to, say,
> 8192)?

On Windows, I tried building pgbench with FD_SETSIZE set to 128. On
running I get:

starting vacuum...end.
add_socket_to_set fd = 372
pgbench: fatal: too many client connections for select()

So it does appear that the return value of socket() is not limited by
whatever FD_SETSIZE is set to. I guess that means making it larger
would reduce the chances of this error occurring. However, I was
unable to find any indication of the range of values that we can
expect socket() to return on Windows. Going by the fd_set struct I
pasted above, we certainly can't make FD_SETSIZE too large.

David