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

Redesign: Create a dora-daemon as a communication broker #162

Merged
merged 230 commits into from
Mar 6, 2023
Merged

Redesign: Create a dora-daemon as a communication broker #162

merged 230 commits into from
Mar 6, 2023

Conversation

phil-opp
Copy link
Collaborator

@phil-opp phil-opp commented Dec 27, 2022 •
edited
Loading

Create a dora-daemon executable to manage communication per machine. The daemon allocates shared memory for messages, which makes it possible to send outputs to local receivers without copying. For remote receivers, the message is copied and sent over the network using some communication layer implementation.

The new process to send outputs looks like this:

image

Since the daemon handles all advanced communication, the nodes no longer need to link the communication layer. This should make building and distribution much simpler since we don't need any C++ dependencies with complex build scripts anymore (e.g. iceoryx).

If the shared memory implementation is fast and flexible enough, we might even be able to unify custom nodes and operators again.

ToDo:

  • Adjust the node APIs for the new design
    • Rust
    • Python
    • C
    • C++
  • Update the examples
  • Update CLI command to work with the new design
    • dora start
    • dora stop
    • dora up
    • dora destroy
  • Adjust the dora-runtime (or remove it?)
  • Integration tests
    • for stopping nodes (tested through the dataflow examples)
    • simulate network disconnects

Performance Improvements:

  • Create performance benchmark (throughput, latency)
  • Use RPC framework instead of custom TCP socket for coordinator <-> daemon <-> node communication
  • Reuse shared memory allocations instead of deallocating them after use
  • Construct metadata directly in node API (instead of daemon)
  • Pass metadata through shared memory
  • Use shared memory for control channels (instead of TCP sockets)
    • Use Event type of raw-sync crate for synchronization

@phil-opp
Start creating a dora-daemon
9b65739
@phil-opp
Move port number constant to dora-core
f7c6cca
@phil-opp
Connect to dora-daemon from node API and register with node ID
7d85b92
@phil-opp
Update Rust node API to communicate through dora-daemon
c2e4948
@phil-opp
Add support to subscribe to messages in daemon
e36f15e
@phil-opp
Remove communication layer from dora-node-api
201fd22
@phil-opp
Comment out uses of communication layer in coordinator for now
1d2d71b
@phil-opp
Update nodes of rust example to new API
d01dc38
@phil-opp
Implement shared memory mapping in node API
2e7ef8b
@phil-opp
Define new message types
4d29192
@phil-opp
Listen for daemon messages in coordinator
c58745b
@phil-opp
Connect to dora-coordinator and implement command to spawn nodes
3312af1
@phil-opp
Update Rust node API to parse new NodeConfig
9a9394c
@phil-opp
Bundle all nodes of a single machine in a single daemon spawn command
e5d7ac5 
Avoids synchronization issues, e.g. if one node sends outputs before the receiver was launched and registered.
@phil-opp
Include dataflow ID in daemon to avoid node ID conflicts
5ae18ea
@phil-opp
Update dora-coordinator to start dataflows through dora-daemon
cc1dc97
@phil-opp
Merge branch 'main' into unify-nodes-and-operators
433b7b4
@phil-opp
Add dataflow ID to daemon messages
0ae5fc2
@phil-opp
Map outputs to inputs of local nodes
a0d0b21
@phil-opp
Also treat BrokenPipe error as stopped connection
1724c86
@phil-opp
Send metadata through TCP for now
ce4e5a1
@phil-opp
Close connections on errors
672fb4d
@phil-opp
Implement timer messages
a10fd0b
@phil-opp
Notify downstream nodes about closed inputs
3cd1b4d
@phil-opp
Report results of spawned nodes as events
f0e31e2
@phil-opp
Cancel timer tasks once a dataflow is finished
3c4469d
@phil-opp
Allow replacing a daemon connection in coordinator
692595d 
The previous connection might be closed already. It's difficult to check for that without sending any actual data, so we just permit re-registering of daemon connections. Previous connections are closed.
@phil-opp
Set TCP_NODELAY for all TCP connections to reduce latency
8bfb665
@phil-opp
Make events a borrowed type and report to daemon when they're dropped
8225c9f
@phil-opp
Use drop tokens and reference counting to free shared memory again af…
a2cc06c 
…ter usage
@phil-opp
Copy link
Collaborator Author

phil-opp commented Mar 1, 2023

@haixuanTao I found and fixed a deadlock that happened when the dora node was dropped before the event stream in the Rust node API. Since the other APIs build on top of the Rust API, it affected them too.

@phil-opp
Fix: don't error if the elapsed time is negative (which can happen on…
3ad6150 
… some systems)
@phil-opp
CI: Run CLI commands in single step
6462d85
@phil-opp
CI: Fix command order in CLI test job
8670d85
@phil-opp
Flush TCP connections on sending in coordinator and daemon too
4bbb724
@phil-opp
Wait for spawned listener tasks before exiting coordinator
97dab13 
Ensures that we don't exit while some listener task is still sending out a reply.
@phil-opp
Wait for control connections to finish too
aae4a66
@phil-opp
Fix: Move InputBuffer break condition to end of loop
9e4f013 
Fixes a deadlock issue when the `incoming` channel is closed first. The problem was that the `send_out_buf` was set to `Fuse::terminated` without breaking the loop, so on the next loop iteration both futures were already terminated.
@phil-opp
Use problem matcher and cache for CLI tests too
a2a6dba
@phil-opp
Add support for shell-based nodes again
b994ab6
@phil-opp
Copy link
Collaborator Author

phil-opp commented Mar 6, 2023

I discussed this PR with @haixuanTao. We believe that all the critical bugs are fixed now. So we decided to go ahead and merge this PR, once the CI is green. We will handle the other open tasks and the possible performance improvements in follow-up PRs.

@phil-opp phil-opp enabled auto-merge March 6, 2023 11:28
@phil-opp phil-opp disabled auto-merge March 6, 2023 11:50
@phil-opp
Copy link
Collaborator Author

phil-opp commented Mar 6, 2023

Looks like we have some stop issues on Windows now. I'll look into it.

@phil-opp
Mark operator Event type as non_exhaustive
1213d70
@phil-opp
Send InputClosed events to operators too
7232a31
@phil-opp
Use more reliable InputClosed event instead of timer as exit condition
bb8d6ed 
We might not receive any `random` input if the startup of the operator is delayed and the source node is already finished.
@phil-opp
Fix: Update open_inputs even if there is no subscriber for closed i…
d5ceb44 
…nput
@phil-opp
Fix: Keep track of InputClosed and Stop messages and send them on…
9439b08 
… subscribe

These two messages can be essential for correctness. For example, a node might not finish properly when an `InputClosed` event is lost. So we need to always send them, even if the target node was not subscribed yet when the event occurred.
@phil-opp
Merge branch 'main' into unify-nodes-and-operators
6af931f
@phil-opp phil-opp merged commit 37fc372 into main Mar 6, 2023
@phil-opp phil-opp deleted the unify-nodes-and-operators branch March 6, 2023 17:21
This was referenced Mar 6, 2023
Open
Closed
Closed
@haixuanTao haixuanTao mentioned this pull request Mar 9, 2023
Closed
@phil-opp phil-opp mentioned this pull request Mar 24, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haixuanTao haixuanTao haixuanTao approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@phil-opp @haixuanTao