Skip to content

overhaul of the erlang bindings #14

Open
wants to merge 28 commits into from

4 participants

@csrl
csrl commented Jan 13, 2011

I've significantly cleaned up and fully implemented all functionality of zeromq in the erlang bindings.

The concept is fairly basic. A zeromq socket has a controlling erlang process. The driver port code has been greatly simplified by this.

The code is also implemented in layers now. One can utilized the new zmq_drv.erl module directly if desired, but must abide by certain constraints. The zmq_drv.erl module is a direct mapping around the port driver functionality.

The next layer is a gen_server socket wrapper and a gen_server context wrapper. These wrappers should generally be used instead of the zmq_drv module. If one wants to integrate the context or sockets into a supervision tree, this provides a very simple way to do so.

Finally, a zeromq C like wrapper is provided by the zmq.erl module.

There are a few things left to do/improve upon, but this exposes the full functionality of zeromq, excepting zeromq drivers.

@csrl
csrl commented Jan 13, 2011

Feel free to squash all of those commits into a single commit when pulling.

@saleyn
saleyn commented Jan 15, 2011

Chris, thanks for your work. I am including some comments based on my quick review of your code.

  1. It's a little unconventional for the "catch all" handle_info/2 clause to stop the gen_server - this makes it quite easy to kill a server unintentionally. The situation is different with handle_call/3 and handle_cast/2 because those calls are specifically designed for particular API use, and terminating the server in those cases indicates of a logic error in API use.

  2. I am a little concerned with current implementation of socket-specific gen_server, because the OTP gen servers should not issue any blocking calls, and the zmq recv calls within handle_call/3 can be blocking. This means that the socket server won't be able to send another message to the driver until it gets a reply back. Perhaps this can also lead to some potential shutdown timeouts and issues?

  3. Introduction of a gen_server layer indicates that any message passed from your Erlang process to the port driver will be copied twice - once from your process to gen_server, and the second time from gen_server to port. Is introduction of this overhead worth the separation of zmq sockets in a different layer?

  4. I definitely like the way you dealt with the potentially blocking zmq_term issue.

@csrl
csrl commented Jan 15, 2011

Hi Serge,

Thanks for looking over the code base.

  1. If an unknown message is delivered to the mailbox of the gen_server, it is a bug somewhere in the system. I can understand that these bindings may be used in a system with different requirements then those I implemented this with - certainly have the gen_server die isn't necessary for correct functionality of these bindings and so can be changed.

  2. Interesting. The area of your concern is specifically why this was done. It should not be possible to have two concurrent calls into the driver for the same socket. The previous implementation allowed that and was the source of many bugs. As for "gen servers should not issue blocking calls" is a confusing statement, see gen_server call vs cast.

  3. I sure hope the erlang vm isn't that inefficient (it isn't). However, zmq_drv.erl is available for direct socket ownership if a specific application implementation requires it. Having the gen_server socket layer abstracts the issue with concurrency control on a single socket, thus allowing many erlang processes to use the same socket with only application logic synchronization necessary.

  4. It seems to be working well.

@saleyn
saleyn commented Jan 15, 2011

Hi Cris,

  1. From what I've seen in many production systems using Erlang was that an unhandled messages may result not only from wrong API, but in some cases when the system goes under significant stress and some blocking calls begin timing out resulting in delivery of unexpected messages from linked/monitored/rpc'd processes and calls, as well as from some retrospection of processes through various GUI tools (similar to appmon) or simply by attaching a shell and sending a message to a wrong process. These are exceptional cases, of course, but they suggest a general guideline of sticking to strict API use between Erlang subsystems instead of pure message passing, and leaving the later for some loose non-standard cases (such as communications with a driver or cross-language messaging). Consequently, handle_info/2 should not be invoked on misused API calls, and therefore it should be safe to ignore those orphaned messages.

  2. The blocking issue to be aware of when implementing a gen behavior is that the implementer should not interfere with delivery of OTP system messages to a gen behavior by blocking a gen server (if a gen_server is blocked it can't properly and timely respond to system messages).

In the proposed changes the message passing chain for a typical send looks like this CallingPid -> GenServerSocketPid -> Port. Can the same socket be used by multiple client Pids? The obvious answer is that this model allows for that, but is this practical? Is the typical case that we have N named servers in our system where each server is associated with one ZMQ socket to communicate with other peers; or is it that we have a pool of reusable sockets and M client user processes that somehow figure out which ZMQ socket they want to use and issue calls to them?

  1. You have a good point regarding the alternative use of zmq_drv API directly as a viable alternative. Regarding your efficiency statement, though, the overhead of these issues is not noticeable on systems with light load, but this cost becomes very noticeable when processing over 50k msg/s - they not necessarily be a problem of VM itself but the fact that the Erlang Pids may be scheduled on different cores and there will be a need for extra context switches in addition to copying, which at some point becomes a bottleneck.

  2. As I looked a little deeper into the proposed Erlang code, I also see some conceptual API changes, such as that the zmq:socket/2 lost its Options argument, which I think was pretty convenient. Moreover, in the current model, the user needs to retain the context, which is owned by some owner pid. If that owner dies, the context dies and all sockets need to be closed. Why would that need to be done? It looks to me that having the named ZMQ context as a "stand-along" server entity (just like mnesia) has no down-side and there's no reason to take it down with the death of the owner. Am I being mistaken?

It's worth noting that the original idea of erlzmq was somewhat similar to gen_tcp/udp/sctp that the socket owner process communicates directly with the port without introducing a middleman process. There is a difference in a way that each gen_tcp creates a separate port instance (if I recall correctly), whereas all ZMQ sockets are handled in the scope of a single port instance.

Regards,

Serge

@csrl
csrl commented Jan 16, 2011

Serge,

Thanks for taking the time to work through this. I understand your points on OTP gen_server handling system messages. My original implementation queued requests and replied to caller when the result became available. I'll look at moving back to that approach, and work through the issues I was running into with that implementation. Since only send and recv are "blocking" when not passed 'noblock' flag, I think I can special case those two calls when that flag is set.

The API change removing Flags list from send and recv has bugged me as well, and your voicing concern of this has convinced me to restore those as a list. Do note that the socket gen_server and zmq_drv modules always excepted a list of flags, only the zmq module interface artificially restricted this.

The message chain in this implementation has not changed from the original. A gen_server still sat between the caller and the port. The difference is all sockets were routed through a single gen_server in the original implementation causing a severe bottleneck. This implementation removes that. The original implementation also allowed an "alternate" direct port api. This implementation does so as well, but does so by making available the zmq_drv module instead of confusingly overloading the zmq module interface. I expect the zmq.erl interface to be a convenient way to quickly integrate zeromq into an erlang application. For high performance implementations I'd expect for the zmq_drv.erl interface to be used directly.

That said, (as I've noted in zmq_context.erl) I think that the implementation should really move to a port per socket rather than a port per context. It sounds like you've had this concept as well.

As for sockets and contexts having a linked owner. That is the default implementation. One can bypass the zmq.erl interface, and write a replacement init or socket call that uses gen_server:start/3 rather than gen_server:start_link/3. It is my expectation that when someone wants to integrate zeromq into their erlang system that they write a wrapper module that implements the use case that meets their needs. I see the zmq.erl, zmq_context.erl and zmq_socket.erl interfaces as a reusable generic wrapper that will meet most evaluation and simple use case needs.

Anyway, your question regarding pool of sockets vs individual users of a socket. I've implemented the port driver and zmq_drv.erl with the concept that there is an individual user of a socket - and certainly zeromq itself dictates that anyway. Providing a gen_server as the individual user of the socket is a convenience to users of the library. Much like zeromq's own socket Type vs XType. You can use XType directly if you want but you have to take on more responsibility; you can use zmq_drv.erl directly if you want.

Again, thanks for your suggestions, I'm working on updating the implementation with them. I've also fully documented the zmq.erl interface and will push these updates in the next couple of days.

chris

@saleyn
saleyn commented Jan 16, 2011

The more important question that I am struggling with is what is that gen_server wrapper giving us that is not available without it (especially that all other networking components gen_tcp/udp/sctp) don't rely on separate gen_servers but are building blocks to be included in users' implementations (which can rely on OTP standard behaviors)?

I haven't looked very carefully, but on the quick search it looks like you removed the "active" socket option. That option is very important, which behaved similarly to the same option in gen_tcp/gen_udp/gen_sctp, except for {active, once}, which I didn't have time to implement. Also this functionality is currently used in the RabbitMQ project that uses 0MQ transport.

Regarding your comment about the fact that in my implementation there was a single gen_server handling all requests - this was actually the original design at the time when I picked up the project that wasn't scalable, so I moved to direct communication with the driver, and implemented on the driver side return of messages to the calling process instead of the port owner in order to resolve that potential scalability concern.

I see your point on zmq_context, but maybe I didn't voice my question very clearly. Your suggestion of not binding an owner to the context via gen_server:start/3 call means that this is not a solution for supervised inclusion an application. Logically, 0MQ dictates that there should be at most one context per application (there's no hard limit, but it's a strong suggestion). Therefore, it would make sense to wrap that in a named gen_server that would be started as part of zmq application, bringing up an instance of the port driver. Unfortunately, the model of inet_drv that serves gen_tcp/udp/sctp, where one instance of the port driver is allocated per socket, cannot be used here because of the need for a shared context. Though, it's actually possible to do that cleanly with reference counted context, but would be harder to implement in view of the polling needs, and there's really no need to go in that direction. The users of ZMQ sockets (either using direct port communication or going through wrapped gen_server) would really not need to be aware of the context. That server would also be responsible for driver loading, which theoretically would permit loading more than one driver instance when the context server is started without using a registered name. I don't see a need right now for more than one driver instance and context (other than testing, upgrading, etc), but think that restricting that is a bit artificial, since the back-end supports that. In the proposed implementation driver loading/unloading happens in the load/0, unload/0 functions, which I believe, makes it impossible to load more than one driver instance (actually, things would become quite tricky during upgrades, since hot code upgrading retains several versions of a module, I am not entirely sure what happens with the load/unload calls during that transition. So, to make it short, it seems to me that having more application-level control over the driver loading would be more flexible on the long run.

If the framework allows for using the direct driver communications as easy and nearly identical to going through the wrapped gen_server socket servers, then it would be very helpful to end-users in choosing and migrating between the two approaches.

Regards,

Serge

@csrl
csrl commented Jan 17, 2011

Let's back up. Where I think we both want to go is this:

  • A feature complete API that makes sense both from an erlang standpoint and a zeromq standpoint

  • As high performance as possible erlang bindings that is bug free and feature complete.

  • Each zeromq socket should only have a single operation pending at a time. Thus, if one erlang process has an outstanding blocking write or read, another erlang process should not be able to access the socket at all. This is most easily ensured by requiring a single controlling erlang process of the socket.

  • A single ZMQ context

  • A port per socket (An approach I believe will help meet the above goals).

Next, there are intermediate steps to reach that goal - or at least I don't have the time to spend up front to do that full implementation all at once - and there are some roadblocks, namely zmq_term() being blocking.

  • The first step, was to clean up the code base and provide full zeromq feature set, I think that is done.

  • The next step was to introduce the necessity of ensuring a single controlling erlang process. I moved the C++ driver code forward towards that goal, but not fully. In the interim I've implemented a gen_server socket wrapper which implements the functionality I intend as if each socket was a port. I do not see zmq_socket.erl being around long term, at least not in its current form.

The rest is yet todo. It is not yet (easily) possible to have a single zeromq context managed within the driver and a port per socket. This is mainly because zmq_term is blocking. To work around that we have to track all open sockets, and when there is a port per socket, requires shared memory within the driver which I was not interested in dealing with when the approach I took was much simpler and the longer term solution hopefully solves itself by being fixed in zeromq itself. Another alternative is to spin a background thread for zmq_term as brought up by sustrik. However that has other hoop jumping that is also not interesting at the moment. Yet another solution is in the erlang level to have a socket manager which maintains the context and tracks all open sockets, not allowing the context to be terminated until all sockets have been closed. But that is a much more brittle approach with other issues - I implemented it and discarded it after a while.

Finally, I view a fully functional, feature complete implementation as the right approach and desirable for a generally released solution over an academic, or narrow use case, performance target. If a certain user/group has a specific need, they can hack it into place until a common general solution is available.

Again, I think we both want to reach the same goals, just differing initial approaches. I had need of a more feature complete stable implementation.

If you think the new code base can be built off of, I'm happy to have it merged as it isn't in my interest to maintain an alternate fork, however the zeromq/erlzmq implementation isn't usable for my use cases.

I'm working on addressing the problem areas you have brought to my attention and will have those fixed in the near term. Also, I would be very interested if my implementation is shown to have impacting reduced real world performance over the original and what those use cases are.

Thanks again for your time,

chris

@saleyn
saleyn commented Jan 17, 2011

Chris,

Thanks for clarifying your goals. I also think that we have similar long-term visions for the erlzmq project.

If we agree on the need to support one ZMQ context, what is the reason for the API giving the user an explicit ability of passing named context other than registered zmq_context?

I don't have a problem pulling your changes if the following requirements are met:

  1. The new version doesn't loose functionality available in the API of the old version (namely: creation of sockets with given socket options, support of active/passive socket mode), so that existing users are not impacted by these changes.

  2. Add the following function for supporting hookup of the context to a supervision tree without the context owner process (correct me if I am wrong, but I believe there's no down side in this):

    zmq:start_link(IoThreads) when is_atom(Name), is_integer(IoThreads) ->  
        gen_server:start_link({local, zmq_context}, zmq_context, [{iothreads, IoThreads}], [])
    

At least with these little changes in place it seems to me that we should be able to work off of a single code base.

Regards,

Serge

@csrl
csrl commented Jan 17, 2011
  1. Ok

  2. Who is responsible for the context then when multiple applications use zmq?

Instead, to add a context to the supervision tree, create your own module exposing a start_link function that calls zmq:init(myapp_zmq_context, IoThreads).

This way, currently, if more than one application wants to use zeromq and place the context under supervision, it is possible. Also, since a single port is currently shared between all sockets - which could potentially be a bottleneck, creating multiple contexts is an option if absolutely necessary.

Long term, zmq itself should be an application, so that there is a single instance of the zmq application on a node, and the IoThreads is initialized by an application environment variable. The context itself is not maintained at the erlang level, but rather internal to the driver.

So, I don't think adding a zmq:start_link is the right approach initially. Either leave it as is, or turn zmq into an application which starts and links the context - of course for now that puts all application's sockets under a single port.

chris

@saleyn
saleyn commented Jan 17, 2011

#2. This request was made for two reasons:

  • To put the API in place for turning zmq into an application having default context (which is indeed what you acknowledged). If some user wanted a "private" instance of the context, he/she could create one by invoking the zmq:init(Context, IoThreads). I agree that without having zmq turned into an application, there's little benefit from adding this function on its own.

  • To have backward compatibility at the API level, and adhering to the standard OTP naming practice for start functions (in OTP init usually refers to a name for the behavior callback).

If you still feel that #2 is unnecessary without zmq being an application, I am ok with doing that work myself after pulling your changes, since this has been on my todo list for a while.

@csrl
csrl commented Jan 17, 2011

I made the assumption that you wanted to remove zmq:init/1 when adding zmq:start_link/1, but it looks like I was wrong, so that is good.

As for backwards compatibility, the rest of the updated api isn't backwards compatible... A code update is required one way or the other.

@saleyn
saleyn commented Jan 17, 2011

Doing a quick look through the old/new zmq.erl reveals little differences in start_link/1 and socket/2. Is there more that users would need to be aware of when upgrading? Let me know when you are done with the changes, and I'll do the pull (initially to a brunch and some time later will merge with the master).

@saleyn
saleyn commented Jan 21, 2011

Chris, I see that you did changes reflecting our discussion. Are you still working on adding the active mode back (I don't see it in the commits)?

@csrl
csrl commented Jan 24, 2011

'active' mode only makes sense for certain socket types. eg. 'sub'. For other socket types eg. 'rep'/'req' it instead causes problems and doesn't provide anything over using 'poll'. Anyway, I haven't had time to implement an 'active' socket mode correctly yet as it isn't a need of mine. However, I see a place for it, but it's implementation doesn't make sense until some other items are put in place. In the mean time, 'poll' functionality likely fulfills many use cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.