You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@yunikorn.apache.org by GitBox <gi...@apache.org> on 2022/03/01 01:24:55 UTC
[GitHub] [incubator-yunikorn-k8shim] craigcondit opened a new pull request #375: [YUNIKORN-1100] Fix scheduler cache inconsistencies
craigcondit opened a new pull request #375:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/375
### What is this PR for?
Fix leaks and inconsistencies in the shim scheduler cache.
Many of these issues have been shown to lead to scheduling failures (such as hitting max Pod limits) because we don't properly cleanup allocations.
- Made Add/Update/Remove Pod/Node handlers idempotent and removed error returns wherever possible
- Ensure RemoveNode properly removes any associated Pod allocations
- Added debugging hooks to pre/post handlers to dump state of scheduler cache
- Handle terminated pods in a similar fashion to removed Pods
- Fix leaking of assumed pods
- Fix duplicate addition of pods to NodeInfo.Pods structure
- Track pod -> node assignments separately from NodeInfo as NodeInfo stores pods in a list and doesn't check for duplicates
- Refactored Add/Update methods to share logic wherever possible
- Log both podKey and podName when updates are made
### What type of PR is it?
* [x] - Bug Fix
* [ ] - Improvement
* [ ] - Feature
* [ ] - Documentation
* [ ] - Hot Fix
* [ ] - Refactoring
### Todos
* [ ] - Task
### What is the Jira issue?
https://issues.apache.org/jira/browse/YUNIKORN-1100
### How should this be tested?
Unit tests updated and debug logs show that we are no longer leaking objects.
### 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] [incubator-yunikorn-k8shim] wilfred-s closed pull request #375: [YUNIKORN-1100] Fix scheduler cache inconsistencies
Posted by GitBox <gi...@apache.org>.
wilfred-s closed pull request #375:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/375
--
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] [incubator-yunikorn-k8shim] codecov[bot] commented on pull request #375: [YUNIKORN-1100] Fix scheduler cache inconsistencies
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #375:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/375#issuecomment-1054888318
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#375](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0e6fff5) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bb5941b08d80f2d3f7e561835a8c2e63e10c6f3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bb5941b) will **decrease** coverage by `0.62%`.
> The diff coverage is `45.40%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #375 +/- ##
==========================================
- Coverage 65.49% 64.86% -0.63%
==========================================
Files 41 41
Lines 6219 6256 +37
==========================================
- Hits 4073 4058 -15
- Misses 1993 2043 +50
- Partials 153 155 +2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `41.73% <0.00%> (+1.06%)` | :arrow_up: |
| [pkg/cache/external/scheduler\_cache.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2V4dGVybmFsL3NjaGVkdWxlcl9jYWNoZS5nbw==) | `43.26% <45.29%> (-15.29%)` | :arrow_down: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `42.83% <46.15%> (+1.27%)` | :arrow_up: |
| [pkg/cache/node\_coordinator.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL25vZGVfY29vcmRpbmF0b3IuZ28=) | `65.27% <100.00%> (-0.94%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [bb5941b...0e6fff5](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #375: [YUNIKORN-1100] Fix scheduler cache inconsistencies
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #375:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/375#issuecomment-1054888318
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#375](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0e6fff5) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bb5941b08d80f2d3f7e561835a8c2e63e10c6f3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bb5941b) will **decrease** coverage by `0.62%`.
> The diff coverage is `45.40%`.
> :exclamation: Current head 0e6fff5 differs from pull request most recent head ce52ac3. Consider uploading reports for the commit ce52ac3 to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #375 +/- ##
==========================================
- Coverage 65.49% 64.86% -0.63%
==========================================
Files 41 41
Lines 6219 6256 +37
==========================================
- Hits 4073 4058 -15
- Misses 1993 2043 +50
- Partials 153 155 +2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `41.73% <0.00%> (+1.06%)` | :arrow_up: |
| [pkg/cache/external/scheduler\_cache.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2V4dGVybmFsL3NjaGVkdWxlcl9jYWNoZS5nbw==) | `43.26% <45.29%> (-15.29%)` | :arrow_down: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `42.83% <46.15%> (+1.27%)` | :arrow_up: |
| [pkg/cache/node\_coordinator.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL25vZGVfY29vcmRpbmF0b3IuZ28=) | `65.27% <100.00%> (-0.94%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [bb5941b...ce52ac3](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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] [incubator-yunikorn-k8shim] craigcondit commented on a change in pull request #375: [YUNIKORN-1100] Fix scheduler cache inconsistencies
Posted by GitBox <gi...@apache.org>.
craigcondit commented on a change in pull request #375:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/375#discussion_r816385827
##########
File path: pkg/cache/context.go
##########
@@ -472,25 +457,26 @@ func (ctx *Context) AssumePod(name string, node string) error {
}
// assign the node name for pod
assumedPod.Spec.NodeName = node
- return ctx.schedulerCache.AssumePod(assumedPod, allBound)
+ ctx.schedulerCache.AssumePod(assumedPod, allBound)
+ return nil
}
}
return nil
}
// forget pod must be called when a pod is assumed to be running on a node,
// but then for some reason it is failed to bind or released.
-func (ctx *Context) ForgetPod(name string) error {
+func (ctx *Context) ForgetPod(name string) {
ctx.lock.Lock()
defer ctx.lock.Unlock()
if pod, ok := ctx.schedulerCache.GetPod(name); ok {
log.Logger().Debug("forget pod", zap.String("pod", pod.Name))
- return ctx.schedulerCache.ForgetPod(pod)
+ ctx.schedulerCache.ForgetPod(pod)
+ return
}
log.Logger().Debug("unable to forget pod",
zap.String("reason", fmt.Sprintf("pod %s not found in scheduler cache", name)))
Review comment:
Updated.
--
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] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #375: [YUNIKORN-1100] Fix scheduler cache inconsistencies
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #375:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/375#issuecomment-1054888318
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#375](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bca0ec2) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bb5941b08d80f2d3f7e561835a8c2e63e10c6f3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bb5941b) will **decrease** coverage by `0.61%`.
> The diff coverage is `45.45%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #375 +/- ##
==========================================
- Coverage 65.49% 64.87% -0.62%
==========================================
Files 41 41
Lines 6219 6255 +36
==========================================
- Hits 4073 4058 -15
- Misses 1993 2042 +49
- Partials 153 155 +2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `41.73% <0.00%> (+1.06%)` | :arrow_up: |
| [pkg/cache/external/scheduler\_cache.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2V4dGVybmFsL3NjaGVkdWxlcl9jYWNoZS5nbw==) | `43.26% <45.29%> (-15.29%)` | :arrow_down: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `42.89% <46.66%> (+1.33%)` | :arrow_up: |
| [pkg/cache/node\_coordinator.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL25vZGVfY29vcmRpbmF0b3IuZ28=) | `65.27% <100.00%> (-0.94%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [bb5941b...bca0ec2](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #375: [YUNIKORN-1100] Fix scheduler cache inconsistencies
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #375:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/375#issuecomment-1054888318
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#375](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ce52ac3) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bb5941b08d80f2d3f7e561835a8c2e63e10c6f3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bb5941b) will **decrease** coverage by `0.61%`.
> The diff coverage is `45.16%`.
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #375 +/- ##
==========================================
- Coverage 65.49% 64.87% -0.62%
==========================================
Files 41 41
Lines 6219 6255 +36
==========================================
- Hits 4073 4058 -15
- Misses 1993 2042 +49
- Partials 153 155 +2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `41.73% <0.00%> (+1.06%)` | :arrow_up: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `42.89% <42.85%> (+1.33%)` | :arrow_up: |
| [pkg/cache/external/scheduler\_cache.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2V4dGVybmFsL3NjaGVkdWxlcl9jYWNoZS5nbw==) | `43.26% <45.29%> (-15.29%)` | :arrow_down: |
| [pkg/cache/node\_coordinator.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL25vZGVfY29vcmRpbmF0b3IuZ28=) | `65.27% <100.00%> (-0.94%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [bb5941b...ce52ac3](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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] [incubator-yunikorn-k8shim] wilfred-s commented on a change in pull request #375: [YUNIKORN-1100] Fix scheduler cache inconsistencies
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on a change in pull request #375:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/375#discussion_r816383879
##########
File path: pkg/cache/context.go
##########
@@ -472,25 +457,26 @@ func (ctx *Context) AssumePod(name string, node string) error {
}
// assign the node name for pod
assumedPod.Spec.NodeName = node
- return ctx.schedulerCache.AssumePod(assumedPod, allBound)
+ ctx.schedulerCache.AssumePod(assumedPod, allBound)
+ return nil
}
}
return nil
}
// forget pod must be called when a pod is assumed to be running on a node,
// but then for some reason it is failed to bind or released.
-func (ctx *Context) ForgetPod(name string) error {
+func (ctx *Context) ForgetPod(name string) {
ctx.lock.Lock()
defer ctx.lock.Unlock()
if pod, ok := ctx.schedulerCache.GetPod(name); ok {
log.Logger().Debug("forget pod", zap.String("pod", pod.Name))
- return ctx.schedulerCache.ForgetPod(pod)
+ ctx.schedulerCache.ForgetPod(pod)
+ return
}
log.Logger().Debug("unable to forget pod",
zap.String("reason", fmt.Sprintf("pod %s not found in scheduler cache", name)))
Review comment:
NIT: Can we clean up this logging to just be:
```
log.Logger().Debug("unable to forget pod: not found in cache", zap.String("pod", name))
```
--
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] [incubator-yunikorn-k8shim] codecov[bot] edited a comment on pull request #375: [YUNIKORN-1100] Fix scheduler cache inconsistencies
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #375:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/375#issuecomment-1054888318
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#375](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ce52ac3) into [master](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/commit/bb5941b08d80f2d3f7e561835a8c2e63e10c6f3c?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (bb5941b) will **decrease** coverage by `0.61%`.
> The diff coverage is `45.16%`.
> :exclamation: Current head ce52ac3 differs from pull request most recent head bca0ec2. Consider uploading reports for the commit bca0ec2 to get more accurate results
[](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #375 +/- ##
==========================================
- Coverage 65.49% 64.87% -0.62%
==========================================
Files 41 41
Lines 6219 6255 +36
==========================================
- Hits 4073 4058 -15
- Misses 1993 2042 +49
- Partials 153 155 +2
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [pkg/cache/context\_recovery.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHRfcmVjb3ZlcnkuZ28=) | `41.73% <0.00%> (+1.06%)` | :arrow_up: |
| [pkg/cache/context.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2NvbnRleHQuZ28=) | `42.89% <42.85%> (+1.33%)` | :arrow_up: |
| [pkg/cache/external/scheduler\_cache.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL2V4dGVybmFsL3NjaGVkdWxlcl9jYWNoZS5nbw==) | `43.26% <45.29%> (-15.29%)` | :arrow_down: |
| [pkg/cache/node\_coordinator.go](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cGtnL2NhY2hlL25vZGVfY29vcmRpbmF0b3IuZ28=) | `65.27% <100.00%> (-0.94%)` | :arrow_down: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [bb5941b...bca0ec2](https://codecov.io/gh/apache/incubator-yunikorn-k8shim/pull/375?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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] [incubator-yunikorn-k8shim] craigcondit commented on pull request #375: [YUNIKORN-1100] Fix scheduler cache inconsistencies
Posted by GitBox <gi...@apache.org>.
craigcondit commented on pull request #375:
URL: https://github.com/apache/incubator-yunikorn-k8shim/pull/375#issuecomment-1054913885
> Is there a point leaving UpdateNode and UpdatePod two-parameter functions? The first parameter (oldNode and oldPod) are never used
Good point. Updated patch to remove the unnecessary parameter.
--
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