Skip to content

Commit

Permalink
Add the first draft of a typed API for accessing Redis keys.
Browse files Browse the repository at this point in the history
  • Loading branch information
@s-ludwig
s-ludwig committed Jul 31, 2014
1 parent 8e98962 commit dcc09b0
Showing 1 changed file with 503 additions and 0 deletions.
Loading

5 comments on commit dcc09b0

@etcimon
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's very practical, I was thinking it would be important to have a locking mechanism though, because modifying shared resources could go along the lines of:

(assuming the value is a serialized Json object with 10 members)

  • user 1 gets the value of key A
  • user 1 modifies the value of member 2 in key A
  • user 2 gets the value of key A
  • user 1 sets a new value for key A
  • user 2 modifies the value of member 10 of key A
  • user 2 writes the value of key A

So, the idea would be to embed lock()ing methods, in these redis objects, implemented with getSet(key~"_lock", 1) == 0?"OK":"wait..." which you can use like this

synchronized(redisVar){ // get and modify the value freely }

@s-ludwig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also planning to add an "idioms" module that implements some common patterns. A lock would fit in there I guess. The other thing is that transactions (MULTI) will also be supported at some point. This file here is just meant as a thin layer above what Redis provides for accessing fields.

@extrawurst
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@etcimon I am not sure if it is such a good idea to implement this in our API layer. For people not familiar with Redis it may appear to be implemented in the database, so accessing it using this locks from a multi process app is safe too where in fact it is not.

That being said it is actually sad that Redis does not support CAS ;)

UPDATE: i am just seeing they support it using this crazy Lua scripting facility now... XD so ... nvm

@etcimon
Copy link
Contributor

@etcimon etcimon commented on dcc09b0 Aug 1, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've thought a while about this and, transactions aren't a replacement for locking "GET redis->D, D modify, D->redis SET". Transactions would return the get result after the transaction is over, and setting afterwards is not going to be safe. It looks like GETSET is really meant for locking, with a ttl it's possible to avoid deadlocks too. I'm working on a drop-in replacement for Redis that will implement this locking mechanism in a "coordinator" instance, which also takes care of distributing the hashes in a cluster installation. In the meantime, this is the only acceptable workaround I can think of

@s-ludwig
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, transactions are not always a replacement, but can be in certain cases. But a general lock idiom implementation would provide full locking and means a much better separation of concerns than integrating it into the Redis type wrappers. It then also allows to lock multiple keys with a single lock.

Please sign in to comment.