[GitHub] [couchdb] rnewson commented on a diff in pull request #4199: Fix spurious unlock in close_db_if_idle

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

rnewson commented on code in PR #4199:
URL: https://github.com/apache/couchdb/pull/4199#discussion_r991084710


##########
src/couch/src/couch_server.erl:
##########
@@ -31,6 +31,7 @@
 -export([num_servers/0, couch_server/1, couch_dbs_pid_to_name/1, couch_dbs/1]).
 -export([aggregate_queue_len/0, get_spidermonkey_version/0]).
 -export([names/0]).
+-export([try_lock/2]).

Review Comment:
   I thought about it but it wouldn't be "symmetric" (in the sense that try_unlock would only unlock if locked, using another select_replace). We _know_ we acquired the lock, so we can release it without checking. Is that worth pulling out into an `unlock` function which is just the `update_element` function? I could do it, not sure it clarifies much. thoughts?



##########
src/couch/src/couch_lru.erl:
##########
@@ -46,9 +46,8 @@ close_int({Lru, DbName, Iter}, {Tree, Dict} = Cache) ->
     CouchDbs = couch_server:couch_dbs(DbName),
     CouchDbsPidToName = couch_server:couch_dbs_pid_to_name(DbName),
 
-    case ets:update_element(CouchDbs, DbName, {#entry.lock, locked}) of
-        true ->
-            [#entry{db = Db, pid = Pid}] = ets:lookup(CouchDbs, DbName),
+    case couch_server:try_lock(DbName, CouchDbs) of

Review Comment:
   yeah.. 



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

To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org