Hi Nathan and Lukas,
I discovered this memory leak in pmsubsys during my testing which
collecting metrics at 1 second interval. After couple of days, I can see
RSS of python process increase from 16M to 30M+. From code inspection,
pmFreeResult is not get called to free memory. This fix seems obvious so I
didn¹t create a test for this.
This week, I found out this fix introduced a bug which may corrupt the
memory and reset timestamp for diff calculation. I had a fix ready and
tested this fix for couple of days to make sure its working. Basically, I
use copy.deepcopy to return a copy of timestamp before calling
pmFreeResult. I will submit this fix today.
Cheers,
Marc
On 7/16/15, 3:44 AM, "Nathan Scott" <nathans@xxxxxxxxxx> wrote:
>Hi Lukas,
>
>----- Original Message -----
>> [...]
>> Marc, could you perhaps elaborate a little bit more on the memory leak
>> you were seeing? How did you identify it? Was there a testcase
>> submitted with the patch at all? (Mark, maybe you have a pointer to
>> this? I haven't see any in the tree or git logs).
>
>There isn't one - I looked into making a valgrind-based test, but there's
>overwhelming memcheck noise coming out of python itself that made me back
>slowly away. Probably we should go back to that at some point, but since
>its in pmsubsys.py which is pretty much deprecated now, I figured it was
>OK & seemed an "obvious" fix. Guess not.
>
>> I'd be happy to create an archived based testcase for this moving
>> forward (once we have the fix), to verify the values we're seeing out of
>> pmcollectl make sense. However, until the time being would it make
>> senes to revert this patch until we have a fix?
>
>Yep, please send a revert through while its considered some more. If we
>find someone to convert pmcollectl over to pcp.pmcc then we could start
>deprecating pmsubsys a bit more formally.
>
>cheers.
>
>--
>Nathan
|