You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by "pbacsko (via GitHub)" <gi...@apache.org> on 2023/05/20 00:49:48 UTC
[GitHub] [yunikorn-k8shim] pbacsko opened a new pull request, #595: [YUNIKORN-1746] Improve the performance of nodeInfoListerImpl.List()
pbacsko opened a new pull request, #595:
URL: https://github.com/apache/yunikorn-k8shim/pull/595
### What is this PR for?
The method `nodeInfoListerImpl.List()` is called very frequently and always creates a slice with the same content over and over again. It's only necessary to update if a nodes joins/leaves the cluster.
The code is also on a critical path, optimizing it has visible performance benefits.
### What type of PR is it?
* [ ] - Bug Fix
* [x] - Improvement
* [ ] - Feature
* [ ] - Documentation
* [ ] - Hot Fix
* [ ] - Refactoring
### Todos
* [ ] - Task
### What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-1746
### How should this be tested?
### Screenshots (if appropriate)
### Questions:
* [ ] - The licenses files need update.
* [ ] - There is breaking changes for older versions.
* [ ] - It needs documentation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] craigcondit closed pull request #595: [YUNIKORN-1746] Improve the performance of nodeInfoListerImpl.List()
Posted by "craigcondit (via GitHub)" <gi...@apache.org>.
craigcondit closed pull request #595: [YUNIKORN-1746] Improve the performance of nodeInfoListerImpl.List()
URL: https://github.com/apache/yunikorn-k8shim/pull/595
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] codecov[bot] commented on pull request #595: [YUNIKORN-1746] Improve the performance of nodeInfoListerImpl.List()
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #595:
URL: https://github.com/apache/yunikorn-k8shim/pull/595#issuecomment-1555404679
## [Codecov](https://app.codecov.io/gh/apache/yunikorn-k8shim/pull/595?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
> Merging [#595](https://app.codecov.io/gh/apache/yunikorn-k8shim/pull/595?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (8f3e16d) into [master](https://app.codecov.io/gh/apache/yunikorn-k8shim/commit/6715ec994ba42f0688514b03776335b2d6eeaabd?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) (6715ec9) will **increase** coverage by `0.00%`.
> The diff coverage is `95.23%`.
```diff
@@ Coverage Diff @@
## master #595 +/- ##
=======================================
Coverage 70.14% 70.15%
=======================================
Files 47 47
Lines 7938 7944 +6
=======================================
+ Hits 5568 5573 +5
- Misses 2164 2165 +1
Partials 206 206
```
| [Impacted Files](https://app.codecov.io/gh/apache/yunikorn-k8shim/pull/595?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [pkg/cache/external/scheduler\_cache.go](https://app.codecov.io/gh/apache/yunikorn-k8shim/pull/595?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL2NhY2hlL2V4dGVybmFsL3NjaGVkdWxlcl9jYWNoZS5nbw==) | `60.88% <75.00%> (-0.05%)` | :arrow_down: |
| [pkg/plugin/support/nodeinfo\_lister.go](https://app.codecov.io/gh/apache/yunikorn-k8shim/pull/595?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGtnL3BsdWdpbi9zdXBwb3J0L25vZGVpbmZvX2xpc3Rlci5nbw==) | `100.00% <100.00%> (ø)` | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [yunikorn-k8shim] craigcondit commented on pull request #595: [YUNIKORN-1746] Improve the performance of nodeInfoListerImpl.List()
Posted by "craigcondit (via GitHub)" <gi...@apache.org>.
craigcondit commented on PR #595:
URL: https://github.com/apache/yunikorn-k8shim/pull/595#issuecomment-1559664951
Had a conversation offline about this. I'd like to see the caching logic encapsulated completely within the SchedulerCache object. We can add a GetNodeList() method that returns the list, and cache the list inside the SchedulerCache itself. If the pointer to that list is nil, then assume it is invalid. Then we compute it on demand in GetNodeList(), and set to nil any time the node map is modified. Finally, update the NodeLister to call GetNodeList() instead of the GetNodesInfoMap().
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@yunikorn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org