[
https://issues.apache.org/jira/browse/SOLR-4547?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13636999#comment-13636999
]
Hoss Man commented on SOLR-4547:
--------------------------------
Shawn: my previous comment was based on the mistaken impression that these log
messages were leveraging the toString() of the IndexCommit and
List<IndexCommit> objects ... looking at your patch, i realize now that these
ugly "str()" methods were doing the work. sorry for the confusion
specific concerns i have about your patch...
1) in the case of logging a List<IndexCommit>, the existing code does a single
INFO message with the details of each commit, but in your patch each commit
gets it's own log message (x2, INFO and DEBUG) because of how you've got
logCommits() looping and delegating to logCommit(). This seems like a bad idea
since in the case of multiple SolrCores these can easily get interleaved and
hard to coalesce.
2) both the existing str functions and your new logCommit/logCommits functions
suffer from the problem of doing a lot of String building even in the case
where hte logging level is so high the messages will be ignored.
3) i'm not a fan of the the dual INFO/DEBUG logging going on inside of
logCommit/logCommits ... i can't really explain it well, but it strikes me as
being too "hidden" from the flow of the methods that call it, i'd rather see
the log.info and log.debug calls inside each method where it happens so it's
more obvious we are infact logging two different ways there.
My suggestion for dealing with this would probably be...
a) introduce some private static inner classes that are really cheap to
construct and can wrap a List<IndexCommit> and have toString() methods that
render out all the details we want
b) use these new cheap classes directly in info & debug calls that leverage
placeholder markers
something along the lines of (not fully thought through)...
{noformat}
log.info("SolrDeletionPolicy.onCommit: commits: {}", new
CommitsLoggingInfo(commits));
log.info("SolrDeletionPolicy.onCommit: commits: {}", new
CommitsLoggingDebug(commits));
...
private static class CommitsLoggingInfo {
...
public final String toString() {
StringBuilder sb;
...
for (IndexCommit c : commits) {
appendDetails(sb, c));
}
...
return sb.toString();
}
protected void appendDetails(StringBuilder s, IndexCommit c) {
// add summary details, ie: not all the freaking files.
}
}
private static class CommitsLoggingDebug extends CommitsLoggingInfo {
...
protected void appendDetails(StringBuilder s, IndexCommit c) {
super.appendDetails(s, c);
s.append(c.getFileNames());
}
}
{noformat}
(of course, another way to go would be to sprinkly a bunch of "if
(log.isDebugEnabled())" and "if (log.isInfoEnabled())" arround your existing
patch, but that would still leave concern #1)
> Commit logging - move index filenames to DEBUG.
> -----------------------------------------------
>
> Key: SOLR-4547
> URL: https://issues.apache.org/jira/browse/SOLR-4547
> Project: Solr
> Issue Type: Improvement
> Components: update
> Affects Versions: 4.1
> Reporter: Shawn Heisey
> Fix For: 4.3, 5.0
>
> Attachments: solr-4547-insanity.txt, SOLR-4547.patch
>
>
> When an index commit is done (at least with MMap/NRT, haven't checked other
> Directory implementations), all of the filenames in the index are logged
> three times at INFO. I think the information has value to someone if they
> are debugging, but not to most users.
> I propose that we move this part of the log to DEBUG and provide text that
> allows the person/program viewing the log to link the smaller INFO log entry
> with the new one, probably the directory path.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
