You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Anindya Sinha <an...@apple.com> on 2017/01/09 22:37:05 UTC
Re: Review Request 53096: Fix handling in shared count in total
resources in the sorter.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53096/
-----------------------------------------------------------
(Updated Jan. 9, 2017, 10:37 p.m.)
Review request for mesos and Jiang Yan Xu.
Changes
-------
Rebase.
Bugs: MESOS-6444
https://issues.apache.org/jira/browse/MESOS-6444
Repository: mesos
Description
-------
We maintain a single copy of shared resource in the role and quota
sorter's total resources. So, when we update these resources, we
remove the previous resources at this agent and add the new resources
at this agent.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.hpp a6424d624864155e1c87a28a63b784512c5c8722
src/master/allocator/mesos/hierarchical.cpp 91b1ec43940a788459f045ca4a4b82d4e8373bca
src/tests/persistent_volume_tests.cpp 8198b6b5ad323d17835ba067c7ff3d34ef948125
Diff: https://reviews.apache.org/r/53096/diff/
Testing
-------
Tests passed.
Thanks,
Anindya Sinha
Re: Review Request 53096: Fix handling in shared count in total
resources in the sorter.
Posted by Jiang Yan Xu <xu...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53096/#review164180
-----------------------------------------------------------
src/master/allocator/mesos/hierarchical.cpp (line 2056)
<https://reviews.apache.org/r/53096/#comment235803>
Did
s/slaves[slaveId].total/slave.total/
when committing.
- Jiang Yan Xu
On Feb. 3, 2017, 2:01 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53096/
> -----------------------------------------------------------
>
> (Updated Feb. 3, 2017, 2:01 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-6444
> https://issues.apache.org/jira/browse/MESOS-6444
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We maintain a single copy of shared resource in the role and quota
> sorter's total resources. So, when we update these resources, we
> remove the previous resources at this agent and add the new resources
> at this agent.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp a99ed35169c0a4a1db3ac3b9b09f510f8fcddbfb
> src/master/allocator/mesos/hierarchical.cpp 5f540569043df9d2bb75416c8c36bb4dd7bd68a1
> src/tests/persistent_volume_tests.cpp 468a85b4a6ce09592341afd07ce12a03f5fc4f73
>
> Diff: https://reviews.apache.org/r/53096/diff/
>
>
> Testing
> -------
>
> Tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>
Re: Review Request 53096: Fix handling in shared count in total
resources in the sorter.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53096/
-----------------------------------------------------------
(Updated Feb. 3, 2017, 10:01 p.m.)
Review request for mesos and Jiang Yan Xu.
Changes
-------
Refactor.
Bugs: MESOS-6444
https://issues.apache.org/jira/browse/MESOS-6444
Repository: mesos
Description
-------
We maintain a single copy of shared resource in the role and quota
sorter's total resources. So, when we update these resources, we
remove the previous resources at this agent and add the new resources
at this agent.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.hpp a99ed35169c0a4a1db3ac3b9b09f510f8fcddbfb
src/master/allocator/mesos/hierarchical.cpp 5f540569043df9d2bb75416c8c36bb4dd7bd68a1
src/tests/persistent_volume_tests.cpp 468a85b4a6ce09592341afd07ce12a03f5fc4f73
Diff: https://reviews.apache.org/r/53096/diff/
Testing
-------
Tests passed.
Thanks,
Anindya Sinha
Re: Review Request 53096: Fix handling in shared count in total
resources in the sorter.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53096/
-----------------------------------------------------------
(Updated Feb. 1, 2017, 10:45 p.m.)
Review request for mesos and Jiang Yan Xu.
Changes
-------
Rebased after batch allocation patch was merged.
Bugs: MESOS-6444
https://issues.apache.org/jira/browse/MESOS-6444
Repository: mesos
Description
-------
We maintain a single copy of shared resource in the role and quota
sorter's total resources. So, when we update these resources, we
remove the previous resources at this agent and add the new resources
at this agent.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.hpp 339b3d2bada526fd66d812df30e9615a96883769
src/master/allocator/mesos/hierarchical.cpp ffa07087f9ed8d28b99cc4cde7c739cfd7edb1e1
src/tests/persistent_volume_tests.cpp 468a85b4a6ce09592341afd07ce12a03f5fc4f73
Diff: https://reviews.apache.org/r/53096/diff/
Testing
-------
Tests passed.
Thanks,
Anindya Sinha
Re: Review Request 53096: Fix handling in shared count in total
resources in the sorter.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53096/
-----------------------------------------------------------
(Updated Jan. 31, 2017, 5:58 a.m.)
Review request for mesos and Jiang Yan Xu.
Changes
-------
Rebased.
Bugs: MESOS-6444
https://issues.apache.org/jira/browse/MESOS-6444
Repository: mesos
Description
-------
We maintain a single copy of shared resource in the role and quota
sorter's total resources. So, when we update these resources, we
remove the previous resources at this agent and add the new resources
at this agent.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.hpp 2cda3e311ce339d82065d68de83e86439fa192ff
src/master/allocator/mesos/hierarchical.cpp f471b6848bebae601a7a0509e9c6ad5eab4fa4a2
src/tests/persistent_volume_tests.cpp 468a85b4a6ce09592341afd07ce12a03f5fc4f73
Diff: https://reviews.apache.org/r/53096/diff/
Testing
-------
Tests passed.
Thanks,
Anindya Sinha
Re: Review Request 53096: Fix handling in shared count in total
resources in the sorter.
Posted by Anindya Sinha <an...@apple.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53096/
-----------------------------------------------------------
(Updated Jan. 26, 2017, 10:06 p.m.)
Review request for mesos and Jiang Yan Xu.
Changes
-------
Addressed review comments.
Bugs: MESOS-6444
https://issues.apache.org/jira/browse/MESOS-6444
Repository: mesos
Description
-------
We maintain a single copy of shared resource in the role and quota
sorter's total resources. So, when we update these resources, we
remove the previous resources at this agent and add the new resources
at this agent.
Diffs (updated)
-----
src/master/allocator/mesos/hierarchical.hpp 9b66c23f26b37c02ed850c06c4518ea99078b02d
src/master/allocator/mesos/hierarchical.cpp c2211be7458755aeb91ef078e4bfe92ac474044a
src/tests/persistent_volume_tests.cpp 468a85b4a6ce09592341afd07ce12a03f5fc4f73
Diff: https://reviews.apache.org/r/53096/diff/
Testing
-------
Tests passed.
Thanks,
Anindya Sinha
Re: Review Request 53096: Fix handling in shared count in total
resources in the sorter.
Posted by Jiang Yan Xu <ya...@jxu.me>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/53096/#review162890
-----------------------------------------------------------
Fix it, then Ship it!
This review is now closely related to the followup review /r/55359/ which almost makes sense to combine them. But because this patch fixes a bug (with tests to prove that) I guess we'll still keep them separate. Just thought I'd mention it for posterity.
src/master/allocator/mesos/hierarchical.hpp (line 479)
<https://reviews.apache.org/r/53096/#comment234272>
As mentioned in the reply from the previous conversation:
s/updateTotal/updateSlaveTotal/
And the comments:
```
// Helper to update the agent's total resources maintained in the allocator and the role
// and quota sorters (whose total resources match the agent's total resources).
```
src/master/allocator/mesos/hierarchical.cpp (line 720)
<https://reviews.apache.org/r/53096/#comment234480>
s/total/agent's total/
src/master/allocator/mesos/hierarchical.cpp (line 754)
<https://reviews.apache.org/r/53096/#comment234481>
s/allocated resources/allocation/
It's synonymous but `allocation` is more consistent with the comments above (now that we don't need to say "total and allocated resources").
Same applies to another occurrence below.
src/master/allocator/mesos/hierarchical.cpp (line 1977)
<https://reviews.apache.org/r/53096/#comment234492>
Can we also add the following sentense before "So, we update them using the resources in `slaves[slaveId].total`." to explain it more thoroughly:
```
(which don't get updated in the allocation runs or during recovery of allocated resources)
```
src/tests/persistent_volume_tests.cpp (lines 1443 - 1447)
<https://reviews.apache.org/r/53096/#comment234495>
"We kill the long-lived task": this duplicates the same "We kill the long-lived task" below. I guess you mentioned them in two places because they are far apart but can we move
```
Future<TaskStatus> status4;
EXPECT_CALL(sched, statusUpdate(&driver, _))
.WillOnce(FutureArg<1>(&status4));
```
down to right before we sent killTask so we just need to comment on it once?
Plus, the block of code below it right now is all about killing the `nonSharedVolume` which is not relevant.
- Jiang Yan Xu
On Jan. 9, 2017, 2:37 p.m., Anindya Sinha wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/53096/
> -----------------------------------------------------------
>
> (Updated Jan. 9, 2017, 2:37 p.m.)
>
>
> Review request for mesos and Jiang Yan Xu.
>
>
> Bugs: MESOS-6444
> https://issues.apache.org/jira/browse/MESOS-6444
>
>
> Repository: mesos
>
>
> Description
> -------
>
> We maintain a single copy of shared resource in the role and quota
> sorter's total resources. So, when we update these resources, we
> remove the previous resources at this agent and add the new resources
> at this agent.
>
>
> Diffs
> -----
>
> src/master/allocator/mesos/hierarchical.hpp a6424d624864155e1c87a28a63b784512c5c8722
> src/master/allocator/mesos/hierarchical.cpp 91b1ec43940a788459f045ca4a4b82d4e8373bca
> src/tests/persistent_volume_tests.cpp 8198b6b5ad323d17835ba067c7ff3d34ef948125
>
> Diff: https://reviews.apache.org/r/53096/diff/
>
>
> Testing
> -------
>
> Tests passed.
>
>
> Thanks,
>
> Anindya Sinha
>
>