-
Notifications
You must be signed in to change notification settings - Fork 3.8k
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
release-23.1: sql/parser: fix memory leak of scanned comments #127757
Conversation
This commit fixes a memory leak that cause a session's `scanner.Scanner`, which is embedded in a `pgwire.conn` via a `parser.Parser`, to retain previously scanned SQL comments in all parsed SQL statements. The scanner's slice of comments would continue to grow as it parsed new SQL strings with comments, as long as the session remained open. Also, the slice of comments holds references to the bytes in `pgwire` read buffers, preventing those buffers from being GC'd. Fixes #127710 Release note (bug fix): A bug has been fixed that caused a memory leak when executing SQL statements with comments, for example, `SELECT /* comment */ 1;`. Memory owned by a SQL session would continue to grow as these types of statements were executed. The memory would only be released when closing the SQL session. This bug has been present since version 23.1.
582dd10
to
732417b
Compare
Thanks for opening a backport. Please check the backport criteria before merging:
If your backport adds new functionality, please ensure that the following additional criteria are satisfied:
Also, please add a brief release justification to the body of your PR to justify this |
It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR? 🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf. |
@yuzefovich I saw you added the |
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.
I don't see any scheduled patch releases for 23.1
I heard that v23.1.25 is scheduled for release in August.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @fqazi and @yuzefovich)
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.
Reviewed 1 of 1 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @fqazi)
Backport 1/1 commits from #127711 on behalf of @mgartner.
/cc @cockroachdb/release
This commit fixes a memory leak that cause a session's
scanner.Scanner
, which is embedded in apgwire.conn
via aparser.Parser
, to retain previously scanned SQL comments in all parsedSQL statements. The scanner's slice of comments would continue to grow
as it parsed new SQL strings with comments, as long as the session
remained open. Also, the slice of comments holds references to the bytes
in
pgwire
read buffers, preventing those buffers from being GC'd.Fixes #127710
Release note (bug fix): A bug has been fixed that caused a memory leak
when executing SQL statements with comments, for example,
SELECT /* comment */ 1;
. Memory owned by a SQL session would continueto grow as these types of statements were executed. The memory would
only be released when closing the SQL session. This bug has been present
since version 23.1.
Release justification: Low-risk fix for a memory leak.