citusdata / pg_shard

ATTENTION: pg_shard is superseded by Citus, its more powerful replacement
https://github.com/citusdata/citus
GNU Lesser General Public License v3.0
1.06k stars 63 forks source link

Fix memory leak on large unbounded queries #81

Closed onderkalaci closed 9 years ago

onderkalaci commented 9 years ago

This pull request aims to fix the memory leak created by the call to PostgreSQL's BuildTupleFromCStrings function. Temporary memory context is created for each call to BuildTupleFromCStrings and reseted after the call.

Fixes #77

jasonmp85 commented 9 years ago

From an email conversation:

On Mar 6, 2015, at 10:04 AM, Jason Petersen wrote:

can you clarify why you free the tuple even in the area guarded by its own context? Resetting the context clears all memory in it, right? So this is a no-op.

@onderkalaci replied:

On Mar 6, 2015, at 10:19 AM, Onder Kalaci wrote:

Oh yes that is no-op. Reset already frees it.

I'm going to update the comment slightly to mention I/O functions (another area that leaks like this mentions them as a culprit) and remove the extra call to heap_freetuple before merging (i.e. this is a :shipit:).

jasonmp85 commented 9 years ago

@jberkus — This should address the crash you had if you want to verify. We're going to follow up with the PostgreSQL hackers about memory leaks in I/O functions and BuildTupleFromCStrings, but we believe this particular issue is fixed.