search

spring-projects / spring-boot

Spring Boot
https://spring.io/projects/spring-boot
Apache License 2.0
74.57k stars 40.55k forks source link

redis sentinel default configuration broken #11855

Closed zil closed 6 years ago

zil commented 6 years ago

https://github.com/spring-projects/spring-boot/blob/d478e9bf57b73ca84539c0e9bfa82961256e50d9/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/data/redis/JedisConnectionConfiguration.java#L88

https://github.com/spring-projects/spring-data-redis/blob/2fe048bfff856012f42aed9f702334bc5b7d28fd/src/main/java/org/springframework/data/redis/connection/jedis/JedisConnectionFactory.java#L275

Using springBoot with version 2.0.0.M6 and redis.clients:jedis:2.9.0,with config:

spring:
  redis:
    sentinel:
      nodes: 0.0.0.0:26379,0.0.0.0:26479,0.0.0.0:26579
      master: mymaster

But Spring Boot not use sentinel config,but resolve to localhost:6379.By debugging I found the above code is the cause: If the pool configuration is leave out,Spring Boot will resolve to localhost:6379

wilkinsona commented 6 years ago

@mp911de Can you offer your opinion on this one please? It looks like Spring Data Redis allows a JedisConnectionFactory to be created with client configuration that means that the accompanying sentinel configuration is ignored. Is that intentional?

mp911de commented 6 years ago

There are two things to this issue which is a regression bug:

  1. It's not possible to use Sentinel without a pool so we need to apply a fix to not allow Sentinel configuration without pooling. We switched from configuring the connection factory by properties to configure the factory via JedisClientConfiguration. JedisClientConfiguration requires explicit pooling activation, which plays into the cause.
  2. With the refactoring to use JedisClientConfiguration, we lost the default in Spring Boot that applied pooling even when spring.redis.jedis.pool properties were not set.

I filed DATAREDIS-765 to address the issue on our side. We should prevent falling back to localhost connections.

wilkinsona commented 6 years ago

Thanks, Mark. I'm watching DATAREDIS-765 now. If you go with the "configure the pool with defaults" option, it sounds like a change in Boot won't be necessary. I'll keep this one open until that decision's been made.

wilkinsona commented 6 years ago

It looks like this has been fixed entirely on the Data Redis side of things. We'll pick this up in the next Spring Data Kay service release. @mp911de do you have any plans for SR4 at the moment?

mp911de commented 6 years ago

Nothing scheduled yet. Releasing Kay SR4 would make sense right before Spring Boot RC2.

The issue is fixed in Spring Data Redis entirely.