Fix IDMP cron expiration not working after RDB load#14869
Merged
sggeorgiev merged 4 commits intoredis:unstablefrom Mar 13, 2026
Merged
Fix IDMP cron expiration not working after RDB load#14869sggeorgiev merged 4 commits intoredis:unstablefrom
sggeorgiev merged 4 commits intoredis:unstablefrom
Conversation
sundb
added
the
release-notes
sundb
reviewed
Mar 10, 2026
Collaborator
|
one test failed: https://github.com/sundb/redis/actions/runs/22950671118/job/66614240728 |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Collaborator
|
test |
sundb
approved these changes
Mar 13, 2026
YaacovHazan
pushed a commit
to YaacovHazan/redis
that referenced
this pull request
Mar 19, 2026
**Summary** Registers streams with IDMP producers in `db->stream_idmp_keys` during RDB load, so that the periodic cron job can expire stale IDMP entries after a server restart. Without this fix, streams loaded from RDB were never registered, causing IDMP entries to accumulate indefinitely and never be cleaned up by the cron. **Changes** - **`rdbLoadRioWithLoadingCtx` (src/rdb.c):** After loading each key-value pair, added a check for `OBJ_STREAM` type with a non-NULL `idmp_producers` rax. When found, creates a string object for the key and inserts it into `db->stream_idmp_keys` via `dictAddRaw`. If the key already exists (duplicate insert), the reference is decremented to avoid a leak. - **Test `XADD IDMP cron expiration works after RDB load` (tests/unit/type/stream.tcl):** Added an integration test that creates a stream with IDMP producers, sets a short `IDMP-DURATION`, saves and restarts the server, then verifies that the cron eventually expires the entries (pids-tracked and iids-tracked drop to 0) and that previously-expired IIDs can be re-added as new entries. **Benefits** - **Cron expiration works after restart:** Streams with IDMP producers are now discoverable by the periodic cleanup cron after an RDB load, ensuring expired entries are reaped as expected. - **No memory leak on stale entries:** Without registration, IDMP entries that outlived their duration would persist in memory forever after a restart, growing unboundedly. This fix ensures they are cleaned up. - **Consistency between runtime and post-load behavior:** IDMP expiration now behaves identically whether the stream was created at runtime or restored from an RDB snapshot.
Merged
YaacovHazan
pushed a commit
that referenced
this pull request
Mar 23, 2026
**Summary** Registers streams with IDMP producers in `db->stream_idmp_keys` during RDB load, so that the periodic cron job can expire stale IDMP entries after a server restart. Without this fix, streams loaded from RDB were never registered, causing IDMP entries to accumulate indefinitely and never be cleaned up by the cron. **Changes** - **`rdbLoadRioWithLoadingCtx` (src/rdb.c):** After loading each key-value pair, added a check for `OBJ_STREAM` type with a non-NULL `idmp_producers` rax. When found, creates a string object for the key and inserts it into `db->stream_idmp_keys` via `dictAddRaw`. If the key already exists (duplicate insert), the reference is decremented to avoid a leak. - **Test `XADD IDMP cron expiration works after RDB load` (tests/unit/type/stream.tcl):** Added an integration test that creates a stream with IDMP producers, sets a short `IDMP-DURATION`, saves and restarts the server, then verifies that the cron eventually expires the entries (pids-tracked and iids-tracked drop to 0) and that previously-expired IIDs can be re-added as new entries. **Benefits** - **Cron expiration works after restart:** Streams with IDMP producers are now discoverable by the periodic cleanup cron after an RDB load, ensuring expired entries are reaped as expected. - **No memory leak on stale entries:** Without registration, IDMP entries that outlived their duration would persist in memory forever after a restart, growing unboundedly. This fix ensures they are cleaned up. - **Consistency between runtime and post-load behavior:** IDMP expiration now behaves identically whether the stream was created at runtime or restored from an RDB snapshot.
Merged
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
3 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.
Summary
Registers streams with IDMP producers in
db->stream_idmp_keysduring RDB load, so that the periodic cron job can expire stale IDMP entries after a server restart. Without this fix, streams loaded from RDB were never registered, causing IDMP entries to accumulate indefinitely and never be cleaned up by the cron.Changes
rdbLoadRioWithLoadingCtx(src/rdb.c): After loading each key-value pair, added a check forOBJ_STREAMtype with a non-NULLidmp_producersrax. When found, creates a string object for the key and inserts it intodb->stream_idmp_keysviadictAddRaw. If the key already exists (duplicate insert), the reference is decremented to avoid a leak.XADD IDMP cron expiration works after RDB load(tests/unit/type/stream.tcl): Added an integration test that creates a stream with IDMP producers, sets a shortIDMP-DURATION, saves and restarts the server, then verifies that the cron eventually expires the entries (pids-tracked and iids-tracked drop to 0) and that previously-expired IIDs can be re-added as new entries.Benefits
Note
Medium Risk
Touches the RDB loading path and stream bookkeeping, so mistakes could cause missed IDMP cleanup or reference-count/key-tracking issues after restart, but the change is small and covered by a new restart/cron integration test.
Overview
Fixes a restart-only bug where streams using IDMP could stop participating in the cron-based cleanup after being loaded from RDB.
During RDB load, streams that have
idmp_producersare now added todb->stream_idmp_keys(with duplicate-safe refcount handling) so the periodic cron can expire and remove stale IDMP entries post-restart. Adds a unit test that saves/restarts, waits forXINFO STREAMtracked counts to drop to 0, and verifies expired IIDs can be added again.Written by Cursor Bugbot for commit 0383f4f. This will update automatically on new commits. Configure here.