1. 01 Apr, 2021 1 commit
  2. 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
  3. 24 Mar, 2021 2 commits
  4. 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
  5. 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
  6. 14 Mar, 2021 1 commit
  7. 10 Mar, 2021 2 commits
    • 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
    • guybe7's avatar
      Cleanup ZADD_* flags (#8559) · 61a73de6
      guybe7 authored
      Have a clear separation between in and out flags
      
      Other changes:
      
      delete dead code in RM_ZsetIncrby: if zsetAdd returned error (happens only if
      the result of the operation is NAN or if score is NAN) we return immediately so
      there is no way that zsetAdd succeeded and returned NAN in the out-flags
      61a73de6
  8. 08 Mar, 2021 1 commit
    • guybe7's avatar
      Fix edge-case when a module client is unblocked (#8618) · e58118cd
      guybe7 authored
      Scenario:
      1. A module client is blocked on keys with a timeout
      2. Shortly before the timeout expires, the key is being populated and signaled
         as ready
      3. Redis calls moduleTryServeClientBlockedOnKey (which replies to client) and
         then moduleUnblockClient
      4. moduleUnblockClient doesn't really unblock the client, it writes to
         server.module_blocked_pipe and only marks the BC as unblocked.
      5. beforeSleep kics in, by this time the client still exists and techincally
         timed-out. beforeSleep replies to the timeout client (double reply) and
         only then moduleHandleBlockedClients is called, reading from module_blocked_pipe
         and calling unblockClient
      
      The solution is similar to what was done in moduleTryServeClientBlockedOnKey: we
      should avoid re-processing an already-unblocked client
      e58118cd
  9. 02 Mar, 2021 1 commit
  10. 28 Feb, 2021 2 commits
    • Bonsai's avatar
      Module API for getting user name of a client (#8508) · 17c226b0
      Bonsai authored
      Adding RM_GetClientUserNameById to get the ACL user name of a client connection.
      17c226b0
    • Viktor Söderqvist's avatar
      Shared reusable client for RM_Call() (#8516) · 6122f1c4
      Viktor Söderqvist authored
      A single client pointer is added in the server struct. This is
      initialized by the first RM_Call() and reused for every subsequent
      RM_Call() except if it's already in use, which means that it's not
      used for (recursive) module calls to modules. For these, a new
      "fake" client is created each time.
      
      Other changes:
      * Avoid allocating a dict iterator in pubsubUnsubscribeAllChannels
        when not needed
      6122f1c4
  11. 16 Feb, 2021 1 commit
    • uriyage's avatar
      Adds INFO fields to track fork child progress (#8414) · fd052d2a
      uriyage authored
      * Adding current_save_keys_total and current_save_keys_processed info fields.
        Present in replication, BGSAVE and AOFRW.
      * Changing RM_SendChildCOWInfo() to RM_SendChildHeartbeat(double progress)
      * Adding new info field current_fork_perc. Present in Replication, BGSAVE, AOFRW,
        and module forks.
      fd052d2a
  12. 15 Feb, 2021 2 commits
  13. 10 Feb, 2021 1 commit
  14. 05 Feb, 2021 1 commit
    • Viktor Söderqvist's avatar
      RM_ZsetRem: Delete key if empty (#8453) · aea6e71e
      Viktor Söderqvist authored
      Without this fix, RM_ZsetRem can leave empty sorted sets which are
      not allowed to exist.
      
      Removing from a sorted set while iterating seems to work (while
      inserting causes failed assetions). RM_ZsetRangeEndReached is
      modified to return 1 if the key doesn't exist, to terminate
      iteration when the last element has been removed.
      aea6e71e
  15. 29 Jan, 2021 1 commit
    • filipe oliveira's avatar
      Enabled background and reply time tracking on blocked on keys/blocked on... · f0c5052a
      filipe oliveira authored
      Enabled background and reply time tracking on blocked on keys/blocked on background work clients (#7491)
      
      This commit enables tracking time of the background tasks and on replies,
      opening the door for properly tracking commands that rely on blocking / background
       work via the slowlog, latency history, and commandstats. 
      
      Some notes:
      - The time spent blocked waiting for key changes, or blocked on synchronous
        replication is not accounted for. 
      
      - **This commit does not affect latency tracking of commands that are non-blocking
        or do not have background work.** ( meaning that it all stays the same with exception to
        `BZPOPMIN`,`BZPOPMAX`,`BRPOP`,`BLPOP`, etc... and module's commands that rely
        on background threads ). 
      
      -  Specifically for latency history command we've added a new event class named
        `command-unblocking` that will enable latency monitoring on commands that spawn
        background threads to do the work.
      
      - For blocking commands we're now considering the total time of a command as the
        time spent on call() + the time spent on replying when unblocked.
      
      - For Modules commands that rely on background threads we're now considering the
        total time of a command as the time spent on call (main thread) + the time spent on
        the background thread ( if marked within `RedisModule_MeasureTimeStart()` and
        `RedisModule_MeasureTimeEnd()` ) + the time spent on replying (main thread)
      
      To test for this feature we've added a `unit/moduleapi/blockonbackground` test that relies on
      a module that blocks the client and sleeps on the background for a given time. 
      - check blocked command that uses RedisModule_MeasureTimeStart() is tracking background time
      - check blocked command that uses RedisModule_MeasureTimeStart() is tracking background time even in timeout
      - check blocked command with multiple calls RedisModule_MeasureTimeStart()  is tracking the total background time
      - check blocked command without calling RedisModule_MeasureTimeStart() is not reporting background time
      f0c5052a
  16. 28 Jan, 2021 2 commits
    • guybe7's avatar
      Modules: Add event for fork child birth and termination (#8289) · 01cbf17b
      guybe7 authored
      Useful to avoid doing background jobs that can cause excessive COW
      01cbf17b
    • Viktor Söderqvist's avatar
      Add modules API for streams (#8288) · 4355145a
      Viktor Söderqvist authored
      APIs added for these stream operations: add, delete, iterate and
      trim (by ID or maxlength). The functions are prefixed by RM_Stream.
      
      * RM_StreamAdd
      * RM_StreamDelete
      * RM_StreamIteratorStart
      * RM_StreamIteratorStop
      * RM_StreamIteratorNextID
      * RM_StreamIteratorNextField
      * RM_StreamIteratorDelete
      * RM_StreamTrimByLength
      * RM_StreamTrimByID
      
      The type RedisModuleStreamID is added and functions for converting
      from and to RedisModuleString.
      
      * RM_CreateStringFromStreamID
      * RM_StringToStreamID
      
      Whenever the stream functions return REDISMODULE_ERR, errno is set to
      provide additional error information.
      
      Refactoring: The zset iterator fields in the RedisModuleKey struct
      are wrapped in a union, to allow the same space to be used for type-
      specific info for streams and allow future use for other key types.
      4355145a
  17. 20 Jan, 2021 1 commit
    • Viktor Söderqvist's avatar
      More modules API ref formatting fixes (#8344) · 16258f21
      Viktor Söderqvist authored
      Fix broken formatting in `RM_Call` and `RM_CreateDataType`,
      `RM_SubscribeToServerEvent` (nested lists, etc. in list items).
      
      Unhide docs of `RM_LoadDataTypeFromString` and
      `RM_SaveDataTypeToString` by removing blank line between docs and
      function.
      
      Clarification added to `RM__Assert`: Recommentation to use the
      `RedisModule_Assert` macro instead.
      
      All names containing underscores (variable and macro names) are
      wrapped in backticks (if not already wrapped in backticks). This
      prevents underscore from being interpreted as italics in some
      cases.
      
      Names including a wildcard star, e.g. RM_Defrag*(), is wrapped in
      backticks (and RM replaced by RedisModule in this case). This
      prevents the * from being interpreted as an italics marker.
      
      A list item with a sublist, a paragraph and another sublist is a
      combination which seems impossible to achieve with RedCarped
      markdown, so the one occurrence of this is rewritten.
      
      Various trivial changes (typos, backticks, etc.).
      
      Ruby script:
      
      * Replace `RM_Xyz` with `RedisModule_Xyz` in docs. (RM is correct
        when refering to the C code but RedisModule is correct in the
        API docs.)
      * Automatic backquotes around C functions like `malloc()`.
      * Turn URLs into links. The link text is the URL itself.
      * Don't add backticks inside bold (**...**)
      16258f21
  18. 19 Jan, 2021 1 commit
  19. 15 Jan, 2021 2 commits
    • Viktor Söderqvist's avatar
      Rename non-API RM-prefixed functions to hide them from API docs · fcb3dfe5
      Viktor Söderqvist authored
      The prefix is changed from `RM_` to `module` on the following
      internal functions, to prevent them from appearing in the API docs:
      
          RM_LogRaw -> moduleLogRaw
          RM_FreeCallReplyRec -> moduleFreeCallReplyRec
          RM_ZsetAddFlagsToCoreFlags -> moduleZsetAddFlagsToCoreFlags
          RM_ZsetAddFlagsFromCoreFlags -> moduleZsetAddFlagsFromCoreFlags
      fcb3dfe5
    • Viktor Söderqvist's avatar
      Modules API reference formatting fixes · ebf20b83
      Viktor Söderqvist authored
      Fixes markdown formatting errors and some functions not showing
      up in the generated documentation at all.
      
      Ruby script (gendoc.rb) fixes:
      
      * Modified automatic instertion of backquotes:
        * Don't add backquotes around names which are already preceded by a
          backquote. Fixes for example \`RedisModule_Reply\*\` which turning
          into \`\`RedisModule_Reply\`\*\` messes up the formatting.
        * Add backquotes around types such as RedisModuleString (in addition
          to function names `RedisModule_[A-z()]*` and macro names
          `REDISMODULE_[A-z]*`).
        * Require 4 spaces indentation for disabling automatic backquotes, i.e.
          code blocks. Fixes continuations of list items (indented 2 spaces).
      * More permissive extraction of doc comments:
        * Allow doc comments starting with `/**`.
        * Make space before `*` on each line optional.
        * Make space after `/*` and `/**` optional (needed when appearing on
          its own line).
      
      Markdown fixes in module.c:
      
      * Fix code blocks not indented enough (4 spaces needed).
      * Add black line before code blocks and lists where missing (needed).
      * Enclose special markdown characters `_*^<>` in backticks to prevent them
        from messing up formatting.
      * Lists with `1)` changed to `1.` for proper markdown lists.
      * Remove excessive indentation which causes text to be unintentionally
        rendered as code blocks.
      * Other minor formatting fixes.
      
      Other fixes in module.c:
      
      * Remove blank lines between doc comment and function definition. A blank
        line here makes the Ruby script exclude the function in docs.
      ebf20b83
  20. 08 Jan, 2021 1 commit
  21. 07 Jan, 2021 2 commits
    • YaacovHazan's avatar
      Report child copy-on-write info continuously · ea930a35
      YaacovHazan authored
      Add INFO field, rdb_active_cow_size, to report COW of a live fork child while
      it's active.
      - once in 1024 keys check the time, and if there's more than one second since
        the last report send a report to the parent via the pipe.
      - refactor the child_info_data struct, it's an implementation detail that
        shouldn't be in the server struct, and not used to communicate data between
        caller and callee
      - remove the magic value from that struct (not sure what it was good for), and
        instead add handling of short reads.
      - add another value to the structure, cow_type, to indicate if the report is
        for the new rdb_active_cow_size field, or it's the last report of a
        successful operation
      - add new Module API to report the active COW
      - add more asserts variants to test.tcl
      ea930a35
    • YaacovHazan's avatar
      Refactory fork child related infra, Unify child pid · f9dacf8a
      YaacovHazan authored
      This is a refactory commit, isn't suppose to have any actual impact.
      it does the following:
      - keep just one server struct fork child pid variable instead of 3
      - have one server struct variable indicating the purpose of the current fork
        child.
      - redisFork is now responsible of updating the server struct with the pid,
        which means it can be the one that calls updateDictResizePolicy
      - move child info pipe handling into redisFork instead of having them
        repeated outside
      - there are two classes of fork purposes, mutually exclusive group (AOF, RDB,
        Module), and one that can create several forks to coexist in parallel (LDB,
        but maybe Modules some day too, Module API allows for that).
      - minor fix to killRDBChild:
        unlike killAppendOnlyChild and TerminateModuleForkChild, the killRDBChild
        doesn't clear the pid variable or call wait4, so checkChildrenDone does
        the cleanup for it.
        This commit removes the explicit calls to rdbRemoveTempFile, closeChildInfoPipe,
        updateDictResizePolicy, which didn't do any harm, but where unnecessary.
      f9dacf8a
  22. 04 Jan, 2021 1 commit
    • Itamar Haber's avatar
      HELP subcommand, continued (#5531) · 9dcdc7e7
      Itamar Haber authored
      
      
      * man-like consistent long formatting
      * Uppercases commands, subcommands and options
      * Adds 'HELP' to HELP for all
      * Lexicographical order
      * Uses value notation and other .md likeness
      * Moves const char *help to top
      * Keeps it under 80 chars
      * Misc help typos, consistent conjuctioning (i.e return and not returns)
      * Uses addReplySubcommandSyntaxError(c) all over
      Signed-off-by: default avatarItamar Haber <itamar@redislabs.com>
      9dcdc7e7
  23. 03 Jan, 2021 1 commit
  24. 31 Dec, 2020 1 commit
    • filipe oliveira's avatar
      Add errorstats info section, Add failed_calls and rejected_calls to commandstats (#8217) · 90b9f08e
      filipe oliveira authored
      This Commit pushes forward the observability on overall error statistics and command statistics within redis-server:
      
      It extends INFO COMMANDSTATS to have
      - failed_calls in - so we can keep track of errors that happen from the command itself, broken by command.
      - rejected_calls - so we can keep track of errors that were triggered outside the commmand processing per se
      
      Adds a new section to INFO, named ERRORSTATS that enables keeping track of the different errors that
      occur within redis ( within processCommand and call ) based on the reply Error Prefix ( The first word
      after the "-", up to the first space ).
      
      This commit also fixes RM_ReplyWithError so that it can be correctly identified as an error reply.
      90b9f08e
  25. 22 Dec, 2020 1 commit
    • Oran Agra's avatar
      Remove read-only flag from non-keyspace cmds, different approach for EXEC to... · 411c18bb
      Oran Agra authored
      Remove read-only flag from non-keyspace cmds, different approach for EXEC to propagate MULTI (#8216)
      
      In the distant history there was only the read flag for commands, and whatever
      command that didn't have the read flag was a write one.
      Then we added the write flag, but some portions of the code still used !read
      Also some commands that don't work on the keyspace at all, still have the read
      flag.
      
      Changes in this commit:
      1. remove the read-only flag from TIME, ECHO, ROLE and LASTSAVE
      
      2. EXEC command used to decides if it should propagate a MULTI by looking at
         the command flags (!read & !admin).
         When i was about to change it to look at the write flag instead, i realized
         that this would cause it not to propagate a MULTI for PUBLISH, EVAL, and
         SCRIPT, all 3 are not marked as either a read command or a write one (as
         they should), but all 3 are calling forceCommandPropagation.
      
         So instead of introducing a new flag to denote a command that "writes" but
         not into the keyspace, and still needs propagation, i decided to rely on
         the forceCommandPropagation, and just fix the code to propagate MULTI when
         needed rather than depending on the command flags at all.
      
         The implication of my change then is that now it won't decide to propagate
         MULTI when it sees one of these: SELECT, PING, INFO, COMMAND, TIME and
         other commands which are neither read nor write.
      
      3. Changing getNodeByQuery and clusterRedirectBlockedClientIfNeeded in
         cluster.c to look at !write rather than read flag.
         This should have no implications, since these code paths are only reachable
         for commands which access keys, and these are always marked as either read
         or write.
      
      This commit improve MULTI propagation tests, for modules and a bunch of
      other special cases, all of which used to pass already before that commit.
      the only one that test change that uncovered a change of behavior is the
      one that DELs a non-existing key, it used to propagate an empty
      multi-exec block, and no longer does.
      411c18bb
  26. 13 Dec, 2020 3 commits
    • Yossi Gottlieb's avatar
      Several (mostly Solaris-related) cleanups (#8171) · 86e3395c
      Yossi Gottlieb authored
      * Allow runtest-moduleapi use a different 'make', for systems where GNU Make is 'gmake'.
      * Fix issue with builds on Solaris re-building everything from scratch due to CFLAGS/LDFLAGS not stored.
      * Fix compile failure on Solaris due to atomicvar and a bunch of warnings.
      * Fix garbled log timestamps on Solaris.
      86e3395c
    • Oran Agra's avatar
      Add module event for repl-diskless-load swapdb (#8153) · ab60dcf5
      Oran Agra authored
      When a replica uses the diskless-load swapdb approach, it backs up the old database,
      then attempts to load a new one, and in case of failure, it restores the backup.
      
      this means that modules with global out of keyspace data, must have an option to
      subscribe to events and backup/restore/discard their global data too.
      ab60dcf5
    • Yossi Gottlieb's avatar
      Modules: add defrag API support. (#8149) · 63c1303c
      Yossi Gottlieb authored
      Add a new set of defrag functions that take a defrag context and allow
      defragmenting memory blocks and RedisModuleStrings.
      
      Modules can register a defrag callback which will be invoked when the
      defrag process handles globals.
      
      Modules with custom data types can also register a datatype-specific
      defrag callback which is invoked for keys that require defragmentation.
      The callback and associated functions support both one-step and
      multi-step options, depending on the complexity of the key as exposed by
      the free_effort callback.
      63c1303c
  27. 09 Dec, 2020 1 commit
    • Yossi Gottlieb's avatar
      Add module data-type support for COPY. (#8112) · 4e064fba
      Yossi Gottlieb authored
      This adds a copy callback for module data types, in order to make
      modules compatible with the new COPY command.
      
      The callback is optional and COPY will fail for keys with data types
      that do not implement it.
      4e064fba
  28. 06 Dec, 2020 2 commits
    • guybe7's avatar
      Make sure we do not propagate nested MULTI/EXEC (#8097) · 1df5bb56
      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.
      1df5bb56
    • Wang Yuan's avatar
      Limit the main db and expires dictionaries to expand (#7954) · 75f9dec6
      Wang Yuan authored
      As we know, redis may reject user's requests or evict some keys if
      used memory is over maxmemory. Dictionaries expanding may make
      things worse, some big dictionaries, such as main db and expires dict,
      may eat huge memory at once for allocating a new big hash table and be
      far more than maxmemory after expanding.
      There are related issues: #4213 #4583
      
      More details, when expand dict in redis, we will allocate a new big
      ht[1] that generally is double of ht[0], The size of ht[1] will be
      very big if ht[0] already is big. For db dict, if we have more than
      64 million keys, we need to cost 1GB for ht[1] when dict expands.
      
      If the sum of used memory and new hash table of dict needed exceeds
      maxmemory, we shouldn't allow the dict to expand. Because, if we
      enable keys eviction, we still couldn't add much more keys after
      eviction and rehashing, what's worse, redis will keep less keys when
      redis only remains a little memory for storing new hash table instead
      of users' data. Moreover users can't write data in redis if disable
      keys eviction.
      
      What this commit changed ?
      
      Add a new member function expandAllowed for dict type, it provide a way
      for caller to allow expand or not. We expose two parameters for this
      function: more memory needed for expanding and dict current load factor,
      users can implement a function to make a decision by them.
      For main db dict and expires dict type, these dictionaries may be very
      big and cost huge memory for expanding, so we implement a judgement
      function: we can stop dict to expand provisionally if used memory will
      be over maxmemory after dict expands, but to guarantee the performance
      of redis, we still allow dict to expand if dict load factor exceeds the
      safe load factor.
      Add test cases to verify we don't allow main db to expand when left
      memory is not enough, so that avoid keys eviction.
      
      Other changes:
      
      For new hash table size when expand. Before this commit, the size is
      that double used of dict and later _dictNextPower. Actually we aim to
      control a dict load factor between 0.5 and 1.0. Now we replace *2 with
      +1, since the first check is that used >= size, the outcome of before
      will usually be the same as _dictNextPower(used+1). The only case where
      it'll differ is when dict_can_resize is false during fork, so that later
      the _dictNextPower(used*2) will cause the dict to jump to *4 (i.e.
      _dictNextPower(1025*2) will return 4096).
      Fix rehash test cases due to changing algorithm of new hash table size
      when expand.
      75f9dec6
  29. 03 Dec, 2020 1 commit
  30. 17 Nov, 2020 1 commit
    • Meir Shpilraien (Spielrein)'s avatar
      Unified MULTI, LUA, and RM_Call with respect to blocking commands (#8025) · d87a0d02
      Meir Shpilraien (Spielrein) authored
      
      
      Blocking command should not be used with MULTI, LUA, and RM_Call. This is because,
      the caller, who executes the command in this context, expects a reply.
      
      Today, LUA and MULTI have a special (and different) treatment to blocking commands:
      
      LUA   - Most commands are marked with no-script flag which are checked when executing
      and command from LUA, commands that are not marked (like XREAD) verify that their
      blocking mode is not used inside LUA (by checking the CLIENT_LUA client flag).
      MULTI - Command that is going to block, first verify that the client is not inside
      multi (by checking the CLIENT_MULTI client flag). If the client is inside multi, they
      return a result which is a match to the empty key with no timeout (for example blpop
      inside MULTI will act as lpop)
      For modules that perform RM_Call with blocking command, the returned results type is
      REDISMODULE_REPLY_UNKNOWN and the caller can not really know what happened.
      
      Disadvantages of the current state are:
      
      No unified approach, LUA, MULTI, and RM_Call, each has a different treatment
      Module can not safely execute blocking command (and get reply or error).
      Though It is true that modules are not like LUA or MULTI and should be smarter not
      to execute blocking commands on RM_Call, sometimes you want to execute a command base
      on client input (for example if you create a module that provides a new scripting
      language like javascript or python).
      While modules (on modules command) can check for REDISMODULE_CTX_FLAGS_LUA or
      REDISMODULE_CTX_FLAGS_MULTI to know not to block the client, there is no way to
      check if the command came from another module using RM_Call. So there is no way
      for a module to know not to block another module RM_Call execution.
      
      This commit adds a way to unify the treatment for blocking clients by introducing
      a new CLIENT_DENY_BLOCKING client flag. On LUA, MULTI, and RM_Call the new flag
      turned on to signify that the client should not be blocked. A blocking command
      verifies that the flag is turned off before blocking. If a blocking command sees
      that the CLIENT_DENY_BLOCKING flag is on, it's not blocking and return results
      which are matches to empty key with no timeout (as MULTI does today).
      
      The new flag is checked on the following commands:
      
      List blocking commands: BLPOP, BRPOP, BRPOPLPUSH, BLMOVE,
      Zset blocking commands: BZPOPMIN, BZPOPMAX
      Stream blocking commands: XREAD, XREADGROUP
      SUBSCRIBE, PSUBSCRIBE, MONITOR
      In addition, the new flag is turned on inside the AOF client, we do not want to
      block the AOF client to prevent deadlocks and commands ordering issues (and there
      is also an existing assert in the code that verifies it).
      
      To keep backward compatibility on LUA, all the no-script flags on existing commands
      were kept untouched. In addition, a LUA special treatment on XREAD and XREADGROUP was kept.
      
      To keep backward compatibility on MULTI (which today allows SUBSCRIBE, and PSUBSCRIBE).
      We added a special treatment on those commands to allow executing them on MULTI.
      
      The only backward compatibility issue that this PR introduces is that now MONITOR
      is not allowed inside MULTI.
      
      Tests were added to verify blocking commands are not blocking the client on LUA, MULTI,
      or RM_Call. Tests were added to verify the module can check for CLIENT_DENY_BLOCKING flag.
      Co-authored-by: default avatarOran Agra <oran@redislabs.com>
      Co-authored-by: default avatarItamar Haber <itamar@redislabs.com>
      d87a0d02