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 new defrag API to allocate and free raw memory. #13509

Merged
merged 6 commits into from
Sep 3, 2024

Conversation

MeirShpilraien
Copy link
Collaborator

@MeirShpilraien MeirShpilraien commented Sep 2, 2024

All the defrag allocations API expects to get a value and replace it, leaving the old value untouchable. In some cases a value might be shared between multiple keys, in such cases we can not simply replace it when the defrag callback is called.

To allow support such use cases, the PR adds two new API's to the defrag API:

  1. RM_DefragAllocRaw - allocate memory base on a given size.
  2. RM_DefragFreeRaw - Free the given pointer.

Those API's avoid using tcache so they operate just like RM_DefragAlloc but allows the user to split the allocation and the memory free operations into two stages and control when those happen.

In addition the PR adds new API to allow the module to receive notifications when defrag start and end: RM_RegisterDefragCallbacks
Those callbacks are the same as RM_RegisterDefragFunc but promised to be called and the start and the end of the defrag process.

All the defrag allocations API expects to get a value and replace it,
leaving the old value untouchable. In some cases a value might be shared
between multiple keys, in such cases we can not simply replace it when
the defrag callback is called.

To allow support such use cases, the PR adds two new API's to the
defrag API:

1. RM_DefragAllocRaw - allocate memory base on a given size.
2. RM_DefragFreeRaw - Free the given pointer.

Those API's avoid using tcache so they operate just like RM_DefragAlloc
but allows the user to split the allocation and the memory free operations
into two stages and control when those happen.
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.

don't you want to add the start / end callbacks in this PR as well?

@MeirShpilraien
Copy link
Collaborator Author

don't you want to add the start / end callbacks in this PR as well?

Done

@oranagra oranagra added the release-notes indication that this issue needs to be mentioned in the release notes label Sep 3, 2024
@oranagra
Copy link
Member

oranagra commented Sep 3, 2024

@sundb please take a quick look and merge

MeirShpilraien added 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.
@MeirShpilraien MeirShpilraien merged commit d3d94cc into redis:unstable Sep 3, 2024
14 checks passed
MeirShpilraien added 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.
@YaacovHazan YaacovHazan mentioned this pull request Sep 11, 2024
YaacovHazan added a commit that referenced this pull request Sep 12, 2024
### New Features in binary distributions

- 7 new data structures: JSON, Time series, Bloom filter, Cuckoo filter,
Count-min sketch, Top-k, t-digest
- Redis scalable query engine (including vector search)

### Potentially breaking changes

- #12272 `GETRANGE` returns an empty bulk when the negative end index is
out of range
- #12395 Optimize `SCAN` command when matching data type

### Bug fixes

- #13510 Fix `RM_RdbLoad` to enable AOF after RDB loading is completed
- #13489 `ACL CAT` - return module commands
- #13476 Fix a race condition in the `cache_memory` of `functionsLibCtx`
- #13473 Fix incorrect lag due to trimming stream via `XTRIM` command
- #13338 Fix incorrect lag field in `XINFO` when tombstone is after the
`last_id` of the consume group
- #13470 On `HDEL` of last field - update the global hash field
expiration data structure
- #13465 Cluster: Pass extensions to node if extension processing is
handled by it
- #13443 Cluster: Ensure validity of myself when loading cluster config
- #13422 Cluster: Fix `CLUSTER SHARDS` command returns empty array

### Modules API

- #13509 New API calls: `RM_DefragAllocRaw`, `RM_DefragFreeRaw`, and
`RM_RegisterDefragCallbacks` - defrag API to allocate and free raw
memory

### Performance and resource utilization improvements

- #13503 Avoid overhead of comparison function pointer calls in listpack
`lpFind`
- #13505 Optimize `STRING` datatype write commands
- #13499 Optimize `SMEMBERS` command
- #13494 Optimize `GEO*` commands reply
- #13490 Optimize `HELLO` command
- #13488 Optimize client query buffer
- #12395 Optimize `SCAN` command when matching data type
- #13529 Optimize `LREM`, `LPOS`, `LINSERT`, and `LINDEX` commands
- #13516 Optimize `LRANGE` and other commands that perform several
writes to client buffers per call
- #13431 Avoid `used_memory` contention when updating from multiple
threads

### Other general improvements

- #13495 Reply `-LOADING` on replica while flushing the db

### CLI tools

- #13411 redis-cli: Fix wrong `dbnum` showed after the client
reconnected

### Notes

- No backward compatibility for replication or persistence.
- Additional distributions, upgrade paths, features, and improvements
will be introduced in upcoming pre-releases.
- With the GA release of 8.0 we will deprecate Redis Stack.
MeirShpilraien added 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 added 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 added 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-notes indication that this issue needs to be mentioned in the release notes
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants