subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From bre...@apache.org
Subject svn commit: r1516022 - in /subversion/branches/1.8.x: ./ STATUS subversion/libsvn_fs_fs/fs_fs.c subversion/tests/libsvn_fs_fs/fs-pack-test.c
Date Tue, 20 Aug 2013 23:10:42 GMT
Author: breser
Date: Tue Aug 20 23:10:41 2013
New Revision: 1516022

URL: http://svn.apache.org/r1516022
Log:
Merge the 1.8.x-r1513879 branch:

 * r1513879, r1513880, r1515516
   Fix packed revprop file splitting logic in FSFS
   Justification:
     Modifying revprops may result in the loss of revprops for one
     or multiple revisions.
   Branch: ^/subversion/branches/1.8.x-r1513879
   Notes:
     I added r1515534 to the branch as an obvious fix.
   Votes:
     +1: stefan2, danielsh, philip

Modified:
    subversion/branches/1.8.x/   (props changed)
    subversion/branches/1.8.x/STATUS
    subversion/branches/1.8.x/subversion/libsvn_fs_fs/fs_fs.c
    subversion/branches/1.8.x/subversion/tests/libsvn_fs_fs/fs-pack-test.c

Propchange: subversion/branches/1.8.x/
------------------------------------------------------------------------------
  Merged /subversion/branches/1.8.x-r1513879:r1514699-1516021
  Merged /subversion/trunk:r1513879-1513880,1515516,1515534

Modified: subversion/branches/1.8.x/STATUS
URL: http://svn.apache.org/viewvc/subversion/branches/1.8.x/STATUS?rev=1516022&r1=1516021&r2=1516022&view=diff
==============================================================================
--- subversion/branches/1.8.x/STATUS (original)
+++ subversion/branches/1.8.x/STATUS Tue Aug 20 23:10:41 2013
@@ -120,13 +120,3 @@ Veto-blocked changes:
 Approved changes:
 =================
 
- * r1513879, r1513880, r1515516
-   Fix packed revprop file splitting logic in FSFS
-   Justification:
-     Modifying revprops may result in the loss of revprops for one
-     or multiple revisions.
-   Branch: ^/subversion/branches/1.8.x-r1513879
-   Notes:
-     I added r1515534 to the branch as an obvious fix.
-   Votes:
-     +1: stefan2, danielsh, philip

Modified: subversion/branches/1.8.x/subversion/libsvn_fs_fs/fs_fs.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.8.x/subversion/libsvn_fs_fs/fs_fs.c?rev=1516022&r1=1516021&r2=1516022&view=diff
==============================================================================
--- subversion/branches/1.8.x/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/branches/1.8.x/subversion/libsvn_fs_fs/fs_fs.c Tue Aug 20 23:10:41 2013
@@ -3536,7 +3536,7 @@ typedef struct packed_revprops_t
   /* sum of values in SIZES */
   apr_size_t total_size;
 
-  /* first revision in the pack */
+  /* first revision in the pack (>= MANIFEST_START) */
   svn_revnum_t start_revision;
 
   /* size of the revprops in PACKED_REVPROPS */
@@ -3550,8 +3550,12 @@ typedef struct packed_revprops_t
    * in the pack, i.e. the pack content without header and compression */
   svn_stringbuf_t *packed_revprops;
 
+  /* First revision covered by MANIFEST.
+   * Will equal the shard start revision or 1, for the 1st shard. */
+  svn_revnum_t manifest_start;
+
   /* content of the manifest.
-   * Maps long(rev - START_REVISION) to const char* pack file name */
+   * Maps long(rev - MANIFEST_START) to const char* pack file name */
   apr_array_header_t *manifest;
 } packed_revprops_t;
 
@@ -3670,9 +3674,11 @@ get_revprop_packname(svn_fs_t *fs,
     }
 
   /* Index for our revision. Rev 0 is excluded from the first shard. */
-  idx = (int)(revprops->revision % ffd->max_files_per_dir);
-  if (revprops->revision < ffd->max_files_per_dir)
-    --idx;
+  revprops->manifest_start = revprops->revision
+                           - (revprops->revision % ffd->max_files_per_dir);
+  if (revprops->manifest_start == 0)
+    ++revprops->manifest_start;
+  idx = (int)(revprops->revision - revprops->manifest_start);
 
   if (revprops->manifest->nelts <= idx)
     return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
@@ -3685,6 +3691,17 @@ get_revprop_packname(svn_fs_t *fs,
   return SVN_NO_ERROR;
 }
 
+/* Return TRUE, if revision R1 and R2 refer to the same shard in FS.
+ */
+static svn_boolean_t
+same_shard(svn_fs_t *fs,
+           svn_revnum_t r1,
+           svn_revnum_t r2)
+{
+  fs_fs_data_t *ffd = fs->fsap_data;
+  return (r1 / ffd->max_files_per_dir) == (r2 / ffd->max_files_per_dir);
+}
+
 /* Given FS and the full packed file content in REVPROPS->PACKED_REVPROPS,
  * fill the START_REVISION, SIZES, OFFSETS members. Also, make
  * PACKED_REVPROPS point to the first serialized revprop.
@@ -3717,6 +3734,25 @@ parse_packed_revprops(svn_fs_t *fs,
   SVN_ERR(read_number_from_stream(&first_rev, NULL, stream, iterpool));
   SVN_ERR(read_number_from_stream(&count, NULL, stream, iterpool));
 
+  /* Check revision range for validity. */
+  if (   !same_shard(fs, revprops->revision, first_rev)
+      || !same_shard(fs, revprops->revision, first_rev + count - 1)
+      || count < 1)
+    return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
+                             _("Revprop pack for revision r%ld"
+                               " contains revprops for r%ld .. r%ld"),
+                             revprops->revision, first_rev,
+                             first_rev + count -1);
+
+  /* Since start & end are in the same shard, it is enough to just test
+   * the FIRST_REV for being actually packed.  That will also cover the
+   * special case of rev 0 never being packed. */
+  if (!is_packed_revprop(fs, first_rev))
+    return svn_error_createf(SVN_ERR_FS_CORRUPT, NULL,
+                             _("Revprop pack for revision r%ld"
+                               " starts at non-packed revisions r%ld"),
+                             revprops->revision, first_rev);
+
   /* make PACKED_REVPROPS point to the first char after the header.
    * This is where the serialized revprops are. */
   header_end = strstr(uncompressed->data, "\n\n");
@@ -4120,7 +4156,8 @@ repack_revprops(svn_fs_t *fs,
   return SVN_NO_ERROR;
 }
 
-/* Allocate a new pack file name for the revisions at index [START,END)
+/* Allocate a new pack file name for revisions
+ *     [REVPROPS->START_REVISION + START, REVPROPS->START_REVISION + END - 1]
  * of REVPROPS->MANIFEST.  Add the name of old file to FILES_TO_DELETE,
  * auto-create that array if necessary.  Return an open file stream to
  * the new file in *STREAM allocated in POOL.
@@ -4139,10 +4176,13 @@ repack_stream_open(svn_stream_t **stream
   svn_string_t *new_filename;
   int i;
   apr_file_t *file;
+  int manifest_offset
+    = (int)(revprops->start_revision - revprops->manifest_start);
 
   /* get the old (= current) file name and enlist it for later deletion */
-  const char *old_filename
-    = APR_ARRAY_IDX(revprops->manifest, start, const char*);
+  const char *old_filename = APR_ARRAY_IDX(revprops->manifest,
+                                           start + manifest_offset,
+                                           const char*);
 
   if (*files_to_delete == NULL)
     *files_to_delete = apr_array_make(pool, 3, sizeof(const char*));
@@ -4164,7 +4204,8 @@ repack_stream_open(svn_stream_t **stream
 
   /* update the manifest to point to the new file */
   for (i = start; i < end; ++i)
-    APR_ARRAY_IDX(revprops->manifest, i, const char*) = new_filename->data;
+    APR_ARRAY_IDX(revprops->manifest, i + manifest_offset, const char*)
+      = new_filename->data;
 
   /* create a file stream for the new file */
   SVN_ERR(svn_io_file_open(&file, svn_dirent_join(revprops->folder,

Modified: subversion/branches/1.8.x/subversion/tests/libsvn_fs_fs/fs-pack-test.c
URL: http://svn.apache.org/viewvc/subversion/branches/1.8.x/subversion/tests/libsvn_fs_fs/fs-pack-test.c?rev=1516022&r1=1516021&r2=1516022&view=diff
==============================================================================
--- subversion/branches/1.8.x/subversion/tests/libsvn_fs_fs/fs-pack-test.c (original)
+++ subversion/branches/1.8.x/subversion/tests/libsvn_fs_fs/fs-pack-test.c Tue Aug 20 23:10:41
2013
@@ -788,6 +788,75 @@ file_hint_at_shard_boundary(const svn_te
 #undef SHARD_SIZE
 
 /* ------------------------------------------------------------------------ */
+#define REPO_NAME "get_set_multiple_huge_revprops_packed_fs"
+#define SHARD_SIZE 4
+#define MAX_REV 9
+static svn_error_t *
+get_set_multiple_huge_revprops_packed_fs(const svn_test_opts_t *opts,
+                                         apr_pool_t *pool)
+{
+  svn_fs_t *fs;
+  svn_string_t *prop_value;
+  svn_revnum_t rev;
+
+  /* Bail (with success) on known-untestable scenarios */
+  if ((strcmp(opts->fs_type, "fsfs") != 0)
+      || (opts->server_minor_version && (opts->server_minor_version < 7)))
+    return SVN_NO_ERROR;
+
+  /* Create the packed FS and open it. */
+  SVN_ERR(prepare_revprop_repo(&fs, REPO_NAME, MAX_REV, SHARD_SIZE, opts,
+                               pool));
+
+  /* Set commit messages to different values */
+  for (rev = 0; rev <= MAX_REV; ++rev)
+    SVN_ERR(svn_fs_change_rev_prop(fs, rev, SVN_PROP_REVISION_LOG,
+                                   default_log(rev, pool),
+                                   pool));
+
+  /* verify */
+  for (rev = 0; rev <= MAX_REV; ++rev)
+    {
+      SVN_ERR(svn_fs_revision_prop(&prop_value, fs, rev,
+                                   SVN_PROP_REVISION_LOG, pool));
+      SVN_TEST_STRING_ASSERT(prop_value->data, default_log(rev, pool)->data);
+    }
+
+  /* Put a huge revprop into revision 1 and 2. */
+  SVN_ERR(svn_fs_change_rev_prop(fs, 1, SVN_PROP_REVISION_LOG,
+                                 huge_log(1, pool),
+                                 pool));
+  SVN_ERR(svn_fs_change_rev_prop(fs, 2, SVN_PROP_REVISION_LOG,
+                                 huge_log(2, pool),
+                                 pool));
+  SVN_ERR(svn_fs_change_rev_prop(fs, 5, SVN_PROP_REVISION_LOG,
+                                 huge_log(5, pool),
+                                 pool));
+  SVN_ERR(svn_fs_change_rev_prop(fs, 6, SVN_PROP_REVISION_LOG,
+                                 huge_log(6, pool),
+                                 pool));
+
+  /* verify */
+  for (rev = 0; rev <= MAX_REV; ++rev)
+    {
+      SVN_ERR(svn_fs_revision_prop(&prop_value, fs, rev,
+                                   SVN_PROP_REVISION_LOG, pool));
+
+      if (rev == 1 || rev == 2 || rev == 5 || rev == 6)
+        SVN_TEST_STRING_ASSERT(prop_value->data,
+                               huge_log(rev, pool)->data);
+      else
+        SVN_TEST_STRING_ASSERT(prop_value->data,
+                               default_log(rev, pool)->data);
+    }
+
+  return SVN_NO_ERROR;
+}
+#undef REPO_NAME
+#undef MAX_REV
+#undef SHARD_SIZE
+
+/* ------------------------------------------------------------------------ */
 
 /* The test table.  */
 
@@ -812,5 +881,7 @@ struct svn_test_descriptor_t test_funcs[
                        "recover a fully packed filesystem"),
     SVN_TEST_OPTS_PASS(file_hint_at_shard_boundary,
                        "test file hint at shard boundary"),
+    SVN_TEST_OPTS_PASS(get_set_multiple_huge_revprops_packed_fs,
+                       "set multiple huge revprops in packed FSFS"),
     SVN_TEST_NULL
   };



Mime
View raw message