-
Notifications
You must be signed in to change notification settings - Fork 122
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
increase number of inotify instances per user #1214
increase number of inotify instances per user #1214
Conversation
Hi @LittleFox94. Thanks for your PR. I'm waiting for a kubermatic member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
f4b819d
to
17d5506
Compare
@LittleFox94 please run |
which do you like better: in a new commit or squashed into the original one? |
@LittleFox94 doesn't matter, in the end all commits will be squashed by the @kubermatic-bot (which is pushing merges). |
This defaulted to 128, meaning a given user ID can have a maximum of 128 inotify instances. Increasing to 8192. Signed-off-by: Mara Sophie Grosch <mgrosch@anexia-it.com>
17d5506
to
aab8ca2
Compare
/test all |
I only see timeouts in the failed tests, is there anything I have to do @kron4eg ? |
/retest |
@LittleFox94: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/approve |
LGTM label has been added. Git tree hash: a970b7a98d8f9b50cb24a125502e4f23e5dcc82d
|
/retest |
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.
/approve
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ahmedwaleedmalik, kron4eg, LittleFox94 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 |
just randomly saw this is mentioned in the v1.46 release as being for our provider only - if anyone stumbles over this, this is, in fact, a provider-independent change |
What this PR does / why we need it:
We often run into the problem of not being able to
kubectl logs -f
because of all allowed inotify instances being in use. This increases the instance limit (from a default of 128 we saw) to 8192, giving some more breathing space.Corresponds to #471, but increases the maximum number of instances instead of watches, as was recommended in kubernetes/kubernetes#10421 roughly a month after #471 was merged.
Special notes for your reviewer:
Instance limit of 8192 works great for us, but I have no calculation for how to choose this value.
Optional Release Note: