Skip to content

improve number of method calls to k8s api server for endpoints catalog watcher#1393

Merged
ryanjbaxter merged 5 commits intospring-cloud:3.0.xfrom
wind57:simplify-catalog-calls
Aug 2, 2023
Merged

improve number of method calls to k8s api server for endpoints catalog watcher#1393
ryanjbaxter merged 5 commits intospring-cloud:3.0.xfrom
wind57:simplify-catalog-calls

Conversation

@wind57
Copy link
Copy Markdown
Contributor

@wind57 wind57 commented Jul 28, 2023

No description provided.

KubernetesClient client, Predicate<Service> filter) {

if (properties.filter() == null || properties.filter().isBlank()) {
if (properties.filter() == null || properties.filter().isBlank() || filter == ALWAYS_TRUE) {
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this one is interesting and got my attention purely by accident, while working on a different thing.

Both discovery and catalog watch need to search for endpoints, as such they both delegate to the same method. At the same time, because of an issue we had, when searching for endpoints, we need to filter them based on a Predicate. That predicate is a service predicate, it sounds weird - but there was a real defect for this. An example will make this may be easier to understand:

  • in discovery implementation, we search for endpoints
  • users have specified a service SPeL expression (predicate)
  • as such, after we find the needed endpoints, we then go on to find the services for these endpoints (they have to match by name), but only take those that match the predicate.
  • with these service names, we then filter the endpoints and that is the result.

In plain english, it's like we filter endpoints based on a service predicate.

On the other hand, we also need to look for endpoints in the catalog implementation. The difference is that catalog does not support a predicate.

Both catalog and discovery delegate to the same method when looking for endpoints.


And now suppose the case:

  • users have enabled some SPeL Expression
  • they also enabled catalog watcher

Suppose catalog watcher calls this method, and since it does not care about service filtering it passes x -> true as the predicate into it. This is how it looks like:

List<Endpoints> endpoints = endpoints(context.properties(), context.kubernetesClient(),
				context.namespaceProvider(), "catalog-watcher", null, x -> true);

This if statement:

if (properties.filter() == null || properties.filter().isBlank()

is false (because users have enabled a SPeL Expression), so catalog watcher will go on and start searching for services and to try and filter them out by the predicate, again, since this is what the common method does.

But it does not need to do that, since it does not support filtering anyway (we used to pass x -> true always).

So with this fix, we essentially trim down a few method calls to the API server, when not needed.

@wind57 wind57 changed the title fix improve number of method calls to k8s api server for endpoints catalog watcher Jul 28, 2023
@wind57 wind57 marked this pull request as ready for review August 2, 2023 04:54
@wind57
Copy link
Copy Markdown
Contributor Author

wind57 commented Aug 2, 2023

@ryanjbaxter this is not a bug, but an improvement. Thank you for looking into it

@ryanjbaxter ryanjbaxter added this to the 3.0.5 milestone Aug 2, 2023
@ryanjbaxter ryanjbaxter merged commit a9c072b into spring-cloud:3.0.x Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants