You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <da...@elego.de> on 2013/03/29 07:19:46 UTC

Disabling ffd->svn_cache__t Re: svn commit: r1462413 - in /subversion/trunk/subversion: include/svn_fs.h libsvn_fs_fs/fs_fs.c

I have a function that looks like this:

    static svn_error_t *f(svn_fs_t *fs)
    {
      svn_fs_t *ft = svn_fs_open(fs);
      svn_fs_verify_root(ft, r42);
    }

I'd like the 'verify' call to bypass any information that is already
cached:  I don't mind if it the 'verify' process caches information for
itself and then reads it back, but I don't want it to read information
that is _right now_ in the cache (due to FS, or some other handle to the
same filesystem in another thread) in lieu of parsing it, for itself,
from the revision file.

What would be the best way to achieve this?  Do I set every cache in
ft->ffd->* to NULL or to a pointer to a newly-created, empty cache object?
Something else?

Thanks

Daniel

danielsh@apache.org wrote on Fri, Mar 29, 2013 at 06:08:55 -0000:
> Author: danielsh
> Date: Fri Mar 29 06:08:55 2013
> New Revision: 1462413
> 
> URL: http://svn.apache.org/r1462413
> Log:
> In the FSFS "paranoid verify" code, disable caching.  See within for a question.
> 
> * subversion/include/svn_fs.h
>   (SVN_FS_CONFIG_FSFS_CACHE_*): Add a note to maintainers who add such symbols.
> 
> * subversion/libsvn_fs_fs/fs_fs.c
>   (verify_as_revision_before_current_plus_plus):
>     Disable caches the API allows to disable, and add a question.
> 
> Modified:
>     subversion/trunk/subversion/include/svn_fs.h
>     subversion/trunk/subversion/libsvn_fs_fs/fs_fs.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=1462413&r1=1462412&r2=1462413&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Fri Mar 29 06:08:55 2013
> @@ -8278,11 +8278,20 @@ verify_as_revision_before_current_plus_p
>    svn_fs_t *ft; /* fs++ == ft */
>    svn_fs_root_t *root;
>    fs_fs_data_t *ft_ffd;
> +  apr_hash_t *fs_config;
>  
>    SVN_ERR_ASSERT(ffd->svn_fs_open_);
>  
> +  fs_config = apr_hash_make(pool);
> +  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_DELTAS, "0");
> +  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_FULLTEXTS, "0");
> +  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_REVPROPS, "0");
> +  /* ### TODO: are there any other intra-process caches that FS populated
> +               and FT will read from?  Can we disable those (for FT at least)?
> +   */
> +
>    SVN_ERR(ffd->svn_fs_open_(&ft, fs->path,
> -                            NULL /* ### TODO fs_config */,
> +                            fs_config,
>                              pool));
>    ft_ffd = ft->fsap_data;
>  
> 
> 

Re: Disabling ffd->svn_cache__t Re: svn commit: r1462413 - in /subversion/trunk/subversion: include/svn_fs.h libsvn_fs_fs/fs_fs.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Fri, Mar 29, 2013 at 8:04 PM, Daniel Shahaf <da...@elego.de> wrote:

> Stefan Fuhrmann wrote on Fri, Mar 29, 2013 at 11:45:23 +0100:
> > On Fri, Mar 29, 2013 at 7:19 AM, Daniel Shahaf <da...@elego.de>
> wrote:
> >
> > > I have a function that looks like this:
> > >
> > >     static svn_error_t *f(svn_fs_t *fs)
> > >     {
> > >       svn_fs_t *ft = svn_fs_open(fs);
> > >       svn_fs_verify_root(ft, r42);
> > >     }
> > >
> > > I'd like the 'verify' call to bypass any information that is already
> > > cached:  I don't mind if it the 'verify' process caches information for
> > > itself and then reads it back, but I don't want it to read information
> > > that is _right now_ in the cache (due to FS, or some other handle to
> the
> > > same filesystem in another thread) in lieu of parsing it, for itself,
> > > from the revision file.
> > >
> > > What would be the best way to achieve this?  Do I set every cache in
> > > ft->ffd->* to NULL or to a pointer to a newly-created, empty cache
> object?
> > > Something else?
> > >
> >
> > Originally, I had planned to introduce cache namespaces in 1.9
> > because all processes would share the same cache memory
> > (so, svnadmin would then need use a separate key space).
> >
> > But since you obviously needed right now, I added the feature
> > now in 1462436 on /trunk. Since I changed your code from
> > r1462413, please have a look at my commit to ensure that it
> > still does what you intended.
> >
>
> Looks good to me, many thanks!
>
> One comment: the new docstring should forbid ":" from being used in the
> namespace name.
>

r1462907 addresses this issue in a more generalized way.

-- Stefan^2.

-- 
*Join one of our free daily demo sessions on* *Scaling Subversion for the
Enterprise <http://www.wandisco.com/training/webinars>*
*

*

Re: Disabling ffd->svn_cache__t Re: svn commit: r1462413 - in /subversion/trunk/subversion: include/svn_fs.h libsvn_fs_fs/fs_fs.c

Posted by Daniel Shahaf <da...@elego.de>.
Stefan Fuhrmann wrote on Fri, Mar 29, 2013 at 11:45:23 +0100:
> On Fri, Mar 29, 2013 at 7:19 AM, Daniel Shahaf <da...@elego.de> wrote:
> 
> > I have a function that looks like this:
> >
> >     static svn_error_t *f(svn_fs_t *fs)
> >     {
> >       svn_fs_t *ft = svn_fs_open(fs);
> >       svn_fs_verify_root(ft, r42);
> >     }
> >
> > I'd like the 'verify' call to bypass any information that is already
> > cached:  I don't mind if it the 'verify' process caches information for
> > itself and then reads it back, but I don't want it to read information
> > that is _right now_ in the cache (due to FS, or some other handle to the
> > same filesystem in another thread) in lieu of parsing it, for itself,
> > from the revision file.
> >
> > What would be the best way to achieve this?  Do I set every cache in
> > ft->ffd->* to NULL or to a pointer to a newly-created, empty cache object?
> > Something else?
> >
> 
> Originally, I had planned to introduce cache namespaces in 1.9
> because all processes would share the same cache memory
> (so, svnadmin would then need use a separate key space).
> 
> But since you obviously needed right now, I added the feature
> now in 1462436 on /trunk. Since I changed your code from
> r1462413, please have a look at my commit to ensure that it
> still does what you intended.
> 

Looks good to me, many thanks!

One comment: the new docstring should forbid ":" from being used in the
namespace name.

Thanks again,

Daniel

> -- Stefan^2.
> 
> danielsh@apache.org wrote on Fri, Mar 29, 2013 at 06:08:55 -0000:
> > > Author: danielsh
> > > Date: Fri Mar 29 06:08:55 2013
> > > New Revision: 1462413
> > >
> > > URL: http://svn.apache.org/r1462413
> > > Log:
> > > In the FSFS "paranoid verify" code, disable caching.  See within for a
> > question.
> > >
> > > * subversion/include/svn_fs.h
> > >   (SVN_FS_CONFIG_FSFS_CACHE_*): Add a note to maintainers who add such
> > symbols.
> > >
> > > * subversion/libsvn_fs_fs/fs_fs.c
> > >   (verify_as_revision_before_current_plus_plus):
> > >     Disable caches the API allows to disable, and add a question.
> > >
> > > Modified:
> > >     subversion/trunk/subversion/include/svn_fs.h
> > >     subversion/trunk/subversion/libsvn_fs_fs/fs_fs.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=1462413&r1=1462412&r2=1462413&view=diff
> > >
> > ==============================================================================
> > > --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> > > +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Fri Mar 29 06:08:55
> > 2013
> > > @@ -8278,11 +8278,20 @@ verify_as_revision_before_current_plus_p
> > >    svn_fs_t *ft; /* fs++ == ft */
> > >    svn_fs_root_t *root;
> > >    fs_fs_data_t *ft_ffd;
> > > +  apr_hash_t *fs_config;
> > >
> > >    SVN_ERR_ASSERT(ffd->svn_fs_open_);
> > >
> > > +  fs_config = apr_hash_make(pool);
> > > +  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_DELTAS, "0");
> > > +  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_FULLTEXTS, "0");
> > > +  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_REVPROPS, "0");
> > > +  /* ### TODO: are there any other intra-process caches that FS
> > populated
> > > +               and FT will read from?  Can we disable those (for FT at
> > least)?
> > > +   */
> > > +
> > >    SVN_ERR(ffd->svn_fs_open_(&ft, fs->path,
> > > -                            NULL /* ### TODO fs_config */,
> > > +                            fs_config,
> > >                              pool));
> > >    ft_ffd = ft->fsap_data;
> > >
> > >
> > >
> >
> 
> 
> 
> -- 
> *Join one of our free daily demo sessions on* *Scaling Subversion for the
> Enterprise <http://www.wandisco.com/training/webinars>*
> *
> 
> *

Re: Disabling ffd->svn_cache__t Re: svn commit: r1462413 - in /subversion/trunk/subversion: include/svn_fs.h libsvn_fs_fs/fs_fs.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Fri, Mar 29, 2013 at 7:19 AM, Daniel Shahaf <da...@elego.de> wrote:

> I have a function that looks like this:
>
>     static svn_error_t *f(svn_fs_t *fs)
>     {
>       svn_fs_t *ft = svn_fs_open(fs);
>       svn_fs_verify_root(ft, r42);
>     }
>
> I'd like the 'verify' call to bypass any information that is already
> cached:  I don't mind if it the 'verify' process caches information for
> itself and then reads it back, but I don't want it to read information
> that is _right now_ in the cache (due to FS, or some other handle to the
> same filesystem in another thread) in lieu of parsing it, for itself,
> from the revision file.
>
> What would be the best way to achieve this?  Do I set every cache in
> ft->ffd->* to NULL or to a pointer to a newly-created, empty cache object?
> Something else?
>

Originally, I had planned to introduce cache namespaces in 1.9
because all processes would share the same cache memory
(so, svnadmin would then need use a separate key space).

But since you obviously needed right now, I added the feature
now in 1462436 on /trunk. Since I changed your code from
r1462413, please have a look at my commit to ensure that it
still does what you intended.

-- Stefan^2.

danielsh@apache.org wrote on Fri, Mar 29, 2013 at 06:08:55 -0000:
> > Author: danielsh
> > Date: Fri Mar 29 06:08:55 2013
> > New Revision: 1462413
> >
> > URL: http://svn.apache.org/r1462413
> > Log:
> > In the FSFS "paranoid verify" code, disable caching.  See within for a
> question.
> >
> > * subversion/include/svn_fs.h
> >   (SVN_FS_CONFIG_FSFS_CACHE_*): Add a note to maintainers who add such
> symbols.
> >
> > * subversion/libsvn_fs_fs/fs_fs.c
> >   (verify_as_revision_before_current_plus_plus):
> >     Disable caches the API allows to disable, and add a question.
> >
> > Modified:
> >     subversion/trunk/subversion/include/svn_fs.h
> >     subversion/trunk/subversion/libsvn_fs_fs/fs_fs.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=1462413&r1=1462412&r2=1462413&view=diff
> >
> ==============================================================================
> > --- subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c (original)
> > +++ subversion/trunk/subversion/libsvn_fs_fs/fs_fs.c Fri Mar 29 06:08:55
> 2013
> > @@ -8278,11 +8278,20 @@ verify_as_revision_before_current_plus_p
> >    svn_fs_t *ft; /* fs++ == ft */
> >    svn_fs_root_t *root;
> >    fs_fs_data_t *ft_ffd;
> > +  apr_hash_t *fs_config;
> >
> >    SVN_ERR_ASSERT(ffd->svn_fs_open_);
> >
> > +  fs_config = apr_hash_make(pool);
> > +  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_DELTAS, "0");
> > +  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_FULLTEXTS, "0");
> > +  svn_hash_sets(fs_config, SVN_FS_CONFIG_FSFS_CACHE_REVPROPS, "0");
> > +  /* ### TODO: are there any other intra-process caches that FS
> populated
> > +               and FT will read from?  Can we disable those (for FT at
> least)?
> > +   */
> > +
> >    SVN_ERR(ffd->svn_fs_open_(&ft, fs->path,
> > -                            NULL /* ### TODO fs_config */,
> > +                            fs_config,
> >                              pool));
> >    ft_ffd = ft->fsap_data;
> >
> >
> >
>



-- 
*Join one of our free daily demo sessions on* *Scaling Subversion for the
Enterprise <http://www.wandisco.com/training/webinars>*
*

*