-
Notifications
You must be signed in to change notification settings - Fork 192
Add support for Couchbase Transactions. #1246
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
Conversation
3422a20
to
d294b2f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the first draft of the PR.
interface RemoveByIdWithCas extends RemoveByIdWithDurability { | ||
|
||
interface RemoveByIdWithCas extends RemoveByIdWithDurability, WithCas<RemoveResult> { | ||
@Override | ||
RemoveByIdWithDurability withCas(Long cas); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WithCas() is unrelated. It was missing and I added it at the same time as WithTransaction.
return Mono.defer(() -> countResult.onErrorMap(throwable -> { | ||
CouchbaseClientFactory clientFactory = template.getCouchbaseClientFactory(); | ||
ReactiveScope rc = clientFactory.withScope(pArgs.getScope()).getScope().reactive(); | ||
Mono<ReactiveQueryResult> countResult = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
all the cluster.query(), collection.query() and ctx.query() return a ReactiveQueryResult, which is nice.
} | ||
return reactiveEntity.onErrorResume(throwable -> { | ||
if (throwable instanceof DocumentNotFoundException) { | ||
return Mono.empty(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The kv operations on a collection return a GetResult, while the kv operations on ctx return a TransactionGetResult.
} | ||
|
||
@Override | ||
public TerminatingFindByQuery<T> withOptions(final QueryOptions options) { | ||
public FindByQueryWithQuery<T> withOptions(final QueryOptions options) { | ||
Assert.notNull(options, "Options must not be null."); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not related to transactions, but was out of line with how each interface extends the previous one.
} | ||
|
||
public <R> FindByQueryWithConsistency<R> as(Class<R> returnType) { | ||
public <R> FindByQueryWithProjecting<R> as(Class<R> returnType) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not related to transactions, but was out of line with how each interface extends the previous one.
@@ -60,7 +60,7 @@ ClusterType type() { | |||
|
|||
@Override | |||
TestClusterConfig _start() throws Exception { | |||
bucketname = UUID.randomUUID().toString(); | |||
bucketname = "my_bucket"; // UUID.randomUUID().toString(); | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To speed up my testing only. Will remove.
.deleteAllById(Arrays.stream(iatas).map((iata) -> "airports::" + iata).collect(Collectors.toSet())); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An extra test for Group By. Unrelated to transactions.
d294b2f
to
b53bbde
Compare
25fcb14
to
1679651
Compare
a1a2a1d
to
3a33f44
Compare
33d8b5f
to
778b10e
Compare
The fluent operations are common for options that are common to operations with and without transactions. Once there is a transaction(ctx), or an option specific to without-transactions (such as scanConsistency), the interfaces are bifurcated, so that an non-transaction option cannot be applied to a transaction operation, and a transaction(ctx) cannot be applied where a non-transaction option has already been applied. Closes #1145.
fa8ee88
to
a226f88
Compare
a226f88
to
7b1ba51
Compare
7b1ba51
to
b4a93d7
Compare
2e6d152
to
fde39ed
Compare
fde39ed
to
95bf2fc
Compare
* Porting to SDK-integrated version of transactions The transactions logic exists in the Java SDK as of 3.3.0, with a slightly different API. This is the first effort at the port, which literally just compiles. It will not run as crucial code has been commented and todo-ed. There is work remaining to figure out how to complete the port, as some crucial parts (such as ctx.commit() and ctx.rollback()) have been intentionally removed. * Continuing work to get the ExtSDKIntegration port working Trying to transition to CallbackPreferring manager. * Added CouchbaseSimpleCallbackTransactionManager, the simplest possible implementation of CallbackPreferringTransactionManager, combined with a simpler approach to ThreadLocal storage in ReactiveInsertByIdSupport. Test 'commitShouldPersistTxEntriesOfTxAnnotatedMethod' is now passing. * Adding WIP get-and-replace @transactional support (Not yet working as CAS/version field in Person is not populated correctly.)
…g-projects/spring-data-couchbase into datacouch_1145_transaction_support
df19157
to
fe82566
Compare
fe82566
to
9e7fdde
Compare
* Porting to SDK-integrated version of transactions The transactions logic exists in the Java SDK as of 3.3.0, with a slightly different API. This is the first effort at the port, which literally just compiles. It will not run as crucial code has been commented and todo-ed. There is work remaining to figure out how to complete the port, as some crucial parts (such as ctx.commit() and ctx.rollback()) have been intentionally removed. * Continuing work to get the ExtSDKIntegration port working Trying to transition to CallbackPreferring manager. * Added CouchbaseSimpleCallbackTransactionManager, the simplest possible implementation of CallbackPreferringTransactionManager, combined with a simpler approach to ThreadLocal storage in ReactiveInsertByIdSupport. Test 'commitShouldPersistTxEntriesOfTxAnnotatedMethod' is now passing. * Adding WIP get-and-replace @transactional support (Not yet working as CAS/version field in Person is not populated correctly.) * Transitioning to use CoreTransactionAttemptContext. Tests may fail. * Removing AttemptContextReactiveAccessor Don't think we need this, as we can pass around CoreTransactionAttemptContext instead, which gives access to a lot of internals. * Removing TransactionsReactive Would prefer not to C&P a huge class out of the transaction internals, and don't think we need it. * Removing some files not currently used To reduce & simplify the amount of code to look at. Some don't seem to be used in any branch, some just aren't used in this branch. * Removing CouchbaseTransactionInterceptor As per offline discussion, CallbackPreferringPlatformTransactionManager is perhaps the optimal solution. * Copying @transactional tests out into separate class * Tidyup * Tidyup test names * Verify GenericSupport is on same thread before and after transactional operation * Refactoring CouchbaseSimpleCallbackTransactionManager ThreadLocalStorage management * Using latest java-client * ReactiveReplaceByIdSupport - Fixing use of CAS now have CoreTransactionAttemptContext. Removing unused code. * ReactiveInsertByIdSupport - fixing use of reactive vs non-reactive, and CAS * Merging upstream * Remove incorrect thread check (.doOnNext could execute on a different thread) * Completing merge from upstream * Removing unused classes * Give GenericSupport a better name * Reject at runtime options that aren't supported in a transaction * Fixing some small todos, partly by removing unused coe * Fix runtime option checks * Simplifying CouchbaseSimpleCallbackTransactionManager ThreadLocalStorage Standardising on ReactiveCouchbaseResourceHolder rather than holding CoreTransactionAttemptContext too * Removing version from CouchbaseDocument Can't recall why I added this, and tests pass without it * Improving CouchbaseTransactionalIntegrationTests and adding more tests * Reject operations that aren't allowed in a transaction (upsertById etc.) * Improve handling of CAS mismatch By calling CoreTransactionAttemptContext.operationFailed, it ensures that internal state is set. So even if the user catches the exception, the transaction still behaves as it should. * Removing a now-redundant non-transactional check on upsertById I missed this when adding TransactionalSupport.verifyNotInTransaction here. * Support @transactional options timeout and isolation level Co-authored-by: Graham Pople <[email protected]>
Closes #1145.