Implement Redis cache extension#1937
Implement Redis cache extension#1937estliberitas wants to merge 1 commit intoapache:masterfrom estliberitas:redis-cache
Conversation
There was a problem hiding this comment.
it might be useful to share the code with memcached cache here instead of having two different implementation, unless there is a good reason to have two different ones.
There was a problem hiding this comment.
Kind of CacheUtils class to put serialize / deserialize stuff in druid-processing?
There was a problem hiding this comment.
@xvrl there is already a class called CacheUtil, can it be used for purpose described above? Or these things worth another utility class which to reside in druid-processing?
This CacheUtil class has quite a few methods right now. Maybe it's a good idea to extend it with those methods?
There was a problem hiding this comment.
@xvrl will use existing CacheUtil then )
There was a problem hiding this comment.
CacheUtils seems fine to me, unless you want to have it as part of io.druid.client.cache, in which case we can call it CacheImplUtils or something
|
@xvrl Thanks for reviewing and notes, I'll make all the additions tomorrow thus very busy today. |
|
@estliberitas no worries, take your time. |
|
@xvrl can you answer a question about Also, about sharding, are we talking about Sentinel or Cluster which is in development as far as I remember? |
|
@estliberitas Regarding CacheUtil, see my answer above. |
|
@xvrl As far as I know, Jedis has nothing similar to this. So I'm gonna implement it or extend some pool class which would given a key pickup client by hash. |
There was a problem hiding this comment.
to be removed according to the contrib-guide
|
@estliberitas it looks like jedis supports client-side sharding using something that looks like ketama https://github.com/xetorthio/jedis/blob/master/src/main/java/redis/clients/jedis/ShardedJedis.java I would just use that and use murmurhash to be consistent with the memcached configuration. |
|
@xvrl Sounds reasonable, thanks, I'll use it! |
|
This needs to be rebased FYI. |
|
Finally got the time for contribution again. Will have something by end of week or so. |
|
@estliberitas any chance of finishing this up? |
|
@fjy yep. Let me check what needs to be done. |
|
@xvrl @fjy Please, check out, guys. You can see that using of Also, for each namespace there is a set with keys, so |
|
@drcrallen Updated with question about |
|
Travis build failed somehow, but output says |
Also implemented `CacheImplUtils` to share functionality with Memcached cache extension.
|
@drcrallen @fjy Guys, I put |
|
@drcrallen going to merge this in 24 hours and then refactor extensions |
|
@fjy You had a thumbs up and I didn't see anything from xavier yet, though I haven't heard any complaints from him yet either. Didn't realized this was blocked on me instead of xavier. Merging to a extensions.contrib should be good. |
|
@xvrl just realized this isn't a module... Are the caches extendable? |
|
@fjy Well, I can try make them truly extendable. Like introduce interfaces in appropriate module (druid-api? druid-processing?) and change existing implementations. Or what do you mean? |
|
@estliberitas I mean like one of the modules in the extensions directory |
|
@fjy And now I don't get at all 😄 So this cache extension is in extensions directory. And it's a separate Maven module. |
|
@fjy what should I do next? Rebase / put into another repo? |
|
@estliberitas is it possible for this work to be done as a extension in the extensions-contrib folder? |
|
@fjy it's your project, man, you are the one to answer )) so now I know, I should move it and rebase based on the new way of handling extensions. thanks |
|
@estliberitas can you please update this PR such that the code is contained in an extension-contrib module similar to other community contributed extensions? Apologies for the delay in reviewing this, we are extremely backed up in reviewing PRs. |
| |`druid.cache.maxObjectSize`|Maximum object size in bytes for a Memcached object.|52428800 (50 MB)| | ||
| |`druid.cache.memcachedPrefix`|Key prefix for all keys in Memcached.|druid| | ||
|
|
||
| #### Redis |
There was a problem hiding this comment.
can we have these docs in a separate file similar to how the other extensions are structured?
Can we also update the extensions.md file to include the new redis extension?
|
@estliberitas there are merge conflicts and let's make sure this is set up similar to the other extensions in extensions-contrib |
|
Removing milestone as this PR appears to be abandoned |
|
@fjy gonna continue working on it. Hopefully, not that much in code base has changed. |
|
Closing to see if #4615 can get through |
6 participants
This is a first working draft of extension. It provides basic caching functionality and cache statistics, while doMonitor() still does nothing(). Data is compressed using LZ4.
This is to resolve #1927.