Is it okay to have logic in a supervisor?

Question

I am currently going through Elixir in Action and i am doing some refactoring of my Todo application code to get a better grasp on the main parts of OTP.

The application uses a database that simply stores data in files on the disk. To ensure that the target folder for the database exists, File.mkdir_p!(db_folder) is called inside the database process. The database process itself uses a bunch of worker processes to perform the actual storing/retrieving of data from disk.

The chapter i am currently at introduces a DIY process registry to implement a more robust supervision tree, by having the workers register themselves to the registry and having the database process lookup the worker using the registry, so both parties can be supervised and will still work after a failure.

When Elixir 1.4 came out i read about the Registry module in the patch notes, so i thought i might refactor the app and use that. Now it turns out, that the database process does not really have to know about the folder the database uses for storing data. So i took the mkdir_p! call out of that module and thought about where to put it. Two options come to mind:

1. The DatabaseWorker
2. The DatabaseWorkerSupervisor

I personally prefer the second approach, since the whole app is bound to crash anyways if the user has no access rights to the persistence folder. But I am not quite sure if it is okay to put logic into a Supervisor.

Is putting logic into a Supervisor bad style or acceptable depending on the situation? If it is bad style, where do i put startup-logic that i do not want to be repeated if a process crashes?

---

My Supervisor code:

defmodule Todo.DatabaseWorkerSupervisor do
  use Supervisor

  def start_link(db_folder) do
    Supervisor.start_link(__MODULE__, db_folder)
  end

  def init(db_folder) do
    File.mkdir_p!(db_folder)

    processes =
      for worker_id <- 1..3 do
        worker(Todo.DatabaseWorker, [db_folder, worker_id], id: {:dbworker, worker_id})
      end

    supervise(processes, strategy: :one_for_one)
  end
end

Answer 1

where do i put startup-logic that i do not want to be repeated if a process crashes?

Calling this from the supervisor init seems like the logical place. Particularly if you don't want it to be repeated.

Perhaps the code could be defined in another module, but it makes sense to call it from the supervisor init, and let the supervisor crash if initialisation fails.

Answer 2

I would prefer not to put logic in the supervisor. If you start to put logic in supervisors you can't reason about crashes / restarts from looking at your supervision tree. Instead, I will suggest the following supervision tree:

If you put the folder creation in the init of the DB gen server, the DBSupervisor will wait for the init to return before moving on to his other children. So if the folder creation fails, the rest of the supervision tree won't even spawn. In addition, if the DBSupervisor strategy is :rest_for_all, any failure in the DB gen server will restart the rest of the supervision tree.

I know that this answer might seems like an overkill, but if the emphasise is on correction and learning I think this is the correct direction.

One important note. As you said, with a registery you don't really need the DB gen server for passing tasks from clients to the workers, and you are right! Although the suggested supervision tree looks similar to the one before the registery you should now just call functions (that can be implemented in the DB module) to query the registery and pass tasks directly from client processes to the workers.