svn commit: r1623402 - /subversion/trunk/subversion/libsvn_fs_fs/caching.c

[ko...@apache.org]

Author: kotkov
Date: Mon Sep  8 14:15:25 2014
New Revision: 1623402

URL: http://svn.apache.org/r1623402
Log:
Do not use the recently introduced instance IDs in the cache keys (as per
discussion in http://svn.haxx.se/dev/archive-2014-08/0239.shtml).  Utilize
them only when accessing the filesystem shared data.

We were trying to avoid stale cache entries in a situation when a repository
is hot swapped with an older version of itself.  However, doing so might give
the users a false sense of security, as long as the instance IDs do not work
on copy-pasted or snapshotted repositories.  Users are unlikely to actually
corrupt the repository with or without the instance IDs (the race is quite
hard to hit, see below).  However, *without* the instance IDs in the cache
keys, they are going to receive different sort of errors both on the client
and on the server, e.g.:

  E200014: Checksum mismatch for 'path'
  E200014: Checksum mismatch while reading representation
  E160004: Malformed svndiff data in representation"

These errors should give the users a hint that what they're doing is wrong
and will vanish after the server restart.  We consider this a better choice
compared to silently narrowing the window of the problem.

A bit more on the possible repository corruption — the race is triggerable
with the following sequence of actions.  The introduction of the instance IDs
does not have an ability to solve the problem, but I suppose there might be
another way (and I am probably going to track this issue separately):

- We start with an empty repository served by Apache HTTP Server
- Then we create a backup of the empty repository with 'cp repository'
- svn checkout [wc1]; echo A > file ; svn add file ; svn commit
- Now we perform a hot swap of the backup (with 0 revisions)
- svn checkout [wc2]; echo B > file ; svn add file ; svn commit
- Now, within [wc1] — svn cp file file-copy ; svn commit

  (this will corrupt the repository with 'svn diff -c 2' returning a
   "svn: E200014: Checksum mismatch for 'file" error)

* subversion/libsvn_fs_fs/caching.c
  (svn_fs_fs__initialize_caches, svn_fs_fs__initialize_txn_caches):
   Do not use the instance IDs in the cache keys.

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

Modified: subversion/trunk/subversion/libsvn_fs_fs/caching.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/caching.c?rev=1623402&r1=1623401&r2=1623402&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/caching.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/caching.c Mon Sep  8 14:15:25 2014
@@ -347,7 +347,6 @@ svn_fs_fs__initialize_caches(svn_fs_t *f
   fs_fs_data_t *ffd = fs->fsap_data;
   const char *prefix = apr_pstrcat(pool,
                                    "fsfs:", fs->uuid,
-                                   ":", ffd->instance_id,
                                    "/", normalize_key_part(fs->path, pool),
                                    ":",
                                    SVN_VA_NULL);
@@ -774,7 +773,6 @@ svn_fs_fs__initialize_txn_caches(svn_fs_
      Therefore, throw in a uuid as well - just to be sure. */
   const char *prefix = apr_pstrcat(pool,
                                    "fsfs:", fs->uuid,
-                                   ":", ffd->instance_id,
                                    "/", fs->path,
                                    ":", txn_id,
                                    ":", svn_uuid_generate(pool), ":",