1. 14 Sep, 2021 1 commit
    • Viktor Söderqvist's avatar
      Modules: Add remaining list API functions (#8439) · ea36d4de
      Viktor Söderqvist authored
      List functions operating on elements by index:
      
      * RM_ListGet
      * RM_ListSet
      * RM_ListInsert
      * RM_ListDelete
      
      Iteration is done using a simple for loop over indices.
      The index based functions use an internal iterator as an optimization.
      This is explained in the docs:
      
      ```
       * Many of the list functions access elements by index. Since a list is in
       * essence a doubly-linked list, accessing elements by index is generally an
       * O(N) operation. However, if elements are accessed sequentially or with
       * indices close together, the functions are optimized to seek the index from
       * the previous index, rather than seeking from the ends of the list.
       *
       * This enables iteration to be done efficiently using a simple for loop:
       *
       *     long n = RM_ValueLength(key);
       *     for (long i = 0; i < n; i++) {
       *         RedisModuleString *elem = RedisModule_ListGet(key, i);
       *         // Do stuff...
       *     }
      ```
      ea36d4de
  2. 09 Sep, 2021 2 commits
    • sundb's avatar
      Replace all usage of ziplist with listpack for t_zset (#9366) · 3ca6972e
      sundb authored
      Part two of implementing #8702 (zset), after #8887.
      
      ## Description of the feature
      Replaced all uses of ziplist with listpack in t_zset, and optimized some of the code to optimize performance.
      
      ## Rdb format changes
      New `RDB_TYPE_ZSET_LISTPACK` rdb type.
      
      ## Rdb loading improvements:
      1) Pre-expansion of dict for validation of duplicate data for listpack and ziplist.
      2) Simplifying the release of empty key objects when RDB loading.
      3) Unify ziplist and listpack data verify methods for zset and hash, and move code to rdb.c.
      
      ## Interface changes
      1) New `zset-max-listpack-entries` config is an alias for `zset-max-ziplist-entries` (same with `zset-max-listpack-value`).
      2) OBJECT ENCODING will return listpack instead of ziplist.
      
      ## Listpack improvements:
      1) Add `lpDeleteRange` and `lpDeleteRangeWithEntry` functions to delete a range of entries from listpack.
      2) Improve the performance of `lpCompare`, converting from string to integer is faster than converting from integer to string.
      3) Replace `snprintf` with `ll2string` to improve performance in converting numbers to strings in `lpGet()`.
      
      ## Zset improvements:
      1) Improve the performance of `zzlFind` method, use `lpFind` instead of `lpCompare` in a loop.
      2) Use `lpDeleteRangeWithEntry` instead of `lpDelete` twice to delete a element of zset.
      
      ## Tests
      1) Add some unittests for `lpDeleteRange` and `lpDeleteRangeWithEntry` function.
      2) Add zset RDB loading test.
      3) Add benchmark test for `lpCompare` and `ziplsitCompare`.
      4) Add empty listpack zset corrupt dump test.
      3ca6972e
    • Binbin's avatar
      Add LMPOP/BLMPOP commands. (#9373) · c50af0ae
      Binbin authored
      We want to add COUNT option for BLPOP.
      But we can't do it without breaking compatibility due to the command arguments syntax.
      So this commit introduce two new commands.
      
      Syntax for the new LMPOP command:
      `LMPOP numkeys [<key> ...] LEFT|RIGHT [COUNT count]`
      
      Syntax for the new BLMPOP command:
      `BLMPOP timeout numkeys [<key> ...] LEFT|RIGHT [COUNT count]`
      
      Some background:
      - LPOP takes one key, and can return multiple elements.
      - BLPOP takes multiple keys, but returns one element from just one key.
      - LMPOP can take multiple keys and return multiple elements from just one key.
      
      Note that LMPOP/BLMPOP  can take multiple keys, it eventually operates on just one key.
      And it will propagate as LPOP or RPOP with the COUNT option.
      
      As a new command, it still return NIL if we can't pop any elements.
      For the normal response is nested arrays in RESP2 and RESP3, like:
      ```
      LMPOP/BLMPOP 
      1) keyname
      2) 1) element1
         2) element2
      ```
      I.e. unlike BLPOP that returns a key name and one element so it uses a flat array,
      and LPOP that returns multiple elements with no key name, and again uses a flat array,
      this one has to return a nested array, and it does for for both RESP2 and RESP3 (like SCAN does)
      
      Some discuss can see: #766 #8824
      c50af0ae
  3. 10 Aug, 2021 1 commit
    • sundb's avatar
      Replace all usage of ziplist with listpack for t_hash (#8887) · 02fd76b9
      sundb authored
      
      
      Part one of implementing #8702 (taking hashes first before other types)
      
      ## Description of the feature
      1. Change ziplist encoded hash objects to listpack encoding.
      2. Convert existing ziplists on RDB loading time. an O(n) operation.
      
      ## Rdb format changes
      1. Add RDB_TYPE_HASH_LISTPACK rdb type.
      2. Bump RDB_VERSION to 10
      
      ## Interface changes
      1. New `hash-max-listpack-entries` config is an alias for `hash-max-ziplist-entries` (same with `hash-max-listpack-value`)
      2. OBJECT ENCODING will return `listpack` instead of `ziplist`
      
      ## Listpack improvements:
      1. Support direct insert, replace integer element (rather than convert back and forth from string)
      3. Add more listpack capabilities to match the ziplist ones (like `lpFind`, `lpRandomPairs` and such)
      4. Optimize element length fetching, avoid multiple calculations
      5. Use inline to avoid function call overhead.
      
      ## Tests
      1. Add a new test to the RDB load time conversion
      2. Adding the listpack unit tests. (based on the one in ziplist.c)
      3. Add a few "corrupt payload: fuzzer findings" tests, and slightly modify existing ones.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      02fd76b9
  4. 05 Aug, 2021 1 commit
    • yoav-steinberg's avatar
      dict struct memory optimizations (#9228) · 5e908a29
      yoav-steinberg authored
      Reduce dict struct memory overhead
      on 64bit dict size goes down from jemalloc's 96 byte bin to its 56 byte bin.
      
      summary of changes:
      - Remove `privdata` from callbacks and dict creation. (this affects many files, see "Interface change" below).
      - Meld `dictht` struct into the `dict` struct to eliminate struct padding. (this affects just dict.c and defrag.c)
      - Eliminate the `sizemask` field, can be calculated from size when needed.
      - Convert the `size` field into `size_exp` (exponent), utilizes one byte instead of 8.
      
      Interface change: pass dict pointer to dict type call back functions.
      This is instead of passing the removed privdata field. In the future if
      we'd like to have private data in the callbacks we can extract it from
      the dict type. We can extend dictType to include a custom dict struct
      allocator and use it to allocate more data at the end of the dict
      struct. This data can then be used to store private data later acccessed
      by the callbacks.
      5e908a29
  5. 04 Aug, 2021 1 commit
    • Meir Shpilraien (Spielrein)'s avatar
      Unified Lua and modules reply parsing and added RESP3 support to RM_Call (#9202) · 2237131e
      Meir Shpilraien (Spielrein) authored
      
      
      ## Current state
      1. Lua has its own parser that handles parsing `reds.call` replies and translates them
        to Lua objects that can be used by the user Lua code. The parser partially handles
        resp3 (missing big number, verbatim, attribute, ...)
      2. Modules have their own parser that handles parsing `RM_Call` replies and translates
        them to RedisModuleCallReply objects. The parser does not support resp3.
      
      In addition, in the future, we want to add Redis Function (#8693) that will probably
      support more languages. At some point maintaining so many parsers will stop
      scaling (bug fixes and protocol changes will need to be applied on all of them).
      We will probably end up with different parsers that support different parts of the
      resp protocol (like we already have today with Lua and modules)
      
      ## PR Changes
      This PR attempt to unified the reply parsing of Lua and modules (and in the future
      Redis Function) by introducing a new parser unit (`resp_parser.c`). The new parser
      handles parsing the reply and calls different callbacks to allow the users (another
      unit that uses the parser, i.e, Lua, modules, or Redis Function) to analyze the reply.
      
      ### Lua API Additions
      The code that handles reply parsing on `scripting.c` was removed. Instead, it uses
      the resp_parser to parse and create a Lua object out of the reply. As mentioned
      above the Lua parser did not handle parsing big numbers, verbatim, and attribute.
      The new parser can handle those and so Lua also gets it for free.
      Those are translated to Lua objects in the following way:
      1. Big Number - Lua table `{'big_number':'<str representation for big number>'}`
      2. Verbatim - Lua table `{'verbatim_string':{'format':'<verbatim format>', 'string':'<verbatim string value>'}}`
      3. Attribute - currently ignored and not expose to the Lua parser, another issue will be open to decide how to expose it.
      
      Tests were added to check resp3 reply parsing on Lua
      
      ### Modules API Additions
      The reply parsing code on `module.c` was also removed and the new resp_parser is used instead.
      In addition, the RedisModuleCallReply was also extracted to a separate unit located on `call_reply.c`
      (in the future, this unit will also be used by Redis Function). A nice side effect of unified parsing is
      that modules now also support resp3. Resp3 can be enabled by giving `3` as a parameter to the
      fmt argument of `RM_Call`. It is also possible to give `0`, which will indicate an auto mode. i.e, Redis
      will automatically chose the reply protocol base on the current client set on the RedisModuleCtx
      (this mode will mostly be used when the module want to pass the reply to the client as is).
      In addition, the following RedisModuleAPI were added to allow analyzing resp3 replies:
      
      * New RedisModuleCallReply types:
         * `REDISMODULE_REPLY_MAP`
         * `REDISMODULE_REPLY_SET`
         * `REDISMODULE_REPLY_BOOL`
         * `REDISMODULE_REPLY_DOUBLE`
         * `REDISMODULE_REPLY_BIG_NUMBER`
         * `REDISMODULE_REPLY_VERBATIM_STRING`
         * `REDISMODULE_REPLY_ATTRIBUTE`
      
      * New RedisModuleAPI:
         * `RedisModule_CallReplyDouble` - getting double value from resp3 double reply
         * `RedisModule_CallReplyBool` - getting boolean value from resp3 boolean reply
         * `RedisModule_CallReplyBigNumber` - getting big number value from resp3 big number reply
         * `RedisModule_CallReplyVerbatim` - getting format and value from resp3 verbatim reply
         * `RedisModule_CallReplySetElement` - getting element from resp3 set reply
         * `RedisModule_CallReplyMapElement` - getting key and value from resp3 map reply
         * `RedisModule_CallReplyAttribute` - getting a reply attribute
         * `RedisModule_CallReplyAttributeElement` - getting key and value from resp3 attribute reply
         
      * New context flags:
         * `REDISMODULE_CTX_FLAGS_RESP3` - indicate that the client is using resp3
      
      Tests were added to check the new RedisModuleAPI
      
      ### Modules API Changes
      * RM_ReplyWithCallReply might return REDISMODULE_ERR if the given CallReply is in resp3
        but the client expects resp2. This is not a breaking change because in order to get a resp3
        CallReply one needs to specifically specify `3` as a parameter to the fmt argument of
        `RM_Call` (as mentioned above).
      
      Tests were added to check this change
      
      ### More small Additions
      * Added `debug set-disable-deny-scripts` that allows to turn on and off the commands no-script
      flag protection. This is used by the Lua resp3 tests so it will be possible to run `debug protocol`
      and check the resp3 parsing code.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      Co-authored-by: default avatarYossi Gottlieb <yossigo@gmail.com>
      2237131e
  6. 03 Aug, 2021 1 commit
  7. 01 Aug, 2021 1 commit
  8. 19 Jul, 2021 1 commit
  9. 01 Jul, 2021 1 commit
    • Yossi Gottlieb's avatar
      Fix CLIENT UNBLOCK crashing modules. (#9167) · aa139e2f
      Yossi Gottlieb authored
      Modules that use background threads with thread safe contexts are likely
      to use RM_BlockClient() without a timeout function, because they do not
      set up a timeout.
      
      Before this commit, `CLIENT UNBLOCK` would result with a crash as the
      `NULL` timeout callback is called. Beyond just crashing, this is also
      logically wrong as it may throw the module into an unexpected client
      state.
      
      This commits makes `CLIENT UNBLOCK` on such clients behave the same as
      any other client that is not in a blocked state and therefore cannot be
      unblocked.
      aa139e2f
  10. 22 Jun, 2021 2 commits
    • Oran Agra's avatar
      Adjustments to recent RM_StringTruncate fix (#3718) (#9125) · ae418eca
      Oran Agra authored
      - Introduce a new sdssubstr api as a building block for sdsrange.
        The API of sdsrange is many times hard to work with and also has
        corner case that cause bugs. sdsrange is easy to work with and also
        simplifies the implementation of sdsrange.
      - Revert the fix to RM_StringTruncate and just use sdssubstr instead of
        sdsrange.
      - Solve valgrind warnings from the new tests introduced by the previous
        PR.
      ae418eca
    • Evan's avatar
      modules: Add newlen == 0 handling to RM_StringTruncate (#3717) (#3718) · 1ccf2ca2
      Evan authored
      Previously, passing 0 for newlen would not truncate the string at all.
      This adds handling of this case, freeing the old string and creating a new empty string.
      
      Other changes:
      - Move `src/modules/testmodule.c` to `tests/modules/basics.c`
      - Introduce that basic test into the test suite
      - Add tests to cover StringTruncate
      - Add `test-modules` build target for the main makefile
      - Extend `distclean` build target to clean modules too
      1ccf2ca2
  11. 16 Jun, 2021 2 commits
    • chenyang8094's avatar
      Enhance mem_usage/free_effort/unlink/copy callbacks and add GetDbFromIO api. (#8999) · e0cd3ad0
      chenyang8094 authored
      Create new module type enhanced callbacks: mem_usage2, free_effort2, unlink2, copy2.
      These will be given a context point from which the module can obtain the key name and database id.
      In addition the digest and defrag context can now be used to obtain the key name and database id.
      e0cd3ad0
    • Uri Shachar's avatar
      Cleaning up the cluster interface by moving almost all related declar… (#9080) · c7e502a0
      Uri Shachar authored
      * Cleaning up the cluster interface by moving almost all related declarations into cluster.h
      (no logic change -- just moving declarations/definitions around)
      
      This initial effort leaves two items out of scope - the configuration parsing into the server
      struct and the internals exposed by the clusterNode struct.
      
      * Remove unneeded declarations of dictSds*
      Ideally all the dictSds functionality would move from server.c into a dedicated module
      so we can avoid the duplication in redis-benchmark/cli
      
      * Move crc16 back into server.h, will be moved out once we create a seperate header file for
      hashing functions
      c7e502a0
  12. 10 Jun, 2021 1 commit
    • Binbin's avatar
      Fixed some typos, add a spell check ci and others minor fix (#8890) · 0bfccc55
      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`
      0bfccc55
  13. 08 Jun, 2021 1 commit
  14. 03 Jun, 2021 1 commit
  15. 01 Jun, 2021 3 commits
  16. 18 May, 2021 1 commit
  17. 03 May, 2021 1 commit
  18. 02 May, 2021 1 commit
  19. 26 Apr, 2021 1 commit
  20. 25 Apr, 2021 2 commits
  21. 21 Apr, 2021 1 commit
  22. 19 Apr, 2021 1 commit
    • Hanna Fadida's avatar
      Modules: adding a module type for key space notification (#8759) · 53a4d6c3
      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.
      53a4d6c3
  23. 13 Apr, 2021 3 commits
    • Viktor Söderqvist's avatar
      Modules API docs: Sections and links (#8442) · d7920ff9
      Viktor Söderqvist authored
      * Modules API docs: Link API function names to their definitions
      
      Occurrences of API functions are linked to their definition.
      
      A function index with links to all functions is added on the bottom
      of the page.
      
      Comment blocks in module.c starting with a markdown h2 heading are
      used as sections. A table of contents is generated from these
      headings.
      
      The functions names are changed from h2 to h3, since they are now
      rendered as sub-headings within each section.
      
      Existing sections in module.c are used with some minor changes.
      Some documentation text is added or sligtly modified.
      
      The markdown renderer will add IDs which may clash with our
      generated IDs. By prefixing section IDs with "section-" we make
      them different.
      
      Replace double dashes with a unicode long ndash
      d7920ff9
    • Viktor Söderqvist's avatar
      Small doc fix for stream module API (#8757) · 4938052f
      Viktor Söderqvist authored
      In a code example, using RedisModule_FreeString instead of
      RedisModule_Free makes it behave correctly regardless of whether
      automatic memory is used or not.
      4938052f
    • Oran Agra's avatar
      Revert "Fix: server will crash if rdbload or rdbsave method is not provided in... · b278e443
      Oran Agra authored
      Revert "Fix: server will crash if rdbload or rdbsave method is not provided in module (#8670)" (#8771)
      
      This reverts commit 808f3004.
      b278e443
  24. 06 Apr, 2021 1 commit
  25. 01 Apr, 2021 1 commit
  26. 25 Mar, 2021 1 commit
    • Oran Agra's avatar
      Fix SLOWLOG for blocked commands (#8632) · 497351ad
      Oran Agra authored
      * SLOWLOG didn't record anything for blocked commands because the client
        was reset and argv was already empty. there was a fix for this issue
        specifically for modules, now it works for all blocked clients.
      * The original command argv (before being re-written) was also reset
        before adding the slowlog on behalf of the blocked command.
      * Latency monitor is now updated regardless of the slowlog flags of the
        command or its execution (their purpose is to hide sensitive info from
        the slowlog, not hide the fact the latency happened).
      * Latency monitor now uses real_cmd rather than c->cmd (which may be
        different if the command got re-written, e.g. GEOADD)
      
      Changes:
      * Unify shared code between slowlog insertion in call() and
        updateStatsOnUnblock(), hopefully prevent future bugs from happening
        due to the later being overlooked.
      * Reset CLIENT_PREVENT_LOGGING in resetClient rather than after command
        processing.
      * Add a test for SLOWLOG and BLPOP
      
      Notes:
      - real_cmd == c->lastcmd, except inside MULTI and Lua.
      - blocked commands never happen in these cases (MULTI / Lua)
      - real_cmd == c->cmd, except for when the command is rewritten (e.g.
        GEOADD)
      - blocked commands (currently) are never rewritten
      - other than the command's CLIENT_PREVENT_LOGGING, and the
        execution flag CLIENT_PREVENT_LOGGING, other cases that we want to
        avoid slowlog are on AOF loading (specifically CMD_CALL_SLOWLOG will
        be off when executed from execCommand that runs from an AOF)
      497351ad
  27. 24 Mar, 2021 2 commits
  28. 16 Mar, 2021 1 commit
    • Oran Agra's avatar
      set module eviction context flag only in masters (#8631) · 40d555db
      Oran Agra authored
      REDISMODULE_CTX_FLAGS_EVICT and REDISMODULE_CTX_FLAGS_MAXMEMORY
      shouldn't be set when the module is run inside a replica that doesn't do eviction.
      
      one may argue that the database is under eviction (the master does eviction and sends DELs to the replica).
      but on the other hand, we don't really know the master's configuration.
      all that matters is if the current instance does evictions or not.
      40d555db
  29. 15 Mar, 2021 1 commit
    • guybe7's avatar
      Missing EXEC on modules propagation after failed EVAL execution (#8654) · dba33a94
      guybe7 authored
      1. moduleReplicateMultiIfNeeded should use server.in_eval like
         moduleHandlePropagationAfterCommandCallback
      2. server.in_eval could have been set to 1 and not reset back
         to 0 (a lot of missed early-exits after in_eval is already 1)
      
      Note: The new assertions in processCommand cover (2) and I added
      two module tests to cover (1)
      
      Implications:
      If an EVAL that failed (and thus left server.in_eval=1) runs before a module
      command that replicates, the replication stream will contain MULTI (because
      moduleReplicateMultiIfNeeded used to check server.lua_caller which is NULL
      at this point) but not EXEC (because server.in_eval==1)
      This only affects modules as module.c the only user of server.in_eval.
      
      Affects versions 6.2.0, 6.2.1
      dba33a94
  30. 14 Mar, 2021 1 commit
  31. 10 Mar, 2021 1 commit
    • guybe7's avatar
      Fix some issues with modules and MULTI/EXEC (#8617) · 3d0b427c
      guybe7 authored
      Bug 1:
      When a module ctx is freed moduleHandlePropagationAfterCommandCallback
      is called and handles propagation. We want to prevent it from propagating
      commands that were not replicated by the same context. Example:
      1. module1.foo does: RM_Replicate(cmd1); RM_Call(cmd2); RM_Replicate(cmd3)
      2. RM_Replicate(cmd1) propagates MULTI and adds cmd1 to also_propagagte
      3. RM_Call(cmd2) create a new ctx, calls call() and destroys the ctx.
      4. moduleHandlePropagationAfterCommandCallback is called, calling
         alsoPropagates EXEC (Note: EXEC is still not written to socket),
         setting server.in_trnsaction = 0
      5. RM_Replicate(cmd3) is called, propagagting yet another MULTI (now
         we have nested MULTI calls, which is no good) and then cmd3
      
      We must prevent RM_Call(cmd2) from resetting server.in_transaction.
      REDISMODULE_CTX_MULTI_EMITTED was revived for that purpose.
      
      Bug 2:
      Fix issues with nested RM_Call where some have '!' and some don't.
      Example:
      1. module1.foo does RM_Call of module2.bar without replication (i.e. no '!')
      2. module2.bar internally calls RM_Call of INCR with '!'
      3. at the end of module1.foo we call RM_ReplicateVerbatim
      
      We want the replica/AOF to see only module1.foo and not the INCR from module2.bar
      
      Introduced a global replication_allowed flag inside RM_Call to determine
      whether we need to replicate or not (even if '!' was specified)
      
      Other changes:
      Split beforePropagateMultiOrExec to beforePropagateMulti afterPropagateExec
      just for better readability
      3d0b427c