You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by br...@apache.org on 2013/08/21 01:10:42 UTC
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
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
};