Skip to content

statistics: use Loads/StoreStats and HistoryLoads instead of []float64 and [][]float64 #9279

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.

Sign up for GitHub

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

Jump to bottom
Merged
ti-chi-bot merged 8 commits into from
May 20, 2025
Merged

statistics: use Loads/StoreStats and HistoryLoads instead of []float64 and [][]float64 #9279

ti-chi-bot merged 8 commits into from
May 20, 2025

Conversation

@okJiang
Copy link
Member

@okJiang okJiang commented Apr 28, 2025

What problem does this PR solve?

Issue Number: Ref #4399

What is changed and how does it work?

Check List

Tests

  • Unit test
  • Integration test

Release note

None.

@okJiang
add Loads and HistoryLoads
40bf17b 
Signed-off-by: okJiang <819421878@qq.com>
@ti-chi-bot ti-chi-bot bot added release-note-none Denotes a PR that doesn't merit a release note. dco-signoff: yes Indicates the PR's author has signed the dco. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Apr 28, 2025
okJiang added 2 commits April 28, 2025 17:44
@okJiang
add StoreStats
806aa09 
Signed-off-by: okJiang <819421878@qq.com>
@okJiang
fix ci
94a8d89 
Signed-off-by: okJiang <819421878@qq.com>
@okJiang okJiang changed the title statistics: use Loads and HistoryLoads instead of []float64 and [][]float64 statistics: use Loads/StoreStats and HistoryLoads instead of []float64 and [][]float64 Apr 28, 2025
@okJiang
Copy link
Member Author

okJiang commented Apr 28, 2025

/retest

1 similar comment
@okJiang
Copy link
Member Author

okJiang commented Apr 29, 2025

/retest

@codecov
Copy link

codecov bot commented Apr 29, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 98.03922% with 2 lines in your changes missing coverage. Please review.

Project coverage is 75.94%. Comparing base (04f1ae0) to head (8ec06b4).
Report is 4 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #9279      +/-   ##
==========================================
- Coverage   75.98%   75.94%   -0.05%     
==========================================
  Files         473      473              
  Lines       73625    73683      +58     
==========================================
+ Hits        55945    55956      +11     
- Misses      14181    14215      +34     
- Partials     3499     3512      +13
Flag Coverage Δ
unittests 75.94% <98.03%> (-0.05%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@okJiang
Copy link
Member Author

okJiang commented Apr 29, 2025

/cc @JmPotato @rleungx

@ti-chi-bot ti-chi-bot bot requested review from JmPotato and rleungx April 29, 2025 07:28
@okJiang
Copy link
Member Author

okJiang commented Apr 29, 2025

/cc @lhy1024

@ti-chi-bot ti-chi-bot bot requested a review from lhy1024 April 29, 2025 09:35
@ti-chi-bot ti-chi-bot bot added needs-1-more-lgtm Indicates a PR needs 1 more LGTM. approved labels Apr 30, 2025
rleungx
rleungx reviewed Apr 30, 2025
View reviewed changes
pkg/statistics/store_load.go Outdated
type HistoryLoads [utils.DimLen][] /* buffer size */ float64

// StoreStats is a vector that contains different dimensions of store stat.
type StoreStats [utils.StoreStatCount]float64
Copy link
Member

@rleungx rleungx Apr 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I think it's a little bit confusing between this variable and Loads. e.g., GetStoresLoads returns StoreStats.

Copy link
Member Author

@okJiang okJiang May 6, 2025

Choose a reason for hiding this comment

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

Do you have any suggestions on this? They had this confusion before. I originally intended to use StoreLoads, but I found that there is also a concept called StoreStatCount.

okJiang added 4 commits May 6, 2025 14:40
@okJiang
fix comment, rename StoreStats to StoreLoads
30d2d08 
Signed-off-by: okJiang <819421878@qq.com>
@okJiang
rename count to HotPeerCount
406cd6e 
Signed-off-by: okJiang <819421878@qq.com>
@okJiang
rename StoreLoads to StoreKindLoads
4667595 
Signed-off-by: okJiang <819421878@qq.com>
@okJiang
fix
7434ddf 
Signed-off-by: okJiang <819421878@qq.com>
@ti-chi-bot ti-chi-bot bot added the lgtm label May 20, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented May 20, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bufferflies, rleungx

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:
  • OWNERS [bufferflies,rleungx]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ti-chi-bot ti-chi-bot bot removed the needs-1-more-lgtm Indicates a PR needs 1 more LGTM. label May 20, 2025
@ti-chi-bot
Copy link
Contributor

ti-chi-bot bot commented May 20, 2025

[LGTM Timeline notifier]

Timeline:

  • 2025-04-30 03:47:47.839321681 +0000 UTC m=+1018611.651112057: ☑️ agreed by bufferflies.
  • 2025-05-20 08:48:17.306725484 +0000 UTC m=+78734.407892703: ☑️ agreed by rleungx.

@ti-chi-bot ti-chi-bot bot merged commit 6f886e0 into tikv:master May 20, 2025
25 checks passed
@okJiang okJiang deleted the introduce-loads branch May 20, 2025 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bufferflies bufferflies bufferflies approved these changes

@rleungx rleungx rleungx approved these changes

@JmPotato JmPotato Awaiting requested review from JmPotato

@lhy1024 lhy1024 Awaiting requested review from lhy1024

Assignees

No one assigned

Labels

approved dco-signoff: yes Indicates the PR's author has signed the dco. lgtm release-note-none Denotes a PR that doesn't merit a release note. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@okJiang @bufferflies @rleungx