On 18 August 2014 11:33, Dan Berindei <dan.berin...@gmail.com> wrote: > 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?
I meant stop(), sorry. > > 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.jus...@gmail.com> 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 <sa...@infinispan.org> wrote: >> >> > On 15 August 2014 14:55, Dan Berindei <dan.berin...@gmail.com> 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. I expect you or your team to do it, so your choice :-) I would also be careful in how you decide to spend a day(week?) vs 1h to provide a feature which is essentially the same stuff for the user. And if you go for dependency graphs, prepare to do it transactionally and concurrently.. > incRef might be easier to implement, but instead it seems harder to explain > to a user in the Javadoc. I didn't invent incRef myself, it's common in several other projects (Lucene for one), so I expect it to be a commonly understood pattern. Also I suggested to add it only on AdvancedCache, as I agree it's "advanced" users only. >> >> 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. I didn't meant to add this stopping feature to components, but that many other components might need an entangled sequence of shutdown of Caches. > > A global pre-stop event, instead, would allow global components to do stuff > before any of the caches is stopped. I haven't seen any need for such a thing so far. Your call, but I don't think we are in the business of service lifecycle management and dependency injection frameworks. Alesj is right: at best we should make this an SPI, provide a trivial implementation and leave the details to be handled by those who thought about it properly; just that the trivial counter is good enough for my needs. Sanne > >> > >> > -- Sanne >> > >> >> >> >> Cheers >> >> Dan >> >> >> >> >> >> >> >> On Fri, Aug 15, 2014 at 3:29 PM, Sanne Grinovero <sa...@infinispan.org> >> >> 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@lists.jboss.org >> >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev >> >> >> >> >> >> >> >> _______________________________________________ >> >> infinispan-dev mailing list >> >> infinispan-dev@lists.jboss.org >> >> https://lists.jboss.org/mailman/listinfo/infinispan-dev >> > _______________________________________________ >> > infinispan-dev mailing list >> > infinispan-dev@lists.jboss.org >> > https://lists.jboss.org/mailman/listinfo/infinispan-dev >> >> >> _______________________________________________ >> infinispan-dev mailing list >> infinispan-dev@lists.jboss.org >> https://lists.jboss.org/mailman/listinfo/infinispan-dev > > > > _______________________________________________ > infinispan-dev mailing list > infinispan-dev@lists.jboss.org > https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list infinispan-dev@lists.jboss.org https://lists.jboss.org/mailman/listinfo/infinispan-dev