From: Oshry Avraham Date: Wed, 19 Feb 2025 11:02:35 +0000 (+0200) Subject: rgw/lua: Update background thread to handle config changes X-Git-Tag: v20.0.0~36^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9a1ee7a3da6479d54cfc45e2f2afc5d9eceb9770;p=ceph.git rgw/lua: Update background thread to handle config changes Ensure Lua background thread updates memory and runtime limits when config changes. - Added setters/getters for max memory and runtime limits in lua_state_guard. - Implemented dynamic updates for memory and runtime limits in the background thread. - Reset Lua state when memory limit require restart. - Added a memory leak check after Lua cleanup to ensure proper resource deallocation. Signed-off-by: Oshry Avraham --- diff --git a/doc/radosgw/lua-scripting.rst b/doc/radosgw/lua-scripting.rst index 2cad9510e1ef..344e17b88663 100644 --- a/doc/radosgw/lua-scripting.rst +++ b/doc/radosgw/lua-scripting.rst @@ -24,6 +24,10 @@ An execution of a script in a context can use up to 500K byte of memory. This in To change this default value, use the ``rgw_lua_max_memory_per_state`` configuration parameter. Note that the basic overhead of Lua with its standard libraries is ~32K bytes. To disable the limit, use zero. By default, the execution of a Lua script is limited to a maximum runtime of 1000 milliseconds. This limit can be changed using the ``rgw_lua_max_runtime_per_state`` configuration parameter. If a Lua script exceeds this runtime, it will be terminated. To disable the runtime limit, use zero. +.. warning:: Be cautious when modifying the memory limit. If the current memory usage exceeds the newly set limit, all previously stored data in the background state will be lost. + +.. warning:: Disabling the runtime limit may result in unbounded script execution, which can lead to excessive resource consumption and potentially impact the RADOS gateway's availability. + By default, all Lua standard libraries are available in the script, however, in order to allow for additional Lua modules to be used in the script, we support adding packages to an allowlist: - Adding a Lua package to the allowlist, or removing a packge from it does not install or remove it. For the changes to take affect a "reload" command should be called. diff --git a/src/rgw/rgw_lua_background.cc b/src/rgw/rgw_lua_background.cc index 9182ba45b378..0b462f9c6e3a 100644 --- a/src/rgw/rgw_lua_background.cc +++ b/src/rgw/rgw_lua_background.cc @@ -107,6 +107,30 @@ int Background::read_script() { return rgw::lua::read_script(&dp, lua_manager, tenant, null_yield, rgw::lua::context::background, rgw_script); } +std::unique_ptr Background::initialize_lguard_state() { + auto lguard = std::make_unique( + cct->_conf->rgw_lua_max_memory_per_state, + cct->_conf->rgw_lua_max_runtime_per_state, &dp); + lua_State* L = lguard->get(); + if (!L) { + ldpp_dout(&dp, 1) << "Failed to create state for Lua background thread" + << dendl; + return nullptr; + } + try { + open_standard_libs(L); + set_package_path(L, lua_manager->luarocks_path()); + create_debug_action(L, cct); + create_background_metatable(L); + } catch (const std::runtime_error& e) { + ldpp_dout(&dp, 1) + << "Failed to create initial setup of Lua background thread. error " + << e.what() << dendl; + return nullptr; + } + return lguard; +} + const BackgroundMapValue Background::empty_table_value; const BackgroundMapValue& Background::get_table_value(const std::string& key) const { @@ -124,21 +148,8 @@ const BackgroundMapValue& Background::get_table_value(const std::string& key) co void Background::run() { ceph_pthread_setname("lua_background"); const DoutPrefixProvider* const dpp = &dp; - lua_state_guard lguard(cct->_conf->rgw_lua_max_memory_per_state, - cct->_conf->rgw_lua_max_runtime_per_state, dpp); - auto L = lguard.get(); - if (!L) { - ldpp_dout(dpp, 1) << "Failed to create state for Lua background thread" << dendl; - return; - } - try { - open_standard_libs(L); - set_package_path(L, lua_manager->luarocks_path()); - create_debug_action(L, cct); - create_background_metatable(L); - } catch (const std::runtime_error& e) { - ldpp_dout(dpp, 1) << "Failed to create initial setup of Lua background thread. error " - << e.what() << dendl; + std::unique_ptr lguard = initialize_lguard_state(); + if (!lguard) { return; } @@ -153,8 +164,20 @@ void Background::run() { } ldpp_dout(dpp, 10) << "Lua background thread resumed" << dendl; } - - lguard.reset_start_time(); + const std::size_t max_memory = cct->_conf->rgw_lua_max_memory_per_state; + const std::uint64_t max_runtime = cct->_conf->rgw_lua_max_runtime_per_state; + auto res = lguard->set_max_memory(max_memory); + if (!res) { + ldpp_dout(dpp, 10) + << "Lua state required reset due to memory limit change" << dendl; + lguard = initialize_lguard_state(); + if (!lguard) { + return; + } + ldpp_dout(dpp, 10) << "Lua state restarted seccessfully." << dendl; + } + lguard->set_max_runtime(max_runtime); + lguard->reset_start_time(); const auto rc = read_script(); if (rc == -ENOENT || rc == -EAGAIN) { // either no script or paused, nothing to do @@ -162,6 +185,7 @@ void Background::run() { ldpp_dout(dpp, 1) << "WARNING: failed to read background script. error " << rc << dendl; } else { auto failed = false; + auto L = lguard->get(); try { //execute the background lua script if (luaL_dostring(L, rgw_script.c_str()) != LUA_OK) { diff --git a/src/rgw/rgw_lua_background.h b/src/rgw/rgw_lua_background.h index 2973a753fff6..f06ca491eb2b 100644 --- a/src/rgw/rgw_lua_background.h +++ b/src/rgw/rgw_lua_background.h @@ -155,8 +155,9 @@ private: std::string rgw_script; int read_script(); + std::unique_ptr initialize_lguard_state(); -public: + public: Background(rgw::sal::Driver* _driver, CephContext* _cct, rgw::sal::LuaManager* _lua_manager, diff --git a/src/rgw/rgw_lua_utils.cc b/src/rgw/rgw_lua_utils.cc index 721076a7074f..b4dbc75e4faa 100644 --- a/src/rgw/rgw_lua_utils.cc +++ b/src/rgw/rgw_lua_utils.cc @@ -93,38 +93,29 @@ void open_standard_libs(lua_State* L) { // allocator function that verifies against maximum allowed memory value void* allocator(void* ud, void* ptr, std::size_t osize, std::size_t nsize) { - auto mem = reinterpret_cast(ud); // remaining memory + auto guard = reinterpret_cast(ud); + auto mem_in_use = guard->get_mem_in_use(); + // free memory if (nsize == 0) { - if (mem && ptr) { - *mem += osize; + if (ptr) { + guard->set_mem_in_use(mem_in_use - osize); } free(ptr); return nullptr; } // re/alloc memory - if (mem) { - const std::size_t realloc_size = ptr ? osize : 0; - if (nsize > realloc_size && (nsize - realloc_size) > *mem) { - return nullptr; - } - *mem += realloc_size; - *mem -= nsize; + const std::size_t new_total = mem_in_use - (ptr ? osize : 0) + nsize; + if (guard->get_max_memory() > 0 && new_total > guard->get_max_memory()) { + return nullptr; } + guard->set_mem_in_use(new_total); return realloc(ptr, nsize); } -// create new lua state together with its memory counter -lua_State* newstate(int max_memory) { - std::size_t* remaining_memory = nullptr; - if (max_memory > 0) { - remaining_memory = new std::size_t(max_memory); - } - lua_State* L = lua_newstate(allocator, remaining_memory); - if (!L) { - delete remaining_memory; - remaining_memory = nullptr; - } +// create new lua state together with reference to the guard +lua_State* newstate(lua_state_guard* guard) { + lua_State* L = lua_newstate(allocator, guard); if (L) { lua_atpanic(L, [](lua_State* L) -> int { const char* msg = lua_tostring(L, -1); @@ -140,10 +131,11 @@ lua_state_guard::lua_state_guard(std::size_t _max_memory, std::uint64_t _max_runtime, const DoutPrefixProvider* _dpp) : max_memory(_max_memory), + mem_in_use(0), max_runtime(std::chrono::milliseconds(_max_runtime)), start_time(ceph::real_clock::now()), dpp(_dpp), - state(newstate(_max_memory)) { + state(newstate(this)) { if (state) { if (max_runtime.count() > 0) { set_runtime_hook(); @@ -160,17 +152,15 @@ lua_state_guard::~lua_state_guard() { if (!L) { return; } - void* ud = nullptr; - lua_getallocf(L, &ud); - auto remaining_memory = static_cast(ud); - - if (remaining_memory) { - const auto used_memory = max_memory - *remaining_memory; - ldpp_dout(dpp, 20) << "Lua is using: " << used_memory << - " bytes (" << 100.0*used_memory/max_memory << "%)" << dendl; - // dont limit memory during cleanup - *remaining_memory = 0; + int temp_max_memory = max_memory; + if (mem_in_use > 0) { + ldpp_dout(dpp, 20) << "Lua is using: " << mem_in_use << " bytes (" + << std::to_string(100.0 - + (100.0 * mem_in_use / max_memory)) + << "%)" << dendl; } + // dont limit memory during cleanup + max_memory = 0; // clear any runtime hooks lua_sethook(L, nullptr, 0, 0); try { @@ -179,15 +169,65 @@ lua_state_guard::~lua_state_guard() { ldpp_dout(dpp, 20) << "Lua cleanup failed with: " << e.what() << dendl; } - // TODO: use max_memory and remaining memory to check for leaks - // this could be done only if we don't zero the remianing memory during clanup + // Check for memory leaks + if (temp_max_memory > 0 && mem_in_use > 0) { + ldpp_dout(dpp, 1) << "ERROR: Lua memory leak detected: " << mem_in_use + << " bytes still in use" << dendl; + } - delete remaining_memory; if (perfcounter) { perfcounter->dec(l_rgw_lua_current_vms, 1); } } +bool lua_state_guard::set_max_memory(std::size_t _max_memory) { + if (_max_memory == max_memory) { + return true; + } + ldpp_dout(dpp, 20) << "Lua is using: " << mem_in_use << " bytes (" + << std::to_string(100.0 - + (100.0 * mem_in_use / max_memory)) + << "%)" << dendl; + + if (mem_in_use > _max_memory && _max_memory > 0) { + max_memory = _max_memory; + ldpp_dout(dpp, 10) << "Lua memory limit is below current usage" << dendl; + ldpp_dout(dpp, 20) << "Lua memory limit set to: " << max_memory << " bytes" + << dendl; + return false; + } + + max_memory = _max_memory; + ldpp_dout(dpp, 20) << "Lua memory limit set to: " + << (max_memory > 0 ? std::to_string(max_memory) : "N/A") + << " bytes" << dendl; + return true; +} + +void lua_state_guard::set_mem_in_use(std::size_t _mem_in_use) { + mem_in_use = _mem_in_use; +} + +void lua_state_guard::set_max_runtime(std::uint64_t _max_runtime) { + if (static_cast(max_runtime.count()) != _max_runtime) { + if (_max_runtime > 0) { + auto omax_runtime = max_runtime.count(); + max_runtime = std::chrono::milliseconds(_max_runtime); + if (omax_runtime == 0) { + set_runtime_hook(); + } + } else { + max_runtime = std::chrono::milliseconds(0); + lua_sethook(state, nullptr, 0, 0); + } + ldpp_dout(dpp, 20) << "Lua runtime limit set to: " + << (max_runtime.count() > 0 + ? std::to_string(max_runtime.count()) + : "N/A") + << " milliseconds" << dendl; + } +} + void lua_state_guard::runtime_hook(lua_State* L, lua_Debug* ar) { auto now = ceph::real_clock::now(); lua_getfield(L, LUA_REGISTRYINDEX, max_runtime_key); diff --git a/src/rgw/rgw_lua_utils.h b/src/rgw/rgw_lua_utils.h index 5a3ce96a9268..bb2927c9d688 100644 --- a/src/rgw/rgw_lua_utils.h +++ b/src/rgw/rgw_lua_utils.h @@ -68,8 +68,9 @@ inline void unsetglobal(lua_State* L, const char* name) void stack_dump(lua_State* L); class lua_state_guard { - const std::size_t max_memory; - const std::chrono::milliseconds max_runtime; + std::size_t max_memory; + std::size_t mem_in_use; + std::chrono::milliseconds max_runtime; ceph::real_clock::time_point start_time; const DoutPrefixProvider* const dpp; lua_State* const state; @@ -83,6 +84,13 @@ class lua_state_guard { ~lua_state_guard(); lua_State* get() { return state; } void reset_start_time() { start_time = ceph::real_clock::now(); } + + std::size_t get_max_memory() const { return max_memory; } + std::size_t get_mem_in_use() const { return mem_in_use; } + std::chrono::milliseconds get_max_runtime() const { return max_runtime; } + bool set_max_memory(std::size_t _max_memory); + void set_mem_in_use(std::size_t _mem_in_use); + void set_max_runtime(std::uint64_t _max_runtime); }; int dostring(lua_State* L, const char* str);