Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature(core): implement core.sleep #2397

Merged
merged 9 commits into from
Oct 19, 2020
Merged

Conversation

dabue
Copy link
Contributor

@dabue dabue commented Oct 12, 2020

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

Copy link
Member

@membphis membphis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, except for minor code style

apisix/core/utils.lua Outdated Show resolved Hide resolved
apisix/core/utils.lua Outdated Show resolved Hide resolved
apisix/core/utils.lua Outdated Show resolved Hide resolved
@moonming
Copy link
Member

Is there a long sleep in the project?

@moonming
Copy link
Member

This change seems to be of little use to the current code

@dabue
Copy link
Contributor Author

dabue commented Oct 13, 2020

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.

@membphis
Copy link
Member

membphis commented Oct 17, 2020

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.

@moonming moonming merged commit 943be09 into apache:master Oct 19, 2020
@membphis
Copy link
Member

@dabue many thx

@moonming moonming added this to the 2.0 milestone Oct 19, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feature(core): implement core.sleep
4 participants