-
Notifications
You must be signed in to change notification settings - Fork 184
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
feat(repository): cache engine-relevant methods #270
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good, only additional thing is the failing load test and integration test. We discussed the integration test and making the expiry configurable.
Looks like the load test is failing with:
cli_e2e_test.go:81: found unexpected goroutines:
[Goroutine 41 in state select, with github.com/hashicorp/golang-lru/v2/expirable.NewLRU[...].func1 on top of the stack:
github.com/hashicorp/golang-lru/v2/expirable.NewLRU[...].func1()
/home/runner/go/pkg/mod/github.com/hashicorp/golang-lru/v2@v2.0.7/expirable/expirable_lru.go:86 +0x1d1
created by github.com/hashicorp/golang-lru/v2/expirable.NewLRU[...] in goroutine 7
/home/runner/go/pkg/mod/github.com/hashicorp/golang-lru/v2@v2.0.7/expirable/expirable_lru.go:82 +0x57d
Goroutine 99 in state select, with github.com/hashicorp/golang-lru/v2/expirable.NewLRU[...].func1 on top of the stack:
github.com/hashicorp/golang-lru/v2/expirable.NewLRU[...].func1()
/home/runner/go/pkg/mod/github.com/hashicorp/golang-lru/v2@v2.0.7/expirable/expirable_lru.go:86 +0x1d1
created by github.com/hashicorp/golang-lru/v2/expirable.NewLRU[...] in goroutine 67
/home/runner/go/pkg/mod/github.com/hashicorp/golang-lru/v2@v2.0.7/expirable/expirable_lru.go:82 +0x57d
]
a7a7995
to
d5b231a
Compare
673d57f
to
6946884
Compare
24764c3
to
ea898c7
Compare
ea898c7
to
fa3166e
Compare
|
||
// Get retrieves the value associated with the given key from the cache. | ||
func (c *TTLCache[K, V]) Get(key K) (V, bool) { | ||
c.mu.Lock() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to add a benchmark test to see how many Gets/second we can handle. Don't imagine this will be a bottleneck anytime soon but would still be good to have a sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, tracked via #282
* feat(repository): cache engine-relevant methods * feat(repository): use generic cacheable function * improvements * undo skip
Description
Fixes # (issue)
Type of change
What's Changed