Performance enhancements
- Dominant language
- Go
- Stars
- 1.1k
- Forks
- 134
- PR merge metrics
- No merged PRs in 30d
Description
**Describe the problem/challenge you have**
We rely on `list` api calls to get information from the cluster which could put some burden on the cluster if the number of objects returned is high. When the number of apps being deployed using kapp increases (in cases of kapp-controller packages), this becomes a problem as the time taken to deploy the apps increases after a certain point without any burden on the cpu or memory of the cluster nodes.
- Throttling warnings when there are multiple kapp apps being used at the same time.
- `socket: too many open files` when ulimit is set to a low number (256)
**Describe the solution you'd like*
We need to minimise the list calls as much as possible (Replacing them with get or watch is also an option).
**Tasks**
* [x] Instead of trying to get [all](https://github.com/vmware-tanzu/carvel-kapp/blob/9e863ee3668282e236e34142b221d75629e637a4/pkg/kapp/resources/resource_types.go#L120) the server resources, get only the ones that are related to the available GKs (anyway we get rid of the others [later](https://github.com/vmware-tanzu/carvel-kapp/blob/develop/pkg/kapp/resources/identified_resources_list.go#L42) so they are not being used) - Cancelled for the time being
- [x] Spike: This require more code changes and makes the code less readable. So we will just revisit this later if required.
* [x] When we deploy an app, we first list the labeled resources (GVs) and then try to get the non labeled resources one by one one that are not found in the first step. When an app is deployed for the first time, the first step would always return nil, so maybe we could skip that?
- [x] Spike: How would this fit into kapp code base? -> [PR](https://github.com/vmware-tanzu/carvel-kapp/pull/659)
* [ ] Use watch instead of get and list while waiting for resources to reconcile. Using watch will be helpful for resources that take more time to reconcile (for example deployments), but for resources that reconcile almost immediately (for example configmap), it might bring some overhead.
- [x] Spike: Test if reducing the wait time interval help with this?
We increased the wait-check-interval to 3s as it is reducing api calls and not affecting deployment time much.
PR for the same is [here](https://github.com/vmware-tanzu/carvel-kapp/pull/637) and the data collected while doing spike can check [here](https://docs.google.com/spreadsheets/d/1ymlvw9ChFnOdGFg__kIYdx1z6TowDTk4ObLUNqE569c/edit#gid=485873872)
- [ ] Spike: Test if using watch impacts performance. -> As we have increased wait-check-interval now and it is giving better result, we can have a look on watch later. Prioritising second steps for now.
* [ ] When we have a CRD and CR present in the same manifest, we try to fetch the server resources again to find the CRD (since it wasn't present in the cached server resources). We should somehow avoid doing this as we wouldn't find the CRD this time as well. (No need to work on this if we already work on the first one)
* [ ] Now that we have added the resource namespaces to the fallbackAllowedNamespaces, should we always use fallbackAllowedNamespaces instead of checking resources cluster wide?
- [ ] Spike: Figure out if scoping to fallbackAllowedNamespaces could have any side effects (testing).
* [ ] Currently we store the unique GKs in the meta ConfigMap and we do a list on the GKs, since list calls are more expensive we can check if doing get calls for all the resources is less expensive than list calls for unique GKs.
* [x] Improving performance enhancement specifically during diff stage. With go profiling, it was noticed that there are too many calls to [deepCopy](https://github.com/carvel-dev/kapp/blob/b75689aedd76c2e022a8497473acd1e5facf2210/pkg/kapp/resources/resource.go#L236) and [AsYAMLBytes](https://github.com/carvel-dev/kapp/blob/develop/pkg/kapp/resources/resource.go#L248). [PR](https://github.com/carvel-dev/kapp/pull/731)
**Anything else you would like to add:**
It might be worth understanding the [API priority and fairness](https://kubernetes.io/docs/concepts/cluster-administration/flow-control/).
---
Vote on this request
This is an invitation to the community to vote on issues, to help us prioritize our backlog. Use the "smiley face" up to the right of this comment to vote.
👍 "I would like to see this addressed as soon as possible"
👎 "There are other more important things to focus on right now"
We are also happy to receive and review Pull Requests if you want to help working on this issue.
Contributor guide
Assessment
This issue has not been assessed yet.