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

Support transactions inside MongoDB with Panache (reloaded) #10097

Conversation

loicmathieu
Copy link
Contributor

This is a new attemp to provides transaction support for MongoDB via Narayana.

It reuses the existing transactional interceptor and only add a synchronization point to commit/rollback the session based on the state of the transaction.

@loicmathieu
Copy link
Contributor Author

@FroMage based on the multiple feedbacks on the transaction support for non-RDBMS and reactive datastore I come back with a different design: reuse the existing interceptors and Transaction object, and create a dedicated interceptor, fired after the Narayana one, that creates the session and registers a synchronization to commit/rollback the session based on the transaction state.

It was surpisingly easy to do and it seems to work!

Of course, the three blocking points for the reactive implementation are still here ... we will need an async syncronization at some point ...

Do you think it's a better design than #7222 ?

@loicmathieu
Copy link
Contributor Author

@FroMage I want to re-work on transactional support for MongoDB, do you think the design provided here is better than the previous one ?

For the reactive impl, I think the best is to provide it on a separate PR, aligned with the way transactions are done for Hibernate RX.

Copy link
Member

@FroMage FroMage left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @loicmathieu, really sorry about the late review.

This design looks great, but I can't understand how we won't get both a blocking and reactive session for every transactional endpoint? Both interceptors are declared in the same module, no?

@loicmathieu
Copy link
Contributor Author

@FroMage yes, your're right!
I need to check if the client exist and open a session only if it exists.

I cannot know which client is used inside the transactional method, so I need to open a session for both client if both exist (in the CDI bean container).

Another solution would be to support reactive transaction programmatively only, I think this is the design for Hibernate RX right ? Because here, as interceptors are blocking, I block on the session handling code.

@FroMage
Copy link
Member

FroMage commented Oct 27, 2020

Perhaps @gavinking or @Sanne have opinions about this?

@gavinking
Copy link

Perhaps @gavinking or @Sanne have opinions about this?

I don't quite understand the question TBH.

@Sanne
Copy link
Member

Sanne commented Oct 27, 2020

Perhaps @gavinking or @Sanne have opinions about this?

I don't quite understand the question TBH.

me neither :)

Generally, I'd be very cautious about integrating such a thing with JTA. MongoDB included some transactional features, but there's limitations to the scope in which they can apply. I'm quite skeptical that one can model this safely using Java scopes, when the transaction boundary of the backing store is limited by the modelling.

What are we going to do if an high level Panache operation is mapped into multiple sub-transactions on MongoDB, but the overall business method is wrapped into a single Transactional annotation?

Personally I would rather not allow this at all - but I don't have enough experience with it to propose a reasonable alternative.

Sorry I know that's not an exciting answer - all I mean is please think carefully about the implications here.

@gavinking
Copy link

@Sanne but is the question specific to Mongo, or is it general to any app with a mix of reactive and blocking?

@Sanne
Copy link
Member

Sanne commented Oct 27, 2020

@Sanne but is the question specific to Mongo, or is it general to any app with a mix of reactive and blocking?

I'm not sure, good point. I don't think we have a good plan yet for transactions + reactive; personally I don't think using annotations would fit but I have no strong opinion about it - I think I lack experience here and would need to experiment with options, but also make sure we experiment with non-trivial use cases.

@FroMage
Copy link
Member

FroMage commented Oct 27, 2020

OK, so you're saying let's hold off on supporting reactive with @Transactional I guess, on the one hand, and be careful with mongo transactions on the other?

I'm not sure I see why mongo transactions should be more complex to make work with @Transactional than JPA ones, but I'm by no means an expert.

@loicmathieu we could perhaps skip supporting the reactive ones at first, and offer an equivalent to the transactional API that we have for HR+Panache. As for the initialisation of the client in the interceptor, I think you should be able to skip the interceptor entirely and delay the bit which registers your client to the TM until it's used. I'm pretty sure ORM does it this way: when you call an operation (not just obtain a client) we look at whether we have a current JTA transaction and if yes, we create a mongo transaction and register it to the TM (the same part you currently do in the interceptor).

This way we never create mongo transactions unless we use them in whatever JTA transaction.

Sounds right?

@loicmathieu
Copy link
Contributor Author

@loicmathieu we could perhaps skip supporting the reactive ones at first, and offer an equivalent to the transactional API that we have for HR+Panache.

Yes, this was my idea.

As for the initialisation of the client in the interceptor, I think you should be able to skip the interceptor entirely and delay the bit which registers your client to the TM until it's used.

I can delay the creation of the session but I still needs the interceptor to gather the MongoTransactionConfiguration and store them somewhere. I can reuse the mechanims proposed for allowing extra transaction configuration for read only transaction: https://github.com/quarkusio/quarkus/pull/10077/files#diff-ba664077eac742dcb202e85d12bb39dedde7bf6e52793ffaca1df9dc623782b1

Base automatically changed from master to main March 12, 2021 15:54
@loicmathieu
Copy link
Contributor Author

Superseded by #14002

@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label Mar 24, 2021
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

4 participants