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

Cancellation API as a first-class citizen? #167

Open
kachayev opened this issue Jan 28, 2019 · 4 comments
Open

Cancellation API as a first-class citizen? #167

kachayev opened this issue Jan 28, 2019 · 4 comments

Comments

@kachayev
Copy link
Collaborator

kachayev commented Jan 28, 2019

The idiomatic way to "cancel" deferred in Manifold is to put an error value in order to short-circuit all chained listeners. Recently we had quite a few questions around this specific functionality... So, I wonder if it's better to have a public API in place to deal with cancellation? E.g.

;; define an exception class to be used in case we want to cancel a deferred
(d/CancelledException.)

;; equivalent to (d/error! d (d/CancelledException.)) 
(d/cancel! d)

;; or if I want to use my own subclass
(d/cancel! d (CustomCancelledException.))

;; to check if it was cancelled
(d/cancelled? d)

We also need to document that cancellation of d/future or d/future-with does not interrupt the underlying thread (which would be expected because of future-cancel semantic from Clojure core).

More advanced thins in terms of cancellations:

  1. Versions of d/zip and d/alt that propagate cancellation to underlying deferreds.
(let [ring1 (http/get "rings")
      ring2 (http/get "rings")
      ring3 (http/get "rings")
      one-ring-to-rule-them-all (d/zip-cancellable ring1 ring2 ring3)]
  ;; cancel all 3 http requests, not only the result
  (d/cancel! one-ring-to-rule-them-all))

It might be simple for 2 mentioned functions... but maybe we can think of a more general approach/solution where we can manually specify (or automatically derive?) a graph of connections between different deferred to use it to propagate CancelledExceptions properly?

  1. Keeping in mind previous item... should we treat TimeoutException as a cancellation?

  2. Self-referencial cancelled? checker for d/future. That's arguable, but often times I need this:

(d/future
  (dotimes [_ 100]
    (when-not (am-I-cancelled?)
      (do-something-useful)))) 

I'm not sure about this specific feature as it undercovers underlying mechanics to some extent. It's still doable by introducing a separate deferred in a lexical scope, but this approach might not be obvious for beginners. So, at least it should be well-documented.

@ztellman WDYT?

@kachayev kachayev changed the title Unified cancellation API? Cancellation API as a first-class citizen? Jan 28, 2019
@ztellman
Copy link
Collaborator

The short-circuiting also allows us to do something like this:

(let [r-a (query-a)
        r-b (query-b)]
  (d/connect r-a r-b)
  r-b)

This means that r-b represents the first query to complete, and if r-a comes in first we'd ideally like query-b to stop doing unnecessary work. I think it's very possible no one's doing this, and most of the use cases reflect the sort of behavior your API encodes, but I definitely don't want to have a special case cancelled? predicate, because realized? is really the important question to be asking.

@kachayev
Copy link
Collaborator Author

I've been thinking about cancelled? not in terms of another realized? predicated but mostly as a way for the user to distinguish error from cancellation: let's say I want to log errors but I don't need to do log anything if I was simply canceled (at least that's not my job to check why that happens, someone who's responsible for cancellation should do this).

I think they don't use d/connect because of 2 reasons:

  1. The approach is not that obvious. Even having some experience with the library.

  2. In practice you probably need to rely on the results of both queries, so you will end up doing d/zip or d/alt at some point.

Regarding "cancellable" d/zip... I can think of at least 2 different use cases that I have in practice:

  1. Cancel the result of d/zip as a way to say "I don't need this thing any longer". I would want the library to take about canceling all related deferreds.

  2. "Fail-fast" version of d/zip: cancel all related deferreds when one of them failed. It's a pretty common practical situation: I fired a bunch of queries, one of them failed (for any reason), I know that I have no way to finish my computation, so I would like to stop the entire graph of computations right now.

I think those exhaust the list of potential use cases which are not covered by short-circuiting of chained listeners. But maybe I'm missing something.

@prepor
Copy link

prepor commented Jan 29, 2019

I think those exhaust the list of potential use cases which are not covered by short-circuiting of chained listeners. But maybe I'm missing something.

As I mentioned in #166 current implementation doesn't cover even chain use cases, for example, nested chains.

@kachayev
Copy link
Collaborator Author

@prepor This one?

(->
   (deferred/chain
     (deferred/chain
       (deferred/future (Thread/sleep 2000))
       (fn [_]
         (prn "----FINISH")
         :finished))
     (fn [_] :finished))
   (deferred/timeout! 1000)
   (deref))

In this example printing of "FINISH" is expected as d/timeout! in case of the error short-circuits all subsequent listeners. E.g.

(->
 (d/chain
  (d/chain
   (d/future (Thread/sleep 2000))
   (fn [_]
     (prn "----FINISH")
     :finished))
  (fn [_] :finished))
 (d/timeout! 1000)
 (d/chain (fn [_] (prn "after timeout")))
 (deref))

"after timeout" will never be printed.

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

4 participants