Skip to content

Disable flaky defrag tests affecting daily run #12672

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.

Sign up for GitHub

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

Jump to bottom
Merged
oranagra merged 1 commit into from
Oct 19, 2023
Merged

Disable flaky defrag tests affecting daily run #12672

oranagra merged 1 commit into from
Oct 19, 2023

Conversation

@hpatro
Copy link
Contributor

@hpatro hpatro commented Oct 18, 2023 •
edited
Loading

Temporarily disabling few of the defrag tests in cluster mode to make the daily run stable:

  1. Active defrag eval scripts
  2. Active defrag big keys
  3. Active defrag big list
  4. Active defrag edge case

Few scenarios observed and I'm investigating:

  1. defrag not started - Either the defrag scan got completed real quick or never started.
  2. Actual fragmentation is lower than the expected fragmentation - Might require us to lower the fragmentation expectation or have separate threshold for cluster mode.
  3. Max latency exceeds from the current set threshold.
  4. defrag didn't stop - Sometimes allocator_frag_ratio reaches 1.06 and the test fails. Expectation is 1.05.

Failure run: https://github.com/redis/redis/actions/runs/6527277037/job/17721912648

This was referenced Oct 18, 2023
Merged
Merged
@oranagra
Copy link
Member

oranagra commented Oct 19, 2023

Trueth be told, when you added the loop to run all the Defrag tests twice, I thought it was excessive, but I think the one about big keys, or big list is actually needed (we had some concerns about the DefragLater list in cluster mode)

@hpatro
Copy link
Contributor Author

hpatro commented Oct 19, 2023

Trueth be told, when you added the loop to run all the Defrag tests twice, I thought it was excessive, but I think the one about big keys, or big list is actually needed (we had some concerns about the DefragLater list in cluster mode)

Wasn't expecting this amount of flakiness at each validation level. Seems the tests were particularly crafted for standalone setup. I'm trying to fix them here #12674

@oranagra
Copy link
Member

oranagra commented Oct 19, 2023

I don't understand the changes of the other PR, i guess it's still WIP.
since it takes time and the diff here is trivial to revert, i'll merge it just to silence the errors for now.

@oranagra oranagra merged commit becd50d into redis:unstable Oct 19, 2023
@hpatro
Copy link
Contributor Author

hpatro commented Oct 19, 2023

I don't understand the changes of the other PR, i guess it's still WIP. since it takes time and the diff here is trivial to revert, i'll merge it just to silence the errors for now.

@oranagra I've explained the issue with the current logic for defrag not started failure in this comment #12674 (comment). PTAL.

@roshkhatri roshkhatri mentioned this pull request Oct 25, 2023
Merged
oranagra pushed a commit that referenced this pull request Oct 27, 2023
@roshkhatri
Reset later item flag after defrag later is done (#12694)
7d68208 
Fixing issues described in #12672, started after #11695
Related to #12674

Fixes the `defrag didn't stop' issue.

In some cases of how the keys were stored in memory
defrag_later_item_in_progress was not getting reset once we finish
defragging the later items and we move to the next slot. This stopped
the scan to happen in the later slots and did not get 
@roshkhatri roshkhatri mentioned this pull request Oct 31, 2023
Merged
oranagra added a commit that referenced this pull request Nov 2, 2023
@roshkhatri @oranagra
re-enable defrag tests in cluster mode. (#12710)
15a048d 
Reverts the skipping defrag tests in cluster mode (done in #12672.
instead it skips only some defrag tests that are relevant for cluster modes.
The test now run well after investigating and making the changes in #12674 and #12694.

Co-authored-by: Oran Agra <oran@redislabs.com>
enjoy-binbin added a commit to enjoy-binbin/redis that referenced this pull request Jul 25, 2025
@enjoy-binbin
Reset later item flag after defrag later is done (redis#12694)
78e34a0 
Fixing issues described in redis#12672, started after redis#11695
Related to redis#12674

Fixes the `defrag didn't stop' issue.

In some cases of how the keys were stored in memory
defrag_later_item_in_progress was not getting reset once we finish
defragging the later items and we move to the next slot. This stopped
the scan to happen in the later slots and did not get
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@oranagra oranagra oranagra approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@hpatro @oranagra