iilyak commented on issue #477: Add LRU for view groups
URL: https://github.com/apache/couchdb/pull/477#issuecomment-305937285
 
 
   > @davisp
    I keep thinking there must be something a bit cleaner here with exposing a 
new API function to ask the module if its index is idle.
   
   Ideally we would want to minimize the interaction with couch_index_server 
since it is a bottlneck. 
   If we can ensure that it is impossible to have at any given time multiple 
index processes for the same index we could move locking out of 
couch_index_server. In such case the maybe_close_index function would be 
simpler (maybe).
   
   I also experimented with some other possibilities a little bit to make it 
more generic.  The goal was to make couch_server and couch_index_server 
similar. Most likely it is overkill. I tend to over engineer at times.
     
   couch_lock.erl
   ---------------
   ```
   new(Table, Key, Pos) ->
       {Table, Key, Pos}.
   with_lock({Table, Key, Pos}, Default, Fun) ->
       case ets:update_element(Table, Key, {Pos, true}) of
            true ->
                 try 
                      [Entry] = ets:lookup(Table, Key),
                     Fun(Entry)
                 after
                     ets:update_element(Table, Key, Pos, false)
                 end;
            false ->
                Default
      end.
   ```
   
   couch_index.erl
   ----------------
   ```
   txn({DbName, DDocId, Sig}, Default, Fun) ->
       Lock = couch_lock:lock(?BY_SIG, {DbName, Sig}, #entry.locked),
       couch_lock:with_lock(Lock, Default, Fun). %% we don't have it yet
   
   is_idle(Pid) ->
            gen_server:call(Pid, is_idle).
   
   set_committing(Pid, Value) ->
            gen_server:call(Pid, {set_committing, Value}).
   
   set_compacting(Pid, Value) ->
            gen_server:call(Pid, {set_compacting, Value}).
   ```
   
   couch_index_server.erl
   -----------------------
   ```
   maybe_close_index({DbName, DDocId, Sig} = Idx) ->
      couch_index:txn(Idx, {false, true}, fun(#entry{pid =Pid}) ->
           case couch_index:is_idle(Pid) of
               true -> 
                    rem_from_ets(DbName, Sig, DDocId, Pid),
                    couch_index:stop(Pid),
                    {true, true};
               false ->
                   couch_stats:increment_counter([couchdb, couch_index_server, 
lru_skip]),
                   {false, false}
           end
      end).
   ```
   
   couch_server.erl
   ----------------
   ```
   txn(DbName, Default, Fun) ->
       Lock = couch_lock:lock(couch_dbs, DbName, #db.fd_monitor),
       couch_lock:with_lock(Lock, Default, Fun). %% we don't have it yet
   
   maybe_close_db(DbName) ->
      couch_server:txn(Idx, {false, true}, fun(#db{main_pid =Pid} = Db) ->
           case couch_db:is_idle(Db) of
               true -> 
                   true = ets:delete(couch_dbs, DbName),
                   true = ets:delete(couch_dbs_pid_to_name, Pid),
                   exit(Pid, kill),
                   {true, true};
               false ->
                   couch_stats:increment_counter([couchdb, couch_server, 
lru_skip]),
                   {false, false}
           end
      end).
   ```
 
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to