search

spring-projects / spring-data-redis

Provides support to increase developer productivity in Java when using Redis, a key-value store. Uses familiar Spring concepts such as a template classes for core API usage and lightweight repository style data access.
https://spring.io/projects/spring-data-redis/
Apache License 2.0
1.76k stars 1.16k forks source link

Filter keyspace expiration events in a shared Redis cluster #2111

Open pbarsotti-glovo opened 3 years ago

pbarsotti-glovo commented 3 years ago

Working on a legacy application which uses a single Redis cluster, I found a mixed scenario where Redis repositories are enabled and also the same cluster is used for a RedisCacheManager.

Due to the use of repositories, this routine is executed on MappingExpirationListener upon the expiration of each element in the cluster. In order to determine if the key should be processed, a validity check is performed. Valid keys must contain at least one delimiter : as we can see here.

We use the same cluster for caching, and some keys contain the delimiter value :, so the listener is trying to execute the routine with them. Unfortunately, due to the nature of the cached values we end up having keys with the format FIXED_PART:VARIABLE_PART, so the listener is misinterpreting the key, and tries to perform a SREM(FIXED_PART, VARIABLE_PART) as we can see here. This leads to a high throughput in one node of the cluster (the one where FIXED_PART is assigned).

I was thinking about adding a prefix to the cached keys and then a filtering layer in the listener to discard them if they start with the prefix, but MappingExpirationListener is instantiated inside RedisKeyValueAdapter and there is no possibility to replace it.

Is there any known way to handle this type of situation?

Thanks!

mp911de commented 3 years ago

MappingExpirationListener is an internal component that cannot be configured externally. It registers for __keyevent@*__:expired which matches notifications for all keys. In theory, there could be two approaches:

  1. Limiting the pub/sub pattern to a known keyspace so that cache events do not get matched
  2. Matching the expired key name against the known entity types: This can be tricky as all @RedisHash types need to be registered upfront so that the MappingContext can provide details about the keyspace prefix. If an entity wasn't registered at that time, it could easily lead to dropped notifications.

Going forward, opening up MappingExpirationListener and RedisKeyValueAdapter for customization through subclassing seems the most promising approach.

pbarsotti-glovo commented 3 years ago

Thanks @mp911de for the response. I see all the options require modifications in the library. How is this handled usually? A solution is discussed in the issue and then anyone can submit a PR following the agreed approach?

mp911de commented 3 years ago

Yes, typically someone crafts a pull request. External contributions speed up the entire process whereas waiting for us typically takes a bit longer.