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

AsyncCache: Fixes the cache to remove values if generator throws an exception #2512

Merged

Conversation

johngallardo
Copy link
Member

@johngallardo johngallardo commented May 28, 2021

Pull Request Template

Description

Implement logic within AsyncCache to immediately remove entries from the cache when the generator function throws an exception. These cache entries are effectively ignored today anyways, as subsequent retrievals simply execute the newly provided generator function. Without this change, AsyncLazy instances are cached for a potentially indefinite amount of time. In some services, this manifests as a slow memory leak as the exception objects gradually accumulate over time in the cache.

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)

Closing issues

closes #2514

@j82w
Copy link
Contributor

j82w commented May 28, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

Copy link
Member

@ealsur ealsur left a comment

Choose a reason for hiding this comment

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

LGTM

@j82w
Copy link
Contributor

j82w commented Jun 2, 2021

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 2 pipeline(s).

@j82w j82w changed the title AsyncCache - Remove exception throwing generators from the cache. AsyncCache: Fixes the cache to remove values if generator throws an exception Jun 2, 2021
@j82w j82w merged commit 6477301 into Azure:master Jun 2, 2021
@johngallardo johngallardo deleted the users/johngallardo/asynccache-exception-remove branch June 4, 2021 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CosmosClient's AsyncCache implementation unnecessarily retains exceptions
3 participants