Re: #4129 is reproducible Re: predecessor count for the root node-revision is wrong message

Philip Martin <philip.martin_at_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