node-cache-manager-redis icon indicating copy to clipboard operation
node-cache-manager-redis copied to clipboard

Support for HSET/HGET feature

Open ghost opened this issue 9 years ago • 11 comments

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.

ghost avatar Oct 02 '16 06:10 ghost

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

ghost avatar Oct 02 '16 06:10 ghost

@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 ?

mrister avatar Oct 03 '16 08:10 mrister

Agree, looks good!

PuKoren avatar Oct 03 '16 08:10 PuKoren

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.

ghost avatar Oct 19 '16 13:10 ghost

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.

jeff-kilbride avatar Aug 21 '17 08:08 jeff-kilbride

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 avatar Aug 21 '17 10:08 mrister

@kcauchy still interested in doing the PR ?

mrister avatar Aug 21 '17 10:08 mrister

sorry. Not involved any more. I made a more powerful cache system and support much more features than just hset/hget.

On 21 Aug 2017, at 18:36, Mihovil Rister [email protected] wrote:

@kcauchy still interested in doing the PR ?

― You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

ghost avatar Aug 21 '17 12:08 ghost

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

mrister avatar Aug 21 '17 12:08 mrister

Sure...

ghost avatar Aug 21 '17 12:08 ghost

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

mrister avatar Aug 21 '17 12:08 mrister