request help: what is the use of item_release in lrucache.lua

[HelloBug0]

Issue description

When I study the code of APISIX, I'm not quite clear the file apisix/core/lrucache.lua from line 100 to line 134(here)

• What is the use of item_release? And I searched the project about this value(actually it's a function，right?), I couldn't find it.
• Based on the last question, the value of item_release is nil, why should we call the function fetch_valid_cache twice in line 100 and line 119?
• Why do we need lock if we are using lru_cache which is process level cache?

Environment

• apisix version (master):
• OS: (Linux VM-0-7-centos 3.10.0-1127.19.1.el7.x86_64 change: added doc of how to load plugin. #1 SMP Tue Aug 25 17:23:54 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux)
• OpenResty version: (nginx version: openresty/1.19.3.1
  built by gcc 8.3.1 20190311 (Red Hat 8.3.1-3) (GCC)
  built with OpenSSL 1.1.1h 22 Sep 2020
  TLS SNI support enabled
  configure arguments: --prefix=/usr/local/openresty/nginx --with-cc-opt='-O2 -DNGX_LUA_ABORT_AT_PANIC -I/usr/local/openresty/zlib/include -I/usr/local/openresty/pcre/include -I/usr/local/openresty/openssl111/include' --add-module=../ngx_devel_kit-0.3.1 --add-module=../echo-nginx-module-0.62 --add-module=../xss-nginx-module-0.06 --add-module=../ngx_coolkit-0.2 --add-module=../set-misc-nginx-module-0.32 --add-module=../form-input-nginx-module-0.12 --add-module=../encrypted-session-nginx-module-0.08 --add-module=../srcache-nginx-module-0.32 --add-module=../ngx_lua-0.10.19 --add-module=../ngx_lua_upstream-0.07 --add-module=../headers-more-nginx-module-0.33 --add-module=../array-var-nginx-module-0.05 --add-module=../memc-nginx-module-0.19 --add-module=../redis2-nginx-module-0.15 --add-module=../redis-nginx-module-0.3.7 --add-module=../ngx_stream_lua-0.0.9 --with-ld-opt='-Wl,-rpath,/usr/local/openresty/luajit/lib -L/usr/local/openresty/zlib/lib -L/usr/local/openresty/pcre/lib -L/usr/local/openresty/openssl111/lib -Wl,-rpath,/usr/local/openresty/zlib/lib:/usr/local/openresty/pcre/lib:/usr/local/openresty/openssl111/lib' --with-cc='ccache gcc -fdiagnostics-color=always' --with-pcre-jit --with-stream --with-stream_ssl_module --with-stream_ssl_preread_module --with-http_v2_module --without-mail_pop3_module --without-mail_imap_module --without-mail_smtp_module --with-http_stub_status_module --with-http_realip_module --with-http_addition_module --with-http_auth_request_module --with-http_secure_link_module --with-http_random_index_module --with-http_gzip_static_module --with-http_sub_module --with-http_dav_module --with-http_flv_module --with-http_mp4_module --with-http_gunzip_module --with-threads --with-compat --with-stream --with-http_ssl_module)

[spacewander]

1, 2: the item_release is a callback to be called when cached value is released. It isn't used now.
3: the lock is bypassed in most of the cases except serial_creating is set. It is a way to protect the backend from hitting by multiple workers, like Nginx's proxy_cache_lock.

[HelloBug0]

I really appreciate your explanation. Merry Christmas 🍬

But why don't we use lock before line 100 if it's serial created ?

[spacewander]

I really appreciate your explanation. Merry Christmas 🍬

But why don't we use lock before line 100 if it's serial created ?

I am confused. From 85 to 100, it is for not serial created:

apisix/apisix/core/lrucache.lua

Lines 85 to 100 in 966d688

	if not serial_creating or not can_yield_phases[get_phase()] then
	local cache_obj = fetch_valid_cache(lru_obj, invalid_stale,
	item_ttl, item_release, key, version)
	if cache_obj then
	return cache_obj.val
	end
	local obj, err = create_obj_fun(...)
	if obj ~= nil then
	lru_obj:set(key, {val = obj, ver = version}, item_ttl)
	end
	return obj, err
	end
	local cache_obj = fetch_valid_cache(lru_obj, invalid_stale, item_ttl,

There is no place to use lock before line 100 if it's serial created.

[HelloBug0]

Oh, I mean locking from line 100 to line 104. Sorry about the mistake.

        local cache_obj = fetch_valid_cache(lru_obj, invalid_stale, item_ttl,
                            item_release, key, version)
        if cache_obj then
            return cache_obj.val
        end

[spacewander]

We only need lock when the cache is miss, and need to create the object (via query other system)

[HelloBug0]

It's possible to create the object in function fetch_valid_cache in line 100.

local function fetch_valid_cache(lru_obj, invalid_stale, item_ttl,
                                 item_release, key, version)
    local obj, stale_obj = lru_obj:get(key)
    if obj and obj.ver == version then
        return obj
    end

    if not invalid_stale and stale_obj and stale_obj.ver == version then
        lru_obj:set(key, stale_obj, item_ttl)
        return stale_obj
    end

    if item_release and obj then
        item_release(obj.val)
    end

    return nil
end

[spacewander]

There is only a getter, and a setter to set a stale object back as valid object. There is not a way to create a new one.

[HelloBug0]

I see. There is need to protect the cache when setting a stale object. In this way, I also have two questions.

• Why do we lock the function execution of fetch_valid_cache in line 119? Is it OK if we call the function out of the lock?
• I still don't clear that why we call the function of fetch_valid_cache twice. It‘s natually to think that the code from line 100 to line 134 to be programed like this.

        local cache_obj = fetch_valid_cache(lru_obj, invalid_stale, item_ttl,
                            item_release, key, version)
        if cache_obj then
            return cache_obj.val
        end

        local lock, err = resty_lock:new(lock_shdict_name)
        if not lock then
            return nil, "failed to create lock: " .. err
        end

        local key_s = tostring(key)
        log.info("try to lock with key ", key_s)

        local elapsed, err = lock:lock(key_s)
        if not elapsed then
            return nil, "failed to acquire the lock: " .. err
        end

        local obj, err = create_obj_fun(...)
        if obj ~= nil then
            lru_obj:set(key, {val = obj, ver = version}, item_ttl)
        end
        lock:unlock()
        log.info("unlock with key ", key_s)

        return obj, err

[spacewander]

The second fetch_valid_cache is a cache with lock pattern. You can take a look at:
https://github.com/thibaultcha/lua-resty-mlcache/blob/acf6d6e9d83d983425e0735b696a721851336167/lib/resty/mlcache.lua#L685
Its comment explains how this pattern work.

However, as we only get cached data from lrucache, which is process level cache, so it is safe to remove the second try. PR is welcome!

[HelloBug0]

OK, I will check that. Thanks for your help.

[spacewander]

Ping @HelloBug0?

[HelloBug0]

Um?

[spacewander]

@HelloBug0

However, as we only get cached data from lrucache, which is process level cache, so it is safe to remove the second try. PR is welcome!

OK, I will check that. Thanks for your help.

If I didn't misunderstand your words, it seems you are going to submit a PR?

[HelloBug0]

Sorry to replay this later and not make myself clearly.
Yeah, I'm willing to submit a PR, but before that, I want to spend more time studying APISIX code which is I am doing now.