> 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
> 
>

Reply via email to