You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Vinod Kone <vi...@gmail.com> on 2015/12/05 01:21:17 UTC
Re: Review Request 39848: Validate revocable resources
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39848/#review109063
-----------------------------------------------------------
src/slave/slave.cpp (line 1503)
<https://reviews.apache.org/r/39848/#comment168473>
I think at this point it's worthwhile to factor out the validations in this function into slave/validation.cpp file like we have done in master/validation.cpp. The validation should likely return an object (TaskStatus?) that contains the message and REASON to be used in the status update.
This would also help with unit testing the validations more easily.
- Vinod Kone
On Nov. 5, 2015, 7:36 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39848/
> -----------------------------------------------------------
>
> (Updated Nov. 5, 2015, 7:36 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-2647
> https://issues.apache.org/jira/browse/MESOS-2647
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Validates if there are enough revocable resources when launching
> the task on slave.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp ddeece4334b13cf8b5ab7d87012bbf946640025b
>
> Diff: https://reviews.apache.org/r/39848/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 39848: Validate revocable resources
Posted by Guangya Liu <gy...@gmail.com>.
> On 十二月 5, 2015, 12:21 a.m., Vinod Kone wrote:
> > src/slave/slave.cpp, line 1503
> > <https://reviews.apache.org/r/39848/diff/2/?file=1116597#file1116597line1503>
> >
> > I think at this point it's worthwhile to factor out the validations in this function into slave/validation.cpp file like we have done in master/validation.cpp. The validation should likely return an object (TaskStatus?) that contains the message and REASON to be used in the status update.
> >
> > This would also help with unit testing the validations more easily.
>
> Guangya Liu wrote:
> OK, will factor out the validations to slave/validation.cpp. Can you please help check if the logic is OK here? As I was using one double value to minus another, shall we use CHECK_NEAR as the solution in https://reviews.apache.org/r/40767/diff/1#index_header ? Thanks!
Vinod, after a second think, putting the validation here may be better as the current validation does not fit into validate() logic in master/validation.cpp slave/validation.cpp well. All of the validate() APIs are returning Error, it may make the logic not clear if we put the above logic to slave/validation.cpp. Actually the current _runTask also have some validateion logic, such as checkpointedResources, checkpointedExecutorResources etc. What do you say? Thanks.
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39848/#review109063
-----------------------------------------------------------
On 十一月 5, 2015, 7:36 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39848/
> -----------------------------------------------------------
>
> (Updated 十一月 5, 2015, 7:36 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-2647
> https://issues.apache.org/jira/browse/MESOS-2647
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Validates if there are enough revocable resources when launching
> the task on slave.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp ddeece4334b13cf8b5ab7d87012bbf946640025b
>
> Diff: https://reviews.apache.org/r/39848/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>
Re: Review Request 39848: Validate revocable resources
Posted by Guangya Liu <gy...@gmail.com>.
> On 十二月 5, 2015, 12:21 a.m., Vinod Kone wrote:
> > src/slave/slave.cpp, line 1503
> > <https://reviews.apache.org/r/39848/diff/2/?file=1116597#file1116597line1503>
> >
> > I think at this point it's worthwhile to factor out the validations in this function into slave/validation.cpp file like we have done in master/validation.cpp. The validation should likely return an object (TaskStatus?) that contains the message and REASON to be used in the status update.
> >
> > This would also help with unit testing the validations more easily.
OK, will factor out the validations to slave/validation.cpp. Can you please help check if the logic is OK here? As I was using one double value to minus another, shall we use CHECK_NEAR as the solution in https://reviews.apache.org/r/40767/diff/1#index_header ? Thanks!
- Guangya
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/39848/#review109063
-----------------------------------------------------------
On 十一月 5, 2015, 7:36 a.m., Guangya Liu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/39848/
> -----------------------------------------------------------
>
> (Updated 十一月 5, 2015, 7:36 a.m.)
>
>
> Review request for mesos and Vinod Kone.
>
>
> Bugs: MESOS-2647
> https://issues.apache.org/jira/browse/MESOS-2647
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Validates if there are enough revocable resources when launching
> the task on slave.
>
>
> Diffs
> -----
>
> src/slave/slave.cpp ddeece4334b13cf8b5ab7d87012bbf946640025b
>
> Diff: https://reviews.apache.org/r/39848/diff/
>
>
> Testing
> -------
>
> make
> make check
>
>
> Thanks,
>
> Guangya Liu
>
>