statistics: limit the batch size when updating the stats cache #57954
Conversation
ti-chi-bot
bot
added
release-note-none
winoros
added
affects-8.5
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #57954 +/- ##
================================================
+ Coverage 73.1776% 75.1128% +1.9352%
================================================
Files 1676 1728 +52
Lines 461995 475471 +13476
================================================
+ Hits 338077 357140 +19063
+ Misses 103156 96071 -7085
- Partials 20762 22260 +1498
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| Options: t.option, | ||
| }) | ||
| t.toUpdate = t.toUpdate[:0] | ||
| t.toDelete = t.toDelete[:0] |
There was a problem hiding this comment.
The clear seems to be reset to zero instead of reset the length.
// The clear built-in function clears maps and slices.
// For maps, clear deletes all entries, resulting in an empty map.
// For slices, clear sets all elements up to the length of the slice
// to the zero value of the respective element type. If the argument
// type is a type parameter, the type parameter's type set must
// contain only map or slice types, and clear performs the operation
// implied by the type argument.
| @@ -65,6 +65,48 @@ func NewStatsCacheImplForTest() (types.StatsCache, error) { | |||
| return NewStatsCacheImpl(nil) | |||
| } | |||
|
|
|||
| type tablesToUpdateOrDelete struct { | |||
There was a problem hiding this comment.
Maybe we can call it cacheUpdateBatch?
There was a problem hiding this comment.
I guess we can at least add some unit tests for this structure.
| tables := make([]*statistics.Table, 0, len(rows)) | ||
| deletedTableIDs := make([]int64, 0, len(rows)) | ||
| tblToUpdateOrDelete := tablesToUpdateOrDelete{ | ||
| toUpdate: make([]*statistics.Table, 0, min(len(rows), 10)), |
There was a problem hiding this comment.
Maybe we can use a const to reuse this magic number.
winoros
force-pushed
the
small-batchsize-when-update-cache
branch
from
b39b0fa to
875f45e
Compare
ti-chi-bot
bot
added
approved
needs-1-more-lgtm
ti-chi-bot
bot
added
size/L
|
@winoros: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hawkingrei, Rustin170506 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
[LGTM Timeline notifier]Timeline:
|
|
In response to a cherrypick label: new pull request created to branch |
What problem does this PR solve?
Issue Number: close #57953
Problem Summary:
What changed and how does it work?
Add a middle layer to submit the update cache job more quickly.
Also memory is saved.
Check List
Tests
Side effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.