You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2018/03/19 18:43:03 UTC
Review Request 66126: Refactored agent task launch for better
composition [1/2].
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/
-----------------------------------------------------------
Review request for mesos, Chun-Hung Hsiao and Greg Mann.
Bugs: MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8624
Repository: mesos
Description
-------
This helps to encapsulate a task launch into a single
future which will come in handy when enforcing the task
launch order.
This patch also consolidated the error handling code
in the task launch path.
Affected tests are also updated.
Diffs
-----
src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
src/slave/slave.cpp 915d4f1bbd642ca5d2587cfdd846262cb7e08788
src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
src/tests/slave_tests.cpp f76500ebdb67f131a57a3b5aaae8c952d019e354
Diff: https://reviews.apache.org/r/66126/diff/1/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/#review199699
-----------------------------------------------------------
src/tests/slave_tests.cpp
Lines 4698-4703 (original), 4696-4701 (patched)
<https://reviews.apache.org/r/66126/#comment280110>
Indented too far.
- Greg Mann
On March 19, 2018, 6:43 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66126/
> -----------------------------------------------------------
>
> (Updated March 19, 2018, 6:43 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
>
>
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
>
> This patch also consolidated the error handling code
> in the task launch path.
>
> Affected tests are also updated.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
> src/slave/slave.cpp 915d4f1bbd642ca5d2587cfdd846262cb7e08788
> src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
> src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
> src/tests/slave_tests.cpp f76500ebdb67f131a57a3b5aaae8c952d019e354
>
>
> Diff: https://reviews.apache.org/r/66126/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Meng Zhu <mz...@mesosphere.io>.
> On March 21, 2018, 11:37 a.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Lines 2197-2199 (original), 2250-2252 (patched)
> > <https://reviews.apache.org/r/66126/diff/1/?file=1982512#file1982512line2252>
> >
> > Looks like you have an extra "because the framework" here. Also, this variable can be `const`. How about this indentation:
> >
> > ```
> > const string error =
> > "Ignoring running " + taskOrTaskGroup(task, taskGroup) +
> > " because the framework does not exist";
> > ```
> >
> > And let's use similar indentation in the other cases below.
Followed suggestion from clang-formatter for consistency.
- Meng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/#review199477
-----------------------------------------------------------
On March 19, 2018, 11:43 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66126/
> -----------------------------------------------------------
>
> (Updated March 19, 2018, 11:43 a.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
>
>
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
>
> This patch also consolidated the error handling code
> in the task launch path.
>
> Affected tests are also updated.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
> src/slave/slave.cpp 915d4f1bbd642ca5d2587cfdd846262cb7e08788
> src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
> src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
> src/tests/slave_tests.cpp f76500ebdb67f131a57a3b5aaae8c952d019e354
>
>
> Diff: https://reviews.apache.org/r/66126/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/#review199477
-----------------------------------------------------------
src/slave/slave.hpp
Lines 172-173 (original), 172-173 (patched)
<https://reviews.apache.org/r/66126/#comment279949>
Let's leave a comment here explaining why this function returns a future, while other continuations in the chain do not.
src/slave/slave.cpp
Lines 2214 (patched)
<https://reviews.apache.org/r/66126/#comment279951>
s/task(s)/task/
src/slave/slave.cpp
Lines 2197-2199 (original), 2250-2252 (patched)
<https://reviews.apache.org/r/66126/#comment279952>
Looks like you have an extra "because the framework" here. Also, this variable can be `const`. How about this indentation:
```
const string error =
"Ignoring running " + taskOrTaskGroup(task, taskGroup) +
" because the framework does not exist";
```
And let's use similar indentation in the other cases below.
src/slave/slave.cpp
Line 2214 (original), 2262 (patched)
<https://reviews.apache.org/r/66126/#comment279956>
`const string`
- Greg Mann
On March 19, 2018, 6:43 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66126/
> -----------------------------------------------------------
>
> (Updated March 19, 2018, 6:43 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
>
>
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
>
> This patch also consolidated the error handling code
> in the task launch path.
>
> Affected tests are also updated.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
> src/slave/slave.cpp 915d4f1bbd642ca5d2587cfdd846262cb7e08788
> src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
> src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
> src/tests/slave_tests.cpp f76500ebdb67f131a57a3b5aaae8c952d019e354
>
>
> Diff: https://reviews.apache.org/r/66126/diff/1/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/#review199858
-----------------------------------------------------------
src/slave/slave.cpp
Lines 2174 (patched)
<https://reviews.apache.org/r/66126/#comment280336>
`framework` is being implicitly captured by value, but the framework might be removed before this callback executes. We should use `getFramework(frameworkId)` to get the framework pointer in this and similar cases.
You will also need some error handling for the `framework == nullptr` case, where you log and return a Failure? Similar to the handling of that case below in `_run`.
- Greg Mann
On March 21, 2018, 9:49 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66126/
> -----------------------------------------------------------
>
> (Updated March 21, 2018, 9:49 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
>
>
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
>
> This patch also consolidated the error handling code
> in the task launch path.
>
> Affected tests are also updated.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
> src/slave/slave.cpp 915d4f1bbd642ca5d2587cfdd846262cb7e08788
> src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
> src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
> src/tests/slave_tests.cpp f76500ebdb67f131a57a3b5aaae8c952d019e354
>
>
> Diff: https://reviews.apache.org/r/66126/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/#review200217
-----------------------------------------------------------
src/slave/slave.cpp
Lines 2153-2201 (patched)
<https://reviews.apache.org/r/66126/#comment280875>
Should be indented 2 more spaces.
- Greg Mann
On March 28, 2018, 1:05 a.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66126/
> -----------------------------------------------------------
>
> (Updated March 28, 2018, 1:05 a.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
>
>
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
>
> This patch also consolidated the error handling code
> in the task launch path.
>
> Affected tests are also updated.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
> src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb
> src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
> src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
> src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353
>
>
> Diff: https://reviews.apache.org/r/66126/diff/3/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/#review200322
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/slave.cpp
Lines 2158 (patched)
<https://reviews.apache.org/r/66126/#comment281019>
I prefer trailing underscores for cases like this (as does our style guide), but since we have another case above in this function where we use a leading underscore for `_task`, let's try to be locally consistent here and use `_framework` instead.
src/slave/slave.cpp
Lines 2159-2162 (patched)
<https://reviews.apache.org/r/66126/#comment281018>
How about
"Cannot handle unschedule GC failure for <task> because the framework <frameworkID> does not exist"
src/slave/slave.cpp
Lines 2169-2170 (patched)
<https://reviews.apache.org/r/66126/#comment281017>
Let's move this log to the top of the lambda, since the failure has occurred even if we cannot locate the framework.
- Greg Mann
On April 2, 2018, 5:57 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66126/
> -----------------------------------------------------------
>
> (Updated April 2, 2018, 5:57 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
>
>
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
>
> This patch also consolidated the error handling code
> in the task launch path.
>
> Affected tests are also updated.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
> src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a
> src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
> src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
> src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353
>
>
> Diff: https://reviews.apache.org/r/66126/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Meng Zhu <mz...@mesosphere.io>.
> On April 2, 2018, 3:38 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Line 2166 (original), 2221-2227 (patched)
> > <https://reviews.apache.org/r/66126/diff/4/?file=1991113#file1991113line2222>
> >
> > Ah whoops, we should defer this continuation to `self()`. I think it's probably safe at the moment, but best to always defer/dispatch except for trivial continuations which do not access actor state. Even if the current implementation is safe, it makes the code less fragile if we defer.
>
> Meng Zhu wrote:
> See my reply in https://reviews.apache.org/r/66144/
OK, need to defer, as the error handler might be called in a different context other than the agent actor.
- Meng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/#review200324
-----------------------------------------------------------
On April 2, 2018, 5:33 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66126/
> -----------------------------------------------------------
>
> (Updated April 2, 2018, 5:33 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
>
>
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
>
> This patch also consolidated the error handling code
> in the task launch path.
>
> Affected tests are also updated.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
> src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a
> src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
> src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
> src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353
>
>
> Diff: https://reviews.apache.org/r/66126/diff/6/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Meng Zhu <mz...@mesosphere.io>.
> On April 2, 2018, 3:38 p.m., Greg Mann wrote:
> > src/slave/slave.cpp
> > Line 2166 (original), 2221-2227 (patched)
> > <https://reviews.apache.org/r/66126/diff/4/?file=1991113#file1991113line2222>
> >
> > Ah whoops, we should defer this continuation to `self()`. I think it's probably safe at the moment, but best to always defer/dispatch except for trivial continuations which do not access actor state. Even if the current implementation is safe, it makes the code less fragile if we defer.
See my reply in https://reviews.apache.org/r/66144/
- Meng
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/#review200324
-----------------------------------------------------------
On April 2, 2018, 5:33 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66126/
> -----------------------------------------------------------
>
> (Updated April 2, 2018, 5:33 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
>
>
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
>
> This patch also consolidated the error handling code
> in the task launch path.
>
> Affected tests are also updated.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
> src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a
> src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
> src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
> src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353
>
>
> Diff: https://reviews.apache.org/r/66126/diff/5/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/#review200324
-----------------------------------------------------------
src/slave/slave.cpp
Line 2166 (original), 2221-2227 (patched)
<https://reviews.apache.org/r/66126/#comment281021>
Ah whoops, we should defer this continuation to `self()`. I think it's probably safe at the moment, but best to always defer/dispatch except for trivial continuations which do not access actor state. Even if the current implementation is safe, it makes the code less fragile if we defer.
- Greg Mann
On April 2, 2018, 5:57 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66126/
> -----------------------------------------------------------
>
> (Updated April 2, 2018, 5:57 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
>
>
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
>
> This patch also consolidated the error handling code
> in the task launch path.
>
> Affected tests are also updated.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
> src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a
> src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
> src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
> src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353
>
>
> Diff: https://reviews.apache.org/r/66126/diff/4/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/
-----------------------------------------------------------
(Updated April 5, 2018, 3:35 p.m.)
Review request for mesos, Chun-Hung Hsiao and Greg Mann.
Changes
-------
Minor fix.
Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624
Repository: mesos
Description
-------
This helps to encapsulate a task launch into a single
future which will come in handy when enforcing the task
launch order.
This patch also consolidated the error handling code
in the task launch path.
Affected tests are also updated.
Diffs (updated)
-----
src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
src/slave/slave.cpp b17854788ceb63f3748380c546a13531e86f0dda
src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353
Diff: https://reviews.apache.org/r/66126/diff/8/
Changes: https://reviews.apache.org/r/66126/diff/7-8/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/
-----------------------------------------------------------
(Updated April 2, 2018, 5:33 p.m.)
Review request for mesos, Chun-Hung Hsiao and Greg Mann.
Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624
Repository: mesos
Description
-------
This helps to encapsulate a task launch into a single
future which will come in handy when enforcing the task
launch order.
This patch also consolidated the error handling code
in the task launch path.
Affected tests are also updated.
Diffs (updated)
-----
src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a
src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353
Diff: https://reviews.apache.org/r/66126/diff/5/
Changes: https://reviews.apache.org/r/66126/diff/4-5/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/
-----------------------------------------------------------
(Updated April 2, 2018, 10:57 a.m.)
Review request for mesos, Chun-Hung Hsiao and Greg Mann.
Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624
Repository: mesos
Description
-------
This helps to encapsulate a task launch into a single
future which will come in handy when enforcing the task
launch order.
This patch also consolidated the error handling code
in the task launch path.
Affected tests are also updated.
Diffs (updated)
-----
src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
src/slave/slave.cpp a4bd4ccd3fc59c3c0e462d9b480f5424b3e52d7a
src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353
Diff: https://reviews.apache.org/r/66126/diff/4/
Changes: https://reviews.apache.org/r/66126/diff/3-4/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/
-----------------------------------------------------------
(Updated March 27, 2018, 6:05 p.m.)
Review request for mesos, Chun-Hung Hsiao and Greg Mann.
Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624
Repository: mesos
Description
-------
This helps to encapsulate a task launch into a single
future which will come in handy when enforcing the task
launch order.
This patch also consolidated the error handling code
in the task launch path.
Affected tests are also updated.
Diffs (updated)
-----
src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
src/slave/slave.cpp e76daabad0d2d68aa42d1da809d4a23459eaaacb
src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
src/tests/slave_tests.cpp 0d7d9726ee39e4c588ea5eb8a54a73fbe1249353
Diff: https://reviews.apache.org/r/66126/diff/3/
Changes: https://reviews.apache.org/r/66126/diff/2-3/
Testing
-------
make check
Thanks,
Meng Zhu
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Greg Mann <gr...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/#review199824
-----------------------------------------------------------
Fix it, then Ship it!
src/slave/slave.hpp
Lines 176 (patched)
<https://reviews.apache.org/r/66126/#comment280299>
Nit: remove the space after `_run()`
src/slave/slave.cpp
Line 2198 (original), 2252 (patched)
<https://reviews.apache.org/r/66126/#comment280302>
Let's continue including the framework ID in this log line.
src/slave/slave.cpp
Line 2199 (original), 2253 (patched)
<https://reviews.apache.org/r/66126/#comment280300>
You don't need either of the plus signs on this line.
- Greg Mann
On March 21, 2018, 9:49 p.m., Meng Zhu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66126/
> -----------------------------------------------------------
>
> (Updated March 21, 2018, 9:49 p.m.)
>
>
> Review request for mesos, Chun-Hung Hsiao and Greg Mann.
>
>
> Bugs: MESOS-8617 and MESOS-8624
> https://issues.apache.org/jira/browse/MESOS-8617
> https://issues.apache.org/jira/browse/MESOS-8624
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This helps to encapsulate a task launch into a single
> future which will come in handy when enforcing the task
> launch order.
>
> This patch also consolidated the error handling code
> in the task launch path.
>
> Affected tests are also updated.
>
>
> Diffs
> -----
>
> src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
> src/slave/slave.cpp 915d4f1bbd642ca5d2587cfdd846262cb7e08788
> src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
> src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
> src/tests/slave_tests.cpp f76500ebdb67f131a57a3b5aaae8c952d019e354
>
>
> Diff: https://reviews.apache.org/r/66126/diff/2/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Meng Zhu
>
>
Re: Review Request 66126: Refactored agent task launch for better
composition [1/2].
Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66126/
-----------------------------------------------------------
(Updated March 21, 2018, 2:49 p.m.)
Review request for mesos, Chun-Hung Hsiao and Greg Mann.
Changes
-------
Thanks for the review! Patch updated.
Bugs: MESOS-8617 and MESOS-8624
https://issues.apache.org/jira/browse/MESOS-8617
https://issues.apache.org/jira/browse/MESOS-8624
Repository: mesos
Description
-------
This helps to encapsulate a task launch into a single
future which will come in handy when enforcing the task
launch order.
This patch also consolidated the error handling code
in the task launch path.
Affected tests are also updated.
Diffs (updated)
-----
src/slave/slave.hpp 37f0361251524e63d02d251e8a03901812b8affb
src/slave/slave.cpp 915d4f1bbd642ca5d2587cfdd846262cb7e08788
src/tests/mock_slave.hpp 8ec2b65a75f774d970ef6507df8b0c02fda5f2fd
src/tests/mock_slave.cpp 8ec55b6aa84dd8b1c3743cbcdc246311585d0f63
src/tests/slave_tests.cpp f76500ebdb67f131a57a3b5aaae8c952d019e354
Diff: https://reviews.apache.org/r/66126/diff/2/
Changes: https://reviews.apache.org/r/66126/diff/1-2/
Testing
-------
make check
Thanks,
Meng Zhu