Make sure replicas don't write their own replies to the replication link#10081
Conversation
The following steps will crash redis-server: ``` [root]# cat crash PSYNC replicationid -1 SLOWLOG GET GET key [root]# nc 127.0.0.1 6379 < crash ``` This one following redis#10020 and the crash was reported in redis#10076. Other changes about the output info: 1. Cmd with a full name by using `getFullCommandName`, now it will print the right subcommand name like `slowlog|get`. 2. Print the full client info by using `catClientInfoString`, the info is also valuable.:
oranagra
left a comment
There was a problem hiding this comment.
LGTM.
@yoav-steinberg anything to comment?
|
Good catch @enjoy-binbin
|
|
we can modify these other places, but i'd like to avoid changing the error message for the output buffer limit (maybe someone depends on it), and i'd like to avoid changing the verbosity level of the other one (may happen too often). |
|
Added regression tests for redis#10020 / redis#10081 / redis#10243. The above PRs fixed some crashes due to an asserting, see function `clientHasPendingReplies` (introduced in redis#9166). This commit added some tests to cover the above scenario. These tests will all fail in redis#9166, althought fixed not, there is value in adding these tests to cover and verify the changes. And it also can cover redis#8868 (verify the logs). Other changes: reduces the wait time in `waitForBgsave` and `waitForBgrewriteaof` from 1s to 50ms, which should reduce the time for some tests.
Added regression tests for #10020 / #10081 / #10243. The above PRs fixed some crashes due to an asserting, see function `clientHasPendingReplies` (introduced in #9166). This commit added some tests to cover the above scenario. These tests will all fail in #9166, althought fixed not, there is value in adding these tests to cover and verify the changes. And it also can cover #8868 (verify the logs). Other changes: 1. Reduces the wait time in `waitForBgsave` and `waitForBgrewriteaof` from 1s to 50ms, which should reduce the time for some tests. 2. Improve the test infra to print context when `assert_match` fails. 3. Improve the test infra to print `$error` when `assert_error` fails. ``` Expected an error matching 'ERR*' but got 'OK' (context: type eval line 4 cmd {assert_error "ERR*" {r set a b}} proc ::test) ```
The following steps will crash redis-server:
This one following #10020 and the crash was reported in #10076.
Add a new common function name
logInvalidUseAndFreeClientthat will log the errors and free the client in async way.
Other changes about the output info:
getFullCommandName, now it will print the right subcommand name likeslowlog|get.catClientInfoString, the info is also valuable.Now the log will like this: