dial-once/node-cache-manager-redis

Support for HSET/HGET feature

Opened this issue · 11 comments

Deleted user commented

To my knowledge, the HSET/HGET will have better performance when there are many keys share the prefix, just like this:

cache:user:session:1
cache:user:session:2

My suggestion is to store the everything after the last / symbol as the HSET/HGET key. For example, we should turn:

GET cache:user:session/1

to:

HGET cache:user:session 1

automatically.

If you like this idea, I will push you a PR.

Deleted user commented

And also, this is a compatible solution for other cache-manager store implementations.

mrister commented

@kcauchy I think It is good Idea, maybe you could do a PR for us to check out the implementation, what do you think @PuKoren ?

PuKoren commented

Agree, looks good!

Deleted user commented

It seems not able to be done.

There are two problems:

  1. ttl is not supported for redis HGET/HSET. Which means I have to disable it for keys as hash table entries. You can't set ttl for entire hash table, that will "expire" every entries.
  2. keys are not supported.
    • Redis provides HKEYS, but no pattern supported.
    • If you try to get cache:session:*, there could be entries like cache:session:db/1 and cache:session:user:1, then you must get wrong values for keys commands, because all other keys under cache:session:db will be ignored. So the only correct solution is to run KEYS first, then run every entry HKEYS (because we don't know whether it is a hash table or not.) That is totally unbearable for performance.

Seems that is a design problem for node-cache-manager.

jeff-kilbride commented

Automatically converting the existing SET/GET commands to HSET/HGET is not a good idea. HSET/HGET are associated with the hash data type in redis and most commonly represent objects with field-value pairs:

> hmset user:1000 username antirez birthyear 1977 verified 1
OK
> hget user:1000 username
"antirez"
> hget user:1000 birthyear
"1977"
> hgetall user:1000
1) "username"
2) "antirez"
3) "birthyear"
4) "1977"
5) "verified"
6) "1"

This is not a good option for a simple key/value store like cache-manager for the reasons noted above -- no ttl / expires command for individual values and no easy way to scan the keys.

To support the hash data type, as well as the other data types redis offers, it would be better to have something like a raw method that accepted the name of a redis command and it's parameters (possibly as an array), and just passed them along to the underlying connection.

redisCache.raw(
  'hmset', 
  ['user:1000', 'username', 'antirez', 'birthyear', '1977', 'verified', '1'],
  function(result) {
    // result is 'OK'
    redisCache.raw('hgetall', 'user:1000', function(result) {
      // result is the object returned by node_redis
      // { username: 'antirez', birthyear: '1977', verified: '1' }
    });
  });
);

In this way, we could support virtually all of the underlying redis commands. However, the user would be responsible for ensuring the command and parameters were correct, and for knowing the return value.

  • 👍1
mrister commented

Very nice observation by @jeff-kilbride, however even us when we use this module we more frequently store objects then we do simple string values, so It might be worth supporting out of the box, or make it configurable, I very much like the idea with the raw approach for flexibility of usages.
Some more reading: https://stackoverflow.com/questions/13557075/redis-set-vs-hash

mrister commented

@kcauchy still interested in doing the PR ?

Deleted user commented
mrister commented

@kcauchy. Ok thanks, might you show us this system if it is open source? :-)

Deleted user commented

Sure...

mrister commented

I'll reopen this in the meantime for further discussion, @dial-once/developers thoughts?