multi_caching checking all caches in series, even if value is found in first cache about node-cache-manager HOT 12 CLOSED

Yeah, it's possible that that could be removed. I can't remember why it's there honestly. I doubt it's a big performance hit, especially if redis, etc. is taking a few milliseconds anyway.

from node-cache-manager.

I probably have a special use case that can be found here:
dial-once/node-cache-manager-redis#19

In my testing it effectively doubles the time it takes to get something from the cache, but that's because my multi cache is a cluster of redis machines behind a load balancer, and then the master of that cluster. This way I can write to the master, but read from any of the machines behind the load balancer.

(BTW, thanks for the quick response!)

from node-cache-manager.

Are you using the neverCache function (or equivalent) as your isCacheableValue function?

from node-cache-manager.

Yes, I am for the first cache in my multiCache.

My intention is to check the load balancer first, break out of the search for the item, and then write it only to the master node.

I raised a pull request:
#42

from node-cache-manager.

Oh, the reason we use isCacheableValue in that spot is because if, for example, null is a cacheable value, and it's found in the first cache, we don't want to keep checking the other caches. Instead, we want to return null right away.

from node-cache-manager.

That use case makes sense, but if one is caching larger values (like a serialized object) in the cache, wouldn't you want to return that as soon as it's found instead of looking for it again?

The check for null would be really fast, even if it has to look for it in a different cache.

I can see the value of what you're saying, though. Maybe we need another function to determine if the null is a valid value?

from node-cache-manager.

Hmm.. I'm probably not following completely. The intention is that if a value is found, like a large serialized object, it should return it immediately and not look for it in other caches. In that case your isCacheableValue function could be function(value) { return !!value; } or something like that.

from node-cache-manager.

OK, I think I see what you're saying now. Not sure how to get around that, other than perhaps having two functions, isCacheableValue and isReturnableValue. By default isReturnableValue could just be set to isCacheableValue. Not sure if that solves anything though.

from node-cache-manager.

Yes, that is what I was suggesting. I'll take a stab at it and update my PR.

from node-cache-manager.

@BryanDonovan finally got around to getting those unit tests in.

from node-cache-manager.

Sorry @3choBoomer -- I didn't see your comment here. Can you merge the latest from upstream/master and add the same isReturnableValue logic to lib/caching.js as well please? Then I think it's good to go.

from node-cache-manager.

Fixes by #203

from node-cache-manager.