You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mesos.apache.org by Niklas Nielsen <ni...@qni.dk> on 2014/12/03 22:20:25 UTC
Re: Review Request 28063: Introduced grace period in CommandInfo.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28063/#review63475
-----------------------------------------------------------
src/slave/slave.hpp
<https://reviews.apache.org/r/28063/#comment105737>
/es/e/
src/slave/slave.cpp
<https://reviews.apache.org/r/28063/#comment105739>
Think I mentioned this before - drop 'updated' prefix. Has some of the previous patches been squashed into this one?
src/slave/slave.cpp
<https://reviews.apache.org/r/28063/#comment106037>
'do not' or 'do only'?
src/slave/slave.cpp
<https://reviews.apache.org/r/28063/#comment106038>
Check for what? :)
src/slave/slave.cpp
<https://reviews.apache.org/r/28063/#comment106039>
else if? If the task has a command, it cannot have an executor
- Niklas Nielsen
On Nov. 24, 2014, 9:07 a.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28063/
> -----------------------------------------------------------
>
> (Updated Nov. 24, 2014, 9:07 a.m.)
>
>
> Review request for mesos, Ben Mahler, Niklas Nielsen, and Till Toenshoff.
>
>
> Bugs: MESOS-1571
> https://issues.apache.org/jira/browse/MESOS-1571
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> CommandExecutor grace_period field is designed to be used by slave to propagate the value of the grace period flag further to containerizers and executors.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 0085aba
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp ed63ded
>
> Diff: https://reviews.apache.org/r/28063/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.4, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 28063: Introduced grace period in CommandInfo.
Posted by Niklas Nielsen <ni...@qni.dk>.
> On Dec. 3, 2014, 1:20 p.m., Niklas Nielsen wrote:
> > src/slave/slave.hpp, line 275
> > <https://reviews.apache.org/r/28063/diff/3/?file=774392#file774392line275>
> >
> > /es/e/
>
> Alexander Rukletsov wrote:
> Current implementation updates all present CommandInfo fields in a TaskInfo.
Fair enough. Feel free to drop
> On Dec. 3, 2014, 1:20 p.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, line 1160
> > <https://reviews.apache.org/r/28063/diff/3/?file=774393#file774393line1160>
> >
> > Think I mentioned this before - drop 'updated' prefix. Has some of the previous patches been squashed into this one?
>
> Alexander Rukletsov wrote:
> Can be, though I can't remember. What name would you suggest? `task` is taken.
We have usually been appending '_' to denote prime (f, f', f'' -> f, f_, f__). See executorInfo and executorInfo_ in launchExecutor
> On Dec. 3, 2014, 1:20 p.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, lines 2666-2667
> > <https://reviews.apache.org/r/28063/diff/3/?file=774393#file774393line2666>
> >
> > 'do not' or 'do only'?
>
> Alexander Rukletsov wrote:
> That is the next step: now we overwrite all TaskInfos. The todo is about the future use case when frameworks will set their grace periods.
Thanks for the clarification. Feel free to drop.
> On Dec. 3, 2014, 1:20 p.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, line 2675
> > <https://reviews.apache.org/r/28063/diff/3/?file=774393#file774393line2675>
> >
> > else if? If the task has a command, it cannot have an executor
>
> Alexander Rukletsov wrote:
> There is only one place
>
> Alexander Rukletsov wrote:
> There is only one place where we enforce this constraint. I do not want to introduce this check in another piece of code, since the behaviour may change. Current grace period update code will handle both situations correctly.
Fair enough. Feel free to drop
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28063/#review63475
-----------------------------------------------------------
On Dec. 17, 2014, 3:25 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28063/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2014, 3:25 p.m.)
>
>
> Review request for mesos, Ben Mahler, Niklas Nielsen, and Till Toenshoff.
>
>
> Bugs: MESOS-1571
> https://issues.apache.org/jira/browse/MESOS-1571
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> CommandExecutor grace_period field is designed to be used by slave to propagate the value of the grace period flag further to containerizers and executors.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 0085aba
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp ed63ded
>
> Diff: https://reviews.apache.org/r/28063/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.4, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 28063: Introduced grace period in CommandInfo.
Posted by Alexander Rukletsov <al...@mesosphere.io>.
> On Dec. 3, 2014, 9:20 p.m., Niklas Nielsen wrote:
> > src/slave/slave.hpp, line 275
> > <https://reviews.apache.org/r/28063/diff/3/?file=774392#file774392line275>
> >
> > /es/e/
Current implementation updates all present CommandInfo fields in a TaskInfo.
> On Dec. 3, 2014, 9:20 p.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, line 1160
> > <https://reviews.apache.org/r/28063/diff/3/?file=774393#file774393line1160>
> >
> > Think I mentioned this before - drop 'updated' prefix. Has some of the previous patches been squashed into this one?
Can be, though I can't remember. What name would you suggest? `task` is taken.
> On Dec. 3, 2014, 9:20 p.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, lines 2666-2667
> > <https://reviews.apache.org/r/28063/diff/3/?file=774393#file774393line2666>
> >
> > 'do not' or 'do only'?
That is the next step: now we overwrite all TaskInfos. The todo is about the future use case when frameworks will set their grace periods.
> On Dec. 3, 2014, 9:20 p.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, line 2675
> > <https://reviews.apache.org/r/28063/diff/3/?file=774393#file774393line2675>
> >
> > else if? If the task has a command, it cannot have an executor
There is only one place
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28063/#review63475
-----------------------------------------------------------
On Nov. 24, 2014, 5:07 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28063/
> -----------------------------------------------------------
>
> (Updated Nov. 24, 2014, 5:07 p.m.)
>
>
> Review request for mesos, Ben Mahler, Niklas Nielsen, and Till Toenshoff.
>
>
> Bugs: MESOS-1571
> https://issues.apache.org/jira/browse/MESOS-1571
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> CommandExecutor grace_period field is designed to be used by slave to propagate the value of the grace period flag further to containerizers and executors.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 0085aba
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp ed63ded
>
> Diff: https://reviews.apache.org/r/28063/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.4, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>
Re: Review Request 28063: Introduced grace period in CommandInfo.
Posted by Alexander Rukletsov <al...@mesosphere.io>.
> On Dec. 3, 2014, 9:20 p.m., Niklas Nielsen wrote:
> > src/slave/slave.cpp, line 2675
> > <https://reviews.apache.org/r/28063/diff/3/?file=774393#file774393line2675>
> >
> > else if? If the task has a command, it cannot have an executor
>
> Alexander Rukletsov wrote:
> There is only one place
There is only one place where we enforce this constraint. I do not want to introduce this check in another piece of code, since the behaviour may change. Current grace period update code will handle both situations correctly.
- Alexander
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28063/#review63475
-----------------------------------------------------------
On Dec. 17, 2014, 11:25 p.m., Alexander Rukletsov wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28063/
> -----------------------------------------------------------
>
> (Updated Dec. 17, 2014, 11:25 p.m.)
>
>
> Review request for mesos, Ben Mahler, Niklas Nielsen, and Till Toenshoff.
>
>
> Bugs: MESOS-1571
> https://issues.apache.org/jira/browse/MESOS-1571
>
>
> Repository: mesos-git
>
>
> Description
> -------
>
> CommandExecutor grace_period field is designed to be used by slave to propagate the value of the grace period flag further to containerizers and executors.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto 0085aba
> src/slave/slave.hpp 70bd8c1
> src/slave/slave.cpp ed63ded
>
> Diff: https://reviews.apache.org/r/28063/diff/
>
>
> Testing
> -------
>
> make check (Mac OS 10.9.4, Ubuntu 14.04)
>
>
> Thanks,
>
> Alexander Rukletsov
>
>