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

Update test cases for sentinel #9408

Merged
merged 4 commits into from
Sep 5, 2021
Merged

Conversation

hwware
Copy link
Collaborator

@hwware hwware commented Aug 24, 2021

Used the sentinel debug command to reduce the overall time it takes for sentinel test cases to run.
Test time reduced from 3 minutes to 1 minute 40 seconds in local environment.

Copy link
Member

@yossigo yossigo left a comment

Choose a reason for hiding this comment

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

@hwware Do you think it would make sense to embed the custom timeout settings in init-tests.tcl so basically every sentinel that we start will always have these settings?

@hwware
Copy link
Collaborator Author

hwware commented Aug 30, 2021

@yossigo I tried that first but not all the test cases work with the same setting and some require setting the settings at different points.

@yossigo yossigo merged commit 763fd09 into redis:unstable Sep 5, 2021
@yossigo
Copy link
Member

yossigo commented Sep 5, 2021

Merged, thanks @hwware!

JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Sep 8, 2021
Use sentinel debug to reduce default timeouts and allow tests to execute faster.
enjoy-binbin added a commit to enjoy-binbin/redis that referenced this pull request Mar 10, 2023
In redis#9408, we added some SENTINEL DEBUG to reduce default
timeouts and allow tests to execute faster. The change
in 05-manual.tcl may cause a race that SENTINEL FAILOVER
response with a NOGOODSLAVE:
```
Manual failover works: FAILED: Expected NOGOODSLAVE No suitable replica to promote eq "OK" (context: type eval line 6 cmd {assert {$reply eq "OK"}} proc ::test)
(Jumping to next unit after error)
FAILED: caught an error in the test
assertion:Expected NOGOODSLAVE No suitable replica to promote eq "OK" (context: type eval line 6 cmd {assert {$reply eq "OK"}} proc ::test)
```

The reason is that the info-period value was reduced in redis#9408
(the default value is 10000), and then manual failover was
performed immediately, but the INFO may not exchanged between
the sentinel and replicas, causing the sentinel to skip all
the replicas in sentinelSelectSlave (Because replica's info_refresh
is not updated, see the code snippet below), then return a NOGOODSLAVE,
break the test.

Code snippet from sentinelSelectSlave:
```
while((de = dictNext(di)) != NULL) {
    sentinelRedisInstance *slave = dictGetVal(de);
    mstime_t info_validity_time;
    if (master->flags & SRI_S_DOWN)
        info_validity_time = sentinel_ping_period*5;
    else
        info_validity_time = sentinel_info_period*3;
    if (mstime() - slave->info_refresh > info_validity_time) continue;
}
```

By adding a wait_for_condition, we have the opportunity to
let sentinel update the info_period of the replicas.
oranagra pushed a commit that referenced this pull request Mar 12, 2023
In #9408, we added some SENTINEL DEBUG to reduce default
timeouts and allow tests to execute faster. The change
in 05-manual.tcl may cause a race that SENTINEL FAILOVER
response with a NOGOODSLAVE:
```
Manual failover works: FAILED: Expected NOGOODSLAVE No suitable replica to promote eq "OK" (context: type eval line 6 cmd {assert {$reply eq "OK"}} proc ::test)
(Jumping to next unit after error)
FAILED: caught an error in the test
assertion:Expected NOGOODSLAVE No suitable replica to promote eq "OK" (context: type eval line 6 cmd {assert {$reply eq "OK"}} proc ::test)
```

The reason is that the info-period value was reduced in #9408
(the default value is 10000), and then manual failover was
performed immediately, but the INFO may not exchanged between
the sentinel and replicas, causing the sentinel to skip all
the replicas in sentinelSelectSlave (Because replica's info_refresh
is not updated, see the code snippet below), then return a NOGOODSLAVE,
break the test.

Code snippet from sentinelSelectSlave:
```
while((de = dictNext(di)) != NULL) {
    sentinelRedisInstance *slave = dictGetVal(de);
    mstime_t info_validity_time;
    if (master->flags & SRI_S_DOWN)
        info_validity_time = sentinel_ping_period*5;
    else
        info_validity_time = sentinel_info_period*3;
    if (mstime() - slave->info_refresh > info_validity_time) continue;
}
```

By adding a wait_for_condition, we have the opportunity to
let sentinel update the info_period of the replicas.
enjoy-binbin added a commit to enjoy-binbin/redis that referenced this pull request Jul 31, 2023
In redis#9408, we added some SENTINEL DEBUG to reduce default
timeouts and allow tests to execute faster. The change
in 05-manual.tcl may cause a race that SENTINEL FAILOVER
response with a NOGOODSLAVE:
```
Manual failover works: FAILED: Expected NOGOODSLAVE No suitable replica to promote eq "OK" (context: type eval line 6 cmd {assert {$reply eq "OK"}} proc ::test)
(Jumping to next unit after error)
FAILED: caught an error in the test
assertion:Expected NOGOODSLAVE No suitable replica to promote eq "OK" (context: type eval line 6 cmd {assert {$reply eq "OK"}} proc ::test)
```

The reason is that the info-period value was reduced in redis#9408
(the default value is 10000), and then manual failover was
performed immediately, but the INFO may not exchanged between
the sentinel and replicas, causing the sentinel to skip all
the replicas in sentinelSelectSlave (Because replica's info_refresh
is not updated, see the code snippet below), then return a NOGOODSLAVE,
break the test.

Code snippet from sentinelSelectSlave:
```
while((de = dictNext(di)) != NULL) {
    sentinelRedisInstance *slave = dictGetVal(de);
    mstime_t info_validity_time;
    if (master->flags & SRI_S_DOWN)
        info_validity_time = sentinel_ping_period*5;
    else
        info_validity_time = sentinel_info_period*3;
    if (mstime() - slave->info_refresh > info_validity_time) continue;
}
```

By adding a wait_for_condition, we have the opportunity to
let sentinel update the info_period of the replicas.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants