-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Attempt to Support fror @Inject StatelessSession #8861
Conversation
For now this is incomplete - basically just setup the actual @Inject option but currently does not actually do anything - thats where I need @Sanne or someone with some jpa/quarkus internal knowledge :) a) in b) for now I could only get injection to work if there is a Entity present - didn't find a good way to have it allow when 0 entites but I reckon we can do that in a future update. |
@maxandersen what is missing now for this? |
Why: * Sometimes you just want to do raw data access This change addreses the need by: * Add StatelessSession as a possible injection. Signed-off-by: Max Rydahl Andersen <manderse@redhat.com>
66806d5
to
3c9a46b
Compare
I've updated to just include a basic producer method + basic docs. Remaining two questions:
|
marking as for review as conceptually its done but I need some feedback on how tx's are handled in these layers. |
@emmanuelbernard look here :) |
|
||
@Transactional | ||
public void queryGifts(String giftDescription) { | ||
ScrollableResults gifts = session.getQuery("from Gift") |
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.
What is getQuery
?
=== Processing raw or large amount of data | ||
|
||
`StatelessSession` are supported too and can be injected and used standalone or inter-mixed with your `EntityManager`. | ||
The benefits of `StatelessSession` are that entities are not put in a session-level cache, allowing for more raw and in some cases higher performance |
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.
Think of it as read and forget.
|
||
`StatelessSession` are supported too and can be injected and used standalone or inter-mixed with your `EntityManager`. | ||
The benefits of `StatelessSession` are that entities are not put in a session-level cache, allowing for more raw and in some cases higher performance | ||
data access. Especially useful when reading/processing large amount of data. |
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.
and avoiding OutOfMemoryException while processing the data.
-- | ||
<.> Inject your stateless session and have fun; it will open or reuse the active transaction. | ||
<.> Using `scroll()` means you can process the data without loading the complete resultset or list of data in memory. | ||
<.> Since using a stateless session you need to explicitly call `update()` to save changes. |
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.
<.> Since using a stateless session you need to explicitly call `update()` to save changes. | |
<.> a stateless session does not remember your objects, you need to explicitly call `update()` to save changes. |
return transactionEntityManagers.getEntityManager(pc.unitName()); | ||
} | ||
}; | ||
return () -> entityManager; |
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.
Looks like something fishy here, shouldn't you return unwrap(SessionFactory.class).openStatelessSession()
. in this case? I fail to see how the right object is injected otherwise.
CC @mkouba
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.
So this should be used for @PersistenceContext(unitName="foo") StatelessSession
injection, right? But we don't support multiple PUs yet, correct?
If possible, I'd remove this code completely because ResourceReferenceProvider
should only be used for non-CDI EE resource injection. In this case, it would be probably better to use a custom qualifier or something like that...
@maxandersen What the use case for @PersistenceContext StatelessSession
?
To answer your question @maxandersen, yes I think your stateless session will join the existing transaction if I read the code correctly. |
But you have me puzzled, I did not know |
To get access to do native queries with simple data mapping layer without having to setup/configure yet another framework. Hibernate has all the apis needed already. I could technically use a Session/EntityManager to kick of most native queries but: a) they come with an overhead that is not used for raw data access With statelessSession have the following advantages: a) I can do raw data access with no to little overhead with mapping to scalars and DTO's Does that help or where you asking for some more code examples beyond what i put in the basic docs? |
@maxandersen scalar queries won't return managed objects, so wouldn't you have the same benefits you listed with any regular Session ? The Session implementation is mostly lazy, it will only allocate its state-holding infrastructure on demand. |
I can't do raw update/insert with EntityManager/Session. thus batch/bulk operations not as effective by long shot. |
I'm not questioning the usefulness of StatelessSession - but you won't do a "raw update/insert" on a scalar or an unmapped type? I'm not sure if you've got me completely confused on the idea, a runnable test would be great. |
Thanks, that helps a lot and I would really like to see concrete code examples. It will hopefully make the value more concrete to me. |
There are two main usescases for stateless session that EntityManager can't do - below they are in my personal "priority":
And yes, #2 can technically be done with Session/EntityManager but the API you get access to assumes you have entities majority of the time. Additionally my hope was that where we now don't start EntityManagerFactory at all when zero entities I was thinking that @Inject StatelessSession could be used to detect we should start up anyways. #1 for me alone is more than enough to justify easy access to StatelessSession and not have every app implement a @produces StatelessSession .... |
@maxandersen I agree that ther are two. I was just pointing out that in your explanation you seemed to imply that one needs the other, which is what I think was confusing Emmanuel.
BTW we should really not call it "batch" as the NON-stateless Session can actually do batching, which is in fact a massive optimisation over what one can achieve via the stateless session, as it can reduce the number of DB roundtrips by orders of magnitude. I think we agree on all points, just suggesting improvements on how to call these things and the explanations. |
The reason we don't start is is that we don't want ti to fail in the face of a new user. I forgot if it failed because zero entity were in or simply because without DB config, then Hibernate can't start. @mkouba I wonder how at build time, we could detect that the application has |
public class DefaultStatelessSessionProducer { | ||
|
||
@Inject | ||
EntityManagerFactory em; |
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 would probably fail if a custom EntityManagerFactory
producer is present (DefaultEntityManagerFactoryProducer
is only registered if such producer does not exist).
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.
Why would it fail?
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.
Oh, sorry. It would fail if a user-provided producer declares some qualifier... such as @Documents
in:
@Produces @PersistenceUnit(unitName="documents") @Documents EntityManagerFactory factory;
But it's true that there is no reason to declare a producer like this if only one PU is supported...
|
||
@Transactional | ||
public void queryGifts(String giftDescription) { | ||
ScrollableResults gifts = session.getQuery("from Gift") |
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.
ScrollableResults gifts = session.getQuery("from Gift") | |
ScrollableResults gifts = session.createQuery("from Gift") |
@gastaldi @emmanuelbernard @mkouba @Sanne |
Hi! sure, a little help here would be awesome. I'm not sure why Max didn't finish it :) I believe this is what's missing:
Specifically, I would recommend to not work on making it possible to start Hibernate ORM without any entities; Max wanted to do that as well but we should keep that as a separate issue, separate patch and separate discussion. Thanks! |
I didn't finish it as awaiting response from @mkouba on how to do the inject work :) If someone else want to give it a try feel very welcome! |
@maxandersen I'm sorry what's the exact problem? I added some comments few months ago but I'm a bit lost in comments now... Also it seems that we do support multiple PUs now, or? |
Closing as this needs to be rebased. Reopen when Sanne's comments in #8861 (comment) are addressed |
@Sanne any pointers on where this code is? Unless I understood your list above incorrectly, I think this is something we can deliver without a terrible amount of effort. |
@geoand have a look into how a N.B. this risks to be fairly different with ORM 6 - perhaps it's the worst time to want to revive this issue :/ |
Understood. Let's discuss again when ORM 6 has been merged as I would really like to have this feature for Quarkus 3 |
Why:
This change addreses the need by:
Fixes #7148