This is a fix for a memory leak in the streaming folds code that emerged in stress testing the query path yesterday. Leveldb iterator was being allocated but not freed in the range scan.
I've made two changes in this PR:
1) I've fixed the obvious iterator leak by wrapping it in auto_ptr. This will delete the iterator when it goes out of scope, either on error or on exit
2) I've identified another possible rare memory leak, which could occur if no further keys matched after a batch was sent (because the enif_release_binary() call was keyed to the value of out_offset, which is reset each time a batch is sent). This is now explicitly marked as allocated when enif_alloc_binary() is called, and released whenever allocated.
I have tested the TS1.0 release code with these changes, and summarize them below:
Note that the original tests had only probed the iteration code (exercised whenever a range scan is executed against TS primary key conditions), and not the filtering code that I added for secondary key searches. The summary plot above include tests with:
and for completeness, since none of this has previsouly been stress-tested, I've tested the put path:
TS put only (blue line)
I see no evidence for significant residual leaks on the query path, and although the put path is much less stable (presumably because it is dominated by garbage collection of erlang terms), there is no obvious linear trend with time
paulplace
commented
8 years ago
… leak
This is a fix for a memory leak in the streaming folds code that emerged in stress testing the query path yesterday. Leveldb iterator was being allocated but not freed in the range scan.
I've made two changes in this PR:
1) I've fixed the obvious iterator leak by wrapping it in auto_ptr. This will delete the iterator when it goes out of scope, either on error or on exit
2) I've identified another possible rare memory leak, which could occur if no further keys matched after a batch was sent (because the enif_release_binary() call was keyed to the value of out_offset, which is reset each time a batch is sent). This is now explicitly marked as allocated when enif_alloc_binary() is called, and released whenever allocated.
I have tested the TS1.0 release code with these changes, and summarize them below:
Note that the original tests had only probed the iteration code (exercised whenever a range scan is executed against TS primary key conditions), and not the filtering code that I added for secondary key searches. The summary plot above include tests with:
and for completeness, since none of this has previsouly been stress-tested, I've tested the put path:
I see no evidence for significant residual leaks on the query path, and although the put path is much less stable (presumably because it is dominated by garbage collection of erlang terms), there is no obvious linear trend with time