nic-6443 commented on code in PR #14061:
URL: https://github.com/apache/apisix/pull/14061#discussion_r4373539704


##########
apisix/core/lrucache.lua:
##########
@@ -218,41 +263,86 @@ end
 
 
 local function refresh_stale_objs()
+    -- snapshot first: creating an object may yield, and the pool can change
+    -- meanwhile (new stale hits, purge_inactive_objs)
+    local pending = {}
     for lru_obj, keys in pairs(stale_obj_pool) do
         for key, new_obj in pairs(keys) do
-            local obj, err = new_obj.fn(unpack(new_obj.args))
-            if obj ~= nil then
-                lru_obj:set(key, {val = obj, ver = new_obj.ver}, new_obj.ttl)
+            pending[#pending + 1] = {lru_obj, keys, key, new_obj}
+        end
+    end
+
+    for _, item in ipairs(pending) do
+        local lru_obj, keys, key, new_obj = item[1], item[2], item[3], item[4]
+        if keys[key] ~= new_obj then
+            goto continue
+        end
+
+        local ver, ttl = new_obj.ver, new_obj.ttl
+        local obj, err = new_obj.fn(unpack(new_obj.args))
+        if obj ~= nil then
+            -- the item may have been purged while refreshing; writing it back
+            -- then would resurrect it without an access record
+            if keys[key] == new_obj then
+                lru_obj:set(key, {val = obj, ver = ver}, ttl)
                 keys[key] = nil
                 log.info("successfully refresh stale obj for key ",
-                            tostring(key), " to ver ", new_obj.ver)
-            else
-                log.error("failed to refresh stale obj for key ", key, ": ", 
err)
+                            tostring(key), " to ver ", ver)
             end
+        else
+            log.error("failed to refresh stale obj for key ", key, ": ", err)
         end
+
+        ::continue::
     end
 end
 
 
-local function init_worker()
+local function purge_inactive_objs()
+    local now = ngx_now()
+    for lru_obj, inactive_lru in pairs(inactive_lru_pool) do
+        local access_times = inactive_lru.access_times
+        local inactive = inactive_lru.inactive
+        local pending = stale_obj_pool[lru_obj]
+        for key, access_time in pairs(access_times) do
+            if now - access_time > inactive then
+                lru_obj:delete(key)

Review Comment:
   I don't think the case mentioned in this comment is an issue. A cache entry 
that has not been accessed during the inactive period is evicted to reclaim 
memory, which is expected behavior. It should be reloaded after invalidation. 
If an error occurs during reloading and causes a 500 request error, that is 
also an expected outcome.
   Previous versions behave the same way when lrucache eviction is triggered.



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

Reply via email to