Skip to content

Fix incorrect memory overhead calculation for watched keys#3359

Merged
enjoy-binbin merged 1 commit into
valkey-io:unstablefrom
enjoy-binbin:fix_multiStateMemOverhead
Mar 17, 2026
Merged

Fix incorrect memory overhead calculation for watched keys#3359
enjoy-binbin merged 1 commit into
valkey-io:unstablefrom
enjoy-binbin:fix_multiStateMemOverhead

Conversation

@enjoy-binbin

Copy link
Copy Markdown
Member

The multiStateMemOverhead() function was incorrectly calculating the
memory overhead for watched keys. It used sizeof(c->mstate->watched_keys)
which is the size of the list structure itself, instead of sizeof(watchedKey)
which is the actual per-key overhead.

This was introduced in #1405.

The multiStateMemOverhead() function was incorrectly calculating the
memory overhead for watched keys. It used sizeof(c->mstate->watched_keys)
which is the size of the list structure itself, instead of sizeof(watchedKey)
which is the actual per-key overhead.

This was introduced in valkey-io#1405.

Signed-off-by: Binbin <binloveplay1314@qq.com>
@enjoy-binbin enjoy-binbin moved this to To be backported in Valkey 8.1 Mar 13, 2026
@enjoy-binbin enjoy-binbin moved this to To be backported in Valkey 9.0 Mar 13, 2026
@codecov

codecov Bot commented Mar 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.47%. Comparing base (c9ce3e0) to head (2db2a5a).
⚠️ Report is 3 commits behind head on unstable.

Additional details and impacted files
@@            Coverage Diff            @@
##           unstable    #3359   +/-   ##
=========================================
  Coverage     74.47%   74.47%           
=========================================
  Files           130      130           
  Lines         72719    72719           
=========================================
+ Hits          54155    54161    +6     
+ Misses        18564    18558    -6     
Files with missing lines Coverage Δ
src/multi.c 97.73% <100.00%> (+0.75%) ⬆️

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

@dvkashapov dvkashapov 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.

Great catch, LGTM!

@zuiderkwast zuiderkwast left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@zuiderkwast zuiderkwast moved this to In Progress in Valkey 9.1 Mar 16, 2026
@enjoy-binbin enjoy-binbin merged commit c921cb5 into valkey-io:unstable Mar 17, 2026
59 checks passed
@github-project-automation github-project-automation Bot moved this from In Progress to Done in Valkey 9.1 Mar 17, 2026
@enjoy-binbin enjoy-binbin deleted the fix_multiStateMemOverhead branch March 17, 2026 07:03
@enjoy-binbin enjoy-binbin moved this from Done to Optional for next release candidate in Valkey 9.1 Mar 17, 2026
@zuiderkwast zuiderkwast moved this from Optional for next release candidate to To be backported in Valkey 9.1 Mar 17, 2026
JimB123 pushed a commit that referenced this pull request Mar 19, 2026
The multiStateMemOverhead() function was incorrectly calculating the
memory overhead for watched keys. It used sizeof(c->mstate->watched_keys)
which is the size of the list structure itself, instead of sizeof(watchedKey)
which is the actual per-key overhead.

This was introduced in #1405.

Signed-off-by: Binbin <binloveplay1314@qq.com>
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request Apr 16, 2026
…#3359)

The multiStateMemOverhead() function was incorrectly calculating the
memory overhead for watched keys. It used sizeof(c->mstate->watched_keys)
which is the size of the list structure itself, instead of sizeof(watchedKey)
which is the actual per-key overhead.

This was introduced in valkey-io#1405.

Signed-off-by: Binbin <binloveplay1314@qq.com>
madolson pushed a commit that referenced this pull request Apr 27, 2026
The multiStateMemOverhead() function was incorrectly calculating the
memory overhead for watched keys. It used sizeof(c->mstate->watched_keys)
which is the size of the list structure itself, instead of sizeof(watchedKey)
which is the actual per-key overhead.

This was introduced in #1405.

Signed-off-by: Binbin <binloveplay1314@qq.com>
sarthakaggarwal97 added a commit to sarthakaggarwal97/valkey that referenced this pull request Apr 28, 2026
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request May 7, 2026
…#3359)

The multiStateMemOverhead() function was incorrectly calculating the
memory overhead for watched keys. It used sizeof(c->mstate->watched_keys)
which is the size of the list structure itself, instead of sizeof(watchedKey)
which is the actual per-key overhead.

This was introduced in valkey-io#1405.

Signed-off-by: Binbin <binloveplay1314@qq.com>
sarthakaggarwal97 pushed a commit to sarthakaggarwal97/valkey that referenced this pull request May 7, 2026
…#3359)

The multiStateMemOverhead() function was incorrectly calculating the
memory overhead for watched keys. It used sizeof(c->mstate->watched_keys)
which is the size of the list structure itself, instead of sizeof(watchedKey)
which is the actual per-key overhead.

This was introduced in valkey-io#1405.

Signed-off-by: Binbin <binloveplay1314@qq.com>
@sarthakaggarwal97 sarthakaggarwal97 moved this from To be backported to Done in Valkey 9.1 May 16, 2026
valkeyrie-ops Bot pushed a commit that referenced this pull request May 18, 2026
The multiStateMemOverhead() function was incorrectly calculating the
memory overhead for watched keys. It used sizeof(c->mstate->watched_keys)
which is the size of the list structure itself, instead of sizeof(watchedKey)
which is the actual per-key overhead.

This was introduced in #1405.

Signed-off-by: Binbin <binloveplay1314@qq.com>
zuiderkwast pushed a commit that referenced this pull request Jun 2, 2026
The multiStateMemOverhead() function was incorrectly calculating the
memory overhead for watched keys. It used sizeof(c->mstate->watched_keys)
which is the size of the list structure itself, instead of sizeof(watchedKey)
which is the actual per-key overhead.

This was introduced in #1405.

Signed-off-by: Binbin <binloveplay1314@qq.com>
@zuiderkwast zuiderkwast added the release-notes This issue should get a line item in the release notes label Jun 2, 2026
@zuiderkwast zuiderkwast moved this from To be backported to 8.1.8 (WIP) in Valkey 8.1 Jun 2, 2026
zuiderkwast pushed a commit that referenced this pull request Jun 2, 2026
The multiStateMemOverhead() function was incorrectly calculating the
memory overhead for watched keys. It used sizeof(c->mstate->watched_keys)
which is the size of the list structure itself, instead of sizeof(watchedKey)
which is the actual per-key overhead.

This was introduced in #1405.

Signed-off-by: Binbin <binloveplay1314@qq.com>
valkeyrie-ops Bot pushed a commit that referenced this pull request Jun 5, 2026
The multiStateMemOverhead() function was incorrectly calculating the
memory overhead for watched keys. It used sizeof(c->mstate->watched_keys)
which is the size of the list structure itself, instead of sizeof(watchedKey)
which is the actual per-key overhead.

This was introduced in #1405.

Signed-off-by: Binbin <binloveplay1314@qq.com>
valkeyrie-ops Bot pushed a commit that referenced this pull request Jun 10, 2026
The multiStateMemOverhead() function was incorrectly calculating the
memory overhead for watched keys. It used sizeof(c->mstate->watched_keys)
which is the size of the list structure itself, instead of sizeof(watchedKey)
which is the actual per-key overhead.

This was introduced in #1405.

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

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

Projects

Status: 8.1.8
Status: To be backported
Status: Done

Development

Successfully merging this pull request may close these issues.

4 participants