Support delay trimming slots after finishing migrating slots by ShooterIT · Pull Request #14567 · redis/redis

ShooterIT · 2025-11-25T09:04:25Z

This PR introduces a mechanism that allows a module to temporarily disable trimming after an ASM migration operation so it can safely finish ongoing asynchronous jobs that depend on keys in migrating (and about to be trimmed) slots.

ClusterDisableTrim/ClusterEnableTrim
We introduce ClusterDisableTrim/ClusterEnableTrim Module APIs to allow module to disable/enable slot migration
```
/* Disable automatic slot trimming. */
int RM_ClusterDisableTrim(RedisModuleCtx *ctx)

/* Enable automatic slot trimming */
int RM_ClusterEnableTrim(RedisModuleCtx *ctx)
```
Please notice: Redis will not start any subsequent import or migrate ASM operations while slot trimming is disabled, so modules must re-enable trimming immediately after completing their pending work.

The only valid and meaningful time for a module to disable trimming appears to be after the MIGRATE_COMPLETED event.
REDISMODULE_OPEN_KEY_ACCESS_TRIMMED
Added REDISMODULE_OPEN_KEY_ACCESS_TRIMMED to RM_OpenKey() so that module can operate with these keys in the unowned slots after trim is paused.

And now we don't delete the key if it is in trim job when we access it. And expireIfNeeded returns KEY_VALID if EXPIRE_ALLOW_ACCESS_TRIMMED is set, otherwise, returns KEY_TRIMMED without deleting key.
REDISMODULE_CTX_FLAGS_TRIM_IN_PROGRESS
We also extend RM_GetContextFlags() to include a flag REDISMODULE_CTX_FLAGS_TRIM_IN_PROGRESS indicating whether a trimming job is pending (due to trim pause) or in progress. Modules could periodically poll this flag to synchronize their internal state, e.g., if a trim job was delayed or if the module incorrectly assumed trimming was still active.

Bugfix: RM_SetClusterFlags could not clear a flag after enabling it first

src/db.c

src/cluster_asm.c

tezc · 2025-11-25T13:19:31Z

src/cluster_asm.c

+        /* On migrating side, the replica is unaware of the slot migration, so it
+         * doesn't know when the migration is completed (start having unowned keys),
+         * We delay to fire the event until the TRIMSLOTS is propagated to the replica.
+         * Maybe optimize it in the future. */


Just thinking out loud, let's say migrating side replica becomes master before receiving TRIMSLOTS.

If it happens after replica receives config update via bus, then the new master will be unaware of unowned keys.

If it happens before replica receives config update via bus, then, on the config update, the new master will do a blocking trim as there is no asm task to associate.

I assume we don't have these issues on importing side. Also, I wonder what we can do to improve these two.

src/module.c

ShooterIT · 2025-12-01T12:40:02Z

Hi @oranagra This PR introduce two APIs:

/* Acquire a server capability that changes the normal behavior of Redis.
 * Capabilities are reference-counted: a capability may be acquired
 * multiple times concurrently by multiple modules.
 *
 * Currently supported capabilities:
 *   - REDISMODULE_SERVER_CAPA_NO_TRIM:
 *       Prevent the server from trimming keys after atomic slot migration.
 *
 * Returns REDISMODULE_OK on success, REDISMODULE_ERR otherwise. */
int RM_AcquireServerCapability(RedisModuleCtx *ctx, int capa) 

/* Release a previously acquired server capability.
 *
 * This function must be called once for each successful call to
 * RM_AcquireServerCapability() with the same capability. The underlying
 * behavior is restored when the reference count for that capability
 * reaches zero.
 *
 * Returns REDISMODULE_OK on success, REDISMODULE_ERR otherwise. */
int RM_ReleaseServerCapability(RedisModuleCtx *ctx, int capa)

RedisModule_AcquireServerCapability acquires a server capability that alters the default behavior of the Redis server. Capabilities are reference-counted: the same capability may be acquired multiple times concurrently by multiple modules.

RedisModule_ReleaseServerCapability releases a previously acquired capability. The underlying behavior is restored only when the last reference to that capability is released.

At the moment, the only supported capability is CLUSTER_MODULE_POLICY_NO_TRIM, which prevents key trimming after atomic slot migration.

These APIs are intentionally generic so that additional server capabilities can be introduced in the future, instead of providing dedicated functions such as disableTrim and enableTrim.

Do you think these APIs make sense?

oranagra · 2025-12-01T15:28:58Z

LGTM except for the terminology. both the "acquire" and more importantly the "capability" terms don't seem good to me.
maybe the key focus is on this (taken from the doc comment): "changes the normal behavior"
how about it could be "DisableServerMechanism" or alike, but i suppose in some cases it'll be to enable something that's disabled by default.
so maybe "ChangeServerBehavior"?
or maybe we're working too hard to prepare for the future, and we should just add an explicit API for just this mechanism (i.e. RM_DisableReshardTriming"

ShooterIT · 2025-12-02T01:36:12Z

Thank you @oranagra

we're working too hard to prepare for the future

yes, I had a long discussion with chatpgt, and did not get satisfactory conclusion, the reason why I use acquire and release is that we use a counter to describe it.

maybe a dedicated API is not a bad idea, and I even prefer it, The word “reshard” may be confused with our internal resharding approach, how about RM_DisableSlotMigrationTrimming ?

and WDYT? @tezc @JoanFM

tezc · 2025-12-02T04:49:47Z

I was the one mainly insisted on generic API as I thought we could need something similar in future. Though, now I feel like it causes more confusion and each one of us has different taste :) So, I vote for dedicated API as well. I think all of the cluster functions include cluster in it, maybe we can all it something like RM_ClusterDisableTrim()? I don't have a strong opinion though.

JoanFM · 2025-12-02T07:34:35Z

Thank you @oranagra

we're working too hard to prepare for the future

yes, I had a long discussion with chatpgt, and did not get satisfactory conclusion, the reason why I use acquire and release is that we use a counter to describe it.

maybe a dedicated API is not a bad idea, and I even prefer it, The word “reshard” may be confused with our internal resharding approach, how about RM_DisableSlotMigrationTrimming ?

and WDYT? @tezc @JoanFM

So I would use whatever, but I agree that this generic API name can feel a little confusing and an overkill trying to generalize for now.

So dedicated API seems best to me

ShooterIT · 2025-12-02T08:00:32Z

Thank you all, let's use dedicated APIs, RM_ClusterDisableTrim looks good to me

src/redismodule.h

src/cluster_asm.c

src/module.c

src/redismodule.h

tests/modules/atomicslotmigration.c

src/redismodule.h

src/cluster_asm.c

oranagra

API LGTM

src/module.c

This PR introduces a mechanism that allows a module to temporarily disable trimming after an ASM migration operation so it can safely finish ongoing asynchronous jobs that depend on keys in migrating (and about to be trimmed) slots. 1. **ClusterDisableTrim/ClusterEnableTrim** We introduce `ClusterDisableTrim/ClusterEnableTrim` Module APIs to allow module to disable/enable slot migration ``` /* Disable automatic slot trimming. */ int RM_ClusterDisableTrim(RedisModuleCtx *ctx) /* Enable automatic slot trimming */ int RM_ClusterEnableTrim(RedisModuleCtx *ctx) ``` **Please notice**: Redis will not start any subsequent import or migrate ASM operations while slot trimming is disabled, so modules must re-enable trimming immediately after completing their pending work. The only valid and meaningful time for a module to disable trimming appears to be after the MIGRATE_COMPLETED event. 2. **REDISMODULE_OPEN_KEY_ACCESS_TRIMMED** Added REDISMODULE_OPEN_KEY_ACCESS_TRIMMED to RM_OpenKey() so that module can operate with these keys in the unowned slots after trim is paused. And now we don't delete the key if it is in trim job when we access it. And `expireIfNeeded` returns `KEY_VALID` if `EXPIRE_ALLOW_ACCESS_TRIMMED` is set, otherwise, returns `KEY_TRIMMED` without deleting key. 3. **REDISMODULE_CTX_FLAGS_TRIM_IN_PROGRESS** We also extend RM_GetContextFlags() to include a flag REDISMODULE_CTX_FLAGS_TRIM_IN_PROGRESS indicating whether a trimming job is pending (due to trim pause) or in progress. Modules could periodically poll this flag to synchronize their internal state, e.g., if a trim job was delayed or if the module incorrectly assumed trimming was still active. Bugfix: RM_SetClusterFlags could not clear a flag after enabling it first. --------- Co-authored-by: Ozan Tezcan <[email protected]>

tezc and others added 7 commits November 13, 2025 01:02

Allow modules to delay trimming

885b002

Allow modules to delay trimming

378031e

introduce unownedkeys event

826be79

add tests

8085ddc

add tests

401ed12

fix tests

7e12887

revert change due to tests

9143f7d

tezc reviewed Nov 25, 2025

View reviewed changes

src/db.c Show resolved Hide resolved

tezc reviewed Nov 25, 2025

View reviewed changes

src/cluster_asm.c Outdated Show resolved Hide resolved

comments

3ea7b73

tezc reviewed Nov 25, 2025

View reviewed changes

ShooterIT added 3 commits November 26, 2025 09:07

Merge branch 'upstream/unstable' into delay-trim2

11f7332

comments

72cd22c

Merge branch 'unstable' into delay-trim2

1833f4b

ShooterIT commented Nov 26, 2025

View reviewed changes

src/module.c Outdated Show resolved Hide resolved

ShooterIT changed the title ~~Trim delay~~ Support delay trimming slots after finishing migrating slots Nov 27, 2025

JoanFM reviewed Nov 27, 2025

View reviewed changes

src/module.c Outdated Show resolved Hide resolved

Support disabling trim multi times

2b6e603

tezc reviewed Nov 28, 2025

View reviewed changes

src/module.c Outdated Show resolved Hide resolved

JoanFM mentioned this pull request Nov 28, 2025

align redismodule.h with Redis Delay Trimming feature RedisLabsModules/RedisModulesSDK#79

Merged

Fix comments

7ce9e73

JoanFM mentioned this pull request Dec 1, 2025

[MOD-12171] [MOD-12169] Implement ASM-aware search flow RediSearch/RediSearch#7343

Merged

Update comments

e1b5fea

ShooterIT added the release-notes indication that this issue needs to be mentioned in the release notes label Dec 1, 2025

dedicated APIs

1d5d0e9

JoanFM reviewed Dec 2, 2025

View reviewed changes

src/redismodule.h Outdated Show resolved Hide resolved

remove out-of-date code

9971db7

galcohen-redislabs reviewed Dec 14, 2025

View reviewed changes

ShooterIT added 3 commits December 15, 2025 19:50

fix comments

fec54e8

fix wrong revert

e24af1c

fix revert

f343e3f

ShooterIT requested a review from tezc December 15, 2025 13:24

tezc reviewed Dec 15, 2025

View reviewed changes

src/redismodule.h Outdated Show resolved Hide resolved

tezc reviewed Dec 15, 2025

View reviewed changes

src/cluster_asm.c Outdated Show resolved Hide resolved

fix comments

a1508c1

tezc approved these changes Dec 16, 2025

View reviewed changes

add comments

bd22b3b

oranagra reviewed Dec 16, 2025

View reviewed changes

src/module.c Show resolved Hide resolved

fix comments

0a409da

ShooterIT merged commit 33391a7 into redis:unstable Dec 16, 2025
19 checks passed

ShooterIT deleted the trim-delay branch December 16, 2025 08:31

sundb added this to Redis 8.6 Dec 17, 2025

sundb moved this to Done in Redis 8.6 Dec 17, 2025

This was referenced Dec 30, 2025

[CLEAN] Synthetic Benchmark PR #14567 - Support delay trimming slots after finishing migrating slots qodo-benchmark/redis#1

Open

[CORRUPTED] Synthetic Benchmark PR #14567 - Support delay trimming slots after finishing migrating slots qodo-benchmark/redis#2

Open

This was referenced Jan 13, 2026

[CLEAN] Synthetic Benchmark PR #14567 - Support delay trimming slots after finishing migrating slots qodo-benchmark/redis#21

Open

[CORRUPTED] Synthetic Benchmark PR #14567 - Support delay trimming slots after finishing migrating slots qodo-benchmark/redis#22

Open

Conversation

ShooterIT commented Nov 25, 2025 • edited by tezc Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tezc Nov 25, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ShooterIT commented Dec 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oranagra commented Dec 1, 2025

Uh oh!

ShooterIT commented Dec 2, 2025

Uh oh!

tezc commented Dec 2, 2025

Uh oh!

JoanFM commented Dec 2, 2025

Uh oh!

ShooterIT commented Dec 2, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

oranagra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ShooterIT commented Nov 25, 2025 •

edited by tezc

Loading

ShooterIT commented Dec 1, 2025 •

edited

Loading