Fix broken protocol in MISCONF error, RM_Yield bugs, RM_Call(EVAL) OOM check bug, and new RM_Call checks.#10786
Merged
oranagra merged 6 commits intoredis:unstablefrom Jun 1, 2022
oranagra:RM_Call_eval_mode
Merged
Fix broken protocol in MISCONF error, RM_Yield bugs, RM_Call(EVAL) OOM check bug, and new RM_Call checks.#10786oranagra merged 6 commits intoredis:unstablefrom oranagra:RM_Call_eval_mode
oranagra merged 6 commits intoredis:unstablefrom
oranagra:RM_Call_eval_mode
Conversation
* Fix broken protocol when redis can't persist to RDB (general commands, not modules) * Fox broken protocol when Redis can't persist to AOF (modules and scripts) * Add new two more checks to RM_Call script mode. 1. READONLY You can't write against a read only replica 2. MASTERDOWN Link with MASTER is down and replica-serve-stale-data is set to 'no' * Add tests to cover varoius errors from Scripts, Modules, Modules calling scripts, and Modules caling commands in script mode. Add tests: * Looks like the MISCONF error was completely uncovered by the tests, add tests for it, including from scripts, and modules * Add tests for NOREPLICAS from scripts * Add tests for the various errors in module RM_Call, including RM_Call that calls EVAL, and RM_call in "eval mode". that includes: NOREPLICAS, READONLY, MASTERDOWN, MISCONF
* Fix bug in OOM check of EVAL scripts called from RM_Call. set the cached OOM state for scripts before executing module commands too, so that it can serve scripts that are executed by modules. i.e. in the past EVAL executed by RM_Call could have either falsely fail or falsely succeeded because of a wrong cached OOM state flag. * Fix bugs with RM_Yield: 1. SHUTDOWN should only accept the NOSAVE mode 2. Avoid eviction during yield command processing. 3. Avoid processing master client commands while yielding from another client * Add new RM_Call flag to let redis automatically refuse `deny-oom` commands while over the memory limit.
MeirShpilraien
approved these changes
May 29, 2022
MeirShpilraien
left a comment
MeirShpilraien
left a comment
There was a problem hiding this comment.
👍 Small comments (not a blocker)
oranagra
changed the title
MeirShpilraien
approved these changes
May 30, 2022
soloestoy
reviewed
May 31, 2022
soloestoy
reviewed
Jun 1, 2022
soloestoy
approved these changes
Jun 1, 2022
yossigo
approved these changes
Jun 1, 2022
oranagra
added
state:major-decision
Merged
enjoy-binbin
pushed a commit
to enjoy-binbin/redis
that referenced
this pull request
Jul 31, 2023
…M check bug, and new RM_Call checks. (redis#10786) * Fix broken protocol when redis can't persist to RDB (general commands, not modules), excessive newline. regression of redis#10372 (7.0 RC3) * Fix broken protocol when Redis can't persist to AOF (modules and scripts), missing newline. * Fix bug in OOM check of EVAL scripts called from RM_Call. set the cached OOM state for scripts before executing module commands too, so that it can serve scripts that are executed by modules. i.e. in the past EVAL executed by RM_Call could have either falsely fail or falsely succeeded because of a wrong cached OOM state flag. * Fix bugs with RM_Yield: 1. SHUTDOWN should only accept the NOSAVE mode 2. Avoid eviction during yield command processing. 3. Avoid processing master client commands while yielding from another client * Add new two more checks to RM_Call script mode. 1. READONLY You can't write against a read only replica 2. MASTERDOWN Link with MASTER is down and `replica-serve-stale-data` is set to `no` * Add new RM_Call flag to let redis automatically refuse `deny-oom` commands while over the memory limit. * Add tests to cover various errors from Scripts, Modules, Modules calling scripts, and Modules calling commands in script mode. Add tests: * Looks like the MISCONF error was completely uncovered by the tests, add tests for it, including from scripts, and modules * Add tests for NOREPLICAS from scripts * Add tests for the various errors in module RM_Call, including RM_Call that calls EVAL, and RM_call in "eval mode". that includes: NOREPLICAS, READONLY, MASTERDOWN, MISCONF
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
modules), excessive newline. regression of Add new RM_Call flags for script mode, no writes, and error replies. #10372 (7.0 RC3)
scripts), missing newline.
set the cached OOM state for scripts before executing module commands too,
so that it can serve scripts that are executed by modules.
i.e. in the past EVAL executed by RM_Call could have either falsely
fail or falsely succeeded because of a wrong cached OOM state flag.
replica-serve-stale-datais set tonodeny-oomcommandswhile over the memory limit.
calling scripts, and Modules calling commands in script mode.
Add tests:
add tests for it, including from scripts, and modules
calls EVAL, and RM_call in "eval mode". that includes:
NOREPLICAS, READONLY, MASTERDOWN, MISCONF