couchdb-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Filipe David Manana <fdman...@apache.org>
Subject Re: svn commit: r1036294 - in /couchdb/trunk/src/couchdb: couch_auth_cache.erl couch_db.erl couch_db_updater.erl couch_view_group.erl
Date Thu, 18 Nov 2010 09:43:42 GMT
Hi Adam,

I tried that idea - opening the DB when needed and then closing it
after serving the client request. Somehow it was posing me problems
doing it like that.
But it's definitely the way to go, I agree. I'm not very familiar with
all the view related processes and their interactions, but it seems to
me they need a good cleanup/refactoring.

cheers

On Thu, Nov 18, 2010 at 4:18 AM, Adam Kocoloski <kocolosk@apache.org> wrote:
> Thanks for fixing this, Filipe.  I think we should revisit the decision to keep an open
reference to the DB in couch_view_group after 1.1.  I don't see any good reason for it; the
updater and compactor could easily open the DB themselves.  I think it would clean up a lot
of very messy and error-prone code.  Regards,
>
> Adam
>
> On Nov 17, 2010, at 7:11 PM, fdmanana@apache.org wrote:
>
>> Author: fdmanana
>> Date: Thu Nov 18 00:11:18 2010
>> New Revision: 1036294
>>
>> URL: http://svn.apache.org/viewvc?rev=1036294&view=rev
>> Log:
>> Make sure that after a database compaction the old database reference counters don't
get unreleased forever.
>> Closes COUCHDB-926.
>>
>>
>> Modified:
>>    couchdb/trunk/src/couchdb/couch_auth_cache.erl
>>    couchdb/trunk/src/couchdb/couch_db.erl
>>    couchdb/trunk/src/couchdb/couch_db_updater.erl
>>    couchdb/trunk/src/couchdb/couch_view_group.erl
>>
>> Modified: couchdb/trunk/src/couchdb/couch_auth_cache.erl
>> URL: http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_auth_cache.erl?rev=1036294&r1=1036293&r2=1036294&view=diff
>> ==============================================================================
>> --- couchdb/trunk/src/couchdb/couch_auth_cache.erl (original)
>> +++ couchdb/trunk/src/couchdb/couch_auth_cache.erl Thu Nov 18 00:11:18 2010
>> @@ -336,7 +336,7 @@ cache_needs_refresh() ->
>>
>>
>> reopen_auth_db(AuthDb) ->
>> -    case (catch gen_server:call(AuthDb#db.main_pid, get_db, infinity)) of
>> +    case (catch couch_db:reopen(AuthDb)) of
>>     {ok, AuthDb2} ->
>>         AuthDb2;
>>     _ ->
>>
>> Modified: couchdb/trunk/src/couchdb/couch_db.erl
>> URL: http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_db.erl?rev=1036294&r1=1036293&r2=1036294&view=diff
>> ==============================================================================
>> --- couchdb/trunk/src/couchdb/couch_db.erl (original)
>> +++ couchdb/trunk/src/couchdb/couch_db.erl Thu Nov 18 00:11:18 2010
>> @@ -27,6 +27,7 @@
>> -export([init/1,terminate/2,handle_call/3,handle_cast/2,code_change/3,handle_info/2]).
>> -export([changes_since/5,changes_since/6,read_doc/2,new_revid/1]).
>> -export([check_is_admin/1, check_is_reader/1]).
>> +-export([reopen/1]).
>>
>> -include("couch_db.hrl").
>>
>> @@ -86,6 +87,18 @@ open(DbName, Options) ->
>>         Else -> Else
>>     end.
>>
>> +reopen(#db{main_pid = Pid, fd_ref_counter = OldRefCntr}) ->
>> +    {ok, #db{fd_ref_counter = NewRefCntr} = NewDb} =
>> +        gen_server:call(Pid, get_db, infinity),
>> +    case NewRefCntr =:= OldRefCntr of
>> +    true ->
>> +        {ok, NewDb};
>> +    false ->
>> +        couch_ref_counter:add(NewRefCntr),
>> +        couch_ref_counter:drop(OldRefCntr),
>> +        {ok, NewDb}
>> +    end.
>> +
>> ensure_full_commit(#db{update_pid=UpdatePid,instance_start_time=StartTime}) ->
>>     ok = gen_server:call(UpdatePid, full_commit, infinity),
>>     {ok, StartTime}.
>>
>> Modified: couchdb/trunk/src/couchdb/couch_db_updater.erl
>> URL: http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_db_updater.erl?rev=1036294&r1=1036293&r2=1036294&view=diff
>> ==============================================================================
>> --- couchdb/trunk/src/couchdb/couch_db_updater.erl (original)
>> +++ couchdb/trunk/src/couchdb/couch_db_updater.erl Thu Nov 18 00:11:18 2010
>> @@ -188,6 +188,7 @@ handle_cast({compact_done, CompactFilepa
>>         close_db(Db),
>>         NewDb3 = refresh_validate_doc_funs(NewDb2),
>>         ok = gen_server:call(Db#db.main_pid, {db_updated, NewDb3}, infinity),
>> +        couch_db_update_notifier:notify({compacted, NewDb3#db.name}),
>>         ?LOG_INFO("Compaction for db \"~s\" completed.", [Db#db.name]),
>>         {noreply, NewDb3#db{compactor_pid=nil}};
>>     false ->
>>
>> Modified: couchdb/trunk/src/couchdb/couch_view_group.erl
>> URL: http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_view_group.erl?rev=1036294&r1=1036293&r2=1036294&view=diff
>> ==============================================================================
>> --- couchdb/trunk/src/couchdb/couch_view_group.erl (original)
>> +++ couchdb/trunk/src/couchdb/couch_view_group.erl Thu Nov 18 00:11:18 2010
>> @@ -32,7 +32,8 @@
>>     compactor_pid=nil,
>>     waiting_commit=false,
>>     waiting_list=[],
>> -    ref_counter=nil
>> +    ref_counter=nil,
>> +    db_update_notifier=nil
>> }).
>>
>> % api methods
>> @@ -75,7 +76,7 @@ start_link(InitArgs) ->
>>     end.
>>
>> % init creates a closure which spawns the appropriate view_updater.
>> -init({InitArgs, ReturnPid, Ref}) ->
>> +init({{_, DbName, _} = InitArgs, ReturnPid, Ref}) ->
>>     process_flag(trap_exit, true),
>>     try prepare_group(InitArgs, false) of
>>     {ok, #group{db=Db, fd=Fd, current_seq=Seq}=Group} ->
>> @@ -86,7 +87,15 @@ init({InitArgs, ReturnPid, Ref}) ->
>>         _ ->
>>             couch_db:monitor(Db),
>>             {ok, RefCounter} = couch_ref_counter:start([Fd]),
>> +            Server = self(),
>> +            {ok, Notifier} = couch_db_update_notifier:start_link(
>> +                fun({compacted, DbName1}) when DbName1 =:= DbName ->
>> +                        ok = gen_server:cast(Server, reopen_db);
>> +                    (_) ->
>> +                        ok
>> +                end),
>>             {ok, #group_state{
>> +                    db_update_notifier=Notifier,
>>                     db_name=couch_db:name(Db),
>>                     init_args=InitArgs,
>>                     group=Group,
>> @@ -119,11 +128,11 @@ init({InitArgs, ReturnPid, Ref}) ->
>> handle_call({request_group, RequestSeq}, From,
>>         #group_state{
>>             db_name=DbName,
>> -            group=#group{current_seq=Seq}=Group,
>> +            group=#group{current_seq=Seq, db=OldDb}=Group,
>>             updater_pid=nil,
>>             waiting_list=WaitList
>>             }=State) when RequestSeq > Seq ->
>> -    {ok, Db} = couch_db:open_int(DbName, []),
>> +    {ok, Db} = reopen_db(DbName, OldDb),
>>     Group2 = Group#group{db=Db},
>>     Owner = self(),
>>     Pid = spawn_link(fun()-> couch_view_updater:update(Owner, Group2) end),
>> @@ -158,11 +167,11 @@ handle_call(request_group_info, _From, S
>> handle_cast({start_compact, CompactFun}, #group_state{compactor_pid=nil}
>>         = State) ->
>>     #group_state{
>> -        group = #group{name = GroupId, sig = GroupSig} = Group,
>> +        group = #group{name = GroupId, sig = GroupSig, db = OldDb} = Group,
>>         init_args = {RootDir, DbName, _}
>>     } = State,
>>     ?LOG_INFO("View index compaction starting for ~s ~s", [DbName, GroupId]),
>> -    {ok, Db} = couch_db:open_int(DbName, []),
>> +    {ok, Db} = reopen_db(DbName, OldDb),
>>     {ok, Fd} = open_index_file(compact, RootDir, DbName, GroupSig),
>>     NewGroup = reset_file(Db, Fd, DbName, Group),
>>     Pid = spawn_link(fun() -> CompactFun(Group, NewGroup) end),
>> @@ -225,13 +234,14 @@ handle_cast({compact_done, NewGroup}, St
>>     ?LOG_INFO("View index compaction still behind for ~s ~s -- current: ~p " ++
>>         "compact: ~p", [DbName, GroupId, CurrentSeq, NewGroup#group.current_seq]),
>>     couch_db:close(NewGroup#group.db),
>> -    {ok, Db} = couch_db:open_int(DbName, []),
>>     Pid = spawn_link(fun() ->
>> +        {ok, Db} = couch_db:open_int(DbName, []),
>>         {_,Ref} = erlang:spawn_monitor(fun() ->
>>             couch_view_updater:update(nil, NewGroup#group{db = Db})
>>         end),
>>         receive
>>             {'DOWN', Ref, _, _, {new_group, NewGroup2}} ->
>> +                couch_db:close(Db),
>>                 #group{name=GroupId} = NewGroup2,
>>                 Pid2 = couch_view:get_group_server(DbName, GroupId),
>>                 gen_server:cast(Pid2, {compact_done, NewGroup2})
>> @@ -255,7 +265,11 @@ handle_cast({partial_update, Pid, NewGro
>>     {noreply, State#group_state{group=NewGroup, waiting_commit=true}};
>> handle_cast({partial_update, _, _}, State) ->
>>     %% message from an old (probably pre-compaction) updater; ignore
>> -    {noreply, State}.
>> +    {noreply, State};
>> +
>> +handle_cast(reopen_db, #group_state{group = Group, db_name = DbName} = State) ->
>> +    {ok, Db} = reopen_db(DbName, Group#group.db),
>> +    {noreply, State#group_state{group = Group#group{db = Db}}}.
>>
>> handle_info(delayed_commit, #group_state{db_name=DbName,group=Group}=State) ->
>>     {ok, Db} = couch_db:open_int(DbName, []),
>> @@ -341,6 +355,7 @@ handle_info({'DOWN',_,_,_,_}, State) ->
>>
>>
>> terminate(Reason, #group_state{updater_pid=Update, compactor_pid=Compact}=S) ->
>> +    couch_db_update_notifier:stop(S#group_state.db_update_notifier),
>>     reply_all(S, Reason),
>>     couch_util:shutdown_sync(Update),
>>     couch_util:shutdown_sync(Compact),
>> @@ -372,8 +387,8 @@ reply_all(#group_state{waiting_list=Wait
>>     [catch gen_server:reply(Pid, Reply) || {Pid, _} <- WaitList],
>>     State#group_state{waiting_list=[]}.
>>
>> -prepare_group({RootDir, DbName, #group{sig=Sig}=Group}, ForceReset)->
>> -    case couch_db:open_int(DbName, []) of
>> +prepare_group({RootDir, DbName, #group{sig=Sig, db=OldDb}=Group}, ForceReset)->
>> +    case reopen_db(DbName, OldDb) of
>>     {ok, Db} ->
>>         case open_index_file(RootDir, DbName, Sig) of
>>         {ok, Fd} ->
>> @@ -634,4 +649,7 @@ init_group(Db, Fd, #group{def_lang=Lang,
>>     Group#group{db=Db, fd=Fd, current_seq=Seq, purge_seq=PurgeSeq,
>>         id_btree=IdBtree, views=Views2}.
>>
>> -
>> +reopen_db(DbName, nil) ->
>> +    couch_db:open_int(DbName, []);
>> +reopen_db(_DbName, Db) ->
>> +    couch_db:reopen(Db).
>>
>>
>
>



-- 
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."

Mime
View raw message