From 0b594680cc4799f2a3946ccce0776df7149afbf3 Mon Sep 17 00:00:00 2001 From: Luke Sneeringer Date: Thu, 10 Aug 2017 13:19:38 -0700 Subject: [PATCH 1/3] Disallow nested transactions. --- spanner/google/cloud/spanner/database.py | 17 ++++++++++++++-- spanner/tests/unit/test_database.py | 25 +++++++++++++++++++++++- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/spanner/google/cloud/spanner/database.py b/spanner/google/cloud/spanner/database.py index b098f7684b7c..87a08ffdb234 100644 --- a/spanner/google/cloud/spanner/database.py +++ b/spanner/google/cloud/spanner/database.py @@ -79,6 +79,7 @@ def __init__(self, database_id, instance, ddl_statements=(), pool=None): self.database_id = database_id self._instance = instance self._ddl_statements = _check_ddl_statements(ddl_statements) + self._transaction_running = False if pool is None: pool = BurstyPool() @@ -332,8 +333,20 @@ def run_in_transaction(self, func, *args, **kw): :rtype: :class:`datetime.datetime` :returns: timestamp of committed transaction """ - with SessionCheckout(self._pool) as session: - return session.run_in_transaction(func, *args, **kw) + # Sanity check: Is there a transaction already running? + # If there is, then raise a red flag. Otherwise, mark that this one + # is running. + if self._transaction_running: + raise RuntimeError('Spanner does not support nested transactions.') + self._transaction_running = True + + # Check out a session and run the function in a transaction; once + # done, flip the sanity check bit back. + try: + with SessionCheckout(self._pool) as session: + return session.run_in_transaction(func, *args, **kw) + finally: + self._transaction_running = False def batch(self): """Return an object which wraps a batch. diff --git a/spanner/tests/unit/test_database.py b/spanner/tests/unit/test_database.py index c1218599b3b3..71281fa0687e 100644 --- a/spanner/tests/unit/test_database.py +++ b/spanner/tests/unit/test_database.py @@ -663,6 +663,25 @@ def test_run_in_transaction_w_args(self): self.assertEqual(session._retried, (_unit_of_work, (SINCE,), {'until': UNTIL})) + def test_run_in_transaction_nested(self): + from datetime import datetime + + # Perform the various setup tasks. + instance = _Instance(self.INSTANCE_NAME, client=_Client()) + pool = _Pool() + session = _Session(run_transaction_function=True) + session._committed = datetime.now() + pool.put(session) + database = self._make_one(self.DATABASE_ID, instance, pool=pool) + + # Define the nested transaction. + def nested_unit_of_work(): + return database.run_in_transaction(lambda: None) # pragma: NO COVER + + # Attempting to run this transaction should raise RuntimeError. + with self.assertRaises(RuntimeError): + database.run_in_transaction(nested_unit_of_work) + def test_batch(self): from google.cloud.spanner.database import BatchCheckout @@ -900,11 +919,15 @@ class _Session(object): _rows = () - def __init__(self, database=None, name=_BaseTest.SESSION_NAME): + def __init__(self, database=None, name=_BaseTest.SESSION_NAME, + run_transaction_function=False): self._database = database self.name = name + self._run_transaction_function = run_transaction_function def run_in_transaction(self, func, *args, **kw): + if self._run_transaction_function: + func(*args, **kw) self._retried = (func, args, kw) return self._committed From 88c61349d74a90fbde4e7ba55da4a118d16408b5 Mon Sep 17 00:00:00 2001 From: Luke Sneeringer Date: Thu, 10 Aug 2017 13:23:11 -0700 Subject: [PATCH 2/3] Make the test a little better. --- spanner/tests/unit/test_database.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/spanner/tests/unit/test_database.py b/spanner/tests/unit/test_database.py index 71281fa0687e..c812176499dd 100644 --- a/spanner/tests/unit/test_database.py +++ b/spanner/tests/unit/test_database.py @@ -223,7 +223,7 @@ def __init__(self, scopes=(), source=None): self._scopes = scopes self._source = source - def requires_scopes(self): # pragma: NO COVER + def requires_scopes(self): # pragma: NO COVER return True def with_scopes(self, scopes): @@ -674,13 +674,17 @@ def test_run_in_transaction_nested(self): pool.put(session) database = self._make_one(self.DATABASE_ID, instance, pool=pool) + # Define the inner function. + inner = mock.Mock(spec=()) + # Define the nested transaction. def nested_unit_of_work(): - return database.run_in_transaction(lambda: None) # pragma: NO COVER + return database.run_in_transaction(inner) # Attempting to run this transaction should raise RuntimeError. with self.assertRaises(RuntimeError): database.run_in_transaction(nested_unit_of_work) + self.assertEqual(inner.call_count, 0) def test_batch(self): from google.cloud.spanner.database import BatchCheckout @@ -920,7 +924,7 @@ class _Session(object): _rows = () def __init__(self, database=None, name=_BaseTest.SESSION_NAME, - run_transaction_function=False): + run_transaction_function=False): self._database = database self.name = name self._run_transaction_function = run_transaction_function From 89da2f1e5ec968e555ffa9fd527dc17a8c4fcd12 Mon Sep 17 00:00:00 2001 From: Luke Sneeringer Date: Thu, 10 Aug 2017 14:28:17 -0700 Subject: [PATCH 3/3] Make the transaction running test thread-local. --- spanner/google/cloud/spanner/database.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/spanner/google/cloud/spanner/database.py b/spanner/google/cloud/spanner/database.py index 87a08ffdb234..40dcc471d1c4 100644 --- a/spanner/google/cloud/spanner/database.py +++ b/spanner/google/cloud/spanner/database.py @@ -15,6 +15,7 @@ """User friendly container for Cloud Spanner Database.""" import re +import threading import google.auth.credentials from google.gax.errors import GaxError @@ -79,7 +80,7 @@ def __init__(self, database_id, instance, ddl_statements=(), pool=None): self.database_id = database_id self._instance = instance self._ddl_statements = _check_ddl_statements(ddl_statements) - self._transaction_running = False + self._local = threading.local() if pool is None: pool = BurstyPool() @@ -336,9 +337,9 @@ def run_in_transaction(self, func, *args, **kw): # Sanity check: Is there a transaction already running? # If there is, then raise a red flag. Otherwise, mark that this one # is running. - if self._transaction_running: + if getattr(self._local, 'transaction_running', False): raise RuntimeError('Spanner does not support nested transactions.') - self._transaction_running = True + self._local.transaction_running = True # Check out a session and run the function in a transaction; once # done, flip the sanity check bit back. @@ -346,7 +347,7 @@ def run_in_transaction(self, func, *args, **kw): with SessionCheckout(self._pool) as session: return session.run_in_transaction(func, *args, **kw) finally: - self._transaction_running = False + self._local.transaction_running = False def batch(self): """Return an object which wraps a batch.