[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] Potentially controversial BDB backend pool usage changes gstein will probably fuss about

From: C. Michael Pilato <cmpilato_at_collab.net>
Date: Thu, 09 Apr 2009 04:20:53 -0400

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 10:21:18 CEST

This is an archived mail posted to the Subversion Dev mailing list.