jboss / jboss-nosql

3 stars 0 forks source link

Is it fine for Cassandra CDI integration to use @Dependent scope #4

Open emmanuelbernard opened 8 years ago

emmanuelbernard commented 8 years ago

hey @antoinesd we need your advice on CDI scoping.

Cassandra's entry poitn is the class Cluster which cannot be proxied by Weld. These kind of global access beans would usually be ApplicationScope but @scottmarlow worked around the proxying issue by using @Dependent scope. See https://github.com/scottmarlow/wildfly/blob/nosql-dev9/nosql/cassandra/src/main/java/org/wildfly/extension/nosql/cdi/CassandraExtension.java

That raises a few questions to me

@scottmarlow How is the Cluster object retrieved? I got lost after ConnectionServiceAccess. Is that some JNDI lookup of an existing object or is that always creating a new Cluster object for each call.

If it is creating a new object every time, then I think Dependent is not an acceptable choice. @antoinesd What is the workaround for non proxyable classes in CDI. We know Cluster has no further CDI bean links to narrower scopes.

If it is a simple lookup, then do you see any harm is using dependent @antoinesd? Furthermore, why shouldn't all NoSQL extensions use dependent, it looks cleaner and safer and leave the module responsible for providing this object in JNDI to define the actual lifecycle.

Thoughts?

emmanuelbernard commented 8 years ago

For Session I am a bit more confused as a Session seems to imply a specific database name. How is that provided @scottmarlow ?

emmanuelbernard commented 8 years ago

Ok so it looks like these objects are created ones and stored in https://github.com/scottmarlow/wildfly/blob/nosql-dev9/nosql/cassandra/src/main/java/org/wildfly/extension/nosql/driver/cassandra/CassandraClientConnectionsService.java what is the scope/lifecycle of CassandraClientConnectionsService?

scottmarlow commented 8 years ago

@scottmarlow How is the Cluster object retrieved? I got lost after ConnectionServiceAccess. Is that some JNDI lookup of an existing object or is that always creating a new Cluster object for each call.

A globally shared (existing) Cluster object is held by CassandraClientConnectionsService and that is returned for each @Inject/Resource lookup.

If it is creating a new object every time, then I think Dependent is not an acceptable choice. @antoinesd What is the workaround for non proxyable classes in CDI. We know Cluster has no further CDI bean links to narrower scopes.

If it is a simple lookup, then do you see any harm is using dependent @antoinesd? Furthermore, why shouldn't all NoSQL extensions use dependent, it looks cleaner and safer and leave the module responsible for providing this object in JNDI to define the actual lifecycle.

I have no CDI expertise myself but thought that the disadvantage of @Dependent, might be that more memory is required for tracking the use of the injected instance, instead of a shared proxy but I'm not really sure if we should use @Dependent for all NoSQL extensions.

For Session I am a bit more confused as a Session seems to imply a specific database name. How is that provided @scottmarlow ?

For Session, the NoSQL source profile defined may include an optional keyspace name, in which case injecting into a Session variable will use the keyspace name. Injecting the same NoSQL source profile into a Cluster variable, will not use the keyspace name.

what is the scope/lifecycle of CassandraClientConnectionsService?

An instance of the CassandraClientConnectionsService service is created for each Cassandra NoSQL source profile and the scope is until the profile is removed or the app server stopped. For example, the following standalone.xml settings would create two (global) CassandraClientConnectionsService instances, one for "cassandratestprofile" and one for "scottdatabase". Each CassandraClientConnectionsService instance, could service multiple application deployments, or users could create separate profiles for each deployment.

        <subsystem xmlns="urn:jboss:domain:cassandra:1.0">
            <cassandra name="default" id="cassandratestprofile" jndi-name="java:jboss/cassandradriver/test">
                <host name="default" outbound-socket-binding-ref="casstesthost"/>
            </cassandra>
            <cassandra name="scott" id="scottdatabase" jndi-name="java:jboss/cassandradriver/scottdatabase">
                <host name="scott" outbound-socket-binding-ref="casstesthost"/>
            </cassandra>
        </subsystem>
antoinesd commented 8 years ago

When a bean is in the @Dependent scope a new instance is created for each injection point injecting this bean. The solution to avoid instance creation each time would be to inject your bean in a normal scope bean and use it to retrieve its instance which would have been created when this containing bean would have been created.

This solution is not enough in certain cases (bean passivation), so you may want to consider other solutions:

  1. You can inherit the class you want to use as a class bean and add a non private constructor with no args. This new class will have the type of its ancestor in its type set (so you'll be able to inject it in a field whose type is the ancestor type). To avoid any issue you have to make sure that the original class is not picked up as bean.
  2. Try the @Singleton scope which is not a normal scope and thus doesn't use proxies. It works until you start injecting normal scope beans in it. So if you don't need to inject other beans that may do the trick for you. Be careful when using @Singleton in the default annotated bean discovery mode, since it's not in the list of bean defining annotation (http://docs.jboss.org/cdi/spec/1.2/cdi-spec.html#bean_defining_annotations). So switch to all bean discovery mode or create a stereotype containing @Singleton to be sure that your bean will be discovered
  3. A non portable solution with Weld would be to use the relaxed construction mode (http://docs.jboss.org/weld/reference/latest/en-US/html_single/#relaxedConstruction). In your case you'll probably have to use a weld.properties to limit the effect to the using app. I'm not sure users won't have to put this config file in their war to make this work. Not very convenient then :-(

That's the first ideas that came to my mind regarding this limitation. Let me know if it solve your problem or if you need further help.

emmanuelbernard commented 8 years ago

Based onw what @scottmarlow is saying, the object is not created each time but retrieved from a shared location. So the @Dependent scope would really only cost us the lookup (done in the producer method of sort_ which is probably fine. @antoinesd would you recommend @Dependent in that case?

antoinesd commented 8 years ago

Yes, it's probably the simplest approach since it only cost the lookup. Also remember that if the user inject it in one of his long living bean the lookup will occurs only once (a dependent bean scope depend on the scope(s) of its consuming bean(s))

emmanuelbernard commented 8 years ago

Ah true @antoinesd but there is nothing one can do to force a lookup each time, right? I don't think I'm concerned about that anyways.

antoinesd commented 8 years ago

To force a lookup each time, user will do declare its own bean in dependent scope or use the programmatic lookup (Instance<> with a get() at each call for instance). So it will come from bad design or be done on purpose IMO.