Fix Cluster.refresh sometimes taking upwards of 20s #4826
Conversation
- Only read and parse the proxy config once - Reuse the AuthorizationV1Api instance for the entire refresh instead of recreating it between 32 and 302 times, this should allow for reusing sockets Signed-off-by: Sebastian Malton <[email protected]>
| return this.canI({ | ||
| verb: "watch", | ||
| resource: "*", | ||
| ...customizeResource, |
There was a problem hiding this comment.
This option is not used?
There was a problem hiding this comment.
No, that function was only used in one place and that was within the refreshAccessibility method above.
| this.allowedNamespaces = await this.getAllowedNamespaces(); | ||
| this.allowedResources = await this.getAllowedResources(); | ||
| const proxyConfig = await this.getProxyKubeconfig(); | ||
| const canI = this.dependencies.createAuthorizationReview(proxyConfig); |
There was a problem hiding this comment.
Is this where the optimization is?
There was a problem hiding this comment.
Yes, it creates the API object only once instead of dozens or hundred of times.
There was a problem hiding this comment.
I can see here where it creates it once instead of twice like the old code. Where else is the creation avoided?
There was a problem hiding this comment.
The big win is from getAllowedResources which calls canI in a loop
There was a problem hiding this comment.
ah, right, the class member version of canI() was creating the api each time it was called.
Only read and parse the proxy config once
Reuse the AuthorizationV1Api instance for the entire refresh instead
of recreating it between 32 and 302 times, this should allow for
reusing sockets
Signed-off-by: Sebastian Malton [email protected]