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/05 03:54:12 UTC

svn commit: r1394345 - /subversion/trunk/subversion/libsvn_fs_fs/caching.c

Author: stefan2
Date: Fri Oct  5 01:54:12 2012
New Revision: 1394345

URL: http://svn.apache.org/viewvc?rev=1394345&view=rev
Log:
Since zero-copy code can cause cache functions to actually return
an error (lost connection etc.), we must make sure these get passed
through to the calling functions.

* subversion/libsvn_fs_fs/caching.c
  (warn_on_cache_errors): don't swallow the error, just log it

Modified:
    subversion/trunk/subversion/libsvn_fs_fs/caching.c

Modified: subversion/trunk/subversion/libsvn_fs_fs/caching.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/caching.c?rev=1394345&r1=1394344&r2=1394345&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs_fs/caching.c (original)
+++ subversion/trunk/subversion/libsvn_fs_fs/caching.c Fri Oct  5 01:54:12 2012
@@ -103,8 +103,7 @@ warn_on_cache_errors(svn_error_t *err,
 {
   svn_fs_t *fs = baton;
   (fs->warning)(fs->warning_baton, err);
-  svn_error_clear(err);
-  return SVN_NO_ERROR;
+  return err;
 }
 
 #ifdef SVN_DEBUG_CACHE_DUMP_STATS



Re: svn commit: r1394345 - /subversion/trunk/subversion/libsvn_fs_fs/caching.c

Posted by Stefan Fuhrmann <st...@wandisco.com>.
On Fri, Oct 5, 2012 at 10:14 AM, Bert Huijben <be...@qqmail.nl> wrote:

>
> > -----Original Message-----
> > From: stefan2@apache.org [mailto:stefan2@apache.org]
> > Sent: vrijdag 5 oktober 2012 03:54
> > To: commits@subversion.apache.org
> > Subject: svn commit: r1394345 -
> > /subversion/trunk/subversion/libsvn_fs_fs/caching.c
> >
> > Author: stefan2
> > Date: Fri Oct  5 01:54:12 2012
> > New Revision: 1394345
> >
> > URL: http://svn.apache.org/viewvc?rev=1394345&view=rev
> > Log:
> > Since zero-copy code can cause cache functions to actually return
> > an error (lost connection etc.), we must make sure these get passed
> > through to the calling functions.
> >
> > * subversion/libsvn_fs_fs/caching.c
> >   (warn_on_cache_errors): don't swallow the error, just log it
> >
> > Modified:
> >     subversion/trunk/subversion/libsvn_fs_fs/caching.c
> >
> > Modified: subversion/trunk/subversion/libsvn_fs_fs/caching.c
> > URL:
> > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/ca
> > ching.c?rev=1394345&r1=1394344&r2=1394345&view=diff
> > ==========================================================
> > ====================
> > --- subversion/trunk/subversion/libsvn_fs_fs/caching.c (original)
> > +++ subversion/trunk/subversion/libsvn_fs_fs/caching.c Fri Oct  5
> 01:54:12
> > 2012
> > @@ -103,8 +103,7 @@ warn_on_cache_errors(svn_error_t *err,
> >  {
> >    svn_fs_t *fs = baton;
> >    (fs->warning)(fs->warning_baton, err);
> > -  svn_error_clear(err);
> > -  return SVN_NO_ERROR;
> > +  return err;
> >  }
>
> Usually warnings are non-fatal (/non errors). Maybe you should rename this
> function or handle specific errors?
>
>
svn_fs_t does not have an error callback but the things we
are logging here are real failures from various sources.
Yes, there is a mismatch.

Please note that this is a relatively old functionality
contributed by David Glasser in 2008. My guess is that he
wanted to log things like intermittent memcached failures.
The latter may now be fragile as all kinds of network issues
cancel the respective operation / report.

r1394470 should address this problem.

Thanks for the review!

-- 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: r1394345 - /subversion/trunk/subversion/libsvn_fs_fs/caching.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: stefan2@apache.org [mailto:stefan2@apache.org]
> Sent: vrijdag 5 oktober 2012 03:54
> To: commits@subversion.apache.org
> Subject: svn commit: r1394345 -
> /subversion/trunk/subversion/libsvn_fs_fs/caching.c
> 
> Author: stefan2
> Date: Fri Oct  5 01:54:12 2012
> New Revision: 1394345
> 
> URL: http://svn.apache.org/viewvc?rev=1394345&view=rev
> Log:
> Since zero-copy code can cause cache functions to actually return
> an error (lost connection etc.), we must make sure these get passed
> through to the calling functions.
> 
> * subversion/libsvn_fs_fs/caching.c
>   (warn_on_cache_errors): don't swallow the error, just log it
> 
> Modified:
>     subversion/trunk/subversion/libsvn_fs_fs/caching.c
> 
> Modified: subversion/trunk/subversion/libsvn_fs_fs/caching.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs_fs/ca
> ching.c?rev=1394345&r1=1394344&r2=1394345&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_fs_fs/caching.c (original)
> +++ subversion/trunk/subversion/libsvn_fs_fs/caching.c Fri Oct  5 01:54:12
> 2012
> @@ -103,8 +103,7 @@ warn_on_cache_errors(svn_error_t *err,
>  {
>    svn_fs_t *fs = baton;
>    (fs->warning)(fs->warning_baton, err);
> -  svn_error_clear(err);
> -  return SVN_NO_ERROR;
> +  return err;
>  }

Usually warnings are non-fatal (/non errors). Maybe you should rename this function or handle specific errors?

	Bert