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

Improve stability of new CSC eviction test #8160

Merged
merged 1 commit into from
Dec 8, 2020

Conversation

oranagra
Copy link
Member

@oranagra oranagra commented Dec 8, 2020

c4fdf09 added a test that now fails with valgrind
it fails for two resons:

  1. the test samples the used memory and then limits the maxmemory to
    that value, but it turns out this is not atomic and on slow machines
    the background cron process that clean out old query buffers reduces
    the memory so that the setting doesn't cause eviction.
  2. the dbsize was tested late, after reading some invalidation messages
    by that time more and more keys got evicted, partially draining the
    db. this is not the focus of this fix (still a known limitation)

c4fdf09 added a test that now fails with valgrind
it fails for two resons:
1) the test samples the used memory and then limits the maxmemory to
   that value, but it turns out this is not atomic and on slow machines
   the background cron process that clean out old query buffers reduces
   the memory so that the setting doesn't cause eviction.
2) the dbsize was tested late, after reading some invalidation messages
   by that time more and more keys got evicted, partially draining the
   db. this is not the focus of this fix (still a known limitation)
@oranagra oranagra requested a review from yossigo December 8, 2020 13:17
@oranagra oranagra merged commit a102b21 into redis:unstable Dec 8, 2020
@oranagra oranagra deleted the fix_csc_eviction_test branch December 8, 2020 14:33
oranagra added a commit to oranagra/redis that referenced this pull request Jan 12, 2021
c4fdf09 added a test that now fails with valgrind
it fails for two resons:
1) the test samples the used memory and then limits the maxmemory to
   that value, but it turns out this is not atomic and on slow machines
   the background cron process that clean out old query buffers reduces
   the memory so that the setting doesn't cause eviction.
2) the dbsize was tested late, after reading some invalidation messages
   by that time more and more keys got evicted, partially draining the
   db. this is not the focus of this fix (still a known limitation)

(cherry picked from commit a102b21)
oranagra added a commit that referenced this pull request Jan 12, 2021
c4fdf09 added a test that now fails with valgrind
it fails for two resons:
1) the test samples the used memory and then limits the maxmemory to
   that value, but it turns out this is not atomic and on slow machines
   the background cron process that clean out old query buffers reduces
   the memory so that the setting doesn't cause eviction.
2) the dbsize was tested late, after reading some invalidation messages
   by that time more and more keys got evicted, partially draining the
   db. this is not the focus of this fix (still a known limitation)

(cherry picked from commit a102b21)
JackieXie168 pushed a commit to JackieXie168/redis that referenced this pull request Mar 2, 2021
c4fdf09 added a test that now fails with valgrind
it fails for two resons:
1) the test samples the used memory and then limits the maxmemory to
   that value, but it turns out this is not atomic and on slow machines
   the background cron process that clean out old query buffers reduces
   the memory so that the setting doesn't cause eviction.
2) the dbsize was tested late, after reading some invalidation messages
   by that time more and more keys got evicted, partially draining the
   db. this is not the focus of this fix (still a known limitation)
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