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

[Sanne Grinovero]

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..

> 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 <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