You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Michael Park <mp...@apache.org> on 2017/06/14 09:56:13 UTC
Review Request 60069: Updated the master to account for the new
resources format.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60069/
-----------------------------------------------------------
Review request for mesos and Neil Conway.
Bugs: MESOS-7667
https://issues.apache.org/jira/browse/MESOS-7667
Repository: mesos
Description
-------
Updated the master to account for the new resources format.
Diffs
-----
src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323
src/master/master.hpp e8ddddf273256b14cde1cac390163f948241757f
src/master/master.cpp 3c6925dc6055fb7cec12ec03bc557ad462863c58
src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7
src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715
Diff: https://reviews.apache.org/r/60069/diff/1/
Testing
-------
Thanks,
Michael Park
Re: Review Request 60069: Updated the master to account for the new
resources format.
Posted by Michael Park <mp...@apache.org>.
> On June 15, 2017, 1:32 a.m., Neil Conway wrote:
> > src/master/master.cpp
> > Lines 3908 (patched)
> > <https://reviews.apache.org/r/60069/diff/1/?file=1750276#file1750276line3911>
> >
> > Do we want to check that if the framework claims to _not_ be aware of reservation refinement, it has _not_ used the new protobuf fields?
We already check this in master validation. https://reviews.apache.org/r/60013
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60069/#review177913
-----------------------------------------------------------
On June 14, 2017, 2:56 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60069/
> -----------------------------------------------------------
>
> (Updated June 14, 2017, 2:56 a.m.)
>
>
> Review request for mesos and Neil Conway.
>
>
> Bugs: MESOS-7667
> https://issues.apache.org/jira/browse/MESOS-7667
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated the master to account for the new resources format.
>
>
> Diffs
> -----
>
> src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323
> src/master/master.hpp e8ddddf273256b14cde1cac390163f948241757f
> src/master/master.cpp 3c6925dc6055fb7cec12ec03bc557ad462863c58
> src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7
> src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715
>
>
> Diff: https://reviews.apache.org/r/60069/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 60069: Adjusted the master to the new resources
format [10/N].
Posted by Michael Park <mp...@apache.org>.
> On June 15, 2017, 1:32 a.m., Neil Conway wrote:
> > src/master/master.cpp
> > Line 3907 (original), 3907 (patched)
> > <https://reviews.apache.org/r/60069/diff/1/?file=1750276#file1750276line3910>
> >
> > Confused why this is in the same loop as `injectAllocationInfo`; updating the comment would help.
This was actually incorrect, as `convertResourceFormat` requires valid resources. This has been moved to be after `operation::validate` calls.
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60069/#review177913
-----------------------------------------------------------
On June 14, 2017, 2:56 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60069/
> -----------------------------------------------------------
>
> (Updated June 14, 2017, 2:56 a.m.)
>
>
> Review request for mesos and Neil Conway.
>
>
> Bugs: MESOS-7667
> https://issues.apache.org/jira/browse/MESOS-7667
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adjusted the master to the new resources format [10/N].
>
>
> Diffs
> -----
>
> src/master/http.cpp 4f838493104146e1a6b68eb64d8ec122cd54b7f7
> src/master/master.hpp 9c1e95c8ba4c649680c8b5dd3bf26aeea1cfcdc0
> src/master/master.cpp 287a5b71bc61949648ac0edff7668f217357a054
> src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7
> src/master/quota_handler.cpp 7fe55808d4561ae5a8850ad904d09a7c65e014ce
>
>
> Diff: https://reviews.apache.org/r/60069/diff/5/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 60069: Updated the master to account for the new
resources format.
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60069/#review177913
-----------------------------------------------------------
src/master/master.cpp
Line 3492 (original), 3492 (patched)
<https://reviews.apache.org/r/60069/#comment251617>
Update this comment? i.e., "all roles included in `reserve.resources`" is no longer a complete description.
src/master/master.cpp
Line 3905 (original), 3905 (patched)
<https://reviews.apache.org/r/60069/#comment251707>
Seems like this comment needs updating.
src/master/master.cpp
Line 3907 (original), 3907 (patched)
<https://reviews.apache.org/r/60069/#comment251710>
Confused why this is in the same loop as `injectAllocationInfo`; updating the comment would help.
src/master/master.cpp
Lines 3908 (patched)
<https://reviews.apache.org/r/60069/#comment251711>
Do we want to check that if the framework claims to _not_ be aware of reservation refinement, it has _not_ used the new protobuf fields?
src/master/master.cpp
Line 9247 (original), 9257 (patched)
<https://reviews.apache.org/r/60069/#comment251618>
Do we use this syntax elsewhere? Seemed a bit surprising to me.
src/master/quota.cpp
Line 145 (original), 150 (patched)
<https://reviews.apache.org/r/60069/#comment251708>
Do we want to remove this check? Seems like resources with `reservation` should still be rejected.
- Neil Conway
On June 14, 2017, 9:56 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60069/
> -----------------------------------------------------------
>
> (Updated June 14, 2017, 9:56 a.m.)
>
>
> Review request for mesos and Neil Conway.
>
>
> Bugs: MESOS-7667
> https://issues.apache.org/jira/browse/MESOS-7667
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated the master to account for the new resources format.
>
>
> Diffs
> -----
>
> src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323
> src/master/master.hpp e8ddddf273256b14cde1cac390163f948241757f
> src/master/master.cpp 3c6925dc6055fb7cec12ec03bc557ad462863c58
> src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7
> src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715
>
>
> Diff: https://reviews.apache.org/r/60069/diff/1/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 60069: Updated the master to account for the new
resources format.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60069/#review178038
-----------------------------------------------------------
src/master/quota.cpp
Lines 159-162 (original), 165-168 (patched)
<https://reviews.apache.org/r/60069/#comment251820>
Since the resources are transformed, `resource.role` will no longer be set. It is now sufficient to just check for `reservations_size() > 0`.
src/master/quota.cpp
Lines 159-162 (original), 165-168 (patched)
<https://reviews.apache.org/r/60069/#comment251821>
Since the resources are transformed, `resource.role` will no longer be set. It is now sufficient to just check for `reservations_size() > 0`.
- Michael Park
On June 14, 2017, 2:56 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60069/
> -----------------------------------------------------------
>
> (Updated June 14, 2017, 2:56 a.m.)
>
>
> Review request for mesos and Neil Conway.
>
>
> Bugs: MESOS-7667
> https://issues.apache.org/jira/browse/MESOS-7667
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Updated the master to account for the new resources format.
>
>
> Diffs
> -----
>
> src/master/http.cpp 1dcfe6ef00b0e3984deb79a511e665f638661323
> src/master/master.hpp e8ddddf273256b14cde1cac390163f948241757f
> src/master/master.cpp 3c6925dc6055fb7cec12ec03bc557ad462863c58
> src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7
> src/master/validation.cpp f45f9e816bdaf6304794604570edbd68d5faf715
>
>
> Diff: https://reviews.apache.org/r/60069/diff/2/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 60069: Adjusted the master to the new resources
format [10/N].
Posted by Neil Conway <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60069/#review178189
-----------------------------------------------------------
Ship it!
Ship It!
- Neil Conway
On June 14, 2017, 9:56 a.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60069/
> -----------------------------------------------------------
>
> (Updated June 14, 2017, 9:56 a.m.)
>
>
> Review request for mesos and Neil Conway.
>
>
> Bugs: MESOS-7667
> https://issues.apache.org/jira/browse/MESOS-7667
>
>
> Repository: mesos
>
>
> Description
> -------
>
> See summary.
>
>
> Diffs
> -----
>
> src/master/http.cpp 4f838493104146e1a6b68eb64d8ec122cd54b7f7
> src/master/master.hpp 9c1e95c8ba4c649680c8b5dd3bf26aeea1cfcdc0
> src/master/master.cpp 287a5b71bc61949648ac0edff7668f217357a054
> src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7
> src/master/quota_handler.cpp 7fe55808d4561ae5a8850ad904d09a7c65e014ce
>
>
> Diff: https://reviews.apache.org/r/60069/diff/3/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 60069: Adjusted the master to the new resources
format [10/N].
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60069/#review178347
-----------------------------------------------------------
src/master/master.cpp
Line 3613 (original), 3638 (patched)
<https://reviews.apache.org/r/60069/#comment252253>
Fix this to account for both formats. We're not validated nor converted at this point.
src/master/master.cpp
Line 3665 (original), 3691 (patched)
<https://reviews.apache.org/r/60069/#comment252254>
Change this since we're not validated nor converted at this point.
src/master/master.cpp
Line 4165 (original), 4191 (patched)
<https://reviews.apache.org/r/60069/#comment252251>
We iterate by copy here
src/master/master.cpp
Lines 4787-4789 (original), 4867-4869 (patched)
<https://reviews.apache.org/r/60069/#comment252252>
Leave this alone.
- Michael Park
On June 19, 2017, 10:33 p.m., Michael Park wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60069/
> -----------------------------------------------------------
>
> (Updated June 19, 2017, 10:33 p.m.)
>
>
> Review request for mesos and Neil Conway.
>
>
> Bugs: MESOS-7667
> https://issues.apache.org/jira/browse/MESOS-7667
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Adjusted the master to the new resources format [10/N].
>
>
> Diffs
> -----
>
> src/master/http.cpp 4f838493104146e1a6b68eb64d8ec122cd54b7f7
> src/master/master.hpp 9c1e95c8ba4c649680c8b5dd3bf26aeea1cfcdc0
> src/master/master.cpp 287a5b71bc61949648ac0edff7668f217357a054
> src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7
> src/master/quota_handler.cpp 7fe55808d4561ae5a8850ad904d09a7c65e014ce
>
>
> Diff: https://reviews.apache.org/r/60069/diff/6/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Michael Park
>
>
Re: Review Request 60069: Adjusted the master to the new resources
format [10/N].
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60069/
-----------------------------------------------------------
(Updated June 20, 2017, 12:16 a.m.)
Review request for mesos and Neil Conway.
Bugs: MESOS-7667
https://issues.apache.org/jira/browse/MESOS-7667
Repository: mesos
Description
-------
Adjusted the master to the new resources format [10/N].
Diffs (updated)
-----
src/master/http.cpp 4f838493104146e1a6b68eb64d8ec122cd54b7f7
src/master/master.hpp 9c1e95c8ba4c649680c8b5dd3bf26aeea1cfcdc0
src/master/master.cpp 287a5b71bc61949648ac0edff7668f217357a054
src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7
src/master/quota_handler.cpp 7fe55808d4561ae5a8850ad904d09a7c65e014ce
Diff: https://reviews.apache.org/r/60069/diff/8/
Changes: https://reviews.apache.org/r/60069/diff/7-8/
Testing
-------
Thanks,
Michael Park
Re: Review Request 60069: Adjusted the master to the new resources
format [10/N].
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/60069/
-----------------------------------------------------------
(Updated June 19, 2017, 10:33 p.m.)
Review request for mesos and Neil Conway.
Bugs: MESOS-7667
https://issues.apache.org/jira/browse/MESOS-7667
Repository: mesos
Description
-------
Adjusted the master to the new resources format [10/N].
Diffs (updated)
-----
src/master/http.cpp 4f838493104146e1a6b68eb64d8ec122cd54b7f7
src/master/master.hpp 9c1e95c8ba4c649680c8b5dd3bf26aeea1cfcdc0
src/master/master.cpp 287a5b71bc61949648ac0edff7668f217357a054
src/master/quota.cpp 3e0d280efff69904b74e5d4c71985ab44ffa34a7
src/master/quota_handler.cpp 7fe55808d4561ae5a8850ad904d09a7c65e014ce
Diff: https://reviews.apache.org/r/60069/diff/6/
Changes: https://reviews.apache.org/r/60069/diff/5-6/
Testing
-------
Thanks,
Michael Park