jeromq icon indicating copy to clipboard operation
jeromq copied to clipboard

Revise socket monitor

Open miniway opened this issue 12 years ago • 5 comments

https://github.com/zeromq/libzmq/commit/759d453368479257638e6b09e1febe19fbef2a3d

There're significant change on the monitoring of various events.

miniway avatar May 14 '13 17:05 miniway

This is our oldest open issue, and I don't really understand the details of what had been changed/revised as of when this issue was opened.

@fredoboulo Do you think your recent addition of ZMonitor might resolve this issue?

daveyarwood avatar Sep 16 '17 02:09 daveyarwood

The referenced commit is quite old (2012), if I interpret this in current implementation, I understand that code should stick to the latest libzmq format of monitor messages, which consist of 2 frames. This should ensure the message-compatibility with jzmq. Internally, as long as users stick to the usage of Event, it should be transparent. It could still be disruptive if users parse the messages by hand, they would adapt their code.

Bonus: we could add the remote address in the relevant events, that could please some people by providing more useful information.

fredoboulo avatar Sep 16 '17 07:09 fredoboulo

@fredoboulo I think that changing the underlying representation to match the libzmq format is sensible as long as the public API (i.e. the Event.read method) stays the same.

I don't think we can necessarily support anyone's code that relies on implementation details. If any issues like that come up, we could always point to this issue and say "we did this to be closer to the libzmq spec." And they could always adapt their code, like you mentioned -- I imagine that shouldn't be too difficult.

To be extra clear, we could bump the minor version of JeroMQ to indicate that there is a breaking change.

daveyarwood avatar Oct 14 '17 18:10 daveyarwood

I spent a little time exploring the history of this part of the codebase in both libzmq and JeroMQ, to get more context.

As far as I can tell, it looks like JeroMQ is generally doing things the way libzmq is with respect to socket monitoring, except that the format of the monitor event messages has been updated a couple of times in libzmq.

As @fredoboulo pointed out in 9/2017, the latest event message format at that time included a second frame with the address:

frame 1 frame 2
event and value address

As of right now, libzmq supports two event message formats: the one above, and a new one that has four frames:

frame 1 frame 2 frame 3 frame 4
event value local endpoint URI remote endpoint URI

In JeroMQ, we are sending a single frame containing (in this order) the event, the address, and the value. (We also have this weird thing I don't fully understand where an event can wrap a SelectableChannel instead of an event number. :thinking: I'm ignoring that part here for simplicity. )

The message format we're using now isn't consistent at all with the current state of libzmq, and I vote that we update our code to be as close to possible with what's on libzmq master right now. I think it would be fine to make a breaking change here, since we are clearly striving for compatibility with libzmq, and as @fredoboulo pointed out, the Event class provides a public API, and the change should be transparent as long as you're using Event.write and Event.read instead of trying to parse the event messages yourself.

Another interesting thing to point out: within the past year and a half, the zmq::socket_base_t::monitor method (which corresponds to our Socketbase.monitor method) gained an int event_version_ argument, the value of which can be 1 or 2, and that controls whether monitor event messages will have the (version 1) 2-frame or (version 2) 4-frame formats that I described above. I think we should mirror this behavior.

daveyarwood avatar Feb 09 '19 19:02 daveyarwood

I might take a stab at updating this myself at some point. I'd love to see this 5 (almost 6) year old issue get resolved!

daveyarwood avatar Feb 09 '19 19:02 daveyarwood