- 16 Oct, 2022 1 commit
-
-
Meir Shpilraien (Spielrein) authored
As discussed on #11084, `propagatePendingCommands` should happened after the del notification is fired so that the notification effect and the `del` will be replicated inside MULTI EXEC. Test was added to verify the fix.
-
- 18 Aug, 2022 1 commit
-
-
Meir Shpilraien (Spielrein) authored
Fix replication inconsistency on modules that uses key space notifications. ### The Problem In general, key space notifications are invoked after the command logic was executed (this is not always the case, we will discuss later about specific command that do not follow this rules). For example, the `set x 1` will trigger a `set` notification that will be invoked after the `set` logic was performed, so if the notification logic will try to fetch `x`, it will see the new data that was written. Consider the scenario on which the notification logic performs some write commands. for example, the notification logic increase some counter, `incr x{counter}`, indicating how many times `x` was changed. The logical order by which the logic was executed is has follow: ``` set x 1 incr x{counter} ``` The issue is that the `set x 1` command is added to the replication buffer at the end of the command invocation (specifically after the key space notification logic was invoked and performed the `incr` command). The replication/aof sees the commands in the wrong order: ``` incr x{counter} set x 1 ``` In this specific example the order is less important. But if, for example, the notification would have deleted `x` then we would end up with primary-replica inconsistency. ### The Solution Put the command that cause the notification in its rightful place. In the above example, the `set x 1` command logic was executed before the notification logic, so it should be added to the replication buffer before the commands that is invoked by the notification logic. To achieve this, without a major code refactoring, we save a placeholder in the replication buffer, when finishing invoking the command logic we check if the command need to be replicated, and if it does, we use the placeholder to add it to the replication buffer instead of appending it to the end. To be efficient and not allocating memory on each command to save the placeholder, the replication buffer array was modified to reuse memory (instead of allocating it each time we want to replicate commands). Also, to avoid saving a placeholder when not needed, we do it only for WRITE or MAY_REPLICATE commands. #### Additional Fixes * Expire and Eviction notifications: * Expire/Eviction logical order was to first perform the Expire/Eviction and then the notification logic. The replication buffer got this in the other way around (first notification effect and then the `del` command). The PR fixes this issue. * The notification effect and the `del` command was not wrap with `multi-exec` (if needed). The PR also fix this issue. * SPOP command: * On spop, the `spop` notification was fired before the command logic was executed. The change in this PR would have cause the replication order to be change (first `spop` command and then notification `logic`) although the logical order is first the notification logic and then the `spop` logic. The right fix would have been to move the notification to be fired after the command was executed (like all the other commands), but this can be considered a breaking change. To overcome this, the PR keeps the current behavior and changes the `spop` code to keep the right logical order when pushing commands to the replication buffer. Another PR will follow to fix the SPOP properly and match it to the other command (we split it to 2 separate PR's so it will be easy to cherry-pick this PR to 7.0 if we chose to). #### Unhanded Known Limitations * key miss event: * On key miss event, if a module performed some write command on the event (using `RM_Call`), the `dirty` counter would increase and the read command that cause the key miss event would be replicated to the replication and aof. This problem can also happened on a write command that open some keys but eventually decides not to perform any action. We decided not to handle this problem on this PR because the solution is complex and will cause additional risks in case we will want to cherry-pick this PR. We should decide if we want to handle it in future PR's. For now, modules writers is advice not to perform any write commands on key miss event. #### Testing * We already have tests to cover cases where a notification is invoking write commands that are also added to the replication buffer, the tests was modified to verify that the replica gets the command in the correct logical order. * Test was added to verify that `spop` behavior was kept unchanged. * Test was added to verify key miss event behave as expected. * Test was added to verify the changes do not break lazy expiration. #### Additional Changes * `propagateNow` function can accept a special dbid, -1, indicating not to replicate `select`. We use this to replicate `multi/exec` on `propagatePendingCommands` function. The side effect of this change is that now the `select` command will appear inside the `multi/exec` block on the replication stream (instead of outside of the `multi/exec` block). Tests was modified to match this new behavior.
-
- 02 Aug, 2022 1 commit
-
-
Binbin authored
There is a -Wimplicit-function-declaration warning in here: ``` keyspace_events.c: In function ‘KeySpace_NotificationGeneric’: keyspace_events.c:67:9: warning: implicit declaration of function ‘usleep’; did you mean ‘sleep’? [-Wimplicit-function-declaration] 67 | usleep(1); | ^~~~~~ | sleep ```
-
- 27 Jul, 2022 1 commit
-
-
guybe7 authored
RM_Microseconds Return the wall-clock Unix time, in microseconds RM_CachedMicroseconds Returns a cached copy of the Unix time, in microseconds. It is updated in the server cron job and before executing a command. It is useful for complex call stacks, such as a command causing a key space notification, causing a module to execute a RedisModule_Call, causing another notification, etc. It makes sense that all these callbacks would use the same clock.
-
- 25 Apr, 2022 1 commit
-
-
guybe7 authored
This case is interesting because it originates from cron, rather than from another command. The idea came from looking at #9890 and #10573, and I was wondering if RM_Call would work properly when `server.current_client == NULL`
-
- 30 Dec, 2021 1 commit
-
-
Viktor Söderqvist authored
These exist for quite some time, and are no longer experimental
-
- 28 Nov, 2021 1 commit
-
-
Viktor Söderqvist authored
Writable replicas now no longer use the values of expired keys. Expired keys are deleted when lookupKeyWrite() is used, even on a writable replica. Previously, writable replicas could use the value of an expired key in write commands such as INCR, SUNIONSTORE, etc.. This commit also sorts out the mess around the functions lookupKeyRead() and lookupKeyWrite() so they now indicate what we intend to do with the key and are not affected by the command calling them. Multi-key commands like SUNIONSTORE, ZUNIONSTORE, COPY and SORT with the store option now use lookupKeyRead() for the keys they're reading from (which will not allow reading from logically expired keys). This commit also fixes a bug where PFCOUNT could return a value of an expired key. Test modules commands have their readonly and write flags updated to correctly reflect their lookups for reading or writing. Modules are not required to correctly reflect this in their command flags, but this change is made for consistency since the tests serve as usage examples. Fixes #6842. Fixes #7475.
-
- 10 Jun, 2021 1 commit
-
-
Binbin authored
This PR adds a spell checker CI action that will fail future PRs if they introduce typos and spelling mistakes. This spell checker is based on blacklist of common spelling mistakes, so it will not catch everything, but at least it is also unlikely to cause false positives. Besides that, the PR also fixes many spelling mistakes and types, not all are a result of the spell checker we use. Here's a summary of other changes: 1. Scanned the entire source code and fixes all sorts of typos and spelling mistakes (including missing or extra spaces). 2. Outdated function / variable / argument names in comments 3. Fix outdated keyspace masks error log when we check `config.notify-keyspace-events` in loadServerConfigFromString. 4. Trim the white space at the end of line in `module.c`. Check: https://github.com/redis/redis/pull/7751 5. Some outdated https link URLs. 6. Fix some outdated comment. Such as: - In README: about the rdb, we used to said create a `thread`, change to `process` - dbRandomKey function coment (about the dictGetRandomKey, change to dictGetFairRandomKey) - notifyKeyspaceEvent fucntion comment (add type arg) - Some others minor fix in comment (Most of them are incorrectly quoted by variable names) 7. Modified the error log so that users can easily distinguish between TCP and TLS in `changeBindAddr`
-
- 19 Apr, 2021 1 commit
-
-
Hanna Fadida authored
Adding a new type mask for key space notification, REDISMODULE_NOTIFY_MODULE, to enable unique notifications from commands on REDISMODULE_KEYTYPE_MODULE type keys (which is currently unsupported). Modules can subscribe to a module key keyspace notification by RM_SubscribeToKeyspaceEvents, and clients by notify-keyspace-events of redis.conf or via the CONFIG SET, with the characters 'd' or 'A' (REDISMODULE_NOTIFY_MODULE type mask is part of the '**A**ll' notation for key space notifications). Refactor: move some pubsub test infra from pubsub.tcl to util.tcl to be re-used by other tests.
-
- 06 Dec, 2020 1 commit
-
-
guybe7 authored
One way this was happening is when a module issued an RM_Call which would inject MULTI. If the module command that does that was itself issued by something else that already did added MULTI (e.g. another module, or a Lua script), it would have caused nested MULTI. In fact the MULTI state in the client or the MULTI_EMITTED flag in the context isn't the right indication that we need to propagate MULTI or not, because on a nested calls (possibly a module action called by a keyspace event of another module action), these flags aren't retained / reflected. instead there's now a global propagate_in_transaction flag for that. in addition to that, we now have a global in_eval and in_exec flags, to serve the flags of RM_GetContextFlags, since their dependence on the current client is wrong for the same reasons mentioned above.
-
- 11 Oct, 2020 1 commit
-
-
Meir Shpilraien (Spielrein) authored
* Introduce a new API's: RM_GetContextFlagsAll, and RM_GetKeyspaceNotificationFlagsAll that will return the full flags mask of each feature. The module writer can check base on this value if the Flags he needs are supported or not. * For each flag, introduce a new value on redismodule.h, this value represents the LAST value and should be there as a reminder to update it when a new value is added, also it will be used in the code to calculate the full flags mask (assuming flags are incrementally increasing). In addition, stated that the module writer should not use the LAST flag directly and he should use the GetFlagAll API's. * Introduce a new API: RM_IsSubEventSupported, that returns for a given event and subevent, whether or not the subevent supported. * Introduce a new macro RMAPI_FUNC_SUPPORTED(func) that returns whether or not a function API is supported by comparing it to NULL. * Introduce a new API: int RM_GetServerVersion();, that will return the current Redis version in the format 0x00MMmmpp; e.g. 0x00060008; * Changed unstable version from 999.999.999 to 255.255.255 Co-authored-by:
Oran Agra <oran@redislabs.com> Co-authored-by:
Yossi Gottlieb <yossigo@gmail.com>
-
- 09 Aug, 2020 1 commit
-
-
Meir Shpilraien (Spielrein) authored
Added RedisModule_HoldString that either returns a shallow copy of the given String (by increasing the String ref count) or a new deep copy of String in case its not possible to get a shallow copy. Co-authored-by:
Itamar Haber <itamar@redislabs.com>
-
- 23 Jul, 2020 1 commit
-
-
Meir Shpilraien (Spielrein) authored
Co-authored-by:
Oran Agra <oran@redislabs.com> Co-authored-by:
Itamar Haber <itamar@redislabs.com>
-
- 23 Oct, 2019 4 commits
- 26 Sep, 2018 2 commits
- 12 Apr, 2018 1 commit
-
-
antirez authored
-
- 31 Mar, 2018 1 commit
-
-
antirez authored
-
- 04 Aug, 2016 1 commit
-
-
Guo Xiao authored
Fix warning: ISO C does not support '__FUNCTION__' predefined identifier [-Wpedantic]
-
- 27 Jul, 2016 1 commit
-
-
antirez authored
By grepping the continuous integration errors log a number of GEORADIUS tests failures were detected. Fortunately when a GEORADIUS failure happens, the test suite logs enough information in order to reproduce the problem: the PRNG seed, coordinates and radius of the query. By reproducing the issues, three different bugs were discovered and fixed in this commit. This commit also improves the already good reporting of the fuzzer and adds the failure vectors as regression tests. The issues found: 1. We need larger squares around the poles in order to cover the area requested by the user. There were already checks in order to use a smaller step (larger squares) but the limit set (+/- 67 degrees) is not enough in certain edge cases, so 66 is used now. 2. Even near the equator, when the search area center is very near the edge of the square, the north, south, west or ovest square may not be able to fully cover the specified radius. Now a test is performed at the edge of the initial guessed search area, and larger squares are used in case the test fails. 3. Because of rounding errors between Redis and Tcl, sometimes the test signaled false positives. This is now addressed. Whenever possible the original code was improved a bit in other ways. A debugging example stanza was added in order to make the next debugging session simpler when the next bug is found.
-
- 26 Jul, 2015 1 commit
-
-
antirez authored
-
- 19 Aug, 2013 2 commits
- 19 Feb, 2012 1 commit
-
-
antirez authored
-
- 05 May, 2011 1 commit
-
-
Pieter Noordhuis authored
-
- 23 Dec, 2010 1 commit
-
-
Pieter Noordhuis authored
-
- 16 Dec, 2010 1 commit
-
-
Pieter Noordhuis authored
-
- 03 Nov, 2010 1 commit
-
-
Pieter Noordhuis authored
-
- 02 Nov, 2010 1 commit
-
-
antirez authored
-
- 02 Sep, 2010 1 commit
-
-
antirez authored
-
- 24 Jul, 2010 1 commit
-
-
Benjamin Kramer authored
calloc is more effecient than malloc+memset when the system uses mmap to allocate memory. mmap always returns zeroed memory so the memset can be avoided. The threshold to use mmap is 16k in osx libc and 128k in bsd libc and glibc. The kernel can lazily allocate the pages, this reduces memory usage when we have a page table or hash table that is mostly empty. This change is most visible when you start a new redis instance with vm enabled. You'll see no increased memory usage no matter how big your page table is.
-
- 01 Jul, 2010 1 commit
-
-
antirez authored
networking related stuff moved into networking.c moved more code more work on layout of source code SDS instantaneuos memory saving. By Pieter and Salvatore at VMware ;) cleanly compiling again after the first split, now splitting it in more C files moving more things around... work in progress split replication code splitting more Sets split Hash split replication split even more splitting more splitting minor change
-
- 19 Feb, 2010 1 commit
-
-
antirez authored
A problem with replication with multiple slaves connectiong to a single master fixed. It was due to a typo, and reported on github by the user micmac. Also the copyright year fixed from many files.
-
- 15 Jan, 2010 1 commit
-
-
antirez authored
-
- 23 Mar, 2009 1 commit
-
-
antirez authored
-
- 22 Mar, 2009 1 commit
-
-
antirez authored
-