Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added RM_Call options #290

Merged
merged 6 commits into from
Mar 22, 2023
Merged

Added RM_Call options #290

merged 6 commits into from
Mar 22, 2023

Conversation

MeirShpilraien
Copy link
Collaborator

The PR added the ability to use RM_Call with different options The available options are:

  1. No writes
  2. Script mode
  3. Acl verification
  4. OOM verification
  5. Return errors as CallReply objects
  6. Replicate the command to replica and AOF
  7. Control the protocol version in which the replies will be returned

The call option can be give to call_ext function which is the same as call but also accept the CallOption object.

In addition the PR allow to get the reply as A CallReply object that wraps the Redis module API CallReply (RedisModuleCallReply*) and allow to avoid unneeded coppies and reply parsing.

Low Level Implementation details

To properly return CallReply object we need to be able to define a proper lifetime for CallReply. The main problem is that Redis makes no distinction between root call reply and inner call reply (that is reachable from the root). The module writer need to make sure he only free the root call reply (and never hold some inner call reply for a longer time then its root).

To expose those limitation we implement 2 types on CallReplies, the RootCallReply and the InnerCallReply. The RootCallReply has no life time restriction. The InnerCallReply can not outlive the RootCallReply it was retrieved from.

To make the user life easier, both, the RootCallReply and the InnerCallReply implements the CallReply trait that allows to write a generic code that will work on both.

The PR added the ability to use RM_Call with different options
The available options are:
1. No writes
2. Script mode
3. Acl verification
4. OOM verification
5. Return errors as CallReply objects
6. Replicate the command to replica and AOF
7. Control the protocol version in which the replies will be returned

The call option can be give to `call_ext` function which is the same as `call`
but also accept the `CallOption` object.

In addition the PR allow to get the reply as A CallReply object that wraps the Redis
module API CallReply (RedisModuleCallReply*) and allow to avoid unneeded coppies and
reply parsing.
@MeirShpilraien MeirShpilraien requested review from gkorland and iddm March 19, 2023 17:49
@MeirShpilraien
Copy link
Collaborator Author

Failure is because we need to upgrade the Redis version on CI

src/context/call_reply.rs Show resolved Hide resolved
src/context/call_reply.rs Outdated Show resolved Hide resolved
src/context/call_reply.rs Outdated Show resolved Hide resolved
src/context/mod.rs Outdated Show resolved Hide resolved
src/context/mod.rs Show resolved Hide resolved
src/redisvalue.rs Outdated Show resolved Hide resolved
@MeirShpilraien MeirShpilraien requested a review from iddm March 21, 2023 14:26
iddm
iddm previously approved these changes Mar 21, 2023
src/context/call_reply.rs Outdated Show resolved Hide resolved
src/context/call_reply.rs Outdated Show resolved Hide resolved
@MeirShpilraien MeirShpilraien merged commit 5b5a61a into master Mar 22, 2023
@MeirShpilraien MeirShpilraien deleted the add_rm_call_options branch March 22, 2023 11:43
MeirShpilraien pushed a commit that referenced this pull request Apr 5, 2023
The PR adds Resp3 support for both, return result from module command and getting result as resp3 on `ctx.call_ext` (when resp3 option is used).

Tests was added to verify both new functionalities.

The PR also refactor the `CallReply` API introduced on #290. Instead of having 2 types of `CallReply` we now have one type such that the root call reply created with `'static` lifetime indicating that the user can hold it for as long as it want. Internal `CallReply` can outlive its father.
As part of the refactoring, the `CallReply` was changed to be more rust friendly. `CallReply` was renamed to `CallResult = Result<CallReply, ErrorCallReply>`, `CallReply` is an enum which allows to check the `CallReply` type using a `match` statement.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants