Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use "info" as the default root logger level #5079

Merged
merged 1 commit into from
Aug 30, 2019
Merged

Conversation

merlimat
Copy link
Contributor

Motivation

In #3661 the root logger level was made configurable and the level was changed from info to debug.

That change had the side effect that the control:

if (log.isDebugEnabled()) {
  log.debug("{}", computeExpensiveString());
}

is now passing the if condition and it's triggering the call to computeExpensiveString() whose value is then never printed. This happens for all non pulsar logger (eg. org.apache.zookeeper.. client logs), with the result of lot of garbage getting created.

Changing back the default value to info.

@merlimat merlimat added the type/bug The PR fixed a bug or issue reported a bug label Aug 30, 2019
@merlimat merlimat added this to the 2.4.2 milestone Aug 30, 2019
@merlimat merlimat self-assigned this Aug 30, 2019
@massakam
Copy link
Contributor

rerun java8 tests

@merlimat merlimat merged commit 3cbfa10 into apache:master Aug 30, 2019
@cdbartholomew
Copy link
Contributor

I believe that this new default was also triggering a bug 3113 in jetty: "Logging of key.readyOps() can throw unchecked CancelledKeyException"

This bug is only exposed if debug logging is enabled:

if (LOG.isDebugEnabled()) LOG.debug("selected {} {} {} ",key.readyOps(),key,attachment);

I was seeing this in the broker after sending lots of REST requests. After hitting this bug, the broker no longer responds to REST requests, so it's pretty ugly.

Once I changed root level logging to info, I no longer see the jetty bug.

I think this is an important change to get in the next milestone so people don't trip over this. It's also something to note can happen with debug logging enabled the broker even when this default is changed, so it might be worth considering upgrading the jetty version. The jetty bug is fixed in 9.4.15 (current Pulsar version is 9.4.12).

wolfstudy pushed a commit that referenced this pull request Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants