You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Bartek Plotka <bw...@gmail.com> on 2015/05/22 00:51:24 UTC
Review Request 34571: Pushed QoS Correction stub message to
mesos.proto
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34571/
-----------------------------------------------------------
Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone.
Bugs: MESOS-2760
https://issues.apache.org/jira/browse/MESOS-2760
Repository: mesos
Description
-------
Pushed QoS Correction stub message to mesos.proto.
This part of proto describies QoS corrections message which includes corrections for particular executor or task.
It's generic message between QoS Controller and slave.
Diffs
-----
include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99
Diff: https://reviews.apache.org/r/34571/diff/
Testing
-------
make check
Thanks,
Bartek Plotka
Re: Review Request 34571: Pushed QoS Correction stub message to
mesos.proto
Posted by Szymon Konefal <sz...@gmail.com>.
> On May 21, 2015, 11:08 p.m., Niklas Nielsen wrote:
> > A higher level question is whether we should have touples of actions.
> >
> > For example:
> >
> > message KillAction {
> > optional ExecutorID executor_id = X;
> > }
> >
> > message ResizeAction {
> > optional TaskID task_id = X;
> > optional Resources resources = Y;
> > }
> >
> > ...
Yes, the resizing action of custom isolators could be tricky to formalize.
We have two choices:
1) Make an union of possible actions with bodies
2) Use labels to carry metadata
I would go for the first choice. If someone would like to resize his custom iso, one should need to extend the ResiseAction message.
- Szymon
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34571/#review84814
-----------------------------------------------------------
On May 21, 2015, 11:02 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34571/
> -----------------------------------------------------------
>
> (Updated May 21, 2015, 11:02 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone.
>
>
> Bugs: MESOS-2760
> https://issues.apache.org/jira/browse/MESOS-2760
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This part of proto describes a QoS corrections message which includes corrections for particular executors or tasks.
> It is a generic message between QoS Controller and slave.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99
>
> Diff: https://reviews.apache.org/r/34571/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34571: Pushed QoS Correction stub message to
mesos.proto
Posted by Niklas Nielsen <ni...@qni.dk>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34571/#review84814
-----------------------------------------------------------
A higher level question is whether we should have touples of actions.
For example:
message KillAction {
optional ExecutorID executor_id = X;
}
message ResizeAction {
optional TaskID task_id = X;
optional Resources resources = Y;
}
...
include/mesos/mesos.proto
<https://reviews.apache.org/r/34571/#comment136205>
Kill this newline :)
include/mesos/mesos.proto
<https://reviews.apache.org/r/34571/#comment136206>
And this one.
- Niklas Nielsen
On May 21, 2015, 4:02 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34571/
> -----------------------------------------------------------
>
> (Updated May 21, 2015, 4:02 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone.
>
>
> Bugs: MESOS-2760
> https://issues.apache.org/jira/browse/MESOS-2760
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This part of proto describes a QoS corrections message which includes corrections for particular executors or tasks.
> It is a generic message between QoS Controller and slave.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99
>
> Diff: https://reviews.apache.org/r/34571/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34571: Pushed QoS Correction stub message to
mesos.proto
Posted by Niklas Nielsen <ni...@qni.dk>.
> On May 21, 2015, 4:24 p.m., Jie Yu wrote:
> > include/mesos/mesos.proto, line 1189
> > <https://reviews.apache.org/r/34571/diff/1/?file=968376#file968376line1189>
> >
> > This is a protocol between slave and the QoSController, both of which are internal to Mesos. So we probably should move this message definition to src/messages/messages.proto.
>
> Niklas Nielsen wrote:
> We need this to be public to make QoS Controller modules :)
>
> Jie Yu wrote:
> ah. Then how about creating a new proto file
>
> include/mesos/slave/oversubscription.proto
I like it!
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34571/#review84817
-----------------------------------------------------------
On May 21, 2015, 4:02 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34571/
> -----------------------------------------------------------
>
> (Updated May 21, 2015, 4:02 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone.
>
>
> Bugs: MESOS-2760
> https://issues.apache.org/jira/browse/MESOS-2760
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This part of proto describes a QoS corrections message which includes corrections for particular executors or tasks.
> It is a generic message between QoS Controller and slave.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99
>
> Diff: https://reviews.apache.org/r/34571/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34571: Pushed QoS Correction stub message to
mesos.proto
Posted by Bartek Plotka <bw...@gmail.com>.
> On May 21, 2015, 11:24 p.m., Jie Yu wrote:
> > include/mesos/mesos.proto, lines 1189-1211
> > <https://reviews.apache.org/r/34571/diff/1/?file=968376#file968376line1189>
> >
> > It reads weired when one wants to create a QoS correction (too many words 'correction'):
> >
> > ```
> > QoSCorrections::Correction correction;
> > ```
> >
> > How about we just define the `QoSCorrection` message and create a wrapper class `QoSCorrections` in C++ (similar to what we did for `Resource`)?
Yeah that's the same story as with the Resource - we can do that.
- Bartek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34571/#review84817
-----------------------------------------------------------
On May 21, 2015, 11:02 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34571/
> -----------------------------------------------------------
>
> (Updated May 21, 2015, 11:02 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone.
>
>
> Bugs: MESOS-2760
> https://issues.apache.org/jira/browse/MESOS-2760
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This part of proto describes a QoS corrections message which includes corrections for particular executors or tasks.
> It is a generic message between QoS Controller and slave.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99
>
> Diff: https://reviews.apache.org/r/34571/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34571: Pushed QoS Correction stub message to
mesos.proto
Posted by Niklas Nielsen <ni...@qni.dk>.
> On May 21, 2015, 4:24 p.m., Jie Yu wrote:
> > include/mesos/mesos.proto, line 1189
> > <https://reviews.apache.org/r/34571/diff/1/?file=968376#file968376line1189>
> >
> > This is a protocol between slave and the QoSController, both of which are internal to Mesos. So we probably should move this message definition to src/messages/messages.proto.
We need this to be public to make QoS Controller modules :)
> On May 21, 2015, 4:24 p.m., Jie Yu wrote:
> > include/mesos/mesos.proto, lines 1189-1211
> > <https://reviews.apache.org/r/34571/diff/1/?file=968376#file968376line1189>
> >
> > It reads weired when one wants to create a QoS correction (too many words 'correction'):
> >
> > ```
> > QoSCorrections::Correction correction;
> > ```
> >
> > How about we just define the `QoSCorrection` message and create a wrapper class `QoSCorrections` in C++ (similar to what we did for `Resource`)?
>
> Bartek Plotka wrote:
> Yeah that's the same story as with the Resource - we can do that.
SGTM - or just return a list of corrections. Don't have a strong opinion on which one.
- Niklas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34571/#review84817
-----------------------------------------------------------
On May 21, 2015, 4:02 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34571/
> -----------------------------------------------------------
>
> (Updated May 21, 2015, 4:02 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone.
>
>
> Bugs: MESOS-2760
> https://issues.apache.org/jira/browse/MESOS-2760
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This part of proto describes a QoS corrections message which includes corrections for particular executors or tasks.
> It is a generic message between QoS Controller and slave.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99
>
> Diff: https://reviews.apache.org/r/34571/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34571: Pushed QoS Correction stub message to
mesos.proto
Posted by Jie Yu <yu...@gmail.com>.
> On May 21, 2015, 11:24 p.m., Jie Yu wrote:
> > include/mesos/mesos.proto, line 1189
> > <https://reviews.apache.org/r/34571/diff/1/?file=968376#file968376line1189>
> >
> > This is a protocol between slave and the QoSController, both of which are internal to Mesos. So we probably should move this message definition to src/messages/messages.proto.
>
> Niklas Nielsen wrote:
> We need this to be public to make QoS Controller modules :)
ah. Then how about creating a new proto file
include/mesos/slave/oversubscription.proto
- Jie
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34571/#review84817
-----------------------------------------------------------
On May 21, 2015, 11:02 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34571/
> -----------------------------------------------------------
>
> (Updated May 21, 2015, 11:02 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone.
>
>
> Bugs: MESOS-2760
> https://issues.apache.org/jira/browse/MESOS-2760
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This part of proto describes a QoS corrections message which includes corrections for particular executors or tasks.
> It is a generic message between QoS Controller and slave.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99
>
> Diff: https://reviews.apache.org/r/34571/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34571: Pushed QoS Correction stub message to
mesos.proto
Posted by Jie Yu <yu...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34571/#review84817
-----------------------------------------------------------
include/mesos/mesos.proto
<https://reviews.apache.org/r/34571/#comment136208>
This is a protocol between slave and the QoSController, both of which are internal to Mesos. So we probably should move this message definition to src/messages/messages.proto.
include/mesos/mesos.proto
<https://reviews.apache.org/r/34571/#comment136212>
It reads weired when one wants to create a QoS correction (too many words 'correction'):
```
QoSCorrections::Correction correction;
```
How about we just define the `QoSCorrection` message and create a wrapper class `QoSCorrections` in C++ (similar to what we did for `Resource`)?
include/mesos/mesos.proto
<https://reviews.apache.org/r/34571/#comment136210>
I liked Niklas's suggestion. How about following the style used by Offer.Operation. Also, Let's try not to introduce those fields that are unneeded right now.
```
message QoSCorrection {
enum Type {
KILL = 1;
}
message Kill {
required ContainerID container_id = 1;
}
required Type type = 1;
optional Kill kill = 2;
}
```
- Jie Yu
On May 21, 2015, 11:02 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34571/
> -----------------------------------------------------------
>
> (Updated May 21, 2015, 11:02 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone.
>
>
> Bugs: MESOS-2760
> https://issues.apache.org/jira/browse/MESOS-2760
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This part of proto describes a QoS corrections message which includes corrections for particular executors or tasks.
> It is a generic message between QoS Controller and slave.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99
>
> Diff: https://reviews.apache.org/r/34571/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34571: Pushed QoS Correction stub message to
mesos.proto
Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34571/#review84829
-----------------------------------------------------------
Patch looks great!
Reviews applied: [34571]
All tests passed.
- Mesos ReviewBot
On May 21, 2015, 11:02 p.m., Bartek Plotka wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/34571/
> -----------------------------------------------------------
>
> (Updated May 21, 2015, 11:02 p.m.)
>
>
> Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone.
>
>
> Bugs: MESOS-2760
> https://issues.apache.org/jira/browse/MESOS-2760
>
>
> Repository: mesos
>
>
> Description
> -------
>
> This part of proto describes a QoS corrections message which includes corrections for particular executors or tasks.
> It is a generic message between QoS Controller and slave.
>
>
> Diffs
> -----
>
> include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99
>
> Diff: https://reviews.apache.org/r/34571/diff/
>
>
> Testing
> -------
>
> make check
>
>
> Thanks,
>
> Bartek Plotka
>
>
Re: Review Request 34571: Pushed QoS Correction stub message to
mesos.proto
Posted by Bartek Plotka <bw...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/34571/
-----------------------------------------------------------
(Updated May 21, 2015, 4:02 p.m.)
Review request for mesos, Jie Yu, Niklas Nielsen, and Vinod Kone.
Bugs: MESOS-2760
https://issues.apache.org/jira/browse/MESOS-2760
Repository: mesos
Description (updated)
-------
This part of proto describes a QoS corrections message which includes corrections for particular executors or tasks.
It is a generic message between QoS Controller and slave.
Diffs
-----
include/mesos/mesos.proto a66888916dc90e476c8bb20e67e3f6b08c47fb99
Diff: https://reviews.apache.org/r/34571/diff/
Testing
-------
make check
Thanks,
Bartek Plotka