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

Mon Aug 18 09:04:53 EDT 2014

as you prefer.. so can we get this soon please ?

-- Sanne

On 18 August 2014 13:46, Dan Berindei <dan.berindei at gmail.com> wrote:
>
>
>
> On Mon, Aug 18, 2014 at 1:56 PM, Sanne Grinovero <sanne at infinispan.org>
> wrote:
>>
>> On 18 August 2014 11:33, Dan Berindei <dan.berindei at 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.justin at 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 <sanne at infinispan.org> wrote:
>> >>
>> >> > On 15 August 2014 14:55, Dan Berindei <dan.berindei at 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..
>
>
> I don't see why we would need transactions for dependency graphs any more
> than we would need them for incRef.
>
>>
>>
>> > 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.
>
>
> AdvancedCache is still public API, so it still needs to be documented. I'm
> not sure Lucene is a good model here, I looked at Lucene's IndexReader
> documentation [1] and it doesn't look encouraging: the close javadoc says it
> "Closes files associated with this index", while the incRef javadoc says
> "Note that close() simply calls decRef()". I also didn't find any mention of
> what the initial reference count is.
>
> To be clear, I don't have anything against reference counting in general.
> But I don't think overloading the Lifecycle.stop() method to have a totally
> different behaviour in Cache is a good idea.
>
> [1]
> http://lucene.apache.org/core/4_0_0/core/org/apache/lucene/index/IndexReader.html
>
>>
>>
>> >> >> 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.
>
>
> Ok, fair enough.
>
>>
>> >
>> > 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.
>
>
> I think we are in that business, whether we like it or not.
>
>>
>> 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.
>
>
> How would that SPI look? And how would someone be able to provide a better
> implementation than our "trivial" implementation?
>
>
>>
>>
>> Sanne
>>
>> >
>> >> >
>> >> > -- Sanne
>> >> >
>> >> >>
>> >> >> Cheers
>> >> >> Dan
>> >> >>
>> >> >>
>> >> >>
>> >> >> On Fri, Aug 15, 2014 at 3:29 PM, Sanne Grinovero
>> >> >> <sanne at 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 at lists.jboss.org
>> >> >>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> >> >>
>> >> >>
>> >> >>
>> >> >> _______________________________________________
>> >> >> infinispan-dev mailing list
>> >> >> infinispan-dev at lists.jboss.org
>> >> >> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> >> > _______________________________________________
>> >> > infinispan-dev mailing list
>> >> > infinispan-dev at lists.jboss.org
>> >> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> >>
>> >>
>> >> _______________________________________________
>> >> infinispan-dev mailing list
>> >> infinispan-dev at lists.jboss.org
>> >> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> >
>> >
>> >
>> > _______________________________________________
>> > infinispan-dev mailing list
>> > infinispan-dev at lists.jboss.org
>> > https://lists.jboss.org/mailman/listinfo/infinispan-dev
>> _______________________________________________
>> infinispan-dev mailing list
>> infinispan-dev at lists.jboss.org
>> https://lists.jboss.org/mailman/listinfo/infinispan-dev
>
>
>
> _______________________________________________
> infinispan-dev mailing list
> infinispan-dev at lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/infinispan-dev