[infinispan-dev] Unit test for ISPN-3698 and more workarounds...

[Galder Zamarreño]

On Nov 13, 2013, at 6:21 PM, Paul Ferraro <paul.ferraro at redhat.com> wrote:

> Excellent.
> 
> I think this will do in the short term, but in the long term, I think we
> need to revisit some of the finer points of the cache manager lifecycle,
> especially within the context of Wildfly/EAP.
> 
> In Wildfly/EAP, 2 deployment thread can easily trigger a case where a
> cache manager is restarted using the same GlobalConfiguration instance.
> 
> The root of the problem, I think, is that the GlobalConfiguration and
> cache Configuration objects currently have references to
> stateful/runtime objects like VersionAwareMarshaller.  I think this is a
> bad idea.  Ideally, the GlobalConfiguration object would simply contain
> a factory for creating instances of the marshaller.  

^ Hmmm, that's what the configuration builders are there for. Whenever you want to create a new cache/cache manager, the normal thing is to create brand new builder classes, call build() on them, and then pass it to the cache/cachemanager constructor. 

If you do that, you'd have no problems. The only issue I see is having to create these builders all the time (time + temporary memory consumption), as opposed to having more finer grained factory methods in the configuration builder objects.

+1 to having factory methods in configuration builder objects.

-1 to having factory methods in configuration objects.

> The marshaller
> instance would only be instantiated as part of the cache/cache manager
> startup process.  This way, the same GlobalConfiguration object can be
> use to create any number of cache managers.  That would make Wildfly/EAP
> very happy.

I thought that you'd reuse the global/configuration builders for creating different cache/cache manager instances, and not configuration objects. Having looked at it again, the marshaller in the builder gets passed to the configuration object, and that goes to the cache, so even sharing builder classes won't get around your issue. The only workaround that'd work is recreating builder objects every time.

I know that DefaultCacheManager has constructors for Configuration and GlobalConfiguration instances, but I would have expect these to always come from *Builder.build() calls, not from other cache/cachemanagers. Builder classes should be sharable without problems.

Definitely something to investigate for Infinispan 7 [1]

Cheers,

[1] https://issues.jboss.org/browse/ISPN-3716

> 
> Paul
> 
> On Wed, 2013-11-13 at 10:49 +0100, Galder Zamarreño wrote:
>> On Nov 13, 2013, at 10:09 AM, Galder Zamarreño <galder at redhat.com> wrote:
>> 
>>> Hey Paul,
>>> 
>>> I've got a unit test for ISPN-3698 [1] [2].
>>> 
>>> The test hangs when in cm1.getCache() at the end of the test. If you look at the logs you'll find a load of NPEs like the one in the JIRA.
>>> 
>>> If this is what you are doing, another thing that should work fine is essentially rebuild the configurations when you start a new cache manager and avoid using the configuration objects of a stopped cache manager. 
>>> 
>>> The key thing here is that if you create a new instance of GlobalConfigurationBuilder when you redeploy, it'll automatically create a brand new instance of VersionAwareMarshaller, and that would get around the issue.
>>> 
>>> As long as the new global configuration has a reference to the old VersionAwareMarshaller, you have the potential for the NPE (unless you do some trick to reset VersionAwareMarshaller's defaultMarshaller instance).
>>> 
>>> WRT the fix I proposed it yesterday, I've run it through our testsuite and a fair few failures appear (~60), so think whether you can get around it with a workaround like the one explained in this email. In the mean time I'm trying to find a better fix within Infinispan itself.
>> 
>> ^ The fix had some issues with those tests that rewire components, but I've fixed that now and I'll send a PR right away.
>> 
>> Cheers,
>> 
>>> 
>>> Cheers,
>>> 
>>> [1] https://issues.jboss.org/browse/ISPN-3698
>>> [2] https://gist.github.com/9d5ab3623f57ec8b23ee
>>> --
>>> Galder Zamarreño
>>> galder at redhat.com
>>> twitter.com/galderz
>>> 
>>> Project Lead, Escalante
>>> http://escalante.io
>>> 
>>> Engineer, Infinispan
>>> http://infinispan.org
>>> 
>>> 
>>> _______________________________________________
>>> infinispan-dev mailing list
>>> infinispan-dev at lists.jboss.org
>>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> 
>> 
>> --
>> Galder Zamarreño
>> galder at redhat.com
>> twitter.com/galderz
>> 
>> Project Lead, Escalante
>> http://escalante.io
>> 
>> Engineer, Infinispan
>> http://infinispan.org
>> 
> 
> 


--
Galder Zamarreño
galder at redhat.com
twitter.com/galderz

Project Lead, Escalante
http://escalante.io

Engineer, Infinispan
http://infinispan.org