Skip to content
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.

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

Fix watcher not fully paginating on Init #1525

Merged
merged 2 commits into from
Jun 19, 2024
Merged

Fix watcher not fully paginating on Init #1525

merged 2 commits into from
Jun 19, 2024

Conversation

clux
Copy link
Member

@clux clux commented Jun 19, 2024

will cherry-pick to a 0.92.1 branch and release from there
cannot believe i missed this :(

cannot believe i missed this :(
fixes #1524

Signed-off-by: clux <[email protected]>
@clux clux added the changelog-fix changelog fix category for prs label Jun 19, 2024
Copy link

codecov bot commented Jun 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 75.1%. Comparing base (f553c4e) to head (42178d1).

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1525     +/-   ##
=======================================
+ Coverage   75.1%   75.1%   +0.1%     
=======================================
  Files         78      78             
  Lines       6980    6991     +11     
=======================================
+ Hits        5238    5249     +11     
  Misses      1742    1742             
Files Coverage Δ
kube-runtime/src/watcher.rs 42.3% <100.0%> (+0.3%) ⬆️
kube/src/mock_tests.rs 98.3% <100.0%> (+0.4%) ⬆️

@clux clux marked this pull request as ready for review June 19, 2024 19:21
@clux clux requested a review from a team June 19, 2024 19:21
@clux clux modified the milestones: 0.93.0, 0.92.1 Jun 19, 2024
@clux
Copy link
Member Author

clux commented Jun 19, 2024

Annoyingly this also "appears to undo" parts of the memory benefits that i benchmarked for 0.92.. will have to update the post later, but so far, even the starting memory (5m in after init) is doing better than 0.91, but it is more situational

controller 0.91 0.92 with page_size 50 this pr page_size 50 this pr page_size 20
2k pod labeller 45M 9M 23M 20M
ks controller 64M 51M 64M 60M

given that these controllers have also had about a week to run 0.92 it's also clear that these leak much more memory than before, and in context of the bug, this also kind of makes sense; the reflectors were not full at the beginning - and the gains i saw by measuring at the start would not last anyway.

maybe there are some more optimizations to do, but it's more important to not have a broken release out here.

Copy link
Member

@Danil-Grigorev Danil-Grigorev left a comment

Choose a reason for hiding this comment

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

/lgtm

:)

@clux clux merged commit 5320d8f into main Jun 19, 2024
18 checks passed
@clux clux deleted the watcher-page-fix branch June 19, 2024 21:47
@clux clux modified the milestones: 0.92.1, 0.93.0 Jun 19, 2024
clux added a commit that referenced this pull request Jun 19, 2024
* Fix watcher not fully paginating on Init

cannot believe i missed this :(
fixes #1524

Signed-off-by: clux <[email protected]>

* upgrade mock test (these fail on master)

Signed-off-by: clux <[email protected]>

---------

Signed-off-by: clux <[email protected]>
@clux
Copy link
Member Author

clux commented Jun 19, 2024

released in https://github.com/kube-rs/kube/releases/tag/0.92.1 from 0.92.x branch (cherry-picked in this commit).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog-fix changelog fix category for prs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

watcher on 0.92 does not get all objects during Init
3 participants