Permalink
Browse files

Make ranch_sup the owner of the ranch_server ets table

Should prove itself more robust when things go wrong.
  • Loading branch information...
essen committed Aug 6, 2012
1 parent eabb029 commit b186d01367a1a744f6195e071611f97f9cc88f8e
Showing with 2 additions and 2 deletions.
  1. +0 −2 src/ranch_server.erl
  2. +2 −0 src/ranch_sup.erl
View
@@ -95,8 +95,6 @@ remove_connection(ListenerPid) ->
%% @private
init([]) ->
- ?TAB = ets:new(?TAB, [
- ordered_set, public, named_table, {write_concurrency, true}]),
{ok, #state{}}.
%% @private
View
@@ -33,6 +33,8 @@ start_link() ->
%% supervisor.
init([]) ->
+ ranch_server = ets:new(ranch_server, [
+ ordered_set, public, named_table, {write_concurrency, true}]),
Procs = [
{ranch_server, {ranch_server, start_link, []},
permanent, 5000, worker, [ranch_server]}

4 comments on commit b186d01

@jaynel

This comment has been minimized.

Show comment Hide comment
@jaynel

jaynel Aug 24, 2012

Putting this table in the supervisor doesn't seem right. It prevents it from disappearing, but it's not really how ets tables should be managed (although this is simpler than what I describe below). The problem comes if you need to add code to manage the table, then you are mixing crashable code with a supervisor.

ets tables now have an inherit property so that if the owner dies, the table is inherited by a surviving process. The following is more typical:

  1. Launch a table manager (use one_for_rest with table manager first)
  2. Send message to table manager to init ets table
    • This starts an ets table which is inherited by the table manager
    • Ask supervisor for a gen_server child
    • Hand ownership of ets to gen_server
  3. gen_server dies
    • table manager inherits ets
    • repeats last two steps of #2

Table manager should have no code other than handing off or receiving ets table.

gen_server owner can have functions for manipulating the ets table without causing grief for a supervisor or manager.

Putting this table in the supervisor doesn't seem right. It prevents it from disappearing, but it's not really how ets tables should be managed (although this is simpler than what I describe below). The problem comes if you need to add code to manage the table, then you are mixing crashable code with a supervisor.

ets tables now have an inherit property so that if the owner dies, the table is inherited by a surviving process. The following is more typical:

  1. Launch a table manager (use one_for_rest with table manager first)
  2. Send message to table manager to init ets table
    • This starts an ets table which is inherited by the table manager
    • Ask supervisor for a gen_server child
    • Hand ownership of ets to gen_server
  3. gen_server dies
    • table manager inherits ets
    • repeats last two steps of #2

Table manager should have no code other than handing off or receiving ets table.

gen_server owner can have functions for manipulating the ets table without causing grief for a supervisor or manager.

@essen

This comment has been minimized.

Show comment Hide comment
@essen

essen Aug 24, 2012

Owner

I know, and that's what I would have done for a more complicated use case, but this here is really all there is to it so it didn't seem worthwhile to go for a complex and harder to test solution. That's adding the table manager, a supervisor (to isolate table manager and ranch_server from listeners), lots of logic, for the same functionality. I like to keep things simple. :)

I'll probably tweak a little the current solution (there's one last thing I should store in the ets table) but past that I don't foresee any change on that end, ever.

Owner

essen replied Aug 24, 2012

I know, and that's what I would have done for a more complicated use case, but this here is really all there is to it so it didn't seem worthwhile to go for a complex and harder to test solution. That's adding the table manager, a supervisor (to isolate table manager and ranch_server from listeners), lots of logic, for the same functionality. I like to keep things simple. :)

I'll probably tweak a little the current solution (there's one last thing I should store in the ets table) but past that I don't foresee any change on that end, ever.

@jaynel

This comment has been minimized.

Show comment Hide comment
@jaynel

jaynel Aug 27, 2012

I've been thinking about this for a few days and I don't think ets inheritance as a supervisory technique has any usefulness. Using the supervisor to hold the ets table should work just fine and is practical as you have shown here.

I think the 'inherit' feature of an ets table is only useful for merging tables when portions of each are created by several processes, but the inherit is set to a central process which will receive and merge the results into a single ets cache.

I've been thinking about this for a few days and I don't think ets inheritance as a supervisory technique has any usefulness. Using the supervisor to hold the ets table should work just fine and is practical as you have shown here.

I think the 'inherit' feature of an ets table is only useful for merging tables when portions of each are created by several processes, but the inherit is set to a central process which will receive and merge the results into a single ets cache.

@essen

This comment has been minimized.

Show comment Hide comment
@essen

essen Aug 27, 2012

Owner

Thanks for your thoughts on this. :)

Owner

essen replied Aug 27, 2012

Thanks for your thoughts on this. :)

Please sign in to comment.