Skip to content
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

Bumped updateCh limit to fix #2022 #2081

Closed
wants to merge 1 commit into from

Conversation

azweb76
Copy link

@azweb76 azweb76 commented Feb 13, 2018

What this PR does / why we need it:
For larger clusters, updateCh with 1024 is not enough and prevents nginx from starting. I bumped updateCh limit to 99999. Ideally this will be auto sized or adjusted to start popping items off during original startup.

Which issue this PR fixes (optional, in fixes #(, fixes #<issue_number>, ...) format, will close that issue when PR gets merged): fixes #2022

Special notes for your reviewer:
Ideally a longer term solution should exist to support even larger clusters.

For larger clusters, updateCh with 1024 is not enough and prevents nginx from starting. Ideally
this will be auto sized or adjusted to start popping items off during original startup.
@k8s-reviewable
Copy link

This change is Reviewable

@k8s-ci-robot k8s-ci-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Feb 13, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: azweb76
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: bprashanth

Assign the PR to them by writing /assign @bprashanth in a comment when ready.

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 OWNERS Files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@aledbf
Copy link
Member

aledbf commented Feb 13, 2018

@azweb76 can you share the number of services and endpoints in your cluster?

@azweb76
Copy link
Author

azweb76 commented Feb 13, 2018

496 services, 499 endpoints

@azweb76
Copy link
Author

azweb76 commented Feb 13, 2018

486 ingress objects

@azweb76
Copy link
Author

azweb76 commented Feb 13, 2018

This is likely a ticking time bomb for those that are about to exceed the 1024 limit. Once their object count exceeds that limit, the pod will fail to start. This will at least extend the time to provide a long term fix.

@aledbf
Copy link
Member

aledbf commented Feb 13, 2018

@azweb76 I am fixing this issue right now using a ring buffer to never block writes. I will open the PR and a temporal image in a couple of minites

@azweb76
Copy link
Author

azweb76 commented Feb 13, 2018

sweet! I can help test if you'd like.

@aledbf
Copy link
Member

aledbf commented Feb 13, 2018

@azweb76 please use quay.io/aledbf/nginx-ingress-controller:0.325 to test the fix

@azweb76
Copy link
Author

azweb76 commented Feb 13, 2018

That appears to have fixed it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NGINX 0.10.2 not starting
5 participants