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

Support for F# async workflows #252

Closed
ploeh opened this issue May 30, 2017 · 6 comments
Closed

Support for F# async workflows #252

ploeh opened this issue May 30, 2017 · 6 comments

Comments

@ploeh
Copy link

ploeh commented May 30, 2017

While you can make a Polly circuit breaker work with an F# async workflow, you have to jump through some hoops. Would it be possible to support F# async workflows out of the box?

I don't mind doing a bit of work myself, but I'd appreciate to hear from the Polly core team if this is something that they're interested in, and that they deem it possible.

@reisenberger
Copy link
Member

@ploeh Am currently on the road travelling, but will come back to this and the interesting qs raised in your blog post in a few days' time.

@reisenberger
Copy link
Member

reisenberger commented Jun 8, 2017

Thank you @ploeh for your enthusiasm for Polly with F#! I have taken four main points from your blog post: You added around Polly:

(a) extra exception-matching predicates to match InnerExceptions of AggregateException, in order for the Polly circuit-breaker to handle these

(b) the curried helper-function to execute an async workflow through the Polly policy, translating between Tasks and F# async workflows

(c) a function as (a) but extracting BrokenCircuitException from AggregateException

(d) a function to translate the thrown BrokenCircuitException into a value response


Taking these in a different order:

(d) Polly has an in-built method .ExecuteandCapture(...) which performs the same conceptual function: returns the result of execution always as a PolicyResult value rather than ever throwing an exception. Could this substitute for some of the work you did in (d)?

(a) and (c) We now have several requests for better-handling of InnerExceptions of AggregateException. See this new proposal for how Polly could support this. Comments welcome!

When that proposal is implemented, the in-built .ExecuteandCapture(...) method (d) would unwrap inner exceptions of AggregateException (PolicyResult.FinalException would return the handled inner exception directly), so this could be an advantage of using the in-built .ExecuteAndCapture().


(b) / general F# support for Polly:

Very happy for contributions! I love the functional style (PolicyWrap is inspired by it), but haven't had time to get into F# yet, so I wouldn't be able to undertake or support this at this time.

(Assuming (d) (a) and (c) as dealt with per above) ...
Would you see better F# support for circuit-breaker as involving:

(b-i) providing just your curried helper-function (b) out-of-box as part of the Polly package (or an extension package, eg Polly.FSharpAsync); ?
OR
(b-ii) writing fresh F# implementations of the CircuitBreakerEngine, CircuitController etc. ?

(or something else?)

If (b-i) is sufficient (or even just a good first step / adds enough value on its own), doing that sounds relatively straightforward and certainly useful. Particularly given it seems to provide an F# bridge for all Polly async executions, across all Policy types? (big win).

If (b-ii), happy to discuss further!

Depending on your thoughts to (b-i)/(b-ii) above, we can then think about how best to manage/package/support this.

All input welcome from wider interested parties - I see that several people have 👍 this issue

Thanks!

@ploeh
Copy link
Author

ploeh commented Jun 16, 2017

I was mostly thinking about your item (b), since all the other features can easily be added in client code, if need be. The problem with a function like the following execute function is that it's likely to be inefficient (although, to my chagrin, I must admit that I haven't measured, because I have nothing against which to measure).

let private execute (policy : Policy) f  req =
    policy.ExecuteAsync(fun () -> f req |> Async.StartAsTask) |> Async.AwaitTask

F# async workflows aren't Task objects; there are some differences, the most important of which is that F# async workflows are pure until started, whereas Task instances just starts running in an uncontrollable fashion.

This means that while it's possible to go back and forth between async workflows and the TPL, such transitions aren't free. There's synchronisation and (possibly) marshalling overhead every time one does that.

The above execute function performs two such translations every time it runs.

It'd be nice to have a native Polly API that mirrors CircuitBreakerAsync, ExecuteAsync, and so on, but for F# async workflows instead of tasks. In order to make any difference, however, it'd need to use F# async workflows all the way. If it needs to translate back and forth to tasks deeper within Polly, it hardly makes any difference.

I haven't looked at the Polly code base, so I don't know how realistic this is. Furthermore, AFAIK, all the types involved in F# async workflows are defined in FSharp.Core, and I don't see how to add support for this without taking a dependency on that assembly.

So it may not be desirable, but instead of reaching that conclusion all by myself, at least I thought I'd discuss it in public 😄

@reisenberger
Copy link
Member

reisenberger commented Jul 12, 2017

Thanks @ploeh. That was the answer I thought we would probably come to, based on your earlier comments, and my own reading around F# async workflows. However, likewise, I wanted to have your deeper F# experience speak to that rather than make an assumption 😄

For anyone interested in a port of Polly to F# in future - the implementations of each Polly policy type are defined quite clearly as higher-order functions - as "Funcs on a Func" - very much a functional mindset to Polly in that sense.

Theory: The original definition of a sync Policy was (beautiful to my mind) an Action<Action>, and async a Func<Func<Task>, Task> (now only slightly more complex, as we support context, cancellation, and control over marshalling async context: sync; async).

Practice: Those implementations can be clearly found in the classes named XXXEngine, eg RetryEngine and RetryEngineAsync.

The rest of Polly is simpler concerns such as syntax/builder classes, supporting classes (eg the Context which flows with each execution) - and over 1200 tests.

The exception to above statement that the entire definition of a Policy can be found in an XXXEngine class is circuit-breaker, which is driven by a group of collaborating classes: CircuitBreakerEngine, implementations of ICircuitController, and classes tracking metrics. That would be a slightly bigger port.

Happy to provide further input/support to anybody wanting to port Polly, or just certain concepts, to F#.

@reisenberger
Copy link
Member

Triage of old issues: The core Polly team are not able to contribute this at present. Happy to provide guidance to anyone who may be looking to take this on as a project. As @ploeh says, to take full advantage of F# async workflows would essentially be a full rewrite of async Polly ("In order to make any difference, it'd need to use F# async workflows all the way.").

@reisenberger
Copy link
Member

Huge thanks again @ploeh for this useful exploration of Polly with F# (we've linked your blog post into the Polly readme) and for providing F# users with a means to bridge Polly nicely into F#, handling the AggregateException unwrapping and currying the Task/async-workflow transition.

I am going to close this out as outside of the scope the existing Polly team can deliver (on unfunded time). However, we continue to be happy to support and provide input/guidance to anyone wanting to take this on.

Stepping right back and looking at the bigger picture: there is some impact in using the current Polly asynchronously in F#, due to the swapping back-and-forth between Tasks and F#'s native async workflows. However, in the context of the network access time (HTTP request and response) that most Polly policies are probably used for, hopefully the async to/fro may not have too much impact on overall execution times. I also see that there are ongoing discussions with the F# design team about native F# support for Task.


(On the smaller point of AggregateException, from Polly v5.6.0 we have added native handling of InnerExceptions to Polly; so you may be able to pick up on Polly's native handling there now.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants