seadynamic8 (10) [Avatar] Offline
#1
Sect 8.1.2 -
I think it would be better to have some of the code for the process registry and explain it in the text. I think I mostly understand it, but there are a lot of new concepts introduced here. Even though the framework is mostly the same with a HashDict to store key-value pairs, the thinking with the ":via" tuples and creating custom names is really different.

In particular, I think you should include the code for the whereis, send, and handle_call(:register_name) and handle_info(smilieOWN) functions. You provide explanations for these, but they seemed a little rushed and without the code there, it is harder to understand the implementation.

Sect 8.2.1
This section is just a little confusing to understand because it is not quite the same as with database workers because the interface is different. I would restate that the interface functions for the Todo.Server don't need the use the "via tuple", because the client has access to the pid from the get_or_create function of the cache. It would help clear some things up. Also mention the helper whereis function.

Maybe a mention here to tie back to the Process Registry for the whereis_name and send interface functions as well.

A question I have is, why is that the Pool Supervisor needs to add an "id" field for the child specs, but the Server Supervisor doesn't. Is it because the the :simple_one_for_one strategy somehow knows how to differentiate between the children without the id?
sjuric (86) [Avatar] Offline
#2
Re: Chapter 8 - Confusing Section on Supervisors & Pooling
Thank you for the excellent feedback! I'll reread that text and think about clearing up issues you mention.

> A question I have is, why is that the Pool Supervisor
> needs to add an "id" field for the child specs, but
> the Server Supervisor doesn't. Is it because the the
> :simple_one_for_one strategy somehow knows how to
> differentiate between the children without the id?

The id field identifies a child specification. This id can be used in functions such as Supervisor.terminate_child/2 or Supervisor.restart_child/2 to manipulate individual workers. By default, module name is used as a child spec id. In pool supervisor we have multiple child specs for the same module, so we need to manually provide different ids.

However, in simple_one_for_one, there is only one child specification, and you can't manipulate individual workers (such as stop or restart them) so there's no need to specify the spec id.

I'll revisit the corresponding text and consider rewording.

Thanks again!
seadynamic8 (10) [Avatar] Offline
#3
Re: Chapter 8 - Confusing Section on Supervisors & Pooling
Sorry, but I'm still confused.

I now understand that when using "simple_one_for_one", we don't have the manual ability to manipulate the individual workers.(through those functions).

But my question is, how does the EVM know internally how to distinguish between the different childs that are started, if they all have the same module name?

Since we are starting multiple Todo Servers through the Server Supervisor (albeit on demand), but that shouldn't change the requirement of having to (at least internally) be able to distinguish between the different childs so that they can be individually restarted by the Supervisor. (if they crash)

I'm sorry if this sounds confusing, it may just me not seeing something really simple.
sjuric (86) [Avatar] Offline
#4
Re: Chapter 8 - Confusing Section on Supervisors & Pooling
What happens internally in supervisor is roughly the following. The supervisor starts the child and keeps mapping of the child's pid to the childspec (data provided by us that describes how the child must be started). When the child crashes, the supervisor receives the :EXIT message which contains the pid of the crashed child. Therefore, the supervisor has all the information needed to uniquely identify the crash child, deduce corresponding start arguments, and restart it.

The id values in childspec are more used for our own convenience. As clients we usually don't deal with pids of supervised processes. Thus, we need something else to identify the process which we want to manipulate (e.g. via Supervisor.terminate_child/2), and this is the point of the id used in childspec. It's just some identifier which we can provide to supervisor, and it will be able to translate it to the actual childspec and the pid of the process.

It's worth noting that supervisor is implemented in pure Erlang. It is implemented as a gen_server that relies on links and exit traps to detect crashes and do something about them. The mapping I mentioned above is simply the state of the supervisor process. I'm mentioning this because I think it's good to know that supervisor is not "magical". It just relies on simpler primitives of the BEAM VM to get the job done.

Does this explain it?
seadynamic8 (10) [Avatar] Offline
#5
Re: Chapter 8 - Confusing Section on Supervisors & Pooling
I think I understand it now.

So the id field in Pool Supervisor is just for convenience if we happen to need to use it later for other functions like Supervisor.terminate_child. It is not required.

Since I was able to do this the following, and start the Pool Supervisor without the ids:

defmodule Todo.PoolSupervisor do
use Supervisor

def start_link(pool_size) do
result = {:ok, sup} = Supervisor.start_link __MODULE__, nil
start_workers(sup, pool_size)
result
end

def start_workers(sup, pool_size) do
Enum.each( 1..pool_size, fn worker_id ->
Supervisor.start_child(sup, [worker(Todo.DatabaseWorker, [worker_id])])
end)
end

def init(_) do
supervise [], strategy: :one_for_one
end
end
sjuric (86) [Avatar] Offline
#6
Re: Chapter 8 - Confusing Section on Supervisors & Pooling
The id field is definitely there for our convenience, so we can reference workers.

However it is required that in "static" supervisors (all that are not :simple_one_for_one), this id field is unique. This holds even when starting the child on demand via start_child.

You may think that your example works, but it actually doesn't for two reasons.

First, when using start_child the second argument shouldn't be enclosed in the []. So it should be:
Supervisor.start_child(sup, worker(Todo.DatabaseWorker, [worker_id]))

More importantly, this won't work because the module name will be implicitly used as the id. So after the first worker is started, all other attempts to start another child with the same specification will return something like:

{:error, {:already_started, #PID<0.127.0>}}

You can verify all this if you add |> IO.inspect after Supervisor.start_child

To fix this, you need to add do something like:
Supervisor.start_child(sup, worker(Todo.DatabaseWorker, [worker_id], id: {:database_worker, worker_id}))


The point is that child id is relevant for all types of supervisor except :simple_one_for_one, which is treated somewhat specifically. Only for this supervisor type is the id field not used.
seadynamic8 (10) [Avatar] Offline
#7
Re: Chapter 8 - Confusing Section on Supervisors & Pooling
Thanks for all the explanations. I understand now.

So only in the case of :simple_one_for_one, where they only need one child spec, you don't need the id, otherwise, if you start multiple children with the same module name, you need the id.

And, in the :simple_one_for_one case, it does it internally with pids.

Gotcha!

Thanks again for the help.
sjuric (86) [Avatar] Offline
#8
Re: Chapter 8 - Confusing Section on Supervisors & Pooling
Glad it's more clear now!

I'd just like to clarify one misinformation I said in the initial response in this thread. It is in fact possible to terminate individual children via terminate_child even in :simple_one_for_one supervisor. However, in this case, you need to provide the child's pid instead of the id. See http://elixir-lang.org/docs/stable/elixir/Supervisor.html#terminate_child/2 for details.