For efficiency reasons, the system has converted the large body of this message into an attachment.
attached mail follows:
Ha!
The patch looks good except that a few of the scratch_pool values are
not destroyed.
In theory, if a scratch_pool was passed "all the way down", then it
would hold the high-water mark of all allocated memory. Iteration
pools break that up into overlapping chunks of allocations. But
sometimes... yah, it is good to just chop up that chain of passing the
scratch pool and create/destroy one. In each case where you did that,
you picked a single string or a checksum out of the scratch_pool,
duplicated it for return, and blew away the pool. That seems like a
reasonable use: the memory in the pool is constructed/used/tossed in
return for a single value.
Your second approach, of passing body functions a result_pool, might
be nice to add as an option for future code. retry_txn2() would take a
result_pool, construct its own scratch_pool (as it does today), and
pass both into the body function (imo, as two args rather than one arg
plus one trail member). As you work on code, you could selectively
switch to txn2() usage for clarity and finer-grained memory usage.
Cheers,
-g
On Thu, Apr 9, 2009 at 10:20, C. Michael Pilato <cmpilato_at_collab.net> wrote:
> See attached. Here's the log message summary:
>
> {{{
> Attempt to button down the BDB backend's memory usage by allowing trail
> producers to tell that subsystem to discard all memory associated with
> the trail.
>
> Most of the time, this pool usage waste isn't a problem (because of
> better pool practices in higher layers). But mod_dav and mod_dav_svn
> have notoriously wicked pool usage behavior, and I'm tired of having
> the theoretically niceties of our pool usage guidelines getting in the
> way of Subversion working. Why should a simple 'svn ls -v' of a
> directory with 10,000 files exhaust all the memory on my 2Gb laptop?
> It shouldn't, ahd if this kind of change is what I have to do to get
> that leakage back down to "only" 340Mb, I feel compelled to do it.
>
> Another approach that might have worked just as well would be to add a
> pool argument to the txn_body_* function type which is provided as the
> same pool passed to svn_fs_base__retry_txn(). That would allow the
> txn_body_* functions (which already operate with 'trail' and
> 'trail->pool' today) to use the 'pool' argument as a final destination
> for returnable stuff, and 'trail->pool' for scratch work. (And then
> do_retry() function would, of course, always whack trail->pool when it
> was finished with a trail.) But that would have been a much more
> complicated change.
> }}}
>
> --
> C. Michael Pilato <cmpilato_at_collab.net>
> CollabNet <> www.collab.net <> Distributed Development On Demand
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=1610874
> Attempt to button down the BDB backend's memory usage by allowing trail
> producers to tell that subsystem to discard all memory associated with
> the trail.
>
> Most of the time, this pool usage waste isn't a problem (because of
> better pool practices in higher layers). But mod_dav and mod_dav_svn
> have notoriously wicked pool usage behavior, and I'm tired of having
> the theoretically niceties of our pool usage guidelines getting in the
> way of Subversion working. Why should a simple 'svn ls -v' of a
> directory with 10,000 files exhaust all the memory on my 2Gb laptop?
> It shouldn't, ahd if this kind of change is what I have to do to get
> that leakage back down to "only" 340Mb, I feel compelled to do it.
>
> Another approach that might have worked just as well would be to add a
> pool argument to the txn_body_* function type which is provided as the
> same pool passed to svn_fs_base__retry_txn(). That would allow the
> txn_body_* functions (which already operate with 'trail' and
> 'trail->pool' today) to use the 'pool' argument as a final destination
> for returnable stuff, and 'trail->pool' for scratch work. (And then
> do_retry() function would, of course, always whack trail->pool when it
> was finished with a trail.) But that would have been a much more
> complicated change.
>
> * subversion/libsvn_fs_base/trail.h
> (svn_fs_base__retry_debug, svn_fs_base__retry_txn, svn_fs_base__retry):
> Add 'destroy_trail_pool' parameter.
>
> * subversion/libsvn_fs_base/trail.c
> (do_retry): Add 'destroy_trail_pool', and, if set, destroy the trail
> subpool even upon successful completion of the transaction.
> (svn_fs_base__retry_debug, svn_fs_base__retry_txn, svn_fs_base__retry):
> Add 'destroy_trail_pool' parameter, passed to do_retry().
>
> * subversion/libsvn_fs_base/reps-strings.c,
> * subversion/libsvn_fs_base/revs-txns.c,
> * subversion/libsvn_fs_base/lock.c,
> * subversion/libsvn_fs_base/dag.c,
> * subversion/tests/libsvn_fs_base/changes-test.c,
> * subversion/tests/libsvn_fs_base/fs-base-test.c,
> * subversion/tests/libsvn_fs_base/strings-reps-test.c
> Update all calls to svn_fs_base__retry() and svn_fs_base__retry_txn(),
> passing TRUE for the new 'destroy_trail_pool' parameter iff the
> caller didn't need any allocations made by the trail subsystem to
> outlive the trail itself.
>
> * subversion/libsvn_fs_base/tree.c
> Same as above, but also...
> (base_node_created_path, base_node_prop, svn_fs_base__miscellaneous_get,
> base_copied_from, base_file_checksum): Use a scratch pool for the
> trail work.
>
> * subversion/libsvn_fs_base/uuid.c
> Same as above, but also...
> (svn_fs_base__get_uuid): Use a scratch pool for the trail work.
>
> Index: subversion/tests/libsvn_fs_base/changes-test.c
> ===================================================================
> --- subversion/tests/libsvn_fs_base/changes-test.c (revision 37119)
> +++ subversion/tests/libsvn_fs_base/changes-test.c (working copy)
> @@ -124,7 +124,7 @@
>
> /* Write new changes to the changes table. */
> SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_changes_add, &args,
> - pool));
> + TRUE, pool));
> }
>
> return SVN_NO_ERROR;
> @@ -209,8 +209,8 @@
> without error. */
> args.fs = fs;
> args.key = "blahbliggityblah";
> - SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_changes_fetch_raw,
> - &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_changes_fetch_raw, &args,
> + FALSE, pool));
> if ((! args.raw_changes) || (args.raw_changes->nelts))
> return svn_error_create(SVN_ERR_TEST_FAILED, NULL,
> "expected empty changes array");
> @@ -233,7 +233,7 @@
>
> /* And get those changes. */
> SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_changes_fetch_raw,
> - &args, pool));
> + &args, FALSE, pool));
> if (! args.raw_changes)
> return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
> "got no changes for key '%s'", txn_id);
> @@ -322,10 +322,10 @@
> args.fs = fs;
> args.key = standard_txns[i];
> SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_changes_delete,
> - &args, pool));
> + &args, FALSE, pool));
> args.changes = 0;
> SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_changes_fetch_raw,
> - &args, pool));
> + &args, FALSE, pool));
> if ((! args.raw_changes) || (args.raw_changes->nelts))
> return svn_error_createf
> (SVN_ERR_TEST_FAILED, NULL,
> @@ -513,7 +513,8 @@
> without error. */
> args.fs = fs;
> args.key = "blahbliggityblah";
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_changes_fetch, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_changes_fetch, &args,
> + FALSE, pool));
> if ((! args.changes) || (apr_hash_count(args.changes)))
> return svn_error_create(SVN_ERR_TEST_FAILED, NULL,
> "expected empty changes hash");
> @@ -539,7 +540,7 @@
> /* And get those changes via in the internal interface, and
> verify that they are accurate. */
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_changes_fetch, &args,
> - pool));
> + FALSE, pool));
> if (! args.changes)
> return svn_error_createf
> (SVN_ERR_TEST_FAILED, NULL,
> @@ -621,7 +622,7 @@
> args.fs = fs;
> args.key = txn_name;
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_changes_fetch, &args,
> - subpool));
> + FALSE, subpool));
> if ((! args.changes) || (apr_hash_count(args.changes) != 3))
> return svn_error_create(SVN_ERR_TEST_FAILED, NULL,
> "expected changes");
> @@ -693,7 +694,7 @@
> args.fs = fs;
> args.key = txn_name;
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_changes_fetch, &args,
> - subpool));
> + FALSE, subpool));
> if ((! args.changes) || (apr_hash_count(args.changes) != 3))
> return svn_error_create(SVN_ERR_TEST_FAILED, NULL,
> "expected changes");
> Index: subversion/tests/libsvn_fs_base/fs-base-test.c
> ===================================================================
> --- subversion/tests/libsvn_fs_base/fs-base-test.c (revision 37119)
> +++ subversion/tests/libsvn_fs_base/fs-base-test.c (working copy)
> @@ -206,7 +206,7 @@
>
> args.id = id;
> args.fs = fs;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_check_id, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_check_id, &args, TRUE, pool));
>
> if (args.present)
> *present = TRUE;
> @@ -1429,7 +1429,7 @@
> args.fs = fs;
> args.txn_name = txn_name;
> args.txn = &transaction;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_get_txn, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_get_txn, &args, FALSE, pool));
> if (transaction->copies->nelts != 1)
> return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
> "Expected 1 copy; got %d",
> @@ -1443,7 +1443,7 @@
>
> /* Now, examine the transaction. There should still only have been
> one copy operation that "took". */
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_get_txn, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_get_txn, &args, FALSE, pool));
> if (transaction->copies->nelts != 1)
> return svn_error_createf(SVN_ERR_TEST_FAILED, NULL,
> "Expected only 1 copy; got %d",
> Index: subversion/tests/libsvn_fs_base/strings-reps-test.c
> ===================================================================
> --- subversion/tests/libsvn_fs_base/strings-reps-test.c (revision 37119)
> +++ subversion/tests/libsvn_fs_base/strings-reps-test.c (working copy)
> @@ -117,7 +117,7 @@
>
> /* Write new rep to reps table. */
> SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_write_new_rep, &args,
> - pool));
> + FALSE, pool));
>
> if (args.key == NULL)
> return svn_error_create(SVN_ERR_FS_GENERAL, NULL,
> @@ -155,8 +155,8 @@
> new_args.key = NULL;
>
> /* Write new rep to reps table. */
> - SVN_ERR(svn_fs_base__retry_txn(new_args.fs,
> - txn_body_write_new_rep, &new_args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(new_args.fs, txn_body_write_new_rep,
> + &new_args, FALSE, pool));
>
> /* Make sure we got a valid key. */
> if (new_args.key == NULL)
> @@ -169,8 +169,8 @@
> args.key = new_args.key;
>
> /* Overwrite first rep in reps table. */
> - SVN_ERR(svn_fs_base__retry_txn(new_args.fs,
> - txn_body_write_rep, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(new_args.fs, txn_body_write_rep, &args,
> + FALSE, pool));
>
> return SVN_NO_ERROR;
> }
> @@ -234,8 +234,8 @@
> new_args.key = NULL;
>
> /* Write new rep to reps table. */
> - SVN_ERR(svn_fs_base__retry_txn(new_args.fs,
> - txn_body_write_new_rep, &new_args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(new_args.fs, txn_body_write_new_rep,
> + &new_args, FALSE, pool));
>
> /* Make sure we got a valid key. */
> if (new_args.key == NULL)
> @@ -246,8 +246,8 @@
> read_args.fs = new_args.fs;
> read_args.skel = NULL;
> read_args.key = new_args.key;
> - SVN_ERR(svn_fs_base__retry_txn(new_args.fs,
> - txn_body_read_rep, &read_args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(new_args.fs, txn_body_read_rep, &read_args,
> + FALSE, pool));
>
> /* Make sure the skel matches. */
> if (! read_args.skel)
> @@ -265,15 +265,15 @@
> args.key = new_args.key;
>
> /* Overwrite first rep in reps table. */
> - SVN_ERR(svn_fs_base__retry_txn(new_args.fs,
> - txn_body_write_rep, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(new_args.fs, txn_body_write_rep, &args,
> + FALSE, pool));
>
> /* Read the new rep back from the reps table (using the same FS and
> key as the first read...let's make sure this thing didn't get
> written to the wrong place). */
> read_args.skel = NULL;
> - SVN_ERR(svn_fs_base__retry_txn(new_args.fs,
> - txn_body_read_rep, &read_args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(new_args.fs, txn_body_read_rep, &read_args,
> + FALSE, pool));
>
> /* Make sure the skel matches. */
> if (! read_args.skel)
> @@ -318,8 +318,8 @@
> new_args.key = NULL;
>
> /* Write new rep to reps table. */
> - SVN_ERR(svn_fs_base__retry_txn(new_args.fs,
> - txn_body_write_new_rep, &new_args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(new_args.fs, txn_body_write_new_rep,
> + &new_args, FALSE, pool));
>
> /* Make sure we got a valid key. */
> if (new_args.key == NULL)
> @@ -329,15 +329,15 @@
> /* Delete the rep we just wrote. */
> delete_args.fs = new_args.fs;
> delete_args.key = new_args.key;
> - SVN_ERR(svn_fs_base__retry_txn(new_args.fs,
> - txn_body_delete_rep, &delete_args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(new_args.fs, txn_body_delete_rep,
> + &delete_args, FALSE, pool));
>
> /* Try to read the new rep back from the reps table. */
> read_args.fs = new_args.fs;
> read_args.skel = NULL;
> read_args.key = new_args.key;
> - err = svn_fs_base__retry_txn(new_args.fs,
> - txn_body_read_rep, &read_args, pool);
> + err = svn_fs_base__retry_txn(new_args.fs, txn_body_read_rep, &read_args,
> + FALSE, pool);
>
> /* We better have an error... */
> if ((! err) && (read_args.skel))
> @@ -564,8 +564,8 @@
> args.key = NULL;
> args.text = bigstring1;
> args.len = strlen(bigstring1);
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_string_append, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_string_append, &args,
> + FALSE, pool));
>
> /* Make sure a key was returned. */
> if (! args.key)
> @@ -573,52 +573,52 @@
> "write of new string failed to return new key");
>
> /* Verify record's size and contents. */
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_verify_string, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_verify_string, &args,
> + FALSE, pool));
>
> /* Append a second string to our first one. */
> args.text = bigstring2;
> args.len = strlen(bigstring2);
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_string_append, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_string_append, &args,
> + FALSE, pool));
>
> /* Verify record's size and contents. */
> string = svn_stringbuf_create(bigstring1, pool);
> svn_stringbuf_appendcstr(string, bigstring2);
> args.text = string->data;
> args.len = string->len;
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_verify_string, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_verify_string, &args,
> + FALSE, pool));
>
> /* Clear the record */
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_string_clear, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_string_clear, &args,
> + FALSE, pool));
>
> /* Verify record's size and contents. */
> args.text = "";
> args.len = 0;
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_verify_string, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_verify_string, &args,
> + FALSE, pool));
>
> /* Append a third string to our first one. */
> args.text = bigstring3;
> args.len = strlen(bigstring3);
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_string_append, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_string_append, &args,
> + FALSE, pool));
>
> /* Verify record's size and contents. */
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_verify_string, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_verify_string, &args,
> + FALSE, pool));
>
> /* Delete our record...she's served us well. */
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_string_delete, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_string_delete, &args,
> + FALSE, pool));
>
> /* Now, we expect a size request on this record to fail with
> SVN_ERR_FS_NO_SUCH_STRING. */
> {
> svn_error_t *err = svn_fs_base__retry_txn(args.fs, txn_body_string_size,
> - &args, pool);
> + &args, FALSE, pool);
>
> if (! err)
> return svn_error_create(SVN_ERR_FS_GENERAL, NULL,
> @@ -657,7 +657,7 @@
> args.text = NULL;
> args.len = 0;
> SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_string_append, &args,
> - pool));
> + FALSE, pool));
>
> return SVN_NO_ERROR;
> }
> @@ -694,8 +694,8 @@
> args.key = NULL;
> args.text = bigstring1;
> args.len = strlen(bigstring1);
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_string_append, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_string_append, &args,
> + FALSE, pool));
>
> /* Make sure a key was returned. */
> if (! args.key)
> @@ -703,8 +703,8 @@
> "write of new string failed to return new key");
>
> /* Verify record's size and contents. */
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_verify_string, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_verify_string, &args,
> + FALSE, pool));
>
> /* Append a second string to our first one. */
> args2.fs = fs;
> @@ -716,7 +716,7 @@
>
> /* This function is *supposed* to fail with SVN_ERR_TEST_FAILED */
> err = svn_fs_base__retry_txn(args.fs, txn_body_string_append_fail,
> - &args2, pool);
> + &args2, FALSE, pool);
> if ((! err) || (err->apr_err != SVN_ERR_TEST_FAILED))
> return svn_error_create(SVN_ERR_TEST_FAILED, err,
> "failed to intentionally abort a trail");
> @@ -724,8 +724,8 @@
> }
>
> /* Verify that record's size and contents are still that of string1 */
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_verify_string, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_verify_string, &args,
> + FALSE, pool));
>
> return SVN_NO_ERROR;
> }
> @@ -755,8 +755,8 @@
> args.key = NULL;
> args.text = bigstring1;
> args.len = strlen(bigstring1);
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_string_append, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_string_append, &args,
> + FALSE, pool));
>
> /* Make sure a key was returned. */
> if (! (old_key = args.key))
> @@ -764,8 +764,8 @@
> "write of new string failed to return new key");
>
> /* Now copy that string into a new location. */
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_string_copy, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_string_copy, &args,
> + FALSE, pool));
>
> /* Make sure a different key was returned. */
> if ((! args.key) || (! strcmp(old_key, args.key)))
> @@ -773,8 +773,8 @@
> "copy of string failed to return new key");
>
> /* Verify record's size and contents. */
> - SVN_ERR(svn_fs_base__retry_txn(args.fs,
> - txn_body_verify_string, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(args.fs, txn_body_verify_string, &args,
> + FALSE, pool));
>
> return SVN_NO_ERROR;
> }
> Index: subversion/libsvn_fs_base/tree.c
> ===================================================================
> --- subversion/libsvn_fs_base/tree.c (revision 37119)
> +++ subversion/libsvn_fs_base/tree.c (working copy)
> @@ -327,7 +327,8 @@
>
> args.root_p = &root;
> args.txn = txn;
> - SVN_ERR(svn_fs_base__retry_txn(txn->fs, txn_body_txn_root, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(txn->fs, txn_body_txn_root, &args,
> + FALSE, pool));
>
> *root_p = root;
> return SVN_NO_ERROR;
> @@ -371,7 +372,8 @@
>
> args.root_p = &root;
> args.rev = rev;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_revision_root, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_revision_root, &args,
> + FALSE, pool));
>
> *root_p = root;
> return SVN_NO_ERROR;
> @@ -1016,7 +1018,7 @@
> args.path = path;
>
> SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_node_id, &args,
> - pool));
> + FALSE, pool));
> *id_p = id;
> }
> return SVN_NO_ERROR;
> @@ -1054,7 +1056,7 @@
> args.root = root;
> args.path = path;
> SVN_ERR(svn_fs_base__retry_txn
> - (root->fs, txn_body_node_created_rev, &args, pool));
> + (root->fs, txn_body_node_created_rev, &args, TRUE, pool));
> *revision = args.revision;
> return SVN_NO_ERROR;
> }
> @@ -1086,12 +1088,18 @@
> apr_pool_t *pool)
> {
> struct node_created_path_args args;
> + apr_pool_t *scratch_pool = svn_pool_create(pool);
>
> args.created_path = created_path;
> args.root = root;
> args.path = path;
> - return svn_fs_base__retry_txn
> - (root->fs, txn_body_node_created_path, &args, pool);
> +
> + SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_node_created_path, &args,
> + FALSE, scratch_pool));
> + if (*created_path)
> + *created_path = apr_pstrdup(pool, *created_path);
> + svn_pool_destroy(scratch_pool);
> + return SVN_NO_ERROR;
> }
>
>
> @@ -1129,7 +1137,8 @@
>
> /* Use the node id to get the real kind. */
> args.id = node_id;
> - SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_node_kind, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_node_kind, &args,
> + TRUE, pool));
>
> *kind_p = args.kind;
> return SVN_NO_ERROR;
> @@ -1193,14 +1202,16 @@
> {
> struct node_prop_args args;
> svn_string_t *value;
> + apr_pool_t *scratch_pool = svn_pool_create(pool);
>
> args.value_p = &value;
> args.root = root;
> args.path = path;
> args.propname = propname;
> - SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_node_prop, &args, pool));
> -
> - *value_p = value;
> + SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_node_prop, &args,
> + FALSE, scratch_pool));
> + *value_p = value ? svn_string_dup(value, pool) : NULL;
> + svn_pool_destroy(scratch_pool);
> return SVN_NO_ERROR;
> }
>
> @@ -1241,7 +1252,7 @@
> args.path = path;
>
> SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_node_proplist, &args,
> - pool));
> + FALSE, pool));
>
> *table_p = table;
> return SVN_NO_ERROR;
> @@ -1345,7 +1356,7 @@
> args.name = name;
> args.value = value;
> return svn_fs_base__retry_txn(root->fs, txn_body_change_node_prop, &args,
> - pool);
> + TRUE, pool);
> }
>
>
> @@ -1396,8 +1407,8 @@
> args.changed_p = changed_p;
> args.pool = pool;
>
> - return svn_fs_base__retry_txn(root1->fs, txn_body_props_changed,
> - &args, pool);
> + return svn_fs_base__retry_txn(root1->fs, txn_body_props_changed, &args,
> + TRUE, pool);
> }
>
>
> @@ -1429,7 +1440,8 @@
> msa.key = key;
> msa.val = val;
>
> - return svn_fs_base__retry_txn(fs, txn_body_miscellaneous_set, &msa, pool);
> + return svn_fs_base__retry_txn(fs, txn_body_miscellaneous_set, &msa,
> + TRUE, pool);
> }
>
> struct miscellaneous_get_args
> @@ -1453,10 +1465,15 @@
> apr_pool_t *pool)
> {
> struct miscellaneous_get_args mga;
> + apr_pool_t *scratch_pool = svn_pool_create(pool);
> +
> mga.key = key;
> mga.val = val;
> -
> - return svn_fs_base__retry_txn(fs, txn_body_miscellaneous_get, &mga, pool);
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_miscellaneous_get, &mga,
> + FALSE, scratch_pool));
> + if (*val)
> + *val = apr_pstrdup(pool, *val);
> + return SVN_NO_ERROR;
> }
>
>
> @@ -1508,7 +1525,7 @@
> args.root = root;
> args.path = path;
> SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_dir_entries, &args,
> - pool));
> + FALSE, pool));
>
> iterpool = svn_pool_create(pool);
>
> @@ -1526,8 +1543,12 @@
> apr_hash_this(hi, NULL, NULL, &val);
> entry = val;
> nk_args.id = entry->id;
> +
> + /* We don't need to have the retry function destroy the trail
> + pool because we're already doing that via the use of an
> + iteration pool. */
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_node_kind, &nk_args,
> - iterpool));
> + FALSE, iterpool));
> entry->kind = nk_args.kind;
> }
>
> @@ -1714,7 +1735,8 @@
> td_args.tgt_id = id;
> td_args.base_id = NULL;
> td_args.is_dir = (kind == svn_node_dir);
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_txn_deltify, &td_args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_txn_deltify, &td_args,
> + TRUE, pool));
>
> /* Finally, deltify old data against this node. */
> {
> @@ -1758,7 +1780,7 @@
>
> tpc_args.id = id;
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_pred_count, &tpc_args,
> - pool));
> + TRUE, pool));
> pred_count = tpc_args.pred_count;
>
> /* If nothing to deltify, then we're done. */
> @@ -1815,7 +1837,7 @@
> tpi_args.id = pred_id;
> tpi_args.pool = subpools[active_subpool];
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_pred_id, &tpi_args,
> - subpools[active_subpool]));
> + FALSE, subpools[active_subpool]));
> pred_id = tpi_args.pred_id;
>
> if (pred_id == NULL)
> @@ -1831,7 +1853,7 @@
> td_args.base_id = pred_id;
> td_args.is_dir = (kind == svn_node_dir);
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_txn_deltify, &td_args,
> - subpools[active_subpool]));
> + TRUE, subpools[active_subpool]));
> }
> else
> {
> @@ -1881,7 +1903,8 @@
>
> tpi_args.id = pred_id;
> tpi_args.pool = subpools[active_subpool];
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_pred_id, &tpi_args,
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_pred_id,
> + &tpi_args, FALSE,
> subpools[active_subpool]));
> pred_id = tpi_args.pred_id;
>
> @@ -1899,7 +1922,7 @@
> td_args.base_id = id;
> td_args.is_dir = (kind == svn_node_dir);
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_txn_deltify, &td_args,
> - subpools[active_subpool]));
> + TRUE, subpools[active_subpool]));
>
> }
> }
> @@ -2681,8 +2704,8 @@
> may have changed by then -- that's why we're careful to get
> this root in its own bdb txn here). */
> get_root_args.root = youngish_root;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_get_root,
> - &get_root_args, subpool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_get_root, &get_root_args,
> + FALSE, subpool));
> youngish_root_node = get_root_args.node;
>
> /* Try to merge. If the merge succeeds, the base root node of
> @@ -2693,7 +2716,8 @@
> merge_args.source_node = youngish_root_node;
> merge_args.txn = txn;
> merge_args.conflict = svn_stringbuf_create("", pool); /* use pool */
> - err = svn_fs_base__retry_txn(fs, txn_body_merge, &merge_args, subpool);
> + err = svn_fs_base__retry_txn(fs, txn_body_merge, &merge_args,
> + FALSE, subpool);
> if (err)
> {
> if ((err->apr_err == SVN_ERR_FS_CONFLICT) && conflict_p)
> @@ -2704,7 +2728,7 @@
> /* Try to commit. */
> commit_args.txn = txn;
> err = svn_fs_base__retry_txn(fs, txn_body_commit, &commit_args,
> - subpool);
> + FALSE, subpool);
> if (err && (err->apr_err == SVN_ERR_FS_TXN_OUT_OF_DATE))
> {
> /* Did someone else finish committing a new revision while we
> @@ -2785,13 +2809,13 @@
> /* Get the ancestor node. */
> get_root_args.root = ancestor_root;
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_get_root, &get_root_args,
> - pool));
> + FALSE, pool));
> ancestor = get_root_args.node;
>
> /* Get the source node. */
> get_root_args.root = source_root;
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_get_root, &get_root_args,
> - pool));
> + FALSE, pool));
> source = get_root_args.node;
>
> /* Open a txn for the txn root into which we're merging. */
> @@ -2802,7 +2826,7 @@
> merge_args.ancestor_node = ancestor;
> merge_args.txn = txn;
> merge_args.conflict = svn_stringbuf_create("", pool);
> - err = svn_fs_base__retry_txn(fs, txn_body_merge, &merge_args, pool);
> + err = svn_fs_base__retry_txn(fs, txn_body_merge, &merge_args, FALSE, pool);
> if (err)
> {
> if ((err->apr_err == SVN_ERR_FS_CONFLICT) && conflict_p)
> @@ -2861,7 +2885,8 @@
>
> args.txn_id = &txn_id;
> args.revision = revision;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_rev_get_txn_id, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_rev_get_txn_id, &args,
> + FALSE, pool));
>
> return deltify_mutable(fs, root, "/", NULL, svn_node_dir, txn_id, pool);
> }
> @@ -2937,7 +2962,8 @@
>
> args.root = root;
> args.path = path;
> - return svn_fs_base__retry_txn(root->fs, txn_body_make_dir, &args, pool);
> + return svn_fs_base__retry_txn(root->fs, txn_body_make_dir, &args,
> + TRUE, pool);
> }
>
>
> @@ -3021,7 +3047,8 @@
>
> args.root = root;
> args.path = path;
> - return svn_fs_base__retry_txn(root->fs, txn_body_delete, &args, pool);
> + return svn_fs_base__retry_txn(root->fs, txn_body_delete, &args,
> + TRUE, pool);
> }
>
>
> @@ -3207,7 +3234,8 @@
> args.to_path = to_path;
> args.preserve_history = preserve_history;
>
> - return svn_fs_base__retry_txn(to_root->fs, txn_body_copy, &args, pool);
> + return svn_fs_base__retry_txn(to_root->fs, txn_body_copy, &args,
> + TRUE, pool);
> }
>
> static svn_error_t *
> @@ -3298,17 +3326,18 @@
> apr_pool_t *pool)
> {
> struct copied_from_args args;
> -
> + apr_pool_t *scratch_pool = svn_pool_create(pool);
> args.root = root;
> args.path = path;
> args.pool = pool;
>
> - SVN_ERR(svn_fs_base__retry_txn(root->fs,
> - txn_body_copied_from, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_copied_from, &args,
> + FALSE, scratch_pool));
>
> *rev_p = args.result_rev;
> - *path_p = args.result_path;
> + *path_p = args.result_path ? apr_pstrdup(pool, args.result_path) : NULL;
>
> + svn_pool_destroy(scratch_pool);
> return SVN_NO_ERROR;
> }
>
> @@ -3380,7 +3409,8 @@
>
> args.root = root;
> args.path = path;
> - return svn_fs_base__retry_txn(root->fs, txn_body_make_file, &args, pool);
> + return svn_fs_base__retry_txn(root->fs, txn_body_make_file, &args,
> + TRUE, pool);
> }
>
>
> @@ -3418,7 +3448,7 @@
> args.root = root;
> args.path = path;
> SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_file_length, &args,
> - pool));
> + TRUE, pool));
>
> *length_p = args.length;
> return SVN_NO_ERROR;
> @@ -3454,13 +3484,16 @@
> apr_pool_t *pool)
> {
> struct file_checksum_args args;
> + apr_pool_t *scratch_pool = svn_pool_create(pool);
>
> args.root = root;
> args.path = path;
> args.kind = kind;
> args.checksum = checksum;
> - return svn_fs_base__retry_txn(root->fs, txn_body_file_checksum, &args,
> - pool);
> + SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_file_checksum, &args,
> + FALSE, scratch_pool));
> + *checksum = svn_checksum_dup(*checksum, pool);
> + return SVN_NO_ERROR;
> }
>
>
> @@ -3516,8 +3549,8 @@
> fb->pool = pool;
>
> /* Create the readable stream in the context of a db txn. */
> - SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_get_file_contents,
> - fb, pool));
> + SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_get_file_contents, fb,
> + FALSE, pool));
>
> *contents = fb->file_stream;
> return SVN_NO_ERROR;
> @@ -3657,7 +3690,7 @@
> /* Tell the dag subsystem that we're finished with our edits. */
> SVN_ERR(svn_fs_base__retry_txn(tb->root->fs,
> txn_body_txdelta_finalize_edits, tb,
> - tb->pool));
> + FALSE, tb->pool));
> }
>
> return SVN_NO_ERROR;
> @@ -3767,7 +3800,7 @@
> tb->result_checksum = NULL;
>
> SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_apply_textdelta, tb,
> - pool));
> + FALSE, pool));
>
> *contents_p = window_consumer;
> *contents_baton_p = tb;
> @@ -3854,7 +3887,7 @@
> /* Need to tell fs that we're done sending text */
> return svn_fs_base__retry_txn(tb->root->fs,
> txn_body_fulltext_finalize_edits, tb,
> - tb->pool);
> + FALSE, tb->pool);
> }
>
>
> @@ -3911,7 +3944,8 @@
> else
> tb->result_checksum = NULL;
>
> - SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_apply_text, tb, pool));
> + SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_apply_text, tb,
> + FALSE, pool));
>
> *contents_p = tb->stream;
> return SVN_NO_ERROR;
> @@ -3976,8 +4010,8 @@
> args.changed_p = changed_p;
> args.pool = pool;
>
> - return svn_fs_base__retry_txn(root1->fs, txn_body_contents_changed,
> - &args, pool);
> + return svn_fs_base__retry_txn(root1->fs, txn_body_contents_changed, &args,
> + TRUE, pool);
> }
>
>
> @@ -4055,7 +4089,8 @@
> struct paths_changed_args args;
> args.root = root;
> args.changes = NULL;
> - SVN_ERR(svn_fs_base__retry(root->fs, txn_body_paths_changed, &args, pool));
> + SVN_ERR(svn_fs_base__retry(root->fs, txn_body_paths_changed, &args,
> + FALSE, pool));
> *changed_paths_p = args.changes;
> return SVN_NO_ERROR;
> }
> @@ -4397,7 +4432,7 @@
> args.cross_copies = cross_copies;
> args.pool = pool;
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_history_prev, &args,
> - pool));
> + FALSE, pool));
> if (! prev_history)
> break;
> bhd = prev_history->fsap_data;
> @@ -4574,7 +4609,8 @@
> args.root = root;
> args.path = path;
> args.pool = pool;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_closest_copy, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_closest_copy, &args,
> + FALSE, pool));
> *root_p = closest_root;
> *path_p = closest_path;
> return SVN_NO_ERROR;
> @@ -4808,8 +4844,8 @@
>
> SVN_ERR(base_node_id(&id, root, path, pool));
> args.node_id = svn_fs_base__id_node_id(id);
> - err = svn_fs_base__retry_txn(root->fs, txn_body_get_node_origin,
> - &args, pool);
> + err = svn_fs_base__retry_txn(root->fs, txn_body_get_node_origin, &args,
> + FALSE, pool);
>
> /* If we got a value for the origin node-revision-ID, that's
> great. If we didn't, that's sad but non-fatal -- we'll just
> @@ -4876,8 +4912,8 @@
> pid_args.id = pred_id;
> pid_args.pred_id = NULL;
> pid_args.pool = subpool;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_pred_id,
> - &pid_args, subpool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_pred_id, &pid_args,
> + FALSE, subpool));
> if (! pid_args.pred_id)
> break;
> svn_pool_clear(predidpool);
> @@ -4893,7 +4929,7 @@
> {
> args.origin_id = origin_id;
> SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_set_node_origin,
> - &args, subpool));
> + &args, TRUE, subpool));
> }
>
> svn_pool_destroy(predidpool);
> @@ -4903,8 +4939,8 @@
> /* Okay. We have an origin node-revision-ID. Let's get a created
> revision from it. */
> icr_args.id = origin_id;
> - SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_id_created_rev,
> - &icr_args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(root->fs, txn_body_id_created_rev, &icr_args,
> + TRUE, pool));
> *revision = icr_args.revision;
> return SVN_NO_ERROR;
> }
> @@ -5048,7 +5084,7 @@
> gmdae_args.node = node;
> gmdae_args.node_path = node_path;
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_get_mergeinfo_data_and_entries,
> - &gmdae_args, pool));
> + &gmdae_args, FALSE, pool));
>
> /* If no children sit atop trees with mergeinfo, we're done.
> Otherwise, recurse on those children. */
> @@ -5269,7 +5305,7 @@
> gmfp_args.pool = pool;
> SVN_ERR(svn_fs_base__retry_txn(root->fs,
> txn_body_get_mergeinfo_for_path,
> - &gmfp_args, iterpool));
> + &gmfp_args, FALSE, iterpool));
> if (path_mergeinfo)
> apr_hash_set(result_catalog, apr_pstrdup(pool, path),
> APR_HASH_KEY_STRING,
> @@ -5286,7 +5322,7 @@
> gnms_args.path = path;
> SVN_ERR(svn_fs_base__retry_txn(root->fs,
> txn_body_get_node_mergeinfo_stats,
> - &gnms_args, iterpool));
> + &gnms_args, FALSE, iterpool));
>
> /* Determine if there's anything worth crawling here. */
> if (svn_fs_base__dag_node_kind(gnms_args.node) != svn_node_dir)
> Index: subversion/libsvn_fs_base/reps-strings.c
> ===================================================================
> --- subversion/libsvn_fs_base/reps-strings.c (revision 37119)
> +++ subversion/libsvn_fs_base/reps-strings.c (working copy)
> @@ -966,18 +966,16 @@
> SVN_ERR(txn_body_read_rep(&args, rb->trail));
> else
> {
> - /* Hey, guess what? trails don't clear their own subpools. In
> - the case of reading from the db, any returned data should
> + /* In the case of reading from the db, any returned data should
> live in our pre-allocated buffer, so the whole operation can
> happen within a single malloc/free cycle. This prevents us
> from creating millions of unnecessary trail subpools when
> - reading a big file. */
> - apr_pool_t *subpool = svn_pool_create(rb->pool);
> + reading a big file. */
> SVN_ERR(svn_fs_base__retry_txn(rb->fs,
> txn_body_read_rep,
> &args,
> - subpool));
> - svn_pool_destroy(subpool);
> + TRUE,
> + rb->pool));
> }
> return SVN_NO_ERROR;
> }
> @@ -1136,19 +1134,17 @@
> SVN_ERR(txn_body_write_rep(&args, wb->trail));
> else
> {
> - /* Hey, guess what? trails don't clear their own subpools. In
> - the case of simply writing the rep to the db, we're *certain*
> - that there's no data coming back to us that needs to be
> - preserved... so the whole operation can happen within a
> + /* In the case of simply writing the rep to the db, we're
> + *certain* that there's no data coming back to us that needs
> + to be preserved... so the whole operation can happen within a
> single malloc/free cycle. This prevents us from creating
> millions of unnecessary trail subpools when writing a big
> file. */
> - apr_pool_t *subpool = svn_pool_create(wb->pool);
> SVN_ERR(svn_fs_base__retry_txn(wb->fs,
> txn_body_write_rep,
> &args,
> - subpool));
> - svn_pool_destroy(subpool);
> + TRUE,
> + wb->pool));
> }
>
> return SVN_NO_ERROR;
> @@ -1203,8 +1199,10 @@
> if (wb->trail)
> return txn_body_write_close_rep(wb, wb->trail);
> else
> + /* We need to keep our trail pool around this time so the
> + checksums we've calculated survive. */
> return svn_fs_base__retry_txn(wb->fs, txn_body_write_close_rep,
> - wb, wb->pool);
> + wb, FALSE, wb->pool);
> }
>
>
> Index: subversion/libsvn_fs_base/uuid.c
> ===================================================================
> --- subversion/libsvn_fs_base/uuid.c (revision 37119)
> +++ subversion/libsvn_fs_base/uuid.c (working copy)
> @@ -15,6 +15,7 @@
> * ====================================================================
> */
>
> +#include "svn_pools.h"
> #include "fs.h"
> #include "trail.h"
> #include "err.h"
> @@ -59,13 +60,22 @@
> else
> {
> struct get_uuid_args args;
> + apr_pool_t *scratch_pool = svn_pool_create(pool);
> +
> args.idx = 1;
> args.uuid = uuid;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_get_uuid, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_get_uuid, &args,
> + FALSE, scratch_pool));
> +
> + if (*uuid)
> + {
> + *uuid = apr_pstrdup(pool, *uuid);
>
> - /* Toss what we find into the cache. */
> - if (*uuid)
> - bfd->uuid = apr_pstrdup(fs->pool, *uuid);
> + /* Toss what we find into the cache. */
> + bfd->uuid = apr_pstrdup(fs->pool, *uuid);
> + }
> +
> + svn_pool_destroy(scratch_pool);
> }
>
> return SVN_NO_ERROR;
> @@ -103,7 +113,7 @@
>
> args.idx = 1;
> args.uuid = uuid;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_set_uuid, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_set_uuid, &args, TRUE, pool));
>
> /* Toss our value into the cache. */
> if (uuid)
> Index: subversion/libsvn_fs_base/revs-txns.c
> ===================================================================
> --- subversion/libsvn_fs_base/revs-txns.c (revision 37119)
> +++ subversion/libsvn_fs_base/revs-txns.c (working copy)
> @@ -161,7 +161,7 @@
> svn_revnum_t youngest;
> SVN_ERR(svn_fs__check_fs(fs, TRUE));
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_youngest_rev, &youngest,
> - pool));
> + TRUE, pool));
> *youngest_p = youngest;
> return SVN_NO_ERROR;
> }
> @@ -199,7 +199,7 @@
> args.table_p = &table;
> args.rev = rev;
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_revision_proplist, &args,
> - pool));
> + FALSE, pool));
>
> *table_p = table ? table : apr_hash_make(pool);
> return SVN_NO_ERROR;
> @@ -222,7 +222,7 @@
> args.table_p = &table;
> args.rev = rev;
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_revision_proplist, &args,
> - pool));
> + FALSE, pool));
>
> /* And then the prop from that list (if there was a list). */
> *value_p = NULL;
> @@ -294,7 +294,8 @@
> args.rev = rev;
> args.name = name;
> args.value = value;
> - return svn_fs_base__retry_txn(fs, txn_body_change_rev_prop, &args, pool);
> + return svn_fs_base__retry_txn(fs, txn_body_change_rev_prop, &args,
> + TRUE, pool);
> }
>
>
> @@ -485,7 +486,8 @@
>
> args.table_p = &table;
> args.id = txn->id;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_txn_proplist, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_txn_proplist, &args,
> + FALSE, pool));
>
> *table_p = table ? table : apr_hash_make(pool);
> return SVN_NO_ERROR;
> @@ -507,7 +509,8 @@
> /* Get the proplist. */
> args.table_p = &table;
> args.id = txn->id;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_txn_proplist, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_txn_proplist, &args,
> + FALSE, pool));
>
> /* And then the prop from that list (if there was a list). */
> *value_p = NULL;
> @@ -579,7 +582,8 @@
> args.id = txn->id;
> args.name = name;
> args.value = value;
> - return svn_fs_base__retry_txn(fs, txn_body_change_txn_prop, &args, pool);
> + return svn_fs_base__retry_txn(fs, txn_body_change_txn_prop, &args,
> + TRUE, pool);
> }
>
>
> @@ -706,7 +710,7 @@
> args.txn_p = &txn;
> args.rev = rev;
> args.flags = flags;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_begin_txn, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_begin_txn, &args, FALSE, pool));
>
> *txn_p = txn;
>
> @@ -763,7 +767,7 @@
>
> args.txn_p = &txn;
> args.name = name;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_open_txn, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_open_txn, &args, FALSE, pool));
>
> *txn_p = txn;
> return SVN_NO_ERROR;
> @@ -880,7 +884,8 @@
> dirent_args.dirents = &dirents;
> dirent_args.id = id;
> dirent_args.txn_id = txn_id;
> - err = svn_fs_base__retry_txn(fs, txn_body_get_dirents, &dirent_args, pool);
> + err = svn_fs_base__retry_txn(fs, txn_body_get_dirents, &dirent_args,
> + FALSE, pool);
> if (err && (err->apr_err == SVN_ERR_FS_ID_NOT_FOUND))
> {
> svn_error_clear(err);
> @@ -910,7 +915,8 @@
> /* Remove the node. */
> rm_args.id = id;
> rm_args.txn_id = txn_id;
> - return svn_fs_base__retry_txn(fs, txn_body_remove_node, &rm_args, pool);
> + return svn_fs_base__retry_txn(fs, txn_body_remove_node, &rm_args,
> + TRUE, pool);
> }
>
>
> @@ -935,7 +941,8 @@
> /* Open the transaction, expecting it to be dead. */
> args.txn_p = &txn;
> args.name = txn_id;
> - SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_cleanup_txn, &args, pool));
> + SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_cleanup_txn, &args,
> + FALSE, pool));
>
> /* Delete the mutable portion of the tree hanging from the
> transaction (which should gracefully recover if we've already
> @@ -945,7 +952,7 @@
> /* Kill the transaction's changes (which should gracefully recover
> if...). */
> SVN_ERR(svn_fs_base__retry_txn(fs, txn_body_cleanup_txn_changes,
> - (void *)txn_id, pool));
> + (void *)txn_id, TRUE, pool));
>
> /* Kill the transaction's copies (which should gracefully...). */
> if (txn->copies)
> @@ -955,14 +962,14 @@
> SVN_ERR(svn_fs_base__retry_txn
> (fs, txn_body_cleanup_txn_copy,
> (void *)APR_ARRAY_IDX(txn->copies, i, const char *),
> - pool));
> + TRUE, pool));
> }
> }
>
> /* Kill the transaction itself (which ... just kidding -- this has
> no graceful failure mode). */
> return svn_fs_base__retry_txn(fs, txn_body_delete_txn, (void *)txn_id,
> - pool);
> + TRUE, pool);
> }
>
>
> @@ -990,7 +997,8 @@
> SVN_ERR(svn_fs__check_fs(txn->fs, TRUE));
>
> /* Set the transaction to "dead". */
> - SVN_ERR(svn_fs_base__retry_txn(txn->fs, txn_body_abort_txn, txn, pool));
> + SVN_ERR(svn_fs_base__retry_txn(txn->fs, txn_body_abort_txn, txn,
> + TRUE, pool));
>
> /* Now, purge it. */
> SVN_ERR_W(svn_fs_base__purge_txn(txn->fs, txn->id, pool),
> @@ -1026,7 +1034,8 @@
>
> args.names_p = &names;
> args.pool = pool;
> - SVN_ERR(svn_fs_base__retry(fs, txn_body_list_transactions, &args, pool));
> + SVN_ERR(svn_fs_base__retry(fs, txn_body_list_transactions, &args,
> + FALSE, pool));
>
> *names_p = names;
> return SVN_NO_ERROR;
> Index: subversion/libsvn_fs_base/trail.c
> ===================================================================
> --- subversion/libsvn_fs_base/trail.c (revision 37119)
> +++ subversion/libsvn_fs_base/trail.c (working copy)
> @@ -190,6 +190,7 @@
> svn_error_t *(*txn_body)(void *baton, trail_t *trail),
> void *baton,
> svn_boolean_t use_txn,
> + svn_boolean_t destroy_trail_pool,
> apr_pool_t *pool,
> const char *txn_body_fn_name,
> const char *filename,
> @@ -214,6 +215,11 @@
> if (use_txn)
> print_trail_debug(trail, txn_body_fn_name, filename, line);
>
> + /* If our caller doesn't want us to keep trail memory
> + around, destroy our subpool. */
> + if (destroy_trail_pool)
> + svn_pool_destroy(trail->pool);
> +
> return SVN_NO_ERROR;
> }
>
> @@ -242,12 +248,13 @@
> svn_fs_base__retry_debug(svn_fs_t *fs,
> svn_error_t *(*txn_body)(void *baton, trail_t *trail),
> void *baton,
> + svn_boolean_t destroy_trail_pool,
> apr_pool_t *pool,
> const char *txn_body_fn_name,
> const char *filename,
> int line)
> {
> - return do_retry(fs, txn_body, baton, TRUE, pool,
> + return do_retry(fs, txn_body, baton, TRUE, destroy_trail_pool, pool,
> txn_body_fn_name, filename, line);
> }
>
> @@ -260,9 +267,10 @@
> svn_fs_base__retry_txn(svn_fs_t *fs,
> svn_error_t *(*txn_body)(void *baton, trail_t *trail),
> void *baton,
> + svn_boolean_t destroy_trail_pool,
> apr_pool_t *pool)
> {
> - return do_retry(fs, txn_body, baton, TRUE, pool,
> + return do_retry(fs, txn_body, baton, TRUE, destroy_trail_pool, pool,
> "unknown", "", 0);
> }
>
> @@ -271,8 +279,9 @@
> svn_fs_base__retry(svn_fs_t *fs,
> svn_error_t *(*txn_body)(void *baton, trail_t *trail),
> void *baton,
> + svn_boolean_t destroy_trail_pool,
> apr_pool_t *pool)
> {
> - return do_retry(fs, txn_body, baton, FALSE, pool,
> + return do_retry(fs, txn_body, baton, FALSE, destroy_trail_pool, pool,
> NULL, NULL, 0);
> }
> Index: subversion/libsvn_fs_base/trail.h
> ===================================================================
> --- subversion/libsvn_fs_base/trail.h (revision 37119)
> +++ subversion/libsvn_fs_base/trail.h (working copy)
> @@ -113,8 +113,8 @@
> If, heavens forbid, your function actually succeeds, returning
> SVN_NO_ERROR, `svn_fs_base__retry_txn' commits the trail's Berkeley DB
> transaction, thus making your DB changes permanent, leaves the
> - trail's pool alone, so all the objects it contains are still
> - around, and returns SVN_NO_ERROR. */
> + trail's pool alone so all the objects it contains are still
> + around (unless you request otherwise), and returns SVN_NO_ERROR. */
>
> struct trail_t
> {
> @@ -150,7 +150,7 @@
> svn_error_t, E.
> - If TXN_BODY returns SVN_NO_ERROR, then commit the transaction,
> run any completion functions, and return SVN_NO_ERROR. Do *not*
> - free TXN_POOL.
> + free TXN_POOL (unless DESTROY_TRAIL_POOL is set).
> - If E is a Berkeley DB error indicating that a deadlock occurred,
> abort the DB transaction and free TXN_POOL. Then retry the whole
> thing from the top.
> @@ -160,37 +160,41 @@
> ensure that whatever transactions a filesystem function starts, it
> either aborts or commits before it returns. If we don't somehow
> complete all our transactions, later operations could deadlock. */
> -svn_error_t *svn_fs_base__retry_txn(svn_fs_t *fs,
> - svn_error_t *(*txn_body)(void *baton,
> - trail_t *trail),
> - void *baton,
> - apr_pool_t *pool);
> +svn_error_t *
> +svn_fs_base__retry_txn(svn_fs_t *fs,
> + svn_error_t *(*txn_body)(void *baton,
> + trail_t *trail),
> + void *baton,
> + svn_boolean_t destroy_trail_pool,
> + apr_pool_t *pool);
>
> svn_error_t *
> svn_fs_base__retry_debug(svn_fs_t *fs,
> svn_error_t *(*txn_body)(void *baton,
> trail_t *trail),
> void *baton,
> + svn_boolean_t destroy_trail_pool,
> apr_pool_t *pool,
> const char *txn_body_fn_name,
> const char *filename,
> int line);
>
> #if defined(SVN_FS__TRAIL_DEBUG)
> -#define svn_fs_base__retry_txn(fs, txn_body, baton, pool) \
> - svn_fs_base__retry_debug(fs, txn_body, baton, pool, #txn_body,
> - __FILE__, __LINE__)
> +#define svn_fs_base__retry_txn(fs, txn_body, baton, destroy, pool) \
> + svn_fs_base__retry_debug(fs, txn_body, baton, destroy, pool, \
> + #txn_body, __FILE__, __LINE__)
> #endif
>
>
> /* Try an action repeatedly until it doesn't deadlock. This is
> exactly like svn_fs_base__retry_txn() (whose documentation you really
> should read) except that no Berkeley DB transaction is created. */
> - svn_error_t *svn_fs_base__retry(svn_fs_t *fs,
> - svn_error_t *(*txn_body)(void *baton,
> - trail_t *trail),
> - void *baton,
> - apr_pool_t *pool);
> +svn_error_t *svn_fs_base__retry(svn_fs_t *fs,
> + svn_error_t *(*txn_body)(void *baton,
> + trail_t *trail),
> + void *baton,
> + svn_boolean_t destroy_trail_pool,
> + apr_pool_t *pool);
>
>
> /* Record that OPeration is being done on TABLE in the TRAIL. */
> Index: subversion/libsvn_fs_base/lock.c
> ===================================================================
> --- subversion/libsvn_fs_base/lock.c (revision 37119)
> +++ subversion/libsvn_fs_base/lock.c (working copy)
> @@ -224,8 +224,7 @@
> args.expiration_date = expiration_date;
> args.current_rev = current_rev;
>
> - return svn_fs_base__retry_txn(fs, txn_body_lock, &args, pool);
> -
> + return svn_fs_base__retry_txn(fs, txn_body_lock, &args, FALSE, pool);
> }
>
>
> @@ -306,7 +305,7 @@
> args.path = svn_fs__canonicalize_abspath(path, pool);
> args.token = token;
> args.break_lock = break_lock;
> - return svn_fs_base__retry_txn(fs, txn_body_unlock, &args, pool);
> + return svn_fs_base__retry_txn(fs, txn_body_unlock, &args, TRUE, pool);
> }
>
>
> @@ -379,7 +378,7 @@
>
> args.path = svn_fs__canonicalize_abspath(path, pool);
> args.lock_p = lock;
> - return svn_fs_base__retry_txn(fs, txn_body_get_lock, &args, pool);
> + return svn_fs_base__retry_txn(fs, txn_body_get_lock, &args, FALSE, pool);
> }
>
>
> @@ -414,7 +413,7 @@
> args.path = svn_fs__canonicalize_abspath(path, pool);
> args.get_locks_func = get_locks_func;
> args.get_locks_baton = get_locks_baton;
> - return svn_fs_base__retry_txn(fs, txn_body_get_locks, &args, pool);
> + return svn_fs_base__retry_txn(fs, txn_body_get_locks, &args, FALSE, pool);
> }
>
>
> Index: subversion/libsvn_fs_base/dag.c
> ===================================================================
> --- subversion/libsvn_fs_base/dag.c (revision 37119)
> +++ subversion/libsvn_fs_base/dag.c (working copy)
> @@ -252,7 +252,8 @@
> svn_error_t *
> svn_fs_base__dag_init_fs(svn_fs_t *fs)
> {
> - return svn_fs_base__retry_txn(fs, txn_body_dag_init_fs, NULL, fs->pool);
> + return svn_fs_base__retry_txn(fs, txn_body_dag_init_fs, NULL,
> + TRUE, fs->pool);
> }
>
>
>
>
Received on 2009-04-09 11:31:27 CEST