Integrated Redis Cache and Redis Sessions into Redis OM Spring.#608
Integrated Redis Cache and Redis Sessions into Redis OM Spring.#608foogaro wants to merge 1 commit intoredis:mainfrom
Conversation
|
👀 |
| try { | ||
| jsonValues = connection.sync().jsonGet(key); | ||
| } catch (NullPointerException e) { | ||
| // Workaround for Lettuce 6.5 bug: |
There was a problem hiding this comment.
we shouldn't need this anymore rihgt since we are using Lettuce 6.7.1.RELEASE?
| this.evictionLatency = Timer.builder("cache.evictions.latency").tag("name", name).description("Cache evictions") | ||
| .register(configuration.getMeterRegistry()); | ||
| if (configuration.isIndexEnabled()) { | ||
| createIndex(); |
There was a problem hiding this comment.
do we want to recreate the index every time RedisCache is instantiated? If this is running in multiple apps that come up and down in k8s pods for example it seems like it would be recreated a lot. Maybe check if its already created and in sync before dropping it?
There was a problem hiding this comment.
@jruaux looking at the code the creation of the index seems to impact only one field, which is the _class field of the JSON/Hash.
If that is the case, it would be easy to implement a check to verify if the index really needs to be recreated or not.
Waiting for your feedback to proceed, once you are back!
There was a problem hiding this comment.
@foogaro yes that is the only field it is indexing. You can use RediSearchCommands.ftList() and RedisModulesUtils.indexInfo(connection.sync().ftInfo(indexName())) to check index exists and its definition
| throw new IllegalArgumentException(String.format("unparseable point %s", s)); | ||
| } | ||
|
|
||
| return new GeoLoc(Double.parseDouble(parts[1]), Double.parseDouble(parts[1])); |
There was a problem hiding this comment.
is this a bug? Should it be parts[0] and parts[1]?
There was a problem hiding this comment.
@jeremyplichta You are right!
How did you catch that? 😳
There was a problem hiding this comment.
maybe I read every line of the 10,800 lines very very carefully. Or maybe I had an LLM help me. I guess we'll never know 🙃
bsbodden
left a comment
bsbodden
left a comment
There was a problem hiding this comment.
Code looks good, just wondering if we can extract some of the common code between these packages and the core
4 participants
Bringing Redis Cache and Redis Sessions into Redis OM Spring.
To keep a standard and "all-in-one" naming convention:
com.redis.cachetocom.redis.om.cachecom.redis.sessionstocom.redis.om.sessions