James_E

James_E

Correctness/safety question with DynamicSupervisor

I have an app that has many users connecting to it.

When each user tries to connect to a room, they should be connected to a “MyApp.Room” GenServer, which is started if a room with the requested ID does not yet exist.

All of the “MyApp.Room” instances are managed by one app-wide DynamicSupervisor.

Unfortunately, DynamicSupervisor just records all children in a PID-keyed map with no method to retrieve a particular child in any way except by PID, so I wrote a function like this to try to implement that “started if…does not yet exist” logic:

  @doc """
  Exactly like [DynamicSupervisor.start_child/2](https://hexdocs.pm/elixir/1.19/DynamicSupervisor.html#start_child/2)
  except that the "id" field is not [disregarded](https://hexdocs.pm/elixir/1.19.0-rc.0/DynamicSupervisor.html#start_child/2:~:text=while%20the%20:id%20field%20is%20still%20required,the%20value%20is%20ignored),
  but instead used as a unique key.

  Only works with simple GenServer supervisees for now.

  Does NOT work with remote DynamicSupervor or distributed systems for now.

  ## Example

      iex> defmodule FooApp.Model do use GenServer; def init(_opts), do: {:ok, nil} end
      iex> children = [{DynamicSupervisor, name: FooApp.ModelInstances}]
      iex> opts = [strategy: :one_for_one, name: FooApp.Supervisor]
      iex> {:ok, _} = Supervisor.start_link(children, opts)
      iex> {:ok, child1} = get_or_start_child(
      ...>   FooApp.ModelInstances,
      ...>   %{id: "one", start: {FooApp.Model, :start_link, [[]]}})
      iex> {:ok, child2} = get_or_start_child(
      ...>   FooApp.ModelInstances,
      ...>   %{id: "two", start: {FooApp.Model, :start_link, [[]]}})
      iex> get_or_start_child(
      ...>   FooApp.ModelInstances,
      ...>   %{id: "one", start: {FooApp.Model, :start_link, [[]]}})
      {:ok, child1} # retrieved the existing child1
  """
  @spec get_or_start_child(
          dynamic_supervisor :: Supervisor.supervisor(),
          child_spec :: Supervisor.module_spec() | Supervisor.child_spec()
        ) :: DynamicSupervisor.on_start_child()
  def get_or_start_child(dynamic_supervisor, module) when is_atom(module) do
    # https://github.com/elixir-lang/elixir/blob/v1.19.0-rc.0/lib/elixir/lib/gen_server.ex#L1036
    get_or_start_child(dynamic_supervisor, module.child_spec([]))
  end
  def get_or_start_child(dynamic_supervisor, {module, arg}) when is_atom(module) do
    get_or_start_child(dynamic_supervisor, module.child_spec(arg))
  end
  def get_or_start_child(dynamic_supervisor, child_spec = %{id: id, start: {_, _, _}}) do
    with supervisor_pid when is_pid(supervisor_pid) <- GenServer.whereis(dynamic_supervisor) do
      # https://hexdocs.pm/elixir/1.19/GenServer.html#module-name-registration
      # FIXME: https://hexdocs.pm/elixir/1.19/Registry.html#module-using-in-via
      reg_name = {:global, {supervisor_pid, id}}

      # https://github.com/elixir-lang/elixir/blob/v1.19.0-rc.0/lib/elixir/lib/dynamic_supervisor.ex#L407-L490
      child_spec =
        Map.update!(child_spec, :start, fn
          {m = GenServer, f = :start_link, [m1, init_arg]} ->
            {m, f, [m1, init_arg, [name: reg_name]]}

          {m = GenServer, f = :start_link, [m1, init_arg, opts]} ->
            {m, f, [m1, init_arg, opts ++ [name: reg_name]]}

          {m, f = :start_link, [init_arg, opts]} when is_list(opts) ->
            #true = GenServer in Keyword.get(m.__info__(:attributes), :behaviour, [])
            {m, f, [init_arg, opts ++ [name: reg_name]]}

          {m, f = :start_link, [opts]} when is_list(opts) ->
            #true = GenServer in Keyword.get(m.__info__(:attributes), :behaviour, [])
            {m, f, [opts ++ [name: reg_name]]}

          _start ->
            raise ArgumentError, """
            only GenServer style start_link children supported at this time.
            \tchild_spec = #{inspect(child_spec)}\
            """
        end)

      # https://github.com/erlang/otp/blob/84adefa331c4159d432d22840663c38f155cd4c1/lib/stdlib/src/gen.erl#L71
      # https://github.com/elixir-lang/elixir/blob/v1.19.0-rc.0/lib/elixir/lib/gen_server.ex#L1050
      case DynamicSupervisor.start_child(supervisor_pid, child_spec) do
        {:error, {:already_started, pid}} -> {:ok, pid}
        result -> result
      end
    else
      {name, node} = server when is_atom(name) and is_atom(node) ->
        # https://github.com/elixir-lang/elixir/blob/v1.19.0-rc.0/lib/elixir/lib/gen_server.ex#L1324-L1326
        raise ArgumentError,
          message: """
          Only local supervisors supported at this time.
          \tserver = #{inspect(server)}\
          """

      nil ->
        # https://github.com/elixir-lang/elixir/blob/v1.19.0-rc.0/lib/elixir/lib/gen_server.ex#L1309
        {:error, :noproc}
    end
  end

However, this seems fragile; I’m wondering in particular whether, as written, it contains any race conditions, or if it’d be safe to call the current version of get_or_start_child blindly from, for example, a Phoenix LiveView mount callback.

If this approach is subject to a race condition, is there any correct way to accomplish this instead?

Most Liked

josevalim

josevalim

Creator of Elixir

We have just rewritten the Mix & OTP guides in Elixir to build something similar, you can read it in this PR. We also built something that relied on similar techniques here: Homemade analytics with Ecto and Elixir - Dashbit Blog

windexoriginal

windexoriginal

I think you should look at the registry module. It will allow you to give arbitrary identifiers to the processes you spawn dynamically.

Registry is implemented with ets which should help with concerns about race conditions.

josevalim

josevalim

Creator of Elixir

The updated docs are here: Simple state with agents — Elixir v1.20.0-dev

We generally treat naming processes and supervising processes as separate things. You don’t need to wrap the registry and supervisor access from within the agent. The architecture built in the getting started guide should be enough and provide uniqueness (either locally or in a cluster).

LostKobrakai

LostKobrakai

You want to decouple the supervision tree concerns from the key management and uniqueness. For the former continue to use DynamicSupervisor. For the latter you can use Registry if you only need to enforce uniqueness on a per node level.

garrison

garrison

The Elixir Registry is local, yes. I don’t think it would be very hard to migrate off of, though.

You can write your own routing table/registry fairly easily with a GenServer and ets as primitives. Then you can use :global to ensure your routing table is registered on one node and send all requests to that node.

That will get you pretty far. If you need to scale further you can have multiple processes read from the ets table, and if you need to scale out much further you can shard your rooms across nodes and route locally on each node with a node id embedded in the room id or similar. Probably you don’t even need to think about doing this right now.

Where Next?

Popular in Questions Top

joaquinalcerro
Hi there, I am working with Ecto-Postgresql and I need to call all of the records from a specific table but the table has 40,000 record...
New
Werner
Hi, I’m using Ubuntu 18.04 and after updating to OTP-24.0 yesterday i have this warning when I run “mix local.hex”: 14:57:30.512 [warn] ...
New
quazar
How to set Jason to encode all fields in ecto schema, I don’t care about security and implementing only is taking long list of attributes...
New
mgjohns61585
Could someone help me? I'm making my first elixir program, number guessing game. I can't figure out how to convert the user's guess from ...
New
makeitrein
Hey all, just started picking up Elixir last week and am writing a scraper as a learning project. Baby step #1 is extracting the number ...
New
minhajuddin
I have seen a lot of code which picks the first element from a list using Enum.at(0) instead of List.first. Is there a reason why people ...
New
alice
Hey, Just curious what are the main benefits of Elixir compared to Clojure? When is Elixir more useful than Clojure and vice versa? Th...
New
ovidiubadita
Hey all, I discovered Elixir and I love it. I always wanted to learn a functional programming and I intended to go for Haskell, but afte...
New
kostonstyle
Hi all I want to have a unix time, from the current time plus 1 hour. DateTime.now + 1 hour How to get it in elixir? Thanks
New
Exadra37
Sometimes I want to check if the input into a function is not a blank string. My first approach: defmodule Example do def do_stuff(s...
New

Other popular topics Top

srinivasu
How to handle excepions in elixir? Suppose i have A, B, C ,D, E modules. and each module has get() function. A.get() method will call th...
New
dotdotdotPaul
Okay, I'm having a heck of a time trying to figure out how to best handle the validation of belongs_to associations in Ecto. I'm sure I'...
New
bsollish-terakeet
Credo is smart enough to check for (something like) this: assert length(the_list) == 0 with this response: Checking if an enum is empt...
New
yawaramin
In the Dialyzer docs ( http://erlang.org/doc/man/dialyzer.html#requesting-or-suppressing-warnings-in-source-files ), there is a way to tu...
New
aalberti333
As the title describes, I’m trying to run Enum.map() over a list of key/value pairs, where the value is a map. My data looks like this: ...
New
vac
Hi, I'm quite new in Elixir and I'm trying to format a string to a PEM format. I have the certificate value like MIIDBTCCAe2...... and ...
New
minhajuddin
I have seen a lot of code which picks the first element from a list using Enum.at(0) instead of List.first. Is there a reason why people ...
New
ashish173
I am using Ecto timestamps with postgres, I can see the timestamps() use the :naive_dateime but for my use case I wanted to store the ti...
New
electic
Hi, I am new to Elixir. I am trying to use the DateTime component to insert a date into MySQL however the there seems to be no way to fo...
New
Fl4m3Ph03n1x
About me? ( if you have nothing better to do than reading about some random guy in the internet :stuck_out_tongue: ) Hello all, this is ...
New

We're in Beta

About us Mission Statement