-
Notifications
You must be signed in to change notification settings - Fork 179
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set open file limit for ScyllaDB processes #2160
Conversation
9630186
to
18e77d8
Compare
cfac7a8
to
db4b88a
Compare
Manager flake - #2061 (comment) |
if err != nil { | ||
return fmt.Errorf("can't change rlimits: %w", err) | ||
} else { | ||
klog.InfoS("Rlimits were changed successfully") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I think this belongs to the end of the function so it's logged for every caller similar to the intro logging there
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's on the end, would you mind providing a suggestion instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant the end of changeRlimits
(inside the function)
Flake - #2096 (comment) |
/approve /assign @rzetelskik |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: tnozicka, zimnx The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
one suggestion, rest lgtm
ScyllaDB, during regular operation, may need to manage millions of open files due to the nature of its workload and architecture. The open file limit (rlimit) for containers is inherited from the CRI and systemd, both of which tend to set conservative limits to avoid misbehavior in other programs when high limits are applied. Simply setting fs.nr_open using ScyllaCluster sysctls API is insufficient to raise these limits for ScyllaDB process. To automate setting it, Scylla Operator `NodeConfig` container optimization was extended with additional Job discovering the maximum possible limit and setting it on main process of ScyllaDB containers. ScyllaDB Pods await until limit is changed before starting ScyllaDB process. Any forks (sidecar starter or hypervisor) should inherit the limits. Users should increase `fs.nr_open` to at least value [recommended by ScyllaDB](https://github.com/scylladb/scylladb/blob/master/dist/common/sysctl.d/99-scylla-filemax.conf#L5), because defaults of popular Container Runtimes are ~1024 times lower. Sysctls can currenly be changed via `scylladbcluster.spec.sysctls` field. Note that this tuning is applied only on Nodes matching deployed NodeConfig selector.
/lgtm |
Description of your changes:
ScyllaDB, during regular operation, may need to manage millions of open files due to the nature of its workload and architecture. The open file limit (rlimit) for containers is inherited from the CRI and systemd, both of which tend to set conservative limits to avoid misbehavior in other programs when high limits are applied. Simply setting fs.nr_open using ScyllaCluster sysctls API is insufficient to raise these limits for ScyllaDB process.
To automate setting it, Scylla Operator
NodeConfig
container optimization was extended with additional Job discovering the maximum possible limit and setting it on main process of ScyllaDB containers. ScyllaDB Pods await until limit is changed before starting ScyllaDB process. Any forks (sidecar starter or hypervisor) should inherit the limits.Users should increase
fs.nr_open
to at least value recommended by ScyllaDB, because defaults of popular Container Runtimes are ~1024 times lower. Sysctls can currenly be changed viascylladbcluster.spec.sysctls
field. Note that this tuning is applied only on Nodes matching deployed NodeConfig selector.Which issue is resolved by this Pull Request:
Resolves #2131