Commit Graph

809 Commits

Author SHA1 Message Date
Madelyn Olson
663fbd3459
Stabilize cluster hostnames tests (#11307)
This PR introduces a couple of changes to improve cluster test stability:
1. Increase the cluster node timeout to 3 seconds, which is similar to the
   normal cluster tests, but introduce a new mechanism to increase the ping
   period so that the tests are still fast. This new config is a debug config.
2. Set `cluster-replica-no-failover yes` on a wider array of tests which are
   sensitive to failovers. This was occurring on the ARM CI.
2022-10-03 09:25:16 +03:00
Binbin
3c02d1acc4
code, typo and comment cleanups (#11280)
- fix `the the` typo
- `LPOPRPUSH` does not exist, should be `RPOPLPUSH`
- `CLUSTER GETKEYINSLOT` 's time complexity should be O(N)
- `there bytes` should be `three bytes`, this closes #11266
- `slave` word to `replica` in log, modified the front and missed the back
- remove useless aofReadDiffFromParent in server.h
- `trackingHandlePendingKeyInvalidations` method adds a void parameter
2022-10-02 13:56:45 +03:00
sundb
f106beebfa
Fix the missing server.dirty increment and redundant signalModifiedKey in serveClientBlockedOnList (#11326)
Mainly fix two minor bug
1. When handle BL*POP/BLMOVE commands with blocked clients, we should increment server.dirty.
2.  `listPopRangeAndReplyWithKey()` in `serveClientBlockedOnList()` should not repeat calling
   `signalModifiedKey()` which has been called when an element was pushed on the list.
   (was skipped in all bpop commands, other than blmpop) 

Other optimization
add `signal` param for `listElementsRemoved` to skip `signalModifiedKey()` to unify all pop operation.

Unifying all pop operations also prepares us for #11303, so that we can avoid having to deal with the
conversion from quicklist to listpack() in various places when the list shrinks.
2022-09-28 21:07:38 +03:00
Shaya Potter
6e993a5dfa
Add RM_SetContextUser to support acl validation in RM_Call (and scripts) (#10966)
Adds a number of user management/ACL validaiton/command execution functions to improve a
Redis module's ability to enforce ACLs correctly and easily.

* RM_SetContextUser - sets a RedisModuleUser on the context, which RM_Call will use to both
  validate ACLs (if requested and set) as well as assign to the client so that scripts executed via
  RM_Call will have proper ACL validation.
* RM_SetModuleUserACLString - Enables one to pass an entire ACL string, not just a single OP
  and have it applied to the user
* RM_GetModuleUserACLString - returns a stringified version of the user's ACL (same format as dump
  and list).  Contains an optimization to cache the stringified version until the underlying ACL is modified.
* Slightly re-purpose the "C" flag to RM_Call from just being about ACL check before calling the
  command, to actually running the command with the right user, so that it also affects commands
  inside EVAL scripts. see #11231
2022-09-22 16:29:00 +03:00
Valentino Geron
e53bf65245
Replica that asks for rdb only should be closed right after the rdb part (#11296)
The bug is that the the server keeps on sending newlines to the client.
As a result, the receiver might not find the EOF marker since it searches
for it only on the end of each payload it reads from the socket.
The but only affects `CLIENT_REPL_RDBONLY`.
This affects `redis-cli --rdb` (depending on timing)

The fixed consist of two steps:
1. The `CLIENT_REPL_RDBONLY` should be closed ASAP (we cannot
   always call to `freeClient` so we use `freeClientAsync`)
2. Add new replication state `SLAVE_STATE_RDB_TRANSMITTED`
2022-09-22 11:22:05 +03:00
Adi Pinsky
d144dc927a
Adds listnode to client struct for clients_pending_write list (#11220) 2022-09-14 22:39:47 -05:00
Shaya Potter
bed6d759bc
Improve cmd_flags for script/functions in RM_Call (#11159)
When RM_Call was used with `M` (reject OOM), `W` (reject writes),
as well as `S` (rejecting stale or write commands in "Script mode"),
it would have only checked the command flags, but not the declared
script flag in case it's a command that runs a script.

Refactoring: extracts out similar code in server.c's processCommand
to be usable in RM_Call as well.
2022-08-28 13:10:10 +03:00
Oran Agra
c789fb0aa7
Fix assertion when a key is lazy expired during cluster key migration (#11176)
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`)
2022-08-24 19:39:15 +03:00
Meir Shpilraien (Spielrein)
c1bd61a4a5
Reverts most of the changes of #10969 (#11178)
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.
2022-08-24 12:51:36 +03:00
Oran Agra
4faddf18ca Build TLS as a loadable module
* 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: zhenwei pi <pizhenwei@bytedance.com>
Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
2022-08-23 12:37:56 +03:00
zhenwei pi
0b27cfe37d Introduce .listen into connection type
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: zhenwei pi <pizhenwei@bytedance.com>
2022-08-22 15:16:08 +08:00
zhenwei pi
eb94d6d36d Introduce unix socket connection type
Unix socket uses different accept handler/create listener from TCP,
to hide these difference to avoid hard code, use a new unix socket
connection type. Also move 'acceptUnixHandler' into unix.c.

Currently, the connection framework becomes like following:

                   uplayer
                      |
               connection layer
                 /    |     \
               TCP   Unix   TLS

It's possible to build Unix socket support as a shared library, and
load it dynamically. Because TCP and Unix socket don't require any
heavy dependencies or overheads, we build them into Redis statically.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
2022-08-22 15:12:31 +08:00
zhenwei pi
0ae02ce95b Abstract accept handler
Abstract accept handler for socket&TLS, and add helper function
'connAcceptHandler' to get accept handler by specified type.

Also move acceptTcpHandler into socket.c, and move
acceptTLSHandler into tls.c.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
2022-08-22 15:12:18 +08:00
zhenwei pi
41fff55d52 Use socketFds for unix
socketFds is also suitable for Unix socket, then we can use
'createSocketAcceptHandler' to create accept handler.
And then, we can abstract accept handler in the future.

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
2022-08-22 15:12:04 +08:00
zhenwei pi
8234a5123d Introduce connection layer framework
Use connTypeRegister() to register a connection type into redis, and
query connection by connectionByType() via type.

With this change, we can hide TLS specified methods into connection
type:
- void tlsInit(void);
- void tlsCleanup(void);
- int tlsConfigure(redisTLSContextConfig *ctx_config);
- int isTlsConfigured(void);

Merge isTlsConfigured & tlsConfigure, use an argument *reconfigure*
to distinguish:
   tlsConfigure(&server.tls_ctx_config)
-> onnTypeConfigure(CONN_TYPE_TLS, &server.tls_ctx_config, 1)

   isTlsConfigured() && tlsConfigure(&server.tls_ctx_config)
-> connTypeConfigure(CONN_TYPE_TLS, &server.tls_ctx_config, 0)

Finally, we can remove USE_OPENSSL from config.c. If redis is built
without TLS, and still run redis with TLS, then redis reports:
 # Missing implement of connection type 1
 # Failed to configure TLS. Check logs for more info.

The log can be optimised, let's leave it in the future. Maybe we can
use connection type as a string.

Although uninitialized fields of a static struct are zero, we still
set them as NULL explicitly in socket.c, let them clear to read & maintain:
    .init = NULL,
    .cleanup = NULL,
    .configure = NULL,

Signed-off-by: zhenwei pi <pizhenwei@bytedance.com>
2022-08-22 15:09:59 +08:00
yourtree
ca6aeadfbe
Support setlocale via CONFIG operation. (#11059)
Till now Redis officially supported tuning it via environment variable see #1074.
But we had other requests to allow changing it at runtime, see #799, and #11041.

Note that `strcoll()` is used as Lua comparison function and also for comparison of
certain string objects in Redis, which leads to a problem that, in different regions,
for some characters, the result may be different. Below is an example.
```
127.0.0.1:6333> SORT test alpha
1) "<"
2) ">"
3) ","
4) "*"
127.0.0.1:6333> CONFIG GET locale-collate
1) "locale-collate"
2) ""
127.0.0.1:6333> CONFIG SET locale-collate 1
(error) ERR CONFIG SET failed (possibly related to argument 'locale')
127.0.0.1:6333> CONFIG SET locale-collate C
OK
127.0.0.1:6333> SORT test alpha
1) "*"
2) ","
3) "<"
4) ">"
```
That will cause accidental code compatibility issues for Lua scripts and some
Redis commands. This commit creates a new config parameter to control the
local environment which only affects `Collate` category. Above shows how it
affects `SORT` command, and below shows the influence on Lua scripts.
```
127.0.0.1:6333> CONFIG GET locale-collate
1) " locale-collate"
2) "C"
127.0.0.1:6333> EVAL "return ',' < '*'" 0
(nil)
127.0.0.1:6333> CONFIG SET locale-collate ""
OK
127.0.0.1:6333> EVAL "return ',' < '*'" 0
(integer) 1
```

Co-authored-by: calvincjli <calvincjli@tencent.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
2022-08-21 17:55:45 +03:00
guybe7
223046ec9a
Repurpose redisCommandArg's name as the unique ID (#11051)
This PR makes sure that "name" is unique for all arguments in the same
level (i.e. all args of a command and all args within a block/oneof).
This means several argument with identical meaning can be referred to together,
but also if someone needs to refer to a specific one, they can use its full path.

In addition, the "display_text" field has been added, to be used by redis.io
in order to render the syntax of the command (for the vast majority it is
identical to "name" but sometimes we want to use a different string
that is not "name")
The "display" field is exposed via COMMAND DOCS and will be present
for every argument, except "oneof" and "block" (which are container
arguments)

Other changes:
1. Make sure we do not have any container arguments ("oneof" or "block")
   that contain less than two sub-args (otherwise it doesn't make sense)
2. migrate.json: both AUTH and AUTH2 should not be "optional"
3. arg names cannot contain underscores, and force the usage of hyphens
  (most of these were a result of the script that generated the initial json files
  from redis.io commands.json).
2022-08-18 15:09:36 +03:00
Meir Shpilraien (Spielrein)
508a138885
Fix replication inconsistency on modules that uses key space notifications (#10969)
Fix replication inconsistency on modules that uses key space notifications.

### The Problem

In general, key space notifications are invoked after the command logic was
executed (this is not always the case, we will discuss later about specific
command that do not follow this rules). For example, the `set x 1` will trigger
a `set` notification that will be invoked after the `set` logic was performed, so
if the notification logic will try to fetch `x`, it will see the new data that was written.
Consider the scenario on which the notification logic performs some write
commands. for example, the notification logic increase some counter,
`incr x{counter}`, indicating how many times `x` was changed.
The logical order by which the logic was executed is has follow:

```
set x 1
incr x{counter}
```

The issue is that the `set x 1` command is added to the replication buffer
at the end of the command invocation (specifically after the key space
notification logic was invoked and performed the `incr` command).
The replication/aof sees the commands in the wrong order:

```
incr x{counter}
set x 1
```

In this specific example the order is less important.
But if, for example, the notification would have deleted `x` then we would
end up with primary-replica inconsistency.

### The Solution

Put the command that cause the notification in its rightful place. In the
above example, the `set x 1` command logic was executed before the
notification logic, so it should be added to the replication buffer before
the commands that is invoked by the notification logic. To achieve this,
without a major code refactoring, we save a placeholder in the replication
buffer, when finishing invoking the command logic we check if the command
need to be replicated, and if it does, we use the placeholder to add it to the
replication buffer instead of appending it to the end.

To be efficient and not allocating memory on each command to save the
placeholder, the replication buffer array was modified to reuse memory
(instead of allocating it each time we want to replicate commands).
Also, to avoid saving a placeholder when not needed, we do it only for
WRITE or MAY_REPLICATE commands.

#### Additional Fixes

* Expire and Eviction notifications:
  * Expire/Eviction logical order was to first perform the Expire/Eviction
    and then the notification logic. The replication buffer got this in the
    other way around (first notification effect and then the `del` command).
    The PR fixes this issue.
  * The notification effect and the `del` command was not wrap with
    `multi-exec` (if needed). The PR also fix this issue.
* SPOP command:
  * On spop, the `spop` notification was fired before the command logic
    was executed. The change in this PR would have cause the replication
    order to be change (first `spop` command and then notification `logic`)
    although the logical order is first the notification logic and then the
    `spop` logic. The right fix would have been to move the notification to
    be fired after the command was executed (like all the other commands),
    but this can be considered a breaking change. To overcome this, the PR
    keeps the current behavior and changes the `spop` code to keep the right
    logical order when pushing commands to the replication buffer. Another PR
    will follow to fix the SPOP properly and match it to the other command (we
    split it to 2 separate PR's so it will be easy to cherry-pick this PR to 7.0 if
    we chose to).

#### Unhanded Known Limitations

* key miss event:
  * On key miss event, if a module performed some write command on the
    event (using `RM_Call`), the `dirty` counter would increase and the read
    command that cause the key miss event would be replicated to the replication
    and aof. This problem can also happened on a write command that open
    some keys but eventually decides not to perform any action. We decided
    not to handle this problem on this PR because the solution is complex
    and will cause additional risks in case we will want to cherry-pick this PR.
    We should decide if we want to handle it in future PR's. For now, modules
    writers is advice not to perform any write commands on key miss event.

#### Testing

* We already have tests to cover cases where a notification is invoking write
  commands that are also added to the replication buffer, the tests was modified
  to verify that the replica gets the command in the correct logical order.
* Test was added to verify that `spop` behavior was kept unchanged.
* Test was added to verify key miss event behave as expected.
* Test was added to verify the changes do not break lazy expiration.

#### Additional Changes

* `propagateNow` function can accept a special dbid, -1, indicating not
  to replicate `select`. We use this to replicate `multi/exec` on `propagatePendingCommands`
  function. The side effect of this change is that now the `select` command
  will appear inside the `multi/exec` block on the replication stream (instead of
  outside of the `multi/exec` block). Tests was modified to match this new behavior.
2022-08-18 10:16:32 +03:00
Oran Agra
ac1cc5a6e1
Trim rdb loading code for pre-release formats (#11058)
The initial module format introduced in 4.0  RC1 and was changed in RC2
The initial function format introduced in 7.0 RC1 and changed in RC3
2022-08-15 21:41:44 +03:00
filipe oliveira
6686c6d774
Avoid the sdslen() on shared.crlf given we know its size beforehand. Improve ~3-4% of cpu cycles to lrange logic (#10987)
* Avoid the sdslen() on shared.crlf given we know its size beforehand
* Removed shared.crlf from sharedObjects
2022-08-04 10:38:20 +03:00
Moti Cohen
1aa6c4ab92
Adding parentheses and do-while(0) to macros (#11080)
Fixing few macros that doesn't follows most basic safety conventions
which is wrapping any usage of passed variable
with parentheses and if written more than one command, then wrap
it with do-while(0) (or parentheses).
2022-08-03 19:38:08 +03:00
Binbin
00097bf4aa
Change the return value of rdbLoad function to enums (#11039)
The reason we do this is because in #11036, we added error
log message when failing to open RDB file for reading.
In loadDdataFromDisk we call rdbLoad and also check errno,
now the logging corrupts errno (reported in alpine daily).

It is not safe to rely on errno as we do today, so we change
the return value of rdbLoad function to enums, like we have
when loading an AOF.
2022-07-26 15:13:13 +03:00
Binbin
95b88f672a
Set RM_StringCompare input args as const (#11010)
Following #10996, it forgot to modify RM_StringCompare in module.c

Modified RM_StringCompare, compareStringObjectsWithFlags,
compareStringObjects and collateStringObjects.
2022-07-19 08:59:39 +03:00
Binbin
8203461120
Fix some outdated comments and some typo (#10946)
* Fix some outdated comments and some typo
2022-07-06 20:31:59 -07:00
Binbin
0132ed7544
Add pubsubshard_channels field in INFO STATS (#10929)
We already have `pubsub_channels` and `pubsub_patterns`
in INFO stats, now add `pubsubshard_channels` (symmetry).

Sharded pubsub was added in #8621
2022-07-06 09:50:08 +03:00
Harkrishn Patro
a3704d4e87
Optimize number of realloc syscall during multi/exec flow (#10921)
## Issue
During the MULTI/EXEC flow, each command gets queued until the `EXEC`
command is received and during this phase on every command queue, a
`realloc` is being invoked. This could be expensive based on the realloc
behavior (if copy to a new memory location). 


## Solution
In order to reduce the no. of syscall, couple of optimization I've used.

1. By default, reserve memory for atleast two commands. `MULTI/EXEC` for a
single command doesn't have any significance. Hence, I believe customer wouldn't use it.
2. For further reservation, increase the memory allocation in exponent growth (power of 2).
This reduces the no. of `realloc` call from `N` to `log(N)` times.

## Other changes:

* Include multi exec queued command array in client memory consumption calculation
(affects client eviction too)
2022-07-04 09:47:34 +03:00
Harkrishn Patro
0ab885a685
Account sharded pubsub channels memory consumption (#10925)
Account sharded pubsub channels memory consumption in client memory usage
computation to accurately evict client based on the set threshold for `maxmemory-clients`.
2022-07-04 09:18:57 +03:00
Binbin
35e836c26d
Add SENTINEL command flag to CLIENT/COMMANDS subcommands (#10904)
This was harmless because we marked the parent command
with SENTINEL flag. So the populateCommandTable was ok.
And we also don't show the flag (SENTINEL and ONLY-SENTNEL)
in COMMAND INFO.

In this PR, we also add the same CMD_SENTINEL and CMD_ONLY_SENTINEL
flags check when populating the sub-commands.
so that in the future it'll be possible to add some sub-commands to sentinel or sentinel-only but not others.
2022-06-30 16:32:40 +03:00
Viktor Söderqvist
6272ca609e
Add RM_SetClientNameById and RM_GetClientNameById (#10839)
Adding Module APIs to let the module read and set the client name of an arbitrary connection.
2022-06-26 14:34:59 +03:00
Binbin
92fb4f4f61
Fixed SET and BITFIELD commands being wrongly marked movablekeys (#10837)
The SET and BITFIELD command were added `get_keys_function` in #10148, causing
them to be wrongly marked movablekeys in `populateCommandMovableKeys`.

This was an unintended side effect introduced in #10148 (7.0 RC1)
which could cause some clients an extra round trip for these commands in cluster mode.

Since we define movablekeys as a way to determine if the legacy range [first, last, step]
doesn't find all keys, then we need a completely different approach.

The right approach should be to check if the legacy range covers all key-specs,
and if none of the key-specs have the INCOMPLETE flag. 
This way, we don't need to look at getkeys_proc of VARIABLE_FLAG at all.
Probably with the exception of modules, who may still not be using key-specs.

In this PR, we removed `populateCommandMovableKeys` and put its logic in
`populateCommandLegacyRangeSpec`.
In order to properly serve both old and new modules, we must probably keep relying
CMD_MODULE_GETKEYS, but do that only for modules that don't declare key-specs. 
For ones that do, we need to take the same approach we take with native redis commands.

This approach was proposed by Oran. Fixes #10833

Co-authored-by: Oran Agra <oran@redislabs.com>
2022-06-12 08:22:18 +03:00
DarrenJiang13
f558583493
Split instantaneous_repl_total_kbps to instantaneous_input_repl_kbps and instantaneous_output_repl_kbps. (#10810)
A supplement to https://github.com/redis/redis/pull/10062
Split `instantaneous_repl_total_kbps` to `instantaneous_input_repl_kbps` and `instantaneous_output_repl_kbps`. 
## Work:
This PR:
- delete 1 info field:
    - `instantaneous_repl_total_kbps`
- add 2 info fields:
    - `instantaneous_input_repl_kbps / instantaneous_output_repl_kbps`
## Result:
- master
```
total_net_input_bytes:26633673
total_net_output_bytes:21716596
total_net_repl_input_bytes:0
total_net_repl_output_bytes:18433052
instantaneous_input_kbps:0.02
instantaneous_output_kbps:0.00
instantaneous_input_repl_kbps:0.00
instantaneous_output_repl_kbps:0.00
```
- slave
```
total_net_input_bytes:18433212
total_net_output_bytes:94790
total_net_repl_input_bytes:18433052
total_net_repl_output_bytes:0
instantaneous_input_kbps:0.00
instantaneous_output_kbps:0.05
instantaneous_input_repl_kbps:0.00
instantaneous_output_repl_kbps:0.00
```
2022-06-06 08:29:24 +03:00
Madelyn Olson
4ad166235e
Update time independent string compare to use hash length (#9759)
* Update time independent string compare to use hash length
2022-06-03 09:30:28 -07:00
Oran Agra
df55861838
Expose script flags to processCommand for better handling (#10744)
The important part is that read-only scripts (not just EVAL_RO
and FCALL_RO, but also ones with `no-writes` executed by normal EVAL or
FCALL), will now be permitted to run during CLIENT PAUSE WRITE (unlike
before where only the _RO commands would be processed).

Other than that, some errors like OOM, READONLY, MASTERDOWN are now
handled by processCommand, rather than the command itself affects the
error string (and even error code in some cases), and command stats.

Besides that, now the `may-replicate` commands, PFCOUNT and PUBLISH, will
be considered `write` commands in scripts and will be blocked in all
read-only scripts just like other write commands.
They'll also be blocked in EVAL_RO (i.e. even for scripts without the
`no-writes` shebang flag.

This commit also hides the `may_replicate` flag from the COMMAND command
output. this is a **breaking change**.

background about may_replicate:
We don't want to expose a no-may-replicate flag or alike to scripts, since we
consider the may-replicate thing an internal concern of redis, that we may
some day get rid of.
In fact, the may-replicate flag was initially introduced to flag EVAL: since
we didn't know what it's gonna do ahead of execution, before function-flags
existed). PUBLISH and PFCOUNT, both of which because they have side effects
which may some day be fixed differently.

code changes:
The changes in eval.c are mostly code re-ordering:
- evalCalcFunctionName is extracted out of evalGenericCommand
- evalExtractShebangFlags is extracted luaCreateFunction
- evalGetCommandFlags is new code
2022-06-01 14:09:40 +03:00
Oran Agra
b2061de2e7
Fix broken protocol in MISCONF error, RM_Yield bugs, RM_Call(EVAL) OOM check bug, and new RM_Call checks. (#10786)
* Fix broken protocol when redis can't persist to RDB (general commands, not
  modules), excessive newline. regression of #10372 (7.0 RC3)
* Fix broken protocol when Redis can't persist to AOF (modules and
  scripts), missing newline.
* Fix bug in OOM check of EVAL scripts called from RM_Call.
  set the cached OOM state for scripts before executing module commands too,
  so that it can serve scripts that are executed by modules.
  i.e. in the past EVAL executed by RM_Call could have either falsely
  fail or falsely succeeded because of a wrong cached OOM state flag.
* Fix bugs with RM_Yield:
  1. SHUTDOWN should only accept the NOSAVE mode
  2. Avoid eviction during yield command processing.
  3. Avoid processing master client commands while yielding from another client
* Add new two more checks to RM_Call script mode.
  1. READONLY You can't write against a read only replica
  2. MASTERDOWN Link with MASTER is down and `replica-serve-stale-data` is set to `no`
* Add new RM_Call flag to let redis automatically refuse `deny-oom` commands
  while over the memory limit. 
* Add tests to cover various errors from Scripts, Modules, Modules
  calling scripts, and Modules calling commands in script mode.

Add tests:
* Looks like the MISCONF error was completely uncovered by the tests,
  add tests for it, including from scripts, and modules
* Add tests for NOREPLICAS from scripts
* Add tests for the various errors in module RM_Call, including RM_Call that
  calls EVAL, and RM_call in "eval mode". that includes:
  NOREPLICAS, READONLY, MASTERDOWN, MISCONF
2022-06-01 13:04:22 +03:00
filipe oliveira
6a6e911f12
Moving client flags to a more cache friendly position within client struct (#10697)
Move the client flags to a more cache friendly position within the client struct
we regain the lost 2% of CPU cycles since v6.2 ( from 630532.57 to 647449.80 ops/sec ).
These are due to higher rate of calls to getClientType due to changes in #9166 and #10020
2022-06-01 10:50:01 +03:00
DarrenJiang13
bb1de082ea
Adds isolated netstats for replication. (#10062)
The amount of `server.stat_net_output_bytes/server.stat_net_input_bytes`
is actually the sum of replication flow and users' data flow. 
It may cause confusions like this:
"Why does my server get such a large output_bytes while I am doing nothing? ". 

After discussions and revisions, now here is the change about what this
PR brings (final version before merge):
- 2 server variables to count the network bytes during replication,
     including fullsync and propagate bytes.
     - `server.stat_net_repl_output_bytes`/`server.stat_net_repl_input_bytes`
- 3 info fields to print the input and output of repl bytes and instantaneous
     value of total repl bytes.
     - `total_net_repl_input_bytes` / `total_net_repl_output_bytes`
     - `instantaneous_repl_total_kbps`
- 1 new API `rioCheckType()` to check the type of rio. So we can use this
     to distinguish between diskless and diskbased replication
- 2 new counting items to keep network statistics consistent between master
     and slave
    - rdb portion during diskless replica. in `rdbLoadProgressCallback()`
    - first line of the full sync payload. in `readSyncBulkPayload()`

Co-authored-by: Oran Agra <oran@redislabs.com>
2022-05-31 08:07:33 +03:00
Harkrishn Patro
4065b4f27e
Sharded pubsub publish messagebulk as smessage (#10792)
To easily distinguish between sharded channel message and a global
channel message, introducing `smessage` (instead of `message`) as
message bulk for sharded channel publish message.

This is gonna be a breaking change in 7.0.1!

Background:
Sharded pubsub introduced in redis 7.0, but after the release we quickly
realized that the fact that it's problematic that the client can't distinguish
between normal (global) pubsub messages and sharded ones.
This is important because the same connection can subscribe to both,
but messages sent to one pubsub system are not propagated to the
other (they're completely separate), so if one connection is used to
subscribe to both, we need to assist the client library to know which
message it got so it can forward it to the correct callback.
2022-05-31 08:03:59 +03:00
Binbin
f18c9da3e1
loadAppendOnlyFiles and loadSingleAppendOnlyFile ret should be AOF_OK, not C_OK (#10790)
The ret value should be AOF_OK instead of C_OK.
AOF_OK and C_OK are both 0, so this is just a cleanup.
Also updated some outdated comments.
2022-05-29 13:33:16 +03:00
Binbin
18cb4a7d93
Remove ziplist dead code in object.c (#10751)
Remove some dead code in object.c, ziplist is no longer used in 7.0

Some backgrounds:
zipmap - hash: replaced by ziplist in #285
ziplist - hash: replaced by listpack in #8887
ziplist - zset: replaced by listpack in #9366
ziplist - list: replaced by quicklist (listpack) in #2143 / #9740

Moved the location of ziplist.h in the server.c
2022-05-22 12:27:54 +03:00
Qu Chen
a7d6ca9770
Make the check for if script is running or not consistent (#10725)
sometimes it is using `scriptIsRunning()` and other times it is using `server.in_script`.
We should use the `scriptIsRunning()` method consistently throughout the code base.
Removed server.in_script sine it's no longer used / needed.
2022-05-15 14:07:45 +03:00
yoav-steinberg
b16d1c2713
Fix possible regression around TLS config changes. Add VOLATILE_CONFIG flag for volatile configurations. (#10713)
This fixes a possible regression in Redis 7.0.0, in which doing CONFIG SET
on a TLS config would not reload the configuration in case the new config is
the same file as before.

A volatile configuration is a configuration value which is a reference to the
configuration data and not the configuration data itself. In such a case Redis
doesn't know if the config data changed under the hood and can't assume a
change happens only when the config value changes. Therefore it needs to
be applied even when setting a config value to the same value as it was before.
2022-05-12 17:06:26 +03:00
guybe7
815a6f846a
Dediacted member to hold RedisModuleCommand (#10681)
Fix #10552

We no longer piggyback getkeys_proc to hold the RedisModuleCommand struct, when exists

Others:
Use `doesCommandHaveKeys` in `RM_GetCommandKeysWithFlags` and `getKeysSubcommandImpl`.
It causes a very minor behavioral change in commands that don't have actual keys, but have a spec
with `CMD_KEY_NOT_KEY`.
For example, before this command `COMMAND GETKEYS SPUBLISH` would return
`Invalid arguments specified for command` but not it returns `The command has no key arguments`
2022-05-10 14:56:12 +03:00
David CARLIER
bdcd4b3df8
zmalloc_get_rss implementation for haiku. (#10687)
also fixing already defined constants build warning while at it.

Co-authored-by: Oran Agra <oran@redislabs.com>
2022-05-08 15:12:17 +03:00
chenyang8094
46ec6ad98e
Fix bug when AOF enabled after startup. put the new incr file in the manifest only when AOFRW is done. (#10616)
Changes:

- When AOF is enabled **after** startup, the data accumulated during `AOF_WAIT_REWRITE`
  will only be stored in a temp INCR AOF file. Only after the first AOFRW is successful, we will
  add it to manifest file.
  Before this fix, the manifest referred to the temp file which could cause a restart during that
  time to load it without it's base.
- Add `aof_rewrites_consecutive_failures` info field for  aofrw limiting implementation.

Now we can guarantee that these behaviors of MP-AOF are the same as before (past redis releases):
- When AOF is enabled after startup, the data accumulated during `AOF_WAIT_REWRITE` will only
  be stored in a visible place. Only after the first AOFRW is successful, we will add it to manifest file.
- When disable AOF, we did not delete the AOF file in the past so there's no need to change that
  behavior now (yet).
- When toggling AOF off and then on (could be as part of a full-sync), a crash or restart before the
  first rewrite is completed, would result with the previous version being loaded (might not be right thing,
  but that's what we always had).
2022-04-26 16:31:19 +03:00
Eduardo Semprebon
3a1d14259d
Allow configuring signaled shutdown flags (#10594)
The SHUTDOWN command has various flags to change it's default behavior,
but in some cases establishing a connection to redis is complicated and it's easier
for the management software to use signals. however, so far the signals could only
trigger the default shutdown behavior.
Here we introduce the option to control shutdown arguments for SIGTERM and SIGINT.

New config options:
`shutdown-on-sigint [nosave | save] [now] [force]` 
`shutdown-on-sigterm [nosave | save] [now] [force]`

Implementation:
Support MULTI_ARG_CONFIG on createEnumConfig to support multiple enums to be applied as bit flags.

Co-authored-by: Oran Agra <oran@redislabs.com>
2022-04-26 14:34:04 +03:00
Binbin
156836bfe5
Fix syntax error in replicationErrorBehavior enum (#10642)
Missing a typeof, we will get errors like this:
- multiple definition of `replicationErrorBehavior'
- ld: error: duplicate symbol: replicationErrorBehavior

Introduced in #10504
2022-04-26 14:29:05 +03:00
Madelyn Olson
6fa8e4f7af
Set replicas to panic on disk errors, and optionally panic on replication errors (#10504)
* Till now, replicas that were unable to persist, would still execute the commands
  they got from the master, now they'll panic by default, and we add a new
  `replica-ignore-disk-errors` config to change that.
* Till now, when a command failed on a replica or AOF-loading, it only logged a
  warning and a stat, we add a new `propagation-error-behavior` config to allow
  panicking in that state (may become the default one day)

Note that commands that fail on the replica can either indicate a bug that could
cause data inconsistency between the replica and the master, or they could be
in some cases (specifically in previous versions), a result of a command (e.g. EVAL)
that failed on the master, but still had to be propagated to fail on the replica as well.
2022-04-26 13:25:33 +03:00
Madelyn Olson
efcd1bf394
By default prevent cross slot operations in functions and scripts with # (#10615)
Adds the `allow-cross-slot-keys` flag to Eval scripts and Functions to allow
scripts to access keys from multiple slots.
The default behavior is now that they are not allowed to do that (unlike before).
This is a breaking change for 7.0 release candidates (to be part of 7.0.0), but
not for previous redis releases since EVAL without shebang isn't doing this check.

Note that the check is done on both the keys declared by the EVAL / FCALL command
arguments, and also the ones used by the script when making a `redis.call`.

A note about the implementation, there seems to have been some confusion
about allowing access to non local keys. I thought I missed something in our
wider conversation, but Redis scripts do block access to non-local keys.
So the issue was just about cross slots being accessed.
2022-04-26 12:09:21 +03:00
Binbin
119ec91a5a
Fix typos and limit unknown command error message (#10634)
minor cleanup for recent changes.
2022-04-25 17:59:39 +03:00
guybe7
df787764e3
Fix regression not aborting transaction on error, and re-edit some error responses (#10612)
1. Disk error and slave count checks didn't flag the transactions or counted correctly in command stats (regression from #10372  , 7.0 RC3)
2. RM_Call will reply the same way Redis does, in case of non-exisitng command or arity error
3. RM_WrongArtiy will consider the full command name
4. Use lowercase 'u' in "unknonw subcommand" (to align with "unknown command")

Followup work of #10127
2022-04-25 13:08:13 +03:00