Skip to content

Commit d328e3b

Browse files
authored
fix(bigquery): dbapi socket leak (#17921)
## 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) ```
1 parent ff58ec9 commit d328e3b

4 files changed

Lines changed: 73 additions & 38 deletions

File tree

packages/google-cloud-bigquery/google/cloud/bigquery/dbapi/connection.py

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,7 @@
1717
import weakref
1818

1919
from google.cloud import bigquery
20-
from google.cloud.bigquery.dbapi import cursor
21-
from google.cloud.bigquery.dbapi import _helpers
20+
from google.cloud.bigquery.dbapi import _helpers, cursor
2221

2322

2423
@_helpers.raise_on_closed("Operating on a closed connection.")
@@ -84,7 +83,17 @@ def close(self):
8483

8584
if self._owns_bqstorage_client:
8685
# There is no close() on the BQ Storage client itself.
87-
self._bqstorage_client._transport.close()
86+
transport = self._bqstorage_client.transport
87+
transport.close()
88+
89+
# Ensure the underlying gRPC channel is closed explicitly.
90+
# This is important because the transport might be wrapped by interceptors
91+
# which may not propagate the close call to the underlying channel immediately,
92+
# or Garbage Collection delays might keep sockets open longer than expected,
93+
# leading to intermittent socket leaks in tests.
94+
channel = getattr(transport, "_grpc_channel", None)
95+
if channel is not None:
96+
channel.close()
8897

8998
for cursor_ in self._cursors_created:
9099
if not cursor_._closed:

packages/google-cloud-bigquery/google/cloud/bigquery/dbapi/cursor.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
from __future__ import annotations
1818

1919
import collections
20-
from collections import abc as collections_abc
2120
import re
21+
from collections import abc as collections_abc
2222
from typing import Optional
2323

2424
try:
@@ -29,11 +29,9 @@
2929
# Having BQ Storage available implies that pyarrow >=1.0.0 is available, too.
3030
_ARROW_COMPRESSION_SUPPORT = True
3131

32-
from google.cloud.bigquery import job
33-
from google.cloud.bigquery.dbapi import _helpers
34-
from google.cloud.bigquery.dbapi import exceptions
3532
import google.cloud.exceptions # type: ignore
36-
33+
from google.cloud.bigquery import job
34+
from google.cloud.bigquery.dbapi import _helpers, exceptions
3735

3836
# Per PEP 249: A 7-item sequence containing information describing one result
3937
# column. The first two items (name and type_code) are mandatory, the other
@@ -105,6 +103,15 @@ def close(self):
105103
"""Mark the cursor as closed, preventing its further use."""
106104
self._closed = True
107105

106+
# Explicitly release references to query data and rows.
107+
# This is important because these objects may hold references to underlying
108+
# transports or gRPC streams (especially when using BQ Storage API).
109+
# Clearing them ensures that resources can be garbage collected deterministically,
110+
# preventing intermittent socket leaks (e.g., ESTABLISHED connections) in tests
111+
# and long-running applications.
112+
self._query_data = None
113+
self._query_rows = None
114+
108115
def _set_description(self, schema):
109116
"""Set description from schema.
110117

packages/google-cloud-bigquery/tests/unit/test_dbapi_connection.py

Lines changed: 30 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,11 @@
1313
# limitations under the License.
1414

1515
import gc
16-
import pytest
1716
import unittest
1817
from unittest import mock
1918

19+
import pytest
20+
2021

2122
class TestConnection(unittest.TestCase):
2223
@staticmethod
@@ -40,8 +41,8 @@ def _mock_bqstorage_client(self):
4041
from google.cloud import bigquery_storage
4142

4243
mock_client = mock.create_autospec(bigquery_storage.BigQueryReadClient)
43-
mock_client._transport = mock.Mock(spec=["channel", "close"])
44-
mock_client._transport.grpc_channel = mock.Mock(spec=["close"])
44+
mock_client.transport.close = mock.Mock()
45+
mock_client.transport._grpc_channel = mock.Mock(spec=["close"])
4546
return mock_client
4647

4748
def test_ctor_wo_bqstorage_client(self):
@@ -77,8 +78,7 @@ def test_ctor_w_bqstorage_client(self):
7778

7879
@mock.patch("google.cloud.bigquery.Client", autospec=True)
7980
def test_connect_wo_client(self, mock_client):
80-
from google.cloud.bigquery.dbapi import connect
81-
from google.cloud.bigquery.dbapi import Connection
81+
from google.cloud.bigquery.dbapi import Connection, connect
8282

8383
connection = connect()
8484
self.assertIsInstance(connection, Connection)
@@ -87,8 +87,7 @@ def test_connect_wo_client(self, mock_client):
8787

8888
def test_connect_w_client(self):
8989
pytest.importorskip("google.cloud.bigquery_storage")
90-
from google.cloud.bigquery.dbapi import connect
91-
from google.cloud.bigquery.dbapi import Connection
90+
from google.cloud.bigquery.dbapi import Connection, connect
9291

9392
mock_client = self._mock_client()
9493
mock_bqstorage_client = self._mock_bqstorage_client()
@@ -103,8 +102,7 @@ def test_connect_w_client(self):
103102

104103
def test_connect_w_both_clients(self):
105104
pytest.importorskip("google.cloud.bigquery_storage")
106-
from google.cloud.bigquery.dbapi import connect
107-
from google.cloud.bigquery.dbapi import Connection
105+
from google.cloud.bigquery.dbapi import Connection, connect
108106

109107
mock_client = self._mock_client()
110108
mock_bqstorage_client = self._mock_bqstorage_client()
@@ -124,8 +122,7 @@ def test_connect_w_both_clients(self):
124122

125123
def test_connect_prefer_bqstorage_client_false(self):
126124
pytest.importorskip("google.cloud.bigquery_storage")
127-
from google.cloud.bigquery.dbapi import connect
128-
from google.cloud.bigquery.dbapi import Connection
125+
from google.cloud.bigquery.dbapi import Connection, connect
129126

130127
mock_client = self._mock_client()
131128
mock_bqstorage_client = self._mock_bqstorage_client()
@@ -155,11 +152,16 @@ def test_raises_error_if_closed(self):
155152
):
156153
getattr(connection, method)()
157154

158-
def test_close_closes_all_created_bigquery_clients(self):
155+
def _verify_close_closes_all_created_bigquery_clients(
156+
self, simulate_no_grpc_channel=False
157+
):
159158
pytest.importorskip("google.cloud.bigquery_storage")
160159
client = self._mock_client()
161160
bqstorage_client = self._mock_bqstorage_client()
162161

162+
if simulate_no_grpc_channel:
163+
bqstorage_client.transport._grpc_channel = None
164+
163165
client_patcher = mock.patch(
164166
"google.cloud.bigquery.dbapi.connection.bigquery.Client",
165167
return_value=client,
@@ -176,7 +178,20 @@ def test_close_closes_all_created_bigquery_clients(self):
176178
connection.close()
177179

178180
self.assertTrue(client.close.called)
179-
self.assertTrue(bqstorage_client._transport.close.called)
181+
self.assertTrue(bqstorage_client.transport.close.called)
182+
183+
if not simulate_no_grpc_channel:
184+
self.assertTrue(bqstorage_client.transport._grpc_channel.close.called)
185+
186+
def test_close_closes_all_created_bigquery_clients(self):
187+
self._verify_close_closes_all_created_bigquery_clients(
188+
simulate_no_grpc_channel=False
189+
)
190+
191+
def test_close_closes_all_created_bigquery_clients_no_grpc_channel(self):
192+
self._verify_close_closes_all_created_bigquery_clients(
193+
simulate_no_grpc_channel=True
194+
)
180195

181196
def test_close_does_not_close_bigquery_clients_passed_to_it(self):
182197
pytest.importorskip("google.cloud.bigquery_storage")
@@ -187,7 +202,8 @@ def test_close_does_not_close_bigquery_clients_passed_to_it(self):
187202
connection.close()
188203

189204
self.assertFalse(client.close.called)
190-
self.assertFalse(bqstorage_client._transport.close.called)
205+
self.assertFalse(bqstorage_client.transport.close.called)
206+
self.assertFalse(bqstorage_client.transport._grpc_channel.close.called)
191207

192208
def test_close_closes_all_created_cursors(self):
193209
connection = self._make_one(client=self._mock_client())

packages/google-cloud-bigquery/tests/unit/test_dbapi_cursor.py

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,8 @@
1717
import unittest
1818
from unittest import mock
1919

20-
import pytest
21-
2220
import google.cloud.bigquery.table as bq_table
23-
21+
import pytest
2422
from google.api_core import exceptions
2523

2624
from tests.unit.helpers import _to_pyarrow
@@ -180,8 +178,7 @@ def _mock_results(self, total_rows=0, schema=None, num_dml_affected_rows=None):
180178
return mock_results
181179

182180
def test_ctor(self):
183-
from google.cloud.bigquery.dbapi import connect
184-
from google.cloud.bigquery.dbapi import Cursor
181+
from google.cloud.bigquery.dbapi import Cursor, connect
185182

186183
connection = connect(self._mock_client())
187184
cursor = self._make_one(connection)
@@ -193,9 +190,17 @@ def test_close(self):
193190

194191
connection = connect(self._mock_client())
195192
cursor = connection.cursor()
196-
# close() is a no-op, there is nothing to test.
193+
194+
# Simulate that cursor has data/references
195+
cursor._query_data = mock.Mock()
196+
cursor._query_rows = mock.Mock()
197+
197198
cursor.close()
198199

200+
self.assertTrue(cursor._closed)
201+
self.assertIsNone(cursor._query_data)
202+
self.assertIsNone(cursor._query_rows)
203+
199204
def test_raises_error_if_closed(self):
200205
from google.cloud.bigquery.dbapi import connect
201206
from google.cloud.bigquery.dbapi.exceptions import ProgrammingError
@@ -519,8 +524,8 @@ def test_execute_w_default_config(self):
519524
self.assertIsNone(used_config)
520525

521526
def test_execute_custom_job_config_wo_default_config(self):
522-
from google.cloud.bigquery.dbapi import connect
523527
from google.cloud.bigquery import job
528+
from google.cloud.bigquery.dbapi import connect
524529

525530
config = job.QueryJobConfig(use_legacy_sql=True)
526531
client = self._mock_client(rows=[], num_dml_affected_rows=0)
@@ -533,8 +538,8 @@ def test_execute_custom_job_config_wo_default_config(self):
533538
self.assertEqual(kwargs["job_config"], config)
534539

535540
def test_execute_custom_job_config_w_default_config(self):
536-
from google.cloud.bigquery.dbapi import connect
537541
from google.cloud.bigquery import job
542+
from google.cloud.bigquery.dbapi import connect
538543

539544
client = self._mock_client(rows=[], num_dml_affected_rows=0)
540545
connection = connect(client)
@@ -563,8 +568,8 @@ def test_execute_w_dml(self):
563568
self.assertEqual(rows, [])
564569

565570
def test_execute_w_query(self):
566-
from google.cloud.bigquery.schema import SchemaField
567571
from google.cloud.bigquery import dbapi
572+
from google.cloud.bigquery.schema import SchemaField
568573

569574
connection = dbapi.connect(
570575
self._mock_client(
@@ -607,9 +612,9 @@ def test_execute_w_query(self):
607612
self.assertIsNone(row)
608613

609614
def test_execute_w_query_dry_run(self):
615+
from google.cloud.bigquery import dbapi
610616
from google.cloud.bigquery.job import QueryJobConfig
611617
from google.cloud.bigquery.schema import SchemaField
612-
from google.cloud.bigquery import dbapi
613618

614619
connection = dbapi.connect(
615620
self._mock_client(
@@ -637,10 +642,8 @@ def test_execute_w_query_dry_run(self):
637642

638643
def test_execute_raises_if_result_raises(self):
639644
import google.cloud.exceptions
640-
641645
from google.cloud.bigquery import client
642-
from google.cloud.bigquery.dbapi import connect
643-
from google.cloud.bigquery.dbapi import exceptions
646+
from google.cloud.bigquery.dbapi import connect, exceptions
644647

645648
client = mock.create_autospec(client.Client)
646649
client.query_and_wait.side_effect = google.cloud.exceptions.GoogleCloudError("")
@@ -702,7 +705,7 @@ def test_query_job_wo_execute(self):
702705
self.assertIsNone(cursor.query_job)
703706

704707
def test_query_job_w_execute(self):
705-
from google.cloud.bigquery import dbapi, QueryJob
708+
from google.cloud.bigquery import QueryJob, dbapi
706709

707710
connection = dbapi.connect(self._mock_client())
708711
cursor = connection.cursor()
@@ -722,7 +725,7 @@ def test_query_job_w_execute_no_job(self):
722725
self.assertIsNone(cursor.query_job)
723726

724727
def test_query_job_w_executemany(self):
725-
from google.cloud.bigquery import dbapi, QueryJob
728+
from google.cloud.bigquery import QueryJob, dbapi
726729

727730
connection = dbapi.connect(self._mock_client())
728731
cursor = connection.cursor()
@@ -932,8 +935,8 @@ def test__extract_types(inp, expect):
932935
],
933936
)
934937
def test__extract_types_fail(match, inp):
935-
from google.cloud.bigquery.dbapi.cursor import _extract_types as et
936938
from google.cloud.bigquery.dbapi import exceptions
939+
from google.cloud.bigquery.dbapi.cursor import _extract_types as et
937940

938941
with pytest.raises(exceptions.ProgrammingError, match=match):
939942
et(inp)

0 commit comments

Comments
 (0)