You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@couchdb.apache.org by Filipe David Manana <fd...@apache.org> on 2011/01/03 13:45:32 UTC
Re: svn commit: r1054594 - in /couchdb/trunk: etc/couchdb/ src/couchdb/
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, <be...@apache.org> 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,
fdmanana@gmail.com, fdmanana@apache.org
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
Re: svn commit: r1054594 - in /couchdb/trunk: etc/couchdb/ src/couchdb/
Posted by Robert Newson <ro...@gmail.com>.
awesome, thanks!
On Mon, Jan 3, 2011 at 2:28 PM, Benoit Chesneau <bc...@gmail.com> wrote:
>> https://issues.apache.org/jira/secure/ManageAttachments.jspa?id=12494493
>>
>
> correct url:
> https://issues.apache.org/jira/browse/COUCHDB-1010
>
Re: svn commit: r1054594 - in /couchdb/trunk: etc/couchdb/ src/couchdb/
Posted by Benoit Chesneau <bc...@gmail.com>.
> https://issues.apache.org/jira/secure/ManageAttachments.jspa?id=12494493
>
correct url:
https://issues.apache.org/jira/browse/COUCHDB-1010
Re: svn commit: r1054594 - in /couchdb/trunk: etc/couchdb/ src/couchdb/
Posted by Benoit Chesneau <bc...@gmail.com>.
On Mon, Jan 3, 2011 at 3:01 PM, Filipe David Manana <fd...@apache.org> wrote:
> On Mon, Jan 3, 2011 at 1:47 PM, Benoit Chesneau <bc...@gmail.com> wrote:
>> On Mon, Jan 3, 2011 at 1:50 PM, Robert Newson <ro...@gmail.com> wrote:
>>> Seconded. That's a big change with zero discussion and no Jira ticket.
>>> +1 on reverting until a discussion is had.
>>>
>>> B.
>> These changes don't introduce any regressions, and are well tested.
>> Did you read the code ?
>>
>
> It's not a question of reading or not the code.
It is if you start to say it's trivial or not.
> All the tests pass, but to me that only means "maybe there aren't any
> regressions".
>
> I believe there are very good reasons for having it in Bigcouch and CouchDB.
> But I would like to have before a vote and have feedback from Adam
> regarding no issues with standard CouchDB, as I believe he's the one
> that knows better what the implications might be.
>
>
WhiIe I don't consider this as a non trivial patch, I reverted it and
opened COUCHDB-1010 issue.
https://issues.apache.org/jira/secure/ManageAttachments.jspa?id=12494493
- benoît.
Re: svn commit: r1054594 - in /couchdb/trunk: etc/couchdb/ src/couchdb/
Posted by Robert Newson <ro...@gmail.com>.
Completely agree. I don't want us to have to file a ticket for every
single commit either. The trouble is one persons "trivial" is not
everyone's. In my opinion, and it's merely that, tweaking
startup/supervisor stuff is non-trivial, even if expressed in a
handful of lines, as it can change how we all reason about how couchdb
processes handle failures.
Reasonable cases for committing without a ticket are correcting typos
in comments and adding new (passing) tests but I don't think it's
useful to provide an exhaustive list of such things.
Finally, if your commit introduces new modules then a ticket and a
patch should be preferred.
B.
On Mon, Jan 3, 2011 at 3:49 PM, Adam Kocoloski <ko...@apache.org> wrote:
> On Jan 3, 2011, at 9:01 AM, Filipe David Manana wrote:
>
>> On Mon, Jan 3, 2011 at 1:47 PM, Benoit Chesneau <bc...@gmail.com> wrote:
>>> On Mon, Jan 3, 2011 at 1:50 PM, Robert Newson <ro...@gmail.com> wrote:
>>>> Seconded. That's a big change with zero discussion and no Jira ticket.
>>>> +1 on reverting until a discussion is had.
>>>>
>>>> B.
>>> These changes don't introduce any regressions, and are well tested.
>>> Did you read the code ?
>>>
>>
>> It's not a question of reading or not the code.
>> All the tests pass, but to me that only means "maybe there aren't any
>> regressions".
>>
>> I believe there are very good reasons for having it in Bigcouch and CouchDB.
>> But I would like to have before a vote and have feedback from Adam
>> regarding no issues with standard CouchDB, as I believe he's the one
>> that knows better what the implications might be.
>
> In my opinion all of the changes in Benoit's commit belong in CouchDB, I simply haven't gotten around to introducing them. I do think that the right way to commit them is to separate the patch into a series of independent, isolated changesets.
>
> I think that the practice of filing a JIRA ticket for each non-trivial commit (aka the fdmanana method) is reasonable and not too onerous. Detailed commit messages describing the reason for the change (aka the davisp method) are also a good idea for the changes we're discussing here, as they improve the interaction between CouchDB and the OTP subsystem in subtle but important ways.
>
> I don't want us to move to a strict RTC procedure; the lazy consensus we've been using so far has worked just fine in my opinion. Best,
>
> Adam
Re: svn commit: r1054594 - in /couchdb/trunk: etc/couchdb/ src/couchdb/
Posted by Benoit Chesneau <bc...@gmail.com>.
On Mon, Jan 3, 2011 at 5:25 PM, Filipe David Manana <fd...@apache.org> wrote:
> On Mon, Jan 3, 2011 at 3:49 PM, Adam Kocoloski <ko...@apache.org> wrote:
>> In my opinion all of the changes in Benoit's commit belong in CouchDB, I simply haven't gotten around to introducing them. I do think that the right way to commit them is to separate the patch into a series of independent, isolated changesets.
>
> I think it's something good to have too, and I doubt anyone has
> something against a more OTP compliant organization.
>
> My concerns were:
>
> 1) The patch introduced 2 distinct changes: supervision tree and
> handling of configuration changes; Like you and Paul said before, it
> should be spitted into separate patches;
>
> 2) Bigcouch has many changes compared to the standard CouchDB. Maybe
> this patch only worked fine along with some of those other
> differences;
>
> 3) While the supervision and configuration changes handling approaches
> Bigcouch uses work, maybe the developers were aware of existing
> limitations and didn't recommend it for standard CouchDB;
>
> 4) I believe that Adam is the one with more knowledge and production
> experience with supervision trees and OTP best practices. Therefore he
> should give his approval before introducing these changes
>
>>
>> I think that the practice of filing a JIRA ticket for each non-trivial commit (aka the fdmanana method) is reasonable and not too onerous. Detailed commit messages describing the reason for the change (aka the davisp method) are also a good idea for the changes we're discussing here, as they improve the interaction between CouchDB and the OTP subsystem in subtle but important ways.
>>
>> I don't want us to move to a strict RTC procedure; the lazy consensus we've been using so far has worked just fine in my opinion. Best,
>
> Agreed.
>
>>
>> Adam
>
>
>
> --
> Filipe David Manana,
> fdmanana@gmail.com, fdmanana@apache.org
>
> "Reasonable men adapt themselves to the world.
> Unreasonable men adapt the world to themselves.
> That's why all progress depends on unreasonable men."
>
This thread is already closed for me. I'm the first to call for a
review process.
Though sometimes, since it was done, someone could just read the code
and test and eventually ask for split, or rever with perfectly good
reason not just because of a process that is ignored sometime for good
reason too. Or maybe we could just remove the word "relax" from our
vocabulary.
- benoît
Re: svn commit: r1054594 - in /couchdb/trunk: etc/couchdb/ src/couchdb/
Posted by Filipe David Manana <fd...@apache.org>.
On Mon, Jan 3, 2011 at 3:49 PM, Adam Kocoloski <ko...@apache.org> wrote:
> In my opinion all of the changes in Benoit's commit belong in CouchDB, I simply haven't gotten around to introducing them. I do think that the right way to commit them is to separate the patch into a series of independent, isolated changesets.
I think it's something good to have too, and I doubt anyone has
something against a more OTP compliant organization.
My concerns were:
1) The patch introduced 2 distinct changes: supervision tree and
handling of configuration changes; Like you and Paul said before, it
should be spitted into separate patches;
2) Bigcouch has many changes compared to the standard CouchDB. Maybe
this patch only worked fine along with some of those other
differences;
3) While the supervision and configuration changes handling approaches
Bigcouch uses work, maybe the developers were aware of existing
limitations and didn't recommend it for standard CouchDB;
4) I believe that Adam is the one with more knowledge and production
experience with supervision trees and OTP best practices. Therefore he
should give his approval before introducing these changes
>
> I think that the practice of filing a JIRA ticket for each non-trivial commit (aka the fdmanana method) is reasonable and not too onerous. Detailed commit messages describing the reason for the change (aka the davisp method) are also a good idea for the changes we're discussing here, as they improve the interaction between CouchDB and the OTP subsystem in subtle but important ways.
>
> I don't want us to move to a strict RTC procedure; the lazy consensus we've been using so far has worked just fine in my opinion. Best,
Agreed.
>
> Adam
--
Filipe David Manana,
fdmanana@gmail.com, fdmanana@apache.org
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
Re: svn commit: r1054594 - in /couchdb/trunk: etc/couchdb/ src/couchdb/
Posted by Adam Kocoloski <ko...@apache.org>.
On Jan 3, 2011, at 9:01 AM, Filipe David Manana wrote:
> On Mon, Jan 3, 2011 at 1:47 PM, Benoit Chesneau <bc...@gmail.com> wrote:
>> On Mon, Jan 3, 2011 at 1:50 PM, Robert Newson <ro...@gmail.com> wrote:
>>> Seconded. That's a big change with zero discussion and no Jira ticket.
>>> +1 on reverting until a discussion is had.
>>>
>>> B.
>> These changes don't introduce any regressions, and are well tested.
>> Did you read the code ?
>>
>
> It's not a question of reading or not the code.
> All the tests pass, but to me that only means "maybe there aren't any
> regressions".
>
> I believe there are very good reasons for having it in Bigcouch and CouchDB.
> But I would like to have before a vote and have feedback from Adam
> regarding no issues with standard CouchDB, as I believe he's the one
> that knows better what the implications might be.
In my opinion all of the changes in Benoit's commit belong in CouchDB, I simply haven't gotten around to introducing them. I do think that the right way to commit them is to separate the patch into a series of independent, isolated changesets.
I think that the practice of filing a JIRA ticket for each non-trivial commit (aka the fdmanana method) is reasonable and not too onerous. Detailed commit messages describing the reason for the change (aka the davisp method) are also a good idea for the changes we're discussing here, as they improve the interaction between CouchDB and the OTP subsystem in subtle but important ways.
I don't want us to move to a strict RTC procedure; the lazy consensus we've been using so far has worked just fine in my opinion. Best,
Adam
Re: svn commit: r1054594 - in /couchdb/trunk: etc/couchdb/ src/couchdb/
Posted by Filipe David Manana <fd...@apache.org>.
On Mon, Jan 3, 2011 at 1:47 PM, Benoit Chesneau <bc...@gmail.com> wrote:
> On Mon, Jan 3, 2011 at 1:50 PM, Robert Newson <ro...@gmail.com> wrote:
>> Seconded. That's a big change with zero discussion and no Jira ticket.
>> +1 on reverting until a discussion is had.
>>
>> B.
> These changes don't introduce any regressions, and are well tested.
> Did you read the code ?
>
It's not a question of reading or not the code.
All the tests pass, but to me that only means "maybe there aren't any
regressions".
I believe there are very good reasons for having it in Bigcouch and CouchDB.
But I would like to have before a vote and have feedback from Adam
regarding no issues with standard CouchDB, as I believe he's the one
that knows better what the implications might be.
--
Filipe David Manana,
fdmanana@gmail.com, fdmanana@apache.org
"Reasonable men adapt themselves to the world.
Unreasonable men adapt the world to themselves.
That's why all progress depends on unreasonable men."
Re: svn commit: r1054594 - in /couchdb/trunk: etc/couchdb/ src/couchdb/
Posted by Benoit Chesneau <bc...@gmail.com>.
On Mon, Jan 3, 2011 at 1:50 PM, Robert Newson <ro...@gmail.com> wrote:
> Seconded. That's a big change with zero discussion and no Jira ticket.
> +1 on reverting until a discussion is had.
>
> B.
These changes don't introduce any regressions, and are well tested.
Did you read the code ?
Re: svn commit: r1054594 - in /couchdb/trunk: etc/couchdb/ src/couchdb/
Posted by Robert Newson <ro...@gmail.com>.
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
<fd...@apache.org> 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, <be...@apache.org> 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,
> fdmanana@gmail.com, fdmanana@apache.org
>
> "Reasonable men adapt themselves to the world.
> Unreasonable men adapt the world to themselves.
> That's why all progress depends on unreasonable men."
>
Re: svn commit: r1054594 - in /couchdb/trunk: etc/couchdb/ src/couchdb/
Posted by Robert Newson <ro...@gmail.com>.
I should note that bigcouch is not an Apache project and the term
"backport" does not apply. Don't misunderstand me, I'm a big fan of
bigcouch, but let's not assume that we can take anything from bigcouch
and put it into couchdb without discussion or even a tracking ticket.
I don't much like where this thread is going. I certainly don't want
to start a revert war, especially when I'm sure we've all made
non-trivial changes to couchdb without a ticket (though not as often
without an IRC discussion).
Instead, I think it's worth remembering that showing your change
before you make it will make everyones lives easier. Filipe, for
example, has made tickets and patches for two small issues today. I
much prefer that approach.
B.
On Mon, Jan 3, 2011 at 1:45 PM, Benoit Chesneau <bc...@gmail.com> wrote:
> On Mon, Jan 3, 2011 at 1:45 PM, Filipe David Manana <fd...@apache.org> 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
>>
>
> These patches are backport of bigcouch and well tested. I don't see
> any reason not using them. These are not really big changzs, just
> reorganisation of code.
>
Re: svn commit: r1054594 - in /couchdb/trunk: etc/couchdb/ src/couchdb/
Posted by Benoit Chesneau <bc...@gmail.com>.
On Mon, Jan 3, 2011 at 1:45 PM, Filipe David Manana <fd...@apache.org> 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
>
These patches are backport of bigcouch and well tested. I don't see
any reason not using them. These are not really big changzs, just
reorganisation of code.