Bug 5554: Cap raw RLIMIT_NOFILE in max_filedescriptors default (#2483)

[squidadm]

Fix a problem where some environments (e.g. Kubernetes) set
very high hard file descriptor limit. Unless the
max_filedescriptors configuration option is set,
in these cases Squid will match these limits
for some internal data strctures, wasting core memory.

---

setMaxFD() updates Squid_MaxFD. Squid_MaxFD is then used to allocate
various FD-indexed tables, including Comm's fd_table. When
Config.max_filedescriptors is not set, Squid has to guess the maximum
number of descriptors this instance can handle. Prior to these changes,
that guess was based on OS-provided ulimits (RLIMIT_NOFILE).

Using raw RLIMIT_NOFILE values to set Squid_MaxFD results in huge
fd_table allocations that kill or incapacitate Squid when OS uses very
large limits (e.g., Kubernets soft limit of 1073741816=2^30-8
effectively means "unlimited" and results in ~432 MB fd_table). We now
cap such raw values at 100K which yields ~42 MB fd_table.

Several other old problems were discovered and marked during this work.
Those out-of-scope problems deserve dedicated fixes.

N.B. setMaxFD() calls setrlimit(2) in some cases, but the primary/final
setrlimit(2) call is in setSystemLimits(). That primary call sets the
soft limit to Squid_MaxFD.

[squid-anubis]

Cannot create a git commit message from PR title and description.

Error while parsing future commit message title: the line is too long 78>72

Problematic parser input:

Bug 5554: Cap raw RLIMIT_NOFILE in max_filedescriptors default (#2483) (#2486)

Please see PR title and description formatting requirements for more details.

This message was added by Anubis bot. Anubis will add a new message if the error text changes. Anubis will remove M-failed-description label when there are no corresponding failures to report.

[yadij]

@kinkie: not sure we want to accept this into v7 as-is. The documentation text has a lot of text that is only relevant to developers (not users). I am thinking there are also a much better long-term fix for the lack of a Squid hard limit, and for misconfigurations.

[kinkie]

@kinkie: not sure we want to accept this into v7 as-is. The documentation text has a lot of text that is only relevant to developers (not users).

Fair point. The text can be changed.

I am thinking there are also a much better long-term fix for the lack of a Squid hard limit, and for misconfigurations.

In my opinion, divergence from trunk is a problem because it increases the risk of conflicts.
Rather than diverging, can you build the improvement over this PR?

[kinkie]

@kinkie: not sure we want to accept this into v7 as-is. The documentation text has a lot of text that is only relevant to developers (not users).

Fair point. The text can be changed.

A proposal for the amended text is in the first section of the PR description.
WDYT?

[rousskov]

Amos: The documentation text has a lot of text that is only relevant to developers (not users).

FWIW, I disagree. Also, the description in master (that this PR is backporting) is overall better than the current description in v7 IMHO.

Francesco: The text can be changed.

In a dedicated PR targeting master/v8, please. Preferably, such polishing non-emergency PRs should come after the backlog issue is dealt with. Revising how Squid determines the maximum number of descriptors (instead of polishing the description of the current problematic algorithm) may be the best way forward.

Amos: I am thinking there are also a much better long-term fix for the lack of a Squid hard limit, and for misconfigurations.

Agreed. A lot of improvements are possible in this area, but they should not be happening in a backport-a-bug-fix-to-v7 PR, and they should not be happening now.

[yadij]

Two PRs coming up that do the cleanup. First to use the IFDEF mechanism for max_filedescriptors. Second to use the DEFAULT_IF_NONE mechanism for this Squid config value, instead of hard coding it.

I am proposing that we hold this PR until we can have all three changes merge together.