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

start-buckets! creates more threads than necessary #29

Closed
namenu opened this issue Apr 7, 2023 · 4 comments
Closed

start-buckets! creates more threads than necessary #29

namenu opened this issue Apr 7, 2023 · 4 comments

Comments

@namenu
Copy link

namenu commented Apr 7, 2023

start-buckets! is using swap! to update bucket information, and swap! must have no side effects. 1

(defn- start-buckets! [{:keys [buckets] :as context}]
(swap! buckets
#(reduce-kv (fn [buckets id _opts]
(update buckets id (partial start-bucket! context id)))
%
%))
context)

And this situation can easily be happening where the server is under heavy load.

This results in creating many unnecessary threads, hogging the thread pool.

Footnotes

  1. https://clojuredocs.org/clojure.core/swap! - Note that f may be called multiple times, and thus should be free of side effects.

@oliyh
Copy link
Owner

oliyh commented Apr 11, 2023

Hi,

Thanks for reporting this, I will try to think of another way of doing this.

Idea: Have a lock for adding/mutating buckets that must be also acquired for starting them

@namenu
Copy link
Author

namenu commented Apr 13, 2023

@oliyh Great!

FYI, we've been workaround with this fix.
green-labs@4fe1e26

@oliyh
Copy link
Owner

oliyh commented Apr 17, 2023

Hi,

Thanks for sharing your workaround. Just reading through it, here's what I see:

  • Calling start-trigger now assocs a :start-fn to each bucket, rather than actually starting it
  • (:buckets context) is no longer an atom, but (-> context :buckets bucket-id) is

This means that adding a bucket dynamically is not really possible anymore - in fact the code still has swap-vals on (:buckets context) which won't work anymore, so I guess you are not using this feature: green-labs@4fe1e26#diff-53fa11e40c1e91d8226bb3a21d4ee41f1c4f14f02ea75e6c4e9e43b57301944aR228

So I don't think I can use your solution as it is, but it has given me some ideas thank you.

@oliyh oliyh closed this as completed in 5636ef7 Apr 21, 2023
@oliyh
Copy link
Owner

oliyh commented Apr 21, 2023

Hi, I've pushed 0.1.5-SNAPSHOT to clojars with a fix - are you able to verify it works for your use case?

Cheers

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

No branches or pull requests

2 participants