Add new OBJ_GCRA type by minchopaskal · Pull Request #14905 · redis/redis

minchopaskal · 2026-03-20T14:50:18Z

PR introduced a new rate limiting command which stores its internal implementation-detail data into a string key.

Since this will prevent a client from detecting type errors or accidental overwrites or value invalidations, f.e via SET or INCR this PR introduces a new data type - OBJ_GCRA specifically created for that new command.

OBJ_GCRA name subject to change.

Note

High Risk
Adds a new core object type with new RDB encoding and AOF/replication rewrite behavior, which can impact persistence compatibility and data consistency if mishandled. Touches multiple critical subsystems (object lifecycle, copy/defrag, modules, keyspace notifications, command table).

Overview
Introduces a dedicated OBJ_GCRA value type for the GCRA rate-limiting command, replacing the previous string-backed storage so type errors/overwrites are detectable and handled consistently.

Adds full core support for the new type across persistence and runtime internals: RDB save/load via new RDB_TYPE_GCRA, AOF rewrite support, dataset copy/dup, memory sizing, defrag, and DEBUG DIGEST handling. Replication/AOF propagation for GCRA is changed to rewrite into a new internal command GCRASETVALUE, which also gets command-table/JSON docs, its own ACL category/group (rate_limit), and new keyspace notification class r (NOTIFY_RATE_LIMIT).

Updates tests to cover RDB reload, DUMP/RESTORE, AOF rewrite, digest stability, and replication behavior for GCRA keys.

^{Reviewed by Cursor Bugbot for commit 17b1f70. Bugbot is set up for automated code reviews on this repo. Configure here.}

minchopaskal · 2026-03-20T14:54:38Z

@sundb we had talked with @oranagra about this and generally agreed that a new data type sounds reasonable, instead of abusing a string object. That said the amount of new code for that specific use case seems an overkill to me.

Another thing that came up while writing it - if we feel very strongly that a new type should be introduced - maybe it could be more generic one. GCRA stores a unix timestamp (in this case in microseconds). The new data type could be OBJ_TIMESTAMP or something like this. Not sure how useful would that be though.

WDYT?

sundb · 2026-03-21T09:01:36Z

Another thing that came up while writing it - if we feel very strongly that a new type should be introduced - maybe it could be more generic one. GCRA stores a unix timestamp (in this case in microseconds). The new data type could be OBJ_TIMESTAMP or something like this. Not sure how useful would that be though.

I think long long * is sufficient. OBJ_TIMESTAMP is theoretically impossible to be used elsewhere in the future.
I didn't see you modify the GCRA command accordingly. Is it not done yet?

oranagra

i skimmed over the file (except for gcra.c and gcra.tcl).
is is a lot of changes, and maybe we can find a way to make some of them in a way that the next time we add a simple type we'll have a little less (few of them may be more generic?).

anyway, despite being a lot of changes, if we do want WRONGTYPE error when people mix keys, then it is what it is.. there's no way around it.

YaacovHazan · 2026-03-22T19:14:24Z

@minchopaskal, as Oran wrote, let's try to see if we can make some of the changes more generic.
In any case, we should document all the required changes for adding a new data type in one place.

minchopaskal · 2026-03-23T09:11:34Z

Another thing that came up while writing it - if we feel very strongly that a new type should be introduced - maybe it could be more generic one. GCRA stores a unix timestamp (in this case in microseconds). The new data type could be OBJ_TIMESTAMP or something like this. Not sure how useful would that be though.

I think long long * is sufficient. OBJ_TIMESTAMP is theoretically impossible to be used elsewhere in the future. I didn't see you modify the GCRA command accordingly. Is it not done yet?

It is modified with the new type already.

augmentcode · 2026-03-30T08:54:35Z

🤖 Augment PR Summary

Summary: Introduces a dedicated Redis object type (OBJ_GCRA) to store the GCRA rate-limiter’s internal TAT state, avoiding accidental overwrites and enabling proper type errors.

Changes:

Adds OBJ_GCRA plus a new encoding (OBJ_ENCODING_PTRINT) for 32-bit builds.
Updates the GCRA command to store/read the new object type and changes its keyspace notification event.
Adds an internal GCRASETTAT command used for replication/AOF rewrite to record TAT deterministically.
Implements persistence support: RDB save/load (RDB_TYPE_GCRA) and AOF rewrite serialization.
Extends core subsystems to understand the new type (copy, digest/debug, defrag, modules keytype, memory introspection).
Adds unit tests covering RDB reload, DUMP/RESTORE, AOF rewrite, replication behavior, and DEBUG DIGEST stability.

Technical notes: Uses pointer-int storage on 64-bit (OBJ_ENCODING_INT) and heap allocation on 32-bit (OBJ_ENCODING_PTRINT) to safely represent microsecond timestamps.

_{🤖 Was this summary useful? React with 👍 or 👎}

augmentcode

Review completed. 5 suggestions posted.

Comment augment review to trigger a new review at any time.

oranagra

some random comments.

rename the utililty command gcrarecord to gcraSetTAT

Co-authored-by: debing.sun <[email protected]>

sundb · 2026-04-15T14:07:46Z

i don't see GCRASETVALUE was mentioned in the top comment, pls udpate the top comment, and check if anything missed.
like new notification type, new rdb type.

cursor

Cursor Bugbot has reviewed your changes and found 2 potential issues.

^{Reviewed by Cursor Bugbot for commit 17b1f70. Configure here.}

ShooterIT · 2026-04-16T02:44:06Z

#define NOTIFY_RATE_LIMIT (1<<18) /* r, notify rate limit event (Note: excluded from NOTIFY_ALL)*/

Hi @minchopaskal why do we exclude RATE_LIMIT from NOTIFY_ALL, it is related to normal data type OBJ_GCRA.

minchopaskal · 2026-04-16T07:32:16Z

@ShooterIT

#define NOTIFY_RATE_LIMIT (1<<18) /* r, notify rate limit event (Note: excluded from NOTIFY_ALL)*/

Hi @minchopaskal why do we exclude RATE_LIMIT from NOTIFY_ALL, it is related to normal data type OBJ_GCRA.

backwards compatibility? I rmbr I did the same for OVERWRITTEN/TYPE_CHANGED KSN, i.e if you upgrade redis you dont get some unexpected new notification type. But TBH I didn't give it much thought.
@sundb WDYT?

sundb · 2026-04-16T07:40:28Z

backwards compatibility? I rmbr I did the same for OVERWRITTEN/TYPE_CHANGED KSN, i.e if you upgrade redis you dont get some unexpected new notification type. But TBH I didn't give it much thought. @sundb WDYT?

I don't know the exact reason either, but I think you're right about that.

oranagra · 2026-04-21T07:40:42Z

backwards compatibility? I rmbr I did the same for OVERWRITTEN/TYPE_CHANGED KSN, i.e if you upgrade redis you dont get some unexpected new notification type. But TBH I didn't give it much thought. @sundb WDYT?

i think that for KEYMISS, CHANGED, OVERWRITTEN it's a different story, because adding these notifications will affect users after upgrade even if they didn't change the app code.
but in case of CGRA, nothing will happen as long as it's unused.
so same as we added stream type in redis 5, we can add rate limit in 8.8

minchopaskal · 2026-04-21T12:57:30Z

Ok makes sense. I reasoned you may upgrade to newer redis version but still not expect the new KSN. @oranagra so we add GCRA to NOTIFY_ALL?

oranagra · 2026-04-23T12:28:37Z

IMHO yes.
the new KSN won't be present for existing applications.

After introducing GCRA algorithm into redis #14826 and subsequent introduction of new RATE_LIMIT object type - #14905. It was internally decided not to introduce GCRA into the new release. As still no decision is made on whether it will be kept or not in the future, this PR only makes the code related to GCRA dead - commands are inaccessible and AOF/RDB load+save is disabled. --------- Co-authored-by: debing.sun <[email protected]>

minchopaskal commented Mar 20, 2026

View reviewed changes

Comment thread src/commands/gcrarecord.json Outdated

minchopaskal commented Mar 20, 2026

View reviewed changes

Comment thread src/commands/gcrasetvalue.json

minchopaskal requested review from oranagra and sundb March 20, 2026 14:54

oranagra reviewed Mar 22, 2026

View reviewed changes

Comment thread src/commands/gcrarecord.json Outdated

Comment thread src/object.c

Comment thread src/commands/gcrarecord.json Outdated

Comment thread src/rdb.h Outdated

minchopaskal commented Mar 23, 2026

View reviewed changes

Comment thread src/object.c

oranagra reviewed Mar 23, 2026

View reviewed changes

Comment thread src/object.c

oranagra reviewed Mar 24, 2026

View reviewed changes

Comment thread src/object.c Outdated

minchopaskal marked this pull request as ready for review March 30, 2026 08:49

augmentcode Bot reviewed Mar 30, 2026

View reviewed changes

Comment thread src/server.h

Comment thread src/object.h Outdated

Comment thread src/rdb.c

Comment thread src/gcra.c

Comment thread src/object.c Outdated

cursor Bot reviewed Mar 30, 2026

View reviewed changes

Comment thread src/server.h

Comment thread src/rdb.c

Comment thread src/rdb.c Outdated

Comment thread src/debug.c Outdated

Comment thread src/object.h Outdated

cursor Bot reviewed Mar 30, 2026

View reviewed changes

Comment thread src/gcra.c

minchopaskal force-pushed the new-dt-for-gcra branch from 984f06d to 0f3be25 Compare March 31, 2026 11:14

oranagra reviewed Apr 1, 2026

View reviewed changes

Comment thread src/commands/gcrasettat.json Outdated

Comment thread src/gcra.c Outdated

Comment thread src/gcra.c Outdated

Comment thread src/gcra.c

Comment thread src/gcra.c Outdated

Comment thread src/object.c

Comment thread src/object.c Outdated

cursor Bot reviewed Apr 1, 2026

View reviewed changes

Comment thread src/redismodule.h

Comment thread src/object.c

Comment thread src/debug.c Outdated

sundb reviewed Apr 2, 2026

View reviewed changes

Comment thread src/debug.c Outdated

Comment thread src/debug.c Outdated

Comment thread src/defrag.c

Comment thread src/rdb.c Outdated

oranagra reviewed Apr 5, 2026

View reviewed changes

Comment thread src/redismodule.h

Comment thread src/notify.c

minchopaskal added 8 commits April 7, 2026 16:27

Add new OBJ_GCRA type

b31a7f8

Add integer encoding for gcra type. Add RATE_LIMIT ACL cat

e5e1373

rename the utililty command gcrarecord to gcraSetTAT

fix 32-bit build and derag syntax err for linux

a42e91b

fix 32bit build again

aaf94ec

Change RAW enc of GCRA to PTRINT

e898f8d

add comments

c885da2

fix 32-bit build

74cea68

fixes

417d285

minchopaskal and others added 2 commits April 9, 2026 11:18

style fix

8f48f96

Co-authored-by: debing.sun <[email protected]>

small refactor

deb9456

Co-authored-by: debing.sun <[email protected]>

sundb reviewed Apr 15, 2026

View reviewed changes

Comment thread src/object.c Outdated

Comment thread src/rdb.c Outdated

sundb reviewed Apr 15, 2026

View reviewed changes

Comment thread src/debug.c Outdated

minchopaskal and others added 3 commits April 15, 2026 16:10

Update src/rdb.c

b19885f

Co-authored-by: debing.sun <[email protected]>

remove unneeded check

8b90fef

assert in getLongLongFromGCRAObject when value is negative

30d4190

cursor Bot reviewed Apr 15, 2026

View reviewed changes

Comment thread src/debug.c

Comment thread src/gcra.c

sundb reviewed Apr 15, 2026

View reviewed changes

Comment thread src/gcra.c Outdated

sundb added release-notes indication that this issue needs to be mentioned in the release notes state:needs-doc-pr requires a PR to redis-doc repository labels Apr 15, 2026

sundb added this to Redis 8.8 Apr 15, 2026

github-project-automation Bot moved this to Todo in Redis 8.8 Apr 15, 2026

add check for negative value in gcrasetvalue

78e4813

sundb approved these changes Apr 15, 2026

View reviewed changes

fix check for debug print

17b1f70

cursor Bot reviewed Apr 15, 2026

View reviewed changes

Comment thread src/debug.c

Comment thread src/gcra.c

minchopaskal merged commit 3bcfbbe into redis:unstable Apr 15, 2026
19 checks passed

github-project-automation Bot moved this from Todo to Done in Redis 8.8 Apr 15, 2026

fcostaoliveira mentioned this pull request Apr 15, 2026

Add batched prefetch for HGETALL on hashtable encoding #14988

Merged

tristantarrant mentioned this pull request Apr 16, 2026

Implement Redis 8.8 GCRA rate limiter command infinispan/infinispan#17274

Open

minchopaskal mentioned this pull request May 12, 2026

Hold GCRA out of the release #15191

Merged

Uh oh!

Conversation

minchopaskal commented Mar 20, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

minchopaskal commented Mar 20, 2026

Uh oh!

sundb commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oranagra left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

YaacovHazan commented Mar 22, 2026

Uh oh!

minchopaskal commented Mar 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

augmentcode Bot commented Mar 30, 2026

Uh oh!

augmentcode Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sundb commented Apr 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ShooterIT commented Apr 16, 2026

Uh oh!

minchopaskal commented Apr 16, 2026

Uh oh!

minchopaskal commented Mar 20, 2026 •

edited by cursor Bot

Loading

sundb commented Mar 21, 2026 •

edited

Loading

sundb commented Apr 15, 2026 •

edited

Loading

oranagra commented Apr 21, 2026 •

edited

Loading