Re: Review Request 64423: Explicitly passed resource-provider information in 'UpdateSlaveMessage'.

2017-12-08 Thread Benjamin Bannier 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64423/
---

(Updated Dec. 8, 2017, 12:32 p.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Addressed Jie's comment.


Summary (updated)
-

Explicitly passed resource-provider information in 'UpdateSlaveMessage'.


Bugs: MESOS-8312
https://issues.apache.org/jira/browse/MESOS-8312


Repository: mesos


Description (updated)
---

This patch changes the way resource-provider related information is
passed. Instead of aggregating all information from both the agent and
resource providers into global per-agent lists in
'UpdateSlaveMessage', with this patch we pass resource-provider
related information explicitly. We can in a subsequent patch surface
this information in e.g., the operator API.


Diffs (updated)
-

  src/master/master.hpp 1f5daae6abbf81cd5fc0341f5ee6c9678beb0d64 
  src/master/master.cpp 5cba50636a9351d29660c54fad7734fcfea547b9 
  src/slave/slave.hpp 5cb0d55970e50c5b8b5ef9c43f2967f4663a7781 
  src/slave/slave.cpp 54d8bcc035227dd6896ffa6e692a91749c0b56a6 
  src/tests/slave_tests.cpp 07145432c35c85a755b66ba575c2a46db072ce5c 


Diff: https://reviews.apache.org/r/64423/diff/3/

Changes: https://reviews.apache.org/r/64423/diff/2-3/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 64423: Explicitly passed resource-provider information in UpdateSlaveMessage.

2017-12-07 Thread Jie Yu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64423/#review193218
---


Ship it!




Ship It!

- Jie Yu


On Dec. 8, 2017, 12:17 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64423/
> ---
> 
> (Updated Dec. 8, 2017, 12:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8312
> https://issues.apache.org/jira/browse/MESOS-8312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes the way resource-provider related information is
> passed. Instead of aggregating all information from both the agent and
> resource providers into global per-agent lists in
> `UpdateSlaveMessage`, with this patch we pass resource-provider
> related information explicitly. We can in a subsequent patch surface
> this information in e.g., the operator API.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 2658312b0d10a72fefda68c7d3137b94afbd8249 
>   src/master/master.cpp 1d192db8edd36dac99ebecd17edd9a4df53a416e 
>   src/slave/slave.hpp bbf5b79528eda42465b62e49d4cea3bda3931241 
>   src/slave/slave.cpp fb077b7a979ebf6b406cd711642dd05f8749f2d0 
>   src/tests/slave_tests.cpp 25cfd4730c3d77d1886745204b13ca3cb140620c 
> 
> 
> Diff: https://reviews.apache.org/r/64423/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 64423: Explicitly passed resource-provider information in UpdateSlaveMessage.

2017-12-07 Thread Benjamin Bannier 


> On Dec. 7, 2017, 8:15 p.m., Jie Yu wrote:
> > src/master/master.cpp
> > Lines 7204-7207 (original), 7223-7226 (patched)
> > 
> >
> > This probably need some adjustment given that message.offer_operations 
> > will be for agent default resources and `slave->offerOperations` will be 
> > for all RPs?

This is only a first heuristic. If `!updated` we perform a more exhaustive 
check below taking offer operations both from the agent and the resource 
providers into account. Most of this code is to avoid unneeded work, and I do 
agree that it currently is not very readible; we should clean it up once we 
explicitly store all interesting resource provider information in the master 
and we wouldn't need to synthetize resource provider information here anymore 
to perform simple checks.


- Benjamin


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64423/#review193150
---


On Dec. 8, 2017, 1:17 a.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64423/
> ---
> 
> (Updated Dec. 8, 2017, 1:17 a.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Bugs: MESOS-8312
> https://issues.apache.org/jira/browse/MESOS-8312
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes the way resource-provider related information is
> passed. Instead of aggregating all information from both the agent and
> resource providers into global per-agent lists in
> `UpdateSlaveMessage`, with this patch we pass resource-provider
> related information explicitly. We can in a subsequent patch surface
> this information in e.g., the operator API.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 2658312b0d10a72fefda68c7d3137b94afbd8249 
>   src/master/master.cpp 1d192db8edd36dac99ebecd17edd9a4df53a416e 
>   src/slave/slave.hpp bbf5b79528eda42465b62e49d4cea3bda3931241 
>   src/slave/slave.cpp fb077b7a979ebf6b406cd711642dd05f8749f2d0 
>   src/tests/slave_tests.cpp 25cfd4730c3d77d1886745204b13ca3cb140620c 
> 
> 
> Diff: https://reviews.apache.org/r/64423/diff/2/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Re: Review Request 64423: Explicitly passed resource-provider information in UpdateSlaveMessage.

2017-12-07 Thread Benjamin Bannier 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64423/
---

(Updated Dec. 8, 2017, 1:17 a.m.)


Review request for mesos, Jie Yu and Jan Schlicht.


Changes
---

Addressed comments from Jie.


Bugs: MESOS-8312
https://issues.apache.org/jira/browse/MESOS-8312


Repository: mesos


Description
---

This patch changes the way resource-provider related information is
passed. Instead of aggregating all information from both the agent and
resource providers into global per-agent lists in
`UpdateSlaveMessage`, with this patch we pass resource-provider
related information explicitly. We can in a subsequent patch surface
this information in e.g., the operator API.


Diffs (updated)
-

  src/master/master.hpp 2658312b0d10a72fefda68c7d3137b94afbd8249 
  src/master/master.cpp 1d192db8edd36dac99ebecd17edd9a4df53a416e 
  src/slave/slave.hpp bbf5b79528eda42465b62e49d4cea3bda3931241 
  src/slave/slave.cpp fb077b7a979ebf6b406cd711642dd05f8749f2d0 
  src/tests/slave_tests.cpp 25cfd4730c3d77d1886745204b13ca3cb140620c 


Diff: https://reviews.apache.org/r/64423/diff/2/

Changes: https://reviews.apache.org/r/64423/diff/1-2/


Testing
---

`make check`


Thanks,

Benjamin Bannier

Re: Review Request 64423: Explicitly passed resource-provider information in UpdateSlaveMessage.

2017-12-07 Thread Jie Yu 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64423/#review193150
---




src/master/master.cpp
Lines 7200 (patched)


no need for this check if it's a required field



src/master/master.cpp
Lines 7207-7208 (patched)


Ditto. let's make it a required field



src/master/master.cpp
Lines 7204-7207 (original), 7223-7226 (patched)


This probably need some adjustment given that message.offer_operations will 
be for agent default resources and `slave->offerOperations` will be for all RPs?



src/master/master.cpp
Line 7306 (original), 7341 (patched)


i was expecting that message.offer_operations is only for agent default 
resources.

Any resource provider resources should go to the corresponding resource 
provider struct..



src/slave/slave.cpp
Lines 6800 (patched)


Let's use `previousOversubscribedResources` here to be more explicit



src/slave/slave.cpp
Lines 6850 (patched)


fix style issue


- Jie Yu


On Dec. 7, 2017, 5:49 p.m., Benjamin Bannier wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/64423/
> ---
> 
> (Updated Dec. 7, 2017, 5:49 p.m.)
> 
> 
> Review request for mesos, Jie Yu and Jan Schlicht.
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> This patch changes the way resource-provider related information is
> passed. Instead of aggregating all information from both the agent and
> resource providers into global per-agent lists in
> `UpdateSlaveMessage`, with this patch we pass resource-provider
> related information explicitly. We can in a subsequent patch surface
> this information in e.g., the operator API.
> 
> 
> Diffs
> -
> 
>   src/master/master.hpp 2658312b0d10a72fefda68c7d3137b94afbd8249 
>   src/master/master.cpp 2fd66c072e9a194680d7653c664bd8a68ea1d2f0 
>   src/slave/slave.hpp fc762fb1e8aeb57b0d7ad551d960ec4be06e8e33 
>   src/slave/slave.cpp dd830d93f8a08720231e3d4a6421f7a058ba6093 
>   src/tests/slave_tests.cpp 25cfd4730c3d77d1886745204b13ca3cb140620c 
> 
> 
> Diff: https://reviews.apache.org/r/64423/diff/1/
> 
> 
> Testing
> ---
> 
> `make check`
> 
> 
> Thanks,
> 
> Benjamin Bannier
> 
>

Review Request 64423: Explicitly passed resource-provider information in UpdateSlaveMessage.

2017-12-07 Thread Benjamin Bannier 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/64423/
---

Review request for mesos, Jie Yu and Jan Schlicht.


Repository: mesos


Description
---

This patch changes the way resource-provider related information is
passed. Instead of aggregating all information from both the agent and
resource providers into global per-agent lists in
`UpdateSlaveMessage`, with this patch we pass resource-provider
related information explicitly. We can in a subsequent patch surface
this information in e.g., the operator API.


Diffs
-

  src/master/master.hpp 2658312b0d10a72fefda68c7d3137b94afbd8249 
  src/master/master.cpp 2fd66c072e9a194680d7653c664bd8a68ea1d2f0 
  src/slave/slave.hpp fc762fb1e8aeb57b0d7ad551d960ec4be06e8e33 
  src/slave/slave.cpp dd830d93f8a08720231e3d4a6421f7a058ba6093 
  src/tests/slave_tests.cpp 25cfd4730c3d77d1886745204b13ca3cb140620c 


Diff: https://reviews.apache.org/r/64423/diff/1/


Testing
---

`make check`


Thanks,

Benjamin Bannier