You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2014/05/29 14:50:12 UTC

svn commit: r1598283 - in /subversion/trunk/subversion/libsvn_fs_x: cached_data.c cached_data.h transaction.c verify.c

Author: stefan2
Date: Thu May 29 12:50:12 2014
New Revision: 1598283

URL: http://svn.apache.org/r1598283
Log:
In FSX, use a faster method to verify that a given item is actually a
representation: Simply ask the index.

Because the indexes themselves get cross-checked against one another
and against the rev / pack contents, we have high confidence that this
is nothing but a representation (no overlaps, nothing missing, no
external corruption).  The contents, however, may still be invalid or
its checksum(s) may not match the ones in REP.  But we only need to
guarantee accessibility here.

As a result, we also don't need file HINTs anymore and can simplify
the verfication walker.

* subversion/libsvn_fs_x/cached_data.h
  (svn_fs_x__check_rep): No HINTs needed anymore.

* subversion/libsvn_fs_x/cached_data.c
  (svn_fs_x__check_rep): Simply check that REP can actually be found
                         as a representation in our indexes.

* subversion/libsvn_fs_x/transaction.c
  (get_shared_rep): Update caller to simplified API.

* subversion/libsvn_fs_x/verify.c
  (verify_walker_baton_t): Remove revision file hinting elements.
  (verify_walker,
   verify_rep_cache): Update / simplify users.

Modified:
    subversion/trunk/subversion/libsvn_fs_x/cached_data.c
    subversion/trunk/subversion/libsvn_fs_x/cached_data.h
    subversion/trunk/subversion/libsvn_fs_x/transaction.c
    subversion/trunk/subversion/libsvn_fs_x/verify.c

Modified: subversion/trunk/subversion/libsvn_fs_x/cached_data.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/cached_data.c?rev=1598283&r1=1598282&r2=1598283&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/cached_data.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/cached_data.c Thu May 29 12:50:12 2014
@@ -728,15 +728,32 @@ create_rep_state(rep_state_t **rep_state
 svn_error_t *
 svn_fs_x__check_rep(representation_t *rep,
                     svn_fs_t *fs,
-                    void **hint,
                     apr_pool_t *pool)
 {
-  rep_state_t *rs;
-  svn_fs_x__rep_header_t *rep_header;
+  apr_off_t offset;
+  apr_uint32_t sub_item;
+  svn_fs_x__p2l_entry_t *entry;
+  svn_revnum_t revision = svn_fs_x__get_revnum(rep->id.change_set);
+
+  /* Does REP->ID refer to an actual item? Which one is it? */
+  SVN_ERR(svn_fs_x__item_offset(&offset, &sub_item, fs, &rep->id, pool));
+
+  /* What is the type of that item? */
+  SVN_ERR(svn_fs_x__p2l_entry_lookup(&entry, fs, revision, offset, pool));
 
-  /* ### Should this be using read_rep_line() directly? */
-  SVN_ERR(create_rep_state(&rs, &rep_header, (shared_file_t**)hint, rep,
-                           fs, pool));
+  /* Verify that we've got an item that is actually a representation. */
+  if (   entry == NULL
+      || (   entry->type != SVN_FS_X__ITEM_TYPE_FILE_REP
+          && entry->type != SVN_FS_X__ITEM_TYPE_DIR_REP
+          && entry->type != SVN_FS_X__ITEM_TYPE_FILE_PROPS
+          && entry->type != SVN_FS_X__ITEM_TYPE_DIR_PROPS
+          && entry->type != SVN_FS_X__ITEM_TYPE_REPS_CONT))
+    return svn_error_createf(SVN_ERR_REPOS_CORRUPTED, NULL,
+                              _("No representation found at offset %s "
+                                "for item %" APR_UINT64_T_FMT
+                                " in revision %ld"),
+                              apr_off_t_toa(pool, offset),
+                              rep->id.number, revision);
 
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/libsvn_fs_x/cached_data.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/cached_data.h?rev=1598283&r1=1598282&r2=1598283&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/cached_data.h (original)
+++ subversion/trunk/subversion/libsvn_fs_x/cached_data.h Thu May 29 12:50:12 2014
@@ -55,14 +55,11 @@ svn_fs_x__rev_get_root(svn_fs_id_t **roo
                        svn_revnum_t rev,
                        apr_pool_t *pool);
 
-/* Verify that representation REP in FS can be accessed.  Successive calls
-   to this function should pass a non-NULL value to HINT.  In that case,
-   many file open / close operations can be eliminated.
+/* Verify that representation REP in FS can be accessed.
    Do any allocations in POOL. */
 svn_error_t *
 svn_fs_x__check_rep(representation_t *rep,
                     svn_fs_t *fs,
-                    void **hint,
                     apr_pool_t *pool);
 
 /* Follow the representation delta chain in FS starting with REP.  The

Modified: subversion/trunk/subversion/libsvn_fs_x/transaction.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/transaction.c?rev=1598283&r1=1598282&r2=1598283&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/transaction.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/transaction.c Thu May 29 12:50:12 2014
@@ -2216,7 +2216,7 @@ get_shared_rep(representation_t **old_re
       if (err == SVN_NO_ERROR)
         {
           if (*old_rep)
-            SVN_ERR(svn_fs_x__check_rep(*old_rep, fs, NULL, pool));
+            SVN_ERR(svn_fs_x__check_rep(*old_rep, fs, pool));
         }
       else if (err->apr_err == SVN_ERR_FS_CORRUPT
                || SVN_ERROR_IN_CATEGORY(err->apr_err,

Modified: subversion/trunk/subversion/libsvn_fs_x/verify.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_x/verify.c?rev=1598283&r1=1598282&r2=1598283&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_x/verify.c (original)
+++ subversion/trunk/subversion/libsvn_fs_x/verify.c Thu May 29 12:50:12 2014
@@ -36,18 +36,14 @@
 
 /** Verifying. **/
 
-/* Baton type expected by verify_walker().  The purpose is to reuse open
- * rev / pack file handles between calls.  Its contents need to be cleaned
- * periodically to limit resource usage.
+/* Baton type expected by verify_walker().  The purpose is to limit the
+ * number of notifications sent.
  */
 typedef struct verify_walker_baton_t
 {
   /* number of calls to verify_walker() since the last clean */
   int iteration_count;
 
-  /* number of files opened since the last clean */
-  int file_count;
-
   /* progress notification callback to invoke periodically (may be NULL) */
   svn_fs_progress_notify_func_t notify_func;
 
@@ -56,12 +52,6 @@ typedef struct verify_walker_baton_t
 
   /* remember the last revision for which we called notify_func */
   svn_revnum_t last_notified_revision;
-
-  /* cached hint for successive calls to svn_fs_x__check_rep() */
-  void *hint;
-
-  /* pool to use for the file handles etc. */
-  apr_pool_t *pool;
 } verify_walker_baton_t;
 
 /* Used by svn_fs_x__verify().
@@ -73,11 +63,9 @@ verify_walker(representation_t *rep,
               apr_pool_t *scratch_pool)
 {
   verify_walker_baton_t *walker_baton = baton;
-  void *previous_hint;
 
   /* notify and free resources periodically */
-  if (   walker_baton->iteration_count > 1000
-      || walker_baton->file_count > 16)
+  if (walker_baton->iteration_count > 1000)
     {
       svn_revnum_t revision = svn_fs_x__get_revnum(rep->id.change_set);
       if (   walker_baton->notify_func
@@ -89,22 +77,14 @@ verify_walker(representation_t *rep,
           walker_baton->last_notified_revision = revision;
         }
 
-      svn_pool_clear(walker_baton->pool);
-
       walker_baton->iteration_count = 0;
-      walker_baton->file_count = 0;
-      walker_baton->hint = NULL;
     }
 
   /* access the repo data */
-  previous_hint = walker_baton->hint;
-  SVN_ERR(svn_fs_x__check_rep(rep, fs, &walker_baton->hint,
-                               walker_baton->pool));
+  SVN_ERR(svn_fs_x__check_rep(rep, fs, scratch_pool));
 
   /* update resource usage counters */
   walker_baton->iteration_count++;
-  if (previous_hint != walker_baton->hint)
-    walker_baton->file_count++;
 
   return SVN_NO_ERROR;
 }
@@ -133,14 +113,13 @@ verify_rep_cache(svn_fs_t *fs,
       /* provide a baton to allow the reuse of open file handles between
          iterations (saves 2/3 of OS level file operations). */
       verify_walker_baton_t *baton = apr_pcalloc(pool, sizeof(*baton));
-      baton->pool = svn_pool_create(pool);
       baton->last_notified_revision = SVN_INVALID_REVNUM;
       baton->notify_func = notify_func;
       baton->notify_baton = notify_baton;
 
       /* tell the user that we are now ready to do *something* */
       if (notify_func)
-        notify_func(SVN_INVALID_REVNUM, notify_baton, baton->pool);
+        notify_func(SVN_INVALID_REVNUM, notify_baton, pool);
 
       /* Do not attempt to walk the rep-cache database if its file does
          not exist,  since doing so would create it --- which may confuse
@@ -149,9 +128,6 @@ verify_rep_cache(svn_fs_t *fs,
                                            verify_walker, baton,
                                            cancel_func, cancel_baton,
                                            pool));
-
-      /* walker resource cleanup */
-      svn_pool_destroy(baton->pool);
     }
 
   return SVN_NO_ERROR;