opt: reset Implicator constraint cache in Init#58306
Merged
craig[bot] merged 1 commit intocockroachdb:masterfrom Dec 28, 2020
Merged
opt: reset Implicator constraint cache in Init#58306craig[bot] merged 1 commit intocockroachdb:masterfrom
craig[bot] merged 1 commit intocockroachdb:masterfrom
Conversation
The Implicator's constraint cache is now cleared when `Init` is called. This prevents the constraint cache from growing indefinitely. Release justification: This is a simple fix that prevents memory leaks in the optimizer when planning queries on tables with partial indexes. Release note (bug fix): A memory leak in the optimizer has been fixed. The leak could have caused unbounded growth of memory usage for a session when planning queries on tables with partial indexes.
Member
RaduBerinde
approved these changes
Dec 28, 2020
Member
RaduBerinde
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 0 LGTMs obtained (waiting on @andy-kimball, @RaduBerinde, and @rytaft)
Contributor
Author
|
bors r=RaduBerinde |
Contributor
|
Build succeeded: |
mgartner
added a commit
to mgartner/cockroach
that referenced
this pull request
Dec 31, 2020
We commonly use a pattern of declaring structs and then initializing them via an `Init` method. This allows internal data structures to be reused, reducing allocations. However, the common initialization implementation is dangerous because fields are implicitly reused. This can lead to inconspicuous bugs (see cockroachdb#58306). This commit introduces a new pattern for initialization functions that requires reused fields to be explicitly listed. This should reduce the risk of these types of bugs. Release note: None
craig bot
pushed a commit
that referenced
this pull request
Jan 4, 2021
58386: opt: use safer struct initialization pattern r=rytaft a=mgartner We commonly use a pattern of declaring structs and then initializing them via an `Init` method. This allows internal data structures to be reused, reducing allocations. However, the common initialization implementation is dangerous because fields are implicitly reused. This can lead to inconspicuous bugs (see #58306). This commit introduces a new pattern for initialization functions that requires reused fields to be explicitly listed. This should reduce the risk of these types of bugs. Release note: None Co-authored-by: Marcus Gartner <marcus@cockroachlabs.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Implicator's constraint cache is now cleared when
Initis called.This prevents the constraint cache from growing indefinitely.
Release justification: This is a simple fix that prevents memory leaks
in the optimizer when planning queries on tables with partial indexes.
Release note (bug fix): A memory leak in the optimizer has been fixed.
The leak could have caused unbounded growth of memory usage for a
session when planning queries on tables with partial indexes.