Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-05-01 Thread Zhitao Li 

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

(Updated May 1, 2018, 3:56 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Review comments.


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


Repository: mesos


Description
---

These operator APIs is implemented as speculative for now, but
we plan to convert them to non-speculative in the future.


Diffs (updated)
-

  src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 
  src/master/master.hpp c30cf082d9ddf2e5dd97e0d887d6d07d8e03cbd4 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


Diff: https://reviews.apache.org/r/66051/diff/16/

Changes: https://reviews.apache.org/r/66051/diff/15-16/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-05-01 Thread Chun-Hung Hsiao 

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


Fix it, then Ship it!





src/master/http.cpp
Lines 1557 (patched)


The `volume` and `addition` fields contain



src/master/http.cpp
Lines 1625 (patched)


Suggestion: The `volume` field contains the resource required for this 
operation.


- Chun-Hung Hsiao


On April 27, 2018, 8:09 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66051/
> ---
> 
> (Updated April 27, 2018, 8:09 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-8747
> https://issues.apache.org/jira/browse/MESOS-8747
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These operator APIs is implemented as speculative for now, but
> we plan to convert them to non-speculative in the future.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 
>   src/master/master.hpp a7cadd9c97add92a0276bf64e0da941cae63fd7c 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
> 
> 
> Diff: https://reviews.apache.org/r/66051/diff/15/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-04-27 Thread Zhitao Li 

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

(Updated April 27, 2018, 1:09 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Updated after validation change.


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


Repository: mesos


Description
---

These operator APIs is implemented as speculative for now, but
we plan to convert them to non-speculative in the future.


Diffs (updated)
-

  src/master/http.cpp 135ae4389623385a6638158f5f23d6daca14a0ad 
  src/master/master.hpp a7cadd9c97add92a0276bf64e0da941cae63fd7c 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


Diff: https://reviews.apache.org/r/66051/diff/15/

Changes: https://reviews.apache.org/r/66051/diff/14-15/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-04-20 Thread Zhitao Li 

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

(Updated April 20, 2018, 4:18 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Renamed Authorization.


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


Repository: mesos


Description
---

These operator APIs is implemented as speculative for now, but
we plan to convert them to non-speculative in the future.


Diffs (updated)
-

  src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
  src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


Diff: https://reviews.apache.org/r/66051/diff/13/

Changes: https://reviews.apache.org/r/66051/diff/12-13/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-04-19 Thread Zhitao Li 

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

(Updated April 19, 2018, 1 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Added authorization.


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


Repository: mesos


Description
---

These operator APIs is implemented as speculative for now, but
we plan to convert them to non-speculative in the future.


Diffs (updated)
-

  src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
  src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


Diff: https://reviews.apache.org/r/66051/diff/12/

Changes: https://reviews.apache.org/r/66051/diff/11-12/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-04-17 Thread Zhitao Li 

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

(Updated April 17, 2018, 11:37 a.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Removed `resource_provider_id`


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


Repository: mesos


Description
---

These operator APIs is implemented as speculative for now, but
we plan to convert them to non-speculative in the future.


Diffs (updated)
-

  src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
  src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


Diff: https://reviews.apache.org/r/66051/diff/11/

Changes: https://reviews.apache.org/r/66051/diff/10-11/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-04-09 Thread Zhitao Li 


> On March 27, 2018, 6:58 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 11881-11887 (original), 11885-11890 (patched)
> > 
> >
> > Currently, it looks like `Slave::usedResources` is the same as the 
> > "allocated resources" from that agent.
> > 
> > When a GROW or SHRINK operation is initiated by an operator, its 
> > consumed resource is in a unique state in the master while that operation 
> > is pending: the volume is not allocated, but it is "used" in some sense.
> > 
> > One place where we use `Slave::usedResources` is when validating 
> > DESTROY operations: 
> > https://github.com/apache/mesos/blob/21e4b45b388d0b272236b1c58313569f8a1d1fc8/src/master/master.cpp#L4841
> > 
> > This means if the master receives a DESTROY call for a persistent 
> > volume while there is a pending operator-initiated GROW_VOLUME call for 
> > that same volume (i.e. the operation was forwarded to the agent for 
> > processing but the master hasn't heard back yet), we would accept the 
> > DESTROY operation and forward it to the agent. Is this what we want?
> > 
> > This brings something else to mind: would it be possible for the 
> > allocator to send out an offer containing the consumed resource of a 
> > GROW_VOLUME or SHRINK_VOLUME operation, while the operation is pending on 
> > the agent? This seems bad, but I believe it's possible. I think this may be 
> > the first time we have a set of resources which we don't want the allocator 
> > to offer for a period of time, but which are not currently allocated to 
> > some framework/role. I wonder if we need some new tools in the allocator to 
> > handle this?
> 
> Chun-Hung Hsiao wrote:
> Yeah this sounds bad. Would it help if we remove the consumed resources 
> from the allocator and put it back after receiving the terminal status update?
> 
> Zhitao Li wrote:
> I agree this is problematic.
> 
> It seems like we need to make sure several things:
> 1. `consumed` resources of a non-speculative operation can not offered to 
> any framework while pending;
> 2. `consumed` resources should still be visible for various validation;
> 
> Therefore, I'm thinking about introducing a new field in `Master::Slave` 
> struct which represents `pendingConsumedResources` (or a function in `Slave` 
> which can extract this info from pending operations). We can then use it to 
> validation of `DESTROY` call.
> 
> Another issue I found was in `Master::apply()`, which we directly call 
> `allocator->updateAvailable(slave->id, {operation})`, but semantically we 
> should be calling `allocator->updateAvailable(slave->id, consumed, empty)` so 
> that allocator would not see the `converted` resources until operation 
> confirmations comes back.

I'm separating the actual logic change of supporting non-speculative operator 
API into future task and patch.


- Zhitao


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


On April 9, 2018, 9:32 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66051/
> ---
> 
> (Updated April 9, 2018, 9:32 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These operator APIs is implemented as speculative for now, but
> we plan to convert them to non-speculative in the future.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
>   src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
> 
> 
> Diff: https://reviews.apache.org/r/66051/diff/10/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-04-09 Thread Zhitao Li 


> On March 27, 2018, 6:05 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 10798-10837 (original), 10794-10839 (patched)
> > 
> >
> > As discussed in chat, we need to update the agent's resources in the 
> > allocator for operator-initiated operations.

I'm separating the actual logic change of supporting non-speculative operator 
API into future task and patch.


- Zhitao


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


On April 9, 2018, 9:32 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66051/
> ---
> 
> (Updated April 9, 2018, 9:32 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> These operator APIs is implemented as speculative for now, but
> we plan to convert them to non-speculative in the future.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
>   src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
>   src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 
> 
> 
> Diff: https://reviews.apache.org/r/66051/diff/10/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-04-09 Thread Zhitao Li 

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

(Updated April 9, 2018, 9:32 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Style and validation comments from Greg.


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


Repository: mesos


Description
---

These operator APIs is implemented as speculative for now, but
we plan to convert them to non-speculative in the future.


Diffs (updated)
-

  src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
  src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


Diff: https://reviews.apache.org/r/66051/diff/10/

Changes: https://reviews.apache.org/r/66051/diff/9-10/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-04-09 Thread Zhitao Li 

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

(Updated April 9, 2018, 9:21 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Implement as speculative for now.


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


Repository: mesos


Description (updated)
---

These operator APIs is implemented as speculative for now, but
we plan to convert them to non-speculative in the future.


Diffs (updated)
-

  src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
  src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
  src/master/validation.cpp ac2e1bb8771841ec59b3bdcdeffb6c6230680d4d 


Diff: https://reviews.apache.org/r/66051/diff/9/

Changes: https://reviews.apache.org/r/66051/diff/8-9/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-03-28 Thread Zhitao Li 


> On March 27, 2018, 6:58 p.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 11881-11887 (original), 11885-11890 (patched)
> > 
> >
> > Currently, it looks like `Slave::usedResources` is the same as the 
> > "allocated resources" from that agent.
> > 
> > When a GROW or SHRINK operation is initiated by an operator, its 
> > consumed resource is in a unique state in the master while that operation 
> > is pending: the volume is not allocated, but it is "used" in some sense.
> > 
> > One place where we use `Slave::usedResources` is when validating 
> > DESTROY operations: 
> > https://github.com/apache/mesos/blob/21e4b45b388d0b272236b1c58313569f8a1d1fc8/src/master/master.cpp#L4841
> > 
> > This means if the master receives a DESTROY call for a persistent 
> > volume while there is a pending operator-initiated GROW_VOLUME call for 
> > that same volume (i.e. the operation was forwarded to the agent for 
> > processing but the master hasn't heard back yet), we would accept the 
> > DESTROY operation and forward it to the agent. Is this what we want?
> > 
> > This brings something else to mind: would it be possible for the 
> > allocator to send out an offer containing the consumed resource of a 
> > GROW_VOLUME or SHRINK_VOLUME operation, while the operation is pending on 
> > the agent? This seems bad, but I believe it's possible. I think this may be 
> > the first time we have a set of resources which we don't want the allocator 
> > to offer for a period of time, but which are not currently allocated to 
> > some framework/role. I wonder if we need some new tools in the allocator to 
> > handle this?
> 
> Chun-Hung Hsiao wrote:
> Yeah this sounds bad. Would it help if we remove the consumed resources 
> from the allocator and put it back after receiving the terminal status update?

I agree this is problematic.

It seems like we need to make sure several things:
1. `consumed` resources of a non-speculative operation can not offered to any 
framework while pending;
2. `consumed` resources should still be visible for various validation;

Therefore, I'm thinking about introducing a new field in `Master::Slave` struct 
which represents `pendingConsumedResources` (or a function in `Slave` which can 
extract this info from pending operations). We can then use it to validation of 
`DESTROY` call.

Another issue I found was in `Master::apply()`, which we directly call 
`allocator->updateAvailable(slave->id, {operation})`, but semantically we 
should be calling `allocator->updateAvailable(slave->id, consumed, empty)` so 
that allocator would not see the `converted` resources until operation 
confirmations comes back.


- Zhitao


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


On March 26, 2018, 11:37 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66051/
> ---
> 
> (Updated March 26, 2018, 11:37 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented operator API to grow and shrink persistent volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
>   src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
>   src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
>   src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 
> 
> 
> Diff: https://reviews.apache.org/r/66051/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-03-27 Thread Chun-Hung Hsiao 


> On March 28, 2018, 1:58 a.m., Greg Mann wrote:
> > src/master/master.cpp
> > Lines 11881-11887 (original), 11885-11890 (patched)
> > 
> >
> > Currently, it looks like `Slave::usedResources` is the same as the 
> > "allocated resources" from that agent.
> > 
> > When a GROW or SHRINK operation is initiated by an operator, its 
> > consumed resource is in a unique state in the master while that operation 
> > is pending: the volume is not allocated, but it is "used" in some sense.
> > 
> > One place where we use `Slave::usedResources` is when validating 
> > DESTROY operations: 
> > https://github.com/apache/mesos/blob/21e4b45b388d0b272236b1c58313569f8a1d1fc8/src/master/master.cpp#L4841
> > 
> > This means if the master receives a DESTROY call for a persistent 
> > volume while there is a pending operator-initiated GROW_VOLUME call for 
> > that same volume (i.e. the operation was forwarded to the agent for 
> > processing but the master hasn't heard back yet), we would accept the 
> > DESTROY operation and forward it to the agent. Is this what we want?
> > 
> > This brings something else to mind: would it be possible for the 
> > allocator to send out an offer containing the consumed resource of a 
> > GROW_VOLUME or SHRINK_VOLUME operation, while the operation is pending on 
> > the agent? This seems bad, but I believe it's possible. I think this may be 
> > the first time we have a set of resources which we don't want the allocator 
> > to offer for a period of time, but which are not currently allocated to 
> > some framework/role. I wonder if we need some new tools in the allocator to 
> > handle this?

Yeah this sounds bad. Would it help if we remove the consumed resources from 
the allocator and put it back after receiving the terminal status update?


- Chun-Hung


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


On March 27, 2018, 6:37 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66051/
> ---
> 
> (Updated March 27, 2018, 6:37 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented operator API to grow and shrink persistent volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
>   src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
>   src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
>   src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 
> 
> 
> Diff: https://reviews.apache.org/r/66051/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-03-27 Thread Greg Mann 

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




src/master/master.cpp
Lines 11881-11887 (original), 11885-11890 (patched)


Currently, it looks like `Slave::usedResources` is the same as the 
"allocated resources" from that agent.

When a GROW or SHRINK operation is initiated by an operator, its consumed 
resource is in a unique state in the master while that operation is pending: 
the volume is not allocated, but it is "used" in some sense.

One place where we use `Slave::usedResources` is when validating DESTROY 
operations: 
https://github.com/apache/mesos/blob/21e4b45b388d0b272236b1c58313569f8a1d1fc8/src/master/master.cpp#L4841

This means if the master receives a DESTROY call for a persistent volume 
while there is a pending operator-initiated GROW_VOLUME call for that same 
volume (i.e. the operation was forwarded to the agent for processing but the 
master hasn't heard back yet), we would accept the DESTROY operation and 
forward it to the agent. Is this what we want?

This brings something else to mind: would it be possible for the allocator 
to send out an offer containing the consumed resource of a GROW_VOLUME or 
SHRINK_VOLUME operation, while the operation is pending on the agent? This 
seems bad, but I believe it's possible. I think this may be the first time we 
have a set of resources which we don't want the allocator to offer for a period 
of time, but which are not currently allocated to some framework/role. I wonder 
if we need some new tools in the allocator to handle this?


- Greg Mann


On March 27, 2018, 6:37 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66051/
> ---
> 
> (Updated March 27, 2018, 6:37 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented operator API to grow and shrink persistent volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
>   src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
>   src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
>   src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 
> 
> 
> Diff: https://reviews.apache.org/r/66051/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-03-27 Thread Greg Mann 

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




src/master/http.cpp
Lines 1525-1526 (patched)


Not indented far enough.



src/master/http.cpp
Lines 1526 (patched)


s/have/have the/



src/master/http.cpp
Lines 1544-1546 (patched)


Nit: you can probably remove this newline?



src/master/http.cpp
Lines 1556 (patched)


s/volume and addition/`volume` and `addition`/



src/master/http.cpp
Lines 1558-1561 (patched)


Not indented far enough.



src/master/http.cpp
Lines 1595-1596 (patched)


Not indented far enough.



src/master/http.cpp
Lines 1614-1616 (patched)


Nit: I would probably remove this newline.



src/master/http.cpp
Lines 1625 (patched)


Suggestion: "The `volume` field contains the resources required for this 
operation."



src/master/master.cpp
Lines 10798-10837 (original), 10794-10839 (patched)


As discussed in chat, we need to update the agent's resources in the 
allocator for operator-initiated operations.



src/master/validation.cpp
Lines 198-201 (patched)


Let's provide more specific feedback for these cases. Perhaps:

```
  if (!call.grow_volume().has_agent_id()) {
return Error("Expecting 'agent_id' to be present; only agent 
default resources are supported right now");
  }
  
  if (call.grow_volume().has_resource_provider_id()) {
return Error("Expecting 'resource_provider_id' to be unset; only 
agent default resources are supported right now");
  }
```


- Greg Mann


On March 27, 2018, 6:37 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66051/
> ---
> 
> (Updated March 27, 2018, 6:37 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented operator API to grow and shrink persistent volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
>   src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
>   src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
>   src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 
> 
> 
> Diff: https://reviews.apache.org/r/66051/diff/7/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-03-26 Thread Zhitao Li 

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

(Updated March 26, 2018, 11:37 p.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


Changes
---

Review comments from Chun.


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


Repository: mesos


Description
---

Implemented operator API to grow and shrink persistent volume.


Diffs (updated)
-

  src/master/http.cpp 34c9023906eca94965acc994f20e888c1f47b962 
  src/master/master.hpp 0d9620dd0c232dc1df83477e838eeb7313bf8828 
  src/master/master.cpp 20692c36c021fdae91de0b156f26fc56cf7c4f45 
  src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 


Diff: https://reviews.apache.org/r/66051/diff/7/

Changes: https://reviews.apache.org/r/66051/diff/6-7/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-03-26 Thread Zhitao Li 

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




src/master/http.cpp
Lines 1511-1513 (patched)


This is already checked in validation so I'll keep the `CHECK`. Will do for 
comment.


- Zhitao Li


On March 26, 2018, 9:54 a.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66051/
> ---
> 
> (Updated March 26, 2018, 9:54 a.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented operator API to grow and shrink persistent volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp f84089e6dcf366716b779f7eff227d1f96d7abda 
>   src/master/master.hpp 7e5d1df1ecb5a0db15782b2480fb7029f8a93ee2 
>   src/master/master.cpp 390fbe9fe71981101ed2bd9a1f1b21b0e3727b7b 
>   src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 
> 
> 
> Diff: https://reviews.apache.org/r/66051/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-03-26 Thread Chun-Hung Hsiao 

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




src/master/http.cpp
Lines 1511-1513 (patched)


"Only agent default resources are supported right now."

Also, probably return a `BadRequest` instead of triggering a check failure?

Ditto on `shrinkVolume`.



src/master/validation.cpp
Lines 198 (patched)


"Only agent default resources are supported"

Ditto below.


- Chun-Hung Hsiao


On March 26, 2018, 4:54 p.m., Zhitao Li wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66051/
> ---
> 
> (Updated March 26, 2018, 4:54 p.m.)
> 
> 
> Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.
> 
> 
> Bugs: MESOS-4965
> https://issues.apache.org/jira/browse/MESOS-4965
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> Implemented operator API to grow and shrink persistent volume.
> 
> 
> Diffs
> -
> 
>   src/master/http.cpp f84089e6dcf366716b779f7eff227d1f96d7abda 
>   src/master/master.hpp 7e5d1df1ecb5a0db15782b2480fb7029f8a93ee2 
>   src/master/master.cpp 390fbe9fe71981101ed2bd9a1f1b21b0e3727b7b 
>   src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 
> 
> 
> Diff: https://reviews.apache.org/r/66051/diff/6/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Zhitao Li
> 
>

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-03-26 Thread Zhitao Li 

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

(Updated March 26, 2018, 9:54 a.m.)


Review request for mesos, Chun-Hung Hsiao, Gaston Kleiman, and Greg Mann.


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


Repository: mesos


Description
---

Implemented operator API to grow and shrink persistent volume.


Diffs (updated)
-

  src/master/http.cpp f84089e6dcf366716b779f7eff227d1f96d7abda 
  src/master/master.hpp 7e5d1df1ecb5a0db15782b2480fb7029f8a93ee2 
  src/master/master.cpp 390fbe9fe71981101ed2bd9a1f1b21b0e3727b7b 
  src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 


Diff: https://reviews.apache.org/r/66051/diff/6/

Changes: https://reviews.apache.org/r/66051/diff/5-6/


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-03-20 Thread Mesos Reviewbot Windows 

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['66049', '66050', '66052', '66051']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66051

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66051/logs/mesos-tests-stdout.log):

```
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.UnsupportedContentMediaType/0 (4 
ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.UnsupportedContentMediaType/1
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.UnsupportedContentMediaType/1 (4 
ms)
[ RUN  ] ContentType/ResourceProviderManagerHttpApiTest.UpdateState/0
[   OK ] ContentType/ResourceProviderManagerHttpApiTest.UpdateState/0 (12 
ms)
[ RUN  ] ContentType/ResourceProviderManagerHttpApiTest.UpdateState/1
[   OK ] ContentType/ResourceProviderManagerHttpApiTest.UpdateState/1 (13 
ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.UpdateOperationStatus/0
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.UpdateOperationStatus/0 (9 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.UpdateOperationStatus/1
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.UpdateOperationStatus/1 (10 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesSuccess/0
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesSuccess/0 (17 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesSuccess/1
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesSuccess/1 (16 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesFailure/0
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesFailure/0 (10 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesFailure/1
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesFailure/1 (14 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesDisconnected/0
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesDisconnected/0 
(10 ms)
[ RUN  ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesDisconnected/1
[   OK ] 
ContentType/ResourceProviderManagerHttpApiTest.PublishResourcesDisconnected/1 
(13 ms)
[ RUN  ] ContentType/ResourceProviderManagerHttpApiTest.AgentEndpoint/0
[   OK ] ContentType/ResourceProviderManagerHttpApiTest.AgentEndpoint/0 
(177 ms)
[ RUN  ] ContentType/ResourceProviderManagerHttpApiTest.AgentEndpoint/1
[   OK ] ContentType/ResourceProviderManagerHttpApiTest.AgentEndpoint/1 
(180 ms)
[ RUN  ] ContentType/ResourceProviderManagerHttpApiTest.ConvertResources/0
D:\DCOS\mesos\mesos\src\tests\resource_provider_manager_tests.cpp(1038): error: 
block is NONE
```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66051/logs/mesos-tests-stderr.log):

```
@   7FF7069D7659  mesos::internal::slave::Slave::updateOperation
@   7FF7069D5A41  
mesos::internal::slave::Slave::handleResourceProviderMessage
@   7FF706B83197   ?? 
@   7FF706A71D08  
std::_Invoker_functor::_Call<,process::Future,process::ProcessBase
 * __ptr64>
@   7FF706AF6738  
std::invoke<,process::Future,process::ProcessBase
 * __ptr64>
@   7FF706B06EEB  
lambda::internal::Partial<,process::Future,std::_Ph<1>
 
>::invoke_expand<,std::tuple,process::Future,std::_Ph<1>
 >,process::ProcessBase * __ptr64>
@   7FF706AFCC6C  
std::invoke,process::Future,std::_Ph<1>
 >,process::ProcessBase * __ptr64>
@   7FF706A55221  
),process::Future,std::_Ph<1>
 >,process::ProcessBase * __ptr64
@   7FF706B91006  process::ProcessBase * 
__ptr64)>::CallableFn,process::Future,std::_Ph<1>
 > >::operator(
@   7FF7084845ED  process::ProcessBase * __ptr64)>::operator(
@   7FF70835F679  process::ProcessBase::consume
@   7FF7084D7EFA  process::DispatchEvent::consume
@   7FF7047F50B7  process::ProcessBase::serve
@   7FF70836CB90  process::ProcessManager::resume
@   7FF708474F91   ?? 
@   7FF7083B4420  
std::_Invoker_functor::_Call< >
@   7FF70840A480  std::invoke< 
>
@   7FF7083C2E3C  
std::_LaunchPad
 >,std::default_delete > > 
> >::_Execute<0>
@   7FF7084C01BA  
std::_LaunchPad
 >,std::default_delete > > 
> >::_Run
@   7FF7084ACC28  
std::_LaunchPad
 >,std::default_delete > > 
> >::_Go
@   7FF708494C5D  std::_Pad::_Call_func
@   7FFCCF7F11E8  _register_onexit_f

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-03-20 Thread Zhitao Li 

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

(Updated March 20, 2018, 6:33 p.m.)


Review request for mesos, Chun-Hung Hsiao and Gaston Kleiman.


Changes
---

Updated after API change.


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


Repository: mesos


Description
---

Implemented operator API to grow and shrink persistent volume.


Diffs (updated)
-

  src/master/http.cpp e0742f9e101fd3da1616640d38e6444a1d5b34aa 
  src/master/master.hpp a54b8fab0d80806063045362079fc266afec7ac2 
  src/master/master.cpp 78f16d685328cf203e0d684338a2d8df943c9292 
  src/master/validation.cpp 9254cd22e67d9aca53135701575fb0857ff6f7d6 


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

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


Testing
---


Thanks,

Zhitao Li

Re: Review Request 66051: Implemented operator API to grow and shrink persistent volume.

2018-03-13 Thread Mesos Reviewbot Windows 

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



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['66049', '66050', '66052', '66051']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: 
http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66051

Relevant logs:

- 
[mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66051/logs/mesos-tests-stdout.log):

```
[--] 9 tests from Endpoint/SlaveEndpointTest (1178 ms total)

[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/0 (39 
ms)
[ RUN  ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1
[   OK ] ContainerizerType/DefaultContainerDNSFlagTest.ValidateFlag/1 (44 
ms)
[--] 2 tests from ContainerizerType/DefaultContainerDNSFlagTest (85 ms 
total)

[--] 1 test from IsolationFlag/CpuIsolatorTest
[ RUN  ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0
[   OK ] IsolationFlag/CpuIsolatorTest.ROOT_UserCpuUsage/0 (2527 ms)
[--] 1 test from IsolationFlag/CpuIsolatorTest (2552 ms total)

[--] 1 test from IsolationFlag/MemoryIsolatorTest
[ RUN  ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0
[   OK ] IsolationFlag/MemoryIsolatorTest.ROOT_MemUsage/0 (2593 ms)
[--] 1 test from IsolationFlag/MemoryIsolatorTest (2618 ms total)

[--] Global test environment tear-down
[==] 916 tests from 91 test cases ran. (479165 ms total)
[  PASSED  ] 914 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] CommandExecutorCheckTest.CommandCheckTimeout
[  FAILED  ] SlaveTest.RestartSlaveRequireExecutorAuthentication

 2 FAILED TESTS
  YOU HAVE 210 DISABLED TESTS

```

- 
[mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/66051/logs/mesos-tests-stderr.log):

```
I0314 03:26:29.838824  8100 master.cpp:10380] Updating the state of task 
b83253aa-a02f-4b84-9b7a-172cf8cef559 of framework 
f033d295-0a24-4df2-ba84-a6caf5e462b1- (latest state: TASK_KILLED, status 
update state: TASK_KILLED)
I0314 03:26:29.838824  9448 slave.cpp:3878] Shutting down framework 
f033d295-0a24-4df2-ba84-a6caf5e462b1I0314 03:26:29.110826  7716 exec.cpp:162] 
Version: 1.6.0
I0314 03:26:29.140822  7680 exec.cpp:236] Executor registered on agent 
f033d295-0a24-4df2-ba84-a6caf5e462b1-S0
I0314 03:26:29.144822 10668 executor.cpp:176] Received SUBSCRIBED event
I0314 03:26:29.150826 10668 executor.cpp:180] Subscribed executor on 
build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net
I0314 03:26:29.150826 10668 executor.cpp:176] Received LAUNCH event
I0314 03:26:29.155833 10668 executor.cpp:648] Starting task 
b83253aa-a02f-4b84-9b7a-172cf8cef559
I0314 03:26:29.239827 10668 executor.cpp:483] Running 
'D:\DCOS\mesos\src\mesos-containerizer.exe launch '
I0314 03:26:29.801826 10668 executor.cpp:661] Forked command at 9432
I0314 03:26:29.842823  5456 exec.cpp:445] Executor asked to shutdown
I0314 03:26:29.843825 10668 executor.cpp:176] Received SHUTDOWN event
I0314 03:26:29.843825 10668 executor.cpp:758] Shutting down
I0314 03:26:29.844822 10668 executor.cpp:868] Sending SIGTERM to process tree 
at pid 9-
I0314 03:26:29.839823  9448 slave.cpp:6571] Shutting down executor 
'b83253aa-a02f-4b84-9b7a-172cf8cef559' of framework 
f033d295-0a24-4df2-ba84-a6caf5e462b1- at executor(1)@10.3.1.11:65161
I0314 03:26:29.841823  9448 slave.cpp:924] Agent terminating
I0314 03:26:29.841823  8100 master.cpp:10479] Removing task 
b83253aa-a02f-4b84-9b7a-172cf8cef559 with resources cpus(allocated: *):4; 
mem(allocated: *):2048; disk(allocated: *):1024; ports(allocated: 
*):[31000-32000] of framework f033d295-0a24-4df2-ba84-a6caf5e462b1- on 
agent f033d295-0a24-4df2-ba84-a6caf5e462b1-S0 at slave(398)@10.3.1.11:65140 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
W0314 03:26:29.841823  9448 slave.cpp:3874] Ignoring shutdown framework 
f033d295-0a24-4df2-ba84-a6caf5e462b1- because it is terminating
I0314 03:26:29.844822  8100 master.cpp:1288] Agent 
f033d295-0a24-4df2-ba84-a6caf5e462b1-S0 at slave(398)@10.3.1.11:65140 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net) disconnected
I0314 03:26:29.844822  8100 master.cpp:3258] Disconnecting agent 
f033d295-0a24-4df2-ba84-a6caf5e462b1-S0 at slave(398)@10.3.1.11:65140 
(build-srv-03.zq4gs31qjdiunm1ryi1452nvnh.dx.internal.cloudapp.net)
I0314 03:26:29.845823  3464 hierarchical.cpp:344] Removed framework 
f033d295-0a24-4df2-ba84-a6caf5e462b1-
I0314 03:26:29.845823  7644 containerizer.cpp:2338] Destroying container 
e7c5bd6e-37c7-46c1-a7e4-b19ccd0156fd