]> git-server-git.apps.pok.os.sepia.ceph.com Git - ceph.git/commitdiff
rgw/lua: create fresh VM for each background script execution 67396/head
authorRotem Shapira <rotem.rs@gmail.com>
Wed, 18 Feb 2026 13:51:45 +0000 (13:51 +0000)
committerRotem Shapira <rotem.rs@gmail.com>
Tue, 10 Mar 2026 19:14:47 +0000 (19:14 +0000)
Previously, the background thread reused the same Lua VM across
iterations, causing stale state to persist. This made operations
like 'pairs(RGW)' fail to iterate properly.

Now we create a fresh VM on each iteration, which:
- Fixes the iteration bug
- Simplifies the code (no need to update limits on existing VM)
- Ensures clean state for each script execution

Verified with unit tests:
- TableIterateBackground
- TableIterateBackgroundBreak
- TableIterateStepByStep

Fixes: https://tracker.ceph.com/issues/74839
Signed-off-by: Rotem Shapira <rotem.rs@gmail.com>
src/rgw/rgw_lua_background.cc
src/rgw/rgw_lua_utils.cc
src/rgw/rgw_lua_utils.h
src/test/rgw/test_rgw_lua.cc

index 0d0f34c91b6be985e08db68dacf5abb5c3a061f4..bd01fc35d96c4b58310e333c05c1ae140e20fbef 100644 (file)
@@ -156,10 +156,6 @@ const BackgroundMapValue& Background::get_table_value(const std::string& key) co
 void Background::run() {
   ceph_pthread_setname("lua_background");
   const DoutPrefixProvider* const dpp = &dp;
-  std::unique_ptr<lua_state_guard> lguard = initialize_lguard_state();
-  if (!lguard) {
-    return;
-  }
 
   while (!stopped) {
     if (paused) {
@@ -172,20 +168,10 @@ void Background::run() {
       }
       ldpp_dout(dpp, 10) << "Lua background thread resumed" << dendl;
     }
-    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 successfully." << dendl;
+    std::unique_ptr<lua_state_guard> lguard = initialize_lguard_state();
+    if (!lguard) {
+      return;
     }
-    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
index 4abed330a9f1fea09730cf7c04279cbd1995e0ed..0b73e6aa14a69eb757283f6c30c0d9b6fddebe4a 100644 (file)
@@ -182,54 +182,10 @@ lua_state_guard::~lua_state_guard() {
   }
 }
 
-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<uint64_t>(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);
index 2a8efe39e38f90bc0c5787cf010e4f43cc00a931..9262ea7e0a200d8b4d231498ca6089962b948dff 100644 (file)
@@ -84,14 +84,10 @@ class lua_state_guard {
                   const DoutPrefixProvider* _dpp);
   ~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);
index 5160acc786cac138100ab165c5b748a0005dc226..f5ce8be2bf8bec22d937315c380add3745c66fcb 100644 (file)
@@ -1447,6 +1447,146 @@ TEST(TestRGWLuaBackground, TableIncrementError)
   ASSERT_NE(rc, 0);
 }
 
+// Regular background iteration test
+TEST(TestRGWLuaBackground, TableIterateBackground)
+{
+  DEFINE_REQ_STATE;
+  
+  // Script counts elements in RGW table, excluding its own "count" key
+  const std::string background_script = R"(
+    local count = 0
+    for k, v in pairs(RGW) do
+      if tostring(k) ~= "count" then
+        count = count + 1
+      end
+    end
+    RGW["count"] = count
+  )";
+
+  // Use the helper function to set the background script
+  set_script(pe.lua.manager.get(), background_script);
+  
+  TestBackground lua_background(pe.lua.manager.get());
+  pe.lua.background = &lua_background;
+  lua_background.start();
+
+  // Wait 1s to ensure background thread is initialized before injecting data
+  std::this_thread::sleep_for(std::chrono::seconds(1));
+
+  // Inject multiple data types via request context to populate the shared RGW table
+  const std::string request_script = R"(
+    RGW["key1"] = "string_value"
+    RGW["key2"] = 100
+    RGW["key3"] = false
+  )";
+
+  // Inject data into the shared table via a request script
+  const auto rc = lua::request::execute(nullptr, nullptr, &s, nullptr, request_script);
+  ASSERT_EQ(rc, 0);
+
+  // Wait 6s to allow at least one full background iteration (interval is 5s)
+  // This verifies that each new VM instance can correctly iterate over the shared RGW table.
+  std::this_thread::sleep_for(std::chrono::seconds(6));
+
+  // Verify that all data types were preserved and correctly counted
+  EXPECT_EQ(get_table_value<std::string>(lua_background, "key1"), "string_value");
+  EXPECT_EQ(get_table_value<long long int>(lua_background, "key2"), 100);
+  EXPECT_FALSE(get_table_value<bool>(lua_background, "key3"));
+  EXPECT_EQ(get_table_value<long long int>(lua_background, "count"), 3);
+
+  lua_background.shutdown();
+}
+
+// Edge case: Test early exit from iteration using 'break'
+TEST(TestRGWLuaBackground, TableIterateBackgroundBreak)
+{
+  DEFINE_REQ_STATE;
+  
+  // Script stops counting after reaching 2 elements to test if partial iteration works
+  const std::string background_script = R"(
+    local count = 0
+    for k, v in pairs(RGW) do
+      if tostring(k) ~= "count" then
+        count = count + 1
+      end
+      if count == 2 then break end 
+    end
+    RGW["count"] = count
+  )";
+
+  // Use the helper function to set the background script
+  set_script(pe.lua.manager.get(), background_script);
+  
+  TestBackground lua_background(pe.lua.manager.get());
+  pe.lua.background = &lua_background;
+  lua_background.start();
+
+  // Wait 1s to ensure background thread is initialized before injecting data
+  std::this_thread::sleep_for(std::chrono::seconds(1));
+
+  // Inject 3 items, but the script above should only count 2
+  const std::string request_script = R"(
+    RGW["key1"] = "string_value"
+    RGW["key2"] = 100
+    RGW["key3"] = false
+  )";
+
+  ASSERT_EQ(lua::request::execute(nullptr, nullptr, &s, nullptr, request_script), 0);
+
+  // Wait 6s to allow at least one full background iteration (interval is 5s)
+  std::this_thread::sleep_for(std::chrono::seconds(6));
+
+  EXPECT_EQ(get_table_value<std::string>(lua_background, "key1"), "string_value");
+  EXPECT_EQ(get_table_value<long long int>(lua_background, "key2"), 100);
+  // Even though 3 items exist, count should be 2 due to the 'break' in Lua
+  EXPECT_EQ(get_table_value<long long int>(lua_background, "count"), 2);
+
+  lua_background.shutdown();
+}
+
+// Incremental test: Verifies that the background thread detects table changes over time
+TEST(TestRGWLuaBackground, TableIterateStepByStep)
+{
+  DEFINE_REQ_STATE;
+  
+  // Background script: Iterates over RGW table and counts elements, excluding the "count" key
+  const std::string background_script = R"(
+    local count = 0
+    for k, v in pairs(RGW) do
+      if tostring(k) ~= "count" then
+        count = count + 1
+      end
+    end
+    RGW["count"] = count
+  )";
+
+  // Use the helper function to set the background script
+  set_script(pe.lua.manager.get(), background_script);
+  
+  TestBackground lua_background(pe.lua.manager.get());
+  pe.lua.background = &lua_background;
+  lua_background.start();
+
+  // --- Step 1: Initial state (Empty table) ---
+  // Wait 1s to ensure background thread is initialized before injecting data
+  std::this_thread::sleep_for(std::chrono::seconds(1));
+  EXPECT_EQ(get_table_value<long long int>(lua_background, "count"), 0);
+
+  // --- Step 2: Add first item ---
+  ASSERT_EQ(lua::request::execute(nullptr, nullptr, &s, nullptr, "RGW['key1'] = 'val1'"), 0);
+  std::this_thread::sleep_for(std::chrono::seconds(6)); 
+  EXPECT_EQ(get_table_value<std::string>(lua_background, "key1"), "val1");
+  EXPECT_EQ(get_table_value<long long int>(lua_background, "count"), 1);
+
+  // --- Step 3: Add second item ---
+  ASSERT_EQ(lua::request::execute(nullptr, nullptr, &s, nullptr, "RGW['key2'] = 42"), 0);
+  std::this_thread::sleep_for(std::chrono::seconds(6));
+  EXPECT_EQ(get_table_value<long long int>(lua_background, "key2"), 42);
+  EXPECT_EQ(get_table_value<long long int>(lua_background, "count"), 2);
+
+  lua_background.shutdown();
+}
+
 TEST(TestRGWLua, TracingSetAttribute)
 {
   const std::string script = R"(