From 48bbce85c88db3dce6b36cf6d760a8342d9a9791 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 17 Feb 2017 17:44:25 -0500 Subject: [PATCH 1/3] Ensure that 'Session' instances are orderable. Some pools store them in priority queues, with a leading timestamp: if two entries have the same timestamp, then the sessions need to be orderable. --- spanner/google/cloud/spanner/session.py | 5 +++++ spanner/unit_tests/test_session.py | 8 ++++++++ 2 files changed, 13 insertions(+) diff --git a/spanner/google/cloud/spanner/session.py b/spanner/google/cloud/spanner/session.py index ecf0995938ef..9e0a6d740dac 100644 --- a/spanner/google/cloud/spanner/session.py +++ b/spanner/google/cloud/spanner/session.py @@ -14,6 +14,7 @@ """Wrapper for Cloud Spanner Session objects.""" +from functools import total_ordering import time from google.gax.errors import GaxError @@ -34,6 +35,7 @@ """Default timeout used by :meth:`Session.run_in_transaction`.""" +@total_ordering class Session(object): """Representation of a Cloud Spanner Session. @@ -53,6 +55,9 @@ class Session(object): def __init__(self, database): self._database = database + def __lt__(self, other): + return self._session_id < other._session_id + @property def session_id(self): """Read-only ID, set by the back-end during :meth:`create`.""" diff --git a/spanner/unit_tests/test_session.py b/spanner/unit_tests/test_session.py index c7257adca15f..937c8293f317 100644 --- a/spanner/unit_tests/test_session.py +++ b/spanner/unit_tests/test_session.py @@ -42,6 +42,14 @@ def test_constructor(self): self.assertTrue(session.session_id is None) self.assertTrue(session._database is database) + def test___lt___(self): + database = _Database(self.DATABASE_NAME) + lhs = self._makeOne(database) + lhs._session_id = b'123' + rhs = self._makeOne(database) + rhs._session_id = b'234' + self.assertTrue(lhs < rhs) + def test_name_property_wo_session_id(self): database = _Database(self.DATABASE_NAME) session = self._make_one(database) From 61ec337d93f13d22edcc7f3a56140588f39a6471 Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Fri, 17 Feb 2017 17:45:38 -0500 Subject: [PATCH 2/3] Ensure that mocked '_Spanner' instances are orderable. Add a test for the two-entries-have-the-same-timestamp race which underlies the Appveyor failure in #3011. Closes #3011. --- spanner/unit_tests/test_pool.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/spanner/unit_tests/test_pool.py b/spanner/unit_tests/test_pool.py index f017fbc84e6b..7812229e2834 100644 --- a/spanner/unit_tests/test_pool.py +++ b/spanner/unit_tests/test_pool.py @@ -13,6 +13,7 @@ # limitations under the License. +from functools import total_ordering import unittest @@ -597,6 +598,32 @@ def test_bind(self): self.assertTrue(pool._pending_sessions.empty()) + def test_bind_w_timestamp_race(self): + import datetime + from google.cloud._testing import _Monkey + from google.cloud.spanner import pool as MUT + NOW = datetime.datetime.utcnow() + pool = self._makeOne() + database = _Database('name') + SESSIONS = [_Session(database) for _ in range(10)] + database._sessions.extend(SESSIONS) + + with _Monkey(MUT, _NOW=lambda: NOW): + pool.bind(database) + + self.assertIs(pool._database, database) + self.assertEqual(pool.size, 10) + self.assertEqual(pool.default_timeout, 10) + self.assertEqual(pool._delta.seconds, 3000) + self.assertTrue(pool._sessions.full()) + + for session in SESSIONS: + self.assertTrue(session._created) + txn = session._transaction + self.assertTrue(txn._begun) + + self.assertTrue(pool._pending_sessions.empty()) + def test_put_full(self): from six.moves.queue import Full @@ -755,6 +782,7 @@ def committed(self): return self._committed +@total_ordering class _Session(object): _transaction = None @@ -767,6 +795,9 @@ def __init__(self, database, exists=True, transaction=None): self._deleted = False self._transaction = transaction + def __lt__(self, other): + return id(self) < id(other) + def create(self): self._created = True From 93b681fc2d06178d9d74b28dc08b491f6dd04bad Mon Sep 17 00:00:00 2001 From: Tres Seaver Date: Sat, 18 Feb 2017 08:30:24 -0500 Subject: [PATCH 3/3] Fix merge conflicts after rebase. --- spanner/unit_tests/test_pool.py | 2 +- spanner/unit_tests/test_session.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/spanner/unit_tests/test_pool.py b/spanner/unit_tests/test_pool.py index 7812229e2834..e4124dcf6b99 100644 --- a/spanner/unit_tests/test_pool.py +++ b/spanner/unit_tests/test_pool.py @@ -603,7 +603,7 @@ def test_bind_w_timestamp_race(self): from google.cloud._testing import _Monkey from google.cloud.spanner import pool as MUT NOW = datetime.datetime.utcnow() - pool = self._makeOne() + pool = self._make_one() database = _Database('name') SESSIONS = [_Session(database) for _ in range(10)] database._sessions.extend(SESSIONS) diff --git a/spanner/unit_tests/test_session.py b/spanner/unit_tests/test_session.py index 937c8293f317..37fad4570e26 100644 --- a/spanner/unit_tests/test_session.py +++ b/spanner/unit_tests/test_session.py @@ -44,9 +44,9 @@ def test_constructor(self): def test___lt___(self): database = _Database(self.DATABASE_NAME) - lhs = self._makeOne(database) + lhs = self._make_one(database) lhs._session_id = b'123' - rhs = self._makeOne(database) + rhs = self._make_one(database) rhs._session_id = b'234' self.assertTrue(lhs < rhs)