HttpClient's concurrency limits should be documented and its error messages could use some context

[chriskiehl]

Howdy Cognitect!

I hit an unexpected limit with aws-api which took a decent amount of time to debug due to the vagueness of the error message (detailed below). I'm opening this issue to suggest that

• HttpClient's pending_ops limit be noted in the aws-api README/docs
• HttpClient's Ops limit reached message include a namespace, a stack trace, or some kind of context to make it clear that it is the source of the error.

Dependencies

{:deps {com.cognitect.aws/api       {:mvn/version "0.8.352"}
        com.cognitect.aws/endpoints {:mvn/version "1.1.11.636"}
        com.cognitect.aws/s3        {:mvn/version "746.2.533.0"}}}

Description with failing test case

Calling (aws/invoke client op-map) concurrently in a pool causes cryptic errors related to "Ops Limit Reached" if you invoke more than 64 concurrent requests.

Error Map:

#:cognitect.anomalies{:category :cognitect.anomalies/busy, :message "Ops limit reached: 64"}

The source of this message is ultimately HttpClient.

Line ~214-221 /cognitect/http_client.clj

(submit*
   [_ request ch]
   (if (< pending-ops-limit (swap! pending-ops inc))
     (do
       (put! ch (merge {:cognitect.anomalies/category :cognitect.anomalies/busy
                        :cognitect.anomalies/message (str "Ops limit reached: " pending-ops-limit)}
                       (select-keys request [::meta])))
       (swap! pending-ops dec))

How to Reproduce:

Any slow AWS endpoint will work, but for consistency, best bet is to set up a simple lambda function so you can control the execution time.

The default Lambda python template with a 5 time.sleep (to simulate work) does the trick.

import json
import time 

def lambda_handler(event, context):
    # TODO implement
    time.sleep(5)
    return {
        'statusCode': 200,
        'body': json.dumps('Hello from Lambda!')
    }

Calling the Lambda concurrently from a pool

(require '[cognitect.aws.client.api :as aws])

(import '(java.util.concurrent ForkJoinPool Executors))

(def lambda (aws/client {:api :lambda}))

(let [concurrent-requests 65
      pool (Executors/newFixedThreadPool concurrent-requests)
      tasks (map (fn [_]
                   (fn []
                     (aws/invoke
                       lambda
                       {:op :Invoke
                        :request {:FunctionName "SlowPythonCall"
                                  :Payload (json/write-str {:hello "world"})}})))
                   (range concurrent-requests))]
  (let [futures (.invokeAll pool tasks)]
    (.shutdown pool)
    (mapv #(.get %) futures)))

If you dial concurrent-request down to 64, then no problemo. 65+ and the error pops due to HttpClient.

That's pretty much it. Surprising enough that it took a lot of looking at the wrong things before I was able to track down the source of the problem.

Work Around:

It's possible to work around this by moving the creation of the client into the call to invoke, that way every execution gets its own instance and spamming calls to Lambda can resume!

(aws/invoke
  (aws/client {:api :lambda})
  {:op ...}

Stack traces

No stack trace is produced as part of this.

[dchelimsky]

Hey @chriskiehl. We just released 0.8.430, which introduces a shared http-client that all aws-clients use by default. I added a section about this to the README, and you can check the upgrade notes for more detail.

This change is breaking for your workaround. For now, you'll need to create a new instance of http-client for each aws-client, e.g. (aws/client {:api :lambda :http-client (cognitect.aws.client.api/default-http-client)}). In the future, we'd like to provide more control over the http client to the user, but design for that is still in progress.

[chriskiehl]

Awesome! Thanks for all your work on this library! ^_^

[dchelimsky]

Thank @ghadishayban too! He's really the brains behind this release. I mostly just typed stuff ;)