nic-6443 commented on code in PR #12773:
URL: https://github.com/apache/apisix/pull/12773#discussion_r2601347098
##########
apisix/discovery/consul/init.lua:
##########
@@ -61,25 +65,62 @@ local watch_type_catalog = 1
local watch_type_health = 2
local max_retry_time = 256
-local _M = {
- version = 0.3,
-}
+local function persist_all_services_to_shm()
+ if not consul_dict then
+ return
+ end
+ local data, err = core.json.encode(all_services)
+ if not data then
+ log.error("failed to encode consul services for shared dict: ", err)
+ return
+ end
-local function discovery_consul_callback(data, event, source, pid)
- all_services = data
- log.notice("update local variable all_services, event is: ", event,
- "source: ", source, "server pid:", pid,
- ", all services: ", json_delay_encode(all_services, true))
+ local ok, set_err = consul_dict:set(consul_dict_services_key, data)
+ if not ok then
+ log.error("failed to store consul services in shared dict: ", set_err)
+ return
+ end
end
+local function sync_all_services_from_shm(force_log)
+ if not consul_dict then
+ return
+ end
+
+ local data = consul_dict:get(consul_dict_services_key)
+ if not data then
+ if force_log then
+ log.info("consul shared dict services empty")
+ end
+ return
+ end
+
+ local decoded, err = core.json.decode(data)
+ if not decoded then
+ log.error("failed to decode consul services from shared dict: ", err)
+ return
+ end
+
+ all_services = decoded
+end
Review Comment:
Both functions writing to and reading from shared memory through
`all_services` seem very dangerous. It may lead to re-writing the data just
read from shared memory instead of getting the latest data from consul due to
unexpected execution order. It is recommended to remove the use of the
`all_services` variable, simplifying it so that only a privileged agent starts
a timer to periodically fetch data from consul, while other workers only load
data from shared memory. Consider implementing nacos.
##########
apisix/discovery/consul/init.lua:
##########
@@ -61,25 +65,62 @@ local watch_type_catalog = 1
local watch_type_health = 2
local max_retry_time = 256
-local _M = {
- version = 0.3,
-}
+local function persist_all_services_to_shm()
+ if not consul_dict then
+ return
+ end
+ local data, err = core.json.encode(all_services)
+ if not data then
+ log.error("failed to encode consul services for shared dict: ", err)
+ return
+ end
-local function discovery_consul_callback(data, event, source, pid)
- all_services = data
- log.notice("update local variable all_services, event is: ", event,
- "source: ", source, "server pid:", pid,
- ", all services: ", json_delay_encode(all_services, true))
+ local ok, set_err = consul_dict:set(consul_dict_services_key, data)
+ if not ok then
+ log.error("failed to store consul services in shared dict: ", set_err)
+ return
+ end
end
+local function sync_all_services_from_shm(force_log)
+ if not consul_dict then
+ return
+ end
+
+ local data = consul_dict:get(consul_dict_services_key)
+ if not data then
+ if force_log then
+ log.info("consul shared dict services empty")
+ end
+ return
+ end
+
+ local decoded, err = core.json.decode(data)
+ if not decoded then
+ log.error("failed to decode consul services from shared dict: ", err)
+ return
+ end
+
+ all_services = decoded
+end
Review Comment:
Both functions writing to and reading from shared memory through
`all_services` seem very dangerous. It may lead to re-writing the data just
read from shared memory instead of getting the latest data from consul due to
unexpected execution order. It is recommended to remove the use of the
`all_services` variable, simplifying it so that only a privileged agent starts
a timer to periodically fetch data from consul, while other workers only load
data from shared memory.
--
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]
