Skip to content

Revert "Update deps/libvalkey to version 0.5.0 (#3697)" on 9.1#3755

Closed
sarthakaggarwal97 wants to merge 1 commit into
valkey-io:9.1from
sarthakaggarwal97:fix/9.1-revert-libvalkey-0.5.0
Closed

Revert "Update deps/libvalkey to version 0.5.0 (#3697)" on 9.1#3755
sarthakaggarwal97 wants to merge 1 commit into
valkey-io:9.1from
sarthakaggarwal97:fix/9.1-revert-libvalkey-0.5.0

Conversation

@sarthakaggarwal97

@sarthakaggarwal97 sarthakaggarwal97 commented May 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Revert 82c386d Update deps/libvalkey to version 0.5.0 (#3697) on the 9.1 branch.

The libvalkey 0.5.0 update breaks valkey-sentinel on 9.1 with a reproducible SIGSEGV, observable on every PR targeting 9.1 (including unrelated ones like 9.1-release-version) and on essentially every CI job that runs the sentinel suite.

The crash

dictResizeWithOptionalCheck
dictExpandIfNeeded
dictFindPositionForInsert
dictAddRaw
dictReplace
valkeyAsyncAppendCmdLen   (libvalkey)
valkeyAsyncCommand        (libvalkey)
sentinelReconnectInstance (sentinel.c)
sentinelHandleValkeyInstance
sentinelTimer
serverCron

Worked with Kiro CLI!

This reverts commit 82c386d.

The libvalkey 0.5.0 update breaks valkey-sentinel on 9.1 with a
reproducible SIGSEGV in the cron path:

    dictResizeWithOptionalCheck
    dictExpandIfNeeded
    dictFindPositionForInsert
    dictAddRaw
    dictReplace
    valkeyAsyncAppendCmdLen   (libvalkey)
    valkeyAsyncCommand        (libvalkey)
    sentinelReconnectInstance (sentinel.c)
    sentinelHandleValkeyInstance
    sentinelTimer
    serverCron

The crash reproduces on every PR targeting 9.1 (verified on PR valkey-io#3733
and the unrelated 9.1-release-version PR) and across every test job
that runs the sentinel suite (jemalloc, libc-malloc, tls-module,
sanitizers, almalinux/centos/fedora variants, alpine, macOS-sentinel,
arm, lttng, fortify, no-malloc-usable-size, reply-schemas-validator).
It does not reproduce on unstable.

Root cause: libvalkey 0.5.0 includes upstream commit
"e4b548b32 Remove support for external dict while still supporting
external sds (#302)". After this libvalkey commit, libvalkey ships its
own bundled dict.c/dict.h and is no longer designed to interoperate
with an externally provided Valkey dict implementation. However,
Valkey 9.1's deps/Makefile still passes
DICT_INCLUDE_DIR=../../src/ when building libvalkey, so libvalkey's
async.c is compiled against Valkey 9.1's legacy src/dict.h while
calling Valkey 9.1's legacy dict.c symbols (dictReplace, dictAddRaw,
dictResizeWithOptionalCheck). The struct dict layouts assumed by the
two sides do not match, leading to the SIGSEGV during sentinel's
first reconnect attempt.

The same libvalkey 0.5.0 update works on unstable because PR valkey-io#3366
("Replace dict with thin wrapper around hashtable") replaced Valkey's
legacy dict with a header-only hashtable wrapper. PR valkey-io#3366 was not
backported to 9.1.

Reverting libvalkey back to 0.4.0 restores 9.1 sentinel functionality
while we land a proper forward fix (either backport the build-system
hunks of libvalkey #302 so libvalkey 0.5.0 uses its own bundled dict,
or apply a 9.1-side adaptation).

Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
@coderabbitai

coderabbitai Bot commented May 18, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 53089d93-f5c4-4d52-94d2-6edc3d837fc4

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands and usage tips.

@roshkhatri roshkhatri left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks!

@sarthakaggarwal97

Copy link
Copy Markdown
Contributor Author

Closing this PR in favor of #3752

@codecov

codecov Bot commented May 18, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.49%. Comparing base (82c386d) to head (d553baa).

Additional details and impacted files
@@            Coverage Diff             @@
##              9.1    #3755      +/-   ##
==========================================
- Coverage   76.53%   76.49%   -0.04%     
==========================================
  Files         163      163              
  Lines       80991    80991              
==========================================
- Hits        61987    61958      -29     
- Misses      19004    19033      +29     

see 23 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.

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.

2 participants