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

Immediately restart the defrag cycle if we still need to defrag #1492

Open
wants to merge 2 commits into
base: unstable
Choose a base branch
from

Conversation

madolson
Copy link
Member

@madolson madolson commented Dec 28, 2024

One very subtle change of #1242, is that we now wait for the server cron to start a new defrag cycle. Whereas previously we started it immediately. All of the current tests rely on the behavior of "Active defrag runs until it is done", which was broken because now there is a short period of active defragmentation reports itself off. Running a bunch of tests while this is in a draft, but opening it incase anyone else was taking a look.

Intended to close #1454.

https://github.com/valkey-io/valkey/actions/runs/12524316525/job/34934755959 [Only failure is active-defrag related, but not related to this issue]
https://github.com/valkey-io/valkey/actions/runs/12524547410

Copy link

codecov bot commented Dec 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.83%. Comparing base (d00c856) to head (358e0d1).
Report is 6 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #1492      +/-   ##
============================================
- Coverage     70.86%   70.83%   -0.03%     
============================================
  Files           119      119              
  Lines         64852    64861       +9     
============================================
- Hits          45958    45945      -13     
- Misses        18894    18916      +22     
Files with missing lines Coverage Δ
src/defrag.c 89.32% <100.00%> (-0.59%) ⬇️

... and 19 files with indirect coverage changes

@madolson madolson marked this pull request as ready for review December 28, 2024 06:54
@madolson
Copy link
Member Author

First full run looked good, so starting a second one but also making it ready for review since I'm going to sleep.

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.

[Test failure] Active Defrag on large item: cluster
1 participant