arjan / singleton

Global, supervised singleton processes for Elixir
MIT License
108 stars 19 forks source link

fix: allow to run more than 3 singletons #9

Closed klacointe closed 2 years ago

klacointe commented 3 years ago

It seems that you cannot run more than 3 singletons at the same time. This limitations came from the max_restarts option of DynamicSupervisor.

Demo to reproduce the issue: https://github.com/klacointe/singleton_demo

klacointe commented 3 years ago

@arjan do you have few minutes to check this PR ?

brianmay commented 2 years ago

I am still seeing the failure in the demo even after using this branch.

diff --git a/mix.exs b/mix.exs
index 192c14a..19b457c 100644
--- a/mix.exs
+++ b/mix.exs
@@ -24,7 +24,8 @@ defmodule SingletonDemo.MixProject do
     [
       # {:dep_from_hexpm, "~> 0.3.0"},
       # {:dep_from_git, git: "https://github.com/elixir-lang/my_dep.git", tag: "0.1.0"}
-      {:singleton, "~> 1.3"}
+          {:singleton,
+       git: "https://github.com/klacointe/singleton.git", branch: "FIX-more-than-three-singletons"},
     ]
   end
 end
diff --git a/mix.lock b/mix.lock
index 18ac914..47ae5ab 100644
--- a/mix.lock
+++ b/mix.lock
@@ -1,3 +1,3 @@
 %{
-  "singleton": {:hex, :singleton, "1.3.0", "e3f7210cbc47a01128181d61062b4e5c9bf1809c9f276622db370dedd2e25ca8", [:mix], [], "hexpm", "7fc25563a2f7e3622c0687743a58182d356601fd040925d3cce92a351817028c"},
+  "singleton": {:git, "https://github.com/klacointe/singleton.git", "9c01cf5da536f30b1fd1fa1dd3bb4f69d0ce0147", [branch: "FIX-more-than-three-singletons"]},
 }
 19:20:46.949 [info]  Application singleton exited: shutdown
brianmay commented 2 years ago

Oh, I see the title of this PR is misleading. It doesn't fix the problem, it just allows overriding the settings:

Application.get_env(:singleton, :dynamic_supervisor, [])

What settings did you use?

klacointe commented 2 years ago

Oh, I see the title of this PR is misleading. It doesn't fix the problem, it just allows overriding the settings:

Application.get_env(:singleton, :dynamic_supervisor, [])

What settings did you use?

I use this configuration to increase the number of allowed restarts:

config :singleton,
  dynamic_supervisor: [max_restarts: 100]
klacointe commented 2 years ago

up ?

arjan commented 2 years ago

PR looks good, @klacointe please add some documentation in the readme to show how to override the settings to allow > 3 children.

klacointe commented 2 years ago

@arjan done

klacointe commented 2 years ago

Thanks !

lessless commented 1 year ago

Hey @klacointe,

thanks for the fix. I'm trying to understand how Singleton works and puzzled by what's going on here 😅 Wouldn't you please mind explaining what the problem was about and how max_restarts solves it?

Thanks!

brianmay commented 1 year ago

My understanding: I think the problem is that the default max_restarts is 3, and as the initial startup of the processes is counted as restarts (?? need to check this), then if you have more then 3 processes you are going to exceed the maximum limit just starting up.