Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
nic-6443 commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r2193898420
##
apisix/consumer.lua:
##
@@ -101,46 +152,21 @@ local function plugin_consumer()
if not plugins[name] then
plugins[name] = {
nodes = {},
+len = 0,
conf_version = consumers.conf_version
}
end
--- if the val is a Consumer, clone it to the local consumer;
--- if the val is a Credential, to get the Consumer by
consumer_name and then clone
--- it to the local consumer.
-local consumer
-if is_credential_etcd_key(val.key) then
-local consumer_name =
get_consumer_name_from_credential_etcd_key(val.key)
-local the_consumer = consumers:get(consumer_name)
-if the_consumer and the_consumer.value then
-consumer = core.table.clone(the_consumer.value)
-consumer.modifiedIndex = the_consumer.modifiedIndex
-consumer.credential_id =
get_credential_id_from_etcd_key(val.key)
-else
--- Normally wouldn't get here:
--- it should belong to a consumer for any credential.
-core.log.error("failed to get the consumer for the
credential,",
-" a wild credential has appeared!",
-" credential key: ", val.key, ", consumer name: ",
consumer_name)
-goto CONTINUE
-end
-else
-consumer = core.table.clone(val.value)
-consumer.modifiedIndex = val.modifiedIndex
-end
-
--- if the consumer has labels, set the field custom_id to it.
--- the custom_id is used to set in the request headers to the
upstream.
-if consumer.labels then
-consumer.custom_id = consumer.labels["custom_id"]
+local consumer = consumers_id_lrucache(val.value.id .. name,
+val.modifiedIndex, construct_consumer_data, val,
config)
Review Comment:
We cannot cache the result of `construct_consumer_data` here, because even
when there is no change in credential (`val.modifiedIndex` has not changed),
the consumer might have been modified. We need to design different versions for
`consumer` and `credential`.
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
membphis commented on PR #11840: URL: https://github.com/apache/apisix/pull/11840#issuecomment-2636822411 many thx @xuruidong 's contribution, merged your PR right now -- 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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
membphis merged PR #11840: URL: https://github.com/apache/apisix/pull/11840 -- 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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
membphis commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1942922210
##
apisix/consumer.lua:
##
@@ -80,7 +85,53 @@ local function filter_consumers_list(data_list)
return list
end
-local function plugin_consumer()
+local plugin_consumer
+do
+local consumers_id_lrucache = core.lrucache.new({
+count = consumers_count_for_lrucache
+})
+
+local function construct_consumer_data(val, plugin_config)
+-- if the val is a Consumer, clone it to the local consumer;
+-- if the val is a Credential, to get the Consumer by consumer_name and
then clone
+-- it to the local consumer.
+local consumer
+if is_credential_etcd_key(val.key) then
+local consumer_name =
get_consumer_name_from_credential_etcd_key(val.key)
+local the_consumer = consumers:get(consumer_name)
+if the_consumer and the_consumer.value then
+consumer = core.table.clone(the_consumer.value)
+consumer.modifiedIndex = the_consumer.modifiedIndex
+consumer.credential_id = get_credential_id_from_etcd_key(val.key)
+else
+-- Normally wouldn't get here:
+-- it should belong to a consumer for any credential.
+core.log.error("failed to get the consumer for the credential,",
+" a wild credential has appeared!",
+" credential key: ", val.key, ", consumer name: ",
consumer_name)
+return nil, "failed to get the consumer for the credential"
+end
+else
+consumer = core.table.clone(val.value)
+consumer.modifiedIndex = val.modifiedIndex
Review Comment:
sorry, it is my fault, you're right
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
xuruidong commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1942443179
##
apisix/consumer.lua:
##
@@ -80,7 +85,53 @@ local function filter_consumers_list(data_list)
return list
end
-local function plugin_consumer()
+local plugin_consumer
+do
+local consumers_id_lrucache = core.lrucache.new({
+count = consumers_count_for_lrucache
+})
+
+local function construct_consumer_data(val, plugin_config)
+-- if the val is a Consumer, clone it to the local consumer;
+-- if the val is a Credential, to get the Consumer by consumer_name and
then clone
+-- it to the local consumer.
+local consumer
+if is_credential_etcd_key(val.key) then
+local consumer_name =
get_consumer_name_from_credential_etcd_key(val.key)
+local the_consumer = consumers:get(consumer_name)
+if the_consumer and the_consumer.value then
+consumer = core.table.clone(the_consumer.value)
+consumer.modifiedIndex = the_consumer.modifiedIndex
+consumer.credential_id = get_credential_id_from_etcd_key(val.key)
+else
+-- Normally wouldn't get here:
+-- it should belong to a consumer for any credential.
+core.log.error("failed to get the consumer for the credential,",
+" a wild credential has appeared!",
+" credential key: ", val.key, ", consumer name: ",
consumer_name)
+return nil, "failed to get the consumer for the credential"
+end
+else
+consumer = core.table.clone(val.value)
+consumer.modifiedIndex = val.modifiedIndex
Review Comment:
`modifiedIndex` is a member of `val`, not a member of `val.value`,
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
membphis commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1942406605
##
apisix/consumer.lua:
##
@@ -80,7 +85,53 @@ local function filter_consumers_list(data_list)
return list
end
-local function plugin_consumer()
+local plugin_consumer
+do
+local consumers_id_lrucache = core.lrucache.new({
+count = consumers_count_for_lrucache
+})
+
+local function construct_consumer_data(val, plugin_config)
+-- if the val is a Consumer, clone it to the local consumer;
+-- if the val is a Credential, to get the Consumer by consumer_name and
then clone
+-- it to the local consumer.
+local consumer
+if is_credential_etcd_key(val.key) then
+local consumer_name =
get_consumer_name_from_credential_etcd_key(val.key)
+local the_consumer = consumers:get(consumer_name)
+if the_consumer and the_consumer.value then
+consumer = core.table.clone(the_consumer.value)
+consumer.modifiedIndex = the_consumer.modifiedIndex
+consumer.credential_id = get_credential_id_from_etcd_key(val.key)
+else
+-- Normally wouldn't get here:
+-- it should belong to a consumer for any credential.
+core.log.error("failed to get the consumer for the credential,",
+" a wild credential has appeared!",
+" credential key: ", val.key, ", consumer name: ",
consumer_name)
+return nil, "failed to get the consumer for the credential"
+end
+else
+consumer = core.table.clone(val.value)
+consumer.modifiedIndex = val.modifiedIndex
Review Comment:
What do you think?
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
membphis commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1942405999
##
apisix/consumer.lua:
##
@@ -80,7 +85,53 @@ local function filter_consumers_list(data_list)
return list
end
-local function plugin_consumer()
+local plugin_consumer
+do
+local consumers_id_lrucache = core.lrucache.new({
+count = consumers_count_for_lrucache
+})
+
+local function construct_consumer_data(val, plugin_config)
+-- if the val is a Consumer, clone it to the local consumer;
+-- if the val is a Credential, to get the Consumer by consumer_name and
then clone
+-- it to the local consumer.
+local consumer
+if is_credential_etcd_key(val.key) then
+local consumer_name =
get_consumer_name_from_credential_etcd_key(val.key)
+local the_consumer = consumers:get(consumer_name)
+if the_consumer and the_consumer.value then
+consumer = core.table.clone(the_consumer.value)
+consumer.modifiedIndex = the_consumer.modifiedIndex
+consumer.credential_id = get_credential_id_from_etcd_key(val.key)
+else
+-- Normally wouldn't get here:
+-- it should belong to a consumer for any credential.
+core.log.error("failed to get the consumer for the credential,",
+" a wild credential has appeared!",
+" credential key: ", val.key, ", consumer name: ",
consumer_name)
+return nil, "failed to get the consumer for the credential"
+end
+else
+consumer = core.table.clone(val.value)
+consumer.modifiedIndex = val.modifiedIndex
Review Comment:
I think `consumer.modifiedIndex` equal `val.modifiedIndex` after we called
`core.table.clone(val.value)1`
We do not need to set this value again
Only comments should be enough.
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
xuruidong commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1942329043
##
apisix/consumer.lua:
##
@@ -80,7 +85,53 @@ local function filter_consumers_list(data_list)
return list
end
-local function plugin_consumer()
+local plugin_consumer
+do
+local consumers_id_lrucache = core.lrucache.new({
+count = consumers_count_for_lrucache
+})
+
+local function construct_consumer_data(val, plugin_config)
+-- if the val is a Consumer, clone it to the local consumer;
+-- if the val is a Credential, to get the Consumer by consumer_name and
then clone
+-- it to the local consumer.
+local consumer
+if is_credential_etcd_key(val.key) then
+local consumer_name =
get_consumer_name_from_credential_etcd_key(val.key)
+local the_consumer = consumers:get(consumer_name)
+if the_consumer and the_consumer.value then
+consumer = core.table.clone(the_consumer.value)
+consumer.modifiedIndex = the_consumer.modifiedIndex
+consumer.credential_id = get_credential_id_from_etcd_key(val.key)
+else
+-- Normally wouldn't get here:
+-- it should belong to a consumer for any credential.
+core.log.error("failed to get the consumer for the credential,",
+" a wild credential has appeared!",
+" credential key: ", val.key, ", consumer name: ",
consumer_name)
+return nil, "failed to get the consumer for the credential"
+end
+else
+consumer = core.table.clone(val.value)
+consumer.modifiedIndex = val.modifiedIndex
Review Comment:
https://github.com/apache/apisix/pull/7965
This PR may explain the purpose of the "modifiedIndex" variable.
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
membphis commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1942250148
##
apisix/consumer.lua:
##
@@ -80,7 +85,53 @@ local function filter_consumers_list(data_list)
return list
end
-local function plugin_consumer()
+local plugin_consumer
+do
+local consumers_id_lrucache = core.lrucache.new({
+count = consumers_count_for_lrucache
+})
+
+local function construct_consumer_data(val, plugin_config)
+-- if the val is a Consumer, clone it to the local consumer;
+-- if the val is a Credential, to get the Consumer by consumer_name and
then clone
+-- it to the local consumer.
+local consumer
+if is_credential_etcd_key(val.key) then
+local consumer_name =
get_consumer_name_from_credential_etcd_key(val.key)
+local the_consumer = consumers:get(consumer_name)
+if the_consumer and the_consumer.value then
+consumer = core.table.clone(the_consumer.value)
+consumer.modifiedIndex = the_consumer.modifiedIndex
+consumer.credential_id = get_credential_id_from_etcd_key(val.key)
+else
+-- Normally wouldn't get here:
+-- it should belong to a consumer for any credential.
+core.log.error("failed to get the consumer for the credential,",
+" a wild credential has appeared!",
+" credential key: ", val.key, ", consumer name: ",
consumer_name)
+return nil, "failed to get the consumer for the credential"
+end
+else
+consumer = core.table.clone(val.value)
+consumer.modifiedIndex = val.modifiedIndex
Review Comment:
we do not need this line, I think we can remove it
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
xuruidong commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1928167829
##
apisix/consumer.lua:
##
@@ -186,20 +215,33 @@ function _M.consumers()
end
-local function create_consume_cache(consumers_conf, key_attr)
+local create_consume_cache
+do
+local consumer_lrucache = core.lrucache.new({
+count = consumers_count_for_lrucache
+})
+
+local function fill_consumer_secret(consumer)
+consumer.auth_conf = secret.fetch_secrets(consumer.auth_conf, false)
Review Comment:
done
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
xuruidong commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1928138978
##
apisix/consumer.lua:
##
@@ -36,6 +36,11 @@ local lrucache = core.lrucache.new({
ttl = 300, count = 512
})
+-- Please calculate and set the value of the "consumers_count_for_lrucache"
+-- variable based on the number of consumers in the current environment,
+-- taking into account the appropriate adjustment coefficient.
+local consumers_count_for_lrucache = 4096
Review Comment:
the usage of the variable is here.
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
membphis commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1926692216
##
apisix/consumer.lua:
##
@@ -80,7 +85,53 @@ local function filter_consumers_list(data_list)
return list
end
-local function plugin_consumer()
+local plugin_consumer
+do
+local consumers_id_lrucache = core.lrucache.new({
+count = consumers_count_for_lrucache
Review Comment:
we can add some comments, tell the other developers how to use this variable
##
apisix/consumer.lua:
##
@@ -186,20 +215,33 @@ function _M.consumers()
end
-local function create_consume_cache(consumers_conf, key_attr)
+local create_consume_cache
+do
+local consumer_lrucache = core.lrucache.new({
+count = consumers_count_for_lrucache
+})
+
+local function fill_consumer_secret(consumer)
+consumer.auth_conf = secret.fetch_secrets(consumer.auth_conf, false)
Review Comment:
it is not safe to update the original `consumer` object.
more safe way:
```lua
local new_consumer = table.clone(consumer)
new_consumer.auth_conf = secret.fetch_secrets(consumer.auth_conf, false)
return new_consumer
```
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
membphis commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1926690020
##
apisix/consumer.lua:
##
@@ -101,46 +152,21 @@ local function plugin_consumer()
if not plugins[name] then
plugins[name] = {
nodes = {},
+len = 0,
Review Comment:
ok, got it
thx for your explanation
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
xuruidong commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1904094821
##
apisix/consumer.lua:
##
@@ -186,20 +215,34 @@ function _M.consumers()
end
-local function create_consume_cache(consumers_conf, key_attr)
+local create_consume_cache
+do
+local consumer_lrucache = core.lrucache.new({
+count = consumers_count_for_lrucache
+})
+
+local function fill_consumer_secret(consumer)
+local new_consumer = core.table.clone(consumer)
Review Comment:
I think deep-copy is not necessary. The consumer in the function
`fill_consumer_secret(consumer)` is created in the `construct_consumer_data`
function and is a copy of an element from the `consumers.values` list.
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
membphis commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1899406160
##
apisix/consumer.lua:
##
@@ -186,20 +215,34 @@ function _M.consumers()
end
-local function create_consume_cache(consumers_conf, key_attr)
+local create_consume_cache
+do
+local consumer_lrucache = core.lrucache.new({
+count = consumers_count_for_lrucache
+})
+
+local function fill_consumer_secret(consumer)
+local new_consumer = core.table.clone(consumer)
Review Comment:
do you think we should use `deep_clone`?
is it safer?
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
xuruidong commented on PR #11840: URL: https://github.com/apache/apisix/pull/11840#issuecomment-2565005116 ping @membphis -- 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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
xuruidong commented on PR #11840: URL: https://github.com/apache/apisix/pull/11840#issuecomment-2565004061 The code introduces a variable `consumers_count_for_lrucache`, which is used to set the size of the LRU cache. In actual use, this value needs to be configured to be greater than the actual number of consumers. Additionally, if a consumer has multiple authentication plugins, each of them will be considered as a separate consumer and cached individually in the LRU cache. When the number of consumers exceeds twice the size of the LRU cache, the process of creating cache during the execution of the `consumer.plugin` function may result in cache misses, leading to performance degradation. This happens because the LRU cache can only store consumers within a single window, and each time a consumer is updated, the order of the consumer IDs in the `consumers.values` array is likely to either remain the same or change very little. As a result, new consumer objects are continuously created, causing the old consumer cache entries to be evicted. --- 代码中引入了一个变量 consumers_count_for_lrucache , 用来设置 lrucache 的大小。实际使用的时候，这个值需要配置为大于 consumer 的实际数量，并且如果一个consumer 中存在多个认证插件，则会被认为是不同的consumer 缓存到 lrucache 中。 当 consumer 数量大于2倍的lrucache 大小的时候，在执行 `consumer.plugin` 函数创建缓存的过程中可能会一直不能命中缓存而导致性能下降。因为 lrucache 只能缓存一个窗口内的 consumer，而每次 consumer 有更新时，consumers.values 数组中的 consumer id 的顺序大概率可能不变或者变化很小，此时会一直存在生成新的 consumer object ，淘汰旧的 consumer 缓存。 -- 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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
xuruidong commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1898448452
##
apisix/consumer.lua:
##
@@ -101,46 +152,21 @@ local function plugin_consumer()
if not plugins[name] then
plugins[name] = {
nodes = {},
+len = 0,
Review Comment:
The purpose of introducing the len variable here is to reduce the redundant
computation of the table length. Refer to the `table.insert` section in this
link: https://api7.ai/learning-center/openresty/lua-table-and-metatable
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
xuruidong commented on PR #11840: URL: https://github.com/apache/apisix/pull/11840#issuecomment-2563547517 > To purge local cache need to restart apisix? yes -- 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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
xuruidong commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1898417604
##
apisix/consumer.lua:
##
@@ -186,24 +204,43 @@ function _M.consumers()
end
-local function create_consume_cache(consumers_conf, key_attr)
+local create_consume_cache
+do
+local consumers_plugin_key_lrucache_tab = {}
+
+local function create_new_consumer(consumer)
+local new_consumer = core.table.clone(consumer)
+new_consumer.auth_conf = secret.fetch_secrets(new_consumer.auth_conf,
false)
+return new_consumer
+end
+
+
+function create_consume_cache(consumers_conf, key_attr, plugin_name)
local consumer_names = {}
+local lru_cache = consumers_plugin_key_lrucache_tab[plugin_name]
+if lru_cache == nil then
+lru_cache = core.lrucache.new({
+ttl = 60 * 60 * 24, count = 20480
+})
+consumers_plugin_key_lrucache_tab[plugin_name] = lru_cache
+end
for _, consumer in ipairs(consumers_conf.nodes) do
core.log.info("consumer node: ", core.json.delay_encode(consumer))
-local new_consumer = core.table.clone(consumer)
-new_consumer.auth_conf = secret.fetch_secrets(new_consumer.auth_conf,
true,
-
new_consumer.auth_conf, "")
-consumer_names[new_consumer.auth_conf[key_attr]] = new_consumer
+local new_consumer = lru_cache(consumer.auth_conf[key_attr],
Review Comment:
Yes，I‘ll refactor this code.
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
xuruidong commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1898414968
##
apisix/consumer.lua:
##
@@ -101,10 +104,19 @@ local function plugin_consumer()
if not plugins[name] then
plugins[name] = {
nodes = {},
+len = 0,
conf_version = consumers.conf_version
}
end
+local cached_consumer = consumers_id_cache[val.value.id]
Review Comment:
The purpose of using consumer_id_cache is to avoid unnecessary table copies.
I will refactor this part of the code to replace the table with an LRU cache.
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
membphis commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1896528782
##
apisix/consumer.lua:
##
@@ -101,10 +104,19 @@ local function plugin_consumer()
if not plugins[name] then
plugins[name] = {
nodes = {},
+len = 0,
conf_version = consumers.conf_version
}
end
+local cached_consumer = consumers_id_cache[val.value.id]
+if cached_consumer and
+cached_consumer.modifiedIndex == val.modifiedIndex then
+plugins[name].len = plugins[name].len + 1
+core.table.insert(plugins[name].nodes, plugins[name].len,
+cached_consumer)
+goto CONTINUE_INTERNAL
Review Comment:
shorter is better
```suggestion
goto CONTINUE
```
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
membphis commented on code in PR #11840:
URL: https://github.com/apache/apisix/pull/11840#discussion_r1896531801
##
apisix/consumer.lua:
##
@@ -186,24 +204,43 @@ function _M.consumers()
end
-local function create_consume_cache(consumers_conf, key_attr)
+local create_consume_cache
+do
+local consumers_plugin_key_lrucache_tab = {}
+
+local function create_new_consumer(consumer)
+local new_consumer = core.table.clone(consumer)
+new_consumer.auth_conf = secret.fetch_secrets(new_consumer.auth_conf,
false)
+return new_consumer
+end
+
+
+function create_consume_cache(consumers_conf, key_attr, plugin_name)
local consumer_names = {}
+local lru_cache = consumers_plugin_key_lrucache_tab[plugin_name]
+if lru_cache == nil then
+lru_cache = core.lrucache.new({
+ttl = 60 * 60 * 24, count = 20480
+})
+consumers_plugin_key_lrucache_tab[plugin_name] = lru_cache
+end
for _, consumer in ipairs(consumers_conf.nodes) do
core.log.info("consumer node: ", core.json.delay_encode(consumer))
-local new_consumer = core.table.clone(consumer)
-new_consumer.auth_conf = secret.fetch_secrets(new_consumer.auth_conf,
true,
-
new_consumer.auth_conf, "")
-consumer_names[new_consumer.auth_conf[key_attr]] = new_consumer
+local new_consumer = lru_cache(consumer.auth_conf[key_attr],
Review Comment:
I think: this should be enough, and it seems easier
```suggestion
local new_consumer = lru_cache(consumer, nil, create_new_consumer,
consumer)
```
##
apisix/consumer.lua:
##
@@ -101,10 +104,19 @@ local function plugin_consumer()
if not plugins[name] then
plugins[name] = {
nodes = {},
+len = 0,
conf_version = consumers.conf_version
}
end
+local cached_consumer = consumers_id_cache[val.value.id]
Review Comment:
I do not know why we have to add `consumers_id_cache `
it seems useless
##
apisix/consumer.lua:
##
@@ -101,10 +104,19 @@ local function plugin_consumer()
if not plugins[name] then
plugins[name] = {
nodes = {},
+len = 0,
conf_version = consumers.conf_version
}
end
+local cached_consumer = consumers_id_cache[val.value.id]
+if cached_consumer and
+cached_consumer.modifiedIndex == val.modifiedIndex then
+plugins[name].len = plugins[name].len + 1
+core.table.insert(plugins[name].nodes, plugins[name].len,
+cached_consumer)
+goto CONTINUE_INTERNAL
Review Comment:
shorter is better
```suggestion
goto CONTINUE
```
##
apisix/consumer.lua:
##
@@ -186,24 +204,43 @@ function _M.consumers()
end
-local function create_consume_cache(consumers_conf, key_attr)
+local create_consume_cache
+do
+local consumers_plugin_key_lrucache_tab = {}
+
+local function create_new_consumer(consumer)
Review Comment:
we need a better name, it is not creating a new consumer
it should be `transform` or `fill secret`, what do you think?
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
xuruidong commented on PR #11840: URL: https://github.com/apache/apisix/pull/11840#issuecomment-2559559931 ping @membphis -- 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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
xuruidong commented on PR #11840:
URL: https://github.com/apache/apisix/pull/11840#issuecomment-2558801362
> this is the core reason whey the new way is better performance.
>
> use the cached data, reduce to recall `secret.fetch_secrets`
>
> https://private-user-images.githubusercontent.com/6814606/398028873-e0b7b9ac-1751-4ef0-a029-fc8c0321c8ac.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3MzQ5MjE1ODAsIm5iZiI6MTczNDkyMTI4MCwicGF0aCI6Ii82ODE0NjA2LzM5ODAyODg3My1lMGI3YjlhYy0xNzUxLTRlZjAtYTAyOS1mYzhjMDMyMWM4YWMucG5nP1gtQW16LUFsZ29yaXRobT1BV1M0LUhNQUMtU0hBMjU2JlgtQW16LUNyZWRlbnRpYWw9QUtJQVZDT0RZTFNBNTNQUUs0WkElMkYyMDI0MTIyMyUyRnVzLWVhc3QtMSUyRnMzJTJGYXdzNF9yZXF1ZXN0JlgtQW16LURhdGU9MjAyNDEyMjNUMDIzNDQwWiZYLUFtei1FeHBpcmVzPTMwMCZYLUFtei1TaWduYXR1cmU9NzdiZjZmMDZjM2ZmNTQ0MmQzZTViZjQ1MWUyYTEyZDQ4MjdhZWU2ZGVlYTQ3MzNjODU0YjlkMWQxNDQyMGJmYiZYLUFtei1TaWduZWRIZWFkZXJzPWhvc3QifQ.Npgprs7qCPTj08GtZeCb9pRDl3f0XISfN_0QP0HaMRk";>
> I think we can use `lrucache`, which is easier way, here is the code
example:
>
> ```lua
> local lrucache = core.lrucache.new({
> ttl = 300, count = 5000 -- we can change the `count` as we need, in
your case, we may change it to more than 17,000+
> })
>
> local new_consumer, err = lrucache(consumer, nil,
> secret.fetch_secrets, consumer.auth_conf, false)
> ```
>
> I can not 100% confirm if it works, welcome to make a test in this way,
many thx
Let me make the ci happy first, and then I'll try your suggestion.
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
membphis commented on PR #11840:
URL: https://github.com/apache/apisix/pull/11840#issuecomment-2558769233
this is the core reason whey the new way is better performance.
use the cached data, reduce to recall `secret.fetch_secrets`
https://github.com/user-attachments/assets/e0b7b9ac-1751-4ef0-a029-fc8c0321c8ac";
/>
I think we can use `lrucache`, which is easier way, here is the code
example:
```lua
local lrucache = core.lrucache.new({
ttl = 300, count = 5000 -- we can change the `count` as we need, in
your case, we may change it to more than 17,000+
})
local new_consumer, err = lrucache(consumer, nil,
secret.fetch_secrets, consumer.auth_conf, false)
```
I can not 100% confirm if it works, welcome to make a test in this way, many
thx
--
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]
Re: [PR] perf: accelerate the creation of the consumer cache [apisix]
praswicaksono commented on PR #11840: URL: https://github.com/apache/apisix/pull/11840#issuecomment-2556981987 To purge local cache need to restart apisix? -- 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]
