From 4826424a8c209c501b78202e38bf53ab60144d39 Mon Sep 17 00:00:00 2001 From: spacewander Date: Wed, 29 Jul 2020 09:06:15 +0800 Subject: [PATCH 1/2] feature: allow ngx.sleep to be used blockingly in non-yieldable phases Allow ngx.sleep everywhere simplify the application's logic. Now we don't need to write a fallback if the same function need to be run in non-yieldable phases. Close #1730. --- README.markdown | 7 ++-- doc/HttpLuaModule.wiki | 7 ++-- src/ngx_http_lua_sleep.c | 23 ++++++++---- t/077-sleep.t | 76 +++++++++++++++++++++++++++++++++++---- t/138-balancer.t | 12 +++---- t/142-ssl-session-store.t | 7 ++-- 6 files changed, 104 insertions(+), 28 deletions(-) diff --git a/README.markdown b/README.markdown index bd29909b53..a446f62d6f 100644 --- a/README.markdown +++ b/README.markdown @@ -5621,14 +5621,17 @@ ngx.sleep **syntax:** *ngx.sleep(seconds)* -**context:** *rewrite_by_lua*, access_by_lua*, content_by_lua*, ngx.timer.*, ssl_certificate_by_lua*, ssl_session_fetch_by_lua** +**context:** *init_by_lua*, init_worker_by_lua*, set_by_lua*, rewrite_by_lua*, access_by_lua*, content_by_lua*, header_filter_by_lua*, body_filter_by_lua*, log_by_lua*, ngx.timer.*, balancer_by_lua*, ssl_certificate_by_lua*, ssl_session_fetch_by_lua*, ssl_session_store_by_lua*, exit_worker_by_lua** -Sleeps for the specified seconds without blocking. One can specify time resolution up to 0.001 seconds (i.e., one millisecond). +Sleeps for the specified seconds without blocking in yieldable phases or blockingly in other phases. +One can specify time resolution up to 0.001 seconds (i.e., one millisecond). Behind the scene, this method makes use of the Nginx timers. Since the `0.7.20` release, The `0` time argument can also be specified. +Since the `FIXME` release, this method can be used in non-yieldable phases blockingly. + This method was introduced in the `0.5.0rc30` release. [Back to TOC](#nginx-api-for-lua) diff --git a/doc/HttpLuaModule.wiki b/doc/HttpLuaModule.wiki index 04a2694972..2fbffc2c19 100644 --- a/doc/HttpLuaModule.wiki +++ b/doc/HttpLuaModule.wiki @@ -4720,14 +4720,17 @@ Since v0.8.3 this function returns 1 on success, or re '''syntax:''' ''ngx.sleep(seconds)'' -'''context:''' ''rewrite_by_lua*, access_by_lua*, content_by_lua*, ngx.timer.*, ssl_certificate_by_lua*, ssl_session_fetch_by_lua*'' +'''context:''' ''init_by_lua*, init_worker_by_lua*, set_by_lua*, rewrite_by_lua*, access_by_lua*, content_by_lua*, header_filter_by_lua*, body_filter_by_lua*, log_by_lua*, ngx.timer.*, balancer_by_lua*, ssl_certificate_by_lua*, ssl_session_fetch_by_lua*, ssl_session_store_by_lua*, exit_worker_by_lua*'' -Sleeps for the specified seconds without blocking. One can specify time resolution up to 0.001 seconds (i.e., one millisecond). +Sleeps for the specified seconds without blocking in yieldable phases or blockingly in other phases. +One can specify time resolution up to 0.001 seconds (i.e., one millisecond). Behind the scene, this method makes use of the Nginx timers. Since the 0.7.20 release, The 0 time argument can also be specified. +Since the FIXME release, this method can be used in non-yieldable phases blockingly. + This method was introduced in the 0.5.0rc30 release. == ngx.escape_uri == diff --git a/src/ngx_http_lua_sleep.c b/src/ngx_http_lua_sleep.c index b8601fee95..6fbbecc6d8 100644 --- a/src/ngx_http_lua_sleep.c +++ b/src/ngx_http_lua_sleep.c @@ -36,23 +36,34 @@ ngx_http_lua_ngx_sleep(lua_State *L) return luaL_error(L, "attempt to pass %d arguments, but accepted 1", n); } - r = ngx_http_lua_get_req(L); - if (r == NULL) { - return luaL_error(L, "no request found"); - } - delay = (ngx_int_t) (luaL_checknumber(L, 1) * 1000); if (delay < 0) { return luaL_error(L, "invalid sleep duration \"%d\"", delay); } + r = ngx_http_lua_get_req(L); + if (r == NULL) { + /* init_by_lua phase */ + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, ngx_cycle->log, 0, + "lua ready to sleep for %d ms", delay); + + ngx_msleep(delay); + return 0; + } + ctx = ngx_http_get_module_ctx(r, ngx_http_lua_module); if (ctx == NULL) { return luaL_error(L, "no request ctx found"); } - ngx_http_lua_check_context(L, ctx, NGX_HTTP_LUA_CONTEXT_YIELDABLE); + if (!(ctx->context & NGX_HTTP_LUA_CONTEXT_YIELDABLE)) { + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "lua ready to sleep for %d ms", delay); + + ngx_msleep(delay); + return 0; + } coctx = ctx->cur_co_ctx; if (coctx == NULL) { diff --git a/t/077-sleep.t b/t/077-sleep.t index 96f04bd98e..5518b09406 100644 --- a/t/077-sleep.t +++ b/t/077-sleep.t @@ -9,7 +9,7 @@ log_level('debug'); repeat_each(2); -plan tests => repeat_each() * 71; +plan tests => repeat_each() * (blocks() * 4); #no_diff(); no_long_string(); @@ -237,21 +237,20 @@ lua sleep timer expired: "/test?" -=== TEST 10: ngx.sleep unavailable in log_by_lua +=== TEST 10: ngx.sleep available in log_by_lua --- config location /t { echo hello; - log_by_lua ' - ngx.sleep(0.1) - '; + log_by_lua_block { + ngx.sleep(0.001) + } } --- request GET /t --- response_body hello ---- wait: 0.1 --- error_log -API disabled in the context of log_by_lua* +lua ready to sleep for 1 ms @@ -500,3 +499,66 @@ f end worker cycle e?poll timer: 0 / + + + +=== TEST 18: ngx.sleep(0) in no-yieldable phases +--- config + location /t { + echo hello; + log_by_lua_block { + ngx.sleep(0) + } + } +--- request +GET /t +--- response_body +hello +--- error_log +lua ready to sleep for 0 ms + + + +=== TEST 19: ngx.sleep available in init_worker_by_lua +--- http_config + init_worker_by_lua_block { + local start = ngx.now() + ngx.sleep(0.1) + ngx.update_time() + package.loaded.gap = ngx.now() - start + } +--- config + location /t { + content_by_lua_block { + ngx.say(package.loaded.gap >= 0.1) + } + } +--- request +GET /t +--- no_error_log +[error] +--- response_body +true + + + +=== TEST 20: ngx.sleep available in init_by_lua +--- http_config + init_by_lua_block { + local start = ngx.now() + ngx.sleep(0.1) + ngx.update_time() + package.loaded.gap = ngx.now() - start + } +--- config + location /t { + content_by_lua_block { + ngx.say(package.loaded.gap >= 0.1) + } + } +--- request +GET /t +--- no_error_log +[error] +--- response_body +true diff --git a/t/138-balancer.t b/t/138-balancer.t index 151db7be44..14afa7d293 100644 --- a/t/138-balancer.t +++ b/t/138-balancer.t @@ -9,7 +9,7 @@ use Test::Nginx::Socket::Lua; repeat_each(2); -plan tests => repeat_each() * (blocks() * 4 + 9); +plan tests => repeat_each() * (blocks() * 4 + 8); #no_diff(); no_long_string(); @@ -258,12 +258,12 @@ qr/\[error\] .*? failed to run balancer_by_lua\*: balancer_by_lua:2: API disable -=== TEST 10: ngx.sleep is disabled +=== TEST 10: ngx.sleep is allowed --- http_config upstream backend { server 0.0.0.1; balancer_by_lua_block { - ngx.sleep(0.1) + ngx.sleep(0.001) } } --- config @@ -272,10 +272,8 @@ qr/\[error\] .*? failed to run balancer_by_lua\*: balancer_by_lua:2: API disable } --- request GET /t ---- response_body_like: 500 Internal Server Error ---- error_code: 500 ---- error_log eval -qr/\[error\] .*? failed to run balancer_by_lua\*: balancer_by_lua:2: API disabled in the context of balancer_by_lua\*/ +--- response_body_like: 502 Bad Gateway +--- error_code: 502 diff --git a/t/142-ssl-session-store.t b/t/142-ssl-session-store.t index 14e90574b0..ddc0b09c36 100644 --- a/t/142-ssl-session-store.t +++ b/t/142-ssl-session-store.t @@ -6,7 +6,7 @@ use File::Basename; repeat_each(3); -plan tests => repeat_each() * (blocks() * 6 - 1); +plan tests => repeat_each() * (blocks() * 5 + 11); $ENV{TEST_NGINX_HTML_DIR} ||= html_dir(); @@ -95,11 +95,11 @@ ssl_session_store_by_lua_block:1: ssl session store by lua is running!, -=== TEST 2: sleep is not allowed +=== TEST 2: sleep is allowed --- http_config ssl_session_store_by_lua_block { local begin = ngx.now() - ngx.sleep(0.1) + ngx.sleep(0.001) print("elapsed in ssl store session by lua: ", ngx.now() - begin) } server { @@ -157,7 +157,6 @@ close: 1 nil --- error_log lua ssl server name: "test.com" -API disabled in the context of ssl_session_store_by_lua* --- no_error_log [alert] From 40a667e462547bd0cd2c2528f33c29ec863e3bb5 Mon Sep 17 00:00:00 2001 From: spacewander Date: Wed, 29 Jul 2020 17:44:03 +0800 Subject: [PATCH 2/2] update format of error log --- src/ngx_http_lua_sleep.c | 9 +++++---- t/077-sleep.t | 4 ++-- 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/ngx_http_lua_sleep.c b/src/ngx_http_lua_sleep.c index 6fbbecc6d8..b638504dd3 100644 --- a/src/ngx_http_lua_sleep.c +++ b/src/ngx_http_lua_sleep.c @@ -45,8 +45,8 @@ ngx_http_lua_ngx_sleep(lua_State *L) r = ngx_http_lua_get_req(L); if (r == NULL) { /* init_by_lua phase */ - ngx_log_debug1(NGX_LOG_DEBUG_HTTP, ngx_cycle->log, 0, - "lua ready to sleep for %d ms", delay); + ngx_log_error(NGX_LOG_WARN, ngx_cycle->log, 0, + "lua sleep blockingly for %d ms in init_by_lua*", delay); ngx_msleep(delay); return 0; @@ -58,8 +58,9 @@ ngx_http_lua_ngx_sleep(lua_State *L) } if (!(ctx->context & NGX_HTTP_LUA_CONTEXT_YIELDABLE)) { - ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "lua ready to sleep for %d ms", delay); + ngx_log_error(NGX_LOG_WARN, r->connection->log, 0, + "lua sleep blockingly for %d ms in %s", + delay, ngx_http_lua_context_name(ctx->context)); ngx_msleep(delay); return 0; diff --git a/t/077-sleep.t b/t/077-sleep.t index 5518b09406..89239ed5a4 100644 --- a/t/077-sleep.t +++ b/t/077-sleep.t @@ -250,7 +250,7 @@ GET /t --- response_body hello --- error_log -lua ready to sleep for 1 ms +lua sleep blockingly for 1 ms in log_by_lua* @@ -515,7 +515,7 @@ GET /t --- response_body hello --- error_log -lua ready to sleep for 0 ms +lua sleep blockingly for 0 ms in log_by_lua*