<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Aug 27, 2014 at 1:28 AM, Sanne Grinovero <span dir="ltr">&lt;<a href="mailto:sanne@infinispan.org" target="_blank">sanne@infinispan.org</a>&gt;</span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class=""><div class="h5">On 26 August 2014 20:29, William Burns &lt;<a href="mailto:mudokonman@gmail.com">mudokonman@gmail.com</a>&gt; wrote:<br>


&gt; On Tue, Aug 26, 2014 at 3:17 PM, Sanne Grinovero &lt;<a href="mailto:sanne@infinispan.org">sanne@infinispan.org</a>&gt; wrote:<br>
&gt;&gt; On 26 August 2014 19:38, William Burns &lt;<a href="mailto:mudokonman@gmail.com">mudokonman@gmail.com</a>&gt; wrote:<br>
&gt;&gt;&gt; On Tue, Aug 26, 2014 at 2:23 PM, Sanne Grinovero &lt;<a href="mailto:sanne@infinispan.org">sanne@infinispan.org</a>&gt; wrote:<br>
&gt;&gt;&gt;&gt; On 26 August 2014 18:38, William Burns &lt;<a href="mailto:mudokonman@gmail.com">mudokonman@gmail.com</a>&gt; wrote:<br>
&gt;&gt;&gt;&gt;&gt; On Mon, Aug 25, 2014 at 3:46 AM, Dan Berindei &lt;<a href="mailto:dan.berindei@gmail.com">dan.berindei@gmail.com</a>&gt; wrote:<br>
&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt; On Mon, Aug 25, 2014 at 10:26 AM, Galder Zamarreño &lt;<a href="mailto:galder@redhat.com">galder@redhat.com</a>&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt; wrote:<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; On 15 Aug 2014, at 15:55, Dan Berindei &lt;<a href="mailto:dan.berindei@gmail.com">dan.berindei@gmail.com</a>&gt; wrote:<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; It looks to me like you actually want a partial order between caches on<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; shutdown, so why not declare an explicit dependency (e.g.<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; manager.stopOrder(before, after)? We could even throw an exception if the<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; user tries to stop a cache manually in the wrong order (e.g.<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; TestingUtil.killCacheManagers).<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; Alternatively, we could add an event CacheManagerStopEvent(pre=true) at<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; the cache manager level that is invoked before any cache is stopped, and you<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; could close all the indexes in that listener. The event could even be at the<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; cache level, if it would make things easier.<br>
&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt; I think something like this would be the simplest for now especially,<br>
&gt;&gt;&gt;&gt;&gt; how this is done though we can still decide.<br>
&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; Not sure you need the listener event since we already have lifecycle event<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; callbacks for external modules.<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; IOW, couldn’t you do this cache stop ordering with an implementation of<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; org.infinispan.lifecycle.ModuleLifecycle? On cacheStarting, you could maybe<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; track each started cache and give it a priority, and then on<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; cacheManagerStopping use that priority to close caches. Note: I’ve not<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; tested this and I don’t know if the callbacks happen at the right time to<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; allow this. Just thinking out loud.<br>
&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt; +1 this is a nice use of what is already in place.  The only issue I<br>
&gt;&gt;&gt;&gt;&gt; see here is that there is no ordering of the lifecycle callbacks if<br>
&gt;&gt;&gt;&gt;&gt; you had more than 1 callback, which could cause issues if users wanted<br>
&gt;&gt;&gt;&gt;&gt; to reference certain caches.<br>
&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt; Unfortunately ModuleLifecycle.cacheManagerStopping is only called _after_<br>
&gt;&gt;&gt;&gt;&gt;&gt; all the caches have been stopped.<br>
&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt; This seems like a bug, not very nice for ordering of callback methods.<br>
&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; Cheers,<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; Cheers<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; Dan<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; On Fri, Aug 15, 2014 at 3:29 PM, Sanne Grinovero &lt;<a href="mailto:sanne@infinispan.org">sanne@infinispan.org</a>&gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; wrote:<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; The goal being to resolve ISPN-4561, I was thinking to expose a very<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; simple reference counter in the AdvancedCache API.<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; As you know the Query module - which triggers on indexed caches - can<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; use the Infinispan Lucene Directory to store its indexes in a<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; (different) Cache.<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; When the CacheManager is stopped, if the index storage caches are<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; stopped first, then the indexed cache is stopped, this might need to<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; flush/close some pending state on the index and this results in an<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; illegal operation as the storate is shut down already.<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; We could either implement a complex dependency graph, or add a method<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; like:<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt;   boolean incRef();<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; on AdvancedCache.<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt;<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; when the Cache#close() method is invoked, this will do an internal<br>
&gt;&gt;&gt;&gt;&gt;&gt;&gt; &gt; decrement, and only when hitting zero it will really close the cache.<br>
&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt; Unfortunately this won&#39;t work except in a simple dependency case (you<br>
&gt;&gt;&gt;&gt;&gt; depend on a cache, but no cache can depend on you).<br>
&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt; Say you have 3 caches (C1, C2, C3).<br>
&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt; The case is C2 depends on C1 and C3 depends on C2.  In this case both<br>
&gt;&gt;&gt;&gt;&gt; C1 and C2 would have a ref count value of 1 and C3 would have 0.  This<br>
&gt;&gt;&gt;&gt;&gt; would allow for C1 and C2 to both be eligible to be closed during the<br>
&gt;&gt;&gt;&gt;&gt; same iteration.<br>
&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt; Yea people could use it the wrong way :-D<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; Oh I agree, but it seems this could be a pretty simple case that a<br>
&gt;&gt;&gt; user may not know the consequences of.  The problem with this you have<br>
&gt;&gt;&gt; to know the dependencies of the cache you are depending on as well,<br>
&gt;&gt;&gt; which I don&#39;t think most users would want.  It would be fine if only<br>
&gt;&gt;&gt; Infinispan used it internally, or at least it should :)<br>
&gt;&gt;<br>
&gt;&gt; Right, I expect this to be used at SPI level: other frameworks<br>
&gt;&gt; integrating still need a stable contract but that doesn&#39;t mean all of<br>
&gt;&gt; the SPI needs to be easy, maybe documented only in an appendix, etc..<br>
&gt;&gt;<br>
&gt;&gt;<br>
&gt;&gt;&gt;&gt; But you can increment in different patterns than what you described to<br>
&gt;&gt;&gt;&gt; model a full graph: the important point is to allow users to define an<br>
&gt;&gt;&gt;&gt; order in *some* way.<br>
&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt; I think if we started doing dependencies we would really need to have<br>
&gt;&gt;&gt;&gt;&gt; some sort of graph to have anything more than the simple case.<br>
&gt;&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt;&gt; Do we know of other use cases where we may want a dependency graph<br>
&gt;&gt;&gt;&gt;&gt; explicitly?  It seems what you want is solvable with what is in place,<br>
&gt;&gt;&gt;&gt;&gt; it just has a bug :(<br>
&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt; True, for my case a two-phases would be good enough *generally<br>
&gt;&gt;&gt;&gt; speaking* as we don&#39;t expect people to index stuff in a Cache which is<br>
&gt;&gt;&gt;&gt; also used to store an index for a different Cache, but that&#39;s a<br>
&gt;&gt;&gt;&gt; &quot;legal&quot; configuration.<br>
&gt;&gt;&gt;&gt; Applying Muprhy&#39;s law, that means someone will try it out and I&#39;d<br>
&gt;&gt;&gt;&gt; rather be safe about that.<br>
&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt; It just so happens that the counter proposal is both trivial and also<br>
&gt;&gt;&gt;&gt; can handle a quite long ordering chain.<br>
&gt;&gt;&gt;&gt;<br>
&gt;&gt;&gt;&gt; I don&#39;t understand how it&#39;s solvable &quot;with what&#39;s in place&quot;, could you<br>
&gt;&gt;&gt;&gt; elaborate?<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; I meant that you could use the ModuleLifecycle callback that Galder<br>
&gt;&gt;&gt; mentioned in the query module to close any caches that are needed<br>
&gt;&gt;&gt; before the manager starts shutting down others.  However until the<br>
&gt;&gt;&gt; mentioned bug is fixed it won&#39;t quite work.  When I said &quot;with what is<br>
&gt;&gt;&gt; in place&quot; I meant more that we wouldn&#39;t have to design a new<br>
&gt;&gt;&gt; implementation to support your use case.<br>
&gt;&gt;<br>
&gt;&gt; The pattern Galder suggested implies some kind of counting right ;-)<br>
&gt;&gt; just that he suggests I could implement my own.<br>
&gt;<br>
&gt; Not necessarily, could you not just when a cache starts check it&#39;s<br>
&gt; configuration and if it has indexing enabled keep a reference to it.<br>
&gt; Then when the cache manager is stopping you stop those caches before<br>
&gt; returning?<br>
<br>
</div></div>I suspect that if we allow extension points to shut down other<br>
components eagerly it&#39;s going to be a mess.<br>
I can&#39;t reliably track if and which other components (and user code)<br>
are still using it?<br>
<div class=""><br></div></blockquote><div><br></div><div style="font-family:arial,sans-serif;font-size:13px">We know that user code isn&#39;t using it any more because the user called CacheManager.stop(). If there are other threads accessing caches in the cache manager, the user should expect those operations to fail.</div>

<div class="im" style="font-family:arial,sans-serif;font-size:13px"><div></div></div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">

<div class="">
<br>
&gt;&gt; But when you close my module (Query), it might be too late.. or too<br>
&gt;&gt; early, as there might be other users of the index cache. So you&#39;re<br>
&gt;&gt; saying I need to build this in the Lucene Directory module?<br>
&gt;&gt; That doesn&#39;t work either as the Lucene Directory should not depend on<br>
&gt;&gt; Query, nor it can tell which module is using it, but more importantly<br>
&gt;&gt; this module isn&#39;t necessarily used exclusively by the Query module.<br>
&gt;<br>
&gt; This wouldn&#39;t be fired when the module is closed but rather as a<br>
&gt; notification that the cache manager is about to begin its stop<br>
&gt; sequence.<br>
<br>
</div>So I would eagerly shut down an indexed cache from a module which<br>
might have been started as its dependant (or not).<br>
<br>
What about other &quot;in flight&quot; operations happening on that Cache, we<br>
let them blow up even if technically we didn&#39;t shut down yet?<br>
<br>
Sorry for playing devil&#39;s advocate, but it seems very wrong.<br></blockquote><div><br></div><div style="font-family:arial,sans-serif;font-size:13px">It wouldn&#39;t be any different from now - stopping the cache manager stops all the caches, we&#39;d just change the order. Transactional caches would wait for in-flight transactions to finish, non-transactional caches would not wait.</div>

<div style="font-family:arial,sans-serif;font-size:13px"> </div><div><span style="font-family:arial,sans-serif;font-size:13px">It&#39;s true that this approach does not compose very well. But as Will showed in a prior email, your reference counting proposal doesn&#39;t compose well either. So we either do a quick fix that supports only one level of dependencies, or we bite the bullet and support a full dependency graph.</span></div>

<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
BTW I omitted it so far as &quot;execution complexity&quot; shouldn&#39;t be an<br>
excuse for an inferior solution, but it&#39;s worth to keep in mind that<br>
some of these resources are managed by Hibernate Search and you really<br>
can&#39;t introduce hard dependencies to it so the solution we&#39;re aiming<br>
at needs to be built and supported by infinispan-core exclusively,<br>
which has to expose this as an SPI.<br></blockquote><div><br></div><div><span style="font-family:arial,sans-serif;font-size:13px">I don&#39;t see the need for an additional SPI, ModuleLifecycle should be enough.</span></div>

<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
That said, I just wanted to explain the problem and propose a solution<br>
but have no longer time for this so any volunteer taking it?<br>
<a href="https://issues.jboss.org/browse/ISPN-4561" target="_blank">https://issues.jboss.org/browse/ISPN-4561</a><br>
<br>
TiA<br>
<div class=""><div class="h5"><br>
-- Sanne<br>
<br>
_______________________________________________<br>
infinispan-dev mailing list<br>
<a href="mailto:infinispan-dev@lists.jboss.org">infinispan-dev@lists.jboss.org</a><br>
<a href="https://lists.jboss.org/mailman/listinfo/infinispan-dev" target="_blank">https://lists.jboss.org/mailman/listinfo/infinispan-dev</a></div></div></blockquote></div><br></div></div>