Re: fd patch to squid

ons 2006-07-19 klockan 15:13 +0200 skrev Martin Stransky:
> Hello guys,
>
> As you probably know, we have one RH specific patch to squid.

Yes, found this out by accident some time ago..

> It's for
> dynamic configuration of filedescriptors number, via. squid.conf. I
> reviewed it for the 2.6 branch, fixed some bugs (which comes with squid
> reconfiguration) so I think it's stable enough now.

A quick review of the patch indicates it's a bit overcomplicated

And I'm not that convinced you really need it either.. A simple
directive limiting Squid_MaxFD and a high limit specified to configure
would do nearly as good.

The parts eleminating SQUID_MAXFD constant dependencies is good.

Note: The patch does not account for comm_select_win32.c, causing
problems for the win32 plaform..

The parts messing with comm_select.c doesn't feel warranted. Systems
using select() is generally not good at handling more than FD_SETSIZE
connections anyway, and the select interface as such crumbles a bit when
used for many concurrent connections. Also not that keen on using a
custom type for the fd_set given to select(), even if we already make
very strict assumptions about fd_set being a bit array.. (but I have
heard of some systems using an array of ints...)

The bit array stuff doesn't feel needed, as it's only used by the
changes to comm_select.c.

Renaming Squid_MaxFD to SQUID_NUMFD does not feel right. Our naming
convention uses all caps for preprocessor constants only, and even with
the patch it's still the max limit this Squid can handle after startup..
(same as Squid_MaxFD).

What I'd like to see for doing this proper is

* The stuff eleminating SQUID_MAXFD dependencies in arrays etc.

* The directive for setting the fd limit

* Checks in comm_select_init or similar to lower Squid_MaxFD if it's
above what the select loop implementation can handle. This means
comm_select.c and comm_select_win32.c limiting Squid_MaxFD to
FD_SETSIZE. (eleminating the similar check from main.c). Some thinking
needed to get rid of comm_select dependencies.. (probably a new
comm_select_fdlimit call needed before comm_init)

* Moving the FD_SETSIZE override from squid.h to comm_select.c before
include of squid.h..

* After this SQUID_MAXFD should only be referenced from the FD_SETSIZE
override in comm_select.c.

Regards
Henrik