Skip to content
This repository was archived by the owner on Mar 31, 2026. It is now read-only.

Commit c064815

Browse files
olavloitegcf-owl-bot[bot]
olavloite
and
gcf-owl-bot[bot]
authored
perf: remove repeated GetSession calls for FixedSizePool (#1252)
* test: add mock server tests * chore: move to testing folder + fix formatting * refactor: move mock server tests to separate directory * feat: add database admin service Adds a DatabaseAdminService to the mock server and sets up a basic test case for this. Also removes the generated stubs in the grpc files, as these are not needed. * test: add DDL test * test: add async client tests * chore: remove async + add transaction handling * chore: cleanup * perf: remove repeated GetSession calls for FixedSizePool Add a _last_use_time to Session and use this to determine whether the FixedSizePool should check whether the session still exists, and whether it should be replaced. This significantly reduces the number of times that GetSession is called when using FixedSizePool. * chore: run code formatter * chore: revert to utcnow() * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix: update _last_use_time in trace_call * chore: fix formatting * fix: remove unnecessary update of _last_use_time --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
1 parent a214885 commit c064815

File tree

6 files changed

+55
-11
lines changed

6 files changed

+55
-11
lines changed

‎google/cloud/spanner_v1/_opentelemetry_tracing.py‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
"""Manages OpenTelemetry trace creation and handling"""
1616

1717
from contextlib import contextmanager
18+
from datetime import datetime
1819
import os
1920

2021
from google.cloud.spanner_v1 import SpannerClient
@@ -56,6 +57,9 @@ def get_tracer(tracer_provider=None):
5657

5758
@contextmanager
5859
def trace_call(name, session, extra_attributes=None, observability_options=None):
60+
if session:
61+
session._last_use_time = datetime.now()
62+
5963
if not HAS_OPENTELEMETRY_INSTALLED or not session:
6064
# Empty context manager. Users will have to check if the generated value is None or a span
6165
yield None

‎google/cloud/spanner_v1/pool.py‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,8 @@ class FixedSizePool(AbstractSessionPool):
145145
- Pre-allocates / creates a fixed number of sessions.
146146
147147
- "Pings" existing sessions via :meth:`session.exists` before returning
148-
them, and replaces expired sessions.
148+
sessions that have not been used for more than 55 minutes and replaces
149+
expired sessions.
149150
150151
- Blocks, with a timeout, when :meth:`get` is called on an empty pool.
151152
Raises after timing out.
@@ -171,18 +172,21 @@ class FixedSizePool(AbstractSessionPool):
171172

172173
DEFAULT_SIZE = 10
173174
DEFAULT_TIMEOUT = 10
175+
DEFAULT_MAX_AGE_MINUTES = 55
174176

175177
def __init__(
176178
self,
177179
size=DEFAULT_SIZE,
178180
default_timeout=DEFAULT_TIMEOUT,
179181
labels=None,
180182
database_role=None,
183+
max_age_minutes=DEFAULT_MAX_AGE_MINUTES,
181184
):
182185
super(FixedSizePool, self).__init__(labels=labels, database_role=database_role)
183186
self.size = size
184187
self.default_timeout = default_timeout
185188
self._sessions = queue.LifoQueue(size)
189+
self._max_age = datetime.timedelta(minutes=max_age_minutes)
186190

187191
def bind(self, database):
188192
"""Associate the pool with a database.
@@ -230,8 +234,9 @@ def get(self, timeout=None):
230234
timeout = self.default_timeout
231235

232236
session = self._sessions.get(block=True, timeout=timeout)
237+
age = _NOW() - session.last_use_time
233238

234-
if not session.exists():
239+
if age >= self._max_age and not session.exists():
235240
session = self._database.session()
236241
session.create()
237242

‎google/cloud/spanner_v1/session.py‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from functools import total_ordering
1818
import random
1919
import time
20+
from datetime import datetime
2021

2122
from google.api_core.exceptions import Aborted
2223
from google.api_core.exceptions import GoogleAPICallError
@@ -69,6 +70,7 @@ def __init__(self, database, labels=None, database_role=None):
6970
labels = {}
7071
self._labels = labels
7172
self._database_role = database_role
73+
self._last_use_time = datetime.utcnow()
7274

7375
def __lt__(self, other):
7476
return self._session_id < other._session_id
@@ -78,6 +80,14 @@ def session_id(self):
7880
"""Read-only ID, set by the back-end during :meth:`create`."""
7981
return self._session_id
8082

83+
@property
84+
def last_use_time(self):
85+
""" "Approximate last use time of this session
86+
87+
:rtype: datetime
88+
:returns: the approximate last use time of this session"""
89+
return self._last_use_time
90+
8191
@property
8292
def database_role(self):
8393
"""User-assigned database-role for the session.
@@ -222,6 +232,7 @@ def ping(self):
222232
metadata = _metadata_with_prefix(self._database.name)
223233
request = ExecuteSqlRequest(session=self.name, sql="SELECT 1")
224234
api.execute_sql(request=request, metadata=metadata)
235+
self._last_use_time = datetime.now()
225236

226237
def snapshot(self, **kw):
227238
"""Create a snapshot to perform a set of reads with shared staleness.

‎google/cloud/spanner_v1/snapshot.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
"""Model a set of read-only queries to a database as a snapshot."""
1616

17+
from datetime import datetime
1718
import functools
1819
import threading
1920
from google.protobuf.struct_pb2 import Struct
@@ -364,6 +365,7 @@ def read(
364365
)
365366

366367
self._read_request_count += 1
368+
self._session._last_use_time = datetime.now()
367369

368370
if self._multi_use:
369371
return StreamedResultSet(

‎tests/mockserver_tests/test_basics.py‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
FixedSizePool,
3030
BatchCreateSessionsRequest,
3131
ExecuteSqlRequest,
32-
GetSessionRequest,
3332
)
3433
from google.cloud.spanner_v1.database import Database
3534
from google.cloud.spanner_v1.instance import Instance
@@ -125,12 +124,9 @@ def test_select1(self):
125124
self.assertEqual(1, row[0])
126125
self.assertEqual(1, len(result_list))
127126
requests = self.spanner_service.requests
128-
self.assertEqual(3, len(requests))
127+
self.assertEqual(2, len(requests), msg=requests)
129128
self.assertTrue(isinstance(requests[0], BatchCreateSessionsRequest))
130-
# TODO: Optimize FixedSizePool so this GetSessionRequest is not executed
131-
# every time a session is fetched.
132-
self.assertTrue(isinstance(requests[1], GetSessionRequest))
133-
self.assertTrue(isinstance(requests[2], ExecuteSqlRequest))
129+
self.assertTrue(isinstance(requests[1], ExecuteSqlRequest))
134130

135131
def test_create_table(self):
136132
database_admin_api = self.client.database_admin_api

‎tests/unit/test_pool.py‎

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
from functools import total_ordering
1717
import unittest
18+
from datetime import datetime, timedelta
1819

1920
import mock
2021

@@ -184,13 +185,30 @@ def test_bind(self):
184185
for session in SESSIONS:
185186
session.create.assert_not_called()
186187

187-
def test_get_non_expired(self):
188+
def test_get_active(self):
188189
pool = self._make_one(size=4)
189190
database = _Database("name")
190191
SESSIONS = sorted([_Session(database) for i in range(0, 4)])
191192
database._sessions.extend(SESSIONS)
192193
pool.bind(database)
193194

195+
# check if sessions returned in LIFO order
196+
for i in (3, 2, 1, 0):
197+
session = pool.get()
198+
self.assertIs(session, SESSIONS[i])
199+
self.assertFalse(session._exists_checked)
200+
self.assertFalse(pool._sessions.full())
201+
202+
def test_get_non_expired(self):
203+
pool = self._make_one(size=4)
204+
database = _Database("name")
205+
last_use_time = datetime.utcnow() - timedelta(minutes=56)
206+
SESSIONS = sorted(
207+
[_Session(database, last_use_time=last_use_time) for i in range(0, 4)]
208+
)
209+
database._sessions.extend(SESSIONS)
210+
pool.bind(database)
211+
194212
# check if sessions returned in LIFO order
195213
for i in (3, 2, 1, 0):
196214
session = pool.get()
@@ -201,7 +219,8 @@ def test_get_non_expired(self):
201219
def test_get_expired(self):
202220
pool = self._make_one(size=4)
203221
database = _Database("name")
204-
SESSIONS = [_Session(database)] * 5
222+
last_use_time = datetime.utcnow() - timedelta(minutes=65)
223+
SESSIONS = [_Session(database, last_use_time=last_use_time)] * 5
205224
SESSIONS[0]._exists = False
206225
database._sessions.extend(SESSIONS)
207226
pool.bind(database)
@@ -915,18 +934,25 @@ def _make_transaction(*args, **kw):
915934
class _Session(object):
916935
_transaction = None
917936

918-
def __init__(self, database, exists=True, transaction=None):
937+
def __init__(
938+
self, database, exists=True, transaction=None, last_use_time=datetime.utcnow()
939+
):
919940
self._database = database
920941
self._exists = exists
921942
self._exists_checked = False
922943
self._pinged = False
923944
self.create = mock.Mock()
924945
self._deleted = False
925946
self._transaction = transaction
947+
self._last_use_time = last_use_time
926948

927949
def __lt__(self, other):
928950
return id(self) < id(other)
929951

952+
@property
953+
def last_use_time(self):
954+
return self._last_use_time
955+
930956
def exists(self):
931957
self._exists_checked = True
932958
return self._exists

0 commit comments

Comments
 (0)