- 25 Apr, 2024 1 commit
-
-
debing.sun authored
For my mistake, in the last revert commit in #13231, I originally wanted to revert the last one, but reverted the penultimate fix. Now that we have fix another potential memory read issue in [`743f1dd` (#13231)](https://github.com/redis/redis/pull/13231/commits/743f1dde79b433fdb8ea13de4fd73457d4fe25eb), now it just seems to avoid confusion, i will verify in the future whether it will have any impact, if so we will add this PR to backport. Failed CI: https://github.com/sundb/redis/actions/runs/8826731960
-
- 24 Apr, 2024 1 commit
-
-
debing.sun authored
Introducted by #13013 After defragmenting the dictionary in the kvstore, if the dict is reallocated, the value of its node in the kvstore rehashing list must be updated.
-
- 01 Apr, 2024 1 commit
-
-
Moti Cohen authored
It calls kvstoreIteratorNextDict() which eventually calls dictResumeRehashing() And then, on return, it calls dictResetIterator(iter) which calls dictResumeRehashing(). We end up with pauserehash value decremented twice instead of once.
-
- 20 Mar, 2024 2 commits
-
-
Pieter Cailliau authored
[Read more about the license change here](https://redis.com/blog/redis-adopts-dual-source-available-licensing/) Live long and prosper
🖖 -
Yanqi Lv authored
In ASAN CI, we find server may crash because of NULL ptr in `kvstoreIncrementallyRehash`. the reason is that we use two phase unlink in `dbGenericDelete`. After `kvstoreDictTwoPhaseUnlinkFind`, the dict may be in rehashing and only have one element in ht[0] of `db->keys`. When we delete the last element in `db->keys` meanwhile `db->keys` is in rehashing, we may free the dict in `kvstoreDictTwoPhaseUnlinkFree` without deleting the node in `kvs->rehashing`. Then we may use this freed ptr in `kvstoreIncrementallyRehash` in the `serverCron` and cause the crash. This is indeed a use-after-free problem. The fix is to call rehashingCompleted in dictRelease and dictEmpty, so that every call for rehashingStarted is always matched with a rehashingCompleted. Adding a test in the unit test to catch it consistently --------- Co-authored-by:
Oran Agra <oran@redislabs.com> Co-authored-by:
debing.sun <debing.sun@redis.com>
-
- 18 Mar, 2024 1 commit
-
-
Binbin authored
After #13072, there is an use-after-free error. In expireScanCallback, we will delete the dict, and then in dictScan we will continue to use the dict, like we will doing `dictResumeRehashing(d)` in the end, this casued an error. In this PR, in freeDictIfNeeded, if the dict's pauserehash is set, don't delete the dict yet, and then when scan returns try to delete it again. At the same time, we noticed that there will be similar problems in iterator. We may also delete elements during the iteration process, causing the dict to be deleted, so the part related to iter in the PR has also been modified. dictResetIterator was also missing from the previous kvstoreIteratorNextDict, we currently have no scenario that elements will be deleted in kvstoreIterator process, deal with it together to avoid future problems. Added some simple tests to verify the changes. In addition, the modification in #13072 omitted initTempDb and emptyDbAsync, and they were also added. This PR also remove the slow flag from the expire test (consumes 1.3s) so that problems can be found in CI in the future.
-
- 04 Mar, 2024 1 commit
-
-
debing.sun authored
After #13013 ### This PR make effort to defrag the pubsub kvstore in the following ways: 1. Till now server.pubsub(shard)_channels only share channel name obj with the first subscribed client, now change it so that the clients and the pubsub kvstore share the channel name robj. This would save a lot of memory when there are many subscribers to the same channel. It also means that we only need to defrag the channel name robj in the pubsub kvstore, and then update all client references for the current channel, avoiding the need to iterate through all the clients to do the same things. 2. Refactor the code to defragment pubsub(shard) in the same way as defragment of keys and EXPIRES, with the exception that we only defragment pubsub(without shard) when slot is zero. ### Other Fix an overlook in #11695, if defragment doesn't reach the end time, we should wait for the current db's keys and expires, pubsub and pubsubshard to finish before leaving, now it's possible to exit early when the keys are defragmented. --------- Co-authored-by:
oranagra <oran@redislabs.com>
-
- 01 Mar, 2024 1 commit
-
-
Chen Tianjie authored
Sometimes we need to make fast judgement about why Redis is suddenly taking more memory. One of the reasons is main DB's dicts doing rehashing. We may use `MEMORY STATS` to monitor the overhead memory of each DB, but there still lacks a total sum to show an overall trend. So this PR adds the total overhead of all DBs to `INFO MEMORY` section, together with the total count of rehashing DB dicts, providing some intuitive metrics about main dicts rehashing. This PR adds the following metrics to INFO MEMORY * `mem_overhead_db_hashtable_rehashing` - only size of ht[0] in dictionaries we're rehashing (i.e. the memory that's gonna get released soon) and a similar ones to MEMORY STATS: * `overhead.db.hashtable.lut` (complements the existing `overhead.hashtable.main` and `overhead.hashtable.expires` which also counts the `dictEntry` structs too) * `overhead.db.hashtable.rehashing` - temporary rehashing overhead. * `db.dict.rehashing.count` - number of top level dictionaries being rehashed. --------- Co-authored-by:
zhaozhao.zz <zhaozhao.zz@alibaba-inc.com> Co-authored-by:
Oran Agra <oran@redislabs.com>
-
- 29 Feb, 2024 1 commit
-
-
zhaozhao.zz authored
just like `kvstoreDictDelete`, we need check `freeDictIfNeeded` when `kvstoreEmpty`.
-
- 19 Feb, 2024 1 commit
-
-
zhaozhao.zz authored
In the `databasesCron()`, the time consumed by `kvstoreIncrementallyRehash()` is used to calculate the exit condition. However, within `kvstoreIncrementallyRehash()`, the loop first checks for timeout before performing rehashing. Therefore, the time for the last rehash isn't accounted for, making the consumed time inaccurate. We need to precisely calculate all the time spent on rehashing. Additionally, the time allocated to `kvstoreIncrementallyRehash()` should be the remaining time, which is `INCREMENTAL_REHASHING_THRESHOLD_US` minus the already consumed `elapsed_us`.
-
- 15 Feb, 2024 1 commit
-
-
Binbin authored
Usually, the probability that a dict exists is much greater than the probability that it does not exist. In kvstoreDictAddRaw, we will call kvstoreGetDict multiple times. Based on this assumption, we change createDictIfNeeded to something like get or create function: ``` before: dict exist: 2 kvstoreGetDict dict non-exist: 2 kvstoreGetDict after: dict exist: 1 kvstoreGetDict dict non-exist: 3 kvstoreGetDict ``` A possible 3% performance improvement was observed: In addition, some typos/comments i saw have been cleaned up.
-
- 11 Feb, 2024 1 commit
-
-
debing.sun authored
Fail CI: https://github.com/redis/redis/actions/runs/7837608438/job/21387609715 ## Why defragment tests only failed under 32-bit First of all, under 32-bit jemalloc will allocate more small bins and less large bins, which will also lead to more external fragmentation, therefore, the fragmentation ratio is higher in 32-bit than in 64-bit, so the defragment tests(`Active defrag eval scripts: cluster` and `Active defrag big keys: cluster`) always fails in 32-bit. ## Why defragment tests only failed with cluster The fowllowing is the result of `Active defrag eval scripts: cluster` test. 1) Before #11695, the fragmentation ratio is 3.11%. 2) After #11695, the fragmentation ratio grew to 4.58%. Since we are using per-slot dictionary to manage slots, we will only defragment the contents of these dictionaries (keys, values), but not the dictionaries' struct and ht_table, which means that frequent shrinking and expanding of the dictionaries, will make more fragments. 3) After #12850 and #12948, In cluster mode, a large number of cluster slot dicts will be shrunk, creating additional fragmention, and the dictionary will not be defragged. ## Solution * Add defragmentation of the per-slot dictionary's own structures, dict struct and ht_table. ## Other change * Increase floating point print precision of `frags` and `rss` in debug logs for defrag --------- Co-authored-by:
Oran Agra <oran@redislabs.com>
-
- 07 Feb, 2024 1 commit
-
-
Binbin authored
After fix for #13033, address sanitizer reports this heap-use-after-free error. When the pubsubshard_channels dict becomes empty, we will delete the dict, and the dictReleaseIterator will call dictResetIterator, it will use the dict so we will trigger the error. This PR introduced a new struct kvstoreDictIterator to wrap dictIterator. Replace the original dict iterator with the new kvstore dict iterator. --------- Co-authored-by:
Oran Agra <oran@redislabs.com> Co-authored-by:
guybe7 <guy.benoish@redislabs.com>
-
- 06 Feb, 2024 1 commit
-
-
Binbin authored
When the dict is NULL, we also need to push resize_cursor, otherwise it will keep doing useless continue here, and there is no way to resize the other dict behind it. Introduced in #12822. --------- Co-authored-by:
Oran Agra <oran@redislabs.com>
-
- 05 Feb, 2024 1 commit
-
-
guybe7 authored
# Description Gather most of the scattered `redisDb`-related code from the per-slot dict PR (#11695) and turn it to a new data structure, `kvstore`. i.e. it's a class that represents an array of dictionaries. # Motivation The main motivation is code cleanliness, the idea of using an array of dictionaries is very well-suited to becoming a self-contained data structure. This allowed cleaning some ugly code, among others: loops that run twice on the main dict and expires dict, and duplicate code for allocating and releasing this data structure. # Notes 1. This PR reverts the part of https://github.com/redis/redis/pull/12848 where the `rehashing` list is global (handling rehashing `dict`s is under the responsibility of `kvstore`, and should not be managed by the server) 2. This PR also replaces the type of `server.pubsubshard_channels` from `dict**` to `kvstore` (original PR: https://github.com/redis/redis/pull/12804). After that was done, server.pubsub_channels was also chosen to be a `kvstore` (with only one `dict`, which seems odd) just to make the code cleaner by making it the same type as `server.pubsubshard_channels`, see `pubsubtype.serverPubSubChannels` 3. the keys and expires kvstores are currenlty configured to allocate the individual dicts only when the first key is added (unlike before, in which they allocated them in advance), but they won't release them when the last key is deleted. Worth mentioning that due to the recent change the reply of DEBUG HTSTATS changed, in case no keys were ever added to the db. before: ``` 127.0.0.1:6379> DEBUG htstats 9 [Dictionary HT] Hash table 0 stats (main hash table): No stats available for empty dictionaries [Expires HT] Hash table 0 stats (main hash table): No stats available for empty dictionaries ``` after: ``` 127.0.0.1:6379> DEBUG htstats 9 [Dictionary HT] [Expires HT] ```
-