API updates #325
Merged
API updates #325
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 commit adds new API methods to create session with the specified bookmark. This means first transaction will ensure that the server, it is connected to, is at least up to the database version denoted by the bookmark. Following new methods are introduced in the `Driver` interface: * `#session(String bookmark)` * `#session(AccessMode mode, String bookmark)` Bookmark is a session related concept and this commit adds symmetry with regards to `Session#lastBookmark()` method.
In favour of `Driver#session(String bookmark)`. This is made to bring symmetry with regards to `Session#lastBookmark()` and emphasize that bookmark is a session level concept.
Now it uses `Driver#session(AccessMode mode, String bookmark)` method and does not use deprecated `Session#beginTransaction(String bookmark)`.
Commit adds two new methods to the `Session` interface to allow declarative transactions: * `#readTransaction(Function<Transaction, T> work)` * `#writeTransaction(Function<Transaction, T> work)` These methods currently only hide transaction creation and closing. They return whatever the given function returns. Main reason for adding them is that later we can hide retries behind such declarative api, which is especially useful for "bolt+routing" driver which talks to a causal cluster.
Added ability to retry transactions executed via `Session#readTransaction()` and `Session#writeTransaction()` methods. Given unit of work will be retried configured number of times with configured delay if it experiences `ServiceUnavailableException` or `SessionExpiredException`. This means it'll be retried when a network error happens of cluster member becomes unavailable.
This commit makes routing table use same ordering of read, write and route servers as returned by the `getServers()` procedure call. Database uses lists to represent these servers and randomly shuffles them before returning, so it makes sense using the provided randomized view. Is also makes boltkit tests more predictable and easy to reason about.
`RetryWithDelayTest#throwsWhenSleepInterrupted()` test mocks throwing of `InterruptedException` and triggers handling code to restore the interrupted status. This caused some subsequent tests to fail because they still saw current thread as being interrupted. This commit adds clearing of the interrupted status to the test.
They were failing because of incorrect ports.
This commit changes default retry policy from fixed backoff to exponential backoff with random jitter. Retry policy is user when retrying transactions executed via `Session#readTransaction()` and `Session#writeTransaction()` methods. Exponential backoff with random jitter is a better policy because it spreads out clusters of retries and progressively reduces load on the database. The only configuration option available is: `ConfigBuilder#withMaxTransactionRetryTime()` which sets a cap on how long particular transaction can be retried with exponential backoff.
zhenlineo
reviewed
Mar 1, 2017
| } | ||
|
|
||
| @Override | ||
| public synchronized Transaction beginTransaction( String bookmark ) | ||
| { | ||
| ensureSessionIsOpen(); | ||
| ensureNoOpenTransactionBeforeOpeningTransaction(); | ||
| setLastBookmark( bookmark ); |
There was a problem hiding this comment.
Why we disallow a user to set null bookmark? (It does not make many sense, but maybe a user wants to break CC from this tx?)
There was a problem hiding this comment.
Good point, I just tried to use same #setLastBookmark() method everywhere and it prohibits nulls. Will fix.
zhenlineo
reviewed
Mar 1, 2017
|
|
||
| if ( decision.shouldRetry() ) | ||
| { | ||
| errors = recordError( newError, errors ); |
There was a problem hiding this comment.
Can we just directly chain the errors? why we have to first remember them (in recordError) and then build the error chain (in addSuppressed)?
There was a problem hiding this comment.
Printed stacktrace of a chain of suppressed methods looks quite strange, every next suppressed trace is shifted to the right. With this approach all previous errors will be on the same level :)
|
Looks good, minor questions. The retry logic impl is a bit scaring |
|
Thanks @zhenlineo! I agree about retry logic, will try to simplify it in subsequent PRs. There is no point in having separate |
So it is possible to "break" causal consistency within a session by calling `#beginTransaction(null)`.
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.
This PR adds following new API methods:
Driver#session(String bookmark)to allow specifying initial bookmark for the session which will be used by the first transaction. This method is mainly added to bring symmetry with regards toSession#lastBookmark()and to emphasize that bookmark is a session concept.Driver#session(AccessMode mode, String bookmark)to allow specifying both initial bookmark and defaultAccessModeper session.Session#readTransaction(Function<Transaction, T> work)to allow declarative read transactions with exponential backoff retry policy.Session#writeTransaction(Function<Transaction, T> work)to allow declarative write transactions with exponential backoff retry policy.ConfigBuilder#withMaxTransactionRetryTime(int value, TimeUnit unit)to configure the retry policy for#readTransaction()and#writeTransaction().It also deprecates:
Session#beginTransaction(String bookmark)in favour orDriver#session(bookmark)