nickva commented on code in PR #5168:
URL: https://github.com/apache/couchdb/pull/5168#discussion_r1703377124
##########
src/couch/src/couch_lru.erl:
##########
@@ -62,5 +71,166 @@ close_int({Lru, DbName, Iter}, {Tree, Dict} = Cache) ->
false ->
NewTree = gb_trees:delete(Lru, Tree),
NewIter = gb_trees:iterator(NewTree),
- close_int(gb_trees:next(NewIter), {NewTree, dict:erase(DbName,
Dict)})
+ close_int(gb_trees:next(NewIter), {NewTree, maps:remove(DbName,
Map)})
end.
+
+-ifdef(TEST).
+
+-include_lib("couch/include/couch_eunit.hrl").
+
+-define(DB1, <<"db1">>).
+-define(DB2, <<"db2">>).
+
+couch_lru_test_() ->
+ {
+ foreach,
+ fun setup/0,
+ fun teardown/1,
+ [
+ ?TDEF_FE(t_new),
+ ?TDEF_FE(t_insert),
+ ?TDEF_FE(t_insert_duplicate),
+ ?TDEF_FE(t_update),
+ ?TDEF_FE(t_close_empty),
+ ?TDEF_FE(t_close_unlocked_idle),
+ ?TDEF_FE(t_close_bump_busy_all),
+ ?TDEF_FE(t_close_bump_busy_one),
+ ?TDEF_FE(t_close_entry_one_is_missing)
+ ]
+ }.
+
+t_new(_) ->
+ Cache = new(),
+ ?assertMatch({_, _}, Cache),
+ {Tree, Map} = Cache,
+ ?assert(gb_trees:is_empty(Tree)),
+ ?assert(is_map(Map) andalso map_size(Map) == 0).
+
+t_insert(_) ->
+ {Tree, Map} = insert(?DB1, new()),
+ ?assertEqual(1, gb_trees:size(Tree)),
+ ?assertEqual(1, map_size(Map)),
+ ?assertMatch(#{?DB1 := _}, Map),
+ #{?DB1 := Int} = Map,
+ ?assert(is_integer(Int)),
+ ?assert(Int > 0),
+ ?assertEqual([{Int, ?DB1}], gb_trees:to_list(Tree)).
+
+t_insert_duplicate(_) ->
+ % We technically allow this, but is this right? Should we always use update
+ % instead which would reap the old LRU entry
Review Comment:
Oh, I wonder if this is the culprit that causes the bug in
https://github.com/apache/couchdb/issues/5166!
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
