> Author: stefan2
> Date: Thu Aug 21 14:35:05 2014
> New Revision: 1619413
>
> URL: http://svn.apache.org/r1619413
> Log:
> Enforce the "everybody or nobody" restriction on revprop caching, i.e.
> either all processes (usually servers) or non must use revprop caching.
>
> This patch makes sure that the first process to use revprop caching will
> create the revprop generation file before even reading and caching any
> revprops - instead of auto-creating it upon the first write. Then this
> file is being used as an indicator that the repo has been accessed at
> least once using revprop caching. FS instances without that feature
> enabled will then be banned from writing any revprops.
Is this a right thing to do? I suspect there are several problems:
- Any *existing* caller of our public API, namely svn_repos_open3(),
svn_fs_open2() and their deprecated versions, will not be able to change
revision properties for repositories that had this feature enabled at
least once (!). We are not the only ones using that API, so should
we tell them everyone to update their software depending on it if they
would like it to work?
- The server now dictates the *caching* configuration for 'svn' and 'svnlook'
clients. This looks problematic to me, especially considering some other
problems stated in http://svn.haxx.se/dev/archive-2014-08/0235.shtml
Shouldn't caching be totally optional by its design?
- How should existing administrators turn this feature off after they had it
enabled for a while? With a change like this they cannot really just go
with something like SVNCacheRevProps=off, because doing this would result
in any attempt of changing a revision property to error out with "Writing
revprops requires revprop caching to be enabled". Do they have to
manually remove all the corresponding revprop generation files we now
use as a marker? What if they have 60000 repositories? And how should
they know about this in the first place?
- Finally, why cannot we just stop sharing the revprop generation data
(as in http://svn.haxx.se/dev/archive-2014-08/0237.shtml)?
> +static svn_error_t *
> +enforce_consistent_revprop_caching(const svn_test_opts_t *opts,
> + apr_pool_t *pool)
> +{
> + svn_fs_t *fs_cache;
> + svn_fs_t *fs_no_cache;
> + svn_fs_t *fs_auto_cache;
> + apr_hash_t *fs_config = apr_hash_make(pool);
> + svn_string_t value = { "0", 1 };
> +
> + if (strcmp(opts->fs_type, "fsfs") != 0)
> + return svn_error_create(SVN_ERR_TEST_SKIPPED, NULL, NULL);
> +
> + /* Create test repository with greek tree. */
> + SVN_ERR(svn_test__create_fs(&fs_no_cache, REPO_NAME, opts, pool));
> +
> + svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_REVPROPS, "1");
> + SVN_ERR(svn_fs_open2(&fs_cache, REPO_NAME, fs_config, pool, pool));
> +
> + svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_REVPROPS, "3");
> + SVN_ERR(svn_fs_open2(&fs_auto_cache, REPO_NAME, fs_config, pool, pool));
I suspect that the first svn_fs_open2() requires a warning handle. Otherwise,
this test would abort on platforms with inefficient named atomics.
> /* The test table. */
>
> @@ -1269,7 +1336,10 @@ static struct svn_test_descriptor_t test
> SVN_TEST_OPTS_WIMP(revprop_caching_on_off,
> "change revprops with enabled and disabled caching",
> "fails with FSFS / svn_named_atomic__is_efficient()"),
> + SVN_TEST_OPTS_PASS(enforce_consistent_revprop_caching,
> + "test revprop cache setting consistency"),
> SVN_TEST_NULL
> };
>
> SVN_TEST_MAIN
> +
> \ No newline at end of file
Shouldn't we now make the revprop_caching_on_off() a SVN_TEST_OPTS_PASS()?
Regards,
Evgeny Kotkov
Received on 2014-08-21 17:59:55 CEST