You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Jiang Yan Xu <ya...@jxu.me> on 2017/05/01 21:29:06 UTC
Review Request 58587: Clarified comments about resource operations
through operator API.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58587/
-----------------------------------------------------------
Review request for mesos, Anindya Sinha and Michael Park.
Repository: mesos
Description
-------
Seems like the comments about "virtually always win the race against
'allocate'" could be made more precise in helping people understand
how it works.
Diffs
-----
src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8
Diff: https://reviews.apache.org/r/58587/diff/1/
Testing
-------
N/A
Thanks,
Jiang Yan Xu
Re: Review Request 58587: Clarified comments about resource operations
through operator API.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58587/#review174854
-----------------------------------------------------------
Ship it!
Ship It!
- Michael Park
On May 9, 2017, 3:29 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58587/
> -----------------------------------------------------------
>
> (Updated May 9, 2017, 3:29 p.m.)
>
>
> Review request for mesos, Anindya Sinha and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Seems like the comments about "virtually always win the race against
> 'allocate'" could be made more precise in helping people understand
> how it works.
>
>
> Diffs
> -----
>
> src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31
>
>
> Diff: https://reviews.apache.org/r/58587/diff/2/
>
>
> Testing
> -------
>
> N/A
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 58587: Clarified comments about resource operations
through operator API.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58587/#review174448
-----------------------------------------------------------
Patch looks great!
Reviews applied: [58587]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On May 9, 2017, 10:29 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58587/
> -----------------------------------------------------------
>
> (Updated May 9, 2017, 10:29 p.m.)
>
>
> Review request for mesos, Anindya Sinha and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Seems like the comments about "virtually always win the race against
> 'allocate'" could be made more precise in helping people understand
> how it works.
>
>
> Diffs
> -----
>
> src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31
>
>
> Diff: https://reviews.apache.org/r/58587/diff/2/
>
>
> Testing
> -------
>
> N/A
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 58587: Clarified comments about resource operations
through operator API.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58587/
-----------------------------------------------------------
(Updated May 9, 2017, 3:29 p.m.)
Review request for mesos, Anindya Sinha and Michael Park.
Changes
-------
Comments.
Repository: mesos
Description
-------
Seems like the comments about "virtually always win the race against
'allocate'" could be made more precise in helping people understand
how it works.
Diffs (updated)
-----
src/master/http.cpp e2590a17044ac019b24a24629428d4ec8adc0c31
Diff: https://reviews.apache.org/r/58587/diff/2/
Changes: https://reviews.apache.org/r/58587/diff/1-2/
Testing
-------
N/A
Thanks,
Jiang Yan Xu
Re: Review Request 58587: Clarified comments about resource operations
through operator API.
Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58587/#review173706
-----------------------------------------------------------
Patch looks great!
Reviews applied: [58587]
Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh
- Mesos Reviewbot
On May 1, 2017, 9:29 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58587/
> -----------------------------------------------------------
>
> (Updated May 1, 2017, 9:29 p.m.)
>
>
> Review request for mesos, Anindya Sinha and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Seems like the comments about "virtually always win the race against
> 'allocate'" could be made more precise in helping people understand
> how it works.
>
>
> Diffs
> -----
>
> src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8
>
>
> Diff: https://reviews.apache.org/r/58587/diff/1/
>
>
> Testing
> -------
>
> N/A
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 58587: Clarified comments about resource operations
through operator API.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On May 2, 2017, 12:13 a.m., Michael Park wrote:
> > src/master/http.cpp
> > Lines 4870-4872 (patched)
> > <https://reviews.apache.org/r/58587/diff/1/?file=1704585#file1704585line4870>
> >
> > Thanks for adding this! This is the issue that I was considering filing a JIRA ticket for.
Do you have some ideas on how to improve this? I guess a JIRA would be good even with this NOTE.
> On May 2, 2017, 12:13 a.m., Michael Park wrote:
> > src/master/http.cpp
> > Lines 4884-4889 (patched)
> > <https://reviews.apache.org/r/58587/diff/1/?file=1704585#file1704585line4884>
> >
> > I'm not quite following this example. Specifically, I don't get what "a persistent volume can be destroyed if it is not allocated" means..
Ok I guess I shoud say "a persistent volume can be destroyed only if it is not used". In this case, it's not going to be pending offers so you would unncessarily rescind all offers.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58587/#review173561
-----------------------------------------------------------
On May 1, 2017, 2:29 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58587/
> -----------------------------------------------------------
>
> (Updated May 1, 2017, 2:29 p.m.)
>
>
> Review request for mesos, Anindya Sinha and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Seems like the comments about "virtually always win the race against
> 'allocate'" could be made more precise in helping people understand
> how it works.
>
>
> Diffs
> -----
>
> src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8
>
>
> Diff: https://reviews.apache.org/r/58587/diff/1/
>
>
> Testing
> -------
>
> N/A
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 58587: Clarified comments about resource operations
through operator API.
Posted by Michael Park <mp...@apache.org>.
> On May 2, 2017, 12:13 a.m., Michael Park wrote:
> > src/master/http.cpp
> > Lines 4884-4889 (patched)
> > <https://reviews.apache.org/r/58587/diff/1/?file=1704585#file1704585line4884>
> >
> > I'm not quite following this example. Specifically, I don't get what "a persistent volume can be destroyed if it is not allocated" means..
>
> Jiang Yan Xu wrote:
> Ok I guess I shoud say "a persistent volume can be destroyed only if it is not used". In this case, it's not going to be pending offers so you would unncessarily rescind all offers.
Hm... it seems like this should be mitigated by
```cpp
// If rescinding the offer would not contribute to satisfying
// the required resources, skip it.
Resources recovered = offer->resources();
recovered.unallocate();
if (required == required - recovered) {
continue;
}
```
Is this not true?
- Michael
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58587/#review173561
-----------------------------------------------------------
On May 1, 2017, 2:29 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58587/
> -----------------------------------------------------------
>
> (Updated May 1, 2017, 2:29 p.m.)
>
>
> Review request for mesos, Anindya Sinha and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Seems like the comments about "virtually always win the race against
> 'allocate'" could be made more precise in helping people understand
> how it works.
>
>
> Diffs
> -----
>
> src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8
>
>
> Diff: https://reviews.apache.org/r/58587/diff/1/
>
>
> Testing
> -------
>
> N/A
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 58587: Clarified comments about resource operations
through operator API.
Posted by Jiang Yan Xu <ya...@jxu.me>.
> On May 2, 2017, 12:13 a.m., Michael Park wrote:
> > src/master/http.cpp
> > Lines 4884-4889 (patched)
> > <https://reviews.apache.org/r/58587/diff/1/?file=1704585#file1704585line4884>
> >
> > I'm not quite following this example. Specifically, I don't get what "a persistent volume can be destroyed if it is not allocated" means..
>
> Jiang Yan Xu wrote:
> Ok I guess I shoud say "a persistent volume can be destroyed only if it is not used". In this case, it's not going to be pending offers so you would unncessarily rescind all offers.
>
> Michael Park wrote:
> Hm... it seems like this should be mitigated by
> ```cpp
> // If rescinding the offer would not contribute to satisfying
> // the required resources, skip it.
> Resources recovered = offer->resources();
> recovered.unallocate();
>
> if (required == required - recovered) {
> continue;
> }
> ```
> Is this not true?
Hm you are right. I'll drop this comment and leave just the one above. I'll send another rr for caveats about shared persistent volumes, which is different.
- Jiang Yan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58587/#review173561
-----------------------------------------------------------
On May 1, 2017, 2:29 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58587/
> -----------------------------------------------------------
>
> (Updated May 1, 2017, 2:29 p.m.)
>
>
> Review request for mesos, Anindya Sinha and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Seems like the comments about "virtually always win the race against
> 'allocate'" could be made more precise in helping people understand
> how it works.
>
>
> Diffs
> -----
>
> src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8
>
>
> Diff: https://reviews.apache.org/r/58587/diff/1/
>
>
> Testing
> -------
>
> N/A
>
>
> Thanks,
>
> Jiang Yan Xu
>
>
Re: Review Request 58587: Clarified comments about resource operations
through operator API.
Posted by Michael Park <mp...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/58587/#review173561
-----------------------------------------------------------
src/master/http.cpp
Lines 4870-4872 (patched)
<https://reviews.apache.org/r/58587/#comment246558>
Thanks for adding this! This is the issue that I was considering filing a JIRA ticket for.
src/master/http.cpp
Lines 4884-4889 (patched)
<https://reviews.apache.org/r/58587/#comment246557>
I'm not quite following this example. Specifically, I don't get what "a persistent volume can be destroyed if it is not allocated" means..
- Michael Park
On May 1, 2017, 2:29 p.m., Jiang Yan Xu wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58587/
> -----------------------------------------------------------
>
> (Updated May 1, 2017, 2:29 p.m.)
>
>
> Review request for mesos, Anindya Sinha and Michael Park.
>
>
> Repository: mesos
>
>
> Description
> -------
>
> Seems like the comments about "virtually always win the race against
> 'allocate'" could be made more precise in helping people understand
> how it works.
>
>
> Diffs
> -----
>
> src/master/http.cpp 5aae52870451d883ef1ea1fda5a27764d7f318e8
>
>
> Diff: https://reviews.apache.org/r/58587/diff/1/
>
>
> Testing
> -------
>
> N/A
>
>
> Thanks,
>
> Jiang Yan Xu
>
>