Daniel,
I had to tweak the patch (noted below) just a little to get it to work on *nix.
I haven't reviewed the patch, per se, but I did give it the old college try.
I also saw a number of failures, but I suspect that most of them share similar
root causes. I'll take a look today.
-Hyrum
Daniel Shahaf wrote:
> Following to the thread I started earlier this week (which still has
> a patch or two that I haven't committed yet), I've looked into running
> 'make check' with packing. Here are the two patches I tried.
>
> Concerningly, I see some test failures with each of the two patches.
>
> Daniel
>
>
>
> [[[
> Patch #1. It should be committed eventually
>
> To use this:
>
> * apply it
> * unix: regenerate Makefile from Makefile.in
> run 'make check FSFS_SHARDING=4 FSFS_PACKING=yes'
> * windows: python win-tests.py --fsfs-sharding=4 --fsfs-packing
> * both: ../cmdline/foo_tests.py --fsfs-sharding=4 --fsfs-packing
> (replace 4 by the shard size)
>
> To verify that the patch is working:
>
> * run 'find svn-test-work/repositories | grep pack' during the tests run
> * look for the revs/%d.pack/ dirs in the output.
>
> Index: Makefile.in
> ===================================================================
> --- Makefile.in (revision 34993)
> +++ Makefile.in (working copy)
> @@ -407,6 +407,12 @@ check: bin $(TEST_DEPS) @BDB_TEST_DEPS@
> if test "$(ENABLE_SASL)" != ""; then \
> flags="--enable-sasl $$flags"; \
> fi; \
> + if test "$(FSFS_SHARDING)" != ""; then \
> + flags="--fsfs-sharding $(FSFS_SHARDING) $$flags"; \
> + fi; \
> + if test "$(FSFS_PACKING)" != ""; then \
> + flags="--fsfs-packing $$flags"; \
> + fi; \
> if test "$(PARALLEL)" != ""; then \
> flags="--parallel $$flags"; \
> fi; \
> Index: win-tests.py
> ===================================================================
> --- win-tests.py (revision 34993)
> +++ win-tests.py (working copy)
> @@ -60,6 +60,8 @@ def _usage_exit():
> print(" --server-minor-version : the minor version of the server being")
> print(" tested")
> print(" --config-file : Configuration file for tests")
> + print(" --fsfs-sharding=size : Specify shard size (for fsfs)")
> + print(" --fsfs-packing : Run 'svnadmin pack' automatically")
>
> sys.exit(0)
>
> @@ -90,6 +92,7 @@ opts, args = my_getopt(sys.argv[1:], 'hrdvcpu:f:',
> 'svnserve-args=', 'fs-type=', 'asp.net-hack',
> 'httpd-dir=', 'httpd-port=', 'httpd-daemon',
> 'httpd-server', 'http-library=', 'help',
> + 'fsfs-packing', 'fsfs-sharding=',
> 'list', 'enable-sasl', 'bin=', 'parallel',
> 'config-file=', 'server-minor-version='])
> if len(args) > 1:
> @@ -110,6 +113,8 @@ list_tests = None
> enable_sasl = None
> svn_bin = None
> parallel = None
> +fsfs_sharding = None
> +fsfs_packing = None
> server_minor_version = None
> config_file = None
>
> @@ -144,6 +149,10 @@ for opt, val in opts:
> httpd_service = 1
> elif opt == '--http-library':
> http_library = val
> + elif opt in ['--fsfs-sharding']:
> + fsfs_sharding = int(val)
> + elif opt in ['--fsfs-packing']:
> + fsfs_packing = 1
> elif opt == '--list':
> list_tests = 1
> elif opt == '--enable-sasl':
> @@ -575,8 +584,9 @@ th = run_tests.TestHarness(abs_srcdir, abs_builddi
> os.path.join(abs_builddir, log),
> base_url, fs_type, http_library,
> server_minor_version, 1, cleanup,
> - enable_sasl, parallel, config_file, list_tests,
> - svn_bin)
> + enable_sasl, parallel, config_file,
> + fsfs_sharding, fsfs_packing,
> + list_tests, svn_bin)
> old_cwd = os.getcwd()
> try:
> os.chdir(abs_builddir)
> Index: subversion/tests/cmdline/svntest/main.py
> ===================================================================
> --- subversion/tests/cmdline/svntest/main.py (revision 34993)
> +++ subversion/tests/cmdline/svntest/main.py (working copy)
> @@ -189,6 +189,13 @@ use_jsvn = False
> # ('neon', 'serf')
> http_library = None
>
> +# Global variable: Number of shards to use in FSFS
> +# 'None' means "use FSFS's default"
> +fsfs_sharding = None
> +
> +# Global variable: automatically pack FSFS repositories after every commit
> +fsfs_packing = None
> +
> # Configuration file (copied into FSFS fsfs.conf).
> config_file = None
>
> @@ -346,6 +353,11 @@ def get_fsfs_conf_file_path(repo_dir):
>
> return os.path.join(repo_dir, "db", "fsfs.conf")
>
> +def get_fsfs_format_file_path(repo_dir):
> + "Return the path of the format file in REPO_DIR."
> +
> + return os.path.join(repo_dir, "db", "format")
> +
> # Run any binary, logging the command line and return code
> def run_command(command, error_expected, binary_mode=0, *varargs):
> """Run COMMAND with VARARGS; return exit code as int; stdout, stderr
> @@ -721,9 +733,49 @@ def create_repos(path):
> file_append(os.path.join(path, "conf", "passwd"),
> "[users]\njrandom = rayjandom\njconstant = rayjandom\n");
>
> - if config_file is not None and (fs_type is None or fs_type == 'fsfs'):
> - shutil.copy(config_file, get_fsfs_conf_file_path(path))
> + if fs_type is None or fs_type == 'fsfs':
> + # fsfs.conf file
> + if config_file is not None:
> + shutil.copy(config_file, get_fsfs_conf_file_path(path))
>
> + # format file
> + if fsfs_sharding is not None:
> + def transform_line(line):
> + if line.startswith('layout '):
> + if fsfs_sharding > 0:
> + line = 'layout sharded %d' % fsfs_sharding
> + else:
> + line = 'layout linear'
> + return line
> +
> + # read it
> + format_file_path = get_fsfs_format_file_path(path)
> + contents = file_read(format_file_path, 'rb')
> +
> + # tweak it
> + new_contents = "".join([transform_line(line) + "\n"
> + for line in contents.split("\n")])
> + if new_contents[-1] == "\n":
> + # we don't currently allow empty lines (\n\n) in the format file.
> + new_contents = new_contents[:-1]
> +
> + # replace it
> + if windows:
> + os.chmod(format_file_path, 0666)
Remove the conditional for the above line. If I didn't do this, I was getting
permissions errors when trying to write the format file on the next line.
> + file_write(format_file_path, new_contents, 'wb')
> +
> + # post-commit
> + # Note that some test (currently only commit_test) create their own
> + # post-commit hooks, which would override this one. :-(
> + if fsfs_packing:
> + create_python_hook_script(get_post_commit_hook_path(path),
> + "import subprocess\n"
> + "import sys\n"
> + "command = %s\n"
> + "sys.stderr.write('Running %%s' %% `command`)\n"
> + "sys.exit(subprocess.Popen(command).wait())\n"
> + % repr([svnadmin_binary, 'pack', path]))
> +
> # make the repos world-writeable, for mod_dav_svn's sake.
> chmod_tree(path, 0666, 0666)
>
> @@ -1350,6 +1402,8 @@ def usage():
> " useful during test development!")
> print(" --server-minor-version Set the minor version for the server.\n"
> " Supports version 4 or 5.")
> + print(" --fsfs-sharding Default shard size (for fsfs)\n"
> + " --fsfs-packing Run 'svnadmin pack' automatically")
> print(" --config-file Configuration file for tests.")
> print(" --help This information")
>
> @@ -1381,6 +1435,8 @@ def run_tests(test_list, serial_only = False):
> global svnversion_binary
> global command_line_parsed
> global http_library
> + global fsfs_sharding
> + global fsfs_packing
> global config_file
> global server_minor_version
> global use_jsvn
> @@ -1399,6 +1455,7 @@ def run_tests(test_list, serial_only = False):
> ['url=', 'fs-type=', 'verbose', 'quiet', 'cleanup',
> 'list', 'enable-sasl', 'help', 'parallel',
> 'bin=', 'http-library=', 'server-minor-version=',
> + 'fsfs-packing', 'fsfs-sharding=',
> 'use-jsvn', 'development', 'config-file='])
> except getopt.GetoptError, e:
> print("ERROR: %s\n" % e)
> @@ -1483,6 +1540,11 @@ def run_tests(test_list, serial_only = False):
> elif opt == '--http-library':
> http_library = val
>
> + elif opt in ['--fsfs-sharding']:
> + fsfs_sharding = int(val)
> + elif opt in ['--fsfs-packing']:
> + fsfs_packing = 1
> +
> elif opt == '--server-minor-version':
> server_minor_version = int(val)
> if server_minor_version < 4 or server_minor_version > 6:
> @@ -1498,6 +1560,9 @@ def run_tests(test_list, serial_only = False):
> elif opt == '--config-file':
> config_file = val
>
> + if fsfs_packing is not None and fsfs_sharding is None:
> + raise Exception, '--fsfs-packing requires --fsfs-sharding'
> +
> if test_area_url[-1:] == '/': # Normalize url to have no trailing slash
> test_area_url = test_area_url[:-1]
>
> Index: build/run_tests.py
> ===================================================================
> --- build/run_tests.py (revision 34993)
> +++ build/run_tests.py (working copy)
> @@ -31,6 +31,7 @@ class TestHarness:
> base_url=None, fs_type=None, http_library=None,
> server_minor_version=None, verbose=None,
> cleanup=None, enable_sasl=None, parallel=None, config_file=None,
> + fsfs_sharding=None, fsfs_packing=None,
> list_tests=None, svn_bin=None):
> '''Construct a TestHarness instance.
>
> @@ -53,6 +54,10 @@ class TestHarness:
> self.cleanup = cleanup
> self.enable_sasl = enable_sasl
> self.parallel = parallel
> + self.fsfs_sharding = fsfs_sharding
> + self.fsfs_packing = fsfs_packing
> + if fsfs_packing is not None and fsfs_sharding is None:
> + raise Exception, '--fsfs-packing requires --fsfs-sharding'
> self.config_file = None
> if config_file is not None:
> self.config_file = os.path.abspath(config_file)
> @@ -152,6 +157,10 @@ class TestHarness:
> cmdline.append('--list')
> if self.svn_bin is not None:
> cmdline.append(quote('--bin=' + self.svn_bin))
> + if self.fsfs_sharding is not None:
> + cmdline.append('--fsfs-sharding=%d' % self.fsfs_sharding)
> + if self.fsfs_packing is not None:
> + cmdline.append('--fsfs-packing')
>
> old_cwd = os.getcwd()
> try:
> @@ -209,6 +218,7 @@ def main():
> opts, args = my_getopt(sys.argv[1:], 'u:f:vc',
> ['url=', 'fs-type=', 'verbose', 'cleanup',
> 'http-library=', 'server-minor-version=',
> + 'fsfs-packing', 'fsfs-sharding=',
> 'enable-sasl', 'parallel', 'config-file='])
> except getopt.GetoptError:
> args = []
> @@ -218,8 +228,9 @@ def main():
> sys.exit(2)
>
> base_url, fs_type, verbose, cleanup, enable_sasl, http_library, \
> - server_minor_version, parallel, config_file = \
> - None, None, None, None, None, None, None, None, None
> + server_minor_version, fsfs_sharding, fsfs_packing, parallel, \
> + config_file = \
> + None, None, None, None, None, None, None, None, None, None, None
> for opt, val in opts:
> if opt in ['-u', '--url']:
> base_url = val
> @@ -227,6 +238,10 @@ def main():
> fs_type = val
> elif opt in ['--http-library']:
> http_library = val
> + elif opt in ['--fsfs-sharding']:
> + fsfs_sharding = int(val)
> + elif opt in ['--fsfs-packing']:
> + fsfs_packing = 1
> elif opt in ['--server-minor-version']:
> server_minor_version = val
> elif opt in ['-v', '--verbose']:
> @@ -245,7 +260,8 @@ def main():
> th = TestHarness(args[0], args[1],
> os.path.abspath('tests.log'),
> base_url, fs_type, http_library, server_minor_version,
> - verbose, cleanup, enable_sasl, parallel, config_file)
> + verbose, cleanup, enable_sasl, parallel, config_file,
> + fsfs_sharding, fsfs_packing)
>
> failed = th.run(args[2:])
> if failed:
> ]]]
>
> [[[
> (not for commit)
>
> To use:
>
> possibly edit it to choose a different value for SVN_FS_FS_DEFAULT_MAX_FILES_PER_DIR
> apply it
> rebuild
> run 'make check' as usual
>
>
> To verify that it applied correctly:
>
> % svnadmin create foo
> % grep 'layout sharded' foo/db/format
> layout sharded 4
> # check that the number is whatever you changed the constant to
> %
>
>
> Index: subversion/libsvn_fs_fs/fs_fs.c
> ===================================================================
> --- subversion/libsvn_fs_fs/fs_fs.c (revision 34962)
> +++ subversion/libsvn_fs_fs/fs_fs.c (working copy)
> @@ -64,7 +64,7 @@
> figure of 1000 is reasonable for VFAT filesystems, which are by far
> the worst performers in this area. */
> #ifndef SVN_FS_FS_DEFAULT_MAX_FILES_PER_DIR
> -#define SVN_FS_FS_DEFAULT_MAX_FILES_PER_DIR 1000
> +#define SVN_FS_FS_DEFAULT_MAX_FILES_PER_DIR 4
> #endif
>
> /* Following are defines that specify the textual elements of the
> Index: subversion/libsvn_fs/fs-loader.c
> ===================================================================
> --- subversion/libsvn_fs/fs-loader.c (revision 34962)
> +++ subversion/libsvn_fs/fs-loader.c (working copy)
> @@ -637,7 +637,18 @@ svn_error_t *
> svn_fs_commit_txn(const char **conflict_p, svn_revnum_t *new_rev,
> svn_fs_txn_t *txn, apr_pool_t *pool)
> {
> - return txn->vtable->commit(conflict_p, new_rev, txn, pool);
> + svn_fs_root_t *txn_root;
> + svn_fs_t *fs;
> + const char *fs_path;
> +
> + SVN_ERR(svn_fs_txn_root(&txn_root, txn, pool));
> + fs = svn_fs_root_fs(txn_root);
> + fs_path = svn_fs_path(fs, pool);
> +
> + SVN_ERR(txn->vtable->commit(conflict_p, new_rev, txn, pool));
> + SVN_ERR(svn_fs_pack(fs_path, NULL, NULL, pool));
> +
> + return SVN_NO_ERROR;
> }
>
> svn_error_t *
> ]]]
>
> It is not recommended to use both patches at the same time.
>
> Daniel
>
------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=998596
Received on 2009-01-01 16:11:18 CET