You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by ko...@apache.org on 2014/03/03 21:50:10 UTC
svn commit: r1573744 - in /subversion/trunk/subversion:
libsvn_fs_fs/hotcopy.c libsvn_fs_fs/recovery.c
tests/cmdline/svnadmin_tests.py
Author: kotkov
Date: Mon Mar 3 20:50:09 2014
New Revision: 1573744
URL: http://svn.apache.org/r1573744
Log:
Fix a regression in the way recover and hotcopy behave for old FSFS format 1/2
repositories described in http://svn.haxx.se/dev/archive-2014-01/0089.shtml
and http://svn.haxx.se/dev/archive-2014-01/0160.shtml
In brief, the problem lies in the way the node ids recovery code makes
assumptions about the current state of the db/current file. Namely, the
svn_fs_fs__find_max_ids() wrapper introduced in r1503742 behaves like this:
svn_fs_fs__find_max_ids()
svn_fs_fs__rev_get_root()
svn_fs_fs__ensure_revision_exists()
...
(checks the presence of the revision by reading
the most recent db/current file)
This last assumption renders the recovery procedure useless for old
repositories. The core of the recovery lies in the ability to recreate the
db/current file (which can be missing, out-of-date or even broken) from the
latest revision in the repository. Thus, requiring the db/current contents
to be valid and up-to-date in order to be able to run the recovery does not
really make sense. Moreover, it messes up the hotcopy process, which uses
the same recovery code when updating db/current. In order to update the
db/current, we need to know the node ids. In order to get these node ids,
we run the recovery, which assumes that the db/current file is up-to-date
(thus creating a cyclic dependency).
This changeset fixes the root cause of the problem by restoring the 1.8.x way
of bootstrapping the node ids recovery. We need to know the root offset in
the revision file, so this can be achieved by parsing the revision file
trailer without assuming (and even knowing) something about db/current.
Since r1560643, we have two regression tests ensuring that recover and hotcopy
work for old repositories. However, this changeset fixes another use case
(recovery with missing or invalid db/current) and it would be nice to have a
test checking it. I splitted the existing recover_fsfs() test into two parts
(for the new and the --compatible-version=1.3 repositories) with the second
part actually being the new regression test.
Finally, with the root cause fixed, we can now drop the first hunk of
r1561427, which is a hotcopy-wide workaround for the erroneous
ensure_revision_exists() assumption. It works by prestamping the db/current
with a valid revnum and fake node ids (0) in order to be able to execute
the "real" node ids recovery. This is no longer required.
See http://svn.haxx.se/dev/archive-2014-01/0160.shtml
* subversion/libsvn_fs_fs/recovery.c
(recover_get_root_offset): New helper function used to retrieve the root
node-id offset for non-packed revision files (given a revision number).
(svn_fs_fs__find_max_ids): Use recover_get_root_offset() to get the root
offset for the node ids recovery.
* subversion/libsvn_fs_fs/hotcopy.c
(hotcopy_update_current): Do not touch the db/current file before calling
svn_fs_fs__find_max_ids().
* subversion/tests/cmdline/svnadmin_tests.py
(recover_fsfs): Move the core of this test into...
(corrupt_and_recover_db_current): ...this new helper receiving the minor
version of the server being tested.
(fsfs_recover_db_current):
New test. Calls corrupt_and_recover_db_current() with the default server
minor version. This is effectively the same test as recover_fsfs() was.
(fsfs_recover_old_db_current):
New test. Calls corrupt_and_recover_db_current() with the server minor
version set to 3.
(test_list): Add references to new tests in place of the old one.
Modified:
subversion/trunk/subversion/libsvn_fs_fs/hotcopy.c
subversion/trunk/subversion/libsvn_fs_fs/recovery.c
subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
Modified: subversion/trunk/subversion/libsvn_fs_fs/hotcopy.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/hotcopy.c?rev=1573744&r1=1573743&r2=1573744&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/hotcopy.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/hotcopy.c Mon Mar 3 20:50:09 2014
@@ -382,12 +382,6 @@ hotcopy_update_current(svn_revnum_t *dst
/* If necessary, get new current next_node and next_copy IDs. */
if (dst_ffd->format < SVN_FS_FS__MIN_NO_GLOBAL_IDS_FORMAT)
{
- /* Make sure NEW_YOUNGEST is a valid revision in DST_FS.
- Because a crash in hotcopy requires at least a full recovery run,
- it is safe to temporarily reset the max IDs to 0. */
- SVN_ERR(svn_fs_fs__write_current(dst_fs, new_youngest, next_node_id,
- next_copy_id, scratch_pool));
-
SVN_ERR(svn_fs_fs__find_max_ids(dst_fs, new_youngest,
&next_node_id, &next_copy_id,
scratch_pool));
Modified: subversion/trunk/subversion/libsvn_fs_fs/recovery.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/recovery.c?rev=1573744&r1=1573743&r2=1573744&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/recovery.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/recovery.c Mon Mar 3 20:50:09 2014
@@ -276,6 +276,51 @@ recover_find_max_ids(svn_fs_t *fs,
return SVN_NO_ERROR;
}
+/* Part of the recovery procedure. Given an open non-packed revision file
+ REV_FILE for REV, locate the trailer that specifies the offset to the root
+ node-id and store this offset in *ROOT_OFFSET. Do temporary allocations in
+ POOL. */
+static svn_error_t *
+recover_get_root_offset(apr_off_t *root_offset,
+ svn_revnum_t rev,
+ svn_fs_fs__revision_file_t *rev_file,
+ apr_pool_t *pool)
+{
+ char buffer[64];
+ svn_stringbuf_t *trailer;
+ apr_off_t start;
+ apr_off_t end;
+ apr_size_t len;
+
+ SVN_ERR_ASSERT(!rev_file->is_packed);
+
+ /* We will assume that the last line containing the two offsets (to the root
+ node-id and to the changed path information) will never be longer than 64
+ characters. */
+ end = 0;
+ SVN_ERR(svn_io_file_seek(rev_file->file, APR_END, &end, pool));
+
+ if (end < sizeof(buffer))
+ {
+ len = (apr_size_t)end;
+ start = 0;
+ }
+ else
+ {
+ len = sizeof(buffer);
+ start = end - sizeof(buffer);
+ }
+
+ SVN_ERR(svn_io_file_seek(rev_file->file, APR_SET, &start, pool));
+ SVN_ERR(svn_io_file_read_full2(rev_file->file, buffer, len,
+ NULL, NULL, pool));
+
+ trailer = svn_stringbuf_ncreate(buffer, len, pool);
+ SVN_ERR(svn_fs_fs__parse_revision_trailer(root_offset, NULL, trailer, rev));
+
+ return SVN_NO_ERROR;
+}
+
svn_error_t *
svn_fs_fs__find_max_ids(svn_fs_t *fs,
svn_revnum_t youngest,
@@ -286,16 +331,12 @@ svn_fs_fs__find_max_ids(svn_fs_t *fs,
fs_fs_data_t *ffd = fs->fsap_data;
apr_off_t root_offset;
svn_fs_fs__revision_file_t *rev_file;
- svn_fs_id_t *root_id;
/* call this function for old repo formats only */
SVN_ERR_ASSERT(ffd->format < SVN_FS_FS__MIN_NO_GLOBAL_IDS_FORMAT);
- SVN_ERR(svn_fs_fs__rev_get_root(&root_id, fs, youngest, pool));
SVN_ERR(svn_fs_fs__open_pack_or_rev_file(&rev_file, fs, youngest, pool));
- SVN_ERR(svn_fs_fs__item_offset(&root_offset, fs, rev_file, youngest, NULL,
- svn_fs_fs__id_item(root_id), pool));
-
+ SVN_ERR(recover_get_root_offset(&root_offset, youngest, rev_file, pool));
SVN_ERR(recover_find_max_ids(fs, youngest, rev_file, root_offset,
max_node_id, max_copy_id, pool));
SVN_ERR(svn_fs_fs__close_revision_file(rev_file));
Modified: subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py?rev=1573744&r1=1573743&r2=1573744&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py (original)
+++ subversion/trunk/subversion/tests/cmdline/svnadmin_tests.py Mon Mar 3 20:50:09 2014
@@ -828,10 +828,14 @@ _0.0.t1-1 add false false /A/B/E/bravo
#----------------------------------------------------------------------
-@SkipUnless(svntest.main.is_fs_type_fsfs)
-def recover_fsfs(sbox):
- "recover a repository (FSFS only)"
- sbox.build()
+# Helper for two test functions.
+def corrupt_and_recover_db_current(sbox, minor_version=None):
+ """Build up a MINOR_VERSION sandbox and test different recovery scenarios
+ with missing, out-of-date or even corrupt db/current files. Recovery should
+ behave the same way with all values of MINOR_VERSION, hence this helper
+ containing the common code that allows us to check it."""
+
+ sbox.build(minor_version=minor_version)
current_path = os.path.join(sbox.repo_dir, 'db', 'current')
# Commit up to r3, so we can test various recovery scenarios.
@@ -906,6 +910,24 @@ def recover_fsfs(sbox):
"Contents of db/current is unexpected.",
'db/current', expected_current_contents, actual_current_contents)
+
+@SkipUnless(svntest.main.is_fs_type_fsfs)
+def fsfs_recover_db_current(sbox):
+ "fsfs recover db/current"
+ corrupt_and_recover_db_current(sbox)
+
+
+@SkipUnless(svntest.main.is_fs_type_fsfs)
+def fsfs_recover_old_db_current(sbox):
+ "fsfs recover db/current --compatible-version=1.3"
+
+ # Around trunk@1573728, 'svnadmin recover' wrongly errored out
+ # for the --compatible-version=1.3 repositories with missing or
+ # invalid db/current file:
+ # svnadmin: E160006: No such revision 1
+
+ corrupt_and_recover_db_current(sbox, minor_version=3)
+
#----------------------------------------------------------------------
@Issue(2983)
def load_with_parent_dir(sbox):
@@ -2312,7 +2334,8 @@ test_list = [ None,
setrevprop,
verify_windows_paths_in_repos,
verify_incremental_fsfs,
- recover_fsfs,
+ fsfs_recover_db_current,
+ fsfs_recover_old_db_current,
load_with_parent_dir,
set_uuid,
reflect_dropped_renumbered_revs,