Support multiple persistence units

FroMage commented 5 years ago

As shown in #2759 comments, while we support defining multiple datasources, we don't actually support defining multiple persistence units:

we don't support them via application.properties
we can't use the datasources in persistence.xml
we can't seem to be able to inject EntityManager for multiple persistence units.

radcortez commented 4 years ago

+1

stale[bot] commented 4 years ago

This issue/pullrequest has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

gsmet commented 4 years ago

This one should still be open.

JfEstrela commented 4 years ago

this approach is necessary

stefanorg commented 4 years ago

+1 we really need this

famod commented 4 years ago

Very much needed for modular monoliths.

ionutbalosin commented 4 years ago

+1

gpietrek commented 4 years ago

+1

GuangjieZHANG commented 4 years ago

Yes, we really want it, please ... lol

yuhaibohotmail commented 4 years ago

+1

dragondiver commented 4 years ago

+1

mickroll commented 4 years ago

We really need that.

chrisgleissner commented 4 years ago

This is currently a blocker for us. Is any work planned on this?

Sanne commented 4 years ago

yes, of course we'll implement this. Sorry it's taking some time: several other issues are pre-requisites to be able to do this.

honeymoan commented 4 years ago

Currently porting a project to Quarkus, really need this. +1

AmsterdamFilho commented 4 years ago

Currently migrating 40 Microservices from Thorntail to Quarkus. Really need this too. +1

chrisgleissner commented 4 years ago

We plan on breaking up a JBoss 4 monolith into microservice and would be really keen to use Quarkus instead of Spring Boot. This feature is the only problem that would currently prevent us from using Quarkus. Until there is official support, maybe there is a workaround to manually create EntityManager instances from Datasource instances as Quarkus already supports multiple Datasources?

mlsdeveloper commented 4 years ago

Currently migranting my projects from spring boot to Quarkus. Realy need this too. +1

sekaijin commented 4 years ago

I have hundreds of camel routes, some of which have three or even four different data sources. Going from servicemix where these datasources are defined once for all routes, to microservices where they have to be defined as many times as there is use is already a major operating constraint. when a connection parameter changes for a source it is necessary to review all the routes which use it. But if you cannot read data in two databases to aggregate them in a third, it is on that the microservice architecture is not the solution for data exchanges. Realy need this too. +5

famod commented 4 years ago

@Sanne Is this something we can expect to land in 1.4.0? Thanks for your feedback!

rmh78 commented 4 years ago

+1

vishalgoel1988 commented 4 years ago

We also really want this, quite crucial for us.

wuiler commented 4 years ago

A much-needed enhancement. In mean while, we could use datasource, store procedure pointing to other datasource or even rest client to solved.

gavinking commented 4 years ago

I agree, this is rather important.

andersonbfn commented 4 years ago

Any feedback about when this feature will be available? This is a blocker for me too. I was trusted in the good recommendations about Quarkus, but seeing that still having problems like too long time.

famod commented 4 years ago

@andersonbfn groundworks have just started. The plan(!) is to release initial support with 1.8 (September).

PS: When and how exactly this feature is released is up to the project maintainers, which I am not part of.

gsmet commented 4 years ago

I have made some significant progress in this PR: https://github.com/quarkusio/quarkus/pull/11322 .

There are still a number of open questions though. Now is the time to give us some feedback if you're interested in this feature.

Please keep the general feedback in this issue. Let's keep the PR comments for the code review. Thanks!

Should we support multiple persistence units per entities

I.e. can you have a given entity attached to multiple persistence units?

Pros:

Flexibility (but I have no idea if it's really that useful in practice?)

Cons:

I think it will make it harder to detect configuration issues
Will make the Panache entity layer a bit more complicated, with possibly more methods that could make auto-completion less helpful. Also I have no idea if all the methods in Panache will support an added String persistenceUnitName without issues? E.g. do we have methods like myMethod(String... myParams). @FroMage maybe you have some clever idea about how to specify the PU name there?

How to attach an entity to a persistence unit?

I went with a package configuration meaning you should do:

quarkus.hibernate-orm."my-pu".packages=com.acme.mypu.package1,com.acme.mypu.package2

Note that it supports subpackages so in most of the cases, you would only have one package there because there's a good chance all the entities of a PU will be in a given package (at least if not sharing entities between several PUs is the common case). The nice thing in using packages is that I can automatically add the jars containing the packages to the Jandex index.

Another way to do that would be to use annotations on the entities. Not sure what's best in practice.

I like the "convention over configuration" approach of using packages.

Remaining work

Here are things that will need some work once the first PR is in:

Multitenancy: I let it aside for now, I don't know how much effort it will be to make it "multiple persistence units"-aware. I need to look into it more closely.
Hibernate ORM with Panache
Hibernate Search
Security JPA - I enforced the default persistence unit for now, we should detect in which PU are the entities used by the security layer and use it (and fail if we have more than one)

One important thing is that I publish the entity -> PUs (yes PUs with an s, even if for now we only support one) mapping to the JPAConfig CDI bean. We could also publish this information as a build item if we need it at build time in some of the extensions.

FroMage commented 4 years ago

I.e. can you have a given entity attached to multiple persistence units?

I would say do whatever you want for ORM, but for Panache we'll only support a single PU per entity. I think this is very much not in the 80% use-case we're targetting and doesn't justify adding extra params to every method. Plus if the user wants to make an entity available into another PU, they can subclass it with another PU config, I think.

Another way to do that would be to use annotations on the entities. Not sure what's best in practice.

I'd definitely support annotations, in fact I was expecting that I could use https://docs.jboss.org/hibernate/jpa/2.1/api/javax/persistence/PersistenceUnit.html on my entity to express this. But it's very unfortunate that I can't place this annotation on the package. Perhaps we should make our own annotation.

I like the "convention over configuration" approach of using packages.

I don't see how this is a convention, unless you're saing that the last part of the package (in com.example.bar.gee) that holds the entities is by convention the PU name (here gee) without any configuration, but given that you've only showed configuration that doesn't seem to be clear. Note that I'd be fine with such a convention.

famod commented 4 years ago

I.e. can you have a given entity attached to multiple persistence units?

Clear +1 from my POV!

Imagine you want to have a (rather technical) base entity for all your "business subdomains" (shared in multiple microservices via some lib jar or in a shared "module" in a modular monolith), with fields like @Version, @CreationTimestamp, @UpdateTimestamp and maybe even entity callbacks (@PrePersist etc.). Typically, such an abstract base entity would be annotated with @MappedSuperclass to have the fields recognized. Mapped superclasses must be registed in every PU they are used in.

Another usecase: Imagine each microservice or modular monolith subdomain brings it's own configuration (options) for which you want to provide a generic CRUD UI. Typically, you would not want to "reinvent the wheel" but use a generic config data model specialized for you business needs. Here you would typically have a small set of concrete entities that are present in each PU.

knutwannheden commented 4 years ago

Imagine you want to have a (rather technical) base entity for all your "business subdomains" (shared in multiple microservices via some lib jar or in a shared "module" in a modular monolith), with fields like @Version, @CreationTimestamp, @UpdateTimestamp and maybe even entity callbacks (@PrePersist etc.).

I also recognize this use case. However, in the project I am working on the "business subdomains" correspond to separate Quarkus applications. In that scenario the shared technical entity (e.g. a @RevisionEntity annotated class) is attached to multiple PUs, but typically not to multiple PUs within one Quarkus application, which should be fine, AFAICT. This being said, I think I have also come across cases where a technical entity is used by multiple PUs within the same application.

famod commented 4 years ago

is attached to multiple PUs, but typically not to multiple PUs within one Quarkus application, which should be fine, AFAICT

You're right. So for a microservice setup, it depends how big or small your microservices are. I've seen teams merging microservices to bigger ones because the management vs. flexibility tradeoff just did not justify too many really "micro" services for them. In such a case you might have two or more PUs in a microservice/Quarkus app.

gavinking commented 4 years ago

I.e. can you have a given entity attached to multiple persistence units?

I would say do whatever you want for ORM, but for Panache we'll only support a single PU per entity. I think this is very much not in the 80% use-case we're targetting

I agree.

Another way to do that would be to use annotations on the entities.

I would personally much prefer to use package-level annotations for this. (Though I have no problem with also allowing the annotation at the entity level.

gavinking commented 4 years ago

@gsmet

I went with a package configuration meaning you should do:
quarkus.hibernate-orm."my-pu".packages=com.acme.mypu.package1,com.acme.mypu.package2

This looks quite ugly to me. I would prefer to annotate the package containing the entities.

gavinking commented 4 years ago

@famod

Typically, such an abstract base entity would be annotated with @MappedSuperclass to have the fields recognized. Mapped superclasses must be registed in every PU they are used in.

@MappedSuperclasses aren't considered entities in JPA, and so I don't think we should need to explicitly assign them to a PU. Hibernate should be able to automatically discover any @MappedSuperclass of an entity that is assigned to the PU.

(And the same goes for @Embeddables, etc.)

So I don't think this particular case justifies the need to assign an entity to multiple PUs.

gavinking commented 4 years ago

@famod

Another usecase: Imagine each microservice or modular monolith subdomain brings it's own configuration (options) for which you want to provide a generic CRUD UI. Typically, you would not want to "reinvent the wheel" but use a generic config data model specialized for you business needs.

If it's a separate microservice, then it can easily specify its own configuration for the PU. Again, I don't see how this motivates the need to assign an entity to multiple PUs.

robmv commented 4 years ago

Should we support multiple persistence units per entities

Please support it!. In situations when you are hosting multiple instances of the same application for different customers, or you are running the same application multiple times because they have to be isolated (different subsidiaries for example), sometimes you have another application that take information for each one of these databases or there is a need to do some processing on all instances of the installed application.

gavinking commented 4 years ago

Please support it!. In situations when you are hosting multiple instances of the same application for different customers, or you are running the same application multiple times because they have to be isolated (different subsidiaries for example), sometimes you have another application that take information for each one of these databases or there is a need to do some processing on all instances of the installed application.

@robmv once again, I do not see how this motivates the feature of assigning one entity to multiple PUs in a single instance.

What you need to solve your problem is the ability to have a different configuration for a certain named PU in each microservice. Which AFAICT is indeed something you can already do.

robmv commented 4 years ago

What you need to solve your problem is the ability to have a different configuration for a certain named PU in each microservice. Which AFAICT is indeed something you can already do.

This isn't a micro service, this is a management application let call it B that connect to multiple instances (multiple databases) of the same application A, that can read the same entity from each instance for reporting or management of all the instances.

I agree that this isn't a common practice, normally people only have one instance of the same application, but when you host multiple customers, or have subsidiaries, you have sometimes to do things on multiple databases with the same schema at the same time

gavinking commented 4 years ago

This isn't a micro service, this is a management application let call it B that connect to multiple instances (multiple databases) of the same application A, that can read the same entity from each instance for reporting or management of all the instances.

Well it still seems to me that multiple PUs are a rather bad way to handle this. Your problem seems more closely related to multi-tenancy (though in fairness I'm not sure if Hibernate's existing support for multi-tenancy can deal with your precise requirements).

I agree that this isn't a common practice

Right, then we do agree. It's a very advanced case, and it's not the case that most people are thinking of when they're asking for support for multiple PUs. What they're thinking of is a case where I have two different schemas, in different databases.

They're not thinking of a clone of the same schema with different connection properties or a different catalog name.

gavinking commented 4 years ago

@robmv have you tried using MultiTenantConnectionProvider for this?

robmv commented 4 years ago

@robmv have you tried using MultiTenantConnectionProvider for this?

I am learning from this Hibernate feature right now, and it looks like it can manage our use case but have to test it. Thanks.

famod commented 4 years ago

@gavinking

Typically, such an abstract base entity would be annotated with @MappedSuperclass to have the fields recognized. Mapped superclasses must be registed in every PU they are used in.

@MappedSuperclasses aren't considered entities in JPA, and so I don't think we should need to explicitly assign them to a PU. Hibernate should be able to automatically discover any @MappedSuperclass of an entity that is assigned to the PU.

I'd just like to be able to use those mapped superclasses, embeddables etc. in multiple PUs. If Hibernate can support this automagically then I'm fine with it!

gavinking commented 4 years ago

I'd just like to be able use those mapped superclasses, embeddables etc. in multiple PUs. If Hibernate can support this automagically then I'm fine with it!

Great, I think we should make sure that it can.

(@gsmet can clarify if there's any problems with that.)

michael-schnell commented 4 years ago

@robmv have you tried using MultiTenantConnectionProvider for this?

I am learning from this Hibernate feature right now, and it looks like it can manage our use case but have to test it. Thanks.

Multitenancy is already supported by Quarkus when you do not use persistence units (See https://quarkus.io/guides/hibernate-orm#multitenancy).

Sanne commented 4 years ago

I'd just like to be able use those mapped superclasses, embeddables etc. in multiple PUs. If Hibernate can support this automagically then I'm fine with it!

Great, I think we should make sure that it can.

I agree, it should be doable to allow the mapped entities to inherit from superclasses without restrictions, and similarly use embeddables across entities. I also don't expect that it would be necessary to explicitly list such types as belonging to a particular PU, they would just need to be pulled into the mapping transitively.

Sanne commented 4 years ago

Regarding Panache: as @FroMage mentioned, it's not worth it to have to "pollute" all methods it generates on entities to also accept some form of PU identifier (name?), but I suppose we could eventually support entity sharing provided the user is using the repository pattern.

gavinking commented 4 years ago

I also don't expect that it would be necessary to explicitly list such types as belonging to a particular PU, they would just need to be pulled into the mapping transitively.

Yup, that's what I mean.

it's not worth it to have to "pollute" all methods it generates on entities to also accept some form of PU identifier (name?)

Totally. I regret having introduced the entityName version of methods on the Session API, way back in the day, and this would be as bad or worse than that.

but I suppose we could eventually support entity sharing provided the user is using the repository pattern.

To clarify, I didn't mean to imply I'm hostile to allowing this if it's easy to do. My above posts were merely arguing that I don't think it's anything like a high priority / hard requirement.

gsmet commented 4 years ago

I also don't expect that it would be necessary to explicitly list such types as belonging to a particular PU, they would just need to be pulled into the mapping transitively.

FWIW, right now, it doesn't work automagically. I only added the entities to the PU first and I ended up with tests failing.

Or do you expect Quarkus to do the work rather than Hibernate ORM?

Sanne commented 4 years ago

FWIW, right now, it doesn't work automagically. I only added the entities to the PU first and I ended up with tests failing.

Or do you expect Quarkus to do the work rather than Hibernate ORM?

I would expect this to work in ORM "standard" but since in Quarkus we have a custom Scanner, we probably need Quarkus to do this extra step. But of course feel free to ignore this for now, it seems a good follow-up task for others to help.

gsmet commented 4 years ago

Regarding Panache: as @FroMage mentioned, it's not worth it to have to "pollute" all methods it generates on entities to also accept some form of PU identifier (name?), but I suppose we could eventually support entity sharing provided the user is using the repository pattern.

We could also provide a specific base class to support that.

quarkusio / quarkus