-
Notifications
You must be signed in to change notification settings - Fork 15
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
Update events forwarder cache to run cleanup manually when cache expires #549
Conversation
Codecov Report
@@ Coverage Diff @@
## main #549 +/- ##
==========================================
- Coverage 83.79% 83.75% -0.04%
==========================================
Files 84 84
Lines 5368 5369 +1
==========================================
- Hits 4498 4497 -1
- Misses 708 709 +1
- Partials 162 163 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
LGTM!
LGTM! |
Not sure I got it. Aren't the clients being closed in the OnEvict method? How are the connections being kept open? |
The OnEvict is not guaranteed to run for all cache items. The execution of this function only happens in the cleanup ticket that runs in a goroutine. If you replace the cache item after its expiration, but before the goroutine execution, the onEvict is never called, since the map item is replaced with the new one with a different expiration time. |
Nice, got it! Thanks! |
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.
LGTM
Relates to the following issue: patrickmn/go-cache#141
Description
The caching dependency is not running the cleanup properly due to how the cache is implemented in the events forwarder client.
The cache implementation only runs calls
OnEvict
eachcleanupInterval
, for each cache item that is expired. Whenever we try to use a cached connection that is expired, we create a new connection, which in turn uses a new expiration date.If we try to access a cached connection that is expired but not properly evicted, we will end up replacing it before calling the
close()
procedure and we will end up with opened grpc connections.The pipeline change is necessary due to Github updating its ubuntu-latest image to Ubuntu 22.04 and Ubuntu 22.04 not being supported by K3s