svn commit: r1400669 - /subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

[st...@apache.org]

Author: stefan2
Date: Sun Oct 21 16:07:36 2012
New Revision: 1400669

URL: http://svn.apache.org/viewvc?rev=1400669&view=rev
Log:
Fix a hole in the recovery code after a revprop writer got killed.

The situation that gets fixed here is:
- the atomic replacement of the revprop got delayed for more than
  the 10s timeout
- some reader already bumped the generation after that timeout
- the the replacement eventually went through after that
- the writer got killed before it could bump the revprop geneneration
  once more
=> revprop caches don't get invalidated after the write

Now, me make sure to bump the revprop generation after a timeout
only when the writing process actually finished or got terminated.
We do that by grabbing the global write lock.

* subversion/libsvn_fs_fs/fs_fs.c
  (revprop_generation_fixup_t): new baton type
  (revprop_generation_fixup): new function that bumps the revprop
   generation after a failed / interrupted write
  (read_revprop_generation): make sure we do the fixup only when the
   original writer no longer exists

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c?rev=1400669&r1=1400668&r2=1400669&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Sun Oct 21 16:07:36 2012
@@ -3268,6 +3268,46 @@ has_revprop_cache(svn_fs_t *fs, apr_pool
   return TRUE;
 }
 
+/* Baton structure for revprop_generation_fixup. */
+typedef struct revprop_generation_fixup_t
+{
+  /* revprop generation to read */
+  apr_int64_t *generation;
+
+  /* containing the revprop_generation member to query */
+  fs_fs_data_t *ffd;
+} revprop_generation_upgrade_t;
+
+/* If the revprop generation has an odd value, it means the original writer
+   of the revprop got killed. We don't know whether that process as able
+   to change the revprop data but we assume that it was. Therefore, we
+   increase the generation in that case to basically invalidate everyones
+   cache content.
+   Execute this onlx while holding the write lock to the repo in baton->FFD.
+ */
+static svn_error_t *
+revprop_generation_fixup(void *void_baton,
+                         apr_pool_t *pool)
+{
+  revprop_generation_upgrade_t *baton = void_baton;
+  assert(baton->ffd->has_write_lock);
+  
+  /* Maybe, either the original revprop writer or some other reader has
+     already corrected / bumped the revprop generation.  Thus, we need
+     to read it again. */
+  SVN_ERR(svn_named_atomic__read(baton->generation,
+                                 baton->ffd->revprop_generation));
+
+  /* Cause everyone to re-read revprops upon their next access, if the
+     last revprop write did not complete properly. */
+  while (*baton->generation % 2)
+    SVN_ERR(svn_named_atomic__add(baton->generation,
+                                  1,
+                                  baton->ffd->revprop_generation));
+
+  return SVN_NO_ERROR;
+}
+
 /* Read the current revprop generation and return it in *GENERATION.
    Also, detect aborted / crashed writers and recover from that.
    Use the access object in FS to set the shared mem values. */
@@ -3297,13 +3337,19 @@ read_revprop_generation(apr_int64_t *gen
        */
       if (apr_time_now() > timeout)
         {
-          /* Cause everyone to re-read revprops upon their next access.
-           * Keep in mind that we may not be the only one trying to do it.
+          revprop_generation_upgrade_t baton;
+          baton.generation = &current;
+          baton.ffd = ffd;
+
+          /* Ensure that the original writer process no longer exists by
+           * acquiring the write lock to this repository.  Then, fix up
+           * the revprop generation.
            */
-          while (current % 2)
-            SVN_ERR(svn_named_atomic__add(&current,
-                                          1,
-                                          ffd->revprop_generation));
+          if (ffd->has_write_lock)
+            SVN_ERR(revprop_generation_fixup(&baton, pool));
+          else
+            SVN_ERR(svn_fs_fs__with_write_lock(fs, revprop_generation_fixup,
+                                               &baton, pool));
         }
     }