-
Notifications
You must be signed in to change notification settings - Fork 6
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
Implement the ZeroMQ transport #27
base: master
Are you sure you want to change the base?
Conversation
4ebc035
to
85af8a6
Compare
50647e8
to
0f5c529
Compare
When a `Connected` input port receives a `CloseOutput` event for for the last output port ID switch back to the `Open` state.
The conditions for the `loop { tokio::select!( ... ) }` to exit are: 1. All the references to `req_send` channel are dropped. This channel is used by the public transport interface to close the port. It clones the `Sender` and `send().awaits` on it. Once all of those have been processed, and the port is in a `Closed` state, there are no other references to the `req_send` port than the one removed in this commit. 2. All the references to `to_worker_send` channel are dropped. When receiving a `Message`, the `SubSocket` worker clones this `Sender` from an input worker's **Connected** state. Therefore if the input worker is in the **Closed** state, all references to the `Sender` will be dropped once the outstanding messages from the socket have been sent. Thus, by dropping the reference to the `req_send` channel inside the input port worker's thread, and after it processes the outstanding sends, no other thread is available to get a reference to the `Sender` channels and the loop is able to exit cleanly
To run tests with full tracing output:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently CI fails due to this PR's new dependency on |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nit-picks that I've noticed while I was fixing CI. Didn't look thoroughly at the rest of the code but at the first glance looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is auto-generated. Maybe it makes more sense to put it in generated/
-like directory and add it to .gitignore
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I explicitly configure prost-build
in build.rs
to output the generated files to src/
as by default they go to the build directory (target/.../
) and are imported with include!(concat!(env!("OUT_DIR"), "/your.proto.package.rs"))
.
So unless we have a preference against it, I'd place this in src/
and commit the generated file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No description provided.