Skip to content

Changes and fixes for the new DELIFEQ command #2120

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

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

Conversation

enjoy-binbin
Copy link
Member

@enjoy-binbin enjoy-binbin commented May 22, 2025

DELIFEQ was added in #1975 but had some issues.

Do some refactoring to reduce the number of lines of code,
write commands need to increment dirty and we can propagate
it as DEL command.

Changes:

  1. Replicate as DEL is important for compatibility with replicas
    running an older Valkey version, also it can save the compare
    logic, like other commands like GETDEL.
  2. Signal modified key is for WATCH and do invalidate client-side
    caching (client tracking)
  3. Keyspace notifications.
  4. Dirty++ indicates there are unsaved changes for RDB.
  5. Using shared strings for the RESP zero and one replies for the
    code cleanup.
  6. Add FAST command flag and remove the ACCESS key specs flag.

DELIFEQ was added in valkey-io#1975 but had some issues.

Do some refactoring to reduce the number of lines of code,
write commands need to increment dirty and we can propagate
it as DEL command.

Signed-off-by: Binbin <binloveplay1314@qq.com>
Copy link

codecov bot commented May 22, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.05%. Comparing base (36b51c9) to head (ab2f507).

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #2120      +/-   ##
============================================
- Coverage     71.19%   71.05%   -0.14%     
============================================
  Files           122      122              
  Lines         66046    66045       -1     
============================================
- Hits          47020    46930      -90     
- Misses        19026    19115      +89     
Files with missing lines Coverage Δ
src/commands.def 100.00% <ø> (ø)
src/t_string.c 96.83% <100.00%> (+0.36%) ⬆️

... and 17 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.

Signed-off-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
Signed-off-by: Binbin <binloveplay1314@qq.com>
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.

3 participants