You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/08/31 08:31:28 UTC
[GitHub] [apisix] monkeyDluffy6017 opened a new pull request, #7826: fix: wait old nginx quit when compress in log roate
monkeyDluffy6017 opened a new pull request, #7826:
URL: https://github.com/apache/apisix/pull/7826
### Description
If we don't wait for the old nginx to exit first before compressing, we may lose the logs
Fixes # (issue)
### Checklist
- [x] I have explained the need for this PR and the problem it solves
- [x] I have explained the changes or the new features added to this PR
- [ ] I have added tests corresponding to this change
- [ ] I have updated the documentation to reflect this change
- [x] I have verified that this change is backward compatible (If not, please discuss on the [APISIX mailing list](https://github.com/apache/apisix/tree/master#community) first)
--
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
[GitHub] [apisix] tzssangglass commented on a diff in pull request #7826: fix: wait old nginx quit when compress in log roate
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7826:
URL: https://github.com/apache/apisix/pull/7826#discussion_r959429748
##########
apisix/plugins/log-rotate.lua:
##########
@@ -219,24 +247,41 @@ end
local function rotate_file(files, now_time, max_kept)
+ if isempty(files) then
+ return
+ end
+
+ local new_files = new_tab(2, 0)
+ -- rename the log files
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()
- core.log.warn("send USR1 signal to master process [", pid, "] for reopening log file")
- local ok, err = signal.kill(pid, signal.signum("USR1"))
- if not ok then
- core.log.error("failed to send USR1 signal for reopening log file: ", err)
- end
+ tab_insert(new_files, new_file)
+ end
- if enable_compression then
+ -- send signal to reopen
+ local pid = process.get_master_pid()
+ core.log.warn("send USR1 signal to master process [", pid, "] for reopening log file")
+ local ok, err = signal.kill(pid, signal.signum("USR1"))
+ if not ok then
+ core.log.error("failed to send USR1 signal for reopening log file: ", err)
+ end
+
+ if enable_compression then
+ -- to avoid losing logs during compression
+ ngx_sleep(0.5)
+ wait_old_nginx_exit()
Review Comment:
sleeping 0.5s then 10s seems strange, sleeping 10s directly?
--
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
[GitHub] [apisix] monkeyDluffy6017 closed pull request #7826: fix: wait old nginx quit when compress in log roate
Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 closed pull request #7826: fix: wait old nginx quit when compress in log roate
URL: https://github.com/apache/apisix/pull/7826
--
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
[GitHub] [apisix] tzssangglass commented on a diff in pull request #7826: fix: wait old nginx quit when compress in log roate
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7826:
URL: https://github.com/apache/apisix/pull/7826#discussion_r959433633
##########
apisix/plugins/log-rotate.lua:
##########
@@ -169,6 +171,32 @@ local function rename_file(log, date_str)
end
+local function wait_old_nginx_exit()
+ local found = true
+ local max_try = 10 -- 10 seconds
+ local prefix = ngx.config.prefix()
+ local file_path = prefix .. "/logs/nginx.pid.oldbin"
Review Comment:
Just curious, where would renamed the old `nginx.pid` be to `nginx.pid.oldbin`
--
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
[GitHub] [apisix] monkeyDluffy6017 commented on a diff in pull request #7826: fix: wait old nginx quit when compress in log roate
Posted by GitBox <gi...@apache.org>.
monkeyDluffy6017 commented on code in PR #7826:
URL: https://github.com/apache/apisix/pull/7826#discussion_r959646639
##########
apisix/plugins/log-rotate.lua:
##########
@@ -169,6 +171,32 @@ local function rename_file(log, date_str)
end
+local function wait_old_nginx_exit()
+ local found = true
+ local max_try = 10 -- 10 seconds
+ local prefix = ngx.config.prefix()
+ local file_path = prefix .. "/logs/nginx.pid.oldbin"
Review Comment:
where or why?
1. where
Before executing the new Nginx binary.
https://github.com/nginx/nginx/blob/f7ea8c76b55f730daa3b63f5511feb564b44d901/src/core/nginx.c#L717
2. why
It's for rollback, if the new Nginx binary doesn't work, we could kill the new one, and use the old Nginx instead.
--
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
[GitHub] [apisix] tzssangglass commented on a diff in pull request #7826: fix: wait old nginx quit when compress in log roate
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7826:
URL: https://github.com/apache/apisix/pull/7826#discussion_r959430640
##########
apisix/plugins/log-rotate.lua:
##########
@@ -169,6 +171,32 @@ local function rename_file(log, date_str)
end
+local function wait_old_nginx_exit()
+ local found = true
+ local max_try = 10 -- 10 seconds
+ local prefix = ngx.config.prefix()
+ local file_path = prefix .. "/logs/nginx.pid.oldbin"
+
+ for i = 1, max_try do
+ -- nginx will delete nginx.pid.oldbin after old master process quit
+ found = file_exists(file_path)
+ if found then
+ if i <= max_try then
+ ngx_sleep(1)
+ end
+
Review Comment:
delete this blank line
--
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
[GitHub] [apisix] tzssangglass commented on a diff in pull request #7826: fix: wait old nginx quit when compress in log roate
Posted by GitBox <gi...@apache.org>.
tzssangglass commented on code in PR #7826:
URL: https://github.com/apache/apisix/pull/7826#discussion_r960149519
##########
apisix/plugins/log-rotate.lua:
##########
@@ -169,6 +171,32 @@ local function rename_file(log, date_str)
end
+local function wait_old_nginx_exit()
+ local found = true
+ local max_try = 10 -- 10 seconds
+ local prefix = ngx.config.prefix()
+ local file_path = prefix .. "/logs/nginx.pid.oldbin"
Review Comment:
I get
--
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