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

Redesign ProducerJob extension of Job #127

Closed
elizarov opened this issue Sep 18, 2017 · 3 comments
Closed

Redesign ProducerJob extension of Job #127

elizarov opened this issue Sep 18, 2017 · 3 comments

Comments

@elizarov
Copy link
Contributor

In the current design produce coroutine builder returns an instance of ProducerJob interface that extends ReceiveChannel and Job interfaces. The former gives access to all the channel operations, while the later contains Job-management API. Unfortunately, the Job is a CoroutineContext.Element and has operations like fold that operate on coroutine contexts. It clashes with the proposed extensions on ReceiveChannel that treats it as a sequence of elements with fold on them having a different meaning. See discussion in #88 for details.

So the proposal is deprecate ProducerJob and replace it with Producer interface that is a ReceiveChannel (to enable convenient pipelining) and has job as a property for cases where it is needed.

Now, the most common use-case for which you might need to access a producer's job is to cancel it when you don't need it. To make it easier, the proposal is to add cancel operation on ReceiveChannel interface directly. The semantics of cancel for a ReceiveChannel if that it signals that no more elements are going to be received from this channel and the producer of the channel must abort if it is still running (send will throw CancellationException). All the built-in pipelining operations like filter, map, etc on ReceiveChannel (see #88) are going to always cancel the receive channel when they are done (even if they crash), so this way an operational pipeline that starts with produce { ... } and continues with one of those pipelining operations will always cancel the producer without a risk of a producer leak if something crashes downstream.

@fvasco
Copy link
Contributor

fvasco commented Sep 18, 2017

Analogously to Java's Channel and Reactive Streams maybe we should introduce a Disposable interface.

What is the purpose of job attribute in case of a plain Channel?
Is the above proposed interface enough?

@elizarov
Copy link
Contributor Author

We will not have job on a plain channel. Only Producer will have it, since it is produced by a single coroutine. However, the is an alternative design where every channel has a producer job whose life-time is bound to the channel's life-time and this job is cancelled when the channel is closed.

@fvasco
Copy link
Contributor

fvasco commented Sep 18, 2017

I looked better,
ReceiveChannel should implement the DisposableHandle, in such case becomes valid:

job.disposeOnCompletion(channel)

More in genera: Job and Channel are both disposable, so might be helpful review some design choices about "Disposable" interface.

elizarov added a commit that referenced this issue Nov 15, 2017
dropped/deprecated ActorJob/ProducerJob, fixes #127
elizarov added a commit that referenced this issue Nov 16, 2017
all operators on ReceiveChannel fully consume the original channel
using a helper consume extension, which is reflected in docs;
removed `suspend` modifier from intermediate channel operators;
consistently renamed channel type param to <E>;
added two versions for xxxTo fun -- with MutableList & SendChannel;
added tests for all channel operators;
dropped/deprecated ActorJob/ProducerJob, fixes #127
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

No branches or pull requests

2 participants