On 10/01/2015 09:52 AM, Pavel Březina wrote:
On 09/29/2015 09:30 PM, Pavel Reichl wrote:
>
>
> On 09/29/2015 05:59 PM, Jakub Hrozek wrote:
>> On Tue, Sep 29, 2015 at 02:12:48PM +0200, Pavel Reichl wrote:
>>>
>>>
>>> On 09/29/2015 02:04 PM, Pavel Březina wrote:
>>>> On 09/03/2015 12:48 PM, Pavel Březina wrote:
>>>>> Hi,
>>>>> due to recent memory leak issues, I think it would be good to
>>>>> provide a
>>>>> built-in way to store talloc full report in a file. It proved to be
>>>>> very
>>>>> helpful in detection of the location where memory leak occurs, but
we
>>>>> always obtained it from custom built.
>>>>>
>>>>> I would very much like to write a patch, but I'd like to hear
your
>>>>> opinion on how it should be obtains. I have few ideas:
>>>>>
>>>>> 1) Periodic task -- periodically (1 hour?) store talloc full report
>>>>> into
>>>>> a file.
>>>>>
>>>>> 2) Generate report on signal.
>>>>>
>>>>> 3) Generate report on D-Bus method.
>>>>>
>>>>> 4) Provide a tool that would do 2) or 3).
>>>>>
>>>>> I personally favor 1).
>>>>
>>>> Hi,
>>>> I'd like to bring back this discussion and create a ticket from it.
>>>> I've gone through those mails again and I'd like to sum it up
here:
>>>>
>>>> 1) Create a new option debug_talloc_report_interval that defaults to
>>>> 0 (disabled). This option says how often SSSD captures talloc report.
>>> +1
>>>>
>>>> 2) Periodically obtain talloc full report
>>> +1
>>>>
>>>> 3) Store the report -- from the discussion it seems that you would
>>>> like to made the report as part of logs. I'd rather store it in a
>>>> separate file since in case of a memory leak it may get very large.
>>>> It would be also simpler to process, in my opinion.
>>> Definitely separate file.
>>> +1
>>>
>>>>
>>>> I think it is OK to override the talloc report on each period so the
>>>> file can be named just $process.talloc.
>>> It might show useful in future to have 'history' of talloc reports,
>>> but for first version overwriting is fine by me.
>>
>> We can later add a format for the new file maybe, which might include
>> some kind of timestamp that would automatically change over time,
>> providing unique filename. But if, from your experience debugging these
>> issues you think a single fine is enough, then fine by me.
>
> I think that single file is way better start then having to prepare and
> deliver special build every time somebody hits memory leak.
> IMO if time shows that it's not enough we can always add the support for
> the format as you proposed.
I think, it is important to define the use case here. So let's see if we are in
agreement.
We want to detect a memory leak with this. Memory leak is something that grows over time
so it is not necessary to have all reports, only the last one.
I agree that this is
mostly true and most common case. However, there are also temporally memory leaks - by
this I mean memory that is not freed as soon as possible but instead it's freed when
parent (or parent's parent...) context is freed. This can
be the source of unnecessary memory peaks and unfortunately it is detected by your user
case number 2) or the changes to detect it might be increased by storing more talloc
reports over time.
Still I believe that we should implement the easy solution described in prev posts as it
should help us with majority of memory leaks.
(I'm not saying it can't be convenient on some occasions, but
in the leaks I remember we didn't
need it.)
Use cases:
1) Administrator sees a memory leak in SSSD. He/she will enable talloc reports and
restarts SSSD. Then waits for the leak to occur again.
2) Administrator sees a memory leak in SSSD. He/she will enable talloc reports and
signals SSSD to enable it on the fly.
3) Talloc reports are enabled all the time and when memory leak is seen, we have them
ready.
I am aiming at 1 and 3. Having 2 would be convenient, but I'm not sure if it is
easily doable. I'll check it when I'll code. All of these use cases requires only
the last report.
_______________________________________________
sssd-devel mailing list
sssd-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/sssd-devel