Re: [jgroups-dev] Improvements in org.jgroups.blocks.mux

I looked at MuxRcpDispatcher. We should rename id to prevent mistaking 
it for the (inherited) id field, which is used for marshalling...


Comments inline.

Brian Stansberry wrote:
> I've been working on switching the JBoss AS clustering code to work off
> a MuxRpcDispatcher to see where any pain points are.[1] So far what I'm
> seeing is:
>
> 1) MuxRequestCorrelator needs to override the sendUnicastRequest()
> method. Simple fix, already done; see [2].

OK

> 2) MuxUpHandler needs to handle more event types. This really comes
> down to 2 cases:
>
> a) misc events that need to be passed to all up handlers, which is simple
> b) state transfer
>
> I've gotten a state transfer solution working. See patch attached to [3].
>
> It's based on a rule that if a bunch of components of a service want to
> share a channel via MuxRpcDispatcher, then at most one can use full
> state transfer; the rest must use partial state transfer. Those that use
> partial state transfer can register an UpHandler with the Muxer that
> implements a new StateTransferFilter interface
>
> boolean accepts(String state_id)
>
> MuxUpHandler handles state transfer related events by iterating through
> the registered handlers, calling accepts(), passing the event to the one
> that returns true, with fallback logic if none accept. See patch at [3]
> for further details.

Do we need this added complexity ? I thought the use case was that we 
have a state-related component (Infinispan or JBossCache) and that the 
other components only used the shared channel for communication via 
JGroups... Has this changed ? Do the other components now need state 
transfer as well ?

We could simplify the model by saying that only 1 component (the 
'default' component registered with the MUX) could do state transfer, 
and everyone else was not allowed to do state transfer. If another 
component requires state transfer, allocate a separate channel for it.

I have a feeling that we're re-creating something like the 
dreaded/misguided/querky/kludgey Multiplexer... :-) I hope we're not 
going to go down the same road, only to find out later that it's a 
oneway road...

One issue I can already see is that flushing applies to the entire 
stack, and letting each registered handler handle the event individually 
may lead to trouble. Another one is BARRIER, which we need for state 
transfer: this will block until all current requests have completed, 
which may not be optimal (or even dangerous) in cases where the 
non-state related components will get blocked, too...

> I think the design requirement here that all but one component must use
> partial state transfer and must provide a StateTransferFilter is OK.
> Unlike the earlier MuxChannel or the shared transport, MuxRpcDispatcher
> is not intended for general multiplexing of completely unrelated
> services where the service developers may know nothing about each other.

OK

> The use case for MuxRpcDispatcher is a set of *components of a larger
> overall service* that need to send RPCs over the same channel. The
> person developing that overall service (e.g. me) can be expected to have
> an understanding of how the service's components work and how they are
> multiplexing over the channel.

What's the use case you have in mind ? 1 handler for Infinispan, which 
is the default handler with state transfer) ? Then 1 additional handler 
for communication ? More handlers on top of that ?

If that's the case, I'd say pass all state related calls to the default 
handler and throw an exception if a non default handler does state.

> 3) The integration with services like JBC that aren't written to use the
> MuxRpcDispatcher can be made easier with some fairly simple changes to
> Muxer/MuxUpHandler/MessageDispatcher. See patch attached to [4].
>
> Basically the patch seeks to formalize a bit the concept of a default
> UpHandler.

OK

> 4) NoMuxHandlerRspFilter can't be used because it will return 'false'
> from isAcceptable(). See [5] for discussion of why that is not reliable.
> Upshot is users of MuxRpcDispatcher need to deal with NoMuxHandler
> responses.

Let's have a discussion about this at JBossWorld. Meanwhile, go ahead 
with your commits. I know you commit responsibly and are fully aware of 
the consequences of your doing ! :-) LOL

> - Brian
>
> [1] The key reason for doing this is to aid in Infinispan integration by
> supporting https://jira.jboss.org/jira/browse/JBAS-7852 . But for my
> initial work I'm converting the AS HAPartition impl to use a
> MuxRpcDispatcher to share a JGroups Channel with the JBoss Cache
> instance used by HAPartition's "DistributedState" component. Doing this
> first lets me expose issues more easily since the large set of
> HAPartition use cases and associated tests have to work.
>
> [2] https://jira.jboss.org/browse/JGRP-1208
>
> [3] https://jira.jboss.org/browse/JGRP-1209
>
> [4] https://jira.jboss.org/browse/JGRP-1210
>
> [5] http://old.nabble.com/Race-condition-with-RspFilter-td28261345.html
>

-- 
Bela Ban
Lead JGroups / Clustering Team
JBoss

------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate 
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the 
lucky parental unit.  See the prize list and enter to win: 
http://p.sf.net/sfu/thinkgeek-promo
_______________________________________________
Javagroups-development mailing list