Re: [infinispan-dev] Caches need be stopped in a specific order to respect cross-cache dependencies

Ales, I don't think the implementation matters that much, I was only concerned about the API. BTW, where could I find some documentation on MSC? Sanne, I missed something in your initial email: you mention a Cache.close() method, did you mean Cache.stop(), or did you mean to add a new close() method?

Cache doesn't actually define a stop() method, it inherits the stop() method from the Lifecycle interface. So changing its semantics only for caches would be hacky. Adding a different close() method would be better, but it still wouldn't be my first choice... On Sat, Aug 16, 2014 at 12:40 AM, Ales Justin <ales.justin(a)gmail.com&gt; wrote: > > What about if you add an SPI for this? > > e.g. this could be nicely implemented on top of WildFly's MSC > > And by default you would keep this simple incRef, > or some similar simple state machine we used in Microcontainer. > > -Ales > > On 15 Aug 2014, at 16:26, Sanne Grinovero <sanne(a)infinispan.org&gt; wrote: > > > On 15 August 2014 14:55, Dan Berindei <dan.berindei(a)gmail.com&gt; wrote: > >> It looks to me like you actually want a partial order between caches on > >> shutdown, so why not declare an explicit dependency (e.g. > >> manager.stopOrder(before, after)? We could even throw an exception if > >> the > >> user tries to stop a cache manually in the wrong order (e.g. > >> TestingUtil.killCacheManagers). > > > > Because that's much more complex to implement? > > incRef() seems trivial, effective and can be used by other components > > for different patterns. Implementing proper dependencies doesn't need to be difficult either, all we need is to keep a list of dependants in the cache and prune the stopped caches from it before doing the check.

incRef might be easier to implement, but instead it seems harder to explain to a user in the Javadoc.

> >> Alternatively, we could add an event CacheManagerStopEvent(pre=true) at > >> the > >> cache manager level that is invoked before any cache is stopped, and > >> you > >> could close all the indexes in that listener. The event could even be > >> at the > >> cache level, if it would make things easier. > > > > I like that more than defining explicit dependency links and it would > > probably be good enough for this specific problem > > but I feel like it doesn't solve similar problems with a more complex > > dependency sequence of services. > > Counters are effectively providing the same semantics, just that you > > can use the pre-close pattern nesting it "count times". > > > > Also having ref-counting available makes it easier for users to > > implement independent components - with an independent lifecycle - > > which might share the same cache. By independent components do you mean global components? That wouldn't work, since we only start stopping global components after we have stopped all the caches - regardless of the order in which we stop caches.

A global pre-stop event, instead, would allow global components to do stuff before any of the caches is stopped.

> > > > -- Sanne > > > >> > >> Cheers > >> Dan > >> > >> > >> > >> On Fri, Aug 15, 2014 at 3:29 PM, Sanne Grinovero <sanne(a)infinispan.org&gt; > >> wrote: > >>> > >>> The goal being to resolve ISPN-4561, I was thinking to expose a very > >>> simple reference counter in the AdvancedCache API. > >>> > >>> As you know the Query module - which triggers on indexed caches - can > >>> use the Infinispan Lucene Directory to store its indexes in a > >>> (different) Cache. > >>> When the CacheManager is stopped, if the index storage caches are > >>> stopped first, then the indexed cache is stopped, this might need to > >>> flush/close some pending state on the index and this results in an > >>> illegal operation as the storate is shut down already. > >>> > >>> We could either implement a complex dependency graph, or add a method > >>> like: > >>> > >>> > >>> boolean incRef(); > >>> > >>> on AdvancedCache. > >>> > >>> when the Cache#close() method is invoked, this will do an internal > >>> decrement, and only when hitting zero it will really close the cache. > >>> > >>> A CacheManager shutdown will loop through all caches, and invoke > >>> close() on all of them; the close() method should return something so > >>> that the CacheManager shutdown loop understand if it really did close > >>> all caches or if not, in which case it will loop again through all > >>> caches, and loops until all cache instances are really closed. > >>> The return type of "close()" doesn't necessarily need to be exposed on > >>> public API, it could be an internal only variant. > >>> > >>> > >>> Could we do this? > >>> > >>> --Sanne > >>> _______________________________________________ > >>> infinispan-dev mailing list > >>> infinispan-dev(a)lists.jboss.org > >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev > >> > >> > >> > >> _______________________________________________ > >> infinispan-dev mailing list > >> infinispan-dev(a)lists.jboss.org > >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > _______________________________________________ > > infinispan-dev mailing list > > infinispan-dev(a)lists.jboss.org > > https://lists.jboss.org/mailman/listinfo/infinispan-dev > > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev(a)lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list infinispan-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev