mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anindya Sinha <anindya_si...@apple.com>
Subject Re: Review Request 45961: Support sharing of resources through reference counting of resources.
Date Thu, 28 Jul 2016 22:44:04 GMT


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp, lines 817-822
> > <https://reviews.apache.org/r/45961/diff/10/?file=1447698#file1447698line817>
> >
> >     As commented below, I realized that we may not need this. We shouldn't calculate
total task resources as it's done in `Master::addTask()`.

Removed this method.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp, line 331
> > <https://reviews.apache.org/r/45961/diff/10/?file=1447698#file1447698line331>
> >
> >     s/assigned/requested/ (assigned is a bit ambiguous, at this point resources
are still not given to the task yet).

Removed `slave->pendingResources` and replaced with `slave->pendingTasks`.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp, line 340
> > <https://reviews.apache.org/r/45961/diff/10/?file=1447698#file1447698line340>
> >
> >     s/accept/ACCEPT/

Removed `slave->pendingResources` and replaced with `slave->pendingTasks`.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp, line 341
> > <https://reviews.apache.org/r/45961/diff/10/?file=1447698#file1447698line341>
> >
> >     s/removing it/removing them/

Removed `slave->pendingResources` and replaced with `slave->pendingTasks`.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp, line 344
> > <https://reviews.apache.org/r/45961/diff/10/?file=1447698#file1447698line344>
> >
> >     With `hashmap<FrameworkID, Resources> pendingResources` we are preventing
DESTROY from affecting pending tasks of the same framework but letting DESTROY to go through
under pending tasks of other frameworks (of the same role).
> >     
> >     If we use `pendingResources` it should be across frameworks right?
> >     
> >     But because we already need to add
> >     
> >     ```
> >     multihashmap<FrameworkID, TaskID> pendingTasks;
> >     ```
> >     
> >     for /r/47082/,
> >     
> >     we can just do that instead in this review, the number of tasks pending for
each slave should be small enough to loop through quickly.
> >     
> >     Overall this should be more straightfoward than maintaining `pendingResources`.

Removed `slave->pendingResources` and replaced with `slave->pendingTasks`.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.hpp, lines 2501-2502
> > <https://reviews.apache.org/r/45961/diff/10/?file=1447698#file1447698line2501>
> >
> >     Prbably can't store them here as we don't aggregate identity-based resources
across agents.
> >     
> >     Additionally it looks like we don't need them per comments below.

Removed these.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/master.cpp, lines 3672-3673
> > <https://reviews.apache.org/r/45961/diff/10/?file=1447699#file1447699line3672>
> >
> >     Shouldn't need to keep track of used shared resources.

Not needed any more.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/validation.hpp, line 158
> > <https://reviews.apache.org/r/45961/diff/10/?file=1447701#file1447701line158>
> >
> >     This will not take pendingTasks on the agent as the last argument.

I think you meant s/not/now. Yes.


> On July 26, 2016, 4:06 p.m., Jiang Yan Xu wrote:
> > src/master/allocator/mesos/hierarchical.cpp, lines 604-607
> > <https://reviews.apache.org/r/45961/diff/10/?file=1447694#file1447694line604>
> >
> >     Now we have to modify this method to process the LAUNCH call and adjust the
API docs accordingly.

As discussed, we are not going to comment in the base class regarding using `Offer::Operation::LAUNCH`
in `updateAllocation()` since it is specific to our implementation. Appropriate comments added
in `hierarchical.cpp`.


- Anindya


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


On July 19, 2016, 10:52 p.m., Anindya Sinha wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45961/
> -----------------------------------------------------------
> 
> (Updated July 19, 2016, 10:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler, Joris Van Remoortere, Michael Park, and Jiang
Yan Xu.
> 
> 
> Bugs: MESOS-4431
>     https://issues.apache.org/jira/browse/MESOS-4431
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> o Each shared resource is accouted via its share count. This count is
>   updated based on the resource operations (such as add and subtract)
>   in various scenarios such as task launch and terminate at multiple
>   modules such as master, allocator, sorter, etc.
> o Only allow DESTROY if there are no running or pending tasks using
>   the volume. However, if the volume is in a pending offer to one or
>   more frameworks, resind that offer before processing the DESTROY.
> 
> 
> Diffs
> -----
> 
>   src/common/resources.cpp b1bd2784aefdebf91892638b40b215373b998574 
>   src/common/resources_utils.cpp 8e881a09500b0966b577e514077460f724868a8d 
>   src/master/allocator/mesos/hierarchical.hpp b72ba16277a3210e4d309b616d185a10e2029a66

>   src/master/allocator/mesos/hierarchical.cpp 7d4064535a20b93950f5a95eef1ad3f0d37d305b

>   src/master/allocator/sorter/drf/sorter.hpp bc6bfb2d5d3b32d55be055a0514861b4e7d889bb

>   src/master/allocator/sorter/drf/sorter.cpp ac85b327fc33d34246788e6a8c8bf5a486c61434

>   src/master/http.cpp 0293e4956afb5968f84b516e3d4b5f6f858ef7a3 
>   src/master/master.hpp bade8af69f567341667b9907368207189d0dab0e 
>   src/master/master.cpp 370fd8712062dc75bb81824cb99ccc7920acbf78 
>   src/master/quota_handler.cpp bf6a613a7bb3c62fd77d1ffde3170749d6c21fa2 
>   src/master/validation.hpp 43d876b00511d4ef652c2685d4950c994b5eef31 
>   src/master/validation.cpp 50ba372d1301d16d1738a3e4f4882b51f9ce06cd 
>   src/master/weights_handler.cpp 9a901e71ba2fbf2ca1c02f658a72d44cfaa5ec62 
>   src/tests/master_validation_tests.cpp 9eb82a569f7c48caa96d4d54a93b199ccac74385 
>   src/tests/sorter_tests.cpp b0e5ef8a55bbcca3553a221bd5691a9c801a04f7 
>   src/v1/resources.cpp 6d4ec75fbb7edab013563142aaf13d5302cc50af 
> 
> Diff: https://reviews.apache.org/r/45961/diff/
> 
> 
> Testing
> -------
> 
> Tests successful.
> 
> 
> Thanks,
> 
> Anindya Sinha
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message