Add Optional User Configurable File Handler Soft Limit#395
Add Optional User Configurable File Handler Soft Limit#395krkeegan wants to merge 3 commits intohassio-addons:mainfrom
Conversation
WalkthroughAdds a configurable Changes
Sequence Diagram(s)sequenceDiagram
participant Config as Config (yaml)
participant Init as init script (run)
participant Kernel as Kernel limits
participant Influx as InfluxDB service
Config->>Init: supply `nofile_soft_limit`
Init->>Init: validate value (>0)
Init->>Kernel: query hard limit (`ulimit -H -n`)
Kernel-->>Init: return hard_limit
Init->>Init: soft_limit = min(soft_limit, hard_limit)
Init->>Kernel: set soft limit (`ulimit -S -n`)
Kernel-->>Init: success / failure
Init->>Influx: start InfluxDB (with adjusted limits)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
influxdb/DOCS.md (1)
34-45: Fix example key to match the actual option name. The example usesnofile, but the schema and option arenofile_soft_limit, which will confuse users.✅ Proposed doc fix
log_level: info auth: true reporting: true ssl: true certfile: fullchain.pem keyfile: privkey.pem -nofile: 65536 +nofile_soft_limit: 65536 envvars: - name: INFLUXDB_HTTP_LOG_ENABLED value: "true"
🤖 Fix all issues with AI agents
In `@influxdb/DOCS.md`:
- Around line 100-105: Fix the typo in the DOCS.md sentence that reads "The
default value in Home Assistant OS is 1024 and the maximum. value is 524,288."
by removing the stray period so it becomes "The default value in Home Assistant
OS is 1024 and the maximum value is 524,288." — locate and edit that exact
sentence in the InfluxDB docs content to ensure the sentence is not broken.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Proposed Changes
This adds a user configurable option for the file handler soft limit. As discussed in #383 the default soft limit of 1024 file handlers introduced in HAOS 16 is too low for some users. This results in a constant barrage of
too many open fileserrors and a lot of CPU usage.InfluxDB itself doesn't set or request any changes to the limits on the number of file handlers. Nor do they provide any guidance on what the expectations should be. Best I can find are forum comments here and here suggest anything from 500,000 to 65,536 are used by users.
The config option is also "hidden" in that the user has to click
Show unused optional configuration optionsin order to see the option.By default, the option is unset and defaults to whatever the container has set, I believe 1024 for HAOS users.
I added safety checks to ensure that the requested soft limit is below the hard limit, and provided log messages to the user.
I was personally affected by this error. I tested this by leaving the soft limits undefined, which resulted in the same error appearing. When I changed the limit to 65536, the error went away. I also tested setting the soft limit to an absurd number and the safety checks limited it to the HAOS maximum.
I am happy to make whatever suggested changes are required.
For reference, I considered using the available addon container config as documented here such as:
However this would have affected all users and it seems like many are not affected by this issue.
Of course if the hard limit of 524,288 proves to be too low for anyone, then we would need to adjust the hard limit in the configuration. But that seems unlikely.
Related Issues
Fixes #383
Summary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.