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

Avoid unnecessary creation of prefix loggers #20571

Merged
merged 1 commit into from Sep 20, 2016

Conversation

Projects
None yet
3 participants
@jasontedor
Copy link
Member

commented Sep 19, 2016

Today when acquiring a prefix logger for a logger info stream, we obtain
a new prefix logger per invocation. This can lead to contention on the
markers lock in the constructor of PrefixLogger. Usually this is not a
problem (because the vast majority of callers hold on to the logger they
obtain). Unfortunately, under heavy indexing with multiple threads, the
contention on the lock can be devastating. This commit modifies
LoggerInfoStream to hold on to the loggers it obtains to avoid
contending over the lock there.

Closes #20570

Avoid unnecessary creation of prefix loggers
Today when acquiring a prefix logger for a logger info stream, we obtain
a new prefix logger per invocation. This can lead to contention on the
markers lock in the constructor of PrefixLogger. Usually this is not a
problem (because the vast majority of callers hold on to the logger they
obtain). Unfortunately, under heavy indexing with multiple threads, the
contention on the lock can be devastating. This commit modifies
LoggerInfoStream to hold on to the loggers it obtains to avoid
contending over the lock there.
@mikemccand

This comment has been minimized.

Copy link
Contributor

commented Sep 19, 2016

LGTM

@jasontedor jasontedor merged commit aa51015 into elastic:master Sep 20, 2016

2 checks passed

CLA Commit author is a member of Elasticsearch
Details
elasticsearch-ci Build finished.
Details
@jasontedor

This comment has been minimized.

Copy link
Member Author

commented Sep 20, 2016

Thanks @mikemccand!

@jasontedor jasontedor deleted the jasontedor:logger-info-stream-new-loggers branch Sep 20, 2016

jasontedor added a commit that referenced this pull request Sep 20, 2016

Avoid unnecessary creation of prefix loggers
Today when acquiring a prefix logger for a logger info stream, we obtain
a new prefix logger per invocation. This can lead to contention on the
markers lock in the constructor of PrefixLogger. Usually this is not a
problem (because the vast majority of callers hold on to the logger they
obtain). Unfortunately, under heavy indexing with multiple threads, the
contention on the lock can be devastating. This commit modifies
LoggerInfoStream to hold on to the loggers it obtains to avoid
contending over the lock there.

Relates #20571

jasontedor added a commit that referenced this pull request Sep 20, 2016

Avoid unnecessary creation of prefix loggers
Today when acquiring a prefix logger for a logger info stream, we obtain
a new prefix logger per invocation. This can lead to contention on the
markers lock in the constructor of PrefixLogger. Usually this is not a
problem (because the vast majority of callers hold on to the logger they
obtain). Unfortunately, under heavy indexing with multiple threads, the
contention on the lock can be devastating. This commit modifies
LoggerInfoStream to hold on to the loggers it obtains to avoid
contending over the lock there.

Relates #20571

@clintongormley clintongormley removed the v5.1.1 label Dec 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.