Skip to content
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

Alternative support for Mongo Transactions in v4 #334

Open
wants to merge 4 commits into
base: development
Choose a base branch
from

Conversation

jameinel
Copy link

Needs some testing around multiple version handling, and proper error
messages if you try to use transactions with Mongo versions that don't
support them.

We'll also need more testing around multi threading, etc. This was
mostly a proof of concept that it could be done without a separate
'Transaction' object that users need to worry about. It also supports
having Query be transaction aware.

This does make sure that Start/Commit/AbortTransaction all do
essentially what you want, and it doesn't involve any client-side
changes. If you call Session.StartTransaction then all
Update/Insert/Remove/Find requests will be done in that context, and
won't be visible to other sessions.

There is also still a fair bit more that can be done wrt things like
WriteConcern. I mostly wanted to get feedback if people like how this is
structured and where it is going.

As far as I can tell, while there is a txnNumber associated with a given
transaction, Mongo doesn't actually support having >1 open transaction
for a given session, so it isn't worth tracking transaction objects for
users.

This is hopefully a simpler method to support Mongo Transactions than #280.

It isn't complete yet, but it is ready for initial feedback before I commit much more time to it.

Needs some testing around multiple version handling, and proper error
messages if you try to use transactions with Mongo versions that don't
support them.

We'll also need more testing around multi threading, etc. This was
mostly a proof of concept that it could be done without a separate
'Transaction' object that users need to worry about. It also supports
having Query be transaction aware.

This does make sure that Start/Commit/AbortTransaction all do
essentially what you want, and it doesn't involve any client-side
changes. If you call Session.StartTransaction then all
Update/Insert/Remove/Find requests will be done in that context, and
won't be visible to other sessions.

There is also still a fair bit more that can be done wrt things like
WriteConcern. I mostly wanted to get feedback if people like how this is
structured and where it is going.

As far as I can tell, while there is a txnNumber associated with a given
transaction, Mongo doesn't actually support having >1 open transaction
for a given session, so it isn't worth tracking transaction objects for
users.
@duskglow
Copy link

duskglow commented Mar 13, 2019

Hi, I'm the author of #280. I'm not sure which is better, honestly, but some feedback really quick: I think the txnNumber does need to be kept track of. You're correct that mongo doesn't support more than one open transaction, but when you start a new one, it just supercedes the old one, and the old one cannot be committed anymore. So what happens if you, for example, start the transaction twice?

I ran into some issues with that, and it turns out that if you leave a transaction open, and restart it or start a new one, then committing has some interesting issues.

I like your approach to backwards compatibility, but what of the cases where you want to do some things in a transaction and some things outside of a transaction? Collections cannot be automatically created in a transaction, for example, so if you want that functionality you have to be able to stop using transactions. I imagine that an abort or commit could close the current transaction and reset it back to not using transactions, but that's something that needs to be taken into account.

I had created the transaction object for the purpose of keeping track of multiple transactions, say, in multiple sessions, but in hindsight, that might be overkill...

Will say that yours has one huge advantage over mine: it passes all the tests. To be honest, I haven't yet figured out how to get the harnesses working properly with 4.x, so...

@jameinel
Copy link
Author

jameinel commented Mar 14, 2019 via email

@jameinel
Copy link
Author

jameinel commented Mar 14, 2019 via email

They don't give guaranteed ordering, but they do avoid object race
conditions.
globalsign now is thread unsafe if you use .Dial because it calls
SetSocketTimeout which mutates dialInfo in an unsafe way. But we can
just use DialWithTimeout instead.
@domodwyer
Copy link

Hi @jameinel

Sorry for the long, long delay!

This looks pretty sweet - I haven't got a >v4.0 mongo server to test against at the moment, but the unit tests look pretty comprehensive. I'll try stand one up and get a run.

Have you managed to try this in any real world usage?

Dom

@jameinel
Copy link
Author

jameinel commented Apr 2, 2019 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants