[GitHub] [apisix] tokers commented on a change in pull request #6202: feat: use keepalive in server-info plugin

[GitBox <gi...@apache.org>]

tokers commented on a change in pull request #6202:
URL: https://github.com/apache/apisix/pull/6202#discussion_r804280358



##########
File path: apisix/plugins/server-info.lua
##########
@@ -87,36 +80,35 @@ local function get_boot_time()
 end
 
 
-local function uninitialized_server_info()
+local function uninitialize_server_info()

Review comment:
       Rename this function is meaningless to me.

##########
File path: apisix/plugins/server-info.lua
##########
@@ -158,25 +150,70 @@ local function report(premature, report_ttl)
         end
     end
 
-    server_info.last_report_time = ngx_time()
-
+    -- get inside etcd data, if not exist, create it
     local key = "/data_plane/server_info/" .. server_info.id
-    local ok, err = core.etcd.set(key, server_info, report_ttl)
+    local res, err = core.etcd.get(key)
+    if err ~= nil then
+        core.log.error("failed to get server_info from etcd: ", err)
+    end
+
+    if  not res.body.node then
+        local newres, err = core.etcd.set(key, server_info, report_ttl)

Review comment:
       Also the `key` for individual instances should be unique but should we use the `atomic_set` method?

##########
File path: apisix/plugins/server-info.lua
##########
@@ -87,36 +80,35 @@ local function get_boot_time()
 end
 
 
-local function uninitialized_server_info()
+local function uninitialize_server_info()
     local boot_time = get_boot_time()
     return {
         etcd_version     = "unknown",
         hostname         = core.utils.gethostname(),
         id               = core.id.get(),
         version          = core.version.VERSION,
-        up_time          = ngx_time() - boot_time,
         boot_time        = boot_time,
-        last_report_time = -1,

Review comment:
       Note we should also update the manager api or the server-info report cannot be shown on the dashboard.

##########
File path: apisix/plugins/server-info.lua
##########
@@ -158,25 +150,70 @@ local function report(premature, report_ttl)
         end
     end
 
-    server_info.last_report_time = ngx_time()
-
+    -- get inside etcd data, if not exist, create it
     local key = "/data_plane/server_info/" .. server_info.id
-    local ok, err = core.etcd.set(key, server_info, report_ttl)
+    local res, err = core.etcd.get(key)
+    if err ~= nil then
+        core.log.error("failed to get server_info from etcd: ", err)
+    end
+
+    if  not res.body.node then
+        local newres, err = core.etcd.set(key, server_info, report_ttl)
+        if not newres then
+            core.log.error("failed to set server_info: ", err)
+            return
+        end
+
+        -- set lease_id to ngx dict
+        local _, err = internal_status:set("lease_id", newres.body.lease_id)
+        if err ~= nil then
+            core.log.error("failed to save boot_time to shdict: ", err)
+        end
+
+        return
+    end
+
+    local ok = core.table.deep_eq(server_info, res.body.node.value)
+    -- not equal, update it
     if not ok then
-        core.log.error("failed to report server info to etcd: ", err)
+        local newres, err = core.etcd.set(key, server_info, report_ttl)
+        if not newres then
+            core.log.error("failed to set server_info to etcd: ", err)
+            return false, err
+        end
+
+        -- update lease_id
+        local _, err = internal_status:set("lease_id", res.body.lease_id)
+        if err ~= nil then
+            core.log.error("failed to set lease_id to shdict: ", err)
+        end

Review comment:
       This paragraph is same to the one in the `if  not res.body.node then .... end` block, we can create a function to reuse it.

##########
File path: apisix/plugins/server-info.lua
##########
@@ -158,25 +150,70 @@ local function report(premature, report_ttl)
         end
     end
 
-    server_info.last_report_time = ngx_time()
-
+    -- get inside etcd data, if not exist, create it
     local key = "/data_plane/server_info/" .. server_info.id
-    local ok, err = core.etcd.set(key, server_info, report_ttl)
+    local res, err = core.etcd.get(key)
+    if err ~= nil then
+        core.log.error("failed to get server_info from etcd: ", err)
+    end
+
+    if  not res.body.node then
+        local newres, err = core.etcd.set(key, server_info, report_ttl)
+        if not newres then
+            core.log.error("failed to set server_info: ", err)
+            return
+        end
+
+        -- set lease_id to ngx dict
+        local _, err = internal_status:set("lease_id", newres.body.lease_id)
+        if err ~= nil then
+            core.log.error("failed to save boot_time to shdict: ", err)

Review comment:
       And the error log is not accurate.




-- 
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: notifications-unsubscribe@apisix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org