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

Wrap debug log statements in conditionals to save allocations #976

Merged
merged 1 commit into from
Sep 6, 2019

Conversation

slim-bean
Copy link
Collaborator

A lot of memory is allocated and has to be garbage collected around forming debug log statements in the log processing pipeline. 99% of the time people will have debug logging turned off yet these allocations still take place, this profile is with debug logging OFF:

profile002

This is the same test with debug log statements wrapped in a conditional:

profile001

The test in this case was writing 100 log files with 50k log lines each and then taking a profile

…prevent unnecessary allocations when debug logging is not enabled
@slim-bean slim-bean merged commit a2e2272 into master Sep 6, 2019
@pracucci
Copy link
Contributor

pracucci commented Sep 8, 2019

Does this PR fixes #904 too?

@slim-bean
Copy link
Collaborator Author

Hrm, I don't think so directly, but maybe indirectly? I wasn't really ever able to figure out what was causing #904, the only other input I had from the original report was their logs which showed the memory use corresponding to a very high number of logs sent receiving a 400 response because their timestamp was older than Loki would accept. It's very possible though the speed at which they were processed and rejected (and log statements generated) was causing a very high amount of allocation the GC couldn't keep up with and causing the memory to grow?

@pracucci
Copy link
Contributor

I see your point @slim-bean. I agree it doesn't directly solve it. I was referring to the fact that the issue #904 mentions to apply an optimization to logger's debug statements which is what this PR has done.

My point on #904 is: the information shared in the issue looks not enough to further investigate it (or even correlate it with similar symptoms we may see in the future). Unless we're able to provide further data, I'm not sure PR #904 is much actionable.

@rfratto rfratto deleted the promtail-debug-logging branch November 1, 2019 12:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants