Cache produce a memor leak

[geisbruch]

Hi, first of all, It's an excelenet module.

We've found that the cache used into the library could produce a memory leak if the amount of keys it's too long and no ring modifications occurs, because the cache var is filled into the line 161

return this.cache[key] = node

and it's clear only when a ring change occurs, may be will be a good idea change this cache by an lru-cache. At this moment we've fork and change the library to don't use the cache because we found a leak produced into our application by this issue.

If you are ok, we can change the cache objet to be an lru-cache and do the pull request.

Thanks. Gabriel.

[3rd-Eden]

Why not kill the cache when a ring change occures? Wouldn't that just be an easier fix?

[geisbruch]

The problem Isn't a change on the ring, the problem it's if you don't change the ring, the cache object could grow forever

[marcosnils]

Arnout,

I'm experiencing a similar behavior over here. I have an application with constantly queries the ring with different keys, which results in hashring cache getting filled quite fast and that makes node to perform Full GC very often.

The result is that you end up getting performance issues due to Full GC being performed against a huge key list.

I agree with Gabriel that it may be better to cap the cache size and implement some sort of LRU to expire unused keys and stabilize nodejs heap.

Regards,

Marcos.

[3rd-Eden]

@marcosnils

Thanks for the extra feedback, really appreciate it

@geisbruch I'll be more than happy to get LRU for caching.

[3rd-Eden]

Btw, accepting pull requests for this ;)

[geisbruch]

Fixed in Pull Request #9