Re: [PATCH] Stop leaking RA connections for each referenced svn:external in an update

On Wed, Oct 21, 2009 at 10:07:26AM -0500, Kevin Radke wrote:
> Subversion is leaking RA connections (TCP connections) for each
> svn:external referenced during a checkout/update. This patch fixes
> that by using the iter_pool in the loop instead of the global pool
> which is only cleaned up at process exit. With this applied, an svn
> update with thousands of externals no longer keeps thousands of TCP
> connections in the CLOSE_WAIT state on the client and FIN_WAIT_2 state
> on the server.
>
> This problem causes Windows XP SP3 to appear to lock up, the svn
> update to fail, and typically disconnects current network connections
> when Subversion uses more than around 500 connections. Linux appears
> to successfully complete the update, but uses a large amount of
> resources. (netstat -t | grep CLOSE_WAIT | wc -l)
>
> This is a simple patch, so I am attaching it inline. I believe it
> should be back ported to the 1.6.x release since that is where I
> observed the problem.

Nice, thanks!

> Kevin R.
>
> [[[
> Use iter_pool instead of pool to avoid leaving RA connections open
> until process exit for each svn:external.
>
> *subversion/libsvn_client/externals.c
> ]]]

Please use diff -u (or svn diff) to create patches in the future.

> *** subversion/libsvn_client/externals.c.orig 2009-10-21
> 09:45:19.000000000 -0500
> --- subversion/libsvn_client/externals.c 2009-10-21 09:45:45.000000000 -0500
> ***************
> *** 776,788 ****
> new_item->url, NULL,
> &(new_item->peg_revision),
> &(new_item->revision), ib->ctx,
> ! ib->pool));
>
> SVN_ERR(svn_ra_get_uuid2(ra_session, &ra_cache.repos_uuid, ib->pool));
> SVN_ERR(svn_ra_get_repos_root2(ra_session, &ra_cache.repos_root_url,
> ! ib->pool));
> SVN_ERR(svn_ra_check_path(ra_session, "", ra_cache.ra_revnum, &kind,
> ! ib->pool));
>
> if (svn_node_none == kind)
> return svn_error_createf(SVN_ERR_RA_ILLEGAL_URL, NULL,
> --- 776,788 ----
> new_item->url, NULL,
> &(new_item->peg_revision),
> &(new_item->revision), ib->ctx,
> ! ib->iter_pool));
>
> SVN_ERR(svn_ra_get_uuid2(ra_session, &ra_cache.repos_uuid, ib->pool));

Why not also put the repos_uuid into the iterpool?

Stefan

> SVN_ERR(svn_ra_get_repos_root2(ra_session, &ra_cache.repos_root_url,
> ! ib->iter_pool));
> SVN_ERR(svn_ra_check_path(ra_session, "", ra_cache.ra_revnum, &kind,
> ! ib->iter_pool));
>
> if (svn_node_none == kind)
> return svn_error_createf(SVN_ERR_RA_ILLEGAL_URL, NULL,
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2409821

-- 
printf("Eh???/n");
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2409824