cache-dot-clj icon indicating copy to clipboard operation
cache-dot-clj copied to clipboard

Masai

Open Raynes opened this issue 14 years ago • 7 comments

I added a Masai external caching algorithm. This gives cache-dot-clj the ability to cache to Redis and Tokyo Cabinet.

I used the ehcache implementation as my base for this. I'm also using all of its non-ehcache-specific tests, and they all pass.

If anything appears out of the ordinary, let me know and I'll fix 'er up! I think this is how it is supposed to work, but I could always be wrong. The fact that the tests pass is probably a good sign though. ;)

Raynes avatar May 18 '11 22:05 Raynes

Nice work! It looks good to me. Do you want to merge it in Saul or shall I?

On a side note... With stores like these we should make it clear in the README that this cache is not blocking. Meaning, no locks are put in place which prevent mutliple threads or processes from computing the same value mutiple times simultaneously.

For simple KV stores like this in the past I've created a lock key based on the original key and it tends to work reasonably well. If we do want to add support for this then we would want to modify the external cache mechanism in store and have each strategy provide a lock and unlock functions as well.

bmabey avatar May 18 '11 23:05 bmabey

I got the go ahead from Sual to be more of a maintainer on this project, so I've merged this in. I plan on making the 0.4.0 release shortly. Thanks again for the patch!

bmabey avatar May 23 '11 19:05 bmabey

Hi again... I merged in the patch, but I think it still needs some work. When I tried to run the tests I got:

Exception in thread "main" java.lang.UnsatisfiedLinkError: no jtokyocabinet in java.library.path (tokyo.clj:2)

This isn't surprising since I don't have tokyo cabient installed, much less in my library path. I think this points out a problem though. Currently the masai subproject is requiring redis, tokyo, and db by default: https://github.com/Raynes/cache-dot-clj/blob/1433e296fb3417209997152813ac6d696fbf928a/masai/src/cache_dot_clj/masai.clj#L3-5

A project that wants to use the masai caching plugin shouldn't have to have all of these unrelated dependencies. Could you make the patch lazily load the appropriate masai namespace based on what is selected by the strategy? (i.e. Move the require down to the strategy definition.)

Also, making jedis, etc a dependency of this project may not be the best option: https://github.com/Raynes/cache-dot-clj/blob/1433e296fb3417209997152813ac6d696fbf928a/masai/project.clj#L4-7 We can add them to the dev-deps for the tests but if someone wanted to use this plugin in a project they will have already declared a dependeny on the store they are using (e.g. jedis) and so adding all the stores provided by masai as deps here will result in needless dep downloading/loading. Taking the approach that masai takes is probably best: https://github.com/ninjudd/masai/blob/develop/project.clj

bmabey avatar May 27 '11 20:05 bmabey

The db require is a necessity. It's just the Masai protocol definition. However, you're right about where the requires should be and such. Not sure why I didn't do that in the first place. I'll have that done for you by the end of the day. :)

Raynes avatar May 27 '11 20:05 Raynes

Cool! BTW, I decided to change the name of the project to clj-cache to be more inline with other clojure project names (e.g. clj-time, clj-file-utils, etc).. and so I could push it to clojars. Anyways, if you want to rebase off my my changes that would be cool: https://github.com/bmabey/clj-cache Otherwise I can update your changes once you are done.

bmabey avatar May 27 '11 21:05 bmabey

One more thing... I wasn't able to run the tests but it looks like you may have a bug here:

https://github.com/Raynes/cache-dot-clj/blob/masai/masai/src/cache_dot_clj/masai.clj#L40

Shouldn't s be passed into prefix like so: (prefix f-name s)? (instead of just (prefix f-name))

bmabey avatar May 27 '11 21:05 bmabey

Nice catch. Don't know how it worked with that broken, but it did. :o

I'll make all my changes off of your clj-cache repo and fix this as well. :)

Raynes avatar May 28 '11 00:05 Raynes