Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

6.2 RC3 #8427

Merged
merged 58 commits into from Feb 1, 2021
Merged

6.2 RC3 #8427

merged 58 commits into from Feb 1, 2021

Conversation

oranagra
Copy link
Member

@oranagra oranagra commented Jan 31, 2021

Upgrade urgency LOW: This is the third Release Candidate of Redis 6.2.

Here is a comprehensive list of changes in this release compared to 6.2 RC2,
each one includes the PR number that added it, so you can get more details
at https://github.com/redis/redis/pull/

New commands / args:

Sentinel:

New configuration options:

Bug fixes:

Special considerations:

Improvements:

Info fields and introspection changes:

Modules:

sundb and others added 30 commits January 13, 2021 11:38
…zunionInterDiffGenericCommand (redis#8316)

* Change zunionInterDiffGenericCommand to use lookupKeyRead if dstkey is null
* Change zrangeGenericCommand to use lookupKey Write if dstkey isn't null

ZRANGESTORE and UNION, ZINTER, ZDIFF are all new commands (6.2 RC1 and RC2).
In redis 6.0 the ZRANGE was using lookupKeyRead, and ZUNIONSTORE / ZINTERSTORE were using lookupKeyWrite.
So there bugs are introduced in 6.2 and will be resolved before it is released.

the implications of this bug are also not big:
The sole difference between LookupKeyRead and LookupKeyWrite is for command executed on a replica, which are not received from its master client. (for the master, and for the master client on the replica, these two functions behave the same)!
Remove several checks that always evaluate to true.
…redis#8182)

Optimize the performance of clusterGenNodesDescription by only checking slot ownership of each slot once, instead of checking each slot for each node.
…dChangeAddress (redis#8319)

Replace many calling zrealloc to one zmalloc in sentinelResetMasterAndChangeAddress
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.
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
…USH[ALL|DB], SCRIPT FLUSH (redis#8258)

* Adds ASYNC and SYNC arguments to SCRIPT FLUSH
* Adds SYNC argument to FLUSHDB and FLUSHALL
* Adds new config to control the default behavior of FLUSHDB, FLUSHALL and SCRIPT FLUASH.

the new behavior is as follows:
* FLUSH[ALL|DB],SCRIPT FLUSH: Determine sync or async according to the
  value of lazyfree-lazy-user-flush.
* FLUSH[ALL|DB],SCRIPT FLUSH ASYNC: Always flushes the database in an async manner.
* FLUSH[ALL|DB],SCRIPT FLUSH SYNC: Always flushes the database in a sync manner.
…d known-sentinel (redis#8332)

Previously invalid configuration errors were not very specific and in some cases hard to understand.

Co-authored-by: Yossi Gottlieb <yossigo@gmail.com>
This adds basic coverage to IO threads by running the cluster and few selected Redis test suite tests with the IO threads enabled.

Also provides some necessary additional improvements to the test suite:

* Add --config to sentinel/cluster tests for arbitrary configuration.
* Fix --tags whitelisting which was broken.
* Add a `network` tag to some tests that are more network intensive. This is work in progress and more tests should be properly tagged in the future.
Don't bother to call fcntl if the flags are not gonna be changed.
…edis#8356)

This was a regression from redis#7625 (only in 6.2 RC2).

This makes it possible again to implement blocking list and zset
commands using the modules API.

This commit also includes a test case for the reverse: A module
unblocks a client blocked on BLPOP by inserting elements using
RedisModule_ListPush(). This already works, but it was untested.
…redis#8357)

This commit adds tests to make sure that relative and absolute expire commands
are propagated as is to replicas and stop any future attempt to change that without
a proper discussion. see redis#8327 and redis#5171

Additionally it slightly improve the AOF test that tests the opposite (always
propagating absolute times), by covering more commands, and shaving 2
seconds from the test time.
The line that said:

Note: you can disable saving completely by commenting out all "save" lines

was incorrect.

Also, note that the save values used in the default redis.conf file are different than the server's defaults, so this PR also addresses this and introduces a potential behavior change for some users.
…runs (redis#8242)

Sentinel uses execve to run scripts, so it needs to use FD_CLOEXEC
on all file descriptors, so that they're not accessible by the script it runs.

This commit includes a change to the sentinel tests, which verifies no
FDs are left opened when the script is executed.
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 (**...**)
…#8367)

The output for COMMAND command was wrong for some commands.
clients can use firstkey,lastkey,step to find (some) key name arguments, and the
"movablekeys" flag to know that they can't know all (or any) of the key name arguments.

These commands had the wrong output:
1. GEORADIUS*_RO used to have "movablekeys" (which it doesn't really need)
2. XREAD and XREADGROUP used to have (1,1,1). but that's completely wrong.
3. Z*STORE used to have (0,0,0) but it can at lest give the index of the dstkey (1,1,1)
the test was misleading because the module would actually woke up on a wrong type and
re-blocked, while the test name suggests the module doesn't not wake up at all on a wrong type..

i changed the name of the test + added verification that indeed the module wakes up and gets
re-blocked after it understand it's the wrong type
Co-authored-by: Oran Agra <oran@redislabs.com>
Fixes a regression introduced due to a new (safer) way of rewriting configuration files. In the past the file was simply overwritten (same inode), but now Redis creates a new temporary file and later renames it over the old one.

The temp file typically gets created with 0600 permissions so we later fchmod it to fix that. Unlike open with O_CREAT, fchmod doesn't consider umask so we have to do that explicitly.

Fixes redis#8369
Fix incorrect documentation about default save time
Update log message to correctly indicate cluster port range
The flag should be set before TLS negotiation begins to avoid a race
condition where a fork+exec before it is completed ends up leaking the
file descriptor.
The module notification was using the wrong value.
…8376)

* Print more details about fd leaks
* temporarily prevent the leaks from failing the tests

Co-authored-by: Oran Agra <oran@redislabs.com>
ShooterIT and others added 20 commits January 27, 2021 17:13
In some scenarios, such as remote backup, we only want to get remote
redis server db snapshot. Currently, redis-cli acts as a replica and
sends SYNC to redis, but redis still accumulates replication buffer
in the replica client output buffer, that may result in using vast
memory, or failing to transfer RDB because of client-output-buffer-limit.
In this commit, we add 'replconf rdb-only 0|1', redis doesn't send
incremental replication buffer to them if they send 'replconf rdb-only 1',
so we can reduce used memory and improve success of getting RDB.
In activeDefragSdsListAndDict when dict_val_type is DEFRAG_SDS_DICT_VAL_VOID_PTR, it should update de->v.val not ln->value.
Because this code path will never be executed, so this bug never happened.
This commit introduces two new command and two options for an existing command

GETEX <key> [PERSIST][EX seconds][PX milliseconds] [EXAT seconds-timestamp]
[PXAT milliseconds-timestamp]

The getexCommand() function implements extended options and variants of the GET
command. Unlike GET command this command is not read-only. Only one of the options
can be used at a given time.

1. PERSIST removes any TTL associated with the key.
2. EX Set expiry TTL in seconds.
3. PX Set expiry TTL in milliseconds.
4. EXAT Same like EX instead of specifying the number of seconds representing the
    TTL (time to live), it takes an absolute Unix timestamp
5. PXAT Same like PX instead of specifying the number of milliseconds representing the
    TTL (time to live), it takes an absolute Unix timestamp

Command would return either the bulk string, error or nil.

GETDEL <key>
Would delete the key after getting.

SET key value [NX] [XX] [KEEPTTL] [GET] [EX <seconds>] [PX <milliseconds>]
[EXAT <seconds-timestamp>][PXAT <milliseconds-timestamp>]

Two new options added here are EXAT and PXAT

Key implementation notes
- `SET` with `PX/EX/EXAT/PXAT` is always translated to `PXAT` in `AOF`. When relative time is
  specified (`PX/EX`), replication will always use `PX`.
- `setexCommand` and `psetexCommand` would no longer need translation in `feedAppendOnlyFile`
  as they are modified to invoke `setGenericCommand ` with appropriate flags which will take care of
  correct AOF translation.
- `GETEX` without any optional argument behaves like `GET`.
- `GETEX` command is never propagated, It is either propagated as `PEXPIRE[AT], or PERSIST`.
- `GETDEL` command is propagated as `DEL`
- Combined the validation for `SET` and `GETEX` arguments. 
- Test cases to validate AOF/Replication propagation
At least in one case the arm64 cow kernel bug test triggers an assert, which is a problem because it cannot be ignored like cases where the bug is found.

On older systems (Linux <4.5) madvise fails because MADV_FREE is not supported. We treat these failures as an indication the system is not affected.

Fixes redis#8351, redis#8406
…edis#3623)

if option `set-proc-title' is no, then do nothing for proc title.

The reason has been explained long ago, see following:

We update redis to 2.8.8, then found there are some side effect when
redis always change the process title.

We run several slave instance on one computer, and all these salves
listen on unix socket only, then ps will show:

  1 S redis 18036 1 0 80 0 - 56130 ep_pol 14:02 ? 00:00:31 /usr/sbin/redis-server *:0
  1 S redis 23949 1 0 80 0 - 11074 ep_pol 15:41 ? 00:00:00 /usr/sbin/redis-server *:0

for redis 2.6 the output of ps is like following:

  1 S redis 18036 1 0 80 0 - 56130 ep_pol 14:02 ? 00:00:31 /usr/sbin/redis-server /etc/redis/a.conf
  1 S redis 23949 1 0 80 0 - 11074 ep_pol 15:41 ? 00:00:00 /usr/sbin/redis-server /etc/redis/b.conf

Later is more informational in our case. The situation
is worse when we manage the config and process running
state by salt. Salt check the process by running "ps |
grep SIG" (for Gentoo System) to check the running
state, where SIG is the string to search for when
looking for the service process with ps. Previously, we
define sig as "/usr/sbin/redis-server
/etc/redis/a.conf". Since the ps output is identical for
our case, so we have no way to check the state of
specified redis instance.

So, for our case, we prefer the old behavior, i.e, do
not change the process title for the main redis process.
Or add an option such as "set-proc-title [yes|no]" to
control this behavior.

Co-authored-by: Yossi Gottlieb <yossigo@gmail.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
This is both a bugfix and an enhancement.

Internally, Sentinel relies entirely on IP addresses to identify
instances. When configured with a new master, it also requires users to
specify and IP and not hostname.

However, replicas may use the replica-announce-ip configuration to
announce a hostname. When that happens, Sentinel fails to match the
announced hostname with the expected IP and considers that a different
instance, triggering reconfiguration, etc.

Another use case is where TLS is used and clients are expected to match
the hostname to connect to with the certificate's SAN attribute. To
properly implement this configuration, it is necessary for Sentinel to
redirect clients to a hostname rather than an IP address.

The new 'resolve-hostnames' configuration parameter determines if
Sentinel is willing to accept hostnames. It is set by default to no,
which maintains backwards compatibility and avoids unexpected DNS
resolution delays on systems with DNS configuration issues.

Internally, Sentinel continues to identify instances by their resolved
IP address and will also report the IP by default. The new
'announce-hostnames' parameter determines if Sentinel should prefer to
announce a hostname, when available, rather than an IP address. This
applies to addresses returned to clients, as well as their
representation in the configuration file, REPLICAOF configuration
commands, etc.

This commit also introduces SENTINEL CONFIG GET and SENTINEL CONFIG SET
which can be used to introspect or configure global Sentinel
configuration that was previously was only possible by directly
accessing the configuration file and possibly restarting the instance.

Co-authored-by: myl1024 <myl92916@qq.com>
Co-authored-by: sundb <sundbcn@gmail.com>
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.
…is#8347)

With AOF policy of fsync "always", redis should respect the contract with the user
that on acknowledged write data is already synced on disk.

Redis was already exiting for AOF write error,  but don't care about fsync failure.
So to guarantee data safe, redis should exit for fsync error too (when the AOF fsync
policy is 'always').
Useful to avoid doing background jobs that can cause excessive COW
Make it possible to customize the process title, i.e. include custom
strings, immutable configuration like port, tls-port, unix socket name,
etc.
* Indicate address can also be a unix socket path name.
* Document the LADDR option as well.
Implement FAILOVER command, which coordinates failover
between the server and one of its replicas.
…dis#8030)

The key point is how to recover from last AOF write error, for example:

1. start redis with appendonly yes, and append some write commands

2. short write or something else error happen, `server.aof_last_write_status` changed to `C_ERR`, now redis doesn't accept write commands

3. execute `CONFIG SET appendonly no` to avoid the above problem, now redis can accept write commands again

4. disk error resolved, and execute `CONFIG SET appendonly yes` to reopen AOF, but `server.aof_last_write_status` cannot be changed to `C_OK` (if background aof rewrite run less then 1 second, it will free `server.aof_buf` and then serverCron cannot fix `aof_last_write_status`), then redis cannot accept write commands forever.

This PR use a simple way to fix it:

1. just free `server.aof_buf` when stop appendonly to save memory, if error happens in `flushAppendOnlyFile(1)`, the `server.aof_buf` may contains some data which has not be written to aof, I think we can ignore it because we turn off the appendonly.

2. reset fsync status after stop appendonly and call `flushAppendOnlyFile` only when `aof_state` is ON

3. reset `server.last_write_status` when reopen aof to accept write commands
New commands:
`HRANDFIELD [<count> [WITHVALUES]]`
`ZRANDMEMBER [<count> [WITHSCORES]]`
Algorithms are similar to the one in SRANDMEMBER.

Both return a simple bulk response when no arguments are given, and an array otherwise.
In case values/scores are requested, RESP2 returns a long array, and RESP3 a nested array.
note: in all 3 commands, the only option that also provides random order is the one with negative count.

Changes to SRANDMEMBER
* Optimization when count is 1, we can use the more efficient algorithm of non-unique random
* optimization: work with sds strings rather than robj

Other changes:
* zzlGetScore: when zset needs to convert string to double, we use safer memcpy (in
  case the buffer is too small)
* Solve a "bug" in SRANDMEMBER test: it intended to test a positive count (case 3 or
  case 4) and by accident used a negative count

Co-authored-by: xinluton <xinluton@qq.com>
Co-authored-by: Oran Agra <oran@redislabs.com>
… on background work clients (redis#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
This commit fixes sentinel announces hostnames test error in certain linux environment
Before this commit, we only check localhost is resolved into 127.0.0.1, however in ubuntu
or some other linux environments "localhost" will be resolved into ::1 ipv6 address first if
the network stack is capable.
* The corrupt dump fuzzer found a division by zero.
* in some cases the random fields from the HRANDFIELD tests produced
  fields with newlines and other special chars (due to \ char), this caused
  the TCL tests to see a bulk response that has a newline in it and add {}
  around it, later it can think this is a nested list. in fact the `alpha` random
  string generator isn't using spaces and newlines, so it should not use `\`
  either.
These tests are not yet stable. on github actions they show some false leaks.
@oranagra oranagra requested a review from a team January 31, 2021 15:54
@oranagra oranagra added the state:major-decision Requires core team consensus label Jan 31, 2021
Copy link
Member

@itamarhaber itamarhaber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First one was needed.
Second one always bothered me, so it can go into the CI with the 1st.

00-RELEASENOTES Show resolved Hide resolved
00-RELEASENOTES Show resolved Hide resolved
Copy link
Contributor

@madolson madolson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@oranagra oranagra merged commit 2dba1e3 into redis:6.2 Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
state:major-decision Requires core team consensus
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet