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

SubstreamCancelStrategy is redundant and confusing considering existence of SupervisionStrategy attribute #31394

Open
mdedetrich opened this issue May 10, 2022 · 0 comments
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:stream

Comments

@mdedetrich
Copy link
Contributor

mdedetrich commented May 10, 2022

When using Akka-Streams its currently documented that if you want to control the behaviour of the stream in the event of an exception you are should use a supervision strategy as part of the attributes, see https://doc.akka.io/docs/akka/current/stream/stream-error.html. While for this works as expected in most cases there is surprising results when you use Flow operations that Split the stream (such as splitAfter).

The surprising part comes down to the fact that it completely ignores the configured SupervisionStrategy and instead asks for a redundant SubstreamCancelStrategy parameter. The reason why this is redundant is that in terms of effect/result SubstreamCancelStrategy.propogate maps to Supervision.stoppingDecider and SubstreamCancelStrategy.drain maps to SupervisionStrategy.resume/SupervisionStrategy.restart so In other words there doesn't seem to be any benefit in doing it this way.

This is quite surprising because generally speaking if you have a stream represented as a RunnableGraph as mentioned before there is an expectation that you would do a

stream.withAttributes(ActorAttributes.supervisionStrategy(Supervision.stoppingDecider)).run()

And strategy propagates to all parts of the stream for all parts of the stream where its possible. Specifically in context of the problem I am dealing with now, i had a custom SupervisionStrategy as shown below

val decider: Supervision.Decider = { e =>
  logger.error("Unhandled exception in stream", e)
  Supervision.Stop
}

In the case of an exception being thrown in the sink defined in splitAfter, since this supervision strategy is ignored and the default SubstreamCancelStrategy is SubstreamCancelStrategy.drain not only did no exception get logged (so we had no idea what was going on) but it started draining the stream where as the expectation was that it should have cancelled the stream completely and logged the exception (in my case the exception was actually incorrect credentials for S3 which was the passed Sink so rather than immediately stopping the stream instead we spammed S3 with thousands of requests per second as we iterated through a large stream!).

Would deprecating the SubstreamCancelStrategy.propogate parameter and instead use the existing SupervisionStrategy from attributes be a reasonable course of action?

@mdedetrich mdedetrich changed the title SubstreamCancelStrategy is redundant and confusing considering existence of SupervisionStrategy SubstreamCancelStrategy is redundant and confusing considering existence of SupervisionStrategy attribute May 10, 2022
@johanandren johanandren added 1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:stream labels May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - triaged Tickets that are safe to pick up for contributing in terms of likeliness of being accepted t:stream
Projects
None yet
Development

No branches or pull requests

2 participants