You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Benjamin Mahler <bm...@apache.org> on 2017/07/03 13:22:54 UTC

Re: Review Request 60369: Exposed allocated resources per each role in /state endpoint on agent.

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




src/slave/http.cpp
Lines 1330-1334 (patched)
<https://reviews.apache.org/r/60369/#comment254243>

    Whoops, I should have noticed this earlier, but when going to commit this I was taking a careful look at the code. This isn't accounting for the pending tasks here:
    
    https://github.com/apache/mesos/blob/0d277bb64fa5a4d0b4f741daedf64095beab4773/src/slave/slave.hpp#L904-L905
    
    Unfortunately the logic is a little tricky. We have to include all of the pending task resources. Also, for each executor in the pending map, we have to add the ExecutorInfo resources only if it's absent from the 'executors' map. You'll want to do a read through of how the data structures are used to confirm whether I'm right. :)
    
    My suggestion here would be to have an `allocatedResources()` function within the `Framework` struct that contains this logic, and here in the http handler we just sum `allocatedResources()` across the frameworks.


- Benjamin Mahler


On June 29, 2017, 3:23 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60369/
> -----------------------------------------------------------
> 
> (Updated June 29, 2017, 3:23 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-6441
>     https://issues.apache.org/jira/browse/MESOS-6441
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The JSON key for this information is "reserved_resources_allocated"
> and "unreserved_resources_allocated".
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp cbbc1dc27cc90bac8d48cbbc84266c3d87490a3c 
> 
> 
> Diff: https://reviews.apache.org/r/60369/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>


Re: Review Request 60369: Exposed allocated resources per each role in /state endpoint on agent.

Posted by Andrei Budnik <ab...@mesosphere.com>.

> On July 3, 2017, 1:22 p.m., Benjamin Mahler wrote:
> > src/slave/http.cpp
> > Lines 1330-1334 (patched)
> > <https://reviews.apache.org/r/60369/diff/4/?file=1766945#file1766945line1330>
> >
> >     Whoops, I should have noticed this earlier, but when going to commit this I was taking a careful look at the code. This isn't accounting for the pending tasks here:
> >     
> >     https://github.com/apache/mesos/blob/0d277bb64fa5a4d0b4f741daedf64095beab4773/src/slave/slave.hpp#L904-L905
> >     
> >     Unfortunately the logic is a little tricky. We have to include all of the pending task resources. Also, for each executor in the pending map, we have to add the ExecutorInfo resources only if it's absent from the 'executors' map. You'll want to do a read through of how the data structures are used to confirm whether I'm right. :)
> >     
> >     My suggestion here would be to have an `allocatedResources()` function within the `Framework` struct that contains this logic, and here in the http handler we just sum `allocatedResources()` across the frameworks.

Fixed.


- Andrei


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


On July 3, 2017, 5:52 p.m., Andrei Budnik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/60369/
> -----------------------------------------------------------
> 
> (Updated July 3, 2017, 5:52 p.m.)
> 
> 
> Review request for mesos, Benjamin Mahler and haosdent huang.
> 
> 
> Bugs: MESOS-6441
>     https://issues.apache.org/jira/browse/MESOS-6441
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> The JSON key for this information is "reserved_resources_allocated"
> and "unreserved_resources_allocated".
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp 700871e1502a65b0bb1fc31219e09219dbdb5340 
>   src/slave/slave.hpp 5ede32d00c5dbf18dc0639ec7af5d2a86f86f619 
>   src/slave/slave.cpp 0e24b8cb8d1020af515e3d1862e121e1daf82ce9 
> 
> 
> Diff: https://reviews.apache.org/r/60369/diff/5/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Andrei Budnik
> 
>