Skip to content

fix infinite sleep in performEvictions when have lazyfree jobs#11237

Merged
oranagra merged 1 commit intoredis:unstablefrom
soloestoy:bugfix-infinite-sleep-when-evcit
Sep 18, 2022
Merged

fix infinite sleep in performEvictions when have lazyfree jobs#11237
oranagra merged 1 commit intoredis:unstablefrom
soloestoy:bugfix-infinite-sleep-when-evcit

Conversation

@soloestoy
Copy link
Contributor

@soloestoy soloestoy commented Sep 5, 2022

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.

@enjoy-binbin enjoy-binbin requested a review from JimB123 September 5, 2022 11:23
Copy link
Member

@oranagra oranagra left a comment

Choose a reason for hiding this comment

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

LGTM.
let's wait a bit to see if there are any comments by others though.

@oranagra oranagra added the release-notes indication that this issue needs to be mentioned in the release notes label Sep 5, 2022
Copy link
Contributor

@enjoy-binbin enjoy-binbin left a comment

Choose a reason for hiding this comment

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

LGTM

@soloestoy soloestoy force-pushed the bugfix-infinite-sleep-when-evcit branch from c2e7756 to 5428cf1 Compare September 6, 2022 06:35
Copy link
Contributor

@JimB123 JimB123 left a comment

Choose a reason for hiding this comment

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

LGTM. Did someone actually try setting this to 100?

@soloestoy
Copy link
Contributor Author

LGTM. Did someone actually try setting this to 100?

me, and almost all users in our platform.

@yossigo
Copy link
Collaborator

yossigo commented Sep 8, 2022

@soloestoy Did you consider adding a test?
It's a serious issue with non-default settings, avoiding possible regression might be a good idea.

@soloestoy
Copy link
Contributor Author

soloestoy commented Sep 13, 2022

@soloestoy Did you consider adding a test? It's a serious issue with non-default settings, avoiding possible regression might be a good idea.

@yossigo I tried, but we cannot make lazyfree and eviction atomic, since the lazyfree time is nondeterministic, do you have another way to test it?

@oranagra oranagra merged commit 464aa04 into redis:unstable Sep 18, 2022
@oranagra oranagra mentioned this pull request Sep 21, 2022
oranagra pushed a commit that referenced this pull request Sep 21, 2022
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.

(cherry picked from commit 464aa04)
@oranagra oranagra mentioned this pull request Dec 11, 2022
oranagra pushed a commit that referenced this pull request Dec 12, 2022
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.

(cherry picked from commit 464aa04)
madolson pushed a commit to madolson/redis that referenced this pull request Apr 19, 2023
…#11237)

This bug is introduced in redis#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.
enjoy-binbin pushed a commit to enjoy-binbin/redis that referenced this pull request Jul 31, 2023
…#11237)

This bug is introduced in redis#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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes indication that this issue needs to be mentioned in the release notes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

6 participants