Hashtab #1
Draft
Hashtab #1
Conversation
This file contains 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
The MANIFESTO is not Valkey's manifesto and it doesn't even mention open source. Let's write another one later, or some other document about our project principles. The other two files are one-line files with no relevant info. They're polluting the file listing at root level. It's the first thing you see when you start exploring the repo for the first time. Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Most of the content of TLS.md has already been copied to README.md in valkey-io#927. The description of how to run tests with TLS is moved to tests/README.md. Descriptions of the additional scripts runtest-cluster, runtest-sentinel and runtest-module are added in tests/README.md. Links to tests/README.md and src/unit/README.md are added in the top-level README.md along with a brief overview of the `make test-*` commands. Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
…io#964) In valkey-io#792, the time complexity became ambiguous, fluctuating between O(1) and O(n), which is a significant difference. And we agree uncertainty can potentially bring disaster to the business, the right thing to do is to persuade users to use EXISTS instead of KEYS in this case, to do the right thing the right way, rather than accommodating this incorrect usage. This reverts commit d66a06e. This reverts valkey-io#792. Signed-off-by: Binbin <binloveplay1314@qq.com>
…#963) Fix a bug in isValidAuxChar where valid characters '.' and ':' were incorrectly included in the banned charset. This issue affected the validation of auxiliary fields in the nodes.conf file used by Valkey in cluster mode, particularly when handling IPv4 and IPv6 addresses. The code now correctly allows '.' and ':' as valid characters, ensuring proper handling of these fields. Comments were added to clarify the use of the banned charset. Related to valkey-io#736 --------- Signed-off-by: Ping Xie <pingxie@google.com>
Release the client's MULTI state when the transaction becomes dirty to save memory. --------- Signed-off-by: zhaozhao.zz <zhaozhao.zz@alibaba-inc.com>
…ard_id (valkey-io#944) When reconfiguring sub-replica, there may a case that the sub-replica will use the old offset and win the election and cause the data loss if the old primary went down. In this case, sender is myself's primary, when executing updateShardId, not only the sender's shard_id is updated, but also the shard_id of myself is updated, casuing the subsequent areInSameShard check, that is, the full_sync_required check to fail. As part of the recent fix of valkey-io#885, the sub-replica needs to decide whether a full sync is required or not when switching shards. This shard membership check is supposed to be done against sub-replica's current shard_id, which however was lost in this code path. This then leads to sub-replica joining the other shard with a completely different and incorrect replication history. This is the only place where replicaof state can be updated on this path so the most natural fix would be to pull the chain replication reduction logic into this code block and before the updateShardId call. This one follow valkey-io#885 and closes valkey-io#942. Signed-off-by: Binbin <binloveplay1314@qq.com> Co-authored-by: Ping Xie <pingxie@outlook.com>
Migrate zipmap unit test to new unit test framework, parent ticket valkey-io#428 . --------- Signed-off-by: Shivshankar-Reddy <shiva.sheri.github@gmail.com> Signed-off-by: hwware <wen.hui.ware@gmail.com> Co-authored-by: hwware <wen.hui.ware@gmail.com>
…#895) In standalone mode, when a `-REDIRECT` error occurs, special handling is required if the client is in the `MULTI` context. We have adopted the same handling method as the cluster mode: 1. If a command in the transaction encounters a `REDIRECT` at the time of queuing, the execution of `EXEC` will return an `EXECABORT` error (we expect the client to redirect and discard the transaction upon receiving a `REDIRECT`). That is: ``` MULTI ==> +OK SET x y ==> -REDIRECT EXEC ==> -EXECABORT ``` 2. If all commands are successfully queued (i.e., `QUEUED` results are received) but a redirect is detected during `EXEC` execution (such as a primary-replica switch), a `REDIRECT` is returned to instruct the client to perform a redirect. That is: ``` MULTI ==> +OK SET x y ==> +QUEUED failover EXEC ==> -REDIRECT ``` --------- Signed-off-by: zhaozhao.zz <zhaozhao.zz@alibaba-inc.com>
The reason is the server 3 still have the server 7 as its replica
due to a short wait, the wait is not enough, we should wait for
server loss its replica.
```
*** [err]: valkey-cli make source node ignores NOREPLICAS error when doing the last CLUSTER SETSLOT
Expected '{127.0.0.1 21497 267}' to be equal to '' (context: type eval line 34 cmd {assert_equal [lindex [R 3 role] 2] {}} proc ::test)
```
Signed-off-by: Binbin <binloveplay1314@qq.com>
Deletes zipmapSet, zipmapGet, etc. Only keep iterator and validate integrity, what we use when loading an old RDB file. Adjust unit tests to not use zipmapSet, etc. Solves a build failure where when compiling with fortify source. --------- Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
If the expiration time passed in SET is expired, for example, it has expired due to the machine time (DTS) or the expiration time passed in (wrong arg). In this case, we don't need to set the key and wait for the active expire scan before deleting the key. Compared with previous changes: 1. If the key does not exist, previously we would set the key and wait for the active expire to delete it, so it is a set + del from the perspective of propaganda. Now we will no set the key and return, so it a NOP. 2. If the key exists, previously we woule set the key and wait for the active expire to delete it, so it is a set + del From the perspective of propaganda. Now we will delete it and return, so it is a del. Adding a new deleteExpiredKeyFromOverwriteAndPropagate function to reduce the duplicate code. Signed-off-by: Binbin <binloveplay1314@qq.com> Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
The previous test does a strncmp on a NULL, which is not valid. It should be using an empty length string instead. Addresses https://github.com/valkey-io/valkey/actions/runs/10649272046/job/29519233939. Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
## Set replica-lazy-flush and lazyfree-lazy-user-flush to yes by default. There are many problems with running flush synchronously. Even in single CPU environments, the thread managers should balance between the freeing and serving incoming requests. ## Set lazy eviction, expire, server-del, user-del to yes by default We now have a del and a lazyfree del, we also have these configuration items to control: lazyfree-lazy-eviction, lazyfree-lazy-expire, lazyfree-lazy-server-del, lazyfree-lazy-user-del. In most cases lazyfree is better since it reduces the risk of blocking the main thread, and because we have lazyfreeGetFreeEffort, on those with high effor (currently 64) will use lazyfree. Part of valkey-io#653. --------- Signed-off-by: Binbin <binloveplay1314@qq.com>
… dict (valkey-io#966) Feature `one-dict-per-slot` refactors the database, and part of it involved splitting the rehashing list from the global level back to the database level, or more specifically, the kvstore level. This change is fine, and it also simplifies the process of swapping databases, which is good. And it should not have a major impact on the efficiency of incremental rehashing. To implement the kvstore-level rehashing list, each `dict` under the `kvstore` needs to know which `kvstore` it belongs. However, kvstore did not insert the reference relationship into the `dict` itself, instead, it placed it in the `dictType`. In my view, this is a somewhat odd way. Theoretically, `dictType` is just a collection of function handles, a kind of virtual type that can be referenced globally, not an entity. But now the `dictType` is instantiated, with each `kvstore` owning an actual `dictType`, which in turn holds a reverse reference to the `kvstore`'s resource pointer. This design is somewhat uncomfortable for me. I think the `dictType` should not be instantiated. The references between actual resources (`kvstore` and `dict`) should occur between specific objects, rather than force materializing the `dictType`, which is supposed to be virtual. --------- Signed-off-by: zhaozhao.zz <zhaozhao.zz@alibaba-inc.com>
…o#980) Test failed because these two PRs valkey-io#865 and valkey-io#913. Signed-off-by: Binbin <binloveplay1314@qq.com>
…er logs (valkey-io#877) Implement data masking for user data in server logs and diagnostic output. This change prevents potential exposure of confidential information, such as PII, and enhances privacy protection. It masks all command arguments, client names, and client usernames. Added a new hide-user-data-from-log configuration item, default yes. --------- Signed-off-by: Amit Nagler <anagler123@gmail.com>
Follow up to valkey-io#966, which didn't update the kvstore tests. I'm not actually entirely clear why it fixes it, but the consistency prevents the crash very reliably so will merge it now and maybe see if Zhao has a better explanation. --------- Signed-off-by: Madelyn Olson <madelyneolson@gmail.com>
This pull request introduces several changes to improve the type safety of Valkey's dictionary implementation: - Getter/Setter Macros: Implemented macros `DICT_SET_VALUE` and `DICT_GET_VALUE` to centralize type casting within these macros. This change emulates the behavior of C++ templates in C, limiting type casting to specific low-level operations and preventing it from being spread across the codebase. - Reduced Assert Overhead: Removed unnecessary asserts from critical hot paths in the dictionary implementation. - Consistent Naming: Standardized the naming of dictionary entry types. For example, all dictionary entry types start their names with `dictEntry`. Fix valkey-io#737 --------- Signed-off-by: Ping Xie <pingxie@google.com> Signed-off-by: Ping Xie <pingxie@outlook.com> Co-authored-by: Madelyn Olson <madelyneolson@gmail.com>
Change from replicsa to replicas in valkey-cli.c Signed-off-by: NAM UK KIM <namuk2004@naver.com>
…ult (valkey-io#983) Before this doc update, the comments in valkey.conf said that DEL is a blocking command, and even refered to other synchronous freeing as "in a blocking way, like if DEL was called". This has now become confusing and incorrect, since DEL is now non-blocking by default. The comments also mentioned too much about the "old default" and only later explain that the "new default" is non-blocking. This doc update focuses on the current default and expresses it like "Starting from Valkey 8.0, lazy freeing is enabled by default", rather than using words like old and new. This is a follow-up to valkey-io#913. --------- Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Add help wording for cluster SLOT-STATS. Signed-off-by: Kyle Kim <kimkyle@amazon.com>
Minor cleanup, introduced in valkey-io#877. Signed-off-by: Binbin <binloveplay1314@qq.com>
…updated in `clusterProcessGossipSection` (valkey-io#965) `clusterProcessGossipSection` currently doesn't trigger a check and call `replicationSetPrimary` when `myself`'s primary node’s IP/port is updated. This fix ensures that after every node address update, `replicationSetPrimary` is called if the updated node is `myself`'s primary. This prevents missed updates and ensures that replicas reconnect properly to maintain their replication link with the primary.
valkey-io#886) If we modify aof-use-rdb-preamble in the middle of rewrite, we may get a wrong aof base suffix. This is because the suffix is concatenated by the main process afterwards, and it may be different from the beginning. We cache this value when we start the rewrite. Signed-off-by: Binbin <binloveplay1314@qq.com>
Print the full client info by using catClientInfoString, the info is useful when we want to identify the source of request. Signed-off-by: Binbin <binloveplay1314@qq.com>
This PR migrates the tests related to listpack into new test framework as part of valkey-io#428. Signed-off-by: haoqixu <hq.xu0o0@gmail.com> Signed-off-by: Binbin <binloveplay1314@qq.com> Co-authored-by: Binbin <binloveplay1314@qq.com>
This PR migrates the tests related to dict into new test framework as part of valkey-io#428. Signed-off-by: haoqixu <hq.xu0o0@gmail.com> Signed-off-by: Binbin <binloveplay1314@qq.com> Co-authored-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: zhaozhao.zz <zhaozhao.zz@alibaba-inc.com>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
Signed-off-by: Viktor Söderqvist <viktor.soderqvist@est.tech>
zuiderkwast
pushed a commit
that referenced
this pull request
Oct 2, 2024
We are updating this variable in the main thread, and the
child threads can printing the logs at the same time. This
generating a warning in SANITIZER=thread:
```
WARNING: ThreadSanitizer: data race (pid=74208)
Read of size 4 at 0x000102875c10 by thread T3:
#0 serverLogRaw <null>:52173615 (valkey-server:x86_64+0x10003c556)
#1 _serverLog <null>:52173615 (valkey-server:x86_64+0x10003ca89)
valkey-io#2 bioProcessBackgroundJobs <null>:52173615 (valkey-server:x86_64+0x1001402c9)
Previous write of size 4 at 0x000102875c10 by main thread (mutexes: write M0):
#0 afterSleep <null>:52173615 (valkey-server:x86_64+0x10004989b)
#1 aeProcessEvents <null>:52173615 (valkey-server:x86_64+0x100031e52)
valkey-io#2 main <null>:52173615 (valkey-server:x86_64+0x100064a3c)
valkey-io#3 start <null>:52173615 (dyld:x86_64+0xfffffffffff5c365)
valkey-io#4 start <null>:52173615 (dyld:x86_64+0xfffffffffff5c365)
```
The refresh of daylight_active is not real time, we update
it in aftersleep, so we don't need a strong synchronization,
so using memory_order_relaxed. But also noted we are doing
load/store operations only for daylight_active, which is an
aligned 32-bit integer, so using memory_order_relaxed will
not provide more consistency than what we have today.
So this is just a cleanup that to clear the warning.
Signed-off-by: Binbin <binloveplay1314@qq.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This is a draft in early stages. It's not ready for public review yet.
Implementation and documentation is in src/hashtab.c and src/hashtab.h.
To test, look at src/unit/test_hashtab.c
To run test, do
TODO
Comparison with dict
Most of the API is similar. The main difference is that the hashtab contains
elements (where an element can contain a key and other data), rather than keys
and values.