Skip to content

Conversation

@sundb
Copy link
Collaborator

@sundb sundb commented Jul 1, 2024

Issue

The current implementation of FUNCTION FLUSH command uses lua_unref() to unreference script closures in Lua vm. However, invoking lua_unref() during lazy free (ASYNC argument) is risky since it is not thread-safe.

Another issue is that using lua_unref() to unreference references does not trigger GC, This can result in the Lua VM leaves a significant amount of garbage, which may never be cleaned up if not properly GC.

Solution

The proposed solution is to completely rebuild the engines, resulting in a brand new Lua VM.

Copy link

@MeirShpilraien MeirShpilraien left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
Looks good, 2 small comments.

MeirShpilraien
MeirShpilraien previously approved these changes Jul 8, 2024
@sundb sundb added the release-notes indication that this issue needs to be mentioned in the release notes label Jul 9, 2024
oranagra
oranagra previously approved these changes Jul 9, 2024
@sundb sundb changed the title Recreate function engines for function flush command Rebuild function engines for function flush command Jul 9, 2024
@sundb sundb requested a review from oranagra July 9, 2024 16:02
@sundb sundb merged commit ffff7fe into redis:unstable Jul 10, 2024
sundb added a commit to sundb/redis that referenced this pull request Jul 10, 2024
@sundb sundb deleted the lua_unref_function branch August 17, 2024 07:09
sundb added a commit that referenced this pull request Aug 19, 2024
…3476)

This is a missing of the PR #13383.
We will call `functionsLibCtxClear()` in bio, so we shouldn't touch
`curr_functions_lib_ctx` in it.
YaacovHazan pushed a commit that referenced this pull request Nov 4, 2024
…3476)

This is a missing of the PR #13383.
We will call `functionsLibCtxClear()` in bio, so we shouldn't touch
`curr_functions_lib_ctx` in it.
YaacovHazan pushed a commit that referenced this pull request Jan 6, 2025
…3476)

This is a missing of the PR #13383.
We will call `functionsLibCtxClear()` in bio, so we shouldn't touch
`curr_functions_lib_ctx` in it.
funny-dog pushed a commit to funny-dog/redis that referenced this pull request Sep 17, 2025
### Issue
The current implementation of `FUNCTION FLUSH` command uses
`lua_unref()` to unreference script closures in Lua vm. However,
invoking `lua_unref()` during lazy free (`ASYNC` argument) is risky
since it is not thread-safe.

Another issue is that using `lua_unref()` to unreference references does
not trigger GC, This can result in the Lua VM leaves a significant
amount of garbage, which may never be cleaned up if not properly GC.

### Solution
The proposed solution is to completely rebuild the engines, resulting in
a brand new Lua VM.

---------

Co-authored-by: meir <meir@redis.com>
funny-dog pushed a commit to funny-dog/redis that referenced this pull request Sep 17, 2025
…dis#13476)

This is a missing of the PR redis#13383.
We will call `functionsLibCtxClear()` in bio, so we shouldn't touch
`curr_functions_lib_ctx` in it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes indication that this issue needs to be mentioned in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants