spacewander commented on code in PR #7749:
URL: https://github.com/apache/apisix/pull/7749#discussion_r954543440


##########
apisix/plugins/log-rotate.lua:
##########
@@ -219,18 +209,67 @@ local function init_default_logs(logs_info, log_type)
 end
 
 
+local function file_size(file)
+    local attr = lfs.attributes(file)
+    if attr then
+        return attr.size
+    end
+    return 0
+end
+
+
+local function rotate_file(files, now_time, max_kept)
+    for _, file in ipairs(files) do
+        local now_date = os_date("%Y-%m-%d_%H-%M-%S", now_time)
+        local new_file = rename_file(default_logs[file], now_date)
+        if not new_file then
+            return
+        end
+
+        local pid = process.get_master_pid()
+        local sig_user1 = signal.signum("USR1")
+
+        core.log.warn("send USR1 signal to master process [", pid, "] for 
reopening log file")
+
+        if (pid and sig_user1) then

Review Comment:
   I am curious in what situation Nginx doesn't have master pid.
   And the SIGUSR1 is part of the POSIX standard. APISIX requires to be run on 
a platform that is POSIX compatible.



##########
apisix/plugins/log-rotate.lua:
##########
@@ -219,18 +209,67 @@ local function init_default_logs(logs_info, log_type)
 end
 
 
+local function file_size(file)
+    local attr = lfs.attributes(file)
+    if attr then
+        return attr.size
+    end
+    return 0
+end
+
+
+local function rotate_file(files, now_time, max_kept)
+    for _, file in ipairs(files) do
+        local now_date = os_date("%Y-%m-%d_%H-%M-%S", now_time)
+        local new_file = rename_file(default_logs[file], now_date)
+        if not new_file then
+            return
+        end
+
+        local pid = process.get_master_pid()
+        local sig_user1 = signal.signum("USR1")
+
+        core.log.warn("send USR1 signal to master process [", pid, "] for 
reopening log file")
+
+        if (pid and sig_user1) then
+            local ok, err = signal.kill(pid, sig_user1)
+            if not ok then
+                core.log.error("failed to send USR1 signal for reopening log 
file: ", err)
+            end
+        end
+
+        if enable_compression then
+            compression_file(new_file)
+        end
+
+        -- clean the oldest file
+        local log_list, log_dir = scan_log_folder(file)
+        for i = max_kept + 1, #log_list do
+            local path = log_dir .. log_list[i]
+            local ok, err = os_remove(path)
+            if err then
+               core.log.error("remove old log file: ", path, " log: ", err, "  
res:", ok)

Review Comment:
   Why change the `err:` in `core.log.error("remove old error file: ", path, " 
err: ", err, "  res:", ok)` to `log:`?



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