[infinispan-dev] ISPN-2314 fix looks fishy

[Galder Zamarreño]

On Sep 20, 2012, at 11:05 AM, Galder Zamarreño <galder at redhat.com> wrote:

> 
> On Sep 19, 2012, at 11:57 AM, Manik Surtani <manik at jboss.org> wrote:
> 
>> 
>> On 19 Sep 2012, at 10:20, Galder Zamarreño <galder at redhat.com> wrote:
>> 
>>> Hi guys,
>>> 
>>> Having briefly investigated https://issues.jboss.org/browse/ISPN-2314, I consider the fix (https://github.com/infinispan/infinispan/pull/1318) to be rather fishy.
>>> 
>>> Sure it works, but it masks the fact that the order in which things are started has changed (This IMO seems to be the result of http://issues.jboss.org/browse/ISPN-2256). 
>>> 
>>> Not only that, but the fact that you have to modify both the component registry and the configuration makes me think that there're situations where the configuration changes are applied, and others where it's not (hence why you have to change the component registry).
>>> 
>>> So, can we please stop and try to understand what really is happening here? I was expecting Mircea to have a look into a fix for this since it looks related to cache start order.
>>> 
>>> Personally, I don't really see why the lifecycle manager has to mess up with the component registry. Changes to the configuration were 'enough' (until 2256) and I don't see why that should change.
>> 
>> It appears that the cache is already constructed when the cache starting callback is issued.  This means any changes to the configuration at this point won't have any effect.  
> 
> ^ This is precisely my point Manik, it didn't used to be like this, and I'm yet to hear a reason why this should change.
> 
> Looks to me it's a side effect of ISPN-2256 without explanation, hence my pull back on the fix.
> 
>> But changing the configuration is still necessary to allow for restarts.
> 
> +1 - that's precisely what I had done by fixing the query cache restarts (see lSPN-2254). I had achieved a single way of dealing with both starts and restarts, but you changed that.

Sorry, meant ISPN-2232:
https://github.com/infinispan/infinispan/commit/01f36b823acabfa3ba5e31886885e3d583ebb2fa

> 
>> 
>>> 
>>> Cheers,
>>> --
>>> 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
>> 
>> --
>> Manik Surtani
>> manik at jboss.org
>> twitter.com/maniksurtani
>> 
>> Platform Architect, JBoss Data Grid
>> http://red.ht/data-grid
>> 
>> _______________________________________________
>> 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
> 
> 
> _______________________________________________
> 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