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

CosmosDiagnostics: Fixes memory leak due to unbounded growth of CosmosDiagnosticsContextCore and removes reference to response stream. #2129

Merged
merged 15 commits into from
Jan 26, 2021

Conversation

neildsh
Copy link
Contributor

@neildsh neildsh commented Jan 19, 2021

Pull Request Template

Description

NetworkAttachedDocumentContainer holds on to a single CosmosDiagnostics context, and adds diagnostics for each response to this context. for long running queries, this amounts to growing the size of CosmosDiagnostics without bound.

In addition StoreResponseStatistics was holding on the entire StoreResult object including the response stream, which leads to the very high memory consumption.

This change severs the link between StoreResponseStatistics and the StoreResult, and it also uses a bounded circular buffer instead of a list to store child contexts in CosmosDiagnosticsCore

Type of change

Please delete options that are not relevant.

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

Closing issues

To automatically close an issue: closes #IssueNumber

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Please follow the required format: "[Internal] Category: (Adds|Fixes|Refactors) Description"

Examples:
Diagnostics: Adds GetElapsedClientLatency to CosmosDiagnostics
PartitionKey: Fixes null reference when using default(PartitionKey)
[v4] Client Encryption: Refactors code to external project
[Internal] Query: Adds code generator for CosmosNumbers for easy additions in the future.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Please follow the required format: "[Internal] Category: (Adds|Fixes|Refactors) Description"

Examples:
Diagnostics: Adds GetElapsedClientLatency to CosmosDiagnostics
PartitionKey: Fixes null reference when using default(PartitionKey)
[v4] Client Encryption: Refactors code to external project
[Internal] Query: Adds code generator for CosmosNumbers for easy additions in the future.

@bchong95 bchong95 changed the title Plug memory leak due to unbounded growth of CosmosDiagnosticsContextCore CosmosDiagnostics: Fixes memory leak due to unbounded growth of CosmosDiagnosticsContextCore and removes reference to response stream. Jan 19, 2021
@github-actions github-actions bot dismissed stale reviews from themself January 19, 2021 18:59

All good!

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.

Please check the comments about thread-safety and the Region public API change

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.

5 participants