Philip Martin wrote on Mon, Nov 21, 2011 at 18:13:53 +0000:
> "Daniel Shahaf" <d...@daniel.shahaf.name> writes:
>
> > On Monday, November 21, 2011 5:45 PM, "Philip Martin"
> > <philip.mar...@wandisco.com> wrote:
> >> phi...@apache.org writes:
> >>
> >> >
> >> > - ap_log_rerror(APLOG_MARK, APLOG_ERR, APR_EGENERAL, r, "%s",
> >> > err->message);
> >> > + while (err)
> >> > + {
> >> > + ap_log_rerror(APLOG_MARK, APLOG_ERR, APR_EGENERAL, r, "%s",
> >> > err->message);
> >> > + err = err->child;
> >> > + }
> >> > }
> >>
> >> I'm wondering whether this is correct. Should it skip duplicates like
> >> svn_handle_error2? Should it use svn_err_best_message like
> >> svnserve/serve.c:log_error?
> >
> > Using svn_err_best_message() would DTRT when err->message is NULL.
>
> ap_log_rerror handles NULL.
>
And the log also includes the numeric value of err->apr_err. So the
difference is whether the log would include svn_strerror(err->apr)err)
or not.
> > Filtering duplicates would be nice. (I'm thinking of multiple processes
> > logging to the same file; it would surely make the file more readable if
> > each process didn't repeat a given message N times.) Perhaps some logic
> > from svn_handle_error2() could be reused?
>
> I suppose it depends on whether the tracing should be logged. svnserve
> use svn_err_best_message and doesn't remove duplicates, so the log looks
> like:
>
> 25058 2011-11-21T17:13:44.479972Z 127.0.0.1 - repo ERR
> ../src/subversion/libsvn_fs_fs/rep-cache.c 226 200029 Couldn't open rep-cache
> database
> 25058 2011-11-21T17:13:44.479972Z 127.0.0.1 - repo ERR-
> ../src/subversion/libsvn_fs_fs/rep-cache.c 124 200029 Couldn't open rep-cache
> database
> 25058 2011-11-21T17:13:44.479972Z 127.0.0.1 - repo ERR-
> ../src/subversion/libsvn_subr/atomic.c 60 200029 Couldn't perform atomic
> initialization
> 25058 2011-11-21T17:13:44.479972Z 127.0.0.1 - repo ERR-
> ../src/subversion/libsvn_fs_fs/rep-cache.c 103 200029 Couldn't perform atomic
> initialization
> 25058 2011-11-21T17:13:44.479972Z 127.0.0.1 - repo ERR-
> ../src/subversion/libsvn_subr/sqlite.c 749 200029 Couldn't perform atomic
> initialization
> 25058 2011-11-21T17:13:44.479972Z 127.0.0.1 - repo ERR-
> ../src/subversion/libsvn_subr/atomic.c 60 200029 Couldn't perform atomic
> initialization
> 25058 2011-11-21T17:13:44.479972Z 127.0.0.1 - repo ERR-
> ../src/subversion/libsvn_subr/sqlite.c 611 200030 SQLite compiled for 3.7.3,
> but running with 3.6.3
>
> Perhaps svn_error_purge_tracing?
>
I don't think there's a need to remove tracing links when logging to
a file. The only persons likely to read such a file are svn devs (and
they'd probably appreciate the extra context).
I'm not strongly leannig either way re having deduplicating; if you
think it makes more sense not to do so, that'd be fine with me.
> --
> uberSVN: Apache Subversion Made Easy
> http://www.uberSVN.com
