- 04 Oct, 2021 2 commits
-
-
meir@redislabs.com authored
When LUA call our C code, by default, the LUA stack has room for 20 elements. In most cases, this is more than enough but sometimes it's not and the caller must verify the LUA stack size before he pushes elements. On 3 places in the code, there was no verification of the LUA stack size. On specific inputs this missing verification could have lead to invalid memory write: 1. On 'luaReplyToRedisReply', one might return a nested reply that will explode the LUA stack. 2. On 'redisProtocolToLuaType', the Redis reply might be deep enough to explode the LUA stack (notice that currently there is no such command in Redis that returns such a nested reply, but modules might do it) 3. On 'ldbRedis', one might give a command with enough arguments to explode the LUA stack (all the arguments will be pushed to the LUA stack) This commit is solving all those 3 issues by calling 'lua_checkstack' and verify that there is enough room in the LUA stack to push elements. In case 'lua_checkstack' returns an error (there is not enough room in the LUA stack and it's not possible to increase the stack), we will do the following: 1. On 'luaReplyToRedisReply', we will return an error to the user. 2. On 'redisProtocolToLuaType' we will exit with panic (we assume this scenario is rare because it can only happen with a module). 3. On 'ldbRedis', we return an error. (cherry picked from commit d32a3f74f2a343846b50920e95754a955c1a10a9)
-
meir@redislabs.com authored
The protocol parsing on 'ldbReplParseCommand' (LUA debugging) Assumed protocol correctness. This means that if the following is given: *1 $100 test The parser will try to read additional 94 unallocated bytes after the client buffer. This commit fixes this issue by validating that there are actually enough bytes to read. It also limits the amount of data that can be sent by the debugger client to 1M so the client will not be able to explode the memory. (cherry picked from commit 4f95e6809902665ced50646107d174dd8137bcaa)
-
- 22 Feb, 2021 1 commit
-
-
杨博东 authored
After fork, the child process(redis-aof-rewrite) will get the fd opened by the parent process(redis), when redis killed by kill -9, it will not graceful exit(call prepareForShutdown()), so redis-aof-rewrite thread may still alive, the fd(lock) will still be held by redis-aof-rewrite thread, and redis restart will fail to get lock, means fail to start. This issue was causing failures in the cluster tests in github actions. Co-authored-by:
Oran Agra <oran@redislabs.com> (cherry picked from commit cbaf3c5b)
-
- 23 Apr, 2020 1 commit
-
-
srzhao authored
Checking OOM by `getMaxMemoryState` inside script might get different result with `freeMemoryIfNeededAndSafe` at script start, because lua stack and arguments also consume memory. This leads to memory `borderline` when memory grows near server.maxmemory: - `freeMemoryIfNeededAndSafe` at script start detects no OOM, no memory freed - `getMaxMemoryState` inside script detects OOM, script aborted We solve this 'borderline' issue by saving OOM state at script start to get stable lua OOM state. related to issue #6565 and #5250.
-
- 05 Mar, 2020 1 commit
-
-
WuYunlong authored
-
- 13 May, 2019 1 commit
-
-
Yossi Gottlieb authored
-
- 09 Oct, 2018 1 commit
-
-
antirez authored
Related to #4804.
-
- 03 Oct, 2018 1 commit
-
-
Bruce Merry authored
sdsZmallocSize assumes a dynamically allocated SDS. When given a string object created by createEmbeddedStringObject, it calls zmalloc_size on a pointer that isn't the one returned by zmalloc
-
- 14 Sep, 2018 1 commit
-
-
antirez authored
-
- 05 Sep, 2018 3 commits
-
-
antirez authored
See issue #5250 and issue #5292 for more info.
-
antirez authored
Here the idea is that we do not want freeMemoryIfNeeded() to propagate a DEL command before the script and change what happens in the script execution once it reaches the slave. For example see this potential issue (in the words of @soloestoy): On master, we run the following script: if redis.call('get','key') then redis.call('set','xxx','yyy') end redis.call('set','c','d') Then when redis attempts to execute redis.call('set','xxx','yyy'), we call freeMemoryIfNeeded(), and the key may get deleted, and because redis.call('set','xxx','yyy') has already been executed on master, this script will be replicated to slave. But the slave received "DEL key" before the script, and will ignore maxmemory, so after that master has xxx and c, slave has only one key c. Note that this patch (and other related work) was authored collaboratively in issue #5250 with the help of @soloestoy and @oranagra. Related to issue #5250. -
antirez authored
See issue #5250 and the new comments added to the code in this commit for details.
-
- 04 Sep, 2018 6 commits
-
-
antirez authored
-
zhaozhao.zz authored
-
antirez authored
Technically speaking we don't really need to put the master client in the clients that need to be processed, since in practice the PING commands from the master will take care, however it is conceptually more sane to do so.
-
antirez authored
However the master scripts will be impossible to kill. Related to #5297.
-
antirez authored
See reasoning in #5297.
-
Chris Lamb authored
-
- 02 Aug, 2018 2 commits
- 23 Jul, 2018 2 commits
-
-
Itamar Haber authored
-
Itamar Haber authored
Implementation notes: as INFO is "already broken", I didn't want to break it further. Instead of computing the server.lua_script dict size on every call, I'm keeping a running sum of the body's length and dict overheads. This implementation is naive as it **does not** take into consideration dict rehashing, but that inaccuracy pays off in speed ;) Demo time: ```bash $ redis-cli info memory | grep "script" used_memory_scripts:96 used_memory_scripts_human:96B number_of_cached_scripts:0 $ redis-cli eval "" 0 ; redis-cli info memory | grep "script" (nil) used_memory_scripts:120 used_memory_scripts_human:120B number_of_cached_scripts:1 $ redis-cli script flush ; redis-cli info memory | grep "script" OK used_memory_scripts:96 used_memory_scripts_human:96B number_of_cached_scripts:0 $ redis-cli eval "return('Hello, Script Cache :)')" 0 ; redis-cli info memory | grep "script" "Hello, Script Cache :)" used_memory_scripts:152 used_memory_scripts_human:152B number_of_cached_scripts:1 $ redis-cli eval "return redis.sha1hex(\"return('Hello, Script Cache :)')\")" 0 ; redis-cli info memory | grep "script" "1be72729d43da5114929c1260a749073732dc822" used_memory_scripts:232 used_memory_scripts_human:232B number_of_cached_scripts:2✔ 19:03:54 redis [lua_scripts-in-info-memory L ✚…⚑] $ redis-cli evalsha 1be72729d43da5114929c1260a749073732dc822 0 "Hello, Script Cache :)" ```
-
- 16 Jul, 2018 1 commit
-
-
Oran Agra authored
A) slave buffers didn't count internal fragmentation and sds unused space, this caused them to induce eviction although we didn't mean for it. B) slave buffers were consuming about twice the memory of what they actually needed. - this was mainly due to sdsMakeRoomFor growing to twice as much as needed each time but networking.c not storing more than 16k (partially fixed recently in 237a38737). - besides it wasn't able to store half of the new string into one buffer and the other half into the next (so the above mentioned fix helped mainly for small items). - lastly, the sds buffers had up to 30% internal fragmentation that was wasted, consumed but not used. C) inefficient performance due to starting from a small string and reallocing many times. what i changed: - creating dedicated buffers for reply list, counting their size with zmalloc_size - when creating a new reply node from, preallocate it to at least 16k. - when appending a new reply to the buffer, first fill all the unused space of the previous node before starting a new one. other changes: - expose mem_not_counted_for_evict info field for the benefit of the test suite - add a test to make sure slave buffers are counted correctly and that they don't cause eviction
-
- 02 Jul, 2018 1 commit
-
-
antirez authored
-
- 09 Jun, 2018 1 commit
-
-
Itamar Haber authored
-
- 07 Jun, 2018 1 commit
-
-
Itamar Haber authored
-
- 06 Dec, 2017 1 commit
-
-
antirez authored
The main change introduced by this commit is pretending that help arrays are more text than code, thus indenting them at level 0. This improves readability, and is an old practice when defining arrays of C strings describing text. Additionally a few useless return statements are removed, and the HELP subcommand capitalized when printed to the user.
-
- 04 Dec, 2017 3 commits
-
-
antirez authored
The function in its initial form, and after the fixes for the PSYNC2 bugs, required code duplication in multiple spots. This commit modifies it in order to always compute the script name independently, and to return the SDS of the SHA of the body: this way it can be used in all the places, including for SCRIPT LOAD, without duplicating the code to create the Lua function name. Note that this requires to re-compute the body SHA1 in the case of EVAL seeing a script for the first time, but this should not change scripting performance in any way because new scripts definition is a rare event happening the first time a script is seen, and the SHA1 computation is anyway not a very slow process against the typical Redis script and compared to the actua Lua byte compiling of the body. Note that the function used to assert() if a duplicated script was loaded, however actually now two times over three, we want the function to handle duplicated scripts just fine: this happens in SCRIPT LOAD and in RDB AUX "lua" loading. Moreover the assert was not defending against some obvious failure mode, so now the function always tests against already defined functions at start.
-
antirez authored
The block is already inside if (allow_dup).
-
antirez authored
Unfortunately, as outlined by @soloestoy in #4505, "lua" AUX RDB field loading in case of duplicated script was still broken. This commit fixes this problem and also a memory leak introduced by the past commit. Note that now we have a regression test able to duplicate the issue, so this commit was actually tested against the regression. The original PR also had a valid fix, but I prefer to hide the details of scripting.c outside scripting.c, and later "SCRIPT LOAD" should also be able to use the function luaCreateFunction() instead of redoing the work.
-
- 01 Dec, 2017 1 commit
-
-
antirez authored
In the case of slaves loading the RDB from master, or in other similar cases, the script is already defined, and the function registering the script should not fail in the assert() call.
-
- 30 Nov, 2017 3 commits
-
-
antirez authored
-
antirez authored
Related to #4483. As suggested by @soloestoy, we can retrieve the SHA1 from the body. Given that in the new implementation using AUX fields we ended copying around a lot to create new objects and strings, extremize such concept and trade CPU for space inside the RDB file.
-
antirez authored
See #4483. This is needed because luaCreateFunction() is now called from RDB loading code outside a client context.
-
- 28 Nov, 2017 1 commit
-
-
Itamar Haber authored
This adds a new `addReplyHelp` helper that's used by commands when returning a help text. The following commands have been touched: DEBUG, OBJECT, COMMAND, PUBSUB, SCRIPT and SLOWLOG. WIP Fix entry command table entry for OBJECT for HELP option. After #4472 the command may have just 2 arguments. Improve OBJECT HELP descriptions. See #4472. WIP 2 WIP 3
-
- 22 Nov, 2017 1 commit
-
-
Yossi Gottlieb authored
For example: 1. A module command called within a MULTI section. 2. A Lua script with replicate_commands() called within a MULTI section. 3. A module command called from a Lua script in the above context.
-
- 24 Jul, 2017 1 commit
-
-
liangsijian authored
-
- 30 Jun, 2017 1 commit
-
-
antirez authored
-
- 11 Apr, 2017 1 commit
-
-
antirez authored
Otherwise, as it was, it will overwrite whatever the user set. Close #3703.
-
- 05 May, 2016 1 commit
-
-
antirez authored
-