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 LRU cache for statement parser #3492

Merged
merged 5 commits into from
Feb 22, 2024

Conversation

JonasKunz
Copy link
Contributor

@JonasKunz JonasKunz commented Jan 15, 2024

What does this PR do?

Replaces the caching for our Statement parsing with a simple LRU cache.
Previously we, used a bounded Map and made some exclusions to caching:

  • Non-prepared statements were not cached
  • Too long or too short statements were not cached

This could lead to edge cases where the caching is not effective, which we encountered with a user.
This PR switches to the same caching approach also used by the OTel-agent: Simply cache everything, but use a bounded LRU cache.

Checklist

@JonasKunz JonasKunz added the ci:agent-integration Enables agent integration tests in build pipeline label Jan 15, 2024
@JonasKunz JonasKunz marked this pull request as ready for review January 15, 2024 13:31
@JonasKunz JonasKunz marked this pull request as draft January 15, 2024 13:31
@v1v
Copy link
Member

v1v commented Feb 9, 2024

run docs-build

@JonasKunz JonasKunz marked this pull request as ready for review February 22, 2024 10:44
@JonasKunz JonasKunz requested a review from a team February 22, 2024 10:44
Copy link
Contributor

@jackshirazi jackshirazi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need any unit/integration tests beyond what is already there, but we should report a manual test that shows the difference. Not urgent though

@JonasKunz JonasKunz merged commit 06a4903 into elastic:main Feb 22, 2024
23 checks passed
@JonasKunz JonasKunz deleted the lru-statement-cache branch February 22, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-java ci:agent-integration Enables agent integration tests in build pipeline
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants