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 2020/08/05 23:27:52 UTC
[GitHub] [incubator-yunikorn-core] wangdatan opened a new pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
wangdatan opened a new pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196
Uploaded design doc. @yangwwei , @wilfred-s can you help to review?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-669608508
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=h1) Report
> Merging [#196](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/742e21611d15da8e3c8d5441f8d3b42d8532634c?el=desc) will **decrease** coverage by `0.53%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #196 +/- ##
==========================================
- Coverage 60.61% 60.08% -0.54%
==========================================
Files 67 67
Lines 6533 6468 -65
==========================================
- Hits 3960 3886 -74
- Misses 2427 2441 +14
+ Partials 146 141 -5
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `25.71% <0.00%> (-19.17%)` | :arrow_down: |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `0.00% <0.00%> (-13.52%)` | :arrow_down: |
| [pkg/cache/application\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2luZm8uZ28=) | `80.37% <0.00%> (-3.22%)` | :arrow_down: |
| [pkg/cache/initializer.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2luaXRpYWxpemVyLmdv) | `80.21% <0.00%> (-0.43%)` | :arrow_down: |
| [pkg/cache/cluster\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NsdXN0ZXJfaW5mby5nbw==) | `2.41% <0.00%> (+0.02%)` | :arrow_up: |
| [pkg/common/configs/configwatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZ3dhdGNoZXIuZ28=) | `85.48% <0.00%> (+0.23%)` | :arrow_up: |
| [pkg/common/configs/config.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZy5nbw==) | `72.72% <0.00%> (+0.85%)` | :arrow_up: |
| [pkg/cache/application\_state.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX3N0YXRlLmdv) | `100.00% <0.00%> (+4.34%)` | :arrow_up: |
| [pkg/cache/cachetestutils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NhY2hldGVzdHV0aWxzLmdv) | `61.76% <0.00%> (+11.76%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=footer). Last update [742e216...e1226ba](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] commented on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-669608508
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=h1) Report
> Merging [#196](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/d2a76c0d7000bb50e377f170538938236c1144c5&el=desc) will **increase** coverage by `0.02%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #196 +/- ##
==========================================
+ Coverage 60.05% 60.08% +0.02%
==========================================
Files 67 67
Lines 6484 6468 -16
==========================================
- Hits 3894 3886 -8
+ Misses 2449 2441 -8
Partials 141 141
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/scheduler/scheduling\_application.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9zY2hlZHVsaW5nX2FwcGxpY2F0aW9uLmdv) | `72.75% <0.00%> (-0.27%)` | :arrow_down: |
| [pkg/scheduler/scheduling\_node.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3NjaGVkdWxlci9zY2hlZHVsaW5nX25vZGUuZ28=) | `85.18% <0.00%> (+3.09%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=footer). Last update [d2a76c0...acf676d](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] TravisBuddy commented on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
TravisBuddy commented on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-687347136
Hey @wangdatan,
Something went wrong with the build.
TravisCI finished with status `errored`, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.
<a href="https://travis-ci.org/apache/incubator-yunikorn-core/builds/724280685">View build log</a>
###### TravisBuddy Request Identifier: 5e274770-eee7-11ea-9705-b7c481b3600a
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-669608508
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=h1) Report
> Merging [#196](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/742e21611d15da8e3c8d5441f8d3b42d8532634c?el=desc) will **decrease** coverage by `0.53%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #196 +/- ##
==========================================
- Coverage 60.61% 60.08% -0.54%
==========================================
Files 67 67
Lines 6533 6468 -65
==========================================
- Hits 3960 3886 -74
- Misses 2427 2441 +14
+ Partials 146 141 -5
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `25.71% <0.00%> (-19.17%)` | :arrow_down: |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `0.00% <0.00%> (-13.52%)` | :arrow_down: |
| [pkg/cache/application\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2luZm8uZ28=) | `80.37% <0.00%> (-3.22%)` | :arrow_down: |
| [pkg/cache/initializer.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2luaXRpYWxpemVyLmdv) | `80.21% <0.00%> (-0.43%)` | :arrow_down: |
| [pkg/cache/cluster\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NsdXN0ZXJfaW5mby5nbw==) | `2.41% <0.00%> (+0.02%)` | :arrow_up: |
| [pkg/common/configs/configwatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZ3dhdGNoZXIuZ28=) | `85.48% <0.00%> (+0.23%)` | :arrow_up: |
| [pkg/common/configs/config.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZy5nbw==) | `72.72% <0.00%> (+0.85%)` | :arrow_up: |
| [pkg/cache/application\_state.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX3N0YXRlLmdv) | `100.00% <0.00%> (+4.34%)` | :arrow_up: |
| [pkg/cache/cachetestutils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NhY2hldGVzdHV0aWxzLmdv) | `61.76% <0.00%> (+11.76%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=footer). Last update [742e216...c49984e](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] wangdatan commented on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
wangdatan commented on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-683456382
Added add a few more changes for what we discussed:
1) Changes in scheduler to NOT async commit allocation (now commit is synced).
2) Changes to remove all allocating resources, and other unused methods.
What I plan to do next is make minimum changes to the test, and make sure at least the smoke test can run. After that, I think we will be good to continue develop this in a branch and stabilize it over time. Before that, I'd like to hear thoughts about package change proposed above.
All changes are pushed to my branch.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-669608508
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=h1) Report
> Merging [#196](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/742e21611d15da8e3c8d5441f8d3b42d8532634c?el=desc) will **decrease** coverage by `0.53%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #196 +/- ##
==========================================
- Coverage 60.61% 60.08% -0.54%
==========================================
Files 67 67
Lines 6533 6468 -65
==========================================
- Hits 3960 3886 -74
- Misses 2427 2441 +14
+ Partials 146 141 -5
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `25.71% <0.00%> (-19.17%)` | :arrow_down: |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `0.00% <0.00%> (-13.52%)` | :arrow_down: |
| [pkg/cache/application\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2luZm8uZ28=) | `80.37% <0.00%> (-3.22%)` | :arrow_down: |
| [pkg/cache/initializer.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2luaXRpYWxpemVyLmdv) | `80.21% <0.00%> (-0.43%)` | :arrow_down: |
| [pkg/cache/cluster\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NsdXN0ZXJfaW5mby5nbw==) | `2.41% <0.00%> (+0.02%)` | :arrow_up: |
| [pkg/common/configs/configwatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZ3dhdGNoZXIuZ28=) | `85.48% <0.00%> (+0.23%)` | :arrow_up: |
| [pkg/common/configs/config.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZy5nbw==) | `72.72% <0.00%> (+0.85%)` | :arrow_up: |
| [pkg/cache/application\_state.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX3N0YXRlLmdv) | `100.00% <0.00%> (+4.34%)` | :arrow_up: |
| [pkg/cache/cachetestutils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NhY2hldGVzdHV0aWxzLmdv) | `61.76% <0.00%> (+11.76%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=footer). Last update [742e216...f0e22a8](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-669608508
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=h1) Report
> Merging [#196](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/742e21611d15da8e3c8d5441f8d3b42d8532634c?el=desc) will **decrease** coverage by `0.53%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #196 +/- ##
==========================================
- Coverage 60.61% 60.08% -0.54%
==========================================
Files 67 67
Lines 6533 6468 -65
==========================================
- Hits 3960 3886 -74
- Misses 2427 2441 +14
+ Partials 146 141 -5
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `25.71% <0.00%> (-19.17%)` | :arrow_down: |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `0.00% <0.00%> (-13.52%)` | :arrow_down: |
| [pkg/cache/application\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2luZm8uZ28=) | `80.37% <0.00%> (-3.22%)` | :arrow_down: |
| [pkg/cache/initializer.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2luaXRpYWxpemVyLmdv) | `80.21% <0.00%> (-0.43%)` | :arrow_down: |
| [pkg/cache/cluster\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NsdXN0ZXJfaW5mby5nbw==) | `2.41% <0.00%> (+0.02%)` | :arrow_up: |
| [pkg/common/configs/configwatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZ3dhdGNoZXIuZ28=) | `85.48% <0.00%> (+0.23%)` | :arrow_up: |
| [pkg/common/configs/config.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZy5nbw==) | `72.72% <0.00%> (+0.85%)` | :arrow_up: |
| [pkg/cache/application\_state.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX3N0YXRlLmdv) | `100.00% <0.00%> (+4.34%)` | :arrow_up: |
| [pkg/cache/cachetestutils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NhY2hldGVzdHV0aWxzLmdv) | `61.76% <0.00%> (+11.76%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=footer). Last update [742e216...d1f85b2](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-669608508
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=h1) Report
> Merging [#196](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/742e21611d15da8e3c8d5441f8d3b42d8532634c?el=desc) will **decrease** coverage by `0.53%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #196 +/- ##
==========================================
- Coverage 60.61% 60.08% -0.54%
==========================================
Files 67 67
Lines 6533 6468 -65
==========================================
- Hits 3960 3886 -74
- Misses 2427 2441 +14
+ Partials 146 141 -5
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `25.71% <0.00%> (-19.17%)` | :arrow_down: |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `0.00% <0.00%> (-13.52%)` | :arrow_down: |
| [pkg/cache/application\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2luZm8uZ28=) | `80.37% <0.00%> (-3.22%)` | :arrow_down: |
| [pkg/cache/initializer.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2luaXRpYWxpemVyLmdv) | `80.21% <0.00%> (-0.43%)` | :arrow_down: |
| [pkg/cache/cluster\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NsdXN0ZXJfaW5mby5nbw==) | `2.41% <0.00%> (+0.02%)` | :arrow_up: |
| [pkg/common/configs/configwatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZ3dhdGNoZXIuZ28=) | `85.48% <0.00%> (+0.23%)` | :arrow_up: |
| [pkg/common/configs/config.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZy5nbw==) | `72.72% <0.00%> (+0.85%)` | :arrow_up: |
| [pkg/cache/application\_state.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX3N0YXRlLmdv) | `100.00% <0.00%> (+4.34%)` | :arrow_up: |
| [pkg/cache/cachetestutils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NhY2hldGVzdHV0aWxzLmdv) | `61.76% <0.00%> (+11.76%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=footer). Last update [742e216...36bae57](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-669608508
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=h1) Report
> Merging [#196](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/742e21611d15da8e3c8d5441f8d3b42d8532634c?el=desc) will **decrease** coverage by `0.53%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #196 +/- ##
==========================================
- Coverage 60.61% 60.08% -0.54%
==========================================
Files 67 67
Lines 6533 6468 -65
==========================================
- Hits 3960 3886 -74
- Misses 2427 2441 +14
+ Partials 146 141 -5
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `25.71% <0.00%> (-19.17%)` | :arrow_down: |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `0.00% <0.00%> (-13.52%)` | :arrow_down: |
| [pkg/cache/application\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2luZm8uZ28=) | `80.37% <0.00%> (-3.22%)` | :arrow_down: |
| [pkg/cache/initializer.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2luaXRpYWxpemVyLmdv) | `80.21% <0.00%> (-0.43%)` | :arrow_down: |
| [pkg/cache/cluster\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NsdXN0ZXJfaW5mby5nbw==) | `2.41% <0.00%> (+0.02%)` | :arrow_up: |
| [pkg/common/configs/configwatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZ3dhdGNoZXIuZ28=) | `85.48% <0.00%> (+0.23%)` | :arrow_up: |
| [pkg/common/configs/config.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZy5nbw==) | `72.72% <0.00%> (+0.85%)` | :arrow_up: |
| [pkg/cache/application\_state.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX3N0YXRlLmdv) | `100.00% <0.00%> (+4.34%)` | :arrow_up: |
| [pkg/cache/cachetestutils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NhY2hldGVzdHV0aWxzLmdv) | `61.76% <0.00%> (+11.76%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=footer). Last update [742e216...5fdeab7](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] wilfred-s commented on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-686375282
I have move the PR to the new YUNIKORN-317 branch.
I will have a quick look at this and even push some changes in the new branch and we'll work on it there.
In the mean time I will work on getting the lint part of the build to pass on a branch (YUNIKORN-285)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-669608508
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=h1) Report
> Merging [#196](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/742e21611d15da8e3c8d5441f8d3b42d8532634c&el=desc) will **decrease** coverage by `0.53%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #196 +/- ##
==========================================
- Coverage 60.61% 60.08% -0.54%
==========================================
Files 67 67
Lines 6533 6468 -65
==========================================
- Hits 3960 3886 -74
- Misses 2427 2441 +14
+ Partials 146 141 -5
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `25.71% <0.00%> (-19.17%)` | :arrow_down: |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `0.00% <0.00%> (-13.52%)` | :arrow_down: |
| [pkg/cache/application\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2luZm8uZ28=) | `80.37% <0.00%> (-3.22%)` | :arrow_down: |
| [pkg/cache/initializer.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2luaXRpYWxpemVyLmdv) | `80.21% <0.00%> (-0.43%)` | :arrow_down: |
| [pkg/cache/cluster\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NsdXN0ZXJfaW5mby5nbw==) | `2.41% <0.00%> (+0.02%)` | :arrow_up: |
| [pkg/common/configs/configwatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZ3dhdGNoZXIuZ28=) | `85.48% <0.00%> (+0.23%)` | :arrow_up: |
| [pkg/common/configs/config.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZy5nbw==) | `72.72% <0.00%> (+0.85%)` | :arrow_up: |
| [pkg/cache/application\_state.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX3N0YXRlLmdv) | `100.00% <0.00%> (+4.34%)` | :arrow_up: |
| [pkg/cache/cachetestutils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NhY2hldGVzdHV0aWxzLmdv) | `61.76% <0.00%> (+11.76%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=footer). Last update [742e216...dcf99fa](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] TravisBuddy commented on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
TravisBuddy commented on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-669608483
Hey @wangdatan,
Your changes look good to me!
<a href="https://travis-ci.org/apache/incubator-yunikorn-core/builds/715330944">View build log</a>
###### TravisBuddy Request Identifier: 39298b20-d779-11ea-85fc-4b61cecd2c91
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-669608508
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=h1) Report
> Merging [#196](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=desc) into [YUNIKORN-317](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/742e21611d15da8e3c8d5441f8d3b42d8532634c?el=desc) will **decrease** coverage by `0.53%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## YUNIKORN-317 #196 +/- ##
================================================
- Coverage 60.61% 60.08% -0.54%
================================================
Files 67 67
Lines 6533 6468 -65
================================================
- Hits 3960 3886 -74
- Misses 2427 2441 +14
+ Partials 146 141 -5
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `25.71% <0.00%> (-19.17%)` | :arrow_down: |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `0.00% <0.00%> (-13.52%)` | :arrow_down: |
| [pkg/cache/application\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2luZm8uZ28=) | `80.37% <0.00%> (-3.22%)` | :arrow_down: |
| [pkg/cache/initializer.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2luaXRpYWxpemVyLmdv) | `80.21% <0.00%> (-0.43%)` | :arrow_down: |
| [pkg/cache/cluster\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NsdXN0ZXJfaW5mby5nbw==) | `2.41% <0.00%> (+0.02%)` | :arrow_up: |
| [pkg/common/configs/configwatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZ3dhdGNoZXIuZ28=) | `85.48% <0.00%> (+0.23%)` | :arrow_up: |
| [pkg/common/configs/config.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZy5nbw==) | `72.72% <0.00%> (+0.85%)` | :arrow_up: |
| [pkg/cache/application\_state.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX3N0YXRlLmdv) | `100.00% <0.00%> (+4.34%)` | :arrow_up: |
| [pkg/cache/cachetestutils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NhY2hldGVzdHV0aWxzLmdv) | `61.76% <0.00%> (+11.76%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=footer). Last update [742e216...61c83fb](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] sunilgovind commented on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
sunilgovind commented on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-678608278
Based on the size of the patch and the approach, it is better we do more tests and ensure the stability is maintained. I think it's worth to keep going with a single patch where all changes goes in and the PR is enough to test all. (Rather compared to 8 to 10 patches in a branch). Usually we go with branch model but this is a complete overhaul based on my initial reviews. So my vote is having a bare minimum single PR where most of the work is done and get that tested and reviewed well before pushing to master.
I think an approach like this will slow down other critical feature development. As a community we need to agree whether this is fine.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] wangdatan commented on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
wangdatan commented on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-678554504
@yangwwei , @wilfred-s , @TaoYang526 I spent quite some time this week to do a PoC based on the design. I have updated PR for the changes.
What I have done:
- Merged all cache objects to corresponding scheduler objects.
- Removed `cache` package entirely.
- Removed all events between `scheduler` and `cache`, now RMProxy/Scheduler are directly communicate with each other.
What is pending:
- Any test or validation of this work, I have merged unit tests source, but haven't modified most of them, so they're far from complete.
- Some features need more time to check, including:
- Reservation.
- Preemption.
- Auto creation of queues.
- etc.
- Current main code (apart from test) is not compilable, I left a bunch of `FIXME` to indicate these parts.
What I learned:
- The proposed approach is quite possible, code structure can be simplified a lot, and duplicated code between scheduler/cache is more than I thought. So the sooner we can finish this effort the better.
- Even we said "duplicated" code between scheduler and cache, they're actually quite different, we have to merge these logics.
- Because of this, I found it is really hard to find a middle ground we can gradually to do this, because of duplicated logic between scheduler and cache, we have to do it cleanly in one shot instead of doing this in 5 patches. Because making it partially work is even harder than a clean cut.
- Changes I have made is quite scary and risky (we're talking about 5k+ LoC addition and 6k+ LoC code removal), I have manually done a lot of logic changes to merge the two fat modules and de-dup logic. (I have rewritten many parts including commit-proposal, add new allocation, update scheduler config, etc. under the new codebase.)
Next step:
- I hope to get more feedback before we spend more effort on this.
- We need Chaos monkey test in place in order to make us feel safe.
- If everybody agrees this is a better approach, I suggest prioritize this effort, and finish it before we merge another big feature.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] wilfred-s commented on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
wilfred-s commented on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-712513686
Thank you for the work that you have done @wangdatan.
The change [committed here](https://github.com/apache/incubator-yunikorn-core/commit/7c37ccb383a430dcc6839fb26af057bddd50e5a5) does not include tests
I have rebased the change to the current head of the dev branch. I added changes on top to rewind the placement rules move into the scheduler package. This includes further refactoring.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] wilfred-s closed pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
wilfred-s closed pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] wangdatan commented on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
wangdatan commented on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-683455906
While working on the refactoring, I want to make a few major changes to the organization of the package. Before that I'd like to hear your thoughts, especially @wilfred-s
Existing package structure:
1) placement package highly depends on scheduler package (mostly data structure like schedulingApp/Queue, etc. of scheduler).
2) scheduler.tests package depends on entry point (to initialize RMProxy).
Proposed package structure:
1) Move placement package into scheduler. (And its tests)
2) Leave core e2e tests which need both RMProxy and Scheduler (tests invokes Entrypoint) to a separate package, such as `pkg/e2e`
3) Scheduler internal tests (Which doesn't depends on RMProxy) still inside scheduler package.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] wangdatan commented on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
wangdatan commented on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-687312780
@wilfred-s somehow I couldn't find my commits in the newly created YK-317 branch, I will continue work on this in my own branch, give me a few days, once I have done I will update this PR.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-669608508
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=h1) Report
> Merging [#196](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/742e21611d15da8e3c8d5441f8d3b42d8532634c&el=desc) will **decrease** coverage by `0.53%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #196 +/- ##
==========================================
- Coverage 60.61% 60.08% -0.54%
==========================================
Files 67 67
Lines 6533 6468 -65
==========================================
- Hits 3960 3886 -74
- Misses 2427 2441 +14
+ Partials 146 141 -5
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `25.71% <0.00%> (-19.17%)` | :arrow_down: |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `0.00% <0.00%> (-13.52%)` | :arrow_down: |
| [pkg/cache/application\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2luZm8uZ28=) | `80.37% <0.00%> (-3.22%)` | :arrow_down: |
| [pkg/cache/initializer.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2luaXRpYWxpemVyLmdv) | `80.21% <0.00%> (-0.43%)` | :arrow_down: |
| [pkg/cache/cluster\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NsdXN0ZXJfaW5mby5nbw==) | `2.41% <0.00%> (+0.02%)` | :arrow_up: |
| [pkg/common/configs/configwatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZ3dhdGNoZXIuZ28=) | `85.48% <0.00%> (+0.23%)` | :arrow_up: |
| [pkg/common/configs/config.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZy5nbw==) | `72.72% <0.00%> (+0.85%)` | :arrow_up: |
| [pkg/cache/application\_state.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX3N0YXRlLmdv) | `100.00% <0.00%> (+4.34%)` | :arrow_up: |
| [pkg/cache/cachetestutils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NhY2hldGVzdHV0aWxzLmdv) | `61.76% <0.00%> (+11.76%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=footer). Last update [742e216...87e4d46](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-yunikorn-core] codecov[bot] edited a comment on pull request #196: YUNIKORN-317. [Design Doc] Cache/Scheduler modules need to be combined to a single module
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #196:
URL: https://github.com/apache/incubator-yunikorn-core/pull/196#issuecomment-669608508
# [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=h1) Report
> Merging [#196](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=desc) into [master](https://codecov.io/gh/apache/incubator-yunikorn-core/commit/742e21611d15da8e3c8d5441f8d3b42d8532634c?el=desc) will **decrease** coverage by `0.53%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## master #196 +/- ##
==========================================
- Coverage 60.61% 60.08% -0.54%
==========================================
Files 67 67
Lines 6533 6468 -65
==========================================
- Hits 3960 3886 -74
- Misses 2427 2441 +14
+ Partials 146 141 -5
```
| [Impacted Files](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pkg/webservice/handlers.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2UvaGFuZGxlcnMuZ28=) | `25.71% <0.00%> (-19.17%)` | :arrow_down: |
| [pkg/webservice/webservice.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL3dlYnNlcnZpY2Uvd2Vic2VydmljZS5nbw==) | `0.00% <0.00%> (-13.52%)` | :arrow_down: |
| [pkg/cache/application\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX2luZm8uZ28=) | `80.37% <0.00%> (-3.22%)` | :arrow_down: |
| [pkg/cache/initializer.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2luaXRpYWxpemVyLmdv) | `80.21% <0.00%> (-0.43%)` | :arrow_down: |
| [pkg/cache/cluster\_info.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NsdXN0ZXJfaW5mby5nbw==) | `2.41% <0.00%> (+0.02%)` | :arrow_up: |
| [pkg/common/configs/configwatcher.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZ3dhdGNoZXIuZ28=) | `85.48% <0.00%> (+0.23%)` | :arrow_up: |
| [pkg/common/configs/config.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NvbW1vbi9jb25maWdzL2NvbmZpZy5nbw==) | `72.72% <0.00%> (+0.85%)` | :arrow_up: |
| [pkg/cache/application\_state.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2FwcGxpY2F0aW9uX3N0YXRlLmdv) | `100.00% <0.00%> (+4.34%)` | :arrow_up: |
| [pkg/cache/cachetestutils.go](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196/diff?src=pr&el=tree#diff-cGtnL2NhY2hlL2NhY2hldGVzdHV0aWxzLmdv) | `61.76% <0.00%> (+11.76%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=footer). Last update [742e216...8714fd5](https://codecov.io/gh/apache/incubator-yunikorn-core/pull/196?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org