My impression is that the Concurrent* classes add a lot of
synchronization overhead. The copy on modify plan I think will prevent
concurrent modification exceptions. At this point I'd prefer to add a
warning comment and see if anyone gets into trouble, and deal with it
then. If I'm missing something, please let me know.
thanks
david jencks
On Wednesday, January 14, 2004, at 11:46 AM, Ed Letifov wrote:
I am sorry guys, was my question too stupid, too late or too off?
On Tuesday, Jan 13, 2004, at 20:27 Europe/Amsterdam, Ed Letifov wrote:
Hello,
since copy on update will not really solve issue of having "stale"
iterators around I have a sideline question:
can we consider usage of ConcurrentHashMap or
ConcurrentReaderHashMap from EDU.oswego.cs.dl.util.concurrent package
for this?
As I understand this code is in public domain and can be used, will
provide a substantial concurrency level and the iterators will
reflect the current state of the collection as much as possible.
Ed Letifov.
On Monday, Jan 12, 2004, at 19:57 Europe/Amsterdam, Aaron Mulder
wrote:
We may also want to consider that instead of copying the
Collection at iteration time, we could copy it only at update time,
and
then write the new (changed) version over the old (iterated) version,
leaving the only references to the old collection with the iterators.
I'm assuming that we'll have more iterating than updating, and if
so, this
ought to result in less copying.
Aaron
On Mon, 12 Jan 2004, Dain Sundstrom wrote:
On Jan 12, 2004, at 8:14 AM, gianny DAMOUR wrote:
Hi,
I have just checked the new GBean implementation and I do have the
feeling that there are - potentially - some concurrency issues with
the CollectionProxy:
I bet there are. It was the last one coded and in a rush so we
could
get the code checked in. I plan on reviewing all the code again
over
the next few days, and writing some documentation as I go.
If I understand correctly, this proxy implements its own
Collection in
order to solve some previous lifecycle limitations. This Collection
returns an Iterator directly backed by an Iterator directly backed
by
the values of the endpoint name to proxy Map of CollectionProxy.
Unfortunately, it means that a service can not iterate safely over
such an Iterator without risking a ConcurrentModificationException
under some specific race conditions.
We had that problem in the previous code also. I'm not sure how we
want to handle this.
For instance, DeploymentController could iterate over its
Collection
of DeploymentPlanner and, at the same time, a DeploymentPlanner
could
be unregistered, which should cause a
ConcurrentModificationException.
One could return an Iterator backed by a copy of the actual Map.
However, this implementation does not work:
I have been thinking of that, or we could change the code to
synchronized on the client collection whenever making a change to
the
backing collection, then the client could guarantee a safe iterator
by
synchronizing on the collection. The problem with the copy
implementation is we could have very old iterators sitting around,
and
the problem with synchronization is poorly written code locking up
the
GMBean for a long time. I'm leaning towards the copy code.
Indeed, this Iterator could returned a disconnected
ProxyMethodInterceptor if at the same time an endpoint is
unregistered. One could use the GeronimoMBeanEndpointListener
feature
in order to detect such a case.
However, as endpoints are inter-connected by listening to JMX
notifications, it is also possible to have a connected
ProxyMethodInterceptor, which reflects a JMX invocation against an
already unregistered MBean.
Hence, I would like to know if the following could fix the previous
issues - if there are indeed some concurrency issues:
I propose to add a Mediator, whose responsibility is to track
synchronously (no notifications involved) the availability of
components and contact synchronously the interested components
when a
GBean switch from the offline to the online state and conversaly.
Could anyone confirm or disprove my concern and approach?
I don't like this approach. I think we will end up with way too
much
synchronization in the container. I'm also not sure it is
possible....
This is a highly threaded system, which means that several
components
could want to change state simultaneously, and if we make the system
state changes synchronous, we are bound to get a system wide
deadlock.
Also the notification happens after the component fails or stops, so
even if we hold up the notification to get a clean system wide state
change, the component is already unavailable (just not reflected in
the
system state).
Besides the implementation difficulty, I don't think we want this.
I
see the current implementation as a "good enough" design.
Components
do die at the wrong time, and all you can do is respond. This is
how I
define a "good enough" design, "It does not solve all the problems,
but
it is good enough to work reliably." If you really needed to work
with
a component, you catch the unavailable exception and fail, which is
exactly how single valued components work. If you don't really need
to
work with a specific component, you just catch the exception and
move
on. Of course this means that people writing gbeans need to know
what
they are doing, but I see that as the price of creating reliable
production ready services.
-dain
