Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot use single-socket devices anymore #1428

Closed
minrk opened this issue Jun 5, 2015 · 11 comments · Fixed by #1433
Closed

Cannot use single-socket devices anymore #1428

minrk opened this issue Jun 5, 2015 · 11 comments · Fixed by #1433

Comments

@minrk
Copy link
Member

minrk commented Jun 5, 2015

I've used zmq_device with a single socket for heartbeating and other ping-type activities. In ≥ 4.0.6, this is no longer possible due to the new check on POLLOUT prior to relaying messages.

For example:

s = socket(REP)
zmq_proxy(s, s) # long ago, this was zmq_device(ZMQ_FORWARDER, s, s)

POLLOUT will not be set prior to receiving the first message, which means no messages will ever be delivered.

This is a backward-incompatible change between zeromq-4.0.5 and 4.0.6.

@hintjens
Copy link
Member

hintjens commented Jun 5, 2015

To be fair, the single-socket behavior isn't documented and isn't covered
by any test code. This is our guideline for defining backwards
compatibility.

On Fri, Jun 5, 2015 at 11:22 PM, Min RK notifications@github.com wrote:

I've used zmq_device with a single socket for heartbeating and other
ping-type activities. In ≥ 4.0.6, this is no longer possible due to the new
check on POLLOUT prior to relaying messages.

For example:

s = socket(REP)
zmq_proxy(s, s) # this used to be zmq_device(ZMQ_FORWARDER, s, s)

POLLOUT will not be set prior to receiving the first message, which means
no messages will ever be delivered.

This is a backward-incompatible change between zeromq-4.0.5 and 4.0.6.


Reply to this email directly or view it on GitHub
#1428.

@minrk
Copy link
Member Author

minrk commented Jun 5, 2015

@hintjens it's been in the pyzmq test suite for a few years, so I cannot update pyzmq to work with libzmq > 4.0.5 until this is fixed.

@minrk
Copy link
Member Author

minrk commented Jun 5, 2015

I'll work on a patch to fix libzmq, but would it be possible to revert the POLLOUT check for 4.0.x, so I have a little more time to figure out a workaround for supporting 4.1?

@hintjens
Copy link
Member

hintjens commented Jun 6, 2015

OK, a few thoughts here.

First, it should be trivial to fix (if frontend == backend, don't expect
POLLOUT).

Second, the same problem should be affecting 4.1 and libzmq, so 4.0.x may
be the wrong place to be discussing it.

Third, this change sat on zeromq4-x master for over a month... I'm puzzled
as to how changes that break downstream (aside from whether downstream is
using documented functionality or not) can remain undetected so long.

So I assume you're not running the PyZMQ test suite on the masters, only on
stable releases. If so, that's extremely fragile. It's almost guaranteed to
give this outcome.

Other bindings like CZMQ run Travis CI on the git masters of libzmq,
zeromq4-1, zeromq4-x, zeromq3-x. This ensures that a breaking change (from
the PoV of CZMQ) gets caught immediately. We never need to git bisect and
we never have surprises on stable releases.

If it's impossible to run the PyZMQ tests like this, then I'd suggest that
any undocumented or other surprising use of the API be (a) documented in
libzmq and (b) tested in the libzmq test cases. There is no other
reasonable definition of "compatibility".

On Sat, Jun 6, 2015 at 12:23 AM, Min RK notifications@github.com wrote:

I'll work on a patch to fix libzmq, but would it be possible to revert the
POLLOUT check for 4.0.x, so I have a little more time to figure out a
workaround for supporting 4.1?


Reply to this email directly or view it on GitHub
#1428 (comment).

@minrk
Copy link
Member Author

minrk commented Jun 7, 2015

First, it should be trivial to fix (if frontend == backend, don't expect
POLLOUT).

If that's the right fix, I'll put together a patch.

Second, the same problem should be affecting 4.1 and libzmq, so 4.0.x may
be the wrong place to be discussing it.

I was mainly talking about 4.0 as a temporary measure, in case the behavior should stay exactly as-is in 4.1, libzmq. If the above fix makes sense, then definitely libzmq is the place to do it, and let it propagate to 4.0, 4.1 as usual.

Third, this change sat on zeromq4-x master for over a month... I'm puzzled
as to how changes that break downstream (aside from whether downstream is
using documented functionality or not) can remain undetected so long.

And it's been causing failures in pyzmq for all that time, but I don't always have time to take action on failures caused by libzmq master, since it's frequently broken and usually fixed promptly.

@minrk
Copy link
Member Author

minrk commented Jun 7, 2015

Proposed fix and test in #1433.

@minrk
Copy link
Member Author

minrk commented Jun 7, 2015

Running tests with libzmq 4-x, 4-1, etc. is a good idea, I'll add those to the pyzmq grid. I already run the tests with libzmq-master, but I don't consider that stable, and don't expect it to always be passing. The maintenance branches I should keep a more careful eye on, and will in the future.

@hintjens
Copy link
Member

hintjens commented Jun 7, 2015

I don't know if this is the right fix or not, without a test case, though I
assume it'll put things back as they were for that specific case.

Testing against the maintenance branches does help; any failures on those
are a red flag, typically that we've short-cut the process and/or our tests
aren't covering everything.

libzmq does aim to be stable so if you are getting failures downstream in
the PyZMQ test suite, it'd be good to know about them sooner, rather than
later. I mean, if a new patch breaks things, the faster we learn about it
the easier it is to fix.

On Sun, Jun 7, 2015 at 6:20 AM, Min RK notifications@github.com wrote:

Running tests with libzmq 4-x, 4-1, etc. is a good idea, I'll add those to
the pyzmq grid. I already run the tests with libzmq-master, but I don't
consider that stable, and don't expect it to always be passing. The
maintenance branches I should keep a more careful eye on, and will in the
future.


Reply to this email directly or view it on GitHub
#1428 (comment).

@minrk
Copy link
Member Author

minrk commented Jun 7, 2015

#1433 includes the test case that was failing on master.

@hintjens
Copy link
Member

@minrk FYI I've cut 4.1.2 and 4.0.7 packages.

@minrk
Copy link
Member Author

minrk commented Jun 15, 2015

@hintjens great, thanks!

mrvn pushed a commit to mrvn/libzmq that referenced this issue Jul 2, 2015
bluca pushed a commit that referenced this issue Oct 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants