[jbosscache-dev] RE: Problems with DefaultCacheFactory

[Brian Stansberry]

Copying the dev list, which I should have done yesterday once I went far
beyond "Have you tagged Alpha2 yet/" :-)

Manik Surtani wrote:
> On 10 Jan 2007, at 04:21, Brian Stansberry wrote:
> 
>> Have you tagged Alpha2 yet?
>> 
> 
> Yes but I got this patch in.
>

Thanks much.  Lets me use it in the AS. :-)
 
>> DefaultCacheFactory is making it impossible to properly construct a
>> cache from the microcontainer.  Internally it's cloning the config,
>> and Configuration.clone() is resetting the RuntimeConfig. This makes
>> it impossible to build a Configuration+RuntimeConfig and then create
>> a cache from it. 
>> 
>> I don't think the factory should clone configs, or at least should
>> expose the flag that turns it off.  The comments around it indicate a
>> concern about different caches using the same Configuration and
>> stepping on each other.  IMHO, that's not a proper concern of the
>> factory -- that's something that needs to be managed by the caller
>> of the factory. 
>> 
>> TBH, I've kind of forgotten the purpose of the factory :-).  The
>> caches in the AS are all being instantiated by CacheJmxWrapper.  I
>> could easily replace the factory call there with one to new CacheImpl
>> (Configuration).
> 
> :-)  See http://jira.jboss.com/jira/browse/JBCACHE-930
> 
> We should force all cache and node instance creation to go
> through the factory, so we have a dedicated layer where all
> construction takes place. 
> 

OK. Definitely one way or the other is good -- factory but able to
directly construct was confusing. :P

>> But that won't work for the Tomcat cache, which uses PojoCache.
>> PojoCache internally is using the DefaultCacheFactory to build its
>> cache (Ben that's why I cc'd you.)
>> 
>> So, possibilities I see are:
>> 
>> 1) CacheJmxWrapper and PojoCache just call new CacheImpl
>> (Configuration). 2) DefaultCacheFactory doesn't clone.
>> 3) CacheFactory is changed to add a createCache method that exposes
>> whether to clone. 4) Configuration.clone() doesn't call reset() on
>> the RuntimeConfig. 5) RuntimeConfig.reset() only clears the
>> NodeFactory field (I don't see why the other fields it clears need
>> to be cleared). 
>> 
> 
> I've gone with 5 instead, for the time being.  Let's revisit
> and fix this properly for BETA1.
>

Sounds good.
 
> Just to clarify though, I was under the impression the
> runtime configuration was generated by the cache, by
> attaching various runtime subsystems that are built based on
> the configuration XML such as the TM.  The reason we exposed
> it is so that such systems could be overridden/injected at
> runtime by the MC.  So it would just need to be a sequence of events
> that change: 
> 
> 1) create cache
> 2) start cache
> 3) inject specific subsystems such as the TM
> 
> Does it not work this way with the MC?
> 

No, the MC builds up the entire Configuration structure and then injects
it into the bean that instantiates the cache (bean is either a
CacheFactory instance or an instance CacheJmxWrapper, which internally
uses the CacheFactory).

In order to have the runtime objects available when needed, the sequence
would need to be:

1) Create config bean x- runtime objects
2) Use config to instantiate cache bean
3) Inject runtime objects into config bean
4) Call create() on cache bean
5) Call start() on cache bean

I don't see how the MC could possibly manage that sequence, since it
requires weaving together operations on 2 different beans (really 3
beans, since step #2 has a factory involved.)  If we used
CacheJmxWrapper, and added properties to it for the runtime objects, it
could have that sequence hard coded, but that's a nasty hack.

> The reason why I want to make sure we reset RuntimeConfig and
> clone Configuration is if we have 2 cache instances in the
> same JVM, we may need cache-specific instances of more than just the
> NodeFactory. 
>

OK, that's fine, but

1) reset() should limit itself to objects that are clearly meant to be
cache-specific (which is what you did w/ #5 above).
2) I don't think cloning the configuration is the responsibility of the
CacheFactory.  It's the responsibility of the caller.  As we explore use
cases for reusing configurations, we may come up with some classes that
help manage configurations for people, e.g. a ConfigurationFactory
similar to JChannelFactory.  Those classes could be responsible for
cloning.

A problem here is TransactionManager, JChannelFactory and MBeanServer
are kind of neither fish nor fowl.  They can reasonably be injected into
the cache and IMHO should be if the environment supports that.  If the
environment doesn't support that, we have stuff like
TransactionManagerLookup to substitute for the missing IOC framework.

Another thing to consider is whether we want the cache to have the
ability to look up the multiplexer in JMX, or whether that becomes the
task of some other integration layer (e.g. CacheJmxWrapper) that updates
the Configuration and builds the cache.  If we remove that from the
cache then:

a) JChannelFactory is only available from external injection -- i.e. the
cache doesn't add it to its own Configuration.
b) Not sure, but I think we don't need MBeanServer anymore.

That would leave only TransactionManager as the "neither fish nor fowl"
type.

- Brian