Transaction support #2465
Transaction support #2465
Conversation
alcaeus
force-pushed
the
feature/transactions
branch
from
9db6b44 to
c7d70e0
Compare
Co-authored-by: klinson <klinson@163.com> Co-authored-by: levon80999 <levonb@ucraft.com>
Co-authored-by: levon80999 <levonb@ucraft.com>
The faster connection and server selection timeouts ensure we don't spend too much time waiting for the inevitable as we're expecting fast connections on CI systems Co-authored-by: levon80999 <levonb@ucraft.com>
.github/workflows/build-ci.yml
Outdated
| @@ -59,6 +55,13 @@ jobs: | |||
|
|
|||
| steps: | |||
| - uses: actions/checkout@v2 | |||
| - name: Create MongoDB Replica Set | |||
| run: | | |||
| docker run --name mongodb -p 27017:27017 -e MONGO_INITDB_DATABASE=unittest --detach mongo:${{ matrix.mongodb }} mongod --replSet rs | |||
There was a problem hiding this comment.
Remember to use --setParameter transactionLifetimeLimitSeconds=5 (or a lower value) to minimize deadlock time in CI.
alcaeus
force-pushed
the
feature/transactions
branch
from
c7d70e0 to
4847be1
Compare
| - name: Create MongoDB Replica Set | ||
| run: | | ||
| docker run --name mongodb -p 27017:27017 -e MONGO_INITDB_DATABASE=unittest --detach mongo:${{ matrix.mongodb }} mongod --replSet rs | ||
| until docker exec --tty mongodb mongo 127.0.0.1:27017 --eval "db.runCommand({ ping: 1 })"; do |
There was a problem hiding this comment.
Just looking at the CI output. I'm not certain, but if this line is the only thing responsible for printing the exact MongoDB server version then perhaps we should use buildInfo here instead of ping. serverStatus was definitely TMI but it'd be helpful to have some details about the server (vs. just trusting the shell verison).
There was a problem hiding this comment.
Done - I added an additional build step for this to avoid people having to look through the entire setup output for this.
alcaeus
force-pushed
the
feature/transactions
branch
2 times, most recently
from
c88dcab to
5b07c78
Compare
This partially reverts commit 203160e. The simplified call unfortunately breaks tests.
alcaeus
force-pushed
the
feature/transactions
branch
from
5b07c78 to
2a82a54
Compare
This is consistent with the behaviour of the original ManagesTransactions concern.
alcaeus
force-pushed
the
feature/transactions
branch
from
3312932 to
df3a53a
Compare
Codecov ReportBase: 87.83% // Head: 88.21% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## master #2465 +/- ##
============================================
+ Coverage 87.83% 88.21% +0.38%
- Complexity 677 696 +19
============================================
Files 31 32 +1
Lines 1553 1595 +42
============================================
+ Hits 1364 1407 +43
+ Misses 189 188 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
@Smolevich thanks for your patience! This PR is now ready for review. I'll make a couple of minor changes to the CI setup to ensure that any aborted or stale transaction doesn't block the CI build for an unnecessary period of time (30 seconds by default), but the functionality is in place and ready to go! |
This ensures that hung transactions don't block any subsequent operations for an unnecessary period of time.
Co-authored-by: Jeremy Mikola <jmikola@gmail.com>
This comment was marked as off-topic.
This comment was marked as off-topic.
Note: contains changes from #2462 folded into a single commit; I'd prefer merging that PR before taking on this one.
This takes the work started by @klinson in #1904 and addresses some of the feedback left in that PR:
DB::beginTransactionorDB::transactionif they need to.DB::transactionmethod will respect the$attemptsargument and not invoke the callback more often, but may also abort before invoking the callback the specified number of times. This is due to the comparable logic in the MongoDB driver aborting after two minutes. I believe this is preferable to retrying for longer, especially in a web context (where two minutes is already quite long).Note that I have not touched the docker-compose logic. Transaction tests are skipped unless run against a deployment that supports them, and I don't think it's necessary to set up a local cluster for quick tests. People who need to debug transaction tests locally should set up their own deployment and change the connection URL via the
MONGODB_URIenvironment variable when running tests.This PR was made possible by the contributions of @klinson and @levon80999 who've worked on this in the past. Commits retain co-authorship to reflect this.