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

Discussion: support manual termination of the pools #294

Open
sasa1977 opened this issue Nov 15, 2024 · 2 comments
Open

Discussion: support manual termination of the pools #294

sasa1977 opened this issue Nov 15, 2024 · 2 comments

Comments

@sasa1977
Copy link

We are using Finch to communicate with dynamically started ephemeral servers. These servers have dynamic names, and so the set of URLs used grows indefinitely over time.

Finch keeps the pool instances perpetually running for these servers, even after we don't need them. As a result, the amount of NimblePool processes also grows indefinitely. Setting the :pool_max_idle_time doesn't work. I am undecided whether this is a bug or a feature, but in any case, the idle option doesn't seem to work if the server is disconnected.

Either way, I don't think idle timeout is an appropriate choice for this situation, partially because it only works on HTTP1, but perhaps more importantly, because it requires us to choose some magical timeout, instead of terminating the pool immediately after we don't need it anymore.

My first thought was to dynamically start a separate Finch instance per each dynamical server, and then stop it after it's not needed anymore. However, the instance name has to be an atom, so this means we would now be leaking atoms. This could be resolved if Finch supported arbitrary terms as names (which I think with some changes to the internals can be accomplished).

Another option, which seems to require much less work, is to support manual termination of the pool. I was able to accomplish this in the following way relying on the undocumented internals:

{s, h, p, _, _} = Finch.Request.parse_url(url)

with {pid, _} = Finch.PoolManager.get_pool(MyFinch, {s, h, p}, auto_start?: false),
  do: DynamicSupervisor.terminate_child(MyFinch.PoolSupervisor, pid)

This could be wrapped in a function called e.g. stop_pool, which can be called by the clients when the pool is not needed anymore.

Thoughts?

@sneako
Copy link
Owner

sneako commented Nov 15, 2024

This sounds great to me! In my Finch usage we are hammering a static set of urls so this issue hasn't come up for us but I'm aware that using it to request arbitrary urls would result in the resource leak you mentioned.

I don't see any issue with introducing your proposal. PR is very welcome.

@sasa1977
Copy link
Author

Great, thanks! We'll try this idea on our project, and see if it works and solves our problems. If all goes well, we'll submit a PR.

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