Skip to content

Conversation

@oranagra
Copy link
Member

@oranagra oranagra commented Jan 30, 2024

tests consistently fail on timeout (sleep that's too short).
it now takes more time because in #12819 we iterate on all dicts, not just non-empty ones.
it passed the PR's CI because it skips the slow tag, which might have been misplaced, but now it is probably required.
with the fix, the tests take quite a lot of time:

[ok]: Redis can trigger resizing (1860 ms)
[ok]: Redis can rewind and trigger smaller slot resizing (744 ms)

before #12819:

[ok]: Redis can trigger resizing (309 ms)
[ok]: Redis can rewind and trigger smaller slot resizing (295 ms)

failure:
https://github.com/redis/redis/actions/runs/7704158180/job/20995931735

*** [err]: expire scan should skip dictionaries with lot's of empty buckets in tests/unit/expire.tcl
scan didn't handle slot skipping logic.
*** [err]: Redis can trigger resizing in tests/unit/other.tcl
Expected '[Dictionary HT]
Hash table 0 stats (main hash table):
 table size: 128
 number of elements: 5
[Expires HT]
Hash table 0 stats (main hash table):
No stats available for empty dictionaries
' to match '*table size: 8*' (context: type eval line 29 cmd {assert_match "*table size: 8*" [r debug HTSTATS 0]} proc ::test) 
*** [err]: Redis can rewind and trigger smaller slot resizing in tests/unit/other.tcl
Expected '[Dictionary HT]
Hash table 0 stats (main hash table):
 table size: 256
 number of elements: 10
[Expires HT]
Hash table 0 stats (main hash table):
No stats available for empty dictionaries
' to match '*table size: 16*' (context: type eval line 27 cmd {assert_match "*table size: 16*" [r debug HTSTATS 0]} proc ::test) 

@oranagra oranagra requested a review from soloestoy January 30, 2024 08:03
@oranagra
Copy link
Member Author

@CharlesChen888 @guybe7 FYI

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.

I have also seen it before. The main reason is that there may be too many empty dicts in tryResizeHashTables and serverCron processes it too slowly.

update hz and use wait can solve this, since most of the time we don’t see so many empty dicts

@oranagra
Copy link
Member Author

i don't yet see how #12819 affects it. both the code before and after it used to iterate 16 dicts in each cron.

@oranagra
Copy link
Member Author

oranagra commented Jan 30, 2024

btw, the PR CI passed because this test is skipped due to --tags -slow
the slow tag on this might have been wrong (copy paste error), but maybe now it became true, and we should keep it.

@guybe7
Copy link
Collaborator

guybe7 commented Jan 30, 2024

i don't yet see how #12819 affects it. both the code before and after it used to iterate 16 dicts in each cron.

before 12819 we used to resize non-empty dicts only

@enjoy-binbin
Copy link
Contributor

i don't yet see how #12819 affects it. both the code before and after it used to iterate 16 dicts in each cron.

It's because before we would skip those empty slots (because test uses hash tag, in fact we should only have one slot), but now we will check every slot

@oranagra oranagra merged commit 7c9f41b into redis:unstable Jan 30, 2024
@oranagra oranagra deleted the fix_dict_resize_tests branch January 30, 2024 12:32
roggervalf pushed a commit to roggervalf/redis that referenced this pull request Feb 11, 2024
…redis#13009)

tests consistently fail on timeout (sleep that's too short).
it now takes more time because in redis#12819 we iterate on all dicts, not
just non-empty ones.
it passed the PR's CI because it skips the `slow` tag, which might have
been misplaced, but now it is probably required.
with the fix, the tests take quite a lot of time:
```
[ok]: Redis can trigger resizing (1860 ms)
[ok]: Redis can rewind and trigger smaller slot resizing (744 ms)
```
before redis#12819:
```
[ok]: Redis can trigger resizing (309 ms)
[ok]: Redis can rewind and trigger smaller slot resizing (295 ms)
```

failure:
https://github.com/redis/redis/actions/runs/7704158180/job/20995931735
```
*** [err]: expire scan should skip dictionaries with lot's of empty buckets in tests/unit/expire.tcl
scan didn't handle slot skipping logic.
*** [err]: Redis can trigger resizing in tests/unit/other.tcl
Expected '[Dictionary HT]
Hash table 0 stats (main hash table):
 table size: 128
 number of elements: 5
[Expires HT]
Hash table 0 stats (main hash table):
No stats available for empty dictionaries
' to match '*table size: 8*' (context: type eval line 29 cmd {assert_match "*table size: 8*" [r debug HTSTATS 0]} proc ::test) 
*** [err]: Redis can rewind and trigger smaller slot resizing in tests/unit/other.tcl
Expected '[Dictionary HT]
Hash table 0 stats (main hash table):
 table size: 256
 number of elements: 10
[Expires HT]
Hash table 0 stats (main hash table):
No stats available for empty dictionaries
' to match '*table size: 16*' (context: type eval line 27 cmd {assert_match "*table size: 16*" [r debug HTSTATS 0]} proc ::test) 
```
funny-dog pushed a commit to funny-dog/redis that referenced this pull request Sep 17, 2025
…redis#13009)

tests consistently fail on timeout (sleep that's too short).
it now takes more time because in redis#12819 we iterate on all dicts, not
just non-empty ones.
it passed the PR's CI because it skips the `slow` tag, which might have
been misplaced, but now it is probably required.
with the fix, the tests take quite a lot of time:
```
[ok]: Redis can trigger resizing (1860 ms)
[ok]: Redis can rewind and trigger smaller slot resizing (744 ms)
```
before redis#12819:
```
[ok]: Redis can trigger resizing (309 ms)
[ok]: Redis can rewind and trigger smaller slot resizing (295 ms)
```

failure:
https://github.com/redis/redis/actions/runs/7704158180/job/20995931735
```
*** [err]: expire scan should skip dictionaries with lot's of empty buckets in tests/unit/expire.tcl
scan didn't handle slot skipping logic.
*** [err]: Redis can trigger resizing in tests/unit/other.tcl
Expected '[Dictionary HT]
Hash table 0 stats (main hash table):
 table size: 128
 number of elements: 5
[Expires HT]
Hash table 0 stats (main hash table):
No stats available for empty dictionaries
' to match '*table size: 8*' (context: type eval line 29 cmd {assert_match "*table size: 8*" [r debug HTSTATS 0]} proc ::test) 
*** [err]: Redis can rewind and trigger smaller slot resizing in tests/unit/other.tcl
Expected '[Dictionary HT]
Hash table 0 stats (main hash table):
 table size: 256
 number of elements: 10
[Expires HT]
Hash table 0 stats (main hash table):
No stats available for empty dictionaries
' to match '*table size: 16*' (context: type eval line 27 cmd {assert_match "*table size: 16*" [r debug HTSTATS 0]} proc ::test) 
```
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.

4 participants