Hi,
the attached patch might seem simple, but it touches a delicate and low-level part of the SSSD, so I'd like to request a detailed review.
I was notified by Bruce Aiesi of the Red Hat QE team that the sssd_nss process grew its memory footprint when stress-tested by logging in many users in parallel. I ran a couple of tests and then found out that the responder context grew because we never freed the internal Data Provider request. Attached is a patch that simple frees the "side request" after all the callbacks have been invoked.
I didn't see the memory growth anymore during my testing and I didn't see any other issues, either during my tests or with valgrind.
I hope I didn't miss any detail and the patch is OK.
On 10/24/2012 12:15 AM, Jakub Hrozek wrote:
Hi,
the attached patch might seem simple, but it touches a delicate and low-level part of the SSSD, so I'd like to request a detailed review.
I was notified by Bruce Aiesi of the Red Hat QE team that the sssd_nss process grew its memory footprint when stress-tested by logging in many users in parallel. I ran a couple of tests and then found out that the responder context grew because we never freed the internal Data Provider request. Attached is a patch that simple frees the "side request" after all the callbacks have been invoked.
I didn't see the memory growth anymore during my testing and I didn't see any other issues, either during my tests or with valgrind.
I hope I didn't miss any detail and the patch is OK.
Ack!
Good catch.
On Fri, Oct 26, 2012 at 01:46:14PM +0200, Pavel Březina wrote:
On 10/24/2012 12:15 AM, Jakub Hrozek wrote:
Hi,
the attached patch might seem simple, but it touches a delicate and low-level part of the SSSD, so I'd like to request a detailed review.
I was notified by Bruce Aiesi of the Red Hat QE team that the sssd_nss process grew its memory footprint when stress-tested by logging in many users in parallel. I ran a couple of tests and then found out that the responder context grew because we never freed the internal Data Provider request. Attached is a patch that simple frees the "side request" after all the callbacks have been invoked.
I didn't see the memory growth anymore during my testing and I didn't see any other issues, either during my tests or with valgrind.
I hope I didn't miss any detail and the patch is OK.
Ack!
Good catch.
Pushed to master.
On Mon, Oct 29, 2012 at 05:10:35PM +0100, Jakub Hrozek wrote:
On Fri, Oct 26, 2012 at 01:46:14PM +0200, Pavel Březina wrote:
On 10/24/2012 12:15 AM, Jakub Hrozek wrote:
Hi,
the attached patch might seem simple, but it touches a delicate and low-level part of the SSSD, so I'd like to request a detailed review.
I was notified by Bruce Aiesi of the Red Hat QE team that the sssd_nss process grew its memory footprint when stress-tested by logging in many users in parallel. I ran a couple of tests and then found out that the responder context grew because we never freed the internal Data Provider request. Attached is a patch that simple frees the "side request" after all the callbacks have been invoked.
I didn't see the memory growth anymore during my testing and I didn't see any other issues, either during my tests or with valgrind.
I hope I didn't miss any detail and the patch is OK.
Ack!
Good catch.
Pushed to master.
Also pushed to sssd-1-9
On Mon, Oct 29, 2012 at 05:20:46PM +0100, Jakub Hrozek wrote:
On Mon, Oct 29, 2012 at 05:10:35PM +0100, Jakub Hrozek wrote:
On Fri, Oct 26, 2012 at 01:46:14PM +0200, Pavel Březina wrote:
On 10/24/2012 12:15 AM, Jakub Hrozek wrote:
Hi,
the attached patch might seem simple, but it touches a delicate and low-level part of the SSSD, so I'd like to request a detailed review.
I was notified by Bruce Aiesi of the Red Hat QE team that the sssd_nss process grew its memory footprint when stress-tested by logging in many users in parallel. I ran a couple of tests and then found out that the responder context grew because we never freed the internal Data Provider request. Attached is a patch that simple frees the "side request" after all the callbacks have been invoked.
I didn't see the memory growth anymore during my testing and I didn't see any other issues, either during my tests or with valgrind.
I hope I didn't miss any detail and the patch is OK.
Ack!
Good catch.
Pushed to master.
Also pushed to sssd-1-9
Also pushed to sssd-1-8
sssd-devel@lists.fedorahosted.org
-
Jakub Hrozek -
Pavel Březina