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

Fix unexpected resize causing test failure #12960

Merged
merged 1 commit into from
Jan 18, 2024

Conversation

enjoy-binbin
Copy link
Collaborator

Before #12850, we will only try to shrink the dict in serverCron,
which we can control by using a child process, but now every time
we delete a key, the shrink check will be called.

In these test (added in #12802), we meant to disable the resizing,
but druing the delete, the dict will meet the force shrink, like
2 / 128 = 0.015 < 0.2, the delete will trigger a force resize and
will cause the test to fail.

In this commit, we try to keep the load factor at 3 / 128 = 0.023,
that is, do not meet the force shrink.

Before redis#12850, we will only try to shrink the dict in serverCron,
which we can control by using a child process, but now every time
we delete a key, the shrink check will be called.

In these test (added in redis#12802), we meant to disable the resizing,
but druing the delete, the dict will meet the force shrink, like
2 / 128 = 0.015 < 0.2, the delete will trigger a force resize and
will cause the test to fail.

In this commit, we try to keep the load factor at 3 / 128 = 0.023,
that is, do not meet the force shrink.
@enjoy-binbin
Copy link
Collaborator Author

https://github.com/redis/redis/actions/runs/7549063700/job/20552381743#step:6:6511

*** [err]: Redis can trigger resizing in tests/unit/other.tcl
Expected '[Dictionary HT]
Hash table 0 stats (main hash table):
No stats available for empty dictionaries
Hash table 1 stats (rehashing target):
 table size: 4
 number of elements: 1
[Expires HT]
Hash table 0 stats (main hash table):
No stats available for empty dictionaries
' to match '*table size: 128*' (context: type eval line 17 cmd {assert_match "*table size: 128*" [r debug HTSTATS 0]} proc ::test) 

@lyq2333
Copy link
Contributor

lyq2333 commented Jan 18, 2024

I ran the test and it passed before #12850 merged. But the test can pass occasionally even the resizing is triggered depending on the seed of the hash function in main db. 👍

@oranagra oranagra merged commit 29e6245 into redis:unstable Jan 18, 2024
13 checks passed
@enjoy-binbin enjoy-binbin deleted the fix_unexpected_resize branch January 18, 2024 09:20
roggervalf pushed a commit to roggervalf/redis that referenced this pull request Feb 11, 2024
Before redis#12850, we will only try to shrink the dict in serverCron,
which we can control by using a child process, but now every time
we delete a key, the shrink check will be called.

In these test (added in redis#12802), we meant to disable the resizing,
but druing the delete, the dict will meet the force shrink, like
2 / 128 = 0.015 < 0.2, the delete will trigger a force resize and
will cause the test to fail.

In this commit, we try to keep the load factor at 3 / 128 = 0.023,
that is, do not meet the force shrink.
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

3 participants