Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription.

[Klaus Ma]

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

(Updated March 11, 2016, 2:07 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Joris Van 
Remoortere, Joseph Wu, and Jian Qiu.


Changes
---

rebase


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


Repository: mesos


Description (updated)
---

In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps to 
update `RevocableInfo::type` for Oversubscription.


Diffs (updated)
-

  include/mesos/slave/resource_estimator.hpp 
27612cabf96a62f7007a8f1480c3966672862d02 
  src/slave/slave.cpp 840534ff0687e82ed063c386e36bbabada230697 

Diff: https://reviews.apache.org/r/40379/diff/


Testing
---

make (make check is on-going)


Thanks,

Klaus Ma

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription.

[Klaus Ma]

> On Jan. 20, 2016, 8:25 a.m., Joseph Wu wrote:
> > Can you talk to Guangya and merge this patch with 
> > https://reviews.apache.org/r/42123/ ?  Logically, they're touching the same 
> > things.

Sure, we'll sync up to merge them.


- Klaus


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


On Jan. 7, 2016, 8:04 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated Jan. 7, 2016, 8:04 p.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Joris 
> Van Remoortere, Joseph Wu, and Jian Qiu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/resource_estimator.hpp 27612ca 
>   src/slave/slave.cpp 9d80c96 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription.

[Joseph Wu]

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


Can you talk to Guangya and merge this patch with 
https://reviews.apache.org/r/42123/ ?  Logically, they're touching the same 
things.


src/slave/slave.cpp (line 4632)


It would be better to filter than to substract.

i.e.
oversubscribed = oversubscribed.usageSlack();



src/slave/slave.cpp (lines 4637 - 4638)


After the above change (and some changes to the helpers I suggested in 
earlier reviews), you can do:

```
oversubscribed += oversubscribable;

Resource::RevocableInfo revocableInfo;
revocableInfo.set_type(Resource::RevocableInfo::USAGE_SLACK);

oversubscribed = oversubscribed.flatten(revocableInfo);
```


- Joseph Wu


On Jan. 7, 2016, 4:04 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated Jan. 7, 2016, 4:04 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Joris 
> Van Remoortere, Joseph Wu, and Jian Qiu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/resource_estimator.hpp 27612ca 
>   src/slave/slave.cpp 9d80c96 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription.

[Klaus Ma]

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

(Updated Jan. 7, 2016, 8:04 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Joris Van 
Remoortere, Joseph Wu, and Jian Qiu.


Changes
---

Add period to the summary


Summary (updated)
-

MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription.


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


Repository: mesos


Description
---

In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps to 
update `RevocableInfo::type` for Oversubscription.


Diffs
-

  include/mesos/slave/resource_estimator.hpp 27612ca 
  src/slave/slave.cpp 9d80c96 

Diff: https://reviews.apache.org/r/40379/diff/


Testing
---

make (make check is on-going)


Thanks,

Klaus Ma

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Klaus Ma]

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

(Updated Jan. 6, 2016, 11:30 p.m.)


Review request for mesos, Ben Mahler, Guangya Liu, Artem Harutyunyan, Joris Van 
Remoortere, Joseph Wu, and Jian Qiu.


Changes
---

Update dependency


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


Repository: mesos


Description
---

In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps to 
update `RevocableInfo::type` for Oversubscription.


Diffs
-

  include/mesos/slave/resource_estimator.hpp 27612ca 
  src/slave/slave.cpp 9d80c96 

Diff: https://reviews.apache.org/r/40379/diff/


Testing
---

make (make check is on-going)


Thanks,

Klaus Ma

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Guangya Liu]

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



src/slave/slave.cpp (lines 4631 - 4632)


I think it should be:

// Exclude oversubscription for reservation.
oversubscribed -= oversubscribed.allocationSlack();
oversubscribed += oversubscribable.get();

Seems you lost the new reported oversubscribed resources here.

To be more clear, what about update 4627 as:
oversubscribed += executor->resources.revocable().allocationSlack();


- Guangya Liu


On Jan. 2, 2016, 1 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated Jan. 2, 2016, 1 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/resource_estimator.hpp 27612ca 
>   src/slave/slave.cpp 9d80c96 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Klaus Ma]

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

(Updated Jan. 2, 2016, 9 p.m.)


Review request for mesos, Guangya Liu, Artem Harutyunyan, Joris Van Remoortere, 
and Joseph Wu.


Changes
---

Update code diff


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


Repository: mesos


Description
---

In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps to 
update `RevocableInfo::type` for Oversubscription.


Diffs (updated)
-

  include/mesos/slave/resource_estimator.hpp 27612ca 
  src/slave/slave.cpp 9d80c96 

Diff: https://reviews.apache.org/r/40379/diff/


Testing
---

make (make check is on-going)


Thanks,

Klaus Ma

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40375, 40379]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 12, 2015, 10:12 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated Dec. 12, 2015, 10:12 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/resource_estimator.hpp 27612ca 
>   src/slave/slave.cpp 9bd86e1 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Klaus Ma]

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

(Updated Dec. 12, 2015, 6:12 p.m.)


Review request for mesos, Guangya Liu, Artem Harutyunyan, Joris Van Remoortere, 
and Joseph Wu.


Changes
---

Address comments


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


Repository: mesos


Description
---

In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps to 
update `RevocableInfo::type` for Oversubscription.


Diffs (updated)
-

  include/mesos/slave/resource_estimator.hpp 27612ca 
  src/slave/slave.cpp 9bd86e1 

Diff: https://reviews.apache.org/r/40379/diff/


Testing
---

make (make check is on-going)


Thanks,

Klaus Ma

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Joseph Wu]

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



include/mesos/slave/resource_estimator.hpp (lines 61 - 63)


Suggestion:

NOTE: For backwards compatibility, the agent will set the `type` of 
`RevocableInfo` for these resources to `USAGE_SLACK`.  This is the only type of 
revocable resources handled by the estimator.



src/slave/slave.cpp (line 4461)


s/overwrote/overwritten/


- Joseph Wu


On Dec. 3, 2015, 9:17 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated Dec. 3, 2015, 9:17 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/resource_estimator.hpp 27612ca 
>   src/slave/slave.cpp 777f36b 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40375, 40379]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 4, 2015, 5:17 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated Dec. 4, 2015, 5:17 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   include/mesos/slave/resource_estimator.hpp 27612ca 
>   src/slave/slave.cpp 777f36b 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Mesos ReviewBot]

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


Patch looks great!

Reviews applied: [40375, 40379]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export 
COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Nov. 25, 2015, 6:41 a.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated Nov. 25, 2015, 6:41 a.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 9055f2a 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Klaus Ma]

> On Dec. 1, 2015, 10:10 a.m., Joseph Wu wrote:
> > src/slave/slave.cpp, lines 4464-4466
> > 
> >
> > Can you add a comment about why this is purposefully and unilaterally 
> > done here (potentially overwriting what the ResourceEstimator has given 
> > you)?
> > 
> > It may also be a good idea to update the documentation in 
> > "include/mesos/slave/resource_estimator.hpp".

Sure, I'll add some comments for it, for the document, can we address them in 
MESOS-3889?
There're two reason to add this code:
1. I'd like to `set_type()` accordingly, so we can make `RevocalbeInfo::type` 
to be `required`
2. I'd like `ResourceEstimator` return the right type


- Klaus


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


On Nov. 25, 2015, 2:41 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated Nov. 25, 2015, 2:41 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 9055f2a 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Joseph Wu]

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



src/slave/slave.cpp (lines 4464 - 4466)


Can you add a comment about why this is purposefully and unilaterally done 
here (potentially overwriting what the ResourceEstimator has given you)?

It may also be a good idea to update the documentation in 
"include/mesos/slave/resource_estimator.hpp".


- Joseph Wu


On Nov. 24, 2015, 10:41 p.m., Klaus Ma wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40379/
> ---
> 
> (Updated Nov. 24, 2015, 10:41 p.m.)
> 
> 
> Review request for mesos, Guangya Liu, Artem Harutyunyan, Joris Van 
> Remoortere, and Joseph Wu.
> 
> 
> Bugs: MESOS-3930
> https://issues.apache.org/jira/browse/MESOS-3930
> 
> 
> Repository: mesos
> 
> 
> Description
> ---
> 
> In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
> for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps 
> to update `RevocableInfo::type` for Oversubscription.
> 
> 
> Diffs
> -
> 
>   src/slave/slave.cpp 9055f2a 
> 
> Diff: https://reviews.apache.org/r/40379/diff/
> 
> 
> Testing
> ---
> 
> make (make check is on-going)
> 
> 
> Thanks,
> 
> Klaus Ma
> 
>

Re: Review Request 40379: MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription

[Klaus Ma]

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

(Updated Nov. 25, 2015, 2:35 p.m.)


Review request for mesos and Guangya Liu.


Changes
---

Mark as ready for review, one open discussion is here: whether this path is 
necessary?
1. Unnecessary because Resources keep the compatibility of unset 
RevocableInfo::type & USAGE_SLACK
2. Necessary because we'd like to introduce USAGE_SLACK & ALLOCATION_SLACK, and 
deprecate unset RevocableInfo::type


Summary (updated)
-

MESOS-3930: Set resource type as USAGE_SLACK for Oversubscription


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


Repository: mesos


Description
---

In Optimistic Offer Phase 1, it introduce `RevocableInfo::type`: USAGE_SLACK 
for Oversubscription and ALLOCATION_SLACK for Optimistic Offer. Slave helps to 
update `RevocableInfo::type` for Oversubscription.


Diffs
-

  src/slave/slave.cpp 9055f2a 

Diff: https://reviews.apache.org/r/40379/diff/


Testing
---

make (make check is on-going)


Thanks,

Klaus Ma