Adding transaction support #758
Open
Conversation
This file contains 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
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #758 +/- ##
==========================================
+ Coverage 93.20% 94.47% +1.27%
==========================================
Files 96 96
Lines 2677 2732 +55
Branches 642 668 +26
==========================================
+ Hits 2495 2581 +86
+ Misses 182 151 -31 ☔ View full report in Codecov by Sentry. |
ejscribner
reviewed
Feb 28, 2024
| ## Creating a Transaction | ||
|
|
||
| To create a transaction, an application must supply its logic inside an arrow function, | ||
| including any conditional logic required. Once the arrow function has successfully run to conclusion, |
There was a problem hiding this comment.
Suggested change
| including any conditional logic required. Once the arrow function has successfully run to conclusion, | |
| including any conditional logic required. Once the arrow function has successfully run to completion, |
| To create a transaction, an application must supply its logic inside an arrow function, | ||
| including any conditional logic required. Once the arrow function has successfully run to conclusion, | ||
| the transaction will be automatically committed. | ||
| If at any point an error occurs, the transaction will rollback and the arrow function may run again. |
There was a problem hiding this comment.
Suggested change
| If at any point an error occurs, the transaction will rollback and the arrow function may run again. | |
| If at any point an error occurs, the transaction will roll back and the arrow function may run again. |
| Since the arrow function could be rerun multiple times, it is important that it does not contain any side effects. | ||
| In particular, you should never perform regular operations on a Collection, such as `create()` without using the `ctx`, inside the arrow function. | ||
| Such operations may be performed multiple times, and will not be performed transactionally. | ||
| Instead, you should perform these operations through the using the `{ transactionContext: ctx }` to pass the Transaction Context. |
There was a problem hiding this comment.
Suggested change
| Instead, you should perform these operations through the using the `{ transactionContext: ctx }` to pass the Transaction Context. | |
| Instead, you should perform these operations by using the `{ transactionContext: ctx }` to pass the Transaction Context. |
|
|
||
| Refer to [Error Handling](https://docs.couchbase.com/nodejs-sdk/current/concept-docs/transactions-error-handling.html#transaction_errors) for more details on these. | ||
|
|
||
| Method that currently support transaction context: |
There was a problem hiding this comment.
Suggested change
| Method that currently support transaction context: | |
| Methods that currently support transaction context: |
|
|
||
| The default configuration should be appropriate for most use-cases. | ||
| Transactions can optionally be globally configured when configuring the Cluster. | ||
| For example, if you want to change the level of durability which must be attained, |
There was a problem hiding this comment.
Suggested change
| For example, if you want to change the level of durability which must be attained, | |
| For example, if you want to change the level of durability that must be attained, |
| ensuring that each write is available in-memory on the majority of replicas before the transaction continues. | ||
| There are two higher durability settings available that will additionally wait for all mutations | ||
| to be written to physical storage on either the active or the majority of replicas, | ||
| before continuing. This further increases safety, at a cost of additional latency. |
There was a problem hiding this comment.
Suggested change
| before continuing. This further increases safety, at a cost of additional latency. | |
| before continuing. This further increases safety, at the cost of additional latency. |
| if (options.maxExpiry !== undefined) { | ||
| options.expiry = options.maxExpiry; | ||
| delete options.maxExpiry; | ||
| } | ||
| if (options.cas) { | ||
| storePromise = collection.replace(key, data, options); | ||
| if (transactionContext) { |
There was a problem hiding this comment.
Won't this always be true because of the || {} on line 18 above?
| if (options.transactionContext) { | ||
| const { transactionContext } = options; | ||
| const result = await transactionContext.get(c, key); | ||
| console.log(result); |
There was a problem hiding this comment.
Do we want to keep this logging in here?
There was a problem hiding this comment.
Should it be behind an if(debug) flag?
| // await delay(2500); | ||
| // | ||
| // const userRefdoc = await User.findRefName(userData.name, { transactionContext: ctx }); | ||
| // expect(userRefdoc.name).toBe(userData.name); |
There was a problem hiding this comment.
Transactions must use ctx.get, which doesn't work with plaintext docs
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.
closes: #749, #741