Skip to content

Add support for stream cloning#203

Closed
goodboy wants to merge 1 commit into
masterfrom
stream_clones
Closed

Add support for stream cloning#203
goodboy wants to merge 1 commit into
masterfrom
stream_clones

Conversation

@goodboy

@goodboy goodboy commented Mar 2, 2021

Copy link
Copy Markdown
Owner

Leverage trio's built-in rx channel cloning with our ReceiveStream to enable multi-consumer usage.

Comment thread tractor/_portal.py
return

# close the local mem chan
rx_chan.close()

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This likely needs to be put above.

Thanks to @richardsheridan for catching 🏄🏼

@goodboy

goodboy commented Apr 5, 2021

Copy link
Copy Markdown
Owner Author

Heh, so this doesn't even work remotely as planned 😂

Turns out receive mem chans do not support broadcasting in trio:

The clones all share the same underlying channel. Whenever a clone receive()s a value, it is removed from the channel and the other clones do not receive that value. If you want to send multiple copies of the same stream of values to multiple destinations, like itertools.tee(), then you need to find some other solution; this method does not do that.

This actually ended up being a source of major confusion in pikers/piker#158 where this PR's change to ReceiveStream.aclose() was actually preventing far end producer cancellation and then a very hard to debug back-pressure instigated lockup 😂

So basically I think we need to drop this idea wholesale (at least if we're sticking with clone as it has meaning in trio) and instead venture into what we're actually after: broadcast channels as in python-trio/trio#987.

@goodboy

goodboy commented Apr 30, 2021

Copy link
Copy Markdown
Owner Author

See also #53 for further discussion on how we don't want trio's .clone() semantics (since it isn't useful cross-process).

@goodboy goodboy closed this Apr 30, 2021
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 this pull request may close these issues.

1 participant