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

Pull Queries: Logger in PullQueryExecutor results in Memory Leak #5527

Closed
AlanConfluent opened this issue Jun 2, 2020 · 2 comments · Fixed by #5532
Closed

Pull Queries: Logger in PullQueryExecutor results in Memory Leak #5527

AlanConfluent opened this issue Jun 2, 2020 · 2 comments · Fixed by #5532
Assignees
Labels

Comments

@AlanConfluent
Copy link
Member

Describe the bug
Discussed here:
https://confluentcommunity.slack.com/archives/C6UJNMY67/p1590998942212800

The logger factory io.confluent.common.logging.StructuredLoggerFactory keeps a ConcurrentHashmap of loggers used, never clearing them. If you look in PullQueryExecutor, the query id is passed as part of the name. This creates a large blowup in loggers as more queries come in.

To Reproduce
Steps to reproduce the behavior, include:
On master, generate a lot of pull queries, and set the java memory to something small. Users were able to hit this while generating high QPS for 30 minutes.

Expected behavior
No memory issues.

Screenshot from a user. This shows a persistent query (so doesn't fully display this case), but you can see that the query ids are part of the logger names and that they take a lot of memory:
Screenshot 2020-06-02 at 9 01 51 PM

@AlanConfluent
Copy link
Member Author

I did a memory profile on this and I've confirmed that the memory use keeps going up and up with lots of pull queries and the source is io.confluent.common.logging.StructuredLoggerFactory

@AlanConfluent
Copy link
Member Author

The loggers are not written to be used this way since the slf4j logger factory itself appears to keep references to the loggers it gives out by name.

I think the solution and intended use is more something like and MDC (http://logback.qos.ch/manual/mdc.html). That way you could always add the query id and other structured data.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants