Re: svn commit: r1099044 - /subversion/trunk/subversion/libsvn_ra_serf/commit.c

[Hyrum K Wright <hy...@hyrumwright.org>]

On Tue, May 3, 2011 at 8:15 AM,  <ph...@apache.org> wrote:
> Author: philip
> Date: Tue May  3 13:14:59 2011
> New Revision: 1099044
>
> URL: http://svn.apache.org/viewvc?rev=1099044&view=rev
> Log:
> Commits that affect large numbers of files should work on systems that
> limit the number of files that can be open simultaneously.
>
> * subversion/libsvn_ra_serf/commit.c
>  (apply_textdelta): Don't rely on pool cleanup.
>  (close_file): Explicitly close file.
>
> Modified:
>    subversion/trunk/subversion/libsvn_ra_serf/commit.c
>
> Modified: subversion/trunk/subversion/libsvn_ra_serf/commit.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_serf/commit.c?rev=1099044&r1=1099043&r2=1099044&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_serf/commit.c (original)
> +++ subversion/trunk/subversion/libsvn_ra_serf/commit.c Tue May  3 13:14:59 2011
> @@ -1978,10 +1978,14 @@ apply_textdelta(void *file_baton,
>    * writing to a temporary file (ugh). A special svn stream serf bucket
>    * that returns EAGAIN until we receive the done call?  But, when
>    * would we run through the serf context?  Grr.
> +   *
> +   * ctx->pool is the same for all files in the commit that send a
> +   * textdelta so this file is explicitly closed in close_file to
> +   * avoid too many simultaneously open files.
>    */
>
>   SVN_ERR(svn_io_open_unique_file3(&ctx->svndiff, NULL, NULL,
> -                                   svn_io_file_del_on_pool_cleanup,
> +                                   svn_io_file_del_on_close,
>                                    ctx->pool, ctx->pool));

This isn't part of your change, but we don't need to use ctx->pool for
both result_pool and scratch_pool in the above function call.  Using
the function-provided pool as scratch_pool is sufficient.

-Hyrum

>   ctx->stream = svn_stream_create(ctx, pool);
> @@ -2140,6 +2144,9 @@ close_file(void *file_baton,
>         }
>     }
>
> +  if (ctx->svndiff)
> +    SVN_ERR(svn_io_file_close(ctx->svndiff, pool));
> +
>   /* If we had any prop changes, push them via PROPPATCH. */
>   if (apr_hash_count(ctx->changed_props) ||
>       apr_hash_count(ctx->removed_props))
>
>
>

Re: svn commit: r1099044 - /subversion/trunk/subversion/libsvn_ra_serf/commit.c

[Philip Martin <ph...@wandisco.com>]

Hyrum K Wright <hy...@hyrumwright.org> writes:

>>   SVN_ERR(svn_io_open_unique_file3(&ctx->svndiff, NULL, NULL,
>> -                                   svn_io_file_del_on_pool_cleanup,
>> +                                   svn_io_file_del_on_close,
>>                                    ctx->pool, ctx->pool));
>
> This isn't part of your change, but we don't need to use ctx->pool for
> both result_pool and scratch_pool in the above function call.  Using
> the function-provided pool as scratch_pool is sufficient.

Agreed; r1099066.

-- 
Philip