Skip to content

Add GetAbsExpire/SetAbsExpire for module#8564

Merged
oranagra merged 1 commit intoredis:unstablefrom
chenyang8094:abs-expire
Mar 24, 2021
Merged

Add GetAbsExpire/SetAbsExpire for module#8564
oranagra merged 1 commit intoredis:unstablefrom
chenyang8094:abs-expire

Conversation

@chenyang8094
Copy link
Contributor

@chenyang8094 chenyang8094 commented Feb 26, 2021

Similar to the SET key value [PXAT <milliseconds-timestamp>], it is often necessary to set and get the absolute expire time of the key in the module. Otherwise I can only do it by this:

long long milliseconds = expire - RedisModule_Milliseconds();  
RedisModule_SetExpire(key, milliseconds);

Now i can do it by this:

RedisModule_SetAbsExpire(key, expire);

At the same time, in the previous implementation of RM_SetExpire, expire is not forced to be a positive value, so it is possible to get a value that conflicts with REDISMODULE_NO_EXPIRE(-1) in RM_GetExpire, so I Add a check to ensure that the expire parameters in RM_SetExpire and RM_SetAbsExpire must be positive.

@soloestoy soloestoy added the state:major-decision Requires core team consensus label Mar 10, 2021
@soloestoy
Copy link
Contributor

I think the new APIs are useful, ping @redis/core-team get more suggestions.

oranagra
oranagra previously approved these changes Mar 10, 2021
Copy link
Member

@oranagra oranagra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i agree with the usefulness of this API.
i have some concern about the details, but on further thinking i think the current version may be ok.

@oranagra oranagra added release-notes indication that this issue needs to be mentioned in the release notes approval-needed Waiting for core team approval to be merged labels Mar 10, 2021
oranagra
oranagra previously approved these changes Mar 18, 2021
itamarhaber
itamarhaber previously approved these changes Mar 18, 2021
yossigo
yossigo previously approved these changes Mar 18, 2021
Add a check to ensure that the expire parameters in RM_SetExpire and RM_SetAbsExpire must be positive.
@oranagra oranagra added the state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten label Mar 21, 2021
@oranagra oranagra merged commit ccc39f6 into redis:unstable Mar 24, 2021
@oranagra oranagra mentioned this pull request Apr 19, 2021
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Sep 8, 2021
Add a check to ensure that the expire parameters in RM_SetExpire and RM_SetAbsExpire must be positive.
@oranagra oranagra mentioned this pull request Sep 21, 2022
@oranagra oranagra mentioned this pull request Dec 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval-needed Waiting for core team approval to be merged release-notes indication that this issue needs to be mentioned in the release notes state:major-decision Requires core team consensus state:to-be-merged The PR should be merged soon, even if not yet ready, this is used so that it won't be forgotten

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants