Seconded. That's a big change with zero discussion and no Jira ticket.
+1 on reverting until a discussion is had.

B.

On Mon, Jan 3, 2011 at 12:45 PM, Filipe David Manana
<[email protected]> wrote:
> Hi Benoît,
>
> Not that I'm against it, but shouldn't such a significant change be
> discussed before committing it?
>
> Tell me if I'm wrong, but I don't recall any discussing about this
> either at the development mailing list or in a Jira ticket.
>
> cheers
>
> On Mon, Jan 3, 2011 at 12:35 PM,  <[email protected]> wrote:
>> Author: benoitc
>> Date: Mon Jan  3 12:35:46 2011
>> New Revision: 1054594
>>
>> URL: http://svn.apache.org/viewvc?rev=1054594&view=rev
>> Log:
>> import some changes from bigcouch. Improve a little the supervision
>> tree.
>>
>> Added:
>>    couchdb/trunk/src/couchdb/couch_drv.erl
>>    couchdb/trunk/src/couchdb/couch_primary_sup.erl
>>    couchdb/trunk/src/couchdb/couch_secondary_sup.erl
>> Modified:
>>    couchdb/trunk/etc/couchdb/default.ini.tpl.in
>>    couchdb/trunk/src/couchdb/Makefile.am
>>    couchdb/trunk/src/couchdb/couch_db_update_notifier_sup.erl
>>    couchdb/trunk/src/couchdb/couch_external_manager.erl
>>    couchdb/trunk/src/couchdb/couch_query_servers.erl
>>    couchdb/trunk/src/couchdb/couch_server_sup.erl
>>
>> Modified: couchdb/trunk/etc/couchdb/default.ini.tpl.in
>> URL: 
>> http://svn.apache.org/viewvc/couchdb/trunk/etc/couchdb/default.ini.tpl.in?rev=1054594&r1=1054593&r2=1054594&view=diff
>> ==============================================================================
>> --- couchdb/trunk/etc/couchdb/default.ini.tpl.in (original)
>> +++ couchdb/trunk/etc/couchdb/default.ini.tpl.in Mon Jan  3 12:35:46 2011
>> @@ -51,7 +51,6 @@ os_process_limit = 25
>>  [daemons]
>>  view_manager={couch_view, start_link, []}
>>  external_manager={couch_external_manager, start_link, []}
>> -db_update_notifier={couch_db_update_notifier_sup, start_link, []}
>>  query_servers={couch_query_servers, start_link, []}
>>  httpd={couch_httpd, start_link, []}
>>  stats_aggregator={couch_stats_aggregator, start, []}
>>
>> Modified: couchdb/trunk/src/couchdb/Makefile.am
>> URL: 
>> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/Makefile.am?rev=1054594&r1=1054593&r2=1054594&view=diff
>> ==============================================================================
>> --- couchdb/trunk/src/couchdb/Makefile.am (original)
>> +++ couchdb/trunk/src/couchdb/Makefile.am Mon Jan  3 12:35:46 2011
>> @@ -38,6 +38,7 @@ source_files = \
>>     couch_db_update_notifier.erl \
>>     couch_db_update_notifier_sup.erl \
>>     couch_doc.erl \
>> +    couch_drv.erl \
>>     couch_event_sup.erl \
>>     couch_external_manager.erl \
>>     couch_external_server.erl \
>> @@ -59,6 +60,7 @@ source_files = \
>>     couch_native_process.erl \
>>     couch_os_daemons.erl \
>>     couch_os_process.erl \
>> +    couch_primary_sup.erl \
>>     couch_query_servers.erl \
>>     couch_ref_counter.erl \
>>     couch_rep.erl \
>> @@ -70,6 +72,7 @@ source_files = \
>>     couch_rep_sup.erl \
>>     couch_rep_writer.erl \
>>     couch_rep_db_listener.erl \
>> +    couch_secondary_sup.erl \
>>     couch_server.erl \
>>     couch_server_sup.erl \
>>     couch_stats_aggregator.erl \
>> @@ -100,6 +103,7 @@ compiled_files = \
>>     couch_db_update_notifier.beam \
>>     couch_db_update_notifier_sup.beam \
>>     couch_doc.beam \
>> +    couch_drv.beam \
>>     couch_event_sup.beam \
>>     couch_external_manager.beam \
>>     couch_external_server.beam \
>> @@ -121,6 +125,7 @@ compiled_files = \
>>     couch_native_process.beam \
>>     couch_os_daemons.beam \
>>     couch_os_process.beam \
>> +    couch_primary_sup.beam \
>>     couch_query_servers.beam \
>>     couch_ref_counter.beam \
>>     couch_rep.beam \
>> @@ -132,6 +137,7 @@ compiled_files = \
>>     couch_rep_sup.beam \
>>     couch_rep_writer.beam \
>>     couch_rep_db_listener.beam \
>> +    couch_secondary_sup.beam \
>>     couch_server.beam \
>>     couch_server_sup.beam \
>>     couch_stats_aggregator.beam \
>>
>> Modified: couchdb/trunk/src/couchdb/couch_db_update_notifier_sup.erl
>> URL: 
>> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_db_update_notifier_sup.erl?rev=1054594&r1=1054593&r2=1054594&view=diff
>> ==============================================================================
>> --- couchdb/trunk/src/couchdb/couch_db_update_notifier_sup.erl (original)
>> +++ couchdb/trunk/src/couchdb/couch_db_update_notifier_sup.erl Mon Jan  3 
>> 12:35:46 2011
>> @@ -22,16 +22,14 @@
>>
>>  -behaviour(supervisor).
>>
>> --export([start_link/0,init/1]).
>> +-export([start_link/0, init/1, config_change/3]).
>>
>>  start_link() ->
>>     supervisor:start_link({local, couch_db_update_notifier_sup},
>>         couch_db_update_notifier_sup, []).
>>
>>  init([]) ->
>> -    ok = couch_config:register(
>> -        fun("update_notification", Key, Value) -> reload_config(Key, Value) 
>> end
>> -    ),
>> +    ok = couch_config:register(fun ?MODULE:config_change/3),
>>
>>     UpdateNotifierExes = couch_config:get("update_notification"),
>>
>> @@ -48,7 +46,7 @@ init([]) ->
>>
>>  %% @doc when update_notification configuration changes, terminate the 
>> process
>>  %%      for that notifier and start a new one with the updated config
>> -reload_config(Id, Exe) ->
>> +config_change("update_notification", Id, Exe) ->
>>     ChildSpec = {
>>         Id,
>>         {couch_db_update_notifier, start_link, [Exe]},
>>
>> Added: couchdb/trunk/src/couchdb/couch_drv.erl
>> URL: 
>> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_drv.erl?rev=1054594&view=auto
>> ==============================================================================
>> --- couchdb/trunk/src/couchdb/couch_drv.erl (added)
>> +++ couchdb/trunk/src/couchdb/couch_drv.erl Mon Jan  3 12:35:46 2011
>> @@ -0,0 +1,46 @@
>> +-module(couch_drv).
>> +-behaviour(gen_server).
>> +-export([init/1, handle_call/3, handle_cast/2, handle_info/2, terminate/2,
>> +    code_change/3]).
>> +
>> +-export([start_link/0]).
>> +
>> +-include("couch_db.hrl").
>> +
>> +start_link() ->
>> +    gen_server:start_link({local, ?MODULE}, ?MODULE, [], []).
>> +
>> +init([]) ->
>> +    LibDir =
>> +    case couch_config:get("couchdb", "util_driver_dir", null) of
>> +    null ->
>> +        filename:join(couch_util:priv_dir(), "lib");
>> +    LibDir0 -> LibDir0
>> +    end,
>> +
>> +
>> +    case erl_ddll:load(LibDir, "couch_icu_driver") of
>> +    ok ->
>> +        {ok, nil};
>> +    {error, already_loaded} ->
>> +        ?LOG_INFO("~p reloading couch_erl_driver", [?MODULE]),
>> +        ok = erl_ddll:reload(LibDir, "couch_erl_driver"),
>> +        {ok, nil};
>> +    {error, Error} ->
>> +        {stop, erl_ddll:format_error(Error)}
>> +    end.
>> +
>> +handle_call(_Request, _From, State) ->
>> +    {reply, ok, State}.
>> +
>> +handle_cast(_Request, State) ->
>> +    {noreply, State}.
>> +
>> +handle_info(_Info, State) ->
>> +    {noreply, State}.
>> +
>> +terminate(_Reason, _State) ->
>> +    ok.
>> +
>> +code_change(_OldVsn, State, _Extra) ->
>> +    {ok, State}.
>>
>> Modified: couchdb/trunk/src/couchdb/couch_external_manager.erl
>> URL: 
>> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_external_manager.erl?rev=1054594&r1=1054593&r2=1054594&view=diff
>> ==============================================================================
>> --- couchdb/trunk/src/couchdb/couch_external_manager.erl (original)
>> +++ couchdb/trunk/src/couchdb/couch_external_manager.erl Mon Jan  3 12:35:46 
>> 2011
>> @@ -39,7 +39,7 @@ config_change("external", UrlName) ->
>>  init([]) ->
>>     process_flag(trap_exit, true),
>>     Handlers = ets:new(couch_external_manager_handlers, [set, private]),
>> -    couch_config:register(fun config_change/2),
>> +    couch_config:register(fun ?MODULE:config_change/2),
>>     {ok, Handlers}.
>>
>>  terminate(_Reason, Handlers) ->
>>
>> Added: couchdb/trunk/src/couchdb/couch_primary_sup.erl
>> URL: 
>> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_primary_sup.erl?rev=1054594&view=auto
>> ==============================================================================
>> --- couchdb/trunk/src/couchdb/couch_primary_sup.erl (added)
>> +++ couchdb/trunk/src/couchdb/couch_primary_sup.erl Mon Jan  3 12:35:46 2011
>> @@ -0,0 +1,60 @@
>> +% Licensed under the Apache License, Version 2.0 (the "License"); you may 
>> not
>> +% use this file except in compliance with the License. You may obtain a 
>> copy of
>> +% the License at
>> +%
>> +%   http://www.apache.org/licenses/LICENSE-2.0
>> +%
>> +% Unless required by applicable law or agreed to in writing, software
>> +% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
>> +% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
>> +% License for the specific language governing permissions and limitations 
>> under
>> +% the License.
>> +
>> +-module(couch_primary_sup).
>> +-behaviour(supervisor).
>> +-export([init/1, start_link/0]).
>> +
>> +start_link() ->
>> +    supervisor:start_link({local,couch_primary_services}, ?MODULE, []).
>> +
>> +init([]) ->
>> +    Children = [
>> +        {collation_driver,
>> +            {couch_drv, start_link, []},
>> +            permanent,
>> +            brutal_kill,
>> +            worker,
>> +            [couch_drv]},
>> +        {couch_task_status,
>> +            {couch_task_status, start_link, []},
>> +            permanent,
>> +            brutal_kill,
>> +            worker,
>> +            [couch_task_status]},
>> +        {couch_server,
>> +            {couch_server, sup_start_link, []},
>> +            permanent,
>> +            brutal_kill,
>> +            worker,
>> +            [couch_server]},
>> +        {couch_db_update_event,
>> +            {gen_event, start_link, [{local, couch_db_update}]},
>> +            permanent,
>> +            brutal_kill,
>> +            worker,
>> +            dynamic},
>> +        {couch_replication_supervisor,
>> +            {couch_rep_sup, start_link, []},
>> +            permanent,
>> +            infinity,
>> +            supervisor,
>> +            [couch_rep_sup]},
>> +        {couch_log,
>> +            {couch_log, start_link, []},
>> +            permanent,
>> +            brutal_kill,
>> +            worker,
>> +            [couch_log]}
>> +    ],
>> +    {ok, {{one_for_one, 10, 3600}, Children}}.
>> +
>>
>> Modified: couchdb/trunk/src/couchdb/couch_query_servers.erl
>> URL: 
>> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_query_servers.erl?rev=1054594&r1=1054593&r2=1054594&view=diff
>> ==============================================================================
>> --- couchdb/trunk/src/couchdb/couch_query_servers.erl (original)
>> +++ couchdb/trunk/src/couchdb/couch_query_servers.erl Mon Jan  3 12:35:46 
>> 2011
>> @@ -13,7 +13,7 @@
>>  -module(couch_query_servers).
>>  -behaviour(gen_server).
>>
>> --export([start_link/0]).
>> +-export([start_link/0, config_change/1]).
>>
>>  -export([init/1, terminate/2, handle_call/3, handle_cast/2, 
>> handle_info/2,code_change/3]).
>>  -export([start_doc_map/3, map_docs/2, stop_doc_map/1]).
>> @@ -265,26 +265,9 @@ with_ddoc_proc(#doc{id=DDocId,revs={Star
>>     end.
>>
>>  init([]) ->
>> -    % read config and register for configuration changes
>> -
>> -    % just stop if one of the config settings change. couch_server_sup
>> -    % will restart us and then we will pick up the new settings.
>> -
>> -    ok = couch_config:register(
>> -        fun("query_servers" ++ _, _) ->
>> -            supervisor:terminate_child(couch_secondary_services, 
>> query_servers),
>> -            supervisor:restart_child(couch_secondary_services, 
>> query_servers)
>> -        end),
>> -    ok = couch_config:register(
>> -        fun("native_query_servers" ++ _, _) ->
>> -            supervisor:terminate_child(couch_secondary_services, 
>> query_servers),
>> -            [supervisor:restart_child(couch_secondary_services, 
>> query_servers)]
>> -        end),
>> -    ok = couch_config:register(
>> -        fun("query_server_config" ++ _, _) ->
>> -            supervisor:terminate_child(couch_secondary_services, 
>> query_servers),
>> -            supervisor:restart_child(couch_secondary_services, 
>> query_servers)
>> -        end),
>> +    % register async to avoid deadlock on restart_child
>> +    Self = self(),
>> +    spawn(couch_config, register, [fun ?MODULE:config_change/1, Self]),
>>
>>     Langs = ets:new(couch_query_server_langs, [set, private]),
>>     LangLimits = ets:new(couch_query_server_lang_limits, [set, private]),
>> @@ -394,6 +377,16 @@ handle_info({'EXIT', Pid, Status}, #qser
>>  code_change(_OldVsn, State, _Extra) ->
>>     {ok, State}.
>>
>> +config_change("query_servers") ->
>> +    supervisor:terminate_child(couch_secondary_services, query_servers),
>> +    supervisor:restart_child(couch_secondary_services, query_servers);
>> +config_change("native_query_servers") ->
>> +    supervisor:terminate_child(couch_secondary_services, query_servers),
>> +    supervisor:restart_child(couch_secondary_services, query_servers);
>> +config_change("query_server_config") ->
>> +    supervisor:terminate_child(couch_secondary_services, query_servers),
>> +    supervisor:restart_child(couch_secondary_services, query_servers).
>> +
>>  % Private API
>>
>>  add_to_waitlist(Info, From, #qserver{waitlist=Waitlist}=Server) ->
>>
>> Added: couchdb/trunk/src/couchdb/couch_secondary_sup.erl
>> URL: 
>> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_secondary_sup.erl?rev=1054594&view=auto
>> ==============================================================================
>> --- couchdb/trunk/src/couchdb/couch_secondary_sup.erl (added)
>> +++ couchdb/trunk/src/couchdb/couch_secondary_sup.erl Mon Jan  3 12:35:46 
>> 2011
>> @@ -0,0 +1,42 @@
>> +% Licensed under the Apache License, Version 2.0 (the "License"); you may 
>> not
>> +% use this file except in compliance with the License. You may obtain a 
>> copy of
>> +% the License at
>> +%
>> +%   http://www.apache.org/licenses/LICENSE-2.0
>> +%
>> +% Unless required by applicable law or agreed to in writing, software
>> +% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
>> +% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
>> +% License for the specific language governing permissions and limitations 
>> under
>> +% the License.
>> +
>> +-module(couch_secondary_sup).
>> +-behaviour(supervisor).
>> +-export([init/1, start_link/0]).
>> +
>> +start_link() ->
>> +    supervisor:start_link({local,couch_secondary_services}, ?MODULE, []).
>> +
>> +init([]) ->
>> +    SecondarySupervisors = [
>> +        {couch_db_update_notifier_sup,
>> +            {couch_db_update_notifier_sup, start_link, []},
>> +            permanent,
>> +            infinity,
>> +            supervisor,
>> +            [couch_db_update_notifier_sup]}
>> +    ],
>> +    Children = SecondarySupervisors ++ [
>> +        begin
>> +            {ok, {Module, Fun, Args}} = couch_util:parse_term(SpecStr),
>> +
>> +            {list_to_atom(Name),
>> +                {Module, Fun, Args},
>> +                permanent,
>> +                brutal_kill,
>> +                worker,
>> +                [Module]}
>> +        end
>> +        || {Name, SpecStr}
>> +        <- couch_config:get("daemons"), SpecStr /= ""],
>> +    {ok, {{one_for_one, 10, 3600}, Children}}.
>>
>> Modified: couchdb/trunk/src/couchdb/couch_server_sup.erl
>> URL: 
>> http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_server_sup.erl?rev=1054594&r1=1054593&r2=1054594&view=diff
>> ==============================================================================
>> --- couchdb/trunk/src/couchdb/couch_server_sup.erl (original)
>> +++ couchdb/trunk/src/couchdb/couch_server_sup.erl Mon Jan  3 12:35:46 2011
>> @@ -7,7 +7,7 @@
>>  % Unless required by applicable law or agreed to in writing, software
>>  % distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
>>  % WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
>> -% License for the specific language governing permissions and limitations 
>> under
>> +% License for the specific languag governing permissions and limitations 
>> under
>>  % the License.
>>
>>  -module(couch_server_sup).
>> @@ -15,8 +15,7 @@
>>
>>
>>  -export([start_link/1,stop/0, couch_config_start_link_wrapper/2,
>> -        start_primary_services/0,start_secondary_services/0,
>> -        restart_core_server/0]).
>> +        restart_core_server/0, config_change/2]).
>>
>>  -include("couch_db.hrl").
>>
>> @@ -68,15 +67,6 @@ start_server(IniFiles) ->
>>     _ -> ok
>>     end,
>>
>> -    LibDir =
>> -    case couch_config:get("couchdb", "util_driver_dir", null) of
>> -    null ->
>> -        filename:join(couch_util:priv_dir(), "lib");
>> -    LibDir0 -> LibDir0
>> -    end,
>> -
>> -    ok = couch_util:start_driver(LibDir),
>> -
>>     BaseChildSpecs =
>>     {{one_for_all, 10, 3600},
>>         [{couch_config,
>> @@ -86,17 +76,17 @@ start_server(IniFiles) ->
>>             worker,
>>             [couch_config]},
>>         {couch_primary_services,
>> -            {couch_server_sup, start_primary_services, []},
>> +            {couch_primary_sup, start_link, []},
>>             permanent,
>>             infinity,
>>             supervisor,
>> -            [couch_server_sup]},
>> +            [couch_primary_sup]},
>>         {couch_secondary_services,
>> -            {couch_server_sup, start_secondary_services, []},
>> +            {couch_secondary_sup, start_link, []},
>>             permanent,
>>             infinity,
>>             supervisor,
>> -            [couch_server_sup]}
>> +            [couch_secondary_sup]}
>>         ]},
>>
>>     % ensure these applications are running
>> @@ -106,15 +96,8 @@ start_server(IniFiles) ->
>>     {ok, Pid} = supervisor:start_link(
>>         {local, couch_server_sup}, couch_server_sup, BaseChildSpecs),
>>
>> -    % launch the icu bridge
>>     % just restart if one of the config settings change.
>> -
>> -    couch_config:register(
>> -        fun("couchdb", "util_driver_dir") ->
>> -            ?MODULE:stop();
>> -        ("daemons", _) ->
>> -            ?MODULE:stop()
>> -        end, Pid),
>> +    couch_config:register(fun ?MODULE:config_change/2, Pid),
>>
>>     unlink(ConfigPid),
>>
>> @@ -132,59 +115,12 @@ start_server(IniFiles) ->
>>
>>     {ok, Pid}.
>>
>> -start_primary_services() ->
>> -    supervisor:start_link({local, couch_primary_services}, couch_server_sup,
>> -        {{one_for_one, 10, 3600},
>> -            [{couch_log,
>> -                {couch_log, start_link, []},
>> -                permanent,
>> -                brutal_kill,
>> -                worker,
>> -                [couch_log]},
>> -            {couch_replication_supervisor,
>> -                {couch_rep_sup, start_link, []},
>> -                permanent,
>> -                infinity,
>> -                supervisor,
>> -                [couch_rep_sup]},
>> -            {couch_task_status,
>> -                {couch_task_status, start_link, []},
>> -                permanent,
>> -                brutal_kill,
>> -                worker,
>> -                [couch_task_status]},
>> -            {couch_server,
>> -                {couch_server, sup_start_link, []},
>> -                permanent,
>> -                1000,
>> -                worker,
>> -                [couch_server]},
>> -            {couch_db_update_event,
>> -                {gen_event, start_link, [{local, couch_db_update}]},
>> -                permanent,
>> -                brutal_kill,
>> -                worker,
>> -                dynamic}
>> -            ]
>> -        }).
>> -
>> -start_secondary_services() ->
>> -    DaemonChildSpecs = [
>> -        begin
>> -            {ok, {Module, Fun, Args}} = couch_util:parse_term(SpecStr),
>> -
>> -            {list_to_atom(Name),
>> -                {Module, Fun, Args},
>> -                permanent,
>> -                1000,
>> -                worker,
>> -                [Module]}
>> -        end
>> -        || {Name, SpecStr}
>> -        <- couch_config:get("daemons"), SpecStr /= ""],
>> -
>> -    supervisor:start_link({local, couch_secondary_services}, 
>> couch_server_sup,
>> -        {{one_for_one, 10, 3600}, DaemonChildSpecs}).
>> +config_change("daemons", _) ->
>> +    exit(whereis(couch_server_sup), shutdown);
>> +config_change("couchdb", "util_driver_dir") ->
>> +    [Pid] = [P || {collation_driver,P,_,_}
>> +        <- supervisor:which_children(couch_primary_services)],
>> +    Pid ! reload_driver.
>>
>>  stop() ->
>>     catch exit(whereis(couch_server_sup), normal).
>>
>>
>>
>
>
>
> --
> Filipe David Manana,
> [email protected], [email protected]
>
> "Reasonable men adapt themselves to the world.
>  Unreasonable men adapt the world to themselves.
>  That's why all progress depends on unreasonable men."
>

Reply via email to