Skip to content

Conversation

@yoav-steinberg
Copy link
Contributor

@yoav-steinberg yoav-steinberg commented Jan 10, 2022

See #8478
Allow configurable behavior for OOM during scripting:

  • Deny all writes (default).
  • Allow writes not flagged as CMD_DENYOOM and all writes if dataset is dirty (previous behavior).
  • Allow writes not flagged as CMD_DENYOOM and if dataset is dirty other writes will fail and break atomicity.

Allow configurable behavior for OOM during scripting:
 - Deny all writes (default).
 - Allow writes not flagged as CMD_DENYOOM and all writes if dataset is dirty.
 - Allow writes not flagged as CMD_DENYOOM and if dataset is dirty other writes will fail and break atomicity.
@oranagra
Copy link
Member

@yoav-steinberg please look at the plan we came up with here: #10066 (comment)
i think it can completely replace this PR.

@oranagra
Copy link
Member

Closing in favor of #10126

@oranagra oranagra closed this Jan 18, 2022
oranagra pushed a commit that referenced this pull request Jan 24, 2022
In #10025 we added a mechanism for flagging certain properties for Redis Functions.
This lead us to think we'd like to "port" this mechanism to Redis Scripts (`EVAL`) as well. 

One good reason for this, other than the added functionality is because it addresses the
poor behavior we currently have in `EVAL` in case the script performs a (non DENY_OOM) write operation
during OOM state. See #8478 (And a previous attempt to handle it via #10093) for details.
Note that in Redis Functions **all** write operations (including DEL) will return an error during OOM state
unless the function is flagged as `allow-oom` in which case no OOM checking is performed at all.

This PR:
- Enables setting `EVAL` (and `SCRIPT LOAD`) script flags as defined in #10025.
- Provides a syntactical framework via [shebang](https://en.wikipedia.org/wiki/Shebang_(Unix)) for
  additional script annotations and even engine selection (instead of just lua) for scripts.
- Provides backwards compatibility so scripts without the new annotations will behave as they did before.
- Appropriate tests.
- Changes `EVAL[SHA]/_RO` to be flagged as `STALE` commands. This makes it possible to flag individual
  scripts as `allow-stale` or not flag them as such. In backwards compatibility mode these commands will
  return the `MASTERDOWN` error as before.
- Changes `SCRIPT LOAD` to be flagged as a `STALE` command. This is mainly to make it logically
  compatible with the change to `EVAL` in the previous point. It enables loading a script on a stale server
  which is technically okay it doesn't relate directly to the server's dataset. Running the script does, but that
  won't work unless the script is explicitly marked as `allow-stale`.

Note that even though the LUA syntax doesn't support hash tag comments `.lua` files do support a shebang
tag on the top so they can be executed on Unix systems like any shell script. LUA's `luaL_loadfile` handles
this as part of the LUA library. In the case of `luaL_loadbuffer`, which is what Redis uses, I needed to fix the
input script in case of a shebang manually. I did this the same way `luaL_loadfile` does, by replacing the
first line with a single line feed character.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Lua script doesn't return OOM when making arbitrary delete command first

2 participants