goczmq icon indicating copy to clipboard operation
goczmq copied to clipboard

Add support for ZMQ_SERVER / ZMQ_CLIENT

Open taotetek opened this issue 9 years ago • 8 comments

ZMQ_CLIENT / ZMQ_SERVER support was added to CZMQ ( see: https://github.com/zeromq/czmq/pull/1059 ).

I need to add support for these new socket types to GoCZMQ. It should be done in a way that doesn't break GoCZMQ when built against older versions of CZMQ without these types.

taotetek avatar Aug 16 '15 03:08 taotetek

I'm musing over the idea of adding new types to handle the client / server sockets. I really want to enable the use of io.ReadWriter on these. while at the same time addressing https://github.com/zeromq/goczmq/issues/157 - which would require caching each message until it had been Read to the end of the message itself.

I think thread safe client / server sockets that provided fully working io.ReadWriter support would be a great thing to have - but I think the under the hood changes required in sock.go would be messy, and the changes could cause problems. If we added support for multiple read calls for a single message to the existing socket implementation, there could be cases like:

  1. User calls sock.Read with a buffer not large enough to read the entire received message
  2. The received message is still cached in a buffer in the socket, waiting to be fully read
  3. The user calls sock.ReadFrame

Now what? The options would be:

  • Return the part of the cached message not yet Read - which makes no sense
  • Return the full cached message - even though we just read part of it with the Read call
  • Read the next message off the socket - while we still have a cached message.

I think the answer "well, don't call Read and then ReadFrame that's silly" while true, is insufficient.

Since ZMQ_CLIENT and ZMQ_SERVER are thought to be "the way forward" I think adding goczmq.Client and goczmq.Server as separate types where we can experiment without adding complexity to or breaking the current socket implementation is probably the way to go.

taotetek avatar Oct 01 '15 12:10 taotetek

Thinking more - I'm conflating issues. I'm going to work on a PR to add ZMQ_CLIENT / ZMQ_SERVER support in the style we handle all the other socket types.

After that, I'll add ReadWriter as a separate type similar to Channeler. It will, given a socket, provide an io.ReadWriter interface that "does the right thing" to handle the situation where a buffer passed to Read is smaller than the last message that was received. I'll leave Read on Sock as well, but I'll add notes that using Read this way is deprecated and the preferred way to use a socket a an io.ReadWriter is via NewReadWriter.

This a) isn't overly complex and b) doesn't break anything.

taotetek avatar Oct 01 '15 12:10 taotetek

@somdoron is working on a poller APi ln libzmq that will support polling the new thread safe sockets. I think the first step on this (since current zpoller doesn't work with them) will be making a new poller in CZMQ that wraps it, and marking the current poller as deprecated (but keeping it around for a good while). Then I can add CLIENT / SERVER sockets and modify ReadWriter to work with the new sockets.

taotetek avatar Oct 21 '15 18:10 taotetek

I'm unclear why zpoller couldn't work with these sockets (regardless of the new libzmq API, which I like). Will check this out when I'm back in the office...

On Wed, Oct 21, 2015 at 8:24 PM, taotetek [email protected] wrote:

@somdoron https://github.com/somdoron is working on a poller APi ln libzmq that will support polling the new thread safe sockets. I think the first step on this (since current zpoller doesn't work with them) will be making a new poller in CZMQ that wraps it, and marking the current poller as deprecated (but keeping it around for a good while). Then I can add CLIENT / SERVER sockets and modify ReadWriter to work with the new sockets.

— Reply to this email directly or view it on GitHub https://github.com/zeromq/goczmq/issues/152#issuecomment-149984712.

hintjens avatar Oct 22 '15 08:10 hintjens

@hintjens zmq_poll doesn't support thread safe sockets as thread safe socket doesn't have a file descriptor.

@taotetek we have the ZMQ_HAVE_POLLER flag, so we can have two zpoller implementations, one based on zmq_poll and one on zmq_poller, depend on the ZMQ_HAVE_POLLER flag. What do you think?

somdoron avatar Oct 22 '15 08:10 somdoron

Ah, OK... I'll make zpoller use the new api if its available. No breaking change for callers then. On 22 Oct 2015 10:28 am, "Doron Somech" [email protected] wrote:

@hintjens https://github.com/hintjens zmq_poll doesn't support thread safe sockets as thread safe socket doesn't have a file descriptor.

@taotetek https://github.com/taotetek we have the ZMQ_HAVE_POLLER flag, so we can have two zpoller implementations, one based on zmq_poll and one on zmq_poller, depend on the ZMQ_HAVE_POLLER flag. What do you think?

— Reply to this email directly or view it on GitHub https://github.com/zeromq/goczmq/issues/152#issuecomment-150145768.

hintjens avatar Oct 22 '15 08:10 hintjens

As it is not documented yet take a look at the test file: https://github.com/zeromq/libzmq/blob/master/tests/test_poller.cpp

The main difference from zpoller is that on timed out the wait method return -1 and errno set to TIMEDOUT. On success the return code is 0 and the event parameter will be filled with the event.

typedef struct zmq_poller_event_t
{
    void *socket;
#if defined _WIN32
    SOCKET fd;
#else
    int fd;
#endif
    void *user_data;
    short events;
} zmq_poller_event_t;

ZMQ_EXPORT int  zmq_poller_wait (void *poller, zmq_poller_event_t *event, long timeout);

somdoron avatar Oct 22 '15 09:10 somdoron

@taotetek I think you're overcombining it a little bit.

  1. As it is still a draft API, I think it is still not worth building io.ReadWriter API above the sockets. It is at first sufficient to add just some support and io.ReadWriter is for another issue.
  2. It is very unlikely that someone who uses experimental draft API will be keen on sticking to the old CZMQ version while desiring for update of goczmq.

p4l1ly avatar Apr 02 '20 16:04 p4l1ly