spring-cloud-commons icon indicating copy to clipboard operation
spring-cloud-commons copied to clipboard

Change the warning to give right suggestion to use cache for Load Bal…

Open superbible opened this issue 2 years ago • 2 comments

Fix #244

  1. When use Zookeeper as service register center, the performance regression happens. But not when use Nacos.

  2. There're cache system for Load Balancer already, but it's doesn't work by default, but give a warning: "Spring Cloud LoadBalancer is currently working with the default cache. While this cache implementation is useful for development and tests, it's recommended to use Caffeine cache in production.You can switch to using Caffeine cache, by adding it and org.springframework.cache.caffeine.CaffeineCacheManager to the classpath." This is the bug. By Default, the Default Cache doen't work. As the suggestion, not matter use Caffeine or Evictor (default), explicit dependecies are required in pom.xml.

  3. So I think probably we can just add Caffeine dependecies by default, or just change the warning to "Spring Cloud LoadBalancer is currently working without cache...", the latter is used in pull request.

superbible avatar Jun 13 '22 06:06 superbible

Hello @superbible . Thanks for submitting the PR. Are you saying the Evictor-based cache is not being used with Zookeeper? That would be a bug and I'll need to fix it then rather than changing the warning. Let me verify it.

OlgaMaciaszek avatar Aug 31 '22 12:08 OlgaMaciaszek

@superbible Please see https://github.com/spring-cloud/spring-cloud-release/issues/244#issuecomment-1232939094. In any case, if we do manage to reproduce the issue, I think we'll need to fix the caching rather than changing the warning.

OlgaMaciaszek avatar Aug 31 '22 13:08 OlgaMaciaszek

This is the bug. By Default, the Default Cache doen't work. As the suggestion, not matter use Caffeine or Evictor (default), explicit dependecies are required in pom.xml.

You can try to use the spring-cloud-starter-loadbalancer to instead spring-cloud-loadbalancer in your demo and test again.

The Evictor dependency in spring-cloud-loadbalancer is declared optional by setting the <optional> element so the Evictor-based cache was not used that performace regression in your demo.

In spring-cloud-starter-loadbalancer the Evictor dependency declared correctly so it will works fine.

aofall avatar Nov 17 '22 08:11 aofall

@superbible @aofall - in order to set up load-balancing for your project, you are supposed to add the starter, not the lib dependency, same as with any other Boot or Cloud projects - you're expected to use starters for setting up your applications.

OlgaMaciaszek avatar Nov 22 '22 15:11 OlgaMaciaszek

@superbible @aofall - in order to set up load-balancing for your project, you are supposed to add the starter, not the lib dependency, same as with any other Boot or Cloud projects - you're expected to use starters for setting up your applications.

yes, you'r right, it's not a bug.

@superbible @aofall - in order to set up load-balancing for your project, you are supposed to add the starter, not the lib dependency, same as with any other Boot or Cloud projects - you're expected to use starters for setting up your applications.

yes, you're right, I checked the Spring Initializer with load balancer depedency, it generated pom.xml with <artifactId>spring-cloud-starter-loadbalancer</artifactId> instead of <artifactId>spring-cloud-loadbalancer</artifactId>, so it's not a bug, thanks.

superbible avatar Nov 28 '22 06:11 superbible