lucene-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Varun Thacker (JIRA)" <>
Subject [jira] [Commented] (SOLR-11884) find/fix inefficiencies in our use of logging
Date Tue, 23 Jan 2018 23:30:00 GMT


Varun Thacker commented on SOLR-11884:

Hi Erick,

How about we split this discussion into two parts?

part1: where the %C is grabbing a stack tace and we should disable that by default and tackle
that in this jira
part2: where expensive log messages should be parameterized . This could be addressed via
SOLR-10415 and jiras linked from there

If you think this is a good idea then we could edit the Description/Title of the Jira to only
cover the first part so that when we look back at this Jira we'll be more clear as to what
this was addressing

> find/fix inefficiencies in our use of logging
> ---------------------------------------------
>                 Key: SOLR-11884
>                 URL:
>             Project: Solr
>          Issue Type: Improvement
>      Security Level: Public(Default Security Level. Issues are Public) 
>            Reporter: Erick Erickson
>            Assignee: Erick Erickson
>            Priority: Major
> We've been looking at Solr using Flight Recorder and ran across some interesting things
I'd like to discuss. Let's discuss general logging approaches here, then perhaps break out
sub-JIRAs when we reach any kind of agreement.
> 1> Every log message generates a new Throwable, presumably to get things like line
number, file, class name and the like. On a 2 minute run blasting updates this meant 150,000
(yes, 150K) instances of "new Throwable()".
> See the section "Asynchronous Logging with Caller Location Information" at:
> []
> I'm not totally sure changing the layout pattern will fix this in log4j 1.x, but apparently
certainly should in log4j 2.
> The cost of course would be that lots of our log messages would lack some of the information.
Exceptions would still contain all the file/class/line information of course.
> Proposal:
> Change the layout pattern to, by default, _NOT_  include information that requires a
Throwable to be created. Also include a pattern that could be un-commented to get this information
back for troubleshooting.
> --------------------
> We generate strings when we don't need them. Any construct like
>"whatever " + method_that_builds_a_string + " : " + some_variable);
> generates the string (some of which are quite expensive) and then throws it away if the
log level is at, say, WARN. The above link also shows that parameterizing this doesn't suffer
this problem, so anything like the above should be re-written as:
>"whatever {} : {} ", method_that_builds_a_string, some_variable);
> The alternative is to do something like but let's make use of the built-in capabilities
> if (log.level >= INFO) {
>"whatever " + method_that_builds_a_string + " : " + some_variable);
> }
> etc.
> This would be a pretty huge thing to fix all-at-once so I suppose we'll have to approach
it incrementally. It's also something that, if we get them all out of the code should be added
to precommit failures. In the meantime, if anyone who has the precommit chops could create
a target that checked for this it'd be a great help in tracking all of them down, then could
be incorporated in the regular precommit checks if/when they're all removed.
> Proposal:
> Use JFR or whatever to identify the egregious violations of this kind of thing (I have
a couple I've found) and change them to parameterized form (and prove it works). Then see
what we can do to move forward with removing them all through the code base.

This message was sent by Atlassian JIRA

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message