Don't stop the dirty scripts when an OOM occurs midway through execution by enjoy-binbin · Pull Request #3030 · valkey-io/valkey

enjoy-binbin · 2026-01-08T11:50:10Z

Before #2858, we has this check:

static int scriptVerifyOOM(scriptRunCtx *run_ctx, char **err) {
    if (run_ctx->flags & SCRIPT_ALLOW_OOM) {
        /* Allow running any command even if OOM reached */
        return C_OK;
    }

    /* If we reached the memory limit configured via maxmemory, commands that
     * could enlarge the memory usage are not allowed, but only if this is the
     * first write in the context of this script, otherwise we can't stop
     * in the middle. */

    if (server.maxmemory &&                          /* Maxmemory is actually enabled. */
        !mustObeyClient(run_ctx->original_client) && /* Don't care about mem for replicas or AOF. */
        !(run_ctx->flags & SCRIPT_WRITE_DIRTY) &&    /* Script had no side effects so far. */
        server.pre_command_oom_state &&              /* Detected OOM when script start. */
        (run_ctx->c->cmd->flags & CMD_DENYOOM)) {
        *err = sdsdup(shared.oomerr->ptr);
        return C_ERR;
    }

    return C_OK;
}

If we reached the memory limit configured via maxmemory, commands that
could enlarge the memory usage are not allowed, but only if this is the
first write in the context of this script, otherwise we can't stop
in the middle.

Before valkey-io#2858, we has this check: ``` static int scriptVerifyOOM(scriptRunCtx *run_ctx, char **err) { if (run_ctx->flags & SCRIPT_ALLOW_OOM) { /* Allow running any command even if OOM reached */ return C_OK; } /* If we reached the memory limit configured via maxmemory, commands that * could enlarge the memory usage are not allowed, but only if this is the * first write in the context of this script, otherwise we can't stop * in the middle. */ if (server.maxmemory && /* Maxmemory is actually enabled. */ !mustObeyClient(run_ctx->original_client) && /* Don't care about mem for replicas or AOF. */ !(run_ctx->flags & SCRIPT_WRITE_DIRTY) && /* Script had no side effects so far. */ server.pre_command_oom_state && /* Detected OOM when script start. */ (run_ctx->c->cmd->flags & CMD_DENYOOM)) { *err = sdsdup(shared.oomerr->ptr); return C_ERR; } return C_OK; } ``` If we reached the memory limit configured via maxmemory, commands that could enlarge the memory usage are not allowed, but only if this is the first write in the context of this script, otherwise we can't stop in the middle. Signed-off-by: Binbin <binloveplay1314@qq.com>

Signed-off-by: Binbin <binloveplay1314@qq.com>

enjoy-binbin · 2026-01-08T12:09:24Z

@rjd15372 did you do this on purpose? I'm not sure if the old behavior was correct (it might cause the script to use too much memory), but we are allow to do this before.

    test "Don't stop the dirty scripts when an OOM occurs midway through execution" {
        r flushall
        r config set maxmemory 1
        r eval {
            server.call('get', KEYS[1])
            server.call('del', KEYS[1])
            server.call('set', KEYS[1], ARGV[1])
        } 1 foo bar
       assert_equal bar [r get foo]
    }

codecov · 2026-01-08T12:27:47Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.26%. Comparing base (35fdcea) to head (ff8f854).
⚠️ Report is 7 commits behind head on unstable.

Additional details and impacted files

@@             Coverage Diff              @@
##           unstable    #3030      +/-   ##
============================================
- Coverage     74.38%   74.26%   -0.12%     
============================================
  Files           129      129              
  Lines         70972    70988      +16     
============================================
- Hits          52791    52719      -72     
- Misses        18181    18269      +88

Files with missing lines	Coverage Δ
src/module.c	`26.51% <100.00%> (+0.01%)`	⬆️

... and 32 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.

Signed-off-by: Binbin <binloveplay1314@qq.com>

rjd15372 · 2026-01-12T17:24:20Z

@rjd15372 did you do this on purpose? I'm not sure if the old behavior was correct (it might cause the script to use too much memory), but we are allow to do this before.

No, this was an oversight from my part. Thanks for catching this.

rjd15372

Thanks for catching this!

madolson · 2026-01-12T22:54:46Z

@rjd15372 The tests were failing when you merged this. Ideally don't do that in the future.

enjoy-binbin · 2026-01-13T00:33:24Z

sorry, i didn't realize the external tests are failing before. I don't know how i missed it, my bad.

rjd15372 · 2026-01-13T09:55:25Z

@madolson I totally missed that tests were failing otherwise I wouldn't have merged the PR. My bad!

…ion (valkey-io#3030) Before valkey-io#2858, we has this check: ``` static int scriptVerifyOOM(scriptRunCtx *run_ctx, char **err) { if (run_ctx->flags & SCRIPT_ALLOW_OOM) { /* Allow running any command even if OOM reached */ return C_OK; } /* If we reached the memory limit configured via maxmemory, commands that * could enlarge the memory usage are not allowed, but only if this is the * first write in the context of this script, otherwise we can't stop * in the middle. */ if (server.maxmemory && /* Maxmemory is actually enabled. */ !mustObeyClient(run_ctx->original_client) && /* Don't care about mem for replicas or AOF. */ !(run_ctx->flags & SCRIPT_WRITE_DIRTY) && /* Script had no side effects so far. */ server.pre_command_oom_state && /* Detected OOM when script start. */ (run_ctx->c->cmd->flags & CMD_DENYOOM)) { *err = sdsdup(shared.oomerr->ptr); return C_ERR; } return C_OK; } ``` If we reached the memory limit configured via maxmemory, commands that could enlarge the memory usage are not allowed, but only if this is the first write in the context of this script, otherwise we can't stop in the middle. --------- Signed-off-by: Binbin <binloveplay1314@qq.com>

…ion (valkey-io#3030) Before valkey-io#2858, we has this check: ``` static int scriptVerifyOOM(scriptRunCtx *run_ctx, char **err) { if (run_ctx->flags & SCRIPT_ALLOW_OOM) { /* Allow running any command even if OOM reached */ return C_OK; } /* If we reached the memory limit configured via maxmemory, commands that * could enlarge the memory usage are not allowed, but only if this is the * first write in the context of this script, otherwise we can't stop * in the middle. */ if (server.maxmemory && /* Maxmemory is actually enabled. */ !mustObeyClient(run_ctx->original_client) && /* Don't care about mem for replicas or AOF. */ !(run_ctx->flags & SCRIPT_WRITE_DIRTY) && /* Script had no side effects so far. */ server.pre_command_oom_state && /* Detected OOM when script start. */ (run_ctx->c->cmd->flags & CMD_DENYOOM)) { *err = sdsdup(shared.oomerr->ptr); return C_ERR; } return C_OK; } ``` If we reached the memory limit configured via maxmemory, commands that could enlarge the memory usage are not allowed, but only if this is the first write in the context of this script, otherwise we can't stop in the middle. --------- Signed-off-by: Binbin <binloveplay1314@qq.com> Signed-off-by: Harkrishn Patro <bunty.hari@gmail.com>

github-actions Bot assigned enjoy-binbin Jan 8, 2026

enjoy-binbin requested a review from rjd15372 January 8, 2026 11:50

Add test

8c538df

Signed-off-by: Binbin <binloveplay1314@qq.com>

enjoy-binbin added this to Valkey 9.1 Jan 8, 2026

sarthakaggarwal97 reviewed Jan 10, 2026

View reviewed changes

Comment thread appendonly Outdated

Remove tmp file

ff8f854

Signed-off-by: Binbin <binloveplay1314@qq.com>

sarthakaggarwal97 approved these changes Jan 10, 2026

View reviewed changes

rjd15372 approved these changes Jan 12, 2026

View reviewed changes

rjd15372 merged commit f0d9810 into valkey-io:unstable Jan 12, 2026
21 of 24 checks passed

github-project-automation Bot moved this to Done in Valkey 9.1 Jan 12, 2026

enjoy-binbin deleted the dirty_scripts branch January 12, 2026 17:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Don't stop the dirty scripts when an OOM occurs midway through execution#3030

Don't stop the dirty scripts when an OOM occurs midway through execution#3030
rjd15372 merged 3 commits into
valkey-io:unstablefrom
enjoy-binbin:dirty_scripts

enjoy-binbin commented Jan 8, 2026

Uh oh!

enjoy-binbin commented Jan 8, 2026

Uh oh!

codecov Bot commented Jan 8, 2026 •

edited

Loading

Uh oh!

Uh oh!

rjd15372 commented Jan 12, 2026

Uh oh!

rjd15372 left a comment

Uh oh!

Uh oh!

madolson commented Jan 12, 2026

Uh oh!

enjoy-binbin commented Jan 13, 2026

Uh oh!

rjd15372 commented Jan 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

enjoy-binbin commented Jan 8, 2026

Uh oh!

enjoy-binbin commented Jan 8, 2026

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

rjd15372 commented Jan 12, 2026

Uh oh!

rjd15372 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

madolson commented Jan 12, 2026

Uh oh!

enjoy-binbin commented Jan 13, 2026

Uh oh!

rjd15372 commented Jan 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Jan 8, 2026 •

edited

Loading