clj-commons / manifold

A compatibility layer for event-driven abstractions
1.02k stars 106 forks source link

execution thread for deferred values #156

Closed pyr closed 5 years ago

pyr commented 5 years ago

Hi!

While using aleph in conjunction with alia-manifold, I ran into a surprising behavior which would exhaust the I/O threadpool used by Cassandra's java-driver (which alia-manifold provides a facade to).

I dumbed the issue down to this reproduction:

(ns reproduction.main
  (:require [manifold.deferred     :as d]
            [clojure.tools.logging :refer [warn]])
  (:import java.util.concurrent.Executors))

(def ex
  "A separate executor"
  (Executors/newFixedThreadPool 10))

(defn listenable-request
  "Create a deferred whose success value will be delivered to
   from a thread running in another executor"
  []
  (let [d (d/deferred)]
    (.submit ex #(do (warn "in future") (d/success! d :done)))
    d))

(defn run-handler
  []
  (warn "starting let-flow")
  @(d/let-flow [resp (listenable-request)]
     ;; The warn should happen on the run-handler calling thread
     (warn "response:" resp)
     resp)
  (warn "back from let-flow"))

(defn -main
  [& args]
  @(d/future (run-handler))
  (System/exit 0))

With this, the following output is produced:

;; 15:37:42.121 [manifold-execute-1] WARN  reproduction.main - starting let-flow
;; 15:37:42.123 [pool-1-thread-1] WARN  reproduction.main - in future
;; 15:37:42.124 [pool-1-thread-1] WARN  reproduction.main - response: :done
;; 15:37:42.124 [manifold-execute-1] WARN  reproduction.main - back from let-flow

The surprising part - at least to me - is the fact that the response notification, outside of let-flow's binding vector - would happen on the thread-pool where listenable-request's computation ran.

This seems to stem from the fact that let-flow removes the thread-local bound executor while in the binding vector, and that as a result, the deferred created in listenable request doesn't have any.

I don't exactly get how this maps to having the resulting code stay on the same thread-pool instead of going back to the calling thread (or to the execution-pool).

rborer commented 5 years ago

Hi,

This seems to come from the use of chain usage within let-flow. The body execution is "subscribed" to the deferred within let-flow as we see here:

https://github.com/ztellman/manifold/blob/72a5bd368bbb6a1fe1a3cfe51a95a0139ef7b828/src/manifold/deferred.clj#L819-L825

As such, the execution of the body happens within the same thread as the deferred. That's a behavior I rely upon a lot :wink: .

Now, as to know if this is the expected outcome of let-flow, I cannot judge. Personally, knowing how chain works this doesn't surprise me much.

rborer commented 5 years ago

I've given a second thought about this and I know agree that this behavior is strange and is probably not what is expected.

I've attempted a fix under #157, here is the output I get now:

11:43:35.576 [manifold-execute-1] WARN reproduction.main - starting let-flow
11:43:35.578 [pool-1-thread-1] WARN reproduction.main - in future
11:43:35.580 [manifold-execute-2] WARN reproduction.main - response: :done
11:43:35.580 [manifold-execute-1] WARN reproduction.main - back from let-flow

Obviously we can't reuse the thread manifold-execute-1 since it's already busy waiting on the let-flow execution.