Re: [jgroups-dev] Advice on best practices for implementing a new protocol

Mike Jensen wrote:
> A couple more notes in addition to the email I sent earlier this 
> morning...(sorry to be throwing so much at you, hopefully your not thinking this 
> would just be easier to implement yourself, hehe)
>   

I was biking [1] WED and THU, and just came back, so throw at me what 
you like but I'll reply in the order the mails were delivered to my 
inbox... :-)

> Because of the way that connections are established (by sending messages to a 
> destination that not already connected to), I realized I might need to handle 
> this different.  I would like your feedback on this proposal:
>
> The idea is that when the protocol gets an incoming message from the 
> application, we remove the dest and instead place the final destination inside 
> the mesh header.  Then we send out event(s) for the nodes connected to us that 
> should receive this message, except we replace destination to be those nodes 
> directly.

That would certainly work, but you might have to copy those messages. 
Take a look at DAISYCHAIN: there I'm also forwarding messages by 
changing the destination, but the to-be-forwarded messages themselves 
have to be copied, or we'll run into very subtle bugs...


>   So when sketched out, it might look something like this for up/down:
>
>   public Object down(Event event) {
>     switch (event.getType()) {
>       case Event.MSG:
>         // add our header on the message before passing on
>         Message msg = (Message)event.getArg();
>         msg.putHeaderIfAbsent(getId(), new TreeMeshHeader(local_addr,
>                                                           (msg.getDest() == null 
> || msg.getDest().isMulticastAddress() ? null : msg.getDest()),   // null here 
> represents that this message should be seen by the entire cluster
>                                                           meshModel.getAvgHops(),
>                                                           
> meshModel.peerConnectionCount(),
>                                                           
> meshModel.leafConnectionCount()));
>        // call down_prot.down(event) with destination to be the connected nodes 
> this should be sent to next
>        // return null?  not sure what to return since we are spawning multiple 
> down prot events from this single event
>         break;
>       case Event.DISCONNECT:
>     return down_prot.down(event);
>       case Event.SET_LOCAL_ADDRESS:
>         local_addr = (Address) event.getArg();
>     return down_prot.down(event);
>       default:
>     return down_prot.down(event);
>     }
>   }
>
>   public Object up(Event event) {
>     switch (event.getType()) {
>       case Event.MSG:
>           Message msg = (Message)event.getArg();
>           TreeMeshHeader hdr = (TreeMeshHeader)msg.getHeader(getId());
>          
>           // analyze the message for more mesh structure information
>           meshModel.analyzeMessageHopRecord(hdr.hopRecord);
>          
>           if (hdr.destination == null) {
>             hdr.addHop(local_addr, meshModel.getAvgHops(), 
> meshModel.peerConnectionCount(), meshModel.leafConnectionCount());
>             // forward the message on to all connected nodes
>             List<Address> nodes = meshModel.getAllConnectedNodes();
>             Iterator<Address> it = nodes.iterator();
>             while (it.hasNext()) {
>               msg.setDest(it.next());
>               down_prot.down(new Event(Event.MSG, msg));
>             }
>              // message will be routed to the application via up_prot.up, as 
> this was broadcast everyone should see it
>           } else if (hdr.destination.equals(local_addr)) {
>             // message will be routed to the application via up_prot.up
>           } else {
>             // TODO - attempt to route the message to the given destination
>              return null;   // return null because we were just acting as a 
> relay node, and thus don't want the application to see this message?
>           }
>     }
>     return up_prot.up(event);
>   }
>   


You really need to copy these messages, see my comment above.

> Thoughts on implementing routing like this?? I am concerned about what 
> would be
> valid return values when I am spawning multiple events from one (and 
> thus why I
> am just using null, which I doubt will work, but not sure what it 
> should be).

No, that's fine.

> Moving on to a different question, as far as I can see there is no 
> mechanism to
> close an open connection. Is this correct?

Yes. The idea is that connection management is hidden from the 
application programmer. Of course, you could always grab the transport 
(TCP), the connection map used by it and call removeConnection(Address)...

> If so, any suggestions on the best
> way to include this functionality? I am not sure if getting a handle 
> on the
> AbstractConnectionMap (or rather what has implemented it) from within my
> protocol is the best idea.

Ah, ok, I should have read your entire email before replying... :-)

> If that is what is needed, maybe I should implement  my own TCP 
> protocol that extends the existing TCP. Suggestions?

Well, if you really need access to the connection map, then grabing the 
connection table and purging connections yourself is an option. But why 
would you want to do this ? You can configure idle connections to get 
purged by the connection table itself.

> Lastly, since multiple TCP connections may be established between two 
> nodes (as
> described earlier, since two nodes probably have the same internal 
> models, there
> is a high probability that they will decide to connect to each other 
> at the same
> time), I need a way to distinguish between two different connections 
> of the same
> node.

The connection map does *not* create 2 connection from A -> B and B -> 
A, but this is the *same* connection. If connection establishment 
happens at the same time, then we always pick one of the two to create it.

> I have so far been implementing everything by referencing to different
> nodes via their "Address" object. Of course that does not work for the 
> above
> situation, because their address objects should return false in 
> .equals, even
> when they are of the same node (it returns false because the ports 
> should be
> different, and if they did return true you might have a connection 
> leak in
> AbstractConnectionMap).
>
> So any suggestions on how to tell that two addresses are actually 
> connected to the same node?

IMO all of this needs to be done in your model of the mesh, and *not* in 
the connection table.

> I don't want to do an IP comparison

You shouldn't use IP addresses as identifiers for peers anyway, I 
suggest use UUIDs or some other unique IDs.

> because I worry it could
> cause confusion in situations where the nodes are possibly behind a 
> NAT (but
> maybe this is outside the scope of JGroups for now?)...

A NAT is another argument in favor of *not* using IP addresses directly, 
use UUIDs. Also, if you later decide to implement bridging (relaying), 
and different clusters have the same IP addresses (e.g. 192.168.x.x), 
then you're in trouble.

> I am also not confident in the ability for this to function correctly 
> when using the SHARED_LOOPBACK.

SHARED_LOOPBACK does *not* use the connection table

> If I can't do it from the Address object, is there some other object 
> handle you
> might recommend to reference nodes?
>
> As always, thank you very much for your help.

[1] http://www.bikemap.net/route/668933

-- 
Bela Ban
Lead JGroups / Clustering Team
JBoss



------------------------------------------------------------------------------
Sell apps to millions through the Intel(R) Atom(Tm) Developer Program
Be part of this innovative community and reach millions of netbook users 
worldwide. Take advantage of special opportunities to increase revenue and 
speed time-to-market. Join now, and jumpstart your future.
http://p.sf.net/sfu/intel-atom-d2d
_______________________________________________
Javagroups-development mailing list