-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Duplicate caches in OpenShift master #8229
Comments
This is without Node watches or any extended admission controllers enabled. |
30s sync loops:
1m sync loops:
2m sync loop:
For anything with a short list interval (<5m) instead of re-listing, we should be scanning the list again. We need to investigate whether any controllers mutate the in-memory store as part of normal operations in a way that they need the store to be refreshed, and separate those out. |
@jeremyeder @timothysc duplicated caches are the likely cause of controller problems on startup and excessive memory use on the controllers. This is not all admission controllers enabled (so there are probably others), but there is plenty of dead weight here. Sync loops should also be adjusted to avoid doing a list (there is no need in most cases). Will spawn appropriate issues. |
ack, this makes sense. /cc @rrati |
@smarterclayton admission plugins probably only need caches, so those shouldn't be too hard. I'll think about how to refactor the controllers to share nicely. |
I've added duplicate cache sharing on the Kube 1.3 list since it
affects a few of them today.
|
Data also rides across the scheduler as well. Per conversations today we need ~= L2 / L4 equiv cache. There is a ton of data in the objects that are not needed for caching. |
We had discussed the use of flyweight caches in a few other areas in On Thu, Mar 24, 2016 at 1:39 PM, Timothy St. Clair <[email protected]
|
Admission plugin caches are largely an upstream problem. We'd need to do surgery upstream to collapse our caches. @derekwaynecarr
|
I am +1 in porting the wants pattern we added in origin into upstream. I think we can ignore NamespaceExists and NamespaceAutoProvision. |
In 1.4.0-alpha.0:
Which is:
|
Which is 74 total. |
14 upstream reflectors (started in the /vendor/ package) and 60 downstream. |
This is a long running effort. No, it will not be resolved in 3.4 or 3.5. Maybe 3.6. |
Lists of actual dynamic resources left. Projects shouldn't be in this list. ScheduledJobs shouldn't be in this list.
|
I fixed a few of these by reusing ForResource and wrapping in #14391. I think the next step is to split the openshift shared informer
|
Looks like we need a bug for GC tracking multiple resources that are backed by the same resource, as well as resources in multiple group versions. HPA is an example. |
Remaining after excluding projects and authorization token types
|
@bparees @mfojtik @pweil- for us to not regress peak memory use in 3.6 we need to
Right now we've grown the number of caches by 20% from 1.5 -> 3.6 |
@deads2k still need to know why we have two informers for RBAC |
#14562, #14391, and #14289 should get us to approximately 56:
and then I'll do the last one to switch builds to generated informers and we'll cut two more out to 54. |
The policy part was fixed |
Well, except for policy binding, will look. kube-proxy is still using internal in 1.6 so there's not much point to fixing service and endpoints. I think we can turn all SA and secrets to v1, and possibly the namespace informers. That could give us 3 more back. |
Caches when we only start master api:
Templates is because of #14216, I added a comment. Secrets is because of service account admission and enforce mountable secrets, which we rarely turn on. Service and endpoints are DNS, which ideally we move back to the nodes post 3.6. |
Looks like we're back up to 68, mostly due to aggregation:
Worst one is endpoints and services - aggregation is using v1, upstream kube-proxy hasn't moved to to it yet, so we're on it for DNS as well. |
Looks like ClusterRole, Role, ClusterRoleBinding, and RoleBinding are also new. |
StorageClass is also duplicated. |
This may be because of ordering with the started informers (aggregation I'm going to put up a patch, because endpoints are expensive and huge). |
Looking at controllers directly, we can shave a few more off (docker cfg service accounts and secrets). And then this is basically done. |
Found duplication mostly because of GC double loading, going to open a PR to avoid that. There's a set of remaining I have to triage. We have 35 singleton caches, and then a set of duplicates (the duplicates are all large and high volume). Each of those will get its own issue and then I'll close this.
|
Should be good
Deployments, going to try to refactor.
Builds, I'm going to try to refactor.
Should be ok - only admission uses the internal version
Everything in API should be using internal, everything in controller should be using namespace.
Several controllers in origin using internal can be converted to v1, should mean we can get to no internal secret.
kube-proxy in 1.6 uses internal, which means dns must use internal, which means API aggregation must use internal. In 1.7 kube-proxy will switch to versioned, and the others should switch.
Upstream uses Internal for admission, we use an internal for controller manager. Can be fixed |
Done, will investigate other wins in future releass |
Here is a list of all unique reflector caches, their type, and the location that initializes them in origin.
The text was updated successfully, but these errors were encountered: