1. 22 Sep, 2022 1 commit
    • Binbin's avatar
      ACL default newly created user set USER_FLAG_SANITIZE_PAYLOAD flag (#11279) · bb6513cb
      Binbin authored
      Starting from 6.2, after ACL SETUSER user reset, the user
      will carry the sanitize-payload flag. It was added in #7807,
      and then ACL SETUSER reset is inconsistent with default
      newly created user which missing sanitize-payload flag.
      
      Same as `off` and `on` these two bits are mutually exclusive,
      the default created user needs to have sanitize-payload flag.
      Adds USER_FLAG_SANITIZE_PAYLOAD flag to ACLCreateUser.
      
      Note that the bug don't have any real implications,
      since the code in rdb.c (rdbLoadObject) checks for
      `USER_FLAG_SANITIZE_PAYLOAD_SKIP`, so the fact that
      `USER_FLAG_SANITIZE_PAYLOAD` is missing doesn't really matters.
      
      Added tests to make sure it won't be broken in the future,
      and updated the comment in ACLSetUser and redis.conf
      bb6513cb
  2. 21 Sep, 2022 1 commit
  3. 19 Sep, 2022 2 commits
    • Binbin's avatar
      Fix Invalid node address specified in redis-cli --cluster create/add-node (#11151) · c2b0c13d
      Binbin authored
      This bug was introduced in #10344 (7.0.3), and it breaks the
      redis-cli --cluster create usage in #10436 (7.0 RC3).
      
      At the same time, the cluster-port support introduced in #10344
      cannot use the DNS lookup brought by #10436.
      c2b0c13d
    • sundb's avatar
      Fix crash due to delete entry from compress quicklistNode and wrongly split quicklistNode (#11242) · 13d25dd9
      sundb authored
      This PR mainly deals with 2 crashes introduced in #9357,
      and fix the QUICKLIST-PACKED-THRESHOLD mess in external test mode.
      
      1. Fix crash due to deleting an entry from a compress quicklistNode
         When inserting a large element, we need to create a new quicklistNode first,
         and then delete its previous element, if the node where the deleted element is
         located is compressed, it will cause a crash.
         Now add `dont_compress` to quicklistNode, if we want to use a quicklistNode
         after some operation, we can use this flag like following:
      
          ```c
          node->dont_compress = 1; /* Prevent to be compressed */
          some_operation(node); /* This operation might try to compress this node */
          some_other_operation(node); /* We can use this node without decompress it */
          node->dont_compress = 0; /* Re-able compression */
          quicklistCompressNode(node);
          ```
      
         Perhaps in the future, we could just disable the current entry from being
         compressed during the iterator loop, but that would require more work.
      
      2. Fix crash due to wrongly split quicklist
         before #9357, the offset param of _quicklistSplitNode() will not negative.
         For now, when offset is negative, the split extent will be wrong.
         following example:
          ```c
          int orig_start = after ? offset + 1 : 0;
          int orig_extent = after ? -1 : offset;
          int new_start = after ? 0 : offset;
          int new_extent = after ? offset + 1 : -1;
          # offset: -2, after: 1, node->count: 2
          # current wrong range: [-1,-1] [0,-1]
          # correct range: [1,-1] [0, 1]
          ```
      
         Because only `_quicklistInsert()` splits the quicklistNode and only
         `quicklistInsertAfter()`, `quicklistInsertBefore()` call _quicklistInsert(), 
         so `quicklistReplaceEntry()` and `listTypeInsert()` might occur this crash.
         But the iterator of `listTypeInsert()` is alway from head to tail(iter->offset is
         always positive), so it is not affected.
         The final conclusion is this crash only occur when we insert a large element
         with negative index into a list, that affects `LSET` command and `RM_ListSet`
         module api.
           
      3. In external test mode, we need to restore quicklist packed threshold after
         when the end of test.
      4. Show `node->count` in quicklistRepr().
      5. Add new tcl proc `config_get_set` to support restoring config in tests.
      13d25dd9
  4. 18 Sep, 2022 1 commit
    • zhaozhao.zz's avatar
      fix infinite sleep in performEvictions when have lazyfree jobs (#11237) · 464aa041
      zhaozhao.zz authored
      This bug is introduced in #7653. (Redis 6.2.0)
      
      When `server.maxmemory_eviction_tenacity` is 100, `eviction_time_limit_us` is
      `ULONG_MAX`, and if we cannot find the best key to delete (e.g. maxmemory-policy
      is `volatile-lru` and all keys with ttl have been evicted), in `cant_free` redis will sleep
      forever if some items are being freed in the lazyfree thread.
      464aa041
  5. 15 Sep, 2022 1 commit
  6. 14 Sep, 2022 1 commit
  7. 13 Sep, 2022 1 commit
  8. 11 Sep, 2022 1 commit
    • Eduardo Semprebon's avatar
      Improve redis.conf documentation on repl-diskless-load (#11213) · 36abc0fa
      Eduardo Semprebon authored
      Just noticed that there are some inaccurate, or at least confusing information about `repl-diskless-load` in `redis.conf`
      It shouldn't scare away users willing to spend the extra memory.
      
      `may mean that we have to flush the contents of the current database before the full rdb was received.`: this is likely related to the time when there was an option `always`, where content on replica was flushed before loading from master.
      36abc0fa
  9. 08 Sep, 2022 4 commits
  10. 06 Sep, 2022 3 commits
  11. 05 Sep, 2022 3 commits
  12. 31 Aug, 2022 1 commit
  13. 29 Aug, 2022 1 commit
  14. 28 Aug, 2022 4 commits
  15. 26 Aug, 2022 2 commits
  16. 24 Aug, 2022 6 commits
    • Oran Agra's avatar
      Fix assertion when a key is lazy expired during cluster key migration (#11176) · c789fb0a
      Oran Agra authored
      Redis 7.0 has #9890 which added an assertion when the propagation queue
      was not flushed and we got to beforeSleep.
      But it turns out that when processCommands calls getNodeByQuery and
      decides to reject the command, it can lead to a key that was lazy
      expired and is deleted without later flushing the propagation queue.
      
      This change prevents lazy expiry from deleting the key at this stage
      (not as part of a command being processed in `call`)
      c789fb0a
    • Binbin's avatar
      Bump codespell from 2.1.0 to 2.2.1 in /.codespell (#11184) · 78259826
      Binbin authored
      add a few terms to the white list, and fix a few newly detected typos
      78259826
    • Brad Dunbar's avatar
      Cleanup in GETDEL: Strings are never freed lazily (#11175) · c0721237
      Brad Dunbar authored
      The GETDEL command only operates on strings, and strings are never freed
      lazily, so there's no need to use `dbAsyncDelete` or `shared.unlink`.
      c0721237
    • Meir Shpilraien (Spielrein)'s avatar
      fix test timeout wait command (#11181) · 3603f194
      Meir Shpilraien (Spielrein) authored
      Fix `Test replication with lazy expire` test to not timeout the wait command.
      This fix will allow the test to pass on slow environments and when running with valgrind.
      3603f194
    • Meir Shpilraien (Spielrein)'s avatar
      Reverts most of the changes of #10969 (#11178) · c1bd61a4
      Meir Shpilraien (Spielrein) authored
      The PR reverts the changes made on #10969.
      The reason for revert was trigger because of occasional test failure
      that started after the PR was merged.
      
      The issue is that if there is a lazy expire during the command invocation,
      the `del` command is added to the replication stream after the command
      placeholder. So the logical order on the primary is:
      
      * Delete the key (lazy expiration)
      * Command invocation
      
      But the replication stream gets it the other way around:
      
      * Command invocation (because the command is written into the placeholder)
      * Delete the key (lazy expiration)
      
      So if the command write to the key that was just lazy expired we will get
      inconsistency between primary and replica.
      
      One solution we considered is to add another lazy expire replication stream
      and write all the lazy expire there. Then when replicating, we will replicate the
      lazy expire replication stream first. This will solve this specific test failure but
      we realize that the issues does not ends here and the more we dig the more
      problems we find.One of the example we thought about (that can actually
      crashes Redis) is as follow:
      
      * User perform SINTERSTORE
      * When Redis tries to fetch the second input key it triggers lazy expire
      * The lazy expire trigger a module logic that deletes the first input key
      * Now Redis hold the robj of the first input key that was actually freed
      
      We believe we took the wrong approach and we will come up with another
      PR that solve the problem differently, for now we revert the changes so we
      will not have the tests failure.
      
      Notice that not the entire code was revert, some parts of the PR are changes
      that we would like to keep. The changes that **was** reverted are:
      
      * Saving a placeholder for replication at the beginning of the command (`call` function)
      * Order of the replication stream on active expire and eviction (we will decide how
        to handle it correctly on follow up PR)
      * `Spop` changes are no longer needed (because we reverted the placeholder code)
      
      Changes that **was not** reverted:
      
      * On expire/eviction, wrap the `del` and the notification effect in a multi exec.
      * `PropagateNow` function can still accept a special dbid, -1, indicating not to replicate select.
      * Keep optimisation for reusing the `alsoPropagate` array instead of allocating it each time.
      
      Tests:
      
      * All tests was kept and only few tests was modify to work correctly with the changes
      * Test was added to verify that the revert fixes the issues.
      c1bd61a4
    • Oran Agra's avatar
      Merge: Fully abstract connection and make TLS dynamically loadable (#9320) · 41d9eb02
      Oran Agra authored
      There are many commits in this PR, the detailed changes is described
      in each commit message.
      
      ### Main changes in this PR
      
      * Fully abstract connection type, and hide connection type specified methods.
        Ex, currently TLS class looks like:
      ```
      static ConnectionType CT_TLS = {
          /* connection type */
          .get_type = connTLSGetType,
      
          /* connection type initialize & finalize & configure */
          .init = tlsInit,
          .cleanup = tlsCleanup,
          .configure = tlsConfigure,
      
          /* ae & accept & listen & error & address handler */
          .ae_handler = tlsEventHandler,
          .accept_handler = tlsAcceptHandler,
          .addr = connTLSAddr,
          .listen = connTLSListen,
      
          /* create/close connection */
          .conn_create = connCreateTLS,
          .conn_create_accepted = connCreateAcceptedTLS,
          .close = connTLSClose,
      
          /* connect & accept */
          .connect = connTLSConnect,
          .blocking_connect = connTLSBlockingConnect,
          .accept = connTLSAccept,
      
          /* IO */
          .read = connTLSRead,
          .write = connTLSWrite,
          .writev = connTLSWritev,
          .set_write_handler = connTLSSetWriteHandler,
          .set_read_handler = connTLSSetReadHandler,
          .get_last_error = connTLSGetLastError,
          .sync_write = connTLSSyncWrite,
          .sync_read = connTLSSyncRead,
          .sync_readline = connTLSSyncReadLine,
      
          /* pending data */
          .has_pending_data = tlsHasPendingData,
          .process_pending_data = tlsProcessPendingData,
      
          /* TLS specified methods */
          .get_peer_cert = connTLSGetPeerCert,
      };
      
      int RedisRegisterConnectionTypeTLS()
      {
          return connTypeRegister(&CT_TLS);
      }
      ```
      
      * Also abstract Unix socket class. Currently, the connection framework becomes like:
      ```
                             uplayer
                                |
                         connection layer
                           /    |     \
                         TCP   Unix   TLS
          
      ```
      
      * It's possible to build TLS as a shared library (`make BUILD_TLS=module`).
        Loading the shared library(redis-tls.so) into Redis by Redis module subsystem,
        and Redis starts to listen TLS port. Ex:
      ```
          ./src/redis-server --tls-port 6379 --port 0 \
              --tls-cert-file ./tests/tls/redis.crt \
              --tls-key-file ./tests/tls/redis.key \
              --tls-ca-cert-file ./tests/tls/ca.crt \
              --loadmodule src/redis-tls.so
      ```
      
      ### Interface changes
      * RM_GetContextFlags supports a new flag: REDISMODULE_CTX_FLAGS_SERVER_STARTUP
      * INFO SERVER includes a list of listeners:
      ```
      listener0:name=tcp,bind=127.0.0.1,port=6380
      listener1:name=unix,bind=/run/redis.sock
      listener2:name=tls,bind=127.0.0.1,port=6379
      ```
      
      ### Other notes
      
      * Fix wrong signature of RedisModuleDefragFunc, this could break
        compilation of a module, but not the ABI
      * Some reordering of initialization order in server.c:
        * Move initialization of listeners to be after loading the modules
        * Config TLS after initialization of listeners
        * Init cluster after initialization of listeners
      * Sentinel does not support the TLS module or any connection module
        since it uses hiredis for outbound connections, so when TLS is built as
        a module, sentinel lacks TLS support.
      41d9eb02
  17. 23 Aug, 2022 2 commits
    • Oran Agra's avatar
      Build TLS as a loadable module · 4faddf18
      Oran Agra authored
      
      
      * Support BUILD_TLS=module to be loaded as a module via config file or
        command line. e.g. redis-server --loadmodule redis-tls.so
      * Updates to redismodule.h to allow it to be used side by side with
        server.h by defining REDISMODULE_CORE_MODULE
      * Changes to server.h, redismodule.h and module.c to avoid repeated
        type declarations (gcc 4.8 doesn't like these)
      * Add a mechanism for non-ABI neutral modules (ones who include
        server.h) to refuse loading if they detect not being built together with
        redis (release.c)
      * Fix wrong signature of RedisModuleDefragFunc, this could break
        compilation of a module, but not the ABI
      * Move initialization of listeners in server.c to be after loading
        the modules
      * Config TLS after initialization of listeners
      * Init cluster after initialization of listeners
      * Add TLS module to CI
      * Fix a test suite race conditions:
        Now that the listeners are initialized later, it's not sufficient to
        wait for the PID message in the log, we need to wait for the "Server
        Initialized" message.
      * Fix issues with moduleconfigs test as a result from start_server
        waiting for "Server Initialized"
      * Fix issues with modules/infra test as a result of an additional module
        present
      
      Notes about Sentinel:
      Sentinel can't really rely on the tls module, since it uses hiredis to
      initiate connections and depends on OpenSSL (won't be able to use any
      other connection modules for that), so it was decided that when TLS is
      built as a module, sentinel does not support TLS at all.
      This means that it keeps using redis_tls_ctx and redis_tls_client_ctx directly.
      
      Example code of config in redis-tls.so(may be use in the future):
      RedisModuleString *tls_cfg = NULL;
      
      void tlsInfo(RedisModuleInfoCtx *ctx, int for_crash_report) {
          UNUSED(for_crash_report);
          RedisModule_InfoAddSection(ctx, "");
          RedisModule_InfoAddFieldLongLong(ctx, "var", 42);
      }
      
      int tlsCommand(RedisModuleCtx *ctx, RedisModuleString **argv, int argc)
      {
          if (argc != 2) return RedisModule_WrongArity(ctx);
          return RedisModule_ReplyWithString(ctx, argv[1]);
      }
      
      RedisModuleString *getStringConfigCommand(const char *name, void *privdata) {
          REDISMODULE_NOT_USED(name);
          REDISMODULE_NOT_USED(privdata);
          return tls_cfg;
      }
      
      int setStringConfigCommand(const char *name, RedisModuleString *new, void *privdata, RedisModuleString **err) {
          REDISMODULE_NOT_USED(name);
          REDISMODULE_NOT_USED(err);
          REDISMODULE_NOT_USED(privdata);
          if (tls_cfg) RedisModule_FreeString(NULL, tls_cfg);
          RedisModule_RetainString(NULL, new);
          tls_cfg = new;
          return REDISMODULE_OK;
      }
      
      int RedisModule_OnLoad(void *ctx, RedisModuleString **argv, int argc)
      {
          ....
          if (RedisModule_CreateCommand(ctx,"tls",tlsCommand,"",0,0,0) == REDISMODULE_ERR)
              return REDISMODULE_ERR;
      
          if (RedisModule_RegisterStringConfig(ctx, "cfg", "", REDISMODULE_CONFIG_DEFAULT, getStringConfigCommand, setStringConfigCommand, NULL, NULL) == REDISMODULE_ERR)
              return REDISMODULE_ERR;
      
          if (RedisModule_LoadConfigs(ctx) == REDISMODULE_ERR) {
              if (tls_cfg) {
                  RedisModule_FreeString(ctx, tls_cfg);
                  tls_cfg = NULL;
              }
              return REDISMODULE_ERR;
          }
          ...
      }
      Co-authored-by: default avatarzhenwei pi <pizhenwei@bytedance.com>
      Signed-off-by: default avatarzhenwei pi <pizhenwei@bytedance.com>
      4faddf18
    • Ariel Shtul's avatar
      [PERF] use snprintf once in addReplyDouble (#11093) · 90223759
      Ariel Shtul authored
      The previous implementation calls `snprintf` twice, the second time used to
      'memcpy' the output of the first, which could be a very large string.
      The new implementation reserves space for the protocol header ahead
      of the formatted double, and then prepends the string length ahead of it.
      
      Measured improvement of simple ZADD of some 25%.
      90223759
  18. 22 Aug, 2022 5 commits
    • Itamar Haber's avatar
      407b5c91
    • zhenwei pi's avatar
      Introduce redis module ctx flag 'server startup' · 89e11486
      zhenwei pi authored
      
      
      A module may be loaded only during initial stage, a typical case is
      connection type shared library.
      
      Introduce REDISMODULE_CTX_FLAGS_SERVER_STARTUP context flag
      to tell the module the stage of Redis. Then the module gets the flag
      by RedisModule_GetContextFlags(ctx), tests flags and returns error in
      onload handler.
      Suggested-by: default avatarOran Agra <oran@redislabs.com>
      Signed-off-by: default avatarzhenwei pi <pizhenwei@bytedance.com>
      89e11486
    • zhenwei pi's avatar
      Introduce redis module ctx flag 'server startup' & 'sentinel' · 8a59c193
      zhenwei pi authored
      
      
      A module may be loaded only during initial stage, a typical case is
      connection type shared library.
      
      Introduce REDISMODULE_CTX_FLAGS_SERVER_STARTUP context flag
      to tell the module the stage of Redis. Then the module gets the flag
      by RedisModule_GetContextFlags(ctx), tests flags and returns error in
      onload handler.
      
      Also introduce 'REDISMODULE_CTX_FLAGS_SENTINEL' context flag to tell
      the module the sentinel mode or not.
      Suggested-by: default avatarOran Agra <oran@redislabs.com>
      Signed-off-by: default avatarzhenwei pi <pizhenwei@bytedance.com>
      8a59c193
    • zhenwei pi's avatar
      Add listeners info string for 'INFO' command · 0c4d2fcc
      zhenwei pi authored
      
      
      Suggested by Oran, add necessary listeners information in 'INFO'
      command. It would be helpful for debug.
      
      Example of this:
      127.0.0.1:6379> INFO SERVER
      redis_version:255.255.255
      ...
      listener0:name=tcp,bind=127.0.0.1,port=6380
      listener1:name=unix,bind=/run/redis.sock
      listener2:name=tls,bind=127.0.0.1,port=6379
      ...
      Signed-off-by: default avatarzhenwei pi <pizhenwei@bytedance.com>
      0c4d2fcc
    • zhenwei pi's avatar
      Introduce .listen into connection type · 0b27cfe3
      zhenwei pi authored
      
      
      Introduce listen method into connection type, this allows no hard code
      of listen logic. Originally, we initialize server during startup like
      this:
          if (server.port)
              listenToPort(server.port,&server.ipfd);
          if (server.tls_port)
              listenToPort(server.port,&server.tlsfd);
          if (server.unixsocket)
              anetUnixServer(...server.unixsocket...);
      
          ...
          if (createSocketAcceptHandler(&server.ipfd, acceptTcpHandler) != C_OK)
          if (createSocketAcceptHandler(&server.tlsfd, acceptTcpHandler) != C_OK)
          if (createSocketAcceptHandler(&server.sofd, acceptTcpHandler) != C_OK)
          ...
      
      If a new connection type gets supported, we have to add more hard code
      to setup listener.
      
      Introduce .listen and refactor listener, and Unix socket supports this.
      this allows to setup listener arguments and create listener in a loop.
      
      What's more, '.listen' is defined in connection.h, so we should include
      server.h to import 'struct socketFds', but server.h has already include
      'connection.h'. To avoid including loop(also to make code reasonable),
      define 'struct connListener' in connection.h instead of 'struct socketFds'
      in server.h. This leads this commit to get more changes.
      
      There are more fields in 'struct connListener', hence it's possible to
      simplify changeBindAddr & applyTLSPort() & updatePort() into a single
      logic: update the listener config from the server.xxx, and re-create
      the listener.
      
      Because of the new field 'priv' in struct connListener, we expect to pass
      this to the accept handler(even it's not used currently), this may be used
      in the future.
      Signed-off-by: default avatarzhenwei pi <pizhenwei@bytedance.com>
      0b27cfe3