[GitHub] [couchdb] theburge commented on a change in pull request #2130: [WIP] close LRU by database path

[GitBox <gi...@apache.org>]

theburge commented on a change in pull request #2130: [WIP] close LRU by database path
URL: https://github.com/apache/couchdb/pull/2130#discussion_r316629651
 
 

 ##########
 File path: src/dreyfus/src/dreyfus_index_manager.erl
 ##########
 @@ -123,6 +123,7 @@ handle_db_event(DbName, deleted, _St) ->
         "enable_database_recovery", false),
     case RecoveryEnabled of
         true ->
+            clouseau_rpc:close_lru(DbName),
 
 Review comment:
   There's an inconsistency here: `clouseau:close_lru/1` is called in the DB event handler, while the `rename` message is processed by the `dreyfus_index_manager` gen_server (after casting to it). As I mentioned over in the [partner clouseau PR](https://github.com/cloudant-labs/clouseau/pull/23), this opens up a race condition.
   
   I think we ought to do one or the other. In general, I'd say offload work to the caller if you can, but in this case the caller is our `couch_event` listener, so that might not be smart (we could end up backlogging that). However, pushing more traffic through `dreyfus_index_manager` is also not ideal... :)
   
   In terms of the race condition, it probably makes the most sense to move both clouseau RPCs into `dreyfus_index_manager:handle_cast/3`, so that the gen_server serialises the close/rename and other open (aka `get_index`) operations.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services