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).
On Thu, Sep 03, 2015 at 12:48:20PM +0200, 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:
- Periodic task -- periodically (1 hour?) store talloc full report into a
file.
Generate report on signal.
Generate report on D-Bus method.
Provide a tool that would do 2) or 3).
I personally favor 1).
Do you plan to use a new debug level to avoid the cost associated with walking the talloc hierarchy and printing the output? This debug aid must be free of cost for anyone not using it..
On 09/03/2015 12:56 PM, Jakub Hrozek wrote:
On Thu, Sep 03, 2015 at 12:48:20PM +0200, 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:
- Periodic task -- periodically (1 hour?) store talloc full report into a
file.
+1 for periodic task
Generate report on signal.
Generate report on D-Bus method.
Provide a tool that would do 2) or 3).
I personally favor 1).
Do you plan to use a new debug level to avoid the cost associated with walking the talloc hierarchy and printing the output? This debug aid must be free of cost for anyone not using it..
New debug level is not bad, but I would personally prefer completely new option. Something like: talloc_report_period = 60
That would start the periodic task every 60 minutes. 0 would be a special (and default) value, which would disable this feature.
Michal
On Thu, Sep 03, 2015 at 12:56:38PM +0200, Jakub Hrozek wrote:
On Thu, Sep 03, 2015 at 12:48:20PM +0200, 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:
- Periodic task -- periodically (1 hour?) store talloc full report into a
file.
Generate report on signal.
Generate report on D-Bus method.
Provide a tool that would do 2) or 3).
I personally favor 1).
I would start with a configurable periodically task as well. An option like debug_talloc_report_interval which can be use in any section like debug_level to individually switch it on or off for the different processes. The default should be 0 which means disabled.
Later on when some general sssctl tool is available we can add a manual trigger as well.
Do you plan to use a new debug level to avoid the cost associated with walking the talloc hierarchy and printing the output? This debug aid must be free of cost for anyone not using it..
Since it is always generated on demand either by configuring a periodic task or manually I think it will have no extra costs and a new debug level is not strictly needed. Nevertheless it should still be easy to filter those messages.
bye, Sumit
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
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.
+1
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:
- Periodic task -- periodically (1 hour?) store talloc full report into a file.
Some leaks might take too much memory to last more than a couple of minutes. Some "semi" leaks could be freed when their parent context is freed - but they should be freed way earlier on their own.
- Generate report on signal.
Do you have any signal in mind? I mean can we abuse some "standard" signals? Using SIGUSR works nice, alas we can steal it...unless we add new configuration option?
Generate report on D-Bus method.
Provide a tool that would do 2) or 3).
I personally favor 1). _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Could we use inotify to handle creation of some file which would signal to generate talloc_report? Too weird?
On 09/03/2015 01:04 PM, Pavel Reichl 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.
+1
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:
- Periodic task -- periodically (1 hour?) store talloc full report
into a file.
Some leaks might take too much memory to last more than a couple of minutes. Some "semi" leaks could be freed when their parent context is freed - but they should be freed way earlier on their own.
- Generate report on signal.
Do you have any signal in mind? I mean can we abuse some "standard" signals? Using SIGUSR works nice, alas we can steal it...unless we add new configuration option?
Well, it is possible to abuse SIGUSR signal or some other, but I would prefer not to do it anymore, since we have nice D-Bus support.
Converting our signal handlers to D-Bus is something I'd like to talk about next week in person.
Generate report on D-Bus method.
Provide a tool that would do 2) or 3).
I personally favor 1). _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Could we use inotify to handle creation of some file which would signal to generate talloc_report? Too weird? _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On 09/03/2015 01:11 PM, Pavel Březina wrote:
On 09/03/2015 01:04 PM, Pavel Reichl 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.
+1
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:
- Periodic task -- periodically (1 hour?) store talloc full report
into a file.
Some leaks might take too much memory to last more than a couple of minutes. Some "semi" leaks could be freed when their parent context is freed - but they should be freed way earlier on their own.
- Generate report on signal.
Do you have any signal in mind? I mean can we abuse some "standard" signals? Using SIGUSR works nice, alas we can steal it...unless we add new configuration option?
Well, it is possible to abuse SIGUSR signal or some other, but I would prefer not to do it anymore, since we have nice D-Bus support.
Converting our signal handlers to D-Bus is something I'd like to talk about next week in person.
OK, looking forward to that.
Generate report on D-Bus method.
Provide a tool that would do 2) or 3).
I personally favor 1). _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
Could we use inotify to handle creation of some file which would signal to generate talloc_report? Too weird? _______________________________________________ sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
On Thu, Sep 03, 2015 at 01:11:23PM +0200, Pavel Březina wrote:
On 09/03/2015 01:04 PM, Pavel Reichl 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.
+1
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:
- Periodic task -- periodically (1 hour?) store talloc full report
into a file.
Some leaks might take too much memory to last more than a couple of minutes. Some "semi" leaks could be freed when their parent context is freed - but they should be freed way earlier on their own.
- Generate report on signal.
Do you have any signal in mind? I mean can we abuse some "standard" signals? Using SIGUSR works nice, alas we can steal it...unless we add new configuration option?
Well, it is possible to abuse SIGUSR signal or some other, but I would prefer not to do it anymore, since we have nice D-Bus support.
But do you think the workflow would be nicely digestable by admins?
The situation we're trying to cover would be admin who notices sssd processes consume too much memory. We should have a nicely documented approach to help this admin..
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:
- Periodic task -- periodically (1 hour?) store talloc full report into
a file.
Generate report on signal.
Generate report on D-Bus method.
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.
2) Periodically obtain talloc full report
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.
I think it is OK to override the talloc report on each period so the file can be named just $process.talloc.
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:
- Periodic task -- periodically (1 hour?) store talloc full report into
a file.
Generate report on signal.
Generate report on D-Bus method.
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:
- Create a new option debug_talloc_report_interval that defaults to 0 (disabled). This option says how often SSSD captures talloc report.
+1
- Periodically obtain talloc full report
+1
- 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.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
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:
- Periodic task -- periodically (1 hour?) store talloc full report into
a file.
Generate report on signal.
Generate report on D-Bus method.
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:
- Create a new option debug_talloc_report_interval that defaults to 0 (disabled). This option says how often SSSD captures talloc report.
+1
- Periodically obtain talloc full report
+1
- 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.
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:
- Periodic task -- periodically (1 hour?) store talloc full report into
a file.
Generate report on signal.
Generate report on D-Bus method.
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:
- Create a new option debug_talloc_report_interval that defaults to 0 (disabled). This option says how often SSSD captures talloc report.
+1
- Periodically obtain talloc full report
+1
- 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.
sssd-devel mailing list sssd-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
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:
- Periodic task -- periodically (1 hour?) store talloc full report
into a file.
Generate report on signal.
Generate report on D-Bus method.
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:
- Create a new option debug_talloc_report_interval that defaults to
0 (disabled). This option says how often SSSD captures talloc report.
+1
- Periodically obtain talloc full report
+1
- 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'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.
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:
- Periodic task -- periodically (1 hour?) store talloc full report
into a file.
Generate report on signal.
Generate report on D-Bus method.
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:
- Create a new option debug_talloc_report_interval that defaults to
0 (disabled). This option says how often SSSD captures talloc report.
+1
- Periodically obtain talloc full report
+1
- 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:
Administrator sees a memory leak in SSSD. He/she will enable talloc reports and restarts SSSD. Then waits for the leak to occur again.
Administrator sees a memory leak in SSSD. He/she will enable talloc reports and signals SSSD to enable it on the fly.
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@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/sssd-devel
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Michal Židek -
Pavel Březina -
Pavel Reichl -
Sumit Bose