Why is it called @ClientProfile

[emmanuelbernard]

There seems to be the need in an application to define somewhere (where?) a @ClientProfile annotation to select a specific NoSQL datasource definition from the Wildfly config.

I am wondering why it is called ClientProfile. What makes it a client, what makes it a profile. I'm bothered because the name is hypra generic and hard to remember. I'd perfer something like NoSQLProfile or something with NoSQL in it I think.

[scottmarlow]

Correct, the @ClientProfile selects a specific NoSQL datasource definition from the WildFly config. +1 I'm all for calling it something better than ClientProfile. I like NoSQLProfile better. NoSQLSourceProfile is probably too long but could be an alternative.

[emmanuelbernard]

How about @NoSQLSource or @NoSQLDataSource

[scottmarlow]

I like @NoSQLSource.

Should we rename the NoSQLSource interface, which I don't actually use yet? I was thinking that the NoSQLSource interface returns a NoSQLConnection. NoSQLConnection has a unwrap(Class).

[emmanuelbernard]

What were you planning to add besides a getConnection?

[scottmarlow]

I'm not really sure, whatever else OGM might need. Perhaps NoSQLConnection is what OGM should instead use.

[emmanuelbernard]

@gunnarmorling does OGM needs more than the connection or equivalent ?

[emmanuelbernard]

@gunnarmorling here is the ClientProfile code and the NoSQLSource interface

[gunnarmorling]

Naming aside, why is @ClientProfile a class-level annotation? I think this should rather be configured at the injection point itself, e.g. to work with clients for different NoSQL stores in one class or with two clients for the same store, but using different end points.

[gunnarmorling]

Instead of having a new annotation, could we simple re-use the existing CDI/common annotations:

@Inject @Named("somemongoconnection")
private MongoClient mongo;

@Resource(name="somemongoconnection")
private MongoClient mongo;

?

[gunnarmorling]

@gunnarmorling does OGM needs more than the connection or equivalent ?

Connection should be enough from what I can say. Would love to try it out soon so to know whether it's going to fly.

How would OGM obtain this? We don't depend on CDI right now; Are you planning to support JNDI look-ups, too, e.g. some sort of "nosql/..." sub-tree?

[emmanuelbernard]

Ah right that's something I meant to say and ask @scottmarlow OGM cannot depend on CDI for that work.

Does that work means that all applications will depend on CDI or only if your subsystem detect @ClientProfile?

[scottmarlow]

With regard to improving @inject by using @Named, which allows a single name, instead of @ClientProfile which allows a JNDI name or NoSQL profile name, to be used (currently). Would @Named work to specify a lookup value, like the JNDI name or NoSQL profile name? Or is @Named more for identifying (or qualifying) which CDI bean to inject?

Regarding how OGM gets the native NoSQL connection object, I wave my arms loosely in the air and say its something like this. Internally, WildFly code can lookup the NoSQL connection objects, the JNDI name or NoSQL profile name is needed for this, we can perform this lookup on behalf of OGM. OGM should not depend on WildFly classes, instead it should be the inverse, where the WildFly OGM adapter, acts as a bridge between WildFly JipiJapa + OGM. I think that the WildFly JipiJapa OGM adapter should pass the NoSQL connection via an integration property. In terms of how JipiJapa gets the NoSQL connection, I think that the application should specify a hint as to which WildFly NoSQL profile is needed.

In terms of trying stuff out, we currently have the topic branch, which we could create a sub-branch for OGM or develop against directly. Others are creating pull requests against this topic branch or will.

[scottmarlow]

I would like to use CDI/common annotations if they fit. Not sure they do though. Maybe we could override @Resource.name to identify the NoSQL connection name

[scottmarlow]

Naming aside, why is @ClientProfile a class-level annotation? I think this should rather be configured at the injection point itself, e.g. to work with clients for different NoSQL stores in one class or with two clients for the same store, but using different end points.

Is it okay to override the CDI @Named to refer to things that aren't a bean name? Currently, our Producer, class constructor, is passed the @ClientProfile.lookup() + @ClientProfile.profile(). If we could get the same information passed on the @Inject, that would be better.

Also, looking at one of the unit tests, I also wonder how we could introduce Read/WriteConcern settings to be passed to the Producer methods as well?

[scottmarlow]

Does that work means that all applications will depend on CDI or only if your subsystem detect @ClientProfile?

Currently, application beans need the @ClientProfile to choose the connection profile to Inject. If we can move the choice of NoSQL profile, into the @Inject, that would be better as pointed out above. Sorry for answering these questions out of order.

[gunnarmorling]

Would this work:

1. Produce a CDI bean for each connection configured in standalone.xml, using the id given there as bean name
2. Export each connection configured in standalone.xml to JNDI, using the JNDI name given there

With that, 1. should allow the user to obtain the connections using plain @Inject. If there are several connections of the same type, that ambiguity would have to be resolved using @Named.

And 2. should allow Hibernate OGM to obtain the connection, provided the user has configured the JNDI name in persistence.xml. We could piggy-back on <jta-data-source>, although "JTA" is a bit of a misnomer here. So we might as well have an OGM-specific property key.

In other words, there would be no new annotation ClientProfile.

Thoughts?

[scottmarlow]

Gunnar, there might be a way to eliminate ClientProfile without 1, I'm just not really sure. I like suggestion 1, which I think we should consider. Perhaps I'm wrong about @Named or there is a different way for the user to pass the profile id, to the InjectionTarget (through the Extension). Does having a CDI bean per configured connection (1), give us a way to pass things like MongoDB write-concern in to get a connection with slightly different configuration? I feel like a newbie that is just not seeing enough of the CDI light yet.

@antoinesd currently, our NoSQL extensions are expecting user code to specify a @ClientProfile, so the user can pass in a JNDI name or NoSQL connection id, which our Extension passes into a InjectionTarget, so the underlying NoSQL connection can be easily found. For example in action, see ClientResource test code here. My question for you is, can the @Inject pass string values into the Extension or InjectionTarget, that specify which JNDI or NoSQL connection id, to use for obtaining the NoSQL connection?

We might also want to pass other information in, such as a string representing custom connection properties (such as MongoDB write-concern), so that user code can declare certain MongoDatabase overrides, that are equivalent of injecting a MongoDatabase and calling MongoDatabase.withWriteConcern(WriteConcern writeConcern), which returns a new MongoDatabase with the specified override for write-concern. Other attributes might also fit in this way. Perhaps we can not adding custom write-concern support in this fashion, but I'd still like to understand how to do it. Once I understand what we can't do, then we can consider workarounds like Gunnar's #1 above, which I also like.

[scottmarlow]

Some example standalone.xml config + test code that would show what #1 could look like in use:

<!-- MongoDB configuration in standalone.xml, global bean name will be "org.mycompany.partsdatabase" -->
<subsystem xmlns="urn:jboss:domain:mongodb:1.0">
   <mongo name="default" id="org.mycompany.partsdatabase" jndi-name="java:jboss/mongodb/test" database="partsdatabase">
      <host name="default" outbound-socket-binding-ref="mongopartshost"/>
   </mongo>
</subsystem>

@Inject 
@Named("org.mycompany.partsdatabase") 
MongoDatabase database;

[scottmarlow]

in the interest of full disclosure. We are currently scanning application deployments to see which NoSQL profiles are used, so we can automatically inject the needed NoSQL driver modules into the deployment. We are currently scanning for @Resource and checking if Resource.lookup matches a known NoSQL jndi name. We also scan for application use of @ClientProfile, to match the specified NoSQL profile name. If we change to #1, we can scan for @Named instead of @ClientProfile, which would also help us discover which NoSQL driver dependencies to auto add to the deployment.

[scottmarlow]

I'm still liking Gunnar's @Named suggestion, which kind of logically treats the WildFly NoSQL profile id as like a bean name.

I see no reason why the current @ClientProfile really needs the (JNDI) "lookup" property, so if we are currently happy with all CDI @Inject being done with @Named, we can just delete the ClientProfile class.

Internally, WildFly can still use JNDI for non-CDI uses, which are currently @Resource + OGM.

Any arguments against deleting @ClientProfile and instead using @Named?

[gunnarmorling]

+1 Sounds good :)

[scottmarlow]

Currently, we are creating a small number of NoSQL integration beans per deployment, one per producer factory (e.g. Cassandra ClusterProducerFactory/SessionProducerFactory). I think that with this change, we will create many more beans per deployment (something like: number of profiles * number of producer factories).

This new approach still sounds like the better approach, I just wonder if we could/should share the NoSQL integration beans globally, so there is less overhead.

[gunnarmorling]

How many beans would that be though realistically? Usually you don't configure dozens of datasources, and most of the time just one or two beans are to be created per datasource.

share the NoSQL integration beans globally

WDYM by "globally" exactly?

[scottmarlow]

How many beans would that be though realistically? Usually you don't configure dozens of datasources, and most of the time just one or two beans are to be created per datasource.

I think that the one or two beans per datasource, would also be per deployment that uses NoSQL (assuming we can be selective about which deployments to create the beans for. This may not be a blocker but sounds a bit bloated, especially if there are a lot of datasources defined.

WDYM by "globally" exactly?

By globally, I meant shared by all applications within the app server instance, which is not a correct description of what we are currently doing in our NoSQL Extension's. We are currently creating a small number of producer beans (e.g. ClusterProducerFactory/SessionProducerFactory) per deployment. I think that the per deployment bean count will be multiplied out, by the number of profiles defined.

[scottmarlow]

Switched from @ClientProfile to @Named! :-) Thanks for the suggestions @gunnarmorling + @emmanuelbernard!

Did I specify @Named for the created bean correctly? I changed the BeanAttributes.getQualifiers() implementation to use @Named but no idea if this is the correct way.

[gunnarmorling]

Some testing should bring clarity? E.g. you could define two connections of the same type, and then

• Assert on proper injection via @Inject @Named("my-connection")
• Assert on an ambiguous injection error when only giving @Inject

[gunnarmorling]

We are currently creating a small number of producer beans (e.g. ClusterProducerFactory/SessionProducerFactory) per deployment

Scoping the connections per deployment seems sensible to me. Otherwise one application might exhaust e.g. the internal pool of a MongoDB connection, starving another application.

But then people tend to deploy only one app per server anyways these days... ;)

[scottmarlow]

Scoping the connections per deployment seems sensible to me. Otherwise one application might exhaust e.g. the internal pool of a MongoDB connection, starving another application.

Currently, the underlying NoSQL connections are shared between all applications, via WildFly services (CassandraClientConnectionsService, MongoClientConnectionsService, Neo4jClientConnectionService). The per deployment CDI beans (created by our NoSQL Extension's) are using the shared connections but are not establishing those connections.

Any concerns about sharing the NoSQL connections between separate deployments? issue#2, brings up the problem of applications closing the shared MongoDB connection, which we haven't solved yet. Any other issues, perhaps where application state could leak into the shared connection somehow?

[scottmarlow]

Some testing should bring clarity? E.g. you could define two connections of the same type, and then

Assert on proper injection via @Inject @Named("my-connection")
Assert on an ambiguous injection error when only giving @Inject

One current issue with only giving @Inject (no @Named qualifier), is that I only scanned for the @Named annotation, which means that naked @Inject doesn't currently do much (deployment gets MongoDB CNFE, since we didn't magically add MongoDB driver classloader to app deployment).

I'll open a separate issue for that. I think this issue can be closed now as ClientProfile is gone.