Add Static Module Support by eifrah-aws · Pull Request #3392 · valkey-io/valkey

eifrah-aws · 2026-03-23T10:08:18Z

Add a build option to compile the Lua scripting engine as a static module and wire the server to load it directly at startup when enabled. The module load path now resolves on-load and on-unload entry points from the main binary, and the module lifecycle keeps those callbacks so unload works without a shared library handle.

The Lua module build was updated to support both static and shared variants, with the static path exporting visible wrapper symbols and linking the server with the module archive. While touching the Lua code, a few internal symbols were renamed for consistency and the monotonic time helper was clarified.

Note that this PR addresses the LUA module, but it can be applied to other "core" modules (like: Bloom, Json, Search and others). With this change, it will be easier to ship Valkey bundle with modules.

Areas touched:

CMake
Makefile
Lua scripting module
Core module loading

Generated by CodeLite

Add a build option to compile the Lua scripting engine as a static module and wire the server to load it directly at startup when enabled. The module load path now resolves on-load and on-unload entry points from the main binary, and the module lifecycle keeps those callbacks so unload works without a shared library handle. The Lua module build was updated to support both static and shared variants, with the static path exporting visible wrapper symbols and linking the server with the module archive. While touching the Lua code, a few internal symbols were renamed for consistency and the monotonic time helper was clarified. * CMake * Makefile * Lua scripting module * Core module loading **Generated by CodeLite** Signed-off-by: Eran Ifrah <eifrah@amazon.com>

codecov · 2026-03-23T10:31:25Z

Codecov Report

❌ Patch coverage is 52.80899% with 42 lines in your changes missing coverage. Please review.
✅ Project coverage is 76.39%. Comparing base (b2e0f63) to head (68a09c7).

Files with missing lines	Patch %	Lines
src/module.c	51.72%	42 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #3392      +/-   ##
============================================
- Coverage     76.45%   76.39%   -0.07%     
============================================
  Files           159      159              
  Lines         79840    79879      +39     
============================================
- Hits          61042    61022      -20     
- Misses        18798    18857      +59

Files with missing lines	Coverage Δ
src/config.c	`78.09% <100.00%> (-0.24%)`	⬇️
src/eval.c	`91.50% <ø> (ø)`
src/module.h	`0.00% <ø> (ø)`
src/server.c	`89.60% <100.00%> (+0.10%)`	⬆️
src/module.c	`25.53% <51.72%> (-0.36%)`	⬇️

... and 24 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ranshid · 2026-03-23T13:18:04Z

No deep dive yet.

I think we need to clarify requirements before proceeding with a deep dive.

The original change aimed to:

Enable users to define and use new scripting engines (e.g., LuaJIT, JavaScript)
Allow users who don't use script commands (EVAL, EVALSHA) to compile and run Valkey without Lua support, avoiding frequent patching for Lua library vulnerabilities

For the 9.1 release, we must decide:

Legacy Lua load/unload support: Should we support dynamically loading/unloading Lua? The current implementation allows unloading Lua (though it remains in the binary) to replace it with a different Lua module. This could enable users to dynamically "patch" Lua, but the old module persists in the code. Does this provide meaningful value?
Observability: The new Lua currently appears in the module list. This may impact installations that verify module list output. This decision depends on /1 if Lua cannot be dynamically loaded/unloaded, should we list it like other modules?

madolson · 2026-03-23T16:12:52Z

Core team discussion:

Is this is an important thing to do in 9.1?
1. We think it's important for 9.1.
We are fine with having the same observability.
We are fine with unloading and not being able to reload it.
If it's statically linked it's loaded automatically. If it's not statically linked it's not loaded automatically.
Statically linked will the default. Review the makefile variable so that it's similar with TLS and RDMA.

@valkey-io/core-team and @rjd15372 specifically in case you have some thoughts.

See #2858 (comment) for some additional discussion.

When a scripting engine is unregistered, its cached eval scripts are now removed as well. This prevents the eval cache from holding dangling references to the engine after it has been unloaded. The eval subsystem now exposes a helper for deleting all scripts owned by a specific engine, updates the eval cache bookkeeping as entries are removed, and logs how many scripts were cleaned up during engine unregistration. * scripting_engine * eval **Generated by CodeLite** Signed-off-by: Eran Ifrah <eifrah@amazon.com>

Update the Lua module to use the engine-provided monotonic clock helper when built with STATIC_LUA, while keeping the module's local implementation for non-static builds. This avoids duplicate timing implementations and keeps elapsed-time handling consistent across build configurations. * Lua module * Static build integration **Generated by CodeLite** Signed-off-by: Eran Ifrah <eifrah@amazon.com>

The server startup path now only auto-loads Lua at runtime for the static build. Consolidate Lua scripting engine build configuration around a single BUILD_LUA value with explicit static, module, and no modes. This removes the separate BUILD_STATIC_LUA toggle and updates the build system to derive compile definitions, link behavior, and module packaging from the selected mode. * Build system * Server startup * Lua module packaging **Generated by CodeLite** Signed-off-by: Eran Ifrah <eifrah@amazon.com>

* Build system **Generated by CodeLite** Signed-off-by: Eran Ifrah <eifrah@amazon.com>

ranshid · 2026-03-26T06:06:34Z

@eifrah-aws please check the CI failures

Adjust the module API hook test to inspect a larger tail of the replica log when verifying the shutdown event. This makes the assertion less brittle and better aligned with the amount of output produced during shutdown. **Generated by CodeLite** Signed-off-by: Eran Ifrah <eifrah@amazon.com>

The extra log line caused an error to some of the TCL tests. Signed-off-by: Eran Ifrah <eifrah@amazon.com>

ranshid

Overall looks very good!

One high level comment I have is the lack of good testing. Can we place some tests that verify statoc AND dynamic lua are not breaking to load/unload and that the engine can start without a lua engine (also when eval/evalsha are being generated)?

In addition, removed duplicate code. Minor fix in the Makefile: settings LUA_MODULE_NAME to its correct name. Signed-off-by: Eran Ifrah <eifrah@amazon.com>

Signed-off-by: Eran Ifrah <eifrah@amazon.com>

zuiderkwast

Looks good in general, but it seems the visibility attributes things are not finalized?

Signed-off-by: Eran Ifrah <eifrah@amazon.com>

Update the documentation comments for the static module loading helpers to better match Valkey's standard. * API docs **Generated by CodeLite** Signed-off-by: Eran Ifrah <eifrah@amazon.com>

Extract the common module initialization logic from `moduleLoad` and `moduleLoadStatic` into a new helper function `moduleInitPostOnLoadResolved`. This function handles the shared workflow of invoking the onload callback, registering the module, performing post-load validation, and firing server events. The refactoring eliminates approximately 70 lines of duplicated code while preserving all existing behavior. The helper function accepts parameters that control static vs. dynamic module handling, including whether to close the dlopen handle and how to format log messages. Both `moduleLoad` and `moduleLoadStatic` now focus on their specific responsibilities (dlopen/symbol resolution) and delegate the common initialization sequence to the shared helper. * Module loading infrastructure * Code deduplication **Generated by CodeLite** Signed-off-by: Eran Ifrah <eifrah@amazon.com>

- Move `evalRemoveScriptsOfEngine` code to a separate PR - Renamed `lua_scripts_flags_def` -> `lua_scripts_flags` with `static` Signed-off-by: Eran Ifrah <eifrah@amazon.com>

When building static lua, use: - `getMonotonicUs()` - `elapsedUs` - `elapsedMs` from the engine's `monotonic.h` header. Signed-off-by: Eran Ifrah <eifrah@amazon.com>

madolson

Sorry for the delay. The tests all seem unrelated, but lgtm

eifrah-aws · 2026-04-20T05:40:04Z

Sorry for the delay. The tests all seem unrelated, but lgtm

Thanks, for the review. I have updated the branch to the latest unstable.

zuiderkwast · 2026-04-21T14:13:40Z

Forgot to update README.md with the build options?

Add a build option to compile the Lua scripting engine as a static module and wire the server to load it directly at startup when enabled. The module load path now resolves on-load and on-unload entry points from the main binary, and the module lifecycle keeps those callbacks so unload works without a shared library handle. The Lua module build was updated to support both static and shared variants, with the static path exporting visible wrapper symbols and linking the server with the module archive. While touching the Lua code, a few internal symbols were renamed for consistency and the monotonic time helper was clarified. Note that this PR addresses the LUA module, but it can be applied to other "core" modules (like: Bloom, Json, Search and others). With this change, it will be easier to ship Valkey bundle with modules. Areas touched: * CMake * Makefile * Lua scripting module * Core module loading **Generated by CodeLite** --------- Signed-off-by: Eran Ifrah <eifrah@amazon.com>

…sers) Removed valkey-io#3504, valkey-io#3545, valkey-io#3503 - these fix bugs introduced by valkey-io#3324, valkey-io#2936, Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com> valkey-io#3392 respectively, all new in RC2. Users never experienced these bugs.

github-actions Bot assigned eifrah-aws Mar 23, 2026

ranshid requested review from madolson and rjd15372 March 23, 2026 10:18

ranshid added the run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) label Mar 23, 2026

ranshid self-requested a review March 23, 2026 13:18

dvkashapov reviewed Mar 23, 2026

View reviewed changes

Comment thread src/modules/lua/script_lua.c Outdated

madolson added this to Valkey 9.1 Mar 23, 2026

madolson moved this to Todo in Valkey 9.1 Mar 23, 2026

yulazariy mentioned this pull request Mar 25, 2026

Add relative rpath for runtime library discovery #3377

Closed

eifrah-aws marked this pull request as ready for review March 25, 2026 15:51

eifrah-aws added 3 commits March 25, 2026 17:53

Fix Lua Module Build Flag Propagation

2be8ea9

* Build system **Generated by CodeLite** Signed-off-by: Eran Ifrah <eifrah@amazon.com>

ranshid mentioned this pull request Mar 29, 2026

Issue compiling valkey-cli and valkey-benchmark without optimizations #3415

Closed

Removed extra log line during shutdown

4b7ff25

The extra log line caused an error to some of the TCL tests. Signed-off-by: Eran Ifrah <eifrah@amazon.com>

ranshid reviewed Mar 30, 2026

View reviewed changes

Comment thread src/modules/lua/Makefile

Comment thread src/modules/lua/Makefile

Comment thread src/modules/lua/script_lua.c Outdated

eifrah-aws added 2 commits March 30, 2026 13:03

Use weak attributes on duplicate symbols

412e8c8

In addition, removed duplicate code. Minor fix in the Makefile: settings LUA_MODULE_NAME to its correct name. Signed-off-by: Eran Ifrah <eifrah@amazon.com>

Mark sha1hex as a weak symbol

08553b6

Signed-off-by: Eran Ifrah <eifrah@amazon.com>

zuiderkwast reviewed Mar 30, 2026

View reviewed changes

Comment thread src/module.c Outdated

eifrah-aws added 4 commits April 5, 2026 11:25

Merge branch 'unstable' into static-lua

e2cfc88

Fixed loading on FreeBSD

ec69dd1

Signed-off-by: Eran Ifrah <eifrah@amazon.com>

Refactor Static Module Loading Documentation

686fde1

Update the documentation comments for the static module loading helpers to better match Valkey's standard. * API docs **Generated by CodeLite** Signed-off-by: Eran Ifrah <eifrah@amazon.com>

Merge branch 'unstable' into static-lua

4c548f0

madolson reviewed Apr 13, 2026

View reviewed changes

Comment thread src/module.c

madolson reviewed Apr 13, 2026

View reviewed changes

Comment thread src/modules/lua/script_lua.c Outdated

madolson reviewed Apr 13, 2026

View reviewed changes

Comment thread src/scripting_engine.c Outdated

madolson reviewed Apr 13, 2026

View reviewed changes

Comment thread src/modules/lua/function_lua.c

eifrah-aws added 4 commits April 14, 2026 14:59

Addressed comments

251f2fe

- Move `evalRemoveScriptsOfEngine` code to a separate PR - Renamed `lua_scripts_flags_def` -> `lua_scripts_flags` with `static` Signed-off-by: Eran Ifrah <eifrah@amazon.com>

When possible, use engine's time helpers

17c0128

When building static lua, use: - `getMonotonicUs()` - `elapsedUs` - `elapsedMs` from the engine's `monotonic.h` header. Signed-off-by: Eran Ifrah <eifrah@amazon.com>

Merge branch 'unstable' into static-lua

68a09c7

eifrah-aws requested a review from madolson April 16, 2026 14:24

madolson approved these changes Apr 19, 2026

View reviewed changes

Merge branch 'unstable' into static-lua

8125f2c

ranshid merged commit 0327c27 into valkey-io:unstable Apr 20, 2026
60 of 64 checks passed

github-project-automation Bot moved this from Todo to To be backported in Valkey 9.1 Apr 20, 2026

This was referenced Apr 21, 2026

[NEW] Disable LUA integration during build #1204

Closed

WIP: Build without Lua scripting via USE_LUA (#1204) #1245

Closed

sarthakaggarwal97 mentioned this pull request Apr 23, 2026

Backport Unstable to 9.1 for RC2 #3519

Merged

hpatro mentioned this pull request Apr 23, 2026

Pin clang to version 17 in sanitizer CI jobs #3546

Merged

sarthakaggarwal97 added the release-notes This issue should get a line item in the release notes label Apr 28, 2026

This was referenced May 3, 2026

[Shadow] Update version to 9.1.0-rc2 and add release notes sarthakaggarwal97/valkey#158

Closed

[Eval] Agent review bench sarthakaggarwal97/valkey#218

Closed

[backport] Weekly backport sweep for 9.1 sarthakaggarwal97/valkey#225

Closed

eifrah-aws deleted the static-lua branch May 11, 2026 08:25

sarthakaggarwal97 moved this from To be backported to Done in Valkey 9.1 May 16, 2026

eifrah-aws restored the static-lua branch June 1, 2026 07:36

eifrah-aws deleted the static-lua branch June 1, 2026 07:37

Uh oh!

Conversation

eifrah-aws commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ranshid commented Mar 23, 2026

Uh oh!

Uh oh!

madolson commented Mar 23, 2026

Uh oh!

ranshid commented Mar 26, 2026

Uh oh!

ranshid left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zuiderkwast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

madolson left a comment

Choose a reason for hiding this comment

Uh oh!

eifrah-aws commented Apr 20, 2026

Uh oh!

Uh oh!

zuiderkwast commented Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

eifrah-aws commented Mar 23, 2026 •

edited

Loading

codecov Bot commented Mar 23, 2026 •

edited

Loading