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

*: Updating hashicorp LRU cache to v2 #7306

Merged
merged 4 commits into from
Apr 26, 2024

Conversation

pedro-stanaka
Copy link
Contributor

  • I added CHANGELOG entry for this change.
  • Change is not relevant to the end user.

Changes

I am upgrading the lib github.com/hashicorp/golang-lru to a new major version in preparation for an upcoming PR.

Verification

Unit tests, should not have big impact.

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
@pedro-stanaka
Copy link
Contributor Author

@yeya24 pinging you for review since you implemented the first version. I dont see an easy way to use the expirable LRU from hashicorp directly, but if you have an idea tell me here and I will work on it.

@pull-request-size pull-request-size bot added size/M and removed size/S labels Apr 25, 2024
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
@pedro-stanaka pedro-stanaka force-pushed the use-expirable-cache branch 2 times, most recently from c8206db to 619d0eb Compare April 25, 2024 17:04
Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
@yeya24
Copy link
Contributor

yeya24 commented Apr 26, 2024

@pedro-stanaka Sorry I don't remember that I implemented the code at the beginning :(
I am trying to understand why we need to use the expiration feature in an inmemory LRU since most of the cases I find that relying on LRU eviction is good enough since the inmemory cache has limited size.

This PR looks good to me. Let's implement LRU expiration in a separate PR. WDYT?

@pedro-stanaka
Copy link
Contributor Author

Yeah. From my side it would be okay to merge like this. Looking forward to approvals. 😆

Copy link
Contributor

@yeya24 yeya24 left a comment

Choose a reason for hiding this comment

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

Thanks!

@yeya24 yeya24 merged commit e6fc833 into thanos-io:main Apr 26, 2024
20 checks passed
Nashluffy pushed a commit to Nashluffy/thanos that referenced this pull request May 14, 2024
* *: Updating hashicorp LRU cache to v2

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>

* Adding some new comments regarding removing complexity of TTL

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>

* Using new version everywhere

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>

* rephrase the comment

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>

---------

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
Signed-off-by: mluffman <nashluffman@gmail.com>
jnyi pushed a commit to jnyi/thanos that referenced this pull request Jun 1, 2024
* *: Updating hashicorp LRU cache to v2

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>

* Adding some new comments regarding removing complexity of TTL

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>

* Using new version everywhere

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>

* rephrase the comment

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>

---------

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
jnyi pushed a commit to jnyi/thanos that referenced this pull request Jun 4, 2024
* *: Updating hashicorp LRU cache to v2

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>

* Adding some new comments regarding removing complexity of TTL

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>

* Using new version everywhere

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>

* rephrase the comment

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>

---------

Signed-off-by: Pedro Tanaka <pedro.tanaka@shopify.com>
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 this pull request may close these issues.

2 participants