(couchdb) 19/20: Fix update bug in ets_lru

[va...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

vatamane pushed a commit to branch 3.3.x-pending-changes-3.3.3
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 291981c6da581955c7e8e36ec8c6857c92d28ee6
Author: Nick Vatamaniuc <va...@gmail.com>
AuthorDate: Thu Nov 9 12:51:34 2023 -0500

    Fix update bug in ets_lru
    
    Previously, when an existing cache entry was updated, we didn't update the
    atime. Subsequently, when the entry was removed, we never cleaned the orphan
    atime entry. During trimming, we traverse the atime entries, and since the
    orphaned one didn't have any matching objects or ctime entries, the ets_lru
    cache would crash, possibly taking down the primary couch supervisor.
---
 src/ets_lru/src/ets_lru.erl       |  5 +++--
 src/ets_lru/test/ets_lru_test.erl | 28 ++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/ets_lru/src/ets_lru.erl b/src/ets_lru/src/ets_lru.erl
index 891aeeb57..6416fc8f1 100644
--- a/src/ets_lru/src/ets_lru.erl
+++ b/src/ets_lru/src/ets_lru.erl
@@ -167,8 +167,9 @@ handle_call({insert, Key, Val}, _From, St) ->
     Pattern = #entry{key = Key, atime = '$1', _ = '_'},
     case ets:match(St#st.objects, Pattern) of
         [[ATime]] ->
-            Update = {#entry.val, Val},
-            true = ets:update_element(St#st.objects, Key, Update),
+            true = ets:update_element(St#st.objects, Key, [
+                {#entry.val, Val}, {#entry.atime, NewATime}
+            ]),
             true = ets:delete(St#st.atimes, ATime),
             true = ets:insert(St#st.atimes, {NewATime, Key});
         [] ->
diff --git a/src/ets_lru/test/ets_lru_test.erl b/src/ets_lru/test/ets_lru_test.erl
index 5dd193f8d..eb5b79841 100644
--- a/src/ets_lru/test/ets_lru_test.erl
+++ b/src/ets_lru/test/ets_lru_test.erl
@@ -132,6 +132,34 @@ basic_behavior_test_() ->
                             ?_assertEqual(not_found, ets_lru:lookup(LRU, foo))
                         }
                     ]
+                end,
+                fun({ok, LRU}) ->
+                    [
+                        {
+                            "Insert the value twice",
+                            ?_assertEqual(ok, ets_lru:insert(LRU, foo, bar))
+                        },
+                        {
+                            "Objects table size should be 1",
+                            ?_assertEqual(1, ets:info(test_lru_objects, size))
+                        },
+                        {
+                            "ATimes table size should be 1",
+                            ?_assertEqual(1, ets:info(test_lru_atimes, size))
+                        },
+                        {
+                            "CTimes table size should be 1",
+                            ?_assertEqual(1, ets:info(test_lru_ctimes, size))
+                        },
+                        {
+                            "Clear LRU after duplicate insert",
+                            ?_assertEqual(ok, ets_lru:clear(LRU))
+                        },
+                        {
+                            "Lookup returned not_found after a clear after a duplicate insert",
+                            ?_assertEqual(not_found, ets_lru:lookup(LRU, foo))
+                        }
+                    ]
                 end
             ]}
     }.