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 2012/03/08 20:28:16 UTC

svn commit: r1298530 - in /subversion/branches/revprop-cache/subversion/libsvn_fs_fs: caching.c fs.h fs_fs.c

Author: stefan2
Date: Thu Mar  8 19:28:15 2012
New Revision: 1298530

URL: http://svn.apache.org/viewvc?rev=1298530&view=rev
Log:
On the revprop-cache branch:
Re-apply revision(s) 1297211, 1296610, 1296604 from subversion/trunk:
Follow-up to r1296610:

* subversion/libsvn_fs_fs/fs_fs.c
  (increment_revprop_generation): Use the correct printf format.

........
Follow-up to r1296604: Fix build.

* subversion/libsvn_fs_fs/fs_fs.c
  (increment_revprop_generation): put declaration at the begin of the block
........
Certain operations, e.g. svn ls -v, will contain timestamp and author
information from many different revisions.  A list of all projects
in the root of the wordpress repository will open, read and close
>75.000 revision property files (3 reads for each list entry)

This commit implements revprop caching.  It will be activated as
part of the full-text caching option.

Since revprops may be written by other threads or processes, we
need to track the revprop changes.  A new special file contains a
counter that will be increased each time revision properties get
rewritten.

This counter is internally called "revprop generation" and will be
read upon the first revprop access for given fs_t.  Later changes
may remain invisible for that fs_t.  This behavior is in line with
our revprop handling in other parts of FS_FS.  If a revprop gets
rewritten, the fs_t doing the write will use the new generation
from that point on and will thus see all caches up to and including
its own.

Since the revprop generation becomes part of the cache key, each
fs_t will only see revprops from its generation.  It may also
create new cache entries tagged with that generation, i.e. those
would appear to be outdated for newer fs_t.  But that will simply 
cause a benign false negative upon lookup.  No fs_t will see
data that got replaced before that fs_t was created.

* subversion/libsvn_fs_fs/fs.h
  (PATH_REVPROP_GEN): declare name of new special file
  (fs_fs_data_t): add member to hold that file's content;
   add new cache for revprops
* subversion/libsvn_fs_fs/caching.c
  (svn_fs_fs__initialize_caches): initialize revprop cache

* subversion/libsvn_fs_fs/fs_fs.c
  (path_revprop_generation): path constructor for new special file
  (check_file_buffer_numeric): new, more generic utility function
  (check_format_file_buffer_numeric): call the new utility
  (read_revprop_generation, check_revprop_generation,
   increment_revprop_generation): new functions to read & write
   the revprop generation from / to our new special file
  (set_revision_proplist): if we change existing revprops, increment
   the revprop generation
  (revision_proplist): use and populate the revprop cache
  (svn_fs_fs__create): initialize our new special file
  (hotcopy_create_empty_dest): hotcopy the new special file

........



........

Modified:
    subversion/branches/revprop-cache/subversion/libsvn_fs_fs/caching.c
    subversion/branches/revprop-cache/subversion/libsvn_fs_fs/fs.h
    subversion/branches/revprop-cache/subversion/libsvn_fs_fs/fs_fs.c

Modified: subversion/branches/revprop-cache/subversion/libsvn_fs_fs/caching.c
URL: http://svn.apache.org/viewvc/subversion/branches/revprop-cache/subversion/libsvn_fs_fs/caching.c?rev=1298530&r1=1298529&r2=1298530&view=diff
==============================================================================
--- subversion/branches/revprop-cache/subversion/libsvn_fs_fs/caching.c (original)
+++ subversion/branches/revprop-cache/subversion/libsvn_fs_fs/caching.c Thu Mar  8 19:28:15 2012
@@ -339,6 +339,27 @@ svn_fs_fs__initialize_caches(svn_fs_t *f
 
   SVN_ERR(init_callbacks(ffd->fulltext_cache, fs, no_handler, pool));
 
+  /* initialize revprop cache, if full-text caching has been enabled */
+  if (cache_fulltexts)
+    {
+      SVN_ERR(create_cache(&(ffd->revprop_cache),
+                           NULL,
+                           membuffer,
+                           0, 0, /* Do not use inprocess cache */
+                           svn_fs_fs__serialize_properties,
+                           svn_fs_fs__deserialize_properties,
+                           APR_HASH_KEY_STRING,
+                           apr_pstrcat(pool, prefix, "REVPROP",
+                                       (char *)NULL),
+                           fs->pool));
+    }
+  else
+    {
+      ffd->revprop_cache = NULL;
+    }
+
+  SVN_ERR(init_callbacks(ffd->revprop_cache, fs, no_handler, pool));
+
   /* initialize txdelta window cache, if that has been enabled */
   if (cache_txdeltas)
     {

Modified: subversion/branches/revprop-cache/subversion/libsvn_fs_fs/fs.h
URL: http://svn.apache.org/viewvc/subversion/branches/revprop-cache/subversion/libsvn_fs_fs/fs.h?rev=1298530&r1=1298529&r2=1298530&view=diff
==============================================================================
--- subversion/branches/revprop-cache/subversion/libsvn_fs_fs/fs.h (original)
+++ subversion/branches/revprop-cache/subversion/libsvn_fs_fs/fs.h Thu Mar  8 19:28:15 2012
@@ -60,6 +60,8 @@ extern "C" {
 #define PATH_LOCKS_DIR        "locks"            /* Directory of locks */
 #define PATH_MIN_UNPACKED_REV "min-unpacked-rev" /* Oldest revision which
                                                     has not been packed. */
+#define PATH_REVPROP_GEN      "revprop-gen"      /* File containing the
+                                                    revprop change counter */
 /* If you change this, look at tests/svn_test_fs.c(maybe_install_fsfs_conf) */
 #define PATH_CONFIG           "fsfs.conf"        /* Configuration */
 
@@ -198,7 +200,8 @@ typedef struct fs_fs_shared_data_t
   apr_pool_t *common_pool;
 } fs_fs_shared_data_t;
 
-/* Private (non-shared) FSFS-specific data for each svn_fs_t object. */
+/* Private (non-shared) FSFS-specific data for each svn_fs_t object.
+   Any caches in here may be NULL. */
 typedef struct fs_fs_data_t
 {
   /* The format number of this FS. */
@@ -237,6 +240,15 @@ typedef struct fs_fs_data_t
      rep key (revision/offset) to svn_string_t. */
   svn_cache__t *fulltext_cache;
 
+  /* Revprop "generation" that is valid for this svn_fs_t.
+     It is the revprop change counter read once from "revprop-gen".
+     Will be read upon first access.  0 means that the value has not
+     been read from disk, yet. */
+  apr_int64_t revprop_generation;
+  
+  /* Revision property cache.  Maps from (rev,generation) to apr_hash_t. */
+  svn_cache__t *revprop_cache;
+
   /* Pack manifest cache; a cache mapping (svn_revnum_t) shard number to
      a manifest; and a manifest is a mapping from (svn_revnum_t) revision
      number offset within a shard to (apr_off_t) byte-offset in the

Modified: subversion/branches/revprop-cache/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/branches/revprop-cache/subversion/libsvn_fs_fs/fs_fs.c?rev=1298530&r1=1298529&r2=1298530&view=diff
==============================================================================
--- subversion/branches/revprop-cache/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/branches/revprop-cache/subversion/libsvn_fs_fs/fs_fs.c Thu Mar  8 19:28:15 2012
@@ -225,6 +225,12 @@ path_lock(svn_fs_t *fs, apr_pool_t *pool
 }
 
 static const char *
+path_revprop_generation(svn_fs_t *fs, apr_pool_t *pool)
+{
+  return svn_dirent_join(fs->path, PATH_REVPROP_GEN, pool);
+}
+
+static const char *
 path_rev_packed(svn_fs_t *fs, svn_revnum_t rev, const char *kind,
                 apr_pool_t *pool)
 {
@@ -870,25 +876,39 @@ get_file_offset(apr_off_t *offset_p, apr
 }
 
 
-/* Check that BUF, a nul-terminated buffer of text from format file PATH,
+/* Check that BUF, a nul-terminated buffer of text from file PATH,
    contains only digits at OFFSET and beyond, raising an error if not.
+   TITLE contains a user-visible description of the file, usually the
+   short file name.
 
    Uses POOL for temporary allocation. */
 static svn_error_t *
-check_format_file_buffer_numeric(const char *buf, apr_off_t offset,
-                                 const char *path, apr_pool_t *pool)
+check_file_buffer_numeric(const char *buf, apr_off_t offset,
+                          const char *path, const char *title,
+                          apr_pool_t *pool)
 {
   const char *p;
 
   for (p = buf + offset; *p; p++)
     if (!svn_ctype_isdigit(*p))
       return svn_error_createf(SVN_ERR_BAD_VERSION_FILE_FORMAT, NULL,
-        _("Format file '%s' contains unexpected non-digit '%c' within '%s'"),
-        svn_dirent_local_style(path, pool), *p, buf);
+        _("%s file '%s' contains unexpected non-digit '%c' within '%s'"),
+        title, svn_dirent_local_style(path, pool), *p, buf);
 
   return SVN_NO_ERROR;
 }
 
+/* Check that BUF, a nul-terminated buffer of text from format file PATH,
+   contains only digits at OFFSET and beyond, raising an error if not.
+
+   Uses POOL for temporary allocation. */
+static svn_error_t *
+check_format_file_buffer_numeric(const char *buf, apr_off_t offset,
+                                 const char *path, apr_pool_t *pool)
+{
+  return check_file_buffer_numeric(buf, offset, path, "Format", pool);
+}
+
 /* Read the format number and maximum number of files per directory
    from PATH and return them in *PFORMAT and *MAX_FILES_PER_DIR
    respectively.
@@ -2687,6 +2707,105 @@ svn_fs_fs__rev_get_root(svn_fs_id_t **ro
   return SVN_NO_ERROR;
 }
 
+/* Reads the revprop_gen file and writes the content into the
+   REVPROP_GENERATION member of FS.  Use pool for allocations. */
+static svn_error_t *
+read_revprop_generation(svn_fs_t *fs,
+                        apr_pool_t *pool)
+{
+  fs_fs_data_t *ffd = fs->fsap_data;
+  
+  const char *path = path_revprop_generation(fs, pool);
+  svn_error_t *err;
+  char buf[80];
+  int i;
+
+  /* Read the raw data from the file, if it exists. If it does
+     not, set the generation to "1" and return.
+     We don't want to have this function fail.  So, patiently
+     retry a couple of times the case the OS denied us access. */
+  apr_pool_t *iterpool = svn_pool_create(pool);
+  for (i = 0; i < RECOVERABLE_RETRY_COUNT; ++i)
+    {
+      apr_file_t *file;
+      apr_size_t len = sizeof(buf);
+      
+      svn_pool_clear(iterpool);
+
+      err = svn_io_file_open(&file, path, APR_READ | APR_BUFFERED,
+                            APR_OS_DEFAULT, iterpool);
+      if (err && APR_STATUS_IS_ENOENT(err->apr_err))
+        {
+          /* No-one changed a revprop -> we are still at gen 1. */
+          ffd->revprop_generation = 1;
+          svn_error_clear(err);
+          return SVN_NO_ERROR;
+        }
+      svn_error_clear(err);
+
+      RETRY_RECOVERABLE(err, file,
+                        svn_io_read_length_line(file,
+                                                buf,
+                                                &len,
+                                                iterpool));
+      IGNORE_RECOVERABLE(err, svn_io_file_close(file,
+                                                iterpool));
+
+      break;
+    }
+  SVN_ERR(err);
+
+  svn_pool_destroy(iterpool);
+
+  /* Check that the first line contains only digits. */
+  SVN_ERR(check_file_buffer_numeric(buf, 0, path, 
+                                    "Revprop generations", pool));
+  SVN_ERR(svn_cstring_atoi64(&ffd->revprop_generation, buf));
+
+  /* Graceful behavior in case someone put a "0" in the file. */
+  if (ffd->revprop_generation <= 0)
+    ffd->revprop_generation = 1;
+
+  return SVN_NO_ERROR;
+}
+
+static svn_error_t *
+check_revprop_generation(svn_fs_t *fs,
+                         apr_pool_t *pool)
+{
+  fs_fs_data_t *ffd = fs->fsap_data;
+  
+  return ffd->revprop_generation == 0
+    ? read_revprop_generation(fs, pool)
+    : SVN_NO_ERROR;
+}
+
+static svn_error_t *
+increment_revprop_generation(svn_fs_t *fs,
+                             apr_pool_t *pool)
+{
+  fs_fs_data_t *ffd = fs->fsap_data;
+  
+  const char *path = path_revprop_generation(fs, pool);
+  const char *tmp_filename;
+  svn_string_t *generation;
+
+  SVN_ERR(read_revprop_generation(fs, pool));
+
+  /* Increment the key and add a trailing \n to the string so the
+     txn-current file has a newline in it. */
+  ++ffd->revprop_generation;
+  generation = svn_string_createf(pool, "%" APR_INT64_T_FMT "\n",
+                                  ffd->revprop_generation);
+
+  SVN_ERR(svn_io_write_unique(&tmp_filename,
+                              svn_dirent_dirname(path, pool),
+                              generation->data, generation->len,
+                              svn_io_file_del_none, pool));
+  return move_into_place(tmp_filename, path, 
+                         svn_fs_fs__path_current(fs, pool), pool);
+}
+
 /* Set the revision property list of revision REV in filesystem FS to
    PROPLIST.  Use POOL for temporary allocations. */
 static svn_error_t *
@@ -2703,6 +2822,10 @@ set_revision_proplist(svn_fs_t *fs,
       const char *tmp_path;
       const char *perms_reference;
       svn_stream_t *stream;
+      svn_node_kind_t kind;
+
+      /* test whether revprops already exist for this revision */
+      SVN_ERR(svn_io_check_path(final_path, &kind, pool));
 
       /* ### do we have a directory sitting around already? we really shouldn't
          ### have to get the dirname here. */
@@ -2719,6 +2842,12 @@ set_revision_proplist(svn_fs_t *fs,
       SVN_ERR(svn_fs_fs__path_rev_absolute(&perms_reference, fs, rev, pool));
       SVN_ERR(move_into_place(tmp_path, final_path, perms_reference, pool));
 
+      /* Invalidate all cached revprops for this FS and for all other
+         users that haven't read any revprops, YET.  Since writing revprops
+         implies a write lock, there can be no races. */
+      if (kind != svn_node_none)
+        SVN_ERR(increment_revprop_generation(fs, pool));
+
       return SVN_NO_ERROR;
     }
 
@@ -2732,8 +2861,22 @@ revision_proplist(apr_hash_t **proplist_
                   apr_pool_t *pool)
 {
   apr_hash_t *proplist;
+  fs_fs_data_t *ffd = fs->fsap_data;
+  const char *key;
 
   SVN_ERR(ensure_revision_exists(fs, rev, pool));
+  SVN_ERR(check_revprop_generation(fs, pool));
+
+  /* Try cache lookup first. */
+  key = svn_fs_fs__combine_two_numbers(rev, ffd->revprop_generation, pool);
+  if (ffd->revprop_cache)
+    {
+      svn_boolean_t is_cached;
+      SVN_ERR(svn_cache__get((void **) proplist_p, &is_cached,
+                              ffd->revprop_cache, key, pool));
+      if (is_cached)
+        return SVN_NO_ERROR;
+    }
 
   /* if (1); null condition for easier merging to revprop-packing */
     {
@@ -2789,6 +2932,10 @@ revision_proplist(apr_hash_t **proplist_
       svn_pool_destroy(iterpool);
     }
 
+  /* Cache the result, if caching has been activated. */
+  if (ffd->revprop_cache)
+    SVN_ERR(svn_cache__set(ffd->revprop_cache, key, proplist, pool));
+
   *proplist_p = proplist;
 
   return SVN_NO_ERROR;
@@ -6762,6 +6909,9 @@ svn_fs_fs__create(svn_fs_t *fs,
                                  "", pool));
     }
 
+  /* Create the revprop generation tracking file. */
+  SVN_ERR(increment_revprop_generation(fs, pool));
+
   /* This filesystem is ready.  Stamp it with a format number. */
   SVN_ERR(write_format(path_format(fs, pool),
                        ffd->format, ffd->max_files_per_dir, FALSE, pool));
@@ -8812,6 +8962,7 @@ hotcopy_create_empty_dest(svn_fs_t *src_
 {
   fs_fs_data_t *src_ffd = src_fs->fsap_data;
   fs_fs_data_t *dst_ffd = dst_fs->fsap_data;
+  svn_node_kind_t kind;
 
   dst_fs->path = apr_pstrdup(pool, dst_path);
 
@@ -8875,6 +9026,14 @@ hotcopy_create_empty_dest(svn_fs_t *src_
                                  "", pool));
     }
 
+  /* Copy the revprop generation file if it exists in SRC_FS. */
+  SVN_ERR(svn_io_check_path(path_revprop_generation(src_fs, pool),
+                            &kind, pool));
+  if (kind == svn_node_file)
+    SVN_ERR(svn_io_copy_file(path_revprop_generation(src_fs, pool),
+                             path_revprop_generation(dst_fs, pool),
+                             TRUE, pool));
+    
   dst_ffd->youngest_rev_cache = 0;
   return SVN_NO_ERROR;
 }