FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix(bigquery): dbapi socket leak by chalmerlowe · Pull Request #17921 · googleapis/google-cloud-python · GitHub

/ google-cloud-python Public

fix(bigquery): dbapi socket leak - #17921

Merged
chalmerlowe merged 6 commits into
mainfrom
fix/bigquery-dbapi-socket-leak
Jul 28, 2026
ConversationCommits (6)ChecksFiles changed
Merged

fix(bigquery): dbapi socket leak#17921
chalmerlowe merged 6 commits into
mainfrom
fix/bigquery-dbapi-socket-leak

Conversation

chalmerlowe commented Jul 28, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Problem

The system test test_dbapi_connection_does_not_leak_sockets fails intermittently because network connections remain in the ESTABLISHED state even after connection.close() is called. This is likely due to the underlying gRPC (remote procedure call) transport not being immediately closed, or delays in Garbage Collection (cleaning up unused memory).

Solution

This Pull Request updates Connection.close() to explicitly close the underlying gRPC channel (transport._grpc_channel) if it exists on the BigQuery Storage client. This ensures that the connection is closed immediately and resources are released deterministically.

Notes to Reviewers

  • This change explicitly targets the underlying gRPC channel to bypass potential delays caused by transport interceptors or non-deterministic garbage collection.
  • It addresses intermittent test failures caused by lingering ESTABLISHED sockets.
  • Also includes several linting revisions (sorry).

Sample debug info from traceback:

E           --- Socket Leak Debug Info ---
E           Start Count: 6
E           End Count: 7
E           Current Connections:
E           Status: ESTABLISHED, Laddr: addr(ip='10.138.0.14', port=48550), Raddr: addr(ip='74.125.195.95', port=443)
E           Status: ESTABLISHED, Laddr: addr(ip='10.138.0.14', port=53976), Raddr: addr(ip='173.194.43.95', port=443)
E           Status: ESTABLISHED, Laddr: addr(ip='10.138.0.14', port=54818), Raddr: addr(ip='74.125.199.95', port=443)
E           Status: ESTABLISHED, Laddr: addr(ip='10.138.0.14', port=43762), Raddr: addr(ip='142.251.188.95', port=443)
E           Status: ESTABLISHED, Laddr: addr(ip='10.138.0.14', port=53946), Raddr: addr(ip='173.194.43.95', port=443)
E           Status: ESTABLISHED, Laddr: addr(ip='10.138.0.14', port=54792), Raddr: addr(ip='74.125.199.95', port=443)
E           Status: ESTABLISHED, Laddr: addr(ip='10.138.0.14', port=48524), Raddr: addr(ip='74.125.195.95', port=443)

chalmerlowe self-assigned this Jul 28, 2026
gemini-code-assist Bot reviewed Jul 28, 2026
View reviewed changes

gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Code Review

This pull request updates the Connection.close() method to explicitly close the underlying gRPC channel of the BigQuery Storage client's transport, preventing potential socket leaks. It also refactors unit tests to mock the public transport property instead of the private _transport attribute. The review feedback suggests using mock.patch.object with self.addCleanup to prevent test pollution when mocking the transport property, and using getattr instead of hasattr to safely check for _grpc_channel and avoid potential AttributeErrors if the channel is None.

Comment thread packages/google-cloud-bigquery/tests/unit/test_dbapi_connection.py Outdated
Comment thread packages/google-cloud-bigquery/google/cloud/bigquery/dbapi/connection.py Outdated
chalmerlowe marked this pull request as ready for review July 28, 2026 15:20
chalmerlowe requested review from a team as code owners July 28, 2026 15:20
chalmerlowe requested review from tswast and removed request for a team July 28, 2026 15:20
daniel-sanche approved these changes Jul 28, 2026
View reviewed changes

daniel-sanche left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

chalmerlowe merged commit d328e3b into main Jul 28, 2026
39 checks passed
chalmerlowe deleted the fix/bigquery-dbapi-socket-leak branch July 28, 2026 17:31
This was referenced Jul 28, 2026
Merged
Merged
zhumin8 pushed a commit that referenced this pull request Jul 30, 2026
chore(main): release google-cloud-bigquery 3.42.3 (#17946)
bc38644 
🤖 I have created a release *beep* *boop*
---


##
[3.42.3](google-cloud-bigquery-v3.42.2...google-cloud-bigquery-v3.42.3)
(2026-07-30)


### Bug Fixes

* **bigquery:** dbapi socket leak
([#17921](#17921))
([d328e3b](d328e3b))
* bump grpcio to 1.59.0; require Python 3.10+
([#17351](#17351))
([a53487a](a53487a))
* require Protobuf 6.33.5+
([#17743](#17743))
([d267342](d267342))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

daniel-sanche daniel-sanche approved these changes
tswast Awaiting requested review from tswast tswast is a code owner automatically assigned from googleapis/bigquery-dataframe-team
+1 more reviewer
gemini-code-assist[bot] gemini-code-assist[bot] left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

chalmerlowe

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL