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.

Sign up for GitHub

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

Jump to bottom

zmq: implement REQ-REP + PUB-SUB comms #3329

Open
oliver-sanders opened this issue Aug 29, 2019 · 8 comments
Open

zmq: implement REQ-REP + PUB-SUB comms #3329

oliver-sanders opened this issue Aug 29, 2019 · 8 comments
Assignees
@dwsutherland
Milestone
8.x

Comments

@oliver-sanders
Copy link
Member

Issue migrated from #2978

At present the suite runtime API is REQ-REP.

  1. Client sends a request.
  2. Server sends a response.

This pattern works fine for information requests, but for mutations the client usually just receives an unhelpful "command queued" message. There is no mechanism to communicate command output to the user.

To solve this we could add a PUB-SUB pattern on-top of the REQ-REP whereby:

  1. Client sends a request.
  2. Server sends a subscription ID.
  3. Client subscribes to the subscription.
  4. Server publishes updates until the command has completed.

From the client side this might look like:

SEND stop_suite
RECV <subid>
RECV <subid> "command queued"
RECV <subid> "waiting for X active tasks to complete before"
RECV <subid> "waiting for suite shutdown handlers to complete"
RECV <subid> "shutting down"
RECV <subid> "command succeeded"

This pattern should be used for mutations (and not for information requests).
@oliver-sanders oliver-sanders added this to the cylc-8.0.0 milestone Aug 29, 2019
This was referenced Aug 29, 2019
Closed
Open
@TomekTrzeciak
Copy link
Contributor

Relevant for #2798.

@oliver-sanders oliver-sanders mentioned this issue Jan 28, 2020
Merged
13 tasks
@oliver-sanders oliver-sanders mentioned this issue Feb 5, 2020
Merged
10 tasks
@dwsutherland
Copy link
Member

PUB/SUB has been merged as part of the WSchd <=> UIS data store sync PR .. So available as an option for mutation feedback

@oliver-sanders oliver-sanders mentioned this issue Jun 9, 2020
Closed
@oliver-sanders
Copy link
Member Author

See the older #423 which proposes much the same thing.

@oliver-sanders oliver-sanders mentioned this issue Dec 15, 2020
Open
@oliver-sanders oliver-sanders mentioned this issue Mar 4, 2021
Open
2 tasks
@oliver-sanders oliver-sanders mentioned this issue Apr 16, 2021
Closed
@dwsutherland
Copy link
Member

dwsutherland commented Aug 4, 2021 •
edited
Loading

See #4274 (which aims to coordinate these two patterns)

@hjoliver
Copy link
Member

hjoliver commented Aug 4, 2021

Does that close this?

@dwsutherland
Copy link
Member

Does that close this?

Yes, it does.

@dwsutherland dwsutherland mentioned this issue Aug 4, 2021
Merged
9 tasks
@hjoliver
Copy link
Member

hjoliver commented Aug 4, 2021 •
edited
Loading

(My bad - I thought I looked and the two issues hadn't been connected)

Oh, they weren't, you just edited the description to add the link 👍

@oliver-sanders oliver-sanders mentioned this issue Aug 13, 2021
Open
@oliver-sanders oliver-sanders modified the milestones: cylc-8.0rc1, cylc-8.x Nov 9, 2021
@oliver-sanders oliver-sanders mentioned this issue Nov 9, 2021
Merged
7 tasks
@oliver-sanders oliver-sanders mentioned this issue Mar 9, 2022
Merged
7 tasks
@oliver-sanders oliver-sanders mentioned this issue Mar 28, 2022
Open
@oliver-sanders oliver-sanders mentioned this issue Jan 25, 2023
Merged
6 tasks
This was referenced May 15, 2023
Merged
Open
@MetRonnie MetRonnie mentioned this issue Jun 20, 2023
Open
@oliver-sanders oliver-sanders mentioned this issue Sep 26, 2023
Merged
16 tasks
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue May 23, 2024
@oliver-sanders
commands:
63a4f85 
* Separate scheduler commands from the main scheudler code.
* Integrate command validators in with the commands themselves.
* Make all commands async and provide a blank validator for each
  (i.e. add a yield at the top of each function).
* Partially addresses cylc#3329 by making commands generators capible of
  returning multiple messages.
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue May 23, 2024
@oliver-sanders
commands:
cf45c66 
* Separate scheduler commands from the main scheudler code.
* Put all commands behind a decorator to protect against leakage.
  (e.g. an import cannot be mistaken for a command/validator).
* Integrate command validators in with the commands themselves.
  This removes the duplicate command/validate function signatures and
  makes validators implicit (i.e. they are called as part of the command
  not searched for and called separately to the command).
* Make all commands async and provide a blank validator for each
  (i.e. add a yield at the top of each function). Note this means
  that command args/kwargs are now validated as part of queueing the
  command itself by default.
* Partially addresses cylc#3329 by making commands generators capible of
  returning multiple messages.
@oliver-sanders oliver-sanders mentioned this issue May 23, 2024
Merged
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue May 24, 2024
@oliver-sanders
scheduler commands: refactor
e07b6f5 
* Separate scheduler commands from the main scheudler code.
* Put all commands behind a decorator to protect against leakage.
  (e.g. an import cannot be mistaken for a command/validator).
* Integrate command validators in with the commands themselves.
  This removes the duplicate command/validate function signatures and
  makes validators implicit (i.e. they are called as part of the command
  not searched for and called separately to the command).
* Make all commands async and provide a blank validator for each
  (i.e. add a yield at the top of each function). Note this means
  that command args/kwargs are now validated as part of queueing the
  command itself by default.
* Partially addresses cylc#3329 by making commands generators capible of
  returning multiple messages.
@oliver-sanders
Copy link
Member Author

It may be possible to achieve this on a single socket using router-dealer or some fancy combination of req-rep and router-dealer (see https://zguide.zeromq.org/docs/chapter3/), but this will require investigation.

Note that req-rep is synchronous (e.g. "kill" followed by "trigger" will be queued in that order), whereas router-dealer is asynchronous ("kill" followed by "trigger" could be queued in either order) so we may need to leverage both (because queue order is somewhat important to us, although we have never claimed to guarantee it).

Adding a new socket is not desirable (port efficiency matters), ideally we would look to consolidate the pub-sub socket too (see also xpub-xsub).

oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue May 30, 2024
@oliver-sanders
scheduler commands: refactor
e01ceda 
* Separate scheduler commands from the main scheduler code.
* Put all commands behind a decorator to protect against leakage.
  (e.g. an import cannot be mistaken for a command/validator).
* Integrate command validators in with the commands themselves.
  This removes the duplicate command/validate function signatures and
  makes validators implicit (i.e. they are called as part of the command
  not searched for and called separately to the command).
* Make all commands async and provide a blank validator for each
  (i.e. add a yield at the top of each function). Note this means
  that command args/kwargs are now validated as part of queueing the
  command itself by default.
* Partially addresses cylc#3329 by making commands generators capable of
  returning multiple messages.
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue May 30, 2024
@oliver-sanders
scheduler commands: refactor
b5bb456 
* Separate scheduler commands from the main scheduler code.
* Put all commands behind a decorator to protect against leakage.
  (e.g. an import cannot be mistaken for a command/validator).
* Integrate command validators in with the commands themselves.
  This removes the duplicate command/validate function signatures and
  makes validators implicit (i.e. they are called as part of the command
  not searched for and called separately to the command).
* Make all commands async and provide a blank validator for each
  (i.e. add a yield at the top of each function). Note this means
  that command args/kwargs are now validated as part of queueing the
  command itself by default.
* Partially addresses cylc#3329 by making commands generators capable of
  returning multiple messages.
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Jun 6, 2024
@oliver-sanders
scheduler commands: refactor
24adfe0 
* Separate scheduler commands from the main scheduler code.
* Put all commands behind a decorator to protect against leakage.
  (e.g. an import cannot be mistaken for a command/validator).
* Integrate command validators in with the commands themselves.
  This removes the duplicate command/validate function signatures and
  makes validators implicit (i.e. they are called as part of the command
  not searched for and called separately to the command).
* Make all commands async and provide a blank validator for each
  (i.e. add a yield at the top of each function). Note this means
  that command args/kwargs are now validated as part of queueing the
  command itself by default.
* Partially addresses cylc#3329 by making commands generators capable of
  returning multiple messages.
* Improve interface for multi-workflow commands:
  - Colour formatting for success/error cases.
  - One line per workflow (presuming single line output).
  - Exceptions in processing one workflow's response caught and logged
    rather than interrupting other workflow's output.
  - Commands exit 1 if any workflow returns an error response.
* Add multi-workflow test for "cylc broadcast".
@oliver-sanders oliver-sanders mentioned this issue Oct 10, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dwsutherland dwsutherland

Labels
None yet
Projects
None yet
Milestone
8.x
Development

No branches or pull requests
4 participants
@hjoliver @TomekTrzeciak @dwsutherland @oliver-sanders