Skip to content
This repository has been archived by the owner on Oct 31, 2021. It is now read-only.

Current Transports require a proper framing implementation. #49

Open
goldfish1974 opened this issue Apr 8, 2015 · 3 comments
Open

Current Transports require a proper framing implementation. #49

goldfish1974 opened this issue Apr 8, 2015 · 3 comments

Comments

@goldfish1974
Copy link

Hi again Colin,

Thanks for your help on the remote agents, I got that working with Message.reply.

I have found what seems to be a bug in Socket.fs. As I'm a bit of a noob, I'm having fun following the code. :D

 type PingPong =
     | Ping of string    // add of string
     | Pong of string  // Add of string
     | Stop

  // PingNode/Program.fs... change loop to:
 let rec loop count = messageHandler {
                     let! msg = Message.receive()
                     match msg with
                     | Pong message when count > 0 ->
                           //if count % 10 = 0 then
                           printfn "Ping: pong %d" count
                           do! Message.post pong (Ping (message + "0")) // Make it longer
                           return! loop (count - 1)
                     | Ping message -> failwithf "Ping: received a ping message, panic..."
                     | _ -> do! Message.post pong Stop
                 }

 // and PingNode/Program.fs main function to call new Pong like:
 let initialMessage = String.Empty
 pingRef <-- Pong initialMessage


 // PongNode/Program.fs: change agent loop to:

 let rec loop count = messageHandler {
            let! msg = Message.receive()
            match msg with
            | Ping message -> 
                  //if count % 10 = 0 then 
                  printfn "Pong: ping %d: %d" count message.Length
                  do! Message.reply (Pong (message))
                  return! loop (count + 1)
            | Pong _ -> failwithf "Pong: received a pong message, panic..."
            | _ -> ()
        }

It appears that either the send or receive (or both) within socket.fs cannot handle serialised content > 4096 bytes (or > bufferSize to be precise). I have seen a few different things. It mainly manifests itself as an OutOfMemory or Memory Violation type exception due to (I believe) the code reading off the end of the buffer, and trying to allocate a large byte[] or access an index outside of the pool buffer.

the let rec send' function seems to be able to write the data out (although, on the second recursion, I'm not sure it if is actually write the second block of the serialised data, or just garbage memory).

At the point where the posted message has just written it's first block (with more to come), the other agent crashes out (before the second block has ben written).

You can see this using a tweaked version of the ping/pong network example which passes an ever increasing string between the 2 nodes. Ping increases the string, pong just replies the same string
I made the following change:

I hope you can shed some light on this...or describe how I could fix it myself.

Thanks in advance,
Nathan

@colinbull
Copy link
Collaborator

Yes, this is an unfortunate limitation that I already knew about. Basically, I need to implement some framing on top of the Socket implementation, that allows large messages to be broken up into smaller chunks and then recombined at the other end. As simple as it sounds, there are a few edge cases and complications to deal with here. It was always my intention to move to a more robust transport implementation, probably something based on ZeroMQ but any robust messaging suite would work, this is simply a case of providing an implementation for the ITransport interface.

This was on my TODO list, however I got very busy with my day job and caught up in some other projects that this completely slipped my mind.

@colinbull colinbull changed the title Cannot Message.post/Message.reply DUs that serialise > 4096 bytes Current Transports require a proper framing implementation. Apr 8, 2015
@goldfish1974
Copy link
Author

Hi Colin,

(HeHe...there's always plenty of TODOs in there... you should see my list!)

I plan to have a go at fixing the framing issue. As far as I can see, the send already outputs the size of the buffer being sent. There is a corresponding read operation, that takes in the size, but then only reads a single block without waiting looping for subsequent blocks. I think a simple rec sub function should be all it needs before returning the byte[], minus the length. That then gets passed to the handler which basically calls Message.unpack.

I will also look at adding ZeroMQ type support a bit later (although I will probably based this on the pure C# NetMQ implementation as the current build (and pre release NuGet package) supports windows IO Completion ports. This is something that I will be needing in my next project :D. The author of NetMQ also talks about fixing the treading issues associated with (BCL) async operations. (As I understand things, async in F# may resume on any thread...which make sense for immutable data...but may be a hurdle using the wrapped ZeroMQ since it talks about item being on the same thread across calls.

I am planning to model this on the current code within Cricket for Sockets. Just to clarify the current implementation methodology is effectively a unidirectional message from post -> receive and then the reverse channel reply -> receive is on the separate socket connection, and not back down the receiving socket connection? (not counting the UDP discovery broadcasts to find). (i.e. This model basically a connected set of peer servers, as opposed to client-server?

Are there any other models possible given the current ITransport. I presume alternative models should be able to be managed through the corresponding registry such that if a client connects in, then the reverse channel could possibly leverages (this would probably mean that the listening port for the client end of the host would need to be passed along with client connections, so that and already established connection could use the reverse path...this may actually already be present in the client actor address...i'd need to look. This could be useful for firewalled/NAT based connections.

If you could quickly clarify the agent -> agent model, that would be helpful in mapping onto ZeroMQ/NetMQ when choosing the best comms model for Cricket Agents.

Thanks for your prompt feedback too.

@colinbull
Copy link
Collaborator

If you could have a look at the framing that should, be the case. Looking at the current implementation what you suggest seems like the simplest route. However, I remember trying this but couldn't get it to work properly for some reason, hence Why I thought we might need something like length prefixed framing.

As for ZeroMQ, I have been using it on other projects without any problems have you got a reference to something I can read about with respect to this? Had a quick look but could find anything, but it is something I'm interested in.

Yes, exactly, the actors are essentially a connected set of peers, you are exactly correct. And indeed other models are possible, actors should not care about the connectivity, of the transports. So achieving what you suggest should be possible. If it isn't I'd count this as a design issue, so feel free to raise it as a bug.

I'm currently a little stacked out at work at the minute, but I'll try and get at least a diagram of the agent -> agent comms model that happens at the minute.

Thanks.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants