You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by "Hyrum K. Wright" <hy...@mail.utexas.edu> on 2009/03/16 02:40:12 UTC
Re: svn commit: r36583 - trunk/subversion/libsvn_ra_serf
Yay! This fixes the svnsync errors I was having awhile back, right?
-Hyrum
On Mar 15, 2009, at 6:36 PM, Lieven Govaerts wrote:
> Author: lgo
> Date: Sun Mar 15 16:36:14 2009
> New Revision: 36583
>
> Log:
> ra_serf: Use a subpool for the commit part in replaying a revision,
> that can be destroyed right after the commit is finished.
> This should reduce the number of open file handles while syncing.
>
> The reason this is needed is because the commit editor was using the
> same per-revision pool as the request to the master server. This pool
> is destroyed only after the whole request is handled.
> Now, ra_serf keeps the pipeline of requests to the server constantly
> filled with up to 100 requests (50 PROPFIND and 50 REPORT). If serf
> can not fill the network fast enough, it will never return from
> serf_context_run. hence there can be up to 50 per-revision pools
> in memory.
>
> Found by: hwright
>
> * subversion/libsvn_ra_serf/replay.c
> (replay_context_t): Add dst_rev_pool. Rename pool to src_rev_pool.
> (push_state): Use replay_ctx->dst_rev_pool for all calls to the
> commit editor.
> (start_replay): At the start of parsing the replay report, create a
> new pool to use with the commit editor, and use it where needed.
> (end_replay): Destroy the commit editor pool right after use.
> (create_replay_body): Rename ctx->pool to ctx->src_rev_pool ...
> (svn_ra_serf__replay): ... here too ...
> (svn_ra_serf__replay_range): ... and here.
>
> Modified:
> trunk/subversion/libsvn_ra_serf/replay.c
>
> Modified: trunk/subversion/libsvn_ra_serf/replay.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_ra_serf/replay.c?pathrev=36583&r1=36582&r2=36583
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/subversion/libsvn_ra_serf/replay.c Sun Mar 15 14:54:32
> 2009 (r36582)
> +++ trunk/subversion/libsvn_ra_serf/replay.c Sun Mar 15 16:36:14
> 2009 (r36583)
> @@ -86,7 +86,8 @@ typedef struct {
> } prop_info_t;
>
> typedef struct {
> - apr_pool_t *pool;
> + apr_pool_t *src_rev_pool;
> + apr_pool_t *dst_rev_pool;
>
> /* Are we done fetching this file? */
> svn_boolean_t done;
> @@ -134,9 +135,9 @@ push_state(svn_ra_serf__xml_parser_t *pa
> {
> replay_info_t *info;
>
> - info = apr_palloc(parser->state->pool, sizeof(*info));
> + info = apr_palloc(replay_ctx->dst_rev_pool, sizeof(*info));
>
> - info->pool = parser->state->pool;
> + info->pool = replay_ctx->dst_rev_pool;
> info->parent = parser->state->private;
> info->baton = NULL;
> info->stream = NULL;
> @@ -147,9 +148,9 @@ push_state(svn_ra_serf__xml_parser_t *pa
> {
> prop_info_t *info;
>
> - info = apr_pcalloc(parser->state->pool, sizeof(*info));
> + info = apr_pcalloc(replay_ctx->dst_rev_pool, sizeof(*info));
>
> - info->pool = parser->state->pool;
> + info->pool = replay_ctx->dst_rev_pool;
> info->parent = parser->state->private;
>
> parser->state->private = info;
> @@ -173,17 +174,19 @@ start_replay(svn_ra_serf__xml_parser_t *
> strcmp(name.name, "editor-report") == 0)
> {
> push_state(parser, ctx, REPORT);
> - ctx->props = apr_hash_make(ctx->pool);
>
> + /* Create a pool for the commit editor. */
> + ctx->dst_rev_pool = svn_pool_create(ctx->src_rev_pool);
> + ctx->props = apr_hash_make(ctx->dst_rev_pool);
> svn_ra_serf__walk_all_props(ctx->revs_props, ctx->report_target,
> ctx->revision,
> svn_ra_serf__set_bare_props,
> - ctx->props, ctx->pool);
> + ctx->props, ctx->dst_rev_pool);
> if (ctx->revstart_func)
> {
> SVN_ERR(ctx->revstart_func(ctx->revision, ctx->replay_baton,
> &ctx->editor, &ctx->editor_baton,
> ctx->props,
> - ctx->pool));
> + ctx->dst_rev_pool));
> }
> }
> else if (state == REPORT &&
> @@ -200,7 +203,7 @@ start_replay(svn_ra_serf__xml_parser_t *
>
> SVN_ERR(ctx->editor->set_target_revision(ctx->editor_baton,
> SVN_STR_TO_REV(rev),
> - parser->state->pool));
> + ctx->dst_rev_pool));
> }
> else if (state == REPORT &&
> strcmp(name.name, "open-root") == 0)
> @@ -219,7 +222,8 @@ start_replay(svn_ra_serf__xml_parser_t *
> info = push_state(parser, ctx, OPEN_DIR);
>
> SVN_ERR(ctx->editor->open_root(ctx->editor_baton,
> - SVN_STR_TO_REV(rev), parser-
> >state->pool,
> + SVN_STR_TO_REV(rev),
> + ctx->dst_rev_pool,
> &info->baton));
> }
> else if ((state == OPEN_DIR || state == ADD_DIR) &&
> @@ -244,7 +248,7 @@ start_replay(svn_ra_serf__xml_parser_t *
> info = push_state(parser, ctx, DELETE_ENTRY);
>
> SVN_ERR(ctx->editor->delete_entry(file_name,
> SVN_STR_TO_REV(rev),
> - info->baton, parser->state-
> >pool));
> + info->baton, ctx-
> >dst_rev_pool));
>
> svn_ra_serf__xml_pop_state(parser);
> }
> @@ -271,7 +275,7 @@ start_replay(svn_ra_serf__xml_parser_t *
>
> SVN_ERR(ctx->editor->open_directory(dir_name, info->parent-
> >baton,
> SVN_STR_TO_REV(rev),
> - parser->state->pool,
> &info->baton));
> + ctx->dst_rev_pool, &info-
> >baton));
> }
> else if ((state == OPEN_DIR || state == ADD_DIR) &&
> strcmp(name.name, "add-directory") == 0)
> @@ -298,14 +302,14 @@ start_replay(svn_ra_serf__xml_parser_t *
>
> SVN_ERR(ctx->editor->add_directory(dir_name, info->parent-
> >baton,
> copyfrom, rev,
> - parser->state->pool, &info-
> >baton));
> + ctx->dst_rev_pool, &info-
> >baton));
> }
> else if ((state == OPEN_DIR || state == ADD_DIR) &&
> strcmp(name.name, "close-directory") == 0)
> {
> replay_info_t *info = parser->state->private;
>
> - SVN_ERR(ctx->editor->close_directory(info->baton, parser-
> >state->pool));
> + SVN_ERR(ctx->editor->close_directory(info->baton, ctx-
> >dst_rev_pool));
>
> svn_ra_serf__xml_pop_state(parser);
> }
> @@ -332,7 +336,7 @@ start_replay(svn_ra_serf__xml_parser_t *
>
> SVN_ERR(ctx->editor->open_file(file_name, info->parent->baton,
> SVN_STR_TO_REV(rev),
> - parser->state->pool, &info-
> >baton));
> + ctx->dst_rev_pool, &info-
> >baton));
> }
> else if ((state == OPEN_DIR || state == ADD_DIR) &&
> strcmp(name.name, "add-file") == 0)
> @@ -359,7 +363,7 @@ start_replay(svn_ra_serf__xml_parser_t *
>
> SVN_ERR(ctx->editor->add_file(file_name, info->parent->baton,
> copyfrom, rev,
> - parser->state->pool, &info-
> >baton));
> + ctx->dst_rev_pool, &info-
> >baton));
> }
> else if ((state == OPEN_FILE || state == ADD_FILE) &&
> strcmp(name.name, "apply-textdelta") == 0)
> @@ -396,7 +400,7 @@ start_replay(svn_ra_serf__xml_parser_t *
> checksum = svn_xml_get_attr_value("checksum", attrs);
>
> SVN_ERR(ctx->editor->close_file(info->baton, checksum,
> - parser->state->pool));
> + ctx->dst_rev_pool));
>
> svn_ra_serf__xml_pop_state(parser);
> }
> @@ -418,7 +422,7 @@ start_replay(svn_ra_serf__xml_parser_t *
>
> info = push_state(parser, ctx, CHANGE_PROP);
>
> - info->name = apr_pstrdup(parser->state->pool, prop_name);
> + info->name = apr_pstrdup(ctx->dst_rev_pool, prop_name);
>
> if (svn_xml_get_attr_value("del", attrs))
> info->del_prop = TRUE;
> @@ -456,8 +460,9 @@ end_replay(svn_ra_serf__xml_parser_t *pa
> SVN_ERR(ctx->revfinish_func(ctx->revision, ctx-
> >replay_baton,
> ctx->editor, ctx->editor_baton,
> ctx->props,
> - ctx->pool));
> + ctx->dst_rev_pool));
> }
> + svn_pool_destroy(ctx->dst_rev_pool);
> }
> else if (state == OPEN_DIR && strcmp(name.name, "open-directory")
> == 0)
> {
> @@ -505,7 +510,7 @@ end_replay(svn_ra_serf__xml_parser_t *pa
> tmp_prop.data = info->data;
> tmp_prop.len = info->len;
>
> - prop_val = svn_base64_decode_string(&tmp_prop, parser-
> >state->pool);
> + prop_val = svn_base64_decode_string(&tmp_prop, ctx-
> >dst_rev_pool);
> }
>
> SVN_ERR(info->change(info->parent->baton, info->name, prop_val,
> @@ -570,16 +575,16 @@ create_replay_body(void *baton,
>
> svn_ra_serf__add_tag_buckets(body_bkt,
> "S:revision",
> - apr_ltoa(ctx->pool, ctx->revision),
> + apr_ltoa(ctx->src_rev_pool, ctx-
> >revision),
> alloc);
> svn_ra_serf__add_tag_buckets(body_bkt,
> "S:low-water-mark",
> - apr_ltoa(ctx->pool, ctx-
> >low_water_mark),
> + apr_ltoa(ctx->src_rev_pool, ctx-
> >low_water_mark),
> alloc);
>
> svn_ra_serf__add_tag_buckets(body_bkt,
> "S:send-deltas",
> - apr_ltoa(ctx->pool, ctx->send_deltas),
> + apr_ltoa(ctx->src_rev_pool, ctx-
> >send_deltas),
> alloc);
>
> svn_ra_serf__add_close_tag_buckets(body_bkt, alloc, "S:replay-
> report");
> @@ -610,7 +615,7 @@ svn_ra_serf__replay(svn_ra_session_t *ra
> SVN_ERR(svn_ra_serf__report_resource(&report_target, session,
> NULL, pool));
>
> replay_ctx = apr_pcalloc(pool, sizeof(*replay_ctx));
> - replay_ctx->pool = pool;
> + replay_ctx->src_rev_pool = pool;
> replay_ctx->editor = editor;
> replay_ctx->editor_baton = edit_baton;
> replay_ctx->done = FALSE;
> @@ -618,7 +623,7 @@ svn_ra_serf__replay(svn_ra_session_t *ra
> replay_ctx->low_water_mark = low_water_mark;
> replay_ctx->send_deltas = send_deltas;
> replay_ctx->report_target = report_target;
> - replay_ctx->revs_props = apr_hash_make(replay_ctx->pool);
> + replay_ctx->revs_props = apr_hash_make(replay_ctx->src_rev_pool);
>
> handler = apr_pcalloc(pool, sizeof(*handler));
>
> @@ -729,7 +734,7 @@ svn_ra_serf__replay_range(svn_ra_session
> apr_pool_t *ctx_pool = svn_pool_create(pool);
>
> replay_ctx = apr_pcalloc(ctx_pool, sizeof(*replay_ctx));
> - replay_ctx->pool = ctx_pool;
> + replay_ctx->src_rev_pool = ctx_pool;
> replay_ctx->revstart_func = revstart_func;
> replay_ctx->revfinish_func = revfinish_func;
> replay_ctx->replay_baton = replay_baton;
> @@ -740,15 +745,16 @@ svn_ra_serf__replay_range(svn_ra_session
> replay_ctx->done_item.data = replay_ctx;
> /* Request all properties of a certain revision. */
> replay_ctx->report_target = report_target;
> - replay_ctx->revs_props = apr_hash_make(replay_ctx->pool);
> + replay_ctx->revs_props = apr_hash_make(replay_ctx-
> >src_rev_pool);
> SVN_ERR(svn_ra_serf__deliver_props(&prop_ctx,
> replay_ctx->revs_props,
> session,
> session->conns[0],
> report_target,
> rev, "0", all_props,
> - TRUE, NULL, replay_ctx-
> >pool));
> + TRUE, NULL,
> + replay_ctx-
> >src_rev_pool));
>
> /* Send the replay report request. */
> - handler = apr_pcalloc(replay_ctx->pool, sizeof(*handler));
> + handler = apr_pcalloc(replay_ctx->src_rev_pool,
> sizeof(*handler));
>
> handler->method = "REPORT";
> handler->path = session->repos_url_str;
> @@ -757,7 +763,8 @@ svn_ra_serf__replay_range(svn_ra_session
> handler->conn = session->conns[0];
> handler->session = session;
>
> - parser_ctx = apr_pcalloc(replay_ctx->pool,
> sizeof(*parser_ctx));
> + parser_ctx = apr_pcalloc(replay_ctx->src_rev_pool,
> + sizeof(*parser_ctx));
>
> /* Setup the XML parser context.
> Because we have not one but a list of requests, the
> 'done' property
> @@ -766,7 +773,7 @@ svn_ra_serf__replay_range(svn_ra_session
> done_item to done_list, so by keeping track of the
> state of
> done_list we know how many requests have been handled
> completely.
> */
> - parser_ctx->pool = replay_ctx->pool;
> + parser_ctx->pool = replay_ctx->src_rev_pool;
> parser_ctx->user_data = replay_ctx;
> parser_ctx->start = start_replay;
> parser_ctx->end = end_replay;
> @@ -816,7 +823,7 @@ svn_ra_serf__replay_range(svn_ra_session
> }
>
> done_list = done_list->next;
> - svn_pool_destroy(ctx->pool);
> + svn_pool_destroy(ctx->src_rev_pool);
> active_reports--;
> }
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=1329817
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1330574
Re: svn commit: r36583 - trunk/subversion/libsvn_ra_serf
Posted by Lieven Govaerts <sv...@mobsol.be>.
Hyrum K. Wright wrote:
> Yay! This fixes the svnsync errors I was having awhile back, right?
>
It does for me! The nr. of open file handles is appr. 80 now, same
number is over ra_neon.
Lieven
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1331457