Stefan2 et al.:

Nodeprops caching is new in 1.10. This caused me to have a look at the code for enabling it, and I noticed the following in FSFS (and the same in FSX).

In subversion/libsvn_fs_fs/caching.c@1787000, around line 100:
>   /* don't cache text deltas by default.

Wrong: the default was changed to TRUE in r1517479, and this new default is contrary to the reasoning given in this comment.

>    * Once we reconstructed the fulltexts from the deltas,
>    * these deltas are rarely re-used. Therefore, only tools
>    * like svnadmin will activate this to speed up operations
>    * dump and verify.
>    */
>   *cache_txdeltas
>     = svn_hash__get_bool(fs->config,
>                          SVN_FS_CONFIG_FSFS_CACHE_DELTAS,
>                          TRUE);
[...]
>   /* by default, cache nodeprops: this will match pre-1.10
>    * behavior where node properties caching was controlled
>    * by SVN_FS_CONFIG_FSFS_CACHE_FULLTEXTS configuration option.

The default of TRUE doesn't match that behaviour, does it? To do so, should the last argument (below) be "*cache_txdeltas" instead of "TRUE"?

>    */
>   *cache_nodeprops
>     = svn_hash__get_bool(fs->config,
>                          SVN_FS_CONFIG_FSFS_CACHE_NODEPROPS,
>                          TRUE);

Also, the handling of these flags in mod_dav_svn (around line 990) is odd:

svn_boolean_t
dav_svn__get_txdelta_cache_flag(request_rec *r)
{
  dir_conf_t *conf;

  conf = ap_get_module_config(r->per_dir_config, &dav_svn_module);

  /* txdelta caching is enabled by default. */
  return get_conf_flag(conf->txdelta_cache, TRUE);
}


svn_boolean_t
dav_svn__get_fulltext_cache_flag(request_rec *r)
{
  dir_conf_t *conf;

  conf = ap_get_module_config(r->per_dir_config, &dav_svn_module);
  return conf->fulltext_cache == CONF_FLAG_ON;

This construction ("==ON") means the default here is OFF, which is the opposite of the default that FSFS would apply if we didn't pass it any value for this flag.

}


svn_boolean_t
dav_svn__get_revprop_cache_flag(request_rec *r)
{
  dir_conf_t *conf;

  conf = ap_get_module_config(r->per_dir_config, &dav_svn_module);
  return conf->revprop_cache == CONF_FLAG_ON;
}

svn_boolean_t
dav_svn__get_nodeprop_cache_flag(request_rec *r)
{
  dir_conf_t *conf;

  conf = ap_get_module_config(r->per_dir_config, &dav_svn_module);
  /* node properties caching is enabled by default. */
  return get_conf_flag(conf->nodeprop_cache, FALSE);

The code here uses a default value of "FALSE", which is different from the fsfs.conf default value (see above), and doesn't match the comment here.

}

- Julian

Reply via email to