Skip to content

Conversation

@enjoy-binbin
Copy link
Member

@enjoy-binbin enjoy-binbin commented Feb 1, 2026

When ZREM / SREM / HDEL delete a large number of elements, we can apply the same
optimization as ZREMRANGEBY*, which is to temporarily pause the autoshrink of
the corresponding hashtable to avoid triggering resize/rehash repeatedly during
the deletion process.

This theoretically leads to some performance improvements. Furthermore, by only
triggering a resize once at the end (i.e., only when hashtableResumeAutoShrink
is called), we can get the most right resize, preventing further resizes during
the resize/rehash process.

When ZREM and SREM delete a large number of elements, we can apply the same
optimization as ZREMRANGEBY*, which is to temporarily pause the autoshrink of
the corresponding hashtable to avoid triggering resize/rehash repeatedly during
the deletion process.

This theoretically leads to some performance improvements.  Furthermore, by only
triggering a resize once at the end (i.e., only when `hashtableResumeAutoShrink`
is called), we can get the most right resize, preventing further resizes during
the resize/rehash process.

Signed-off-by: Binbin <[email protected]>
@enjoy-binbin enjoy-binbin added the run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) label Feb 1, 2026
@codecov
Copy link

codecov bot commented Feb 1, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.87%. Comparing base (851eaaf) to head (5d74a8f).
⚠️ Report is 7 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #3144      +/-   ##
============================================
+ Coverage     74.85%   74.87%   +0.02%     
============================================
  Files           129      129              
  Lines         71210    71215       +5     
============================================
+ Hits          53302    53322      +20     
+ Misses        17908    17893      -15     
Files with missing lines Coverage Δ
src/t_hash.c 95.47% <100.00%> (+0.08%) ⬆️
src/t_set.c 97.71% <100.00%> (-0.34%) ⬇️
src/t_zset.c 97.07% <100.00%> (+0.04%) ⬆️

... and 18 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions bot removed the run-extra-tests Run extra tests on this PR (Runs all tests from daily except valgrind and RESP) label Feb 1, 2026
Copy link
Member

@dvkashapov dvkashapov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, clean and simple

Co-authored-by: Daniil Kashapov <[email protected]>
Signed-off-by: Binbin <[email protected]>
@enjoy-binbin enjoy-binbin changed the title Pause auto shrink in ZREM and SREM commands Pause auto shrink in ZREM / SREM / HDEL commands Feb 3, 2026
Copy link
Contributor

@zuiderkwast zuiderkwast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is better, without behavior change.

For release notes, we can say something like "optimize srem/zrem/hdel when deleting multiple items".

@enjoy-binbin enjoy-binbin changed the title Pause auto shrink in ZREM / SREM / HDEL commands Optimize SREM/ZREM/HDEL to pause auto shrink when deleting multiple items Feb 3, 2026
@enjoy-binbin enjoy-binbin merged commit 8ed270f into valkey-io:unstable Feb 3, 2026
24 checks passed
@enjoy-binbin enjoy-binbin deleted the pause_resize branch February 3, 2026 16:15
@zuiderkwast zuiderkwast added the release-notes This issue should get a line item in the release notes label Feb 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes This issue should get a line item in the release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants