-
15. Re: JBoss Cache Public API
"mircea.markus" wrote:
move throws an exception if either source or destination does not exist. This is inconsistent with the rest of the API, as it is the only method that trows an exception if nodes(s) are inexistent. E.g. removeNode(fqn) will return a false if the node to be added was not found.
Here are some suggestions for changing this:
1) return a false if either source or destination does not exist (no-op)
2) treat it as a remove followed by an put: remove the node (return false if does not exist) and if destination does not exist create it (put semantics).
I vote for 1, mainly because 2 is hard to explain to the user
You are correct, although I'd vote for 2. Like removeNode(), move() should return a success flag, returning false and not doing anything else if the source node does not exist. It should silently create the destination path if it does not exist just like put(). Not that hard to explain, we do so with put(). :-) -
16. Re: JBoss Cache Public API
Mircea - is there a JIRA for the changes to move()? Since we are changing pretty fundamental behaviour, should we also change the method name, just to make sure people know something is changing? E.g., moveNode(), to be more consistent with getNode() and removeNode() ?
-
17. Re: JBoss Cache Public API
"manik.surtani@jboss.com" wrote:
Re: lifecycle, don't people find it useful to create a cache, then set up stuff, such as register classloaders for regions, register listeners, etc., and then start?
Or should create() really be an initial state that is attained by calling CacheFactory.createCache()?
Yeah, I think that's the idea.And then what about stop() and destroy()? Does it not make sense that a cache, once stopped, could be started again - which implies that internal components should not be cleaned up/released?
Yes, absolutely.In which case isn't a destroy() necessasry to actually release resources and clean up?
Depends on whether they require some cleanup beyond garbage collection. Sounds like that's the thing to investigate.
Not sure if I was clear in my previous comment on this. Not saying we should definitely drop the 4 step lifecycle; just that the original reason for it no longer really applies, so worth looking into it. -
18. Re: JBoss Cache Public API
Just for some background on my comment on "create" being a legacy of the old JMX microkernel. Here's the content of the slide on create() from the JBoss Advanced course:
Create is called when all bean I depend on are "created"
When a service's create method is called
- This gives an MBean an opportunity to check that required MBeans or resources exist.
- The service typically cannot utilize other MBean services at this point, as most JBoss MBean services do not become fully functional until they have been started via their start method.
- Because of this, service implementations often do not implement create in favor of just the start method because that is the first point at which the service can be fully functional.
The key bit is the "gives an MBean an opportunity to check that required MBeans or resources exist" comment. The create method was really an added step to get around the lack of dependency injection in early JBoss. An MBean couldn't have another MBean injected. Instead it would have an ObjectName or some other handle injected, and in create() would look up the dependency. Proper IOC removes the need for this. -
19. Re: JBoss Cache Public API
Hmm, good points. I've created a JIRA to track this: http://jira.jboss.org/jira/browse/JBCACHE-1369
-
20. Re: JBoss Cache Public API
Bringing this back up again.
"bstansberry@jboss.com" wrote:
Depends on whether they require some cleanup beyond garbage collection. Sounds like that's the thing to investigate.
Consider a lot of the collections maintained by several components, like Notifiers, RegionManagers, etc. Usually these are uninitialized, are set on creation so that after creation, for example, regions and listeners can be created and added. Upon calling stop(), these are emptied, but the collections still exist so that more such elements could be added and the cache restarted. Destroy() on the other hand assigns the refs to these collections to null, so that the collections themselves can be gc()'d.
I guess the question to ask is how important is it to have these collections hanging around? Not a severe impact, I imagine, given that a ref to the cache still exists. -
21. Re: JBoss Cache Public API
IMHO if an application is concerned about that level of memory cleanup, they should just clear their refs to the cache after stopping it.
-
22. Re: JBoss Cache Public API
IMHO this *might* be useful and we should add it only if there are explicit requirements from users for it. This is a public API we want to keep as simple/small as possible. It should also do the job, of course :)
-
23. Re: JBoss Cache Public API
Ok, so for the sake of API changes, fr the next Alpha I propose we:
1. remove create() and destroy() on the public API.
2. CacheFactory creates the cache and calls create().
3. stop() will call stop(), destroy() and create() in that order to reset the cache.
and later we can optimise what happens internally in start() and stop(). WDYT? -
24. Re: JBoss Cache Public API
Hmm - how would this work with the CacheJMXWrapper and the registering/unregistering of MBeans? Thoughts?
-
25. Re: JBoss Cache Public API
Re: CacheJmxWrapper, it should be OK. It doesn't do anything in create() that it can't do in start(). So, we can either:
1) Remove create() altogether from CacheJmxWrapper .
2) or leave it there but remove the call to Cache.create().
#1 seems illogical but has minor advantages. One of the things CacheJmxWrapper,create() does is instantiate the cache if one wasn't injected. Leaving that in a "create" step allows sophisticated users to get a ref to the cache via CacheJmxWrapper.getCache(), register listeners, etc, before start() is called. -
26. Re: JBoss Cache Public API
Re: getting rid of create/destroy altogether, how many other significant incompatible changes to the API have been made? If we got rid of the Fqn constructors as was discussed at the beginning of this thread, ignore the rest of this post. :-)
If the API is basically compatible w/ 2.x, that's an argument for leaving create/destroy around even if we internally move their functionality to start/stop. That is, don't make people rewrite their apps/prevent upgrades for a minor API cleanliness benefit.
No longer having Fqn constructors means nearly everyone has to rewrite their apps, in which case might as well clean up whatever you can. -
27. Re: JBoss Cache Public API
There are some incompatibilities such as Fqn ctors.
The reason why I asked about the JMX wrapper is because there is some unregistering code in destroy which isn't there in stop. -
28. Re: JBoss Cache Public API
The registration/unregistration of interceptors should be tied to the existence of the interceptors. If Cache.stop() removes the interceptors, then the unregistration should move to CacheJmxWrapper.stop(). And of course they shouldn't be registered before they exist. ;)
-