Async transaction functions #404
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a test-only implementation of `RetryLogic` interface that retries up to a specified amount of times. It previously had a separate implementation which was basically a simplified version of `ExponentialBackoffRetryLogic`. It is problematic for following `retryAsync` functionality because implementation will not be that simple. This commit makes `FixedRetryLogic` extend `ExponentialBackoffRetryLogic` and override just a single method. So that they can share complicated retrying code.
This commit makes retry logic able to retry work that executes async operations and return a `Future`. It's a first step towards supporting async transaction functions. Also renamed future combinator methods to better represent their intent.
Via `Session#readTransactionAsync()` and `#writeTransactionAsync()`. Both accept a single parameter - async function that takes a transaction and returns a `Response`. When function returns a failed response or throws exception it will be retried. Retries only happen on non-fatal & transient errors. API is not fully complete yet - there is no way to create a `Result` object so it'll be inconvenient to use. Helpers to create some sort of "settable" response will come in subsequent commits.
Given work might fail by throwing exception or by returning a failed future. This commit makes retries gracefully handle exceptions. Previously retrying code would only log an exception and hang.
Transaction should be marked as failed when queries fail. It was previously only marked when PULL_ALL failed. This commit makes RUN handler notify transaction on failure.
ali-ince
approved these changes
Sep 22, 2017
| @@ -240,7 +245,7 @@ public void close() | |||
| { | |||
| if ( asyncConnectionFuture != null ) | |||
| { | |||
| return asyncConnectionFuture.thenCombine( new Function<AsyncConnection,InternalFuture<Void>>() | |||
| return asyncConnectionFuture.thenCompose( new Function<AsyncConnection,InternalFuture<Void>>() | |||
There was a problem hiding this comment.
Can both asyncConnectionFuture and currentAsyncTransactionFuture be not null?
There was a problem hiding this comment.
They can, yeah. I see your point, basically if we close session it will first force-release connection (with RESET message) and then try to close active transaction. Later, however, is not really needed after RESET. I think else if ( currentAsyncTransactionFuture != null ) branch here is not really required. Will think a bit more about this and probably remove it in a separate PR.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added support for transaction functions that return futures. Cleaned up future helper methods.