Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak in ScriptEvaluate(string) #617

Closed
Funbit opened this issue Apr 25, 2017 · 6 comments
Closed

Memory leak in ScriptEvaluate(string) #617

Funbit opened this issue Apr 25, 2017 · 6 comments

Comments

@Funbit
Copy link

Funbit commented Apr 25, 2017

Hello,
First of all, I want to say that I've been using StackExchange.Redis package for a couple of years without any problems. However, recently, I started experiencing strange memory leaks. Usually, I always used prepared LUA scripts (via LuaScript.Prepare), but after some point I had to introduce some manually generated LUA scripts. The new code I'm using looks like this:

var builder = new StringBuilder("redis.call('HMSET',");
builder.Append(...); 
// build some script here and then run it as string:
_connectionMultiplexer.GetDatabase().ScriptEvaluate(builder.ToString());

After several days of running, the server memory goes above 1GB (usually it stayed around 200MB). I dumped the process a couple of times and result is the same: there are many strings (500000+) with reference count = 1 in a Hashtable bucket. I'm not sure why it's there and why it cannot be freed (the script is not prepared, so it shouldn't be cached, etc).

I have attached a couple of screenshots (from MemoScope 0.9.999).

Is there any way to fix/avoid such behavior?

PS. If you need more information please let me know.
PPS. I'm using the latest version of the NuGet package (1.2.1).

Total string count:
capture1
This is how strings look like (builder.ToString() output):
capture
Here is the reference map for each string:
capture2

@mgravell
Copy link
Collaborator

mgravell commented Apr 25, 2017 via email

@mgravell
Copy link
Collaborator

mgravell commented Apr 25, 2017

From the screenshot, it actually looks like an ideal case for parameterization - you can see https://redis.io/commands/eval for what this means in terms of the Lua - you just use KEYS[n] and ARGS[n] in the script to lookup values. The SE.Redis API exposes separate RedisKey[] keys and RedisValue[] values parameters on ScriptEvaluate that are mapped into KEYS and ARGS respectively.

As a minor note: HMSET is exposed directly via HashSet and HashSetAsync

@Funbit
Copy link
Author

Funbit commented Apr 25, 2017

Thanks for the quick response!
Generally speaking, it's not that simple. The script I'm trying to evaluate looks like this:

redis.call('HMSET','key1','hashkey1','NEWVALUE','hashkey2','NEWVALUE', ...); 
redis.call('DEL','keys:hashkey2', ...);

HMSET sets "NEWVALUE" for some hash keys AND at the same time (i.e. as a single atomic transaction) DEL deletes some other keys. I'm not sure whether it's doable using script parameterization.

Probably, I can achieve the same effect via _connectionMultiplexer.GetDatabase().CreateTransaction() (can I?) instead of LUA, but it will help only this time. There's a big chance that at some point I would need to perform a condition ("if"/"for" etc) and use some local variables, which can be done only via LUA scripting.
So I think it would be great to have a new NoScriptCache option anyway.

@mgravell
Copy link
Collaborator

mgravell commented May 8, 2017

Targeting 1.2.3, later today

@mgravell
Copy link
Collaborator

mgravell commented May 8, 2017

@Funbit
Copy link
Author

Funbit commented May 8, 2017

Thank you very much for the update!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants