You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2012/10/20 20:35:24 UTC

svn commit: r1400498 - /subversion/trunk/subversion/svnadmin/main.c

Author: stefan2
Date: Sat Oct 20 18:35:24 2012
New Revision: 1400498

URL: http://svn.apache.org/viewvc?rev=1400498&view=rev
Log:
On systems without efficient 64 bit atomics, svnadmin should not attempt
to enable revprop caching because FSFS will reject it and log a warning.
svnadmin writes the latter to stderr - which confuses our tests.

* subversion/svnadmin/main.c
  (open_repos): enable revprop caching only if efficient

Modified:
    subversion/trunk/subversion/svnadmin/main.c

Modified: subversion/trunk/subversion/svnadmin/main.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnadmin/main.c?rev=1400498&r1=1400497&r2=1400498&view=diff
==============================================================================
--- subversion/trunk/subversion/svnadmin/main.c (original)
+++ subversion/trunk/subversion/svnadmin/main.c Sat Oct 20 18:35:24 2012
@@ -43,6 +43,7 @@
 #include "svn_xml.h"
 
 #include "private/svn_opt_private.h"
+#include "private/svn_named_atomic.h"
 
 #include "svn_private_config.h"
 
@@ -115,7 +116,8 @@ open_repos(svn_repos_t **repos,
   apr_hash_set(fs_config, SVN_FS_CONFIG_FSFS_CACHE_FULLTEXTS,
                APR_HASH_KEY_STRING, "1");
   apr_hash_set(fs_config, SVN_FS_CONFIG_FSFS_CACHE_REVPROPS,
-               APR_HASH_KEY_STRING, "1");
+               APR_HASH_KEY_STRING,
+               svn_named_atomic__is_efficient() ? "1" : "0");
 
   /* now, open the requested repository */
   SVN_ERR(svn_repos_open2(repos, path, fs_config, pool));



Re: svn commit: r1400498 - /subversion/trunk/subversion/svnadmin/main.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Sun, Oct 21, 2012 at 9:42 PM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:

> stefan2@apache.org wrote on Sat, Oct 20, 2012 at 18:35:24 -0000:
> > Author: stefan2
> > Date: Sat Oct 20 18:35:24 2012
> > New Revision: 1400498
> >
> > URL: http://svn.apache.org/viewvc?rev=1400498&view=rev
> > Log:
> > On systems without efficient 64 bit atomics, svnadmin should not attempt
> > to enable revprop caching because FSFS will reject it and log a warning.
> > svnadmin writes the latter to stderr - which confuses our tests.
> >
> > * subversion/svnadmin/main.c
> >   (open_repos): enable revprop caching only if efficient
> >
> > Modified:
> >     subversion/trunk/subversion/svnadmin/main.c
> >
> > Modified: subversion/trunk/subversion/svnadmin/main.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svnadmin/main.c?rev=1400498&r1=1400497&r2=1400498&view=diff
> >
> ==============================================================================
> > --- subversion/trunk/subversion/svnadmin/main.c (original)
> > +++ subversion/trunk/subversion/svnadmin/main.c Sat Oct 20 18:35:24 2012
> > @@ -43,6 +43,7 @@
> >  #include "svn_xml.h"
> >
> >  #include "private/svn_opt_private.h"
> > +#include "private/svn_named_atomic.h"
> >
> >  #include "svn_private_config.h"
> >
> > @@ -115,7 +116,8 @@ open_repos(svn_repos_t **repos,
> >    apr_hash_set(fs_config, SVN_FS_CONFIG_FSFS_CACHE_FULLTEXTS,
> >                 APR_HASH_KEY_STRING, "1");
> >    apr_hash_set(fs_config, SVN_FS_CONFIG_FSFS_CACHE_REVPROPS,
> > -               APR_HASH_KEY_STRING, "1");
> > +               APR_HASH_KEY_STRING,
> > +               svn_named_atomic__is_efficient() ? "1" : "0");
>
> svnadmin shouldn't use private APIs here.  (Which probably means this
> code is a layering violation)
>
> Maybe add a value that means "enable if it would be efficient"?  Or just
> unconditionally never enable the cache (regardless of the config) when
> it wouldn't be efficient.
>

r1405553 implements something along those lines now.

-- Stefan^2.

-- 
Certified & Supported Apache Subversion Downloads:
*

http://www.wandisco.com/subversion/download
*

Re: svn commit: r1400498 - /subversion/trunk/subversion/svnadmin/main.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Sun, Oct 21, 2012 at 9:42 PM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:

> stefan2@apache.org wrote on Sat, Oct 20, 2012 at 18:35:24 -0000:
> > Author: stefan2
> > Date: Sat Oct 20 18:35:24 2012
> > New Revision: 1400498
> >
> > URL: http://svn.apache.org/viewvc?rev=1400498&view=rev
> > Log:
> > On systems without efficient 64 bit atomics, svnadmin should not attempt
> > to enable revprop caching because FSFS will reject it and log a warning.
> > svnadmin writes the latter to stderr - which confuses our tests.
> >
> > * subversion/svnadmin/main.c
> >   (open_repos): enable revprop caching only if efficient
> >
> > Modified:
> >     subversion/trunk/subversion/svnadmin/main.c
> >
> > Modified: subversion/trunk/subversion/svnadmin/main.c
> > URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svnadmin/main.c?rev=1400498&r1=1400497&r2=1400498&view=diff
> >
> ==============================================================================
> > --- subversion/trunk/subversion/svnadmin/main.c (original)
> > +++ subversion/trunk/subversion/svnadmin/main.c Sat Oct 20 18:35:24 2012
> > @@ -43,6 +43,7 @@
> >  #include "svn_xml.h"
> >
> >  #include "private/svn_opt_private.h"
> > +#include "private/svn_named_atomic.h"
> >
> >  #include "svn_private_config.h"
> >
> > @@ -115,7 +116,8 @@ open_repos(svn_repos_t **repos,
> >    apr_hash_set(fs_config, SVN_FS_CONFIG_FSFS_CACHE_FULLTEXTS,
> >                 APR_HASH_KEY_STRING, "1");
> >    apr_hash_set(fs_config, SVN_FS_CONFIG_FSFS_CACHE_REVPROPS,
> > -               APR_HASH_KEY_STRING, "1");
> > +               APR_HASH_KEY_STRING,
> > +               svn_named_atomic__is_efficient() ? "1" : "0");
>
> svnadmin shouldn't use private APIs here.  (Which probably means this
> code is a layering violation)
>
> Maybe add a value that means "enable if it would be efficient"?  Or just
> unconditionally never enable the cache (regardless of the config) when
> it wouldn't be efficient.
>

r1405553 implements something along those lines now.

-- Stefan^2.

-- 
Certified & Supported Apache Subversion Downloads:
*

http://www.wandisco.com/subversion/download
*

Re: svn commit: r1400498 - /subversion/trunk/subversion/svnadmin/main.c

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stefan2@apache.org wrote on Sat, Oct 20, 2012 at 18:35:24 -0000:
> Author: stefan2
> Date: Sat Oct 20 18:35:24 2012
> New Revision: 1400498
> 
> URL: http://svn.apache.org/viewvc?rev=1400498&view=rev
> Log:
> On systems without efficient 64 bit atomics, svnadmin should not attempt
> to enable revprop caching because FSFS will reject it and log a warning.
> svnadmin writes the latter to stderr - which confuses our tests.
> 
> * subversion/svnadmin/main.c
>   (open_repos): enable revprop caching only if efficient
> 
> Modified:
>     subversion/trunk/subversion/svnadmin/main.c
> 
> Modified: subversion/trunk/subversion/svnadmin/main.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnadmin/main.c?rev=1400498&r1=1400497&r2=1400498&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/svnadmin/main.c (original)
> +++ subversion/trunk/subversion/svnadmin/main.c Sat Oct 20 18:35:24 2012
> @@ -43,6 +43,7 @@
>  #include "svn_xml.h"
>  
>  #include "private/svn_opt_private.h"
> +#include "private/svn_named_atomic.h"
>  
>  #include "svn_private_config.h"
>  
> @@ -115,7 +116,8 @@ open_repos(svn_repos_t **repos,
>    apr_hash_set(fs_config, SVN_FS_CONFIG_FSFS_CACHE_FULLTEXTS,
>                 APR_HASH_KEY_STRING, "1");
>    apr_hash_set(fs_config, SVN_FS_CONFIG_FSFS_CACHE_REVPROPS,
> -               APR_HASH_KEY_STRING, "1");
> +               APR_HASH_KEY_STRING,
> +               svn_named_atomic__is_efficient() ? "1" : "0");

svnadmin shouldn't use private APIs here.  (Which probably means this
code is a layering violation)

Maybe add a value that means "enable if it would be efficient"?  Or just
unconditionally never enable the cache (regardless of the config) when
it wouldn't be efficient.

Re: svn commit: r1400498 - /subversion/trunk/subversion/svnadmin/main.c

Posted by Ben Reser <be...@reser.org>.
On Sun, Oct 21, 2012 at 11:56 AM, Stefan Fuhrmann
<st...@wandisco.com> wrote:
> All caches are enabled by default. With revprop packing,
> we read the whole packed revprop file only once and put
> all items into the cache. That speeds up dump & friends.
>
> Also, any manipulation to some revprop will instantly
> become visible to all other processes (cache invalidation).

Ok thanks, makes sense.

Re: svn commit: r1400498 - /subversion/trunk/subversion/svnadmin/main.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Sun, Oct 21, 2012 at 8:35 PM, Ben Reser <be...@reser.org> wrote:

> On Sat, Oct 20, 2012 at 11:35 AM,  <st...@apache.org> wrote:
> > Author: stefan2
> > Date: Sat Oct 20 18:35:24 2012
> > New Revision: 1400498
> >
> > URL: http://svn.apache.org/viewvc?rev=1400498&view=rev
> > Log:
> > On systems without efficient 64 bit atomics, svnadmin should not attempt
> > to enable revprop caching because FSFS will reject it and log a warning.
> > svnadmin writes the latter to stderr - which confuses our tests.
> >
> > * subversion/svnadmin/main.c
> >   (open_repos): enable revprop caching only if efficient
> >
> > Modified:
> >     subversion/trunk/subversion/svnadmin/main.c
>
> What value does the revprop cache have for svnadmin?  Are there any
> operations that would actually look at revprops more than once in a
> single svnadmin run?
>

All caches are enabled by default. With revprop packing,
we read the whole packed revprop file only once and put
all items into the cache. That speeds up dump & friends.

Also, any manipulation to some revprop will instantly
become visible to all other processes (cache invalidation).

-- Stefan^2.

-- 
*

Join us this October at Subversion Live
2012<http://www.wandisco.com/svn-live-2012>
 for two days of best practice SVN training, networking, live demos,
committer meet and greet, and more! Space is limited, so get signed up
today<http://www.wandisco.com/svn-live-2012>
!
*

Re: svn commit: r1400498 - /subversion/trunk/subversion/svnadmin/main.c

Posted by Ben Reser <be...@reser.org>.
On Sat, Oct 20, 2012 at 11:35 AM,  <st...@apache.org> wrote:
> Author: stefan2
> Date: Sat Oct 20 18:35:24 2012
> New Revision: 1400498
>
> URL: http://svn.apache.org/viewvc?rev=1400498&view=rev
> Log:
> On systems without efficient 64 bit atomics, svnadmin should not attempt
> to enable revprop caching because FSFS will reject it and log a warning.
> svnadmin writes the latter to stderr - which confuses our tests.
>
> * subversion/svnadmin/main.c
>   (open_repos): enable revprop caching only if efficient
>
> Modified:
>     subversion/trunk/subversion/svnadmin/main.c

What value does the revprop cache have for svnadmin?  Are there any
operations that would actually look at revprops more than once in a
single svnadmin run?

Re: svn commit: r1400498 - /subversion/trunk/subversion/svnadmin/main.c

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
stefan2@apache.org wrote on Sat, Oct 20, 2012 at 18:35:24 -0000:
> Author: stefan2
> Date: Sat Oct 20 18:35:24 2012
> New Revision: 1400498
> 
> URL: http://svn.apache.org/viewvc?rev=1400498&view=rev
> Log:
> On systems without efficient 64 bit atomics, svnadmin should not attempt
> to enable revprop caching because FSFS will reject it and log a warning.
> svnadmin writes the latter to stderr - which confuses our tests.
> 
> * subversion/svnadmin/main.c
>   (open_repos): enable revprop caching only if efficient
> 
> Modified:
>     subversion/trunk/subversion/svnadmin/main.c
> 
> Modified: subversion/trunk/subversion/svnadmin/main.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnadmin/main.c?rev=1400498&r1=1400497&r2=1400498&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/svnadmin/main.c (original)
> +++ subversion/trunk/subversion/svnadmin/main.c Sat Oct 20 18:35:24 2012
> @@ -43,6 +43,7 @@
>  #include "svn_xml.h"
>  
>  #include "private/svn_opt_private.h"
> +#include "private/svn_named_atomic.h"
>  
>  #include "svn_private_config.h"
>  
> @@ -115,7 +116,8 @@ open_repos(svn_repos_t **repos,
>    apr_hash_set(fs_config, SVN_FS_CONFIG_FSFS_CACHE_FULLTEXTS,
>                 APR_HASH_KEY_STRING, "1");
>    apr_hash_set(fs_config, SVN_FS_CONFIG_FSFS_CACHE_REVPROPS,
> -               APR_HASH_KEY_STRING, "1");
> +               APR_HASH_KEY_STRING,
> +               svn_named_atomic__is_efficient() ? "1" : "0");

svnadmin shouldn't use private APIs here.  (Which probably means this
code is a layering violation)

Maybe add a value that means "enable if it would be efficient"?  Or just
unconditionally never enable the cache (regardless of the config) when
it wouldn't be efficient.