Fix missing condition for max_filedescriptors use

[yadij]

The max_filedescriptors directive is only useful when the OS
setrlimit() API is available. Add markup to the squid.conf
definition that enables appropriate errors and documentation
automatically.

[kinkie]

Nice! But would it be even nicer if the logic were moved to some header and here it used only a derived value?

[yadij]

If we all agree I can do that and the RLIMIT_OFILE hack in compat/compat_shared.h where the related SQUID_MAXFD_LIMIT is defined.

[kinkie]

works for me

[rousskov]

PR description: The max_filedescriptors directive is only useful when the OS
setrlimit() API is available

I strongly disagree with that statement and, hence, this PR direction. If we continue to improve how Squid determines the maximum number of descriptors, max_filedescriptors should continue to be available in all Squid builds because it represents (or should represent) a fundamental limit/setting that is applicable to all builds, regardless of setrlimit() API availability.

[yadij]

PR description: The max_filedescriptors directive is only useful when the OS
setrlimit() API is available

I strongly disagree with that statement and, hence, this PR direction.
If ...should ... should ...

Document what reality is, not what aspirations one has.

@rousskov: you "disagree" with this Squid code line:

squid/src/tools.cc

Line 757 in dd5ea20

#if HAVE_SETRLIMIT && defined(RLIMIT_NOFILE)

... all the logic using this directive for meaningful action is disabled (not even built). The directive is literally only useful when the OS setrlimit() API is available to the built binary.

[rousskov]

PR description: The max_filedescriptors directive is only useful when the OS
setrlimit() API is available

I strongly disagree with that statement and, hence, this PR direction. If we continue to improve how Squid determines the maximum number of descriptors, max_filedescriptors should continue to be available in all Squid builds because it represents (or should represent) a fundamental limit/setting that is applicable to all builds, regardless of setrlimit() API availability.

Document what reality is, not what aspirations one has.

In many cases, documenting both is better, especially when it helps to clarify why one would want to move in the opposite direction of one's aspirations. Do you agree that we should aspire to honor max_filedescriptors directive when setrlimit() is unavailable?

@rousskov: you "disagree" with this Squid code line:

squid/src/tools.cc

Line 757 in dd5ea20

#if HAVE_SETRLIMIT && defined(RLIMIT_NOFILE)

Yes, that code is essentially a bug. I "disagree" that we should adjust existing max_filedescriptors directive metadata to match existing problematic code. The correct adjustment should go in the opposite direction (i.e. we should fix code rather than break metadata).

... all the logic using this directive for meaningful action is disabled (not even built). The directive is literally only useful when the OS setrlimit() API is available to the built binary.

I am aware of this existing implementation problem. When the backlog problem is addressed, we should fix that implementation.

P.S. In the official code, a bit of useful max_filedescriptors "logic using this directive for meaningful action" exists even when setrlimit() is unavailable.

[kinkie]

Thinking back about this PR, I wonder if it makes sense to leave max_filedescriptors in.
In the end, we should enable the administrator to have fewer FDs then the system allows,
and it boils down to making fd_table smaller than the system allows.

what do you think?

[rousskov]

Thinking back about this PR, I wonder if it makes sense to leave max_filedescriptors in. In the end, we should enable the administrator to have fewer FDs then the system allows, and it boils down to making fd_table smaller than the system allows.

what do you think?

Yes, max_filedescriptors (the concept, not necessarily the current implementations) is meaningful in all environments, including:

• environments where admin wants fewer FDs than the system allows (i.e. the case you outline above);
• environments where admin wants more FDs than the system allows;
• environments where admin wants the maximum number of FDs allowed by the system;
• environments where Squid instance does not know what the system allows.

And the above list does not enumerate all important environments. To get closer to a comprehensive list, we at least need to "multiply" the above by what we think --with-filedescriptors concept means (including its default).

Once/if we agree on what those two directives/options should mean, we can adjust Squid code accordingly.