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 2020/10/12 08:48:18 UTC
[GitHub] [apisix] dabue opened a new pull request #2397: feature(core): implement `core.sleep`
dabue opened a new pull request #2397:
URL: https://github.com/apache/apisix/pull/2397
fix #2170
Use a small time interval (such as 1s) to complete a relatively long sleep behavior, so that the worker process can be shut down gracefully.
···
local max_sleep_interval = 1
local function sleep(sec)
if sec <= max_sleep_interval then
ngx_sleep(sec)
return
end
ngx_sleep(max_sleep_interval)
if exiting() then
return
end
sec = sec - max_sleep_interval
sleep(sec)
end
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on a change in pull request #2397: feature(core): implement `core.sleep`
Posted by GitBox <gi...@apache.org>.
membphis commented on a change in pull request #2397:
URL: https://github.com/apache/apisix/pull/2397#discussion_r503366943
##########
File path: apisix/core/utils.lua
##########
@@ -194,4 +196,22 @@ function _M.validate_header_value(value)
end
+local function sleep(sec)
+ if sec <= max_sleep_interval then
+ ngx_sleep(sec)
+ return
+ end
+ ngx_sleep(max_sleep_interval)
+ if exiting() then
+ return
+ end
+ sec = sec - max_sleep_interval
+ sleep(sec)
+end
+
+
+function _M.sleep(sec)
Review comment:
`_M.sleep = sleep` is enough
##########
File path: apisix/core/utils.lua
##########
@@ -194,4 +196,22 @@ function _M.validate_header_value(value)
end
+local function sleep(sec)
+ if sec <= max_sleep_interval then
+ ngx_sleep(sec)
+ return
Review comment:
`return ngx_sleep(sec)`
use proper tail recursion is good here
##########
File path: apisix/core/utils.lua
##########
@@ -194,4 +196,22 @@ function _M.validate_header_value(value)
end
+local function sleep(sec)
+ if sec <= max_sleep_interval then
+ ngx_sleep(sec)
+ return
+ end
+ ngx_sleep(max_sleep_interval)
+ if exiting() then
+ return
+ end
+ sec = sec - max_sleep_interval
+ sleep(sec)
Review comment:
`return sleep(sec)` ditto
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2397: feature(core): implement `core.sleep`
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2397:
URL: https://github.com/apache/apisix/pull/2397#issuecomment-707181445
> local function sleep(sec)
> if sec <= max_sleep_interval then
> ngx_sleep(sec)
> return
> end
> ngx_sleep(max_sleep_interval)
> if exiting() then
> return
> end
> sec = sec - max_sleep_interval
> sleep(sec)
> end
missing`while true do ... end`, so your current way is wrong @dabue
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] dabue commented on pull request #2397: feature(core): implement `core.sleep`
Posted by GitBox <gi...@apache.org>.
dabue commented on pull request #2397:
URL: https://github.com/apache/apisix/pull/2397#issuecomment-707527661
> This change seems to be of little use to the current code
Among the current code, it really only work better when the worker process being shut down at the time of triggering request limiting and delaying processing. And we can keep it as a better way to sleep for the future usage scenarios.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on pull request #2397: feature(core): implement `core.sleep`
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2397:
URL: https://github.com/apache/apisix/pull/2397#issuecomment-707451754
Is there a long sleep in the project?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] dabue commented on a change in pull request #2397: feature(core): implement `core.sleep`
Posted by GitBox <gi...@apache.org>.
dabue commented on a change in pull request #2397:
URL: https://github.com/apache/apisix/pull/2397#discussion_r503581194
##########
File path: apisix/core/utils.lua
##########
@@ -194,4 +196,22 @@ function _M.validate_header_value(value)
end
+local function sleep(sec)
+ if sec <= max_sleep_interval then
+ ngx_sleep(sec)
+ return
+ end
+ ngx_sleep(max_sleep_interval)
+ if exiting() then
+ return
+ end
+ sec = sec - max_sleep_interval
+ sleep(sec)
+end
+
+
+function _M.sleep(sec)
Review comment:
I‘ve updated 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming merged pull request #2397: feature(core): implement `core.sleep`
Posted by GitBox <gi...@apache.org>.
moonming merged pull request #2397:
URL: https://github.com/apache/apisix/pull/2397
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] moonming commented on pull request #2397: feature(core): implement `core.sleep`
Posted by GitBox <gi...@apache.org>.
moonming commented on pull request #2397:
URL: https://github.com/apache/apisix/pull/2397#issuecomment-707452129
This change seems to be of little use to the current 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis edited a comment on pull request #2397: feature(core): implement `core.sleep`
Posted by GitBox <gi...@apache.org>.
membphis edited a comment on pull request #2397:
URL: https://github.com/apache/apisix/pull/2397#issuecomment-710993762
I think this feature is useful.
When the APISIX instance restarts, we need to exit `sleep` as soon as possible.
Then the Nginx process can be gracefully shut down immediately, and free the system resources asap.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis removed a comment on pull request #2397: feature(core): implement `core.sleep`
Posted by GitBox <gi...@apache.org>.
membphis removed a comment on pull request #2397:
URL: https://github.com/apache/apisix/pull/2397#issuecomment-707181445
> local function sleep(sec)
> if sec <= max_sleep_interval then
> ngx_sleep(sec)
> return
> end
> ngx_sleep(max_sleep_interval)
> if exiting() then
> return
> end
> sec = sec - max_sleep_interval
> sleep(sec)
> end
missing`while true do ... end`, so your current way is wrong @dabue
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2397: feature(core): implement `core.sleep`
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2397:
URL: https://github.com/apache/apisix/pull/2397#issuecomment-710993762
I think this feature is useful.
When the APISIX instance restarts, we need to exit `sleep` as soon as possible.
Then the Nginx process can be gracefully shut down.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] membphis commented on pull request #2397: feature(core): implement `core.sleep`
Posted by GitBox <gi...@apache.org>.
membphis commented on pull request #2397:
URL: https://github.com/apache/apisix/pull/2397#issuecomment-711603835
@dabue 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] dabue edited a comment on pull request #2397: feature(core): implement `core.sleep`
Posted by GitBox <gi...@apache.org>.
dabue edited a comment on pull request #2397:
URL: https://github.com/apache/apisix/pull/2397#issuecomment-707527661
> This change seems to be of little use to the current code
Among the current code, it really only work better when the worker process being shut down at the time of triggering request limiting and delaying processing. And we can keep it as a better way to sleep for the future usage scenarios.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org