spring-cloud / spring-cloud-release

Spring Cloud Release Train - dependency management across a wide range of Spring Cloud projects.
http://projects.spring.io/spring-cloud
Apache License 2.0
874 stars 179 forks source link

Performace regression from springcloud 2020 #244

Closed superbible closed 1 year ago

superbible commented 2 years ago

I compare the performance between multiple versions,here is my test result:

Greenwich.SR4: 2245 TPS Hoxton.SR11: 2271 TPS 2020.0.5: 1263 TPS 2021.0.2: 1293 TPS

I just use 100 threads to access a empty spring cloud service. you can see the performance of 2021.0.2 is only 69% of Hoxton.SR11. I studied the change, and I believe the replacement for netflix ribbon to loadbalancer is the reason. So the problem is why spring-cloud-loadbalancer's performance so poor?

marcingrzejszczak commented 2 years ago

Can you provide a complete, minimal, verifiable sample that reproduces the problem? It should be available as a GitHub (or similar) project or attached to this issue as a zip file.

superbible commented 2 years ago

Can you provide a complete, minimal, verifiable sample that reproduces the problem? It should be available as a GitHub (or similar) project or attached to this issue as a zip file.

Please check the code and see the README: https://github.com/superbible/springcloud-performance-compare/tree/master

superbible commented 2 years ago

any progress?

spencergibb commented 2 years ago

It's a holiday weekend. When there's progress we will mention it here

superbible commented 2 years ago

I fixed the problem, please merge my changes mentioned above.

superbible commented 2 years ago

When use Zookeeper as service register center, the performance regression happens. But not when use Nacos and Consul, it seems they have their own cache system.

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.

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.

OlgaMaciaszek commented 2 years ago

@superbible Do I understand correctly that you're saying the default Evictor-based cache is not being used with load-balancing with ZooKeeper? Let me verify it.

OlgaMaciaszek commented 2 years ago

I have just tested against 2021.0.4-SNAPSHOT with Zookeeper (see this sample repo, main and reactive branches; demo works in conjunction with apps from this branch). The Evictor-based cache was used, so I was not able to reproduce the issue. Please provide a minimal, complete, verifiable example that reproduces the issue.

superbible commented 1 year ago

I have just tested against 2021.0.4-SNAPSHOT with Zookeeper (see this sample repo, main and reactive branches; demo works in conjunction with apps from this branch). The Evictor-based cache was used, so I was not able to reproduce the issue. Please provide a minimal, complete, verifiable example that reproduces the issue.

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