On Tue, Aug 26, 2014 at 3:17 PM, Sanne Grinovero <[email protected]> wrote: > On 26 August 2014 19:38, William Burns <[email protected]> wrote: >> On Tue, Aug 26, 2014 at 2:23 PM, Sanne Grinovero <[email protected]> >> wrote: >>> On 26 August 2014 18:38, William Burns <[email protected]> wrote: >>>> On Mon, Aug 25, 2014 at 3:46 AM, Dan Berindei <[email protected]> >>>> wrote: >>>>> >>>>> >>>>> >>>>> On Mon, Aug 25, 2014 at 10:26 AM, Galder Zamarreño <[email protected]> >>>>> wrote: >>>>>> >>>>>> >>>>>> On 15 Aug 2014, at 15:55, Dan Berindei <[email protected]> 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). >>>>>> > >>>>>> > 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 think something like this would be the simplest for now especially, >>>> how this is done though we can still decide. >>>> >>>>>> >>>>>> Not sure you need the listener event since we already have lifecycle >>>>>> event >>>>>> callbacks for external modules. >>>>>> >>>>>> IOW, couldn’t you do this cache stop ordering with an implementation of >>>>>> org.infinispan.lifecycle.ModuleLifecycle? On cacheStarting, you could >>>>>> maybe >>>>>> track each started cache and give it a priority, and then on >>>>>> cacheManagerStopping use that priority to close caches. Note: I’ve not >>>>>> tested this and I don’t know if the callbacks happen at the right time to >>>>>> allow this. Just thinking out loud. >>>> >>>> +1 this is a nice use of what is already in place. The only issue I >>>> see here is that there is no ordering of the lifecycle callbacks if >>>> you had more than 1 callback, which could cause issues if users wanted >>>> to reference certain caches. >>>> >>>>>> >>>>> >>>>> Unfortunately ModuleLifecycle.cacheManagerStopping is only called _after_ >>>>> all the caches have been stopped. >>>> >>>> This seems like a bug, not very nice for ordering of callback methods. >>>> >>>>> >>>>> >>>>>> >>>>>> Cheers, >>>>>> >>>>>> > >>>>>> > Cheers >>>>>> > Dan >>>>>> > >>>>>> > >>>>>> > >>>>>> > On Fri, Aug 15, 2014 at 3:29 PM, Sanne Grinovero <[email protected]> >>>>>> > 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. >>>> >>>> Unfortunately this won't work except in a simple dependency case (you >>>> depend on a cache, but no cache can depend on you). >>>> >>>> Say you have 3 caches (C1, C2, C3). >>>> >>>> The case is C2 depends on C1 and C3 depends on C2. In this case both >>>> C1 and C2 would have a ref count value of 1 and C3 would have 0. This >>>> would allow for C1 and C2 to both be eligible to be closed during the >>>> same iteration. >>> >>> Yea people could use it the wrong way :-D >> >> Oh I agree, but it seems this could be a pretty simple case that a >> user may not know the consequences of. The problem with this you have >> to know the dependencies of the cache you are depending on as well, >> which I don't think most users would want. It would be fine if only >> Infinispan used it internally, or at least it should :) > > Right, I expect this to be used at SPI level: other frameworks > integrating still need a stable contract but that doesn't mean all of > the SPI needs to be easy, maybe documented only in an appendix, etc.. > > >>> But you can increment in different patterns than what you described to >>> model a full graph: the important point is to allow users to define an >>> order in *some* way. >>> >>> >>>> I think if we started doing dependencies we would really need to have >>>> some sort of graph to have anything more than the simple case. >>>> >>>> Do we know of other use cases where we may want a dependency graph >>>> explicitly? It seems what you want is solvable with what is in place, >>>> it just has a bug :( >>> >>> True, for my case a two-phases would be good enough *generally >>> speaking* as we don't expect people to index stuff in a Cache which is >>> also used to store an index for a different Cache, but that's a >>> "legal" configuration. >>> Applying Muprhy's law, that means someone will try it out and I'd >>> rather be safe about that. >>> >>> It just so happens that the counter proposal is both trivial and also >>> can handle a quite long ordering chain. >>> >>> I don't understand how it's solvable "with what's in place", could you >>> elaborate? >> >> I meant that you could use the ModuleLifecycle callback that Galder >> mentioned in the query module to close any caches that are needed >> before the manager starts shutting down others. However until the >> mentioned bug is fixed it won't quite work. When I said "with what is >> in place" I meant more that we wouldn't have to design a new >> implementation to support your use case. > > The pattern Galder suggested implies some kind of counting right ;-) > just that he suggests I could implement my own.
Not necessarily, could you not just when a cache starts check it's configuration and if it has indexing enabled keep a reference to it. Then when the cache manager is stopping you stop those caches before returning? > > But when you close my module (Query), it might be too late.. or too > early, as there might be other users of the index cache. So you're > saying I need to build this in the Lucene Directory module? > That doesn't work either as the Lucene Directory should not depend on > Query, nor it can tell which module is using it, but more importantly > this module isn't necessarily used exclusively by the Query module. This wouldn't be fired when the module is closed but rather as a notification that the cache manager is about to begin its stop sequence. > > So we're back to square one, i.e. some kind of general-purpose counting. > > -- Sanne > > _______________________________________________ > infinispan-dev mailing list > [email protected] > https://lists.jboss.org/mailman/listinfo/infinispan-dev _______________________________________________ infinispan-dev mailing list [email protected] https://lists.jboss.org/mailman/listinfo/infinispan-dev
