Skip to content

re-enable defrag tests in cluster mode #12710

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 2 commits into from
Nov 2, 2023
Merged

re-enable defrag tests in cluster mode #12710

oranagra merged 2 commits into from
Nov 2, 2023

Conversation

@roshkhatri
Copy link
Contributor

@roshkhatri roshkhatri commented Oct 31, 2023 •
edited by oranagra
Loading

Related to #12694 (comment)

This PR 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.

@enjoy-binbin enjoy-binbin requested a review from oranagra October 31, 2023 09:28
oranagra
oranagra reviewed Oct 31, 2023
View reviewed changes
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.

i think we should still skip these in cluster mode:

  • Active defrag eval scripts
  • Active defrag big list
  • Active defrag edge case
    they have a very specific aim, and don't make sense to run twice.

which means we'll keep:

  • Active defrag main dictionary
  • Active defrag - AOF loading
  • Active defrag big keys
    @hpatro let me know if you think i'm wrong.

@hpatro
Copy link
Contributor

hpatro commented Oct 31, 2023

Hi @oranagra,

I had kept the Active Defrag - AOF loading disabled in the first place considering there is no changes around it in cluster-enabled mode.

I would keep these three tests enabled for better coverage:

  1. Active defrag main dictionary (coverage for regular key/value defrag).
  2. Active defrag eval scripts (coverage for other globals defrag).
  3. Active defrag big keys (coverage for later items).

@hpatro
Copy link
Contributor

hpatro commented Oct 31, 2023

We tried running with taskset and --loop tcl test feature to run it under constrained environment. Please do a full daily run for more confidence across multiple environment type before merging it in @oranagra .

@oranagra
Copy link
Member

oranagra commented Nov 1, 2023

so we can at least exclude

  • Active defrag big list
  • Active defrag edge case

@oranagra
Copy link
Member

oranagra commented Nov 1, 2023

full CI: https://github.com/redis/redis/actions/runs/6716775347

@oranagra oranagra changed the title Enables defrag test. re-enable defrag tests in cluster mode Nov 2, 2023
@oranagra oranagra merged commit 15a048d into redis:unstable Nov 2, 2023
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

+1 more reviewer

@enjoy-binbin enjoy-binbin enjoy-binbin approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

Redis 7.4
Status: Done

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@roshkhatri @hpatro @oranagra @enjoy-binbin