You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Zhitao Li <zh...@gmail.com> on 2018/03/28 18:24:43 UTC
Review Request 66344: Supported non-speculative operations on agent
default resources.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66344/
-----------------------------------------------------------
Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
Bugs: MESOS-4965
https://issues.apache.org/jira/browse/MESOS-4965
Repository: mesos
Description
-------
Supported non-speculative operations on agent default resources.
Diffs
-----
src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb
Diff: https://reviews.apache.org/r/66344/diff/1/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 66344: Supported non-speculative operations on
agent default resources.
Posted by Gaston Kleiman <ga...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66344/#review201237
-----------------------------------------------------------
src/slave/slave.cpp
Lines 4328 (patched)
<https://reviews.apache.org/r/66344/#comment282375>
The agent should retry this update for as long as possible, so it should set a status UUID.
- Gaston Kleiman
On March 28, 2018, 2:41 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66344/
> -----------------------------------------------------------
>
> (Updated March 28, 2018, 2:41 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We use `getResourceConversions` to calculate `conversions`,
> apply that to agent default resources if not on resource provider,
> and send `UpdateOperationStatusMessage` message back to master.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb
>
>
> Diff: https://reviews.apache.org/r/66344/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 66344: Supported non-speculative operations on
agent default resources.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66344/#review213343
-----------------------------------------------------------
Bad review!
Reviews applied: [66344, 66049, 66218]
Error:
2019-03-02 05:11:09 URL:https://reviews.apache.org/r/66344/diff/raw/ [3810/3810] -> "66344.patch" [1]
error: patch failed: src/slave/slave.cpp:4298
error: src/slave/slave.cpp: patch does not apply
- Mesos Reviewbot
On March 28, 2018, 9:41 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66344/
> -----------------------------------------------------------
>
> (Updated March 28, 2018, 9:41 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Gastón Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We use `getResourceConversions` to calculate `conversions`,
> apply that to agent default resources if not on resource provider,
> and send `UpdateOperationStatusMessage` message back to master.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb
>
>
> Diff: https://reviews.apache.org/r/66344/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 66344: Supported non-speculative operations on
agent default resources.
Posted by Benjamin Bannier <be...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66344/#review200188
-----------------------------------------------------------
src/slave/slave.cpp
Line 8030 (original)
<https://reviews.apache.org/r/66344/#comment280823>
The agent's `totalResources` includes resource provider resources, so moving this part into the agent resources-only branch is incorrect (this even breaks an existing test).
- Benjamin Bannier
On March 28, 2018, 11:41 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66344/
> -----------------------------------------------------------
>
> (Updated March 28, 2018, 11:41 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We use `getResourceConversions` to calculate `conversions`,
> apply that to agent default resources if not on resource provider,
> and send `UpdateOperationStatusMessage` message back to master.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb
>
>
> Diff: https://reviews.apache.org/r/66344/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 66344: Supported non-speculative operations on
agent default resources.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66344/#review201241
-----------------------------------------------------------
FAIL: Failed to apply the current review.
Failed command: `python.exe .\support\apply-reviews.py -n -r 66344`
All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66344
Relevant logs:
- [apply-review-66344-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66344/logs/apply-review-66344-stdout.log):
```
error: patch failed: src/slave/slave.cpp:4298
error: src/slave/slave.cpp: patch does not apply
```
- Mesos Reviewbot Windows
On March 28, 2018, 9:41 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66344/
> -----------------------------------------------------------
>
> (Updated March 28, 2018, 9:41 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We use `getResourceConversions` to calculate `conversions`,
> apply that to agent default resources if not on resource provider,
> and send `UpdateOperationStatusMessage` message back to master.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb
>
>
> Diff: https://reviews.apache.org/r/66344/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 66344: Supported non-speculative operations on
agent default resources.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66344/#review200149
-----------------------------------------------------------
- Greg Mann
On March 28, 2018, 9:41 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66344/
> -----------------------------------------------------------
>
> (Updated March 28, 2018, 9:41 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We use `getResourceConversions` to calculate `conversions`,
> apply that to agent default resources if not on resource provider,
> and send `UpdateOperationStatusMessage` message back to master.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb
>
>
> Diff: https://reviews.apache.org/r/66344/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 66344: Supported non-speculative operations on
agent default resources.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66344/#review201267
-----------------------------------------------------------
Bad patch!
Reviews applied: [66344, 66049, 66218]
Failed command: python support/apply-reviews.py -n -r 66344
Error:
2018-04-16 22:00:48 URL:https://reviews.apache.org/r/66344/diff/raw/ [3810/3810] -> "66344.patch" [1]
error: patch failed: src/slave/slave.cpp:4298
error: src/slave/slave.cpp: patch does not apply
Full log: https://builds.apache.org/job/Mesos-Reviewbot/22199/console
- Mesos Reviewbot
On March 28, 2018, 9:41 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66344/
> -----------------------------------------------------------
>
> (Updated March 28, 2018, 9:41 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We use `getResourceConversions` to calculate `conversions`,
> apply that to agent default resources if not on resource provider,
> and send `UpdateOperationStatusMessage` message back to master.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb
>
>
> Diff: https://reviews.apache.org/r/66344/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 66344: Supported non-speculative operations on
agent default resources.
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66344/#review200143
-----------------------------------------------------------
src/slave/slave.cpp
Line 4311 (original), 4308 (patched)
<https://reviews.apache.org/r/66344/#comment280798>
s/Calculated/Calculate/
src/slave/slave.cpp
Lines 4314 (patched)
<https://reviews.apache.org/r/66344/#comment280799>
Perhaps we should leave a comment here explaining the validity of this CHECK? Something like "Operation validation guarantees that we can generate resource conversions from all operations on agent default resources."
Currently, we rely on the fact that operations like CREATE_VOLUME (which cannot be used to generate resource conversions) are invalidated if they have a resource provider ID set. It might be nice to validate this case more explicitly in the future, but since we have separate validation functions for each operation type that would be cumbersome right now.
src/slave/slave.cpp
Lines 4321-4323 (patched)
<https://reviews.apache.org/r/66344/#comment280796>
Indented too far.
src/slave/slave.cpp
Lines 4339 (patched)
<https://reviews.apache.org/r/66344/#comment280800>
s/updateOperation/`updateOperation`/
s/operation/operations/
- Greg Mann
On March 28, 2018, 9:41 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66344/
> -----------------------------------------------------------
>
> (Updated March 28, 2018, 9:41 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We use `getResourceConversions` to calculate `conversions`,
> apply that to agent default resources if not on resource provider,
> and send `UpdateOperationStatusMessage` message back to master.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb
>
>
> Diff: https://reviews.apache.org/r/66344/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 66344: Supported non-speculative operations on
agent default resources.
Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66344/#review213708
-----------------------------------------------------------
FAIL: Failed to apply the current review.
Failed command: `python.exe .\support\apply-reviews.py -n -r 66344`
All the build artifacts available at: http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2957/mesos-review-66344
Relevant logs:
- [apply-review-66344.log](http://dcos-win.westus2.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/2957/mesos-review-66344/logs/apply-review-66344.log):
```
error: patch failed: src/slave/slave.cpp:4298
error: src/slave/slave.cpp: patch does not apply
```
- Mesos Reviewbot Windows
On March 28, 2018, 9:41 p.m., Zhitao Li wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66344/
> -----------------------------------------------------------
>
> (Updated March 28, 2018, 9:41 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao, Gastón Kleiman, and Greg Mann.
>
>
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We use `getResourceConversions` to calculate `conversions`,
> apply that to agent default resources if not on resource provider,
> and send `UpdateOperationStatusMessage` message back to master.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb
>
>
> Diff: https://reviews.apache.org/r/66344/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhitao Li
>
>
Re: Review Request 66344: Supported non-speculative operations on
agent default resources.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66344/
-----------------------------------------------------------
(Updated March 28, 2018, 2:41 p.m.)
Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
Changes
-------
Update logging and summary.
Bugs: MESOS-4965
https://issues.apache.org/jira/browse/MESOS-4965
Repository: mesos
Description (updated)
-------
We use `getResourceConversions` to calculate `conversions`,
apply that to agent default resources if not on resource provider,
and send `UpdateOperationStatusMessage` message back to master.
Diffs (updated)
-----
src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb
Diff: https://reviews.apache.org/r/66344/diff/3/
Changes: https://reviews.apache.org/r/66344/diff/2-3/
Testing
-------
Thanks,
Zhitao Li
Re: Review Request 66344: Supported non-speculative operations on
agent default resources.
Posted by Zhitao Li <zh...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66344/
-----------------------------------------------------------
(Updated March 28, 2018, 2:29 p.m.)
Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
Changes
-------
Review comments from https://reviews.apache.org/r/66050/#comment280724
Bugs: MESOS-4965
https://issues.apache.org/jira/browse/MESOS-4965
Repository: mesos
Description
-------
Supported non-speculative operations on agent default resources.
Diffs (updated)
-----
src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb
Diff: https://reviews.apache.org/r/66344/diff/2/
Changes: https://reviews.apache.org/r/66344/diff/1-2/
Testing
-------
Thanks,
Zhitao Li