Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent nested transactions. #3789

Merged
merged 3 commits into from
Aug 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions spanner/google/cloud/spanner/database.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -79,6 +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._local = threading.local()

if pool is None:
pool = BurstyPool()
Expand Down Expand Up @@ -332,8 +334,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 getattr(self._local, 'transaction_running', False):
raise RuntimeError('Spanner does not support nested transactions.')
self._local.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._local.transaction_running = False

def batch(self):
"""Return an object which wraps a batch.
Expand Down
31 changes: 29 additions & 2 deletions spanner/tests/unit/test_database.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -663,6 +663,29 @@ 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 inner function.
inner = mock.Mock(spec=())

# Define the nested transaction.
def nested_unit_of_work():
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

Expand Down Expand Up @@ -900,11 +923,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

Expand Down