Added support for defrag API.#387
Merged
Merged
Conversation
The PR adds support for defrag API. The PR introduce a new struct, `DefragContext`, which provide a safe API over `*mut raw::RedisModuleDefragCtx`. **Notice** that we do expose an unsafe API to create `DefragContext` from `*mut raw::RedisModuleDefragCtx`. This is because we do not have a safe API for datatype registeration. User must register an unsafe function as the defrag callback of the datatype and create the `DefragContext` from `*mut raw::RedisModuleDefragCtx`. We should consider adding a safe API for datatype creation. In addition, the PR introduce 3 new proc macros: * defrag_function - allows to register a function to defrag global data * defrag_start_function - allows to register a function to defrag global data when defrag cycle starts. * defrag_end_function - allows to register a function to defrag global data when defrag cycle ends. Example and test for the new API were added.
MeirShpilraien
pushed a commit
to RedisJSON/RedisJSON
that referenced
this pull request
Sep 3, 2024
The PR adds support for active defrag on RedisJSON. A pre condition for this PR is that the following PR's will be megred: * [Redis defrad module API extentions](redis/redis#13509) * [redismodule-rs support for active defrag API](RedisLabsModules/redismodule-rs#387) * [IJSON support for defrag](RedisJSON/ijson#1) The PR register defrag function on the json datatype and uses the new capability of ISON to defrag the key. **Notice**: * that increamental defrag of the json key is **not** support. In order to support it we need to implement the free_effort callback. This is not trivial and even if it was it would have cause the json object to potentially be freed when the GIL is not hold, which violate the assumption of our shared string implementation (it is not thread safe). We leave it for future improvment. * If we run on a Redis version that do not support the defrag start callback, we can still partially support defrag. In that case the IJSON object will be defraged but the shared strings dictionaty will not be reinitialize. This basically means that shared strings will not be defraged. Tests were added to cover the new functionality.
1 task
ephraimfeldblum
previously approved these changes
Sep 3, 2024
ephraimfeldblum
previously approved these changes
Sep 4, 2024
iddm
reviewed
Sep 4, 2024
| }); | ||
| } | ||
|
|
||
| pub fn register_defrag_functions(ctx: &Context) -> Result<(), RedisError> { |
Contributor
There was a problem hiding this comment.
Forgotten documentation for this public function. :-)
Author
There was a problem hiding this comment.
Its public because it is use from the macro, but sure will add a comment.
| #[test] | ||
| fn test_defrag() -> Result<()> { | ||
| let port: u16 = 6503; | ||
| let _guards = vec
YaacovHazan
approved these changes
Sep 5, 2024
MeirShpilraien
pushed a commit
to RedisJSON/RedisJSON
that referenced
this pull request
Sep 10, 2024
The PR adds support for active defrag on RedisJSON. A pre condition for this PR is that the following PR's will be megred: * [Redis defrad module API extentions](redis/redis#13509) * [redismodule-rs support for active defrag API](RedisLabsModules/redismodule-rs#387) * [IJSON support for defrag](RedisJSON/ijson#1) The PR register defrag function on the json datatype and uses the new capability of ISON to defrag the key. **Notice**: * Increamental defrag of the json key is **not** support. In order to support it we need to implement the free_effort callback. This is not trivial and even if it was it would have cause the json object to potentially be freed when the GIL is not hold, which violate the assumption of our shared string implementation (it is not thread safe). We leave it for future improvment. * If we run on a Redis version that do not support the defrag start callback, we can still partially support defrag. In that case the IJSON object will be defraged but the shared strings dictionatrywill not be reinitialize. This basically means that shared strings will not be defraged. Tests were added to cover the new functionality.
MeirShpilraien
pushed a commit
to RedisJSON/RedisJSON
that referenced
this pull request
Sep 16, 2024
The PR adds support for active defrag on RedisJSON. A pre condition for this PR is that the following PR's will be megred: * [Redis defrad module API extentions](redis/redis#13509) * [redismodule-rs support for active defrag API](RedisLabsModules/redismodule-rs#387) * [IJSON support for defrag](RedisJSON/ijson#1) The PR register defrag function on the json datatype and uses the new capability of ISON to defrag the key. **Notice**: * Increamental defrag of the json key is **not** support. In order to support it we need to implement the free_effort callback. This is not trivial and even if it was it would have cause the json object to potentially be freed when the GIL is not hold, which violate the assumption of our shared string implementation (it is not thread safe). We leave it for future improvment. * If we run on a Redis version that do not support the defrag start callback, we can still partially support defrag. In that case the IJSON object will be defraged but the shared strings dictionatrywill not be reinitialize. This basically means that shared strings will not be defraged. Tests were added to cover the new functionality. (cherry picked from commit 6539e1e)
MeirShpilraien
pushed a commit
to RedisJSON/RedisJSON
that referenced
this pull request
Sep 16, 2024
The PR adds support for active defrag on RedisJSON. A pre condition for this PR is that the following PR's will be megred: * [Redis defrad module API extentions](redis/redis#13509) * [redismodule-rs support for active defrag API](RedisLabsModules/redismodule-rs#387) * [IJSON support for defrag](RedisJSON/ijson#1) The PR register defrag function on the json datatype and uses the new capability of ISON to defrag the key. **Notice**: * Increamental defrag of the json key is **not** support. In order to support it we need to implement the free_effort callback. This is not trivial and even if it was it would have cause the json object to potentially be freed when the GIL is not hold, which violate the assumption of our shared string implementation (it is not thread safe). We leave it for future improvment. * If we run on a Redis version that do not support the defrag start callback, we can still partially support defrag. In that case the IJSON object will be defraged but the shared strings dictionatrywill not be reinitialize. This basically means that shared strings will not be defraged. Tests were added to cover the new functionality. (cherry picked from commit 6539e1e)
MeirShpilraien
pushed a commit
to RedisJSON/RedisJSON
that referenced
this pull request
Sep 16, 2024
The PR adds support for active defrag on RedisJSON. A pre condition for this PR is that the following PR's will be megred: * [Redis defrad module API extentions](redis/redis#13509) * [redismodule-rs support for active defrag API](RedisLabsModules/redismodule-rs#387) * [IJSON support for defrag](RedisJSON/ijson#1) The PR register defrag function on the json datatype and uses the new capability of ISON to defrag the key. **Notice**: * Increamental defrag of the json key is **not** support. In order to support it we need to implement the free_effort callback. This is not trivial and even if it was it would have cause the json object to potentially be freed when the GIL is not hold, which violate the assumption of our shared string implementation (it is not thread safe). We leave it for future improvment. * If we run on a Redis version that do not support the defrag start callback, we can still partially support defrag. In that case the IJSON object will be defraged but the shared strings dictionatrywill not be reinitialize. This basically means that shared strings will not be defraged. Tests were added to cover the new functionality. (cherry picked from commit 6539e1e)
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
4 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.
The PR adds support for defrag API.
The PR introduce a new struct,
DefragContext, which provide a safe API over*mut raw::RedisModuleDefragCtx.Notice that we do expose an unsafe API to create
DefragContextfrom*mut raw::RedisModuleDefragCtx. This is because we do not have a safe API for datatype registration. User must register an unsafe function as the defrag callback of the datatype and create theDefragContextfrom*mut raw::RedisModuleDefragCtx. We should consider adding a safe API for datatype creation.In addition, the PR introduce 3 new proc macros:
Example and test for the new API were added.
Notice: The start and the end callbacks for defrag was added on this PR