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 2014/09/27 12:52:45 UTC
svn commit: r1627949 - in /subversion/trunk/subversion: libsvn_fs_fs/fs_fs.c
libsvn_fs_fs/fs_fs.h libsvn_fs_fs/transaction.c tests/libsvn_fs/fs-test.c
Author: stefan2
Date: Sat Sep 27 10:52:44 2014
New Revision: 1627949
URL: http://svn.apache.org/r1627949
Log:
Don't silently corrupt revisions that need a format upgrade due to a
background FS upgrade. See here and the new test case for a detailed
reproduction:
https://mail-archives.apache.org/mod_mbox/subversion-dev/201409.mbox/%3CCABw-3Yfv_N4OcLWbCi-EDv6OphRst%3DZ77p14TxaX92ZcEyhxWw%40mail.gmail.com%3E
To fix this, we need to re-read the format info at the beginning of the
commit procedure. Provide a private API function to do that. Finally,
add the test case provided by Ivan.
Reported by: ivan
* subversion/libsvn_fs_fs/fs_fs.h
(svn_fs_fs__read_format_file): Declare new private API function.
* subversion/libsvn_fs_fs/fs_fs.c
(svn_fs_fs__read_format_file): Implement it, mostly by taking code out
of ...
(svn_fs_fs__open): ... this. Use the new function here instead.
* subversion/libsvn_fs_fs/transaction.c
(commit_body): The actual 1-line fix.
* subversion/tests/libsvn_fs/fs-test.c
(upgrade_while_committing): New test case.
(test_funcs): Register new test.
Modified:
subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c
subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h
subversion/trunk/subversion/libsvn_fs_fs/transaction.c
subversion/trunk/subversion/tests/libsvn_fs/fs-test.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=1627949&r1=1627948&r2=1627949&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Sat Sep 27 10:52:44 2014
@@ -1037,23 +1037,33 @@ read_uuid(svn_fs_t *fs,
}
svn_error_t *
-svn_fs_fs__open(svn_fs_t *fs, const char *path, apr_pool_t *pool)
+svn_fs_fs__read_format_file(svn_fs_t *fs, apr_pool_t *scratch_pool)
{
fs_fs_data_t *ffd = fs->fsap_data;
int format, max_files_per_dir;
svn_revnum_t min_log_addressing_rev;
- fs->path = apr_pstrdup(fs->pool, path);
-
- /* Read the FS format number. */
+ /* Read info from format file. */
SVN_ERR(read_format(&format, &max_files_per_dir, &min_log_addressing_rev,
- path_format(fs, pool), pool));
+ path_format(fs, scratch_pool), scratch_pool));
- /* Now we've got a format number no matter what. */
+ /* Now that we've got *all* info, store / update values in FFD. */
ffd->format = format;
ffd->max_files_per_dir = max_files_per_dir;
ffd->min_log_addressing_rev = min_log_addressing_rev;
+ return SVN_NO_ERROR;
+}
+
+svn_error_t *
+svn_fs_fs__open(svn_fs_t *fs, const char *path, apr_pool_t *pool)
+{
+ fs_fs_data_t *ffd = fs->fsap_data;
+ fs->path = apr_pstrdup(fs->pool, path);
+
+ /* Read the FS format file. */
+ SVN_ERR(svn_fs_fs__read_format_file(fs, pool));
+
/* Read in and cache the repository uuid. */
SVN_ERR(read_uuid(fs, pool));
Modified: subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h?rev=1627949&r1=1627948&r2=1627949&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.h Sat Sep 27 10:52:44 2014
@@ -25,6 +25,11 @@
#include "fs.h"
+/* Read the 'format' file of fsfs filesystem FS and store its info in FS.
+ * Use SCRATCH_POOL for temporary allocations. */
+svn_error_t *
+svn_fs_fs__read_format_file(svn_fs_t *fs, apr_pool_t *scratch_pool);
+
/* Open the fsfs filesystem pointed to by PATH and associate it with
filesystem object FS. Use POOL for temporary allocations.
Modified: subversion/trunk/subversion/libsvn_fs_fs/transaction.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/transaction.c?rev=1627949&r1=1627948&r2=1627949&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/transaction.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/transaction.c Sat Sep 27 10:52:44 2014
@@ -3638,6 +3638,22 @@ commit_body(void *baton, apr_pool_t *poo
const svn_fs_fs__id_part_t *txn_id = svn_fs_fs__txn_get_id(cb->txn);
apr_hash_t *changed_paths;
+ /* Re-Read the current repository format. All our repo upgrade and
+ config evaluation strategies are such that existing information in
+ FS and FFD remains valid.
+
+ Although we don't recommend upgrading hot repositories, people may
+ still do it and we must make sure to either handle them gracefully
+ or to error out.
+
+ Committing pre-format 3 txns will fail after upgrade to format 3+
+ because the proto-rev cannot be found; no further action needed.
+ Upgrades from pre-f7 to f7+ means a potential change in addressing
+ mode for the final rev. We must be sure to detect that cause because
+ the failure would only manifest once the new revision got committed.
+ */
+ SVN_ERR(svn_fs_fs__read_format_file(cb->fs, pool));
+
/* Read the current youngest revision and, possibly, the next available
node id and copy id (for old format filesystems). Update the cached
value for the youngest revision, because we have just checked it. */
Modified: subversion/trunk/subversion/tests/libsvn_fs/fs-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_fs/fs-test.c?rev=1627949&r1=1627948&r2=1627949&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_fs/fs-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_fs/fs-test.c Sat Sep 27 10:52:44 2014
@@ -5403,6 +5403,56 @@ reopen_modify(const svn_test_opts_t *opt
#endif
}
+static svn_error_t *
+upgrade_while_committing(const svn_test_opts_t *opts,
+ apr_pool_t *pool)
+{
+ svn_fs_t *fs;
+ svn_revnum_t head_rev = 0;
+ svn_fs_root_t *root;
+ svn_fs_txn_t *txn;
+ const char *fs_path;
+ apr_hash_t *fs_config = apr_hash_make(pool);
+
+ /* Bail (with success) on known-untestable scenarios */
+ if (strcmp(opts->fs_type, "fsfs") != 0)
+ return svn_error_create(SVN_ERR_TEST_SKIPPED, NULL,
+ "this will test FSFS repositories only");
+
+ if (opts->server_minor_version && (opts->server_minor_version < 6))
+ return svn_error_create(SVN_ERR_TEST_SKIPPED, NULL,
+ "pre-1.6 SVN doesn't support FSFS packing");
+
+ /* Create test repository with greek tree. */
+ fs_path = "test-upgrade-while-committing";
+
+ svn_hash_sets(fs_config, SVN_FS_CONFIG_COMPATIBLE_VERSION, "1.7");
+ svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_SHARD_SIZE, "2");
+ SVN_ERR(svn_test__create_fs2(&fs, fs_path, opts, fs_config, pool));
+
+ SVN_ERR(svn_fs_open(&fs, fs_path, NULL, pool));
+ SVN_ERR(svn_fs_begin_txn(&txn, fs, head_rev, pool));
+ SVN_ERR(svn_fs_txn_root(&root, txn, pool));
+ SVN_ERR(svn_test__create_greek_tree(root, pool));
+ SVN_ERR(test_commit_txn(&head_rev, txn, NULL, pool));
+
+ /* Upgrade filesystem, but keep existing svn_fs_t object. */
+ SVN_ERR(svn_fs_upgrade(fs_path, pool));
+
+ /* Create txn with changes. */
+ SVN_ERR(svn_fs_begin_txn(&txn, fs, head_rev, pool));
+ SVN_ERR(svn_fs_txn_root(&root, txn, pool));
+ SVN_ERR(svn_fs_make_dir(root, "/foo", pool));
+
+ /* Commit txn. */
+ SVN_ERR(test_commit_txn(&head_rev, txn, NULL, pool));
+
+ /* Verify filesystem content. */
+ SVN_ERR(svn_fs_verify(fs_path, NULL, 0, SVN_INVALID_REVNUM, NULL, NULL,
+ NULL, NULL, pool));
+
+ return SVN_NO_ERROR;
+}
/* ------------------------------------------------------------------------ */
@@ -5501,6 +5551,8 @@ static struct svn_test_descriptor_t test
SVN_TEST_OPTS_WIMP(reopen_modify,
"test reopen and modify txn",
"txn_dir_cache fail in FSFS"),
+ SVN_TEST_OPTS_PASS(upgrade_while_committing,
+ "upgrade while committing"),
SVN_TEST_NULL
};