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

DefaultRedisClusterClientFactory uses default StringCodec #536

Closed
herisn23 opened this issue Jun 5, 2024 · 3 comments · Fixed by #571
Closed

DefaultRedisClusterClientFactory uses default StringCodec #536

herisn23 opened this issue Jun 5, 2024 · 3 comments · Fixed by #571

Comments

@herisn23
Copy link

herisn23 commented Jun 5, 2024

There is a issue with RedisCodec when using different environment configuration (cluster, non-cluster) when using custom codec.

When developing on localhost i use standalone non-cluster redis. This configuration uses codec created in my factory class.

@Singleton
@Replaces(RedisCodec::class)
@Primary
fun serializedObjectCodec(): RedisCodec<String, Any> =
    createJdkSerializerCodec()

But DefaultRedisClusterClientFactory bypassing this codec because not provide codec when call client.connect() like in non-cluster configuration.

This behaviour causes errors on kubernetes where redis-cluster running, because connection uses default lettuce StringCodec instead of user-defined codec.

It is possible to inherit AbstractRedisClientFactory<K, V> like in DefaultRedisClientFactory and create StatefulRedisClusterConnection with user defined codec?

For now i have work around for this issue with custom configuration but i wont have this code in my app.

@Bean(preDestroy = "close")
@Replaces(StatefulRedisClusterConnection::class)
@Singleton
@Primary
fun redisConnection(
    @Primary codec: RedisCodec<String, Any>,
    @Primary redisClient: RedisClusterClient
): StatefulRedisClusterConnection<String, Any> {
    return redisClient.connect(codec)
}

This is the class which, i think, need improvements.

@graemerocher
Copy link
Contributor

seems like you diagnosed the problem, would you send a PR?

@yibo-long
Copy link
Contributor

@herisn23 are you interested in a PR? Otherwise I also intend to contribute on this as we faced with this issue too.

@yibo-long
Copy link
Contributor

Created #571, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants