From 20d5280af2b3575375f85034f479f2a6875eaa7f Mon Sep 17 00:00:00 2001 From: Zhen Date: Mon, 24 Apr 2017 15:02:12 +0200 Subject: [PATCH 1/3] Support transient but not terminated by user errors in retry --- neo4j/v1/api.py | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/neo4j/v1/api.py b/neo4j/v1/api.py index 306c534ab..a94744dc5 100644 --- a/neo4j/v1/api.py +++ b/neo4j/v1/api.py @@ -27,14 +27,12 @@ from neo4j.bolt import ProtocolError, ServiceUnavailable from neo4j.compat import urlparse -from neo4j.exceptions import CypherError +from neo4j.exceptions import CypherError, TransientError from .exceptions import DriverError, SessionError, SessionExpired, TransactionError - _warned_about_transaction_bookmarks = False - READ_ACCESS = "READ" WRITE_ACCESS = "WRITE" @@ -53,7 +51,6 @@ def retry_delay_generator(initial_delay, multiplier, jitter_factor): class ValueSystem(object): - def hydrate(self, values): """ Hydrate values from raw representations into client objects. """ @@ -435,6 +432,11 @@ def _run_transaction(self, access_mode, unit_of_work, *args, **kwargs): return unit_of_work(tx, *args, **kwargs) except (ServiceUnavailable, SessionExpired) as error: last_error = error + except TransientError as error: + if is_retriable_transientError(error): + last_error = error + else: + raise error sleep(next(retry_delay)) t1 = clock() raise last_error @@ -461,6 +463,17 @@ def __bookmark__(self, result): pass +def is_retriable_transientError(error): + """ + :type error: TransientError + """ + if (error.code != "Neo.TransientError.Transaction.Terminated" + and error.code != "Neo.TransientError.Transaction.LockClientStopped"): + return True + else: + return False + + class Transaction(object): """ Container for multiple Cypher queries to be executed within a single context. Transactions can be used within a :py:const:`with` From 0845925981d1a4d78077bf9d2e68cb309b3437e7 Mon Sep 17 00:00:00 2001 From: Zhen Date: Mon, 24 Apr 2017 16:12:44 +0200 Subject: [PATCH 2/3] Adding tests to ensure transient error caused by user canceling execution is not retried --- .../scripts/user_canceled_tx.script.script | 20 ++++++++++++++++ test/stub/test_accesslevel.py | 24 ++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 test/stub/scripts/user_canceled_tx.script.script diff --git a/test/stub/scripts/user_canceled_tx.script.script b/test/stub/scripts/user_canceled_tx.script.script new file mode 100644 index 000000000..a3280a3e0 --- /dev/null +++ b/test/stub/scripts/user_canceled_tx.script.script @@ -0,0 +1,20 @@ +!: AUTO INIT +!: AUTO RESET + +C: RUN "BEGIN" {} + PULL_ALL +S: SUCCESS {"fields": []} + SUCCESS {} + +C: RUN "RETURN 1" {} + PULL_ALL +S: FAILURE {"code": "Neo.TransientError.Transaction.LockClientStopped", "message": "X"} + IGNORED {} + +C: ACK_FAILURE +S: SUCCESS {} + +C: RUN "ROLLBACK" {} + PULL_ALL +S: SUCCESS {} + SUCCESS {} \ No newline at end of file diff --git a/test/stub/test_accesslevel.py b/test/stub/test_accesslevel.py index 6188c1815..742dd4510 100644 --- a/test/stub/test_accesslevel.py +++ b/test/stub/test_accesslevel.py @@ -19,7 +19,7 @@ # limitations under the License. -from neo4j.v1 import GraphDatabase, CypherError +from neo4j.v1 import GraphDatabase, CypherError, TransientError from test.stub.tools import StubTestCase, StubCluster @@ -159,3 +159,25 @@ def unit_of_work_2(tx): assert value == 1 value = session.read_transaction(unit_of_work_2) assert value == 2 + + def test_no_retry_read_on_user_canceled_tx(self): + with StubCluster({9001: "router.script", 9004: "user_canceled_tx.script.script"}): + uri = "bolt+routing://127.0.0.1:9001" + with GraphDatabase.driver(uri, auth=self.auth_token, encrypted=False) as driver: + with driver.session() as session: + def unit_of_work(tx): + tx.run("RETURN 1") + + with self.assertRaises(TransientError): + _ = session.read_transaction(unit_of_work) + + def test_no_retry_write_on_user_canceled_tx(self): + with StubCluster({9001: "router.script", 9006: "user_canceled_tx.script.script"}): + uri = "bolt+routing://127.0.0.1:9001" + with GraphDatabase.driver(uri, auth=self.auth_token, encrypted=False) as driver: + with driver.session() as session: + def unit_of_work(tx): + tx.run("RETURN 1") + + with self.assertRaises(TransientError): + _ = session.write_transaction(unit_of_work) From f960b4103a587e67f95a5fa09b18160c0f044006 Mon Sep 17 00:00:00 2001 From: Zhen Date: Wed, 26 Apr 2017 18:13:17 +0200 Subject: [PATCH 3/3] Refine the code after review --- neo4j/v1/api.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/neo4j/v1/api.py b/neo4j/v1/api.py index a94744dc5..b90453625 100644 --- a/neo4j/v1/api.py +++ b/neo4j/v1/api.py @@ -467,11 +467,8 @@ def is_retriable_transientError(error): """ :type error: TransientError """ - if (error.code != "Neo.TransientError.Transaction.Terminated" - and error.code != "Neo.TransientError.Transaction.LockClientStopped"): - return True - else: - return False + return not (error.code in ("Neo.TransientError.Transaction.Terminated", + "Neo.TransientError.Transaction.LockClientStopped")) class Transaction(object):