Wip rgw/lua: performance improvements by nbalacha · Pull Request #66832 · ceph/ceph

nbalacha · 2026-01-08T06:15:35Z

WIP
Improve performance by caching Lua bytecode.

Fixes: https://tracker.ceph.com/issues/74219

Contribution Guidelines

To sign and title your commits, please refer to Submitting Patches to Ceph.
If you are submitting a fix for a stable branch (e.g. "quincy"), please refer to Submitting Patches to Ceph - Backports for the proper workflow.
When filling out the below checklist, you may click boxes directly in the GitHub web UI. When entering or editing the entire PR message in the GitHub web UI editor, you may also select a checklist item by adding an x between the brackets: [x]. Spaces and capitalization matter when checking off items this way.

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

Show available Jenkins commands

jenkins test classic perf Jenkins Job | Jenkins Job Definition
jenkins test crimson perf Jenkins Job | Jenkins Job Definition
jenkins test signed Jenkins Job | Jenkins Job Definition
jenkins test make check Jenkins Job | Jenkins Job Definition
jenkins test make check arm64 Jenkins Job | Jenkins Job Definition
jenkins test submodules Jenkins Job | Jenkins Job Definition
jenkins test dashboard Jenkins Job | Jenkins Job Definition
jenkins test dashboard cephadm Jenkins Job | Jenkins Job Definition
jenkins test api Jenkins Job | Jenkins Job Definition
jenkins test docs ReadTheDocs | Github Workflow Definition
jenkins test ceph-volume all Jenkins Jobs | Jenkins Jobs Definition
jenkins test windows Jenkins Job | Jenkins Job Definition
jenkins test rook e2e Jenkins Job | Jenkins Job Definition

You must only issue one Jenkins command per-comment. Jenkins does not understand
comments with more than one command.

anthonyeleven · 2026-01-08T06:39:11Z

doc/radosgw/lua-scripting.rst

 - The maximum number of entries in the table is 100,000. Each entry has a string key a value with a combined length of no more than 1KB.
 A Lua script will abort with an error if the number of entries or entry size exceeds these limits.
- The ``RGW`` Lua table uses string indices and can store values of type: string, integer, double and boolean
+- The ``RGW`` Lua table uses string indices and can store values of type string, integer, double and boolean


Suggest double backticks around the four type literals.

anthonyeleven · 2026-01-08T06:40:14Z

doc/radosgw/lua-scripting.rst

- if the value of ``key`` is not numeric, the execution of the script would fail
- if we try to increment or decrement by non-numeric values, the execution of the script would fail
+- If the value of ``key`` is not numeric, the execution of the script would fail
+- If we try to increment or decrement with non-numeric values, the execution of the script would fail


S/would/will/

yuvalif · 2026-01-08T08:44:23Z

src/rgw/rgw_lua_background.h

@@ -151,11 +154,19 @@ class Background : public RGWRealmReloader::Pauser {
  std::mutex pause_mutex;
  std::condition_variable cond;

+  std::set<std::string> updated_scripts;


this could be a local variable of the processing function

yuvalif · 2026-01-08T08:48:33Z

src/rgw/rgw_lua_background.cc

+  }
+}
+
+int Background::bytecode_writer (lua_State *L, const void* p,


dont think this should be a member function. just make it a standalone function here and remove from the class definition

src/rgw/rgw_lua_background.h

yuvalif · 2026-01-08T08:50:46Z

src/rgw/rgw_lua_background.cc

+    }
+    ldpp_dout(&dp, 10) << "NITHYA: processing : " << key << dendl;
+    auto L = lguard->get();
+// TODO: use a unique_ptr?


yes, please try to avoid new/delete as much as you can

yuvalif · 2026-01-08T08:56:34Z

src/rgw/driver/rados/rgw_sal_rados.h

+  std::map<std::string, uint64_t> script_watches;
+  std::map<uint64_t, std::string> reverse_script_watches;
+
+  std::mutex updates;


is this being used?

src/rgw/driver/rados/rgw_sal_rados.cc

yuvalif · 2026-01-08T09:17:02Z

src/rgw/driver/rados/rgw_sal_rados.cc

+  }
+}
+
+uint64_t RadosLuaManager::get_watch_handle_for_script(std::string script_oid) {


Suggested change

uint64_t RadosLuaManager::get_watch_handle_for_script(std::string script_oid) {

uint64_t RadosLuaManager::get_watch_handle_for_script(const std::string& script_oid) {

yuvalif · 2026-01-08T09:18:24Z

src/rgw/driver/rados/rgw_sal_rados.cc

@@ -23,6 +23,7 @@
 #include <boost/process.hpp>

 #include <fmt/core.h>
+#include <lua.hpp>


what do you need this include for?

yuvalif · 2026-01-08T09:20:25Z

src/rgw/driver/posix/rgw_sal_posix.h

@@ -458,6 +458,8 @@ class POSIXLuaManager : public StoreLuaManager {
  virtual ~POSIXLuaManager() = default;

  virtual int get_script(const DoutPrefixProvider* dpp, optional_yield y, const std::string& key, std::string& script) override;
+  virtual int get_script_or_bytecode(const DoutPrefixProvider* dpp, optional_yield y, const std::string& key,
+                                     std::string& script, std::vector<char>& lua_bytecode) override;


can we replace these 2 parameters with std::variant holding an std::string or std::vector<char> ?
I think this would make the intent here more clear.

yuvalif · 2026-01-08T09:25:00Z

src/rgw/driver/rados/rgw_sal_rados.cc

+    return r;
+  }
+
+  lua_background->mark_script_updated(key);


maybe rename this function to "script_needs_processing"?
we dont know if the script was updated or not

src/rgw/driver/rados/rgw_sal_rados.cc

yuvalif · 2026-01-08T09:35:14Z

src/rgw/rgw_lua_background.h

@@ -151,11 +154,19 @@ class Background : public RGWRealmReloader::Pauser {
  std::mutex pause_mutex;
  std::condition_variable cond;

+  std::set<std::string> updated_scripts;
+  std::map<std::string, std::vector<char>*> lua_bytecode_cache; // script-name -> bytecode


is it possible to use:

Suggested change

std::map<std::string, std::vector<char>*> lua_bytecode_cache; // script-name -> bytecode

std::map<std::string, std::unique_ptr<std::vector<char>>> lua_bytecode_cache; // script-name -> bytecode

src/rgw/driver/rados/rgw_sal_rados.cc

yuvalif · 2026-01-14T09:53:41Z

src/rgw/rgw_lua.h

@@ -32,6 +32,9 @@ enum class context {
 // return "none" if not matched
 context to_context(const std::string& s);

+// Can hold the script or the bytecode
+typedef std::variant<std::string, std::vector<char>> luaCodeType;


nit:

Suggested change

typedef std::variant<std::string, std::vector<char>> luaCodeType;

using luaCodeType = std::variant<std::string, std::vector<char>> ;

yuvalif · 2026-01-14T09:54:37Z

src/rgw/rgw_lua_background.cc

@@ -218,5 +219,98 @@ void Background::set_manager(rgw::sal::LuaManager* _lua_manager) {
  lua_manager = _lua_manager;
 }

+void Background::process_script_add(std::string script_oid) {
+  std::unique_ptr<std::string> script_ptr = std::make_unique<std::string>(script_oid);
+  processing_q.push(script_ptr.release());


just use new (this is where it actually makes more sense)

yuvalif · 2026-01-14T09:56:43Z

src/rgw/rgw_lua_background.cc

+    }
+    ldpp_dout(&dp, 10) << "NITHYA: processing : " << key << dendl;
+    auto L = lguard->get();
+    std::unique_ptr<std::vector<char>> buffer = std::make_unique<std::vector<char>>();


nit (since it is clear what the type is from the make_unique call):

Suggested change

std::unique_ptr<std::vector<char>> buffer = std::make_unique<std::vector<char>>();

auto buffer = std::make_unique<std::vector<char>>();

yuvalif · 2026-01-14T10:08:35Z

src/rgw/rgw_lua_request.cc

-      const std::string err(lua_tostring(L, -1));
-      ldpp_dout(s, 1) << "Lua ERROR: " << err << dendl;
+    // execute the lua script or bytecode
+    if (std::holds_alternative<std::vector<char>>(code)) {


nit:
instead of if/else maybe you can use std::visit with 2 lambdas, one that accept std::string and one that accepts std::vector<char> ?

I'll look into this.

yuvalif · 2026-01-14T10:10:05Z

src/rgw/rgw_process.cc

-      auto rc = rgw::lua::read_script(s, penv.lua.manager.get(),
-                                      s->bucket_tenant, s->yield,
-                                      rgw::lua::context::postRequest, script);
+      std::vector<char> lua_bytecode;


are you using script and lua_bytecode ?

yuvalif · 2026-01-14T10:11:13Z

src/rgw/rgw_process.cc

-      auto rc = rgw::lua::read_script(s, penv.lua.manager.get(),
-                                      s->bucket_tenant, s->yield,
-                                      rgw::lua::context::preRequest, script);
+      std::vector<char> lua_bytecode;


same as line 446

nbalacha · 2026-02-03T05:24:51Z

The PR has passed the lua suite in teuthology
https://pulpito.ceph.com/nithyab-2026-02-03_04:34:22-rgw:lua-wip-nbalacha-lua-74219-distro-default-trial/

nbalacha · 2026-02-03T05:39:33Z

jenkins test make check

yuvalif · 2026-02-03T07:10:34Z

jenkins test api

yuvalif · 2026-02-03T07:16:05Z

@nbalacha i think we can merge once we have perf results

github-actions · 2026-02-12T19:25:16Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

mkogan1

LGTM, there is a performance improvement observed with this PR.

Test methodology:

cat > ./autotier.lua
-- Lua script to auto-tier S3 object PUT requests

-- exit script quickly if it is not a PUT request
if Request == nil or Request.RGWOp ~= "put_obj"
then
  return
end

-- apply StorageClass only if user hasn't already assigned a storage-class
if Request.HTTP.StorageClass == nil or Request.HTTP.StorageClass == '' then
  if Request.ContentLength < 16384 then
    Request.HTTP.StorageClass = "SMALL_OBJ"
  elseif Request.ContentLength < 1048576 then
    Request.HTTP.StorageClass = "MEDIUM_OBJ"
  else
    Request.HTTP.StorageClass = "STANDARD"
  end
end
^D

sudo ./bin/radosgw-admin zonegroup placement add --rgw-zone default --placement-id default-placement --storage-class SMALL_OBJ |jq -C
sudo ./bin/radosgw-admin zone placement add --rgw-zone default --placement-id default-placement --storage-class SMALL_OBJ --data-pool default.rgw.buckets.data.lukewarm |jq -C

sudo ./bin/radosgw-admin zonegroup placement add --rgw-zone default --placement-id default-placement --storage-class MEDIUM_OBJ |jq -C
sudo ./bin/radosgw-admin zone placement add --rgw-zone default --placement-id default-placement --storage-class MEDIUM_OBJ --data-pool default.rgw.buckets.data.frozen |jq -C



sudo ./bin/radosgw-admin script put --context=preRequest --infile=./autotier.lua --context=preRequest
#sudo ./bin/radosgw-admin script rm --context=preRequest



sudo truncate -s0 ./out/radosgw.8000.log ; sudo fuser -vk -9 ./bin/radosgw
sleep 1.25 ; sudo numactl -N 0 -m 0 --  ./bin/radosgw  --nolockdep -c ./ceph.conf --log-file=./out/radosgw.8000.log --admin-socket=./out/radosgw.8000.asok --pid-file=./out/radosgw.8000.pid -n client.rgw.8000 --rgw_frontends="beast port=8000 tcp_nodelay=1 request_timeout_ms=0 ssl_port=8443 ssl_certificate=./rgw.crt ssl_private_key=./rgw.key" --debug_ms=0 --debug_rgw=1 --debug_rgw_sync=0 --debug_rgw_notification=1 --debug_compressor=1 --debug_crypto=1  -f


sudo ./bin/radosgw-admin script put --context=preRequest --infile=./autotier.lua --context=preRequest


## WORKLOAD:
echo $RANDOM ; time (nice numactl -N 1 -m 1 -- env GOMAXPROCS=$(( $(numactl -N 0 -- nproc) / 2 )) ~/go/bin/hsbench -a b2345678901234567890 -s b234567890123456789012345678901234567890 -u http://127.0.0.1:8000 -z 4K -d -1 -t $(( $(numactl -N 0 -- nproc) / 2 )) -b $(( $(numactl -N 0 -- nproc) * 2 )) -n 100000 -m cxip -bp b01b${RANDOM}- -op 'folder01/stage01_' |& tee hsbench.log | stdbuf -o0 colrm $((${COLUMNS} - 1)))

Baseline performance: --rgw_lua_enable=false

performance: --rgw_lua_enable=true & WITHOUT PR #66832

performance: --rgw_lua_enable=true & WITH PR #66832
(sample of two runs)

On average, performance is better than without the PR.

nbalacha · 2026-02-19T15:43:11Z

@anthonyeleven , I will move the doc changes to a different PR to make it easier to address comments.

nbalacha · 2026-02-20T04:26:59Z

@anthonyeleven , I will move the doc changes to a different PR to make it easier to address comments.

@anthonyeleven , please see #67441

github-actions · 2026-02-23T13:42:23Z

This pull request can no longer be automatically merged: a rebase is needed and changes have to be manually resolved

yuvalif · 2026-02-25T09:41:32Z

src/rgw/rgw_lua_request.cc

    if (s->penv.lua.background) {
      s->penv.lua.background->create_background_metatable(L);
    }
+    rc = rgw::lua::lua_execute(L, s, code);


why do you ignore rc?

i'm wondering if the origina lcode is correct?
if rc != LUA_OK should we truct the value of lua_tointeger(L, -1) or just reporn an error and dont set the return code value (similar to what happens in case of the exception)?

sorry. not an issue. a script that has a runtime issue after the "return" call, will not fail:

return RGW_ABORT_REQUEST RGWDebugLog("hello "..Request.Kaboom)

so this is not an issue

Improves performance by caching the Lua bytecode. Fixes: https://tracker.ceph.com/issues/74219 Signed-off-by: Nithya Balachandran <nithya.balachandran@ibm.com>

yuvalif · 2026-02-26T07:55:23Z

jenkins test make check

yuvalif · 2026-02-26T07:55:31Z

jenkins test make check arm64

nbalacha · 2026-03-02T10:54:22Z

Latest teuthology run results:

https://pulpito.ceph.com/nithyab-2026-02-26_13:17:07-rgw-wip-nbalacha-lua-74219-distro-default-trial/

There are 22 failures, of which:

crypt and tempest failures (keystone related) : 4
multisite failures: 2
valgrind failures: 8
d4n failure: 1
singleton ('boto.vendored.six.moves') : 3
bucket notification : 1
multifs: 3

These are unrelated to this PR.

issue was resolved

github-actions · 2026-03-02T10:58:34Z

This is an automated message by src/script/redmine-upkeep.py.

I found one or more Fixes: tags in the commit messages in

git log a5263d8de61cc137043bac141b288fe65d3266cc^..a5263d8de61cc137043bac141b288fe65d3266cc

The referenced tickets are:

https://tracker.ceph.com/issues/74219

Those tickets do not reference this merged Pull Request. If this Pull Request merge resolves any of those tickets, please update the "Pull Request ID" field on each ticket. A future run of this script will appropriately update them.

Update Log: https://github.com/ceph/ceph/actions/runs/22572870530

nbalacha requested review from a team as code owners January 8, 2026 06:15

github-actions bot added documentation rgw labels Jan 8, 2026

nbalacha requested a review from yuvalif January 8, 2026 06:15

nbalacha marked this pull request as draft January 8, 2026 06:16

anthonyeleven reviewed Jan 8, 2026

View reviewed changes

yuvalif reviewed Jan 8, 2026

View reviewed changes

src/rgw/rgw_lua_background.h Show resolved Hide resolved

yuvalif reviewed Jan 8, 2026

View reviewed changes

src/rgw/driver/rados/rgw_sal_rados.cc Show resolved Hide resolved

yuvalif reviewed Jan 8, 2026

View reviewed changes

src/rgw/driver/rados/rgw_sal_rados.cc Outdated Show resolved Hide resolved

yuvalif reviewed Jan 8, 2026

View reviewed changes

src/rgw/driver/rados/rgw_sal_rados.cc Show resolved Hide resolved

yuvalif reviewed Jan 8, 2026

View reviewed changes

nbalacha force-pushed the wip-nbalacha-lua-74219 branch 2 times, most recently from 7c9bddc to 56649c0 Compare January 14, 2026 05:37

yuvalif reviewed Jan 14, 2026

View reviewed changes

src/rgw/driver/rados/rgw_sal_rados.cc Show resolved Hide resolved

yuvalif reviewed Jan 14, 2026

View reviewed changes

nbalacha force-pushed the wip-nbalacha-lua-74219 branch from 56649c0 to fdf517e Compare January 14, 2026 12:40

yuvalif approved these changes Feb 2, 2026

View reviewed changes

yuvalif added the needs-qa label Feb 2, 2026

github-actions bot added the needs-rebase label Feb 12, 2026

mkogan1 self-requested a review February 18, 2026 16:21

mkogan1 approved these changes Feb 18, 2026

View reviewed changes

nbalacha force-pushed the wip-nbalacha-lua-74219 branch from 0e26b0e to 345166c Compare February 19, 2026 15:42

github-actions bot removed the needs-rebase label Feb 19, 2026

nbalacha force-pushed the wip-nbalacha-lua-74219 branch from 345166c to d7bcb16 Compare February 19, 2026 15:54

anrao19 added the wip-anrao2-testing label Feb 23, 2026

github-actions bot added the needs-rebase label Feb 23, 2026

nbalacha force-pushed the wip-nbalacha-lua-74219 branch 2 times, most recently from 3df4c7c to 5690d38 Compare February 25, 2026 09:24

github-actions bot removed the needs-rebase label Feb 25, 2026

yuvalif reviewed Feb 25, 2026

View reviewed changes

rgw/lua: cache lua bytecode

b5fe6fa

Improves performance by caching the Lua bytecode. Fixes: https://tracker.ceph.com/issues/74219 Signed-off-by: Nithya Balachandran <nithya.balachandran@ibm.com>

nbalacha force-pushed the wip-nbalacha-lua-74219 branch from 5690d38 to b5fe6fa Compare February 25, 2026 10:29

nbalacha merged commit a5263d8 into ceph:main Mar 2, 2026
13 checks passed

	uint64_t RadosLuaManager::get_watch_handle_for_script(std::string script_oid) {
	uint64_t RadosLuaManager::get_watch_handle_for_script(const std::string& script_oid) {

	std::map<std::string, std::vector<char>*> lua_bytecode_cache; // script-name -> bytecode
	std::map<std::string, std::unique_ptr<std::vector<char>>> lua_bytecode_cache; // script-name -> bytecode

	typedef std::variant<std::string, std::vector<char>> luaCodeType;
	using luaCodeType = std::variant<std::string, std::vector<char>> ;

	std::unique_ptr<std::vector<char>> buffer = std::make_unique<std::vector<char>>();
	auto buffer = std::make_unique<std::vector<char>>();

Conversation

nbalacha commented Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution Guidelines

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yuvalif Jan 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nbalacha commented Feb 3, 2026

Uh oh!

nbalacha commented Feb 3, 2026

Uh oh!

yuvalif commented Feb 3, 2026

Uh oh!

yuvalif commented Feb 3, 2026

Uh oh!

github-actions bot commented Feb 12, 2026

nbalacha commented Jan 8, 2026 •

edited

Loading

yuvalif Jan 8, 2026 •

edited

Loading

mkogan1 left a comment •

edited

Loading