Philip Martin <philip.mar...@wandisco.com> writes:

> If I use the debugger to manually set target->node_revision to NULL
> inside svn_fs_fs__dag_increment_mergeinfo_count then the commit works.
> I'm not exactly sure how all the FSFS caching layers are supposed to
> interact.  Is tree.c:update_ancestry supposed to update the in-memory
> predecessor_count?  Should there be a svn_fs_fs__dag_xxx function to
> change the predecessor count?  Should target->node_revision be set to
> NULL soemwehere?  Something else?

Moving update_ancestry from tree.c to dag.c is one way to fix the
problem.  Daniel also suggested removing the node_revision member of
dag_node_t altogether and relying on new 1.7 caching to give us the
performance.  I suppose we would still need a patch like this for 1.6.


Index: ../src/subversion/libsvn_fs_fs/dag.c
===================================================================
--- ../src/subversion/libsvn_fs_fs/dag.c        (revision 1302591)
+++ ../src/subversion/libsvn_fs_fs/dag.c        (working copy)
@@ -1296,3 +1296,27 @@
 
   return SVN_NO_ERROR;
 }
+
+svn_error_t *
+svn_fs_fs__dag_update_ancestry(dag_node_t *target,
+                               dag_node_t *source,
+                               apr_pool_t *pool)
+{
+  node_revision_t *source_noderev, *target_noderev;
+
+  if (! svn_fs_fs__dag_check_mutable(target))
+    return svn_error_createf
+      (SVN_ERR_FS_NOT_MUTABLE, NULL,
+       _("Attempted to update ancestry of non-mutable node"));
+
+  SVN_ERR(get_node_revision(&source_noderev, source, pool));
+  SVN_ERR(get_node_revision(&target_noderev, target, pool));
+
+  target_noderev->predecessor_id = source->id;
+  target_noderev->predecessor_count = source_noderev->predecessor_count;
+  if (target_noderev->predecessor_count != -1)
+    target_noderev->predecessor_count++;
+
+  return svn_fs_fs__put_node_revision(target->fs, target->id, target_noderev,
+                                      FALSE, pool);
+}
Index: ../src/subversion/libsvn_fs_fs/tree.c
===================================================================
--- ../src/subversion/libsvn_fs_fs/tree.c       (revision 1302591)
+++ ../src/subversion/libsvn_fs_fs/tree.c       (working copy)
@@ -1142,32 +1142,6 @@
 }
 
 
-/* Teach node-revision TARGET_ID that node-revision SOURCE_ID is its
-   predecessor.  TARGET_PATH is used for error messages only. */
-static svn_error_t *
-update_ancestry(svn_fs_t *fs,
-                const svn_fs_id_t *source_id,
-                const svn_fs_id_t *target_id,
-                const char *target_path,
-                int source_pred_count,
-                apr_pool_t *pool)
-{
-  node_revision_t *noderev;
-
-  if (svn_fs_fs__id_txn_id(target_id) == NULL)
-    return svn_error_createf
-      (SVN_ERR_FS_NOT_MUTABLE, NULL,
-       _("Unexpected immutable node at '%s'"), target_path);
-
-  SVN_ERR(svn_fs_fs__get_node_revision(&noderev, fs, target_id, pool));
-  noderev->predecessor_id = source_id;
-  noderev->predecessor_count = source_pred_count;
-  if (noderev->predecessor_count != -1)
-    noderev->predecessor_count++;
-  return svn_fs_fs__put_node_revision(fs, target_id, noderev, FALSE, pool);
-}
-
-
 /* Set the contents of CONFLICT_PATH to PATH, and return an
    SVN_ERR_FS_CONFLICT error that indicates that there was a conflict
    at PATH.  Perform all allocations in POOL (except the allocation of
@@ -1219,7 +1193,6 @@
   apr_hash_index_t *hi;
   svn_fs_t *fs;
   apr_pool_t *iterpool;
-  int pred_count;
   apr_int64_t mergeinfo_increment = 0;
 
   /* Make sure everyone comes from the same filesystem. */
@@ -1543,9 +1516,7 @@
     }
   svn_pool_destroy(iterpool);
 
-  SVN_ERR(svn_fs_fs__dag_get_predecessor_count(&pred_count, source, pool));
-  SVN_ERR(update_ancestry(fs, source_id, target_id, target_path,
-                          pred_count, pool));
+  SVN_ERR(svn_fs_fs__dag_update_ancestry(target, source, pool));
 
   if (svn_fs_fs__fs_supports_mergeinfo(fs))
     SVN_ERR(svn_fs_fs__dag_increment_mergeinfo_count(target,
Index: ../src/subversion/libsvn_fs_fs/dag.h
===================================================================
--- ../src/subversion/libsvn_fs_fs/dag.h        (revision 1302591)
+++ ../src/subversion/libsvn_fs_fs/dag.h        (working copy)
@@ -603,6 +603,10 @@
                                               dag_node_t *node,
                                               apr_pool_t *pool);
 
+svn_error_t *
+svn_fs_fs__dag_update_ancestry(dag_node_t *target,
+                               dag_node_t *source,
+                               apr_pool_t *pool);
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Reply via email to