Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RedisReactiveHealthIndicator is broken with Redis cluster mode #22061

Closed
edwardsre opened this issue Jun 22, 2020 · 3 comments
Closed

RedisReactiveHealthIndicator is broken with Redis cluster mode #22061

edwardsre opened this issue Jun 22, 2020 · 3 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@edwardsre
Copy link
Contributor

After upgrading to Spring Boot 2.2.8.RELEASE from 2.2.7.RELEASE, the Redis health indicator started reporting down with the following error detail on clustered Redis:

"redis": {
   "details": {
     "error": "java.lang.IllegalArgumentException: Value must not be null"
  },
  "status": "DOWN"
 }

It appears that the info properties coming back in 2.2.8 have the IP address and port prefix on the keys (e.g. 127.0.0.1:7002.redis_version). It also appears the properties come back from each node this way.

The attached zip file reproduces the issue. Instructions can be found in instructions.md in the attached zip for how we are starting a clustered Redis docker container.

clustered-redis-issue.zip

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 22, 2020
@edwardsre
Copy link
Contributor Author

@wilkinsona Here is the separate issue you asked for in #21514

@scottfrederick
Copy link
Contributor

scottfrederick commented Jun 22, 2020

Thanks for the report and the sample application. I can reproduce the problem with the provided sample and instructions.

The root cause is a change made to Spring Data Redis 2.2.8 in this commit. With this change, Spring Data Redis is detecting the cluster configuration and providing a LettuceReactiveRedisClusterConnection to Spring Boot's RedisReactiveHealthIndicator, where it was providing a LettuceReactiveRedisConnection in previous versions. When RedisReactiveHealthIndicator calls connection.serverCommands().info(), The Cluster form of the connection eventually delegates to LettuceReactiveClusterServerCommands, which adds the host and port prefix to each info property.

This seems to me like a condition that Spring Boot will need to handle, but I'm not sure how Boot should figure out what's going on or if there's something more Spring Data Redis should be doing to simplify the situation.

cc @mp911de

@mp911de
Copy link
Member

mp911de commented Jun 23, 2020

We changed the behavior as bugfix but weren't aware that the reactive health check would fail. I suggest issuing a PING command if the connection is of type ReactiveRedisClusterConnection and skip the detail for the 2.2.x line. #21514 aims for a proper change in Boot 2.3.x and 2.4.x to align with the blocking healthcheck.

@scottfrederick scottfrederick self-assigned this Jun 23, 2020
@scottfrederick scottfrederick added type: regression A regression from a previous release and removed status: waiting-for-triage An issue we've not yet triaged labels Jun 23, 2020
@scottfrederick scottfrederick added this to the 2.2.x milestone Jun 23, 2020
@scottfrederick scottfrederick added type: bug A general bug and removed type: regression A regression from a previous release labels Jun 23, 2020
@scottfrederick scottfrederick changed the title Redis health indicator started failing on upgrade to 2.2.8.RELEASE RedisReactiveHealthIndicator is broken with Redis cluster mode Jun 23, 2020
scottfrederick added a commit that referenced this issue Jun 24, 2020
@scottfrederick scottfrederick modified the milestones: 2.2.x, 2.2.9 Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

4 participants