Skip to content

Conversation

@withRiver
Copy link
Contributor

@withRiver withRiver commented Aug 8, 2025

Previously CONFIG RESETSTATS only resets the slot statistics in
cluster part, this PR makes it reset cluster bus messages at well.
Additionally, we also reset stat_cluster_links_buffer_limit_exceeded.

Now we will reset:

  • cluster_stats_messages_sent*
  • cluster_stats_messages_received*
  • total_cluster_links_buffer_limit_exceeded

Closes #2439.

Copy link
Member

@enjoy-binbin enjoy-binbin left a comment

Choose a reason for hiding this comment

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

@codecov
Copy link

codecov bot commented Aug 9, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 71.55%. Comparing base (70d336d) to head (331310b).
⚠️ Report is 4 commits behind head on unstable.

Additional details and impacted files
@@             Coverage Diff              @@
##           unstable    #2458      +/-   ##
============================================
- Coverage     71.58%   71.55%   -0.03%     
============================================
  Files           125      125              
  Lines         69214    69218       +4     
============================================
- Hits          49545    49531      -14     
- Misses        19669    19687      +18     
Files with missing lines Coverage Δ
src/cluster.c 90.43% <100.00%> (+0.05%) ⬆️

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

@withRiver
Copy link
Contributor Author

@enjoy-binbin @sarthakaggarwal97
Thanks!! I have modified the code according to your suggestions and sign the DOC in my latest commit message.

@ranshid
Copy link
Member

ranshid commented Aug 10, 2025

and sign the DOC in my latest commit message.

@withRiver, unfortunately even though we squash all commits before merging the DCO check will verify all the commits in this PR are signed. you can probably do a git rebase --signoff HEAD~3 or manually rebase and edit the relevant commits massage.

@withRiver
Copy link
Contributor Author

Sorry. I have added DCO signature to all my commits now.

@enjoy-binbin enjoy-binbin moved this to In Progress in Valkey 9.0 Aug 11, 2025
@enjoy-binbin enjoy-binbin added release-notes This issue should get a line item in the release notes major-decision-pending Major decision pending by TSC team labels Aug 11, 2025
Copy link
Member

@ranshid ranshid left a comment

Choose a reason for hiding this comment

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

LGTM

…en executing CONFIG RESETSTATS

Reset server.cluster->stats_bus_messages_xx and server.cluster->stat_cluster_links_buffer_limit_exceeded in resetClusterStats()

Signed-off-by: Hongrui <[email protected]>
@enjoy-binbin enjoy-binbin changed the title Reset statistics of cluster bus messages when executing CONFIG RESETSTATS Reset cluster related stats in CONFIG RESETSTATS Aug 11, 2025
Copy link
Member

@enjoy-binbin enjoy-binbin left a comment

Choose a reason for hiding this comment

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

@valkey-io/core-team This is a minor change, please take a look at the top comment and approve.

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.

Approving the behavior change. I didn't review the code.

@sarthakaggarwal97
Copy link
Contributor

Thanks @withRiver!

Copy link
Collaborator

@hpatro hpatro left a comment

Choose a reason for hiding this comment

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

Looks good to me. Just checked new slot migration code, it has different mechanism to maintain/cleanup the finished slot migration operation, so we can avoid that here I guess.

@zuiderkwast zuiderkwast added major-decision-approved Major decision approved by TSC team and removed major-decision-pending Major decision pending by TSC team labels Aug 25, 2025
@enjoy-binbin enjoy-binbin merged commit 3db75f5 into valkey-io:unstable Aug 29, 2025
60 of 61 checks passed
@github-project-automation github-project-automation bot moved this from In Progress to Done in Valkey 9.0 Aug 29, 2025
@enjoy-binbin enjoy-binbin moved this from Done to RC2 in Valkey 9.0 Aug 29, 2025
@madolson madolson moved this from RC2 to Done in Valkey 9.0 Sep 3, 2025
rjd15372 pushed a commit to rjd15372/valkey that referenced this pull request Sep 19, 2025
Previously CONFIG RESETSTATS only resets the slot statistics in
cluster part, this PR makes it reset cluster bus messages at well.
Additionally, we also reset stat_cluster_links_buffer_limit_exceeded.

Now we will reset:
- cluster_stats_messages_sent*
- cluster_stats_messages_received*
- total_cluster_links_buffer_limit_exceeded

Closes valkey-io#2439.

Signed-off-by: Hongrui <[email protected]>
rjd15372 pushed a commit that referenced this pull request Sep 23, 2025
Previously CONFIG RESETSTATS only resets the slot statistics in
cluster part, this PR makes it reset cluster bus messages at well.
Additionally, we also reset stat_cluster_links_buffer_limit_exceeded.

Now we will reset:
- cluster_stats_messages_sent*
- cluster_stats_messages_received*
- total_cluster_links_buffer_limit_exceeded

Closes #2439.

Signed-off-by: Hongrui <[email protected]>
hpatro pushed a commit to hpatro/valkey that referenced this pull request Oct 3, 2025
Previously CONFIG RESETSTATS only resets the slot statistics in
cluster part, this PR makes it reset cluster bus messages at well.
Additionally, we also reset stat_cluster_links_buffer_limit_exceeded.

Now we will reset:
- cluster_stats_messages_sent*
- cluster_stats_messages_received*
- total_cluster_links_buffer_limit_exceeded

Closes valkey-io#2439.

Signed-off-by: Hongrui <[email protected]>
Signed-off-by: Harkrishn Patro <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

major-decision-approved Major decision approved by TSC team release-notes This issue should get a line item in the release notes

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Make CONFIG RESETSTATS reset all cluster stats.

7 participants