You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Meng Zhu <mz...@mesosphere.io> on 2018/07/04 01:01:00 UTC

Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

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

Review request for mesos and Benjamin Mahler.


Bugs: MESOS-8916
    https://issues.apache.org/jira/browse/MESOS-8916


Repository: mesos


Description
-------

This helper removes any resources that the framework is not
capable of receiving based on the given framework capability.


Diffs
-----

  src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
  src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 


Diff: https://reviews.apache.org/r/67827/diff/1/


Testing
-------

make check


Thanks,

Meng Zhu


Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67827/#review206178
-----------------------------------------------------------



FAIL: Failed to apply the current review.

Failed command: `python.exe .\support\python3\apply-reviews.py -n -r 67827`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1944/mesos-review-67827

Relevant logs:

- [apply-review-67827-stdout.log](http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1944/mesos-review-67827/logs/apply-review-67827-stdout.log):

```
error: patch failed: src/master/allocator/mesos/hierarchical.hpp:675
error: src/master/allocator/mesos/hierarchical.hpp: patch does not apply
```

- Mesos Reviewbot Windows


On July 17, 2018, 9:32 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> -----------------------------------------------------------
> 
> (Updated July 17, 2018, 9:32 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp 1f1a73fc2308301cfed6ea1131e09fbf3393dfc7 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67827/#review206341
-----------------------------------------------------------



Patch looks great!

Reviews applied: [67777, 67945, 67826, 67827]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On July 21, 2018, 1:30 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> -----------------------------------------------------------
> 
> (Updated July 21, 2018, 1:30 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9104
>     https://issues.apache.org/jira/browse/MESOS-9104
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 702e7c0aa84b4b672d82c759c25a28a77c78ad50 
>   src/master/allocator/mesos/hierarchical.cpp 707dd6bd0f255a64d759ce87cbf75be57d86b392 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67827/#review206309
-----------------------------------------------------------



PASS: Mesos patch 67827 was successfully built and tested.

Reviews applied: `['67777', '67945', '67826', '67827']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1967/mesos-review-67827

- Mesos Reviewbot Windows


On July 20, 2018, 11:30 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> -----------------------------------------------------------
> 
> (Updated July 20, 2018, 11:30 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9104
>     https://issues.apache.org/jira/browse/MESOS-9104
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 702e7c0aa84b4b672d82c759c25a28a77c78ad50 
>   src/master/allocator/mesos/hierarchical.cpp 707dd6bd0f255a64d759ce87cbf75be57d86b392 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

Posted by Meng Zhu <mz...@mesosphere.io>.

> On July 23, 2018, 4:19 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 685 (patched)
> > <https://reviews.apache.org/r/67827/diff/3/?file=2061977#file2061977line685>
> >
> >     const Resources& per comment below

Done.


> On July 23, 2018, 4:19 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 1812-1813 (original), 1812-1813 (patched)
> > <https://reviews.apache.org/r/67827/diff/3/?file=2061978#file2061978line1812>
> >
> >     What is "normal offer behavior"? In this patch we can just adjust the comment to reflect that we're not doing an explicit copy anymore, and if we want to document *why* the current approach is used, we can do that in a separate patch after hearing from Yan?

Commented why we are subtracting already offered resources in this cycle.


> On July 23, 2018, 4:19 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2044-2047 (original), 2024-2027 (patched)
> > <https://reviews.apache.org/r/67827/diff/3/?file=2061978#file2061978line2044>
> >
> >     Ditto here.

Commented why we are subtracting already offered resources in this cycle.


> On July 23, 2018, 4:19 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2689 (patched)
> > <https://reviews.apache.org/r/67827/diff/3/?file=2061978#file2061978line2734>
> >
> >     This can be `const Resources&` to avoid a copy?

Done.


- Meng


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


On July 24, 2018, 12:58 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> -----------------------------------------------------------
> 
> (Updated July 24, 2018, 12:58 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9104
>     https://issues.apache.org/jira/browse/MESOS-9104
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp c1a6789f1808a57dd94ede7bbd2636031f136ea3 
>   src/master/allocator/mesos/hierarchical.cpp 7b4e9db3435b51c9ce025b7b522e10db4b907ebb 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67827/#review206365
-----------------------------------------------------------


Ship it!





src/master/allocator/mesos/hierarchical.hpp
Lines 685 (patched)
<https://reviews.apache.org/r/67827/#comment289305>

    const Resources& per comment below



src/master/allocator/mesos/hierarchical.cpp
Lines 1812-1813 (original), 1812-1813 (patched)
<https://reviews.apache.org/r/67827/#comment289306>

    What is "normal offer behavior"? In this patch we can just adjust the comment to reflect that we're not doing an explicit copy anymore, and if we want to document *why* the current approach is used, we can do that in a separate patch after hearing from Yan?



src/master/allocator/mesos/hierarchical.cpp
Lines 2044-2047 (original), 2024-2027 (patched)
<https://reviews.apache.org/r/67827/#comment289307>

    Ditto here.



src/master/allocator/mesos/hierarchical.cpp
Lines 2689 (patched)
<https://reviews.apache.org/r/67827/#comment289304>

    This can be `const Resources&` to avoid a copy?


- Benjamin Mahler


On July 20, 2018, 11:30 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> -----------------------------------------------------------
> 
> (Updated July 20, 2018, 11:30 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9104
>     https://issues.apache.org/jira/browse/MESOS-9104
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 702e7c0aa84b4b672d82c759c25a28a77c78ad50 
>   src/master/allocator/mesos/hierarchical.cpp 707dd6bd0f255a64d759ce87cbf75be57d86b392 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/3/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67827/#review206422
-----------------------------------------------------------



PASS: Mesos patch 67827 was successfully built and tested.

Reviews applied: `['67826', '67827']`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/artifacts/mesos-reviewbot-testing/1977/mesos-review-67827

- Mesos Reviewbot Windows


On July 24, 2018, 12:58 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> -----------------------------------------------------------
> 
> (Updated July 24, 2018, 12:58 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-9104
>     https://issues.apache.org/jira/browse/MESOS-9104
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp c1a6789f1808a57dd94ede7bbd2636031f136ea3 
>   src/master/allocator/mesos/hierarchical.cpp 7b4e9db3435b51c9ce025b7b522e10db4b907ebb 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/4/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67827/
-----------------------------------------------------------

(Updated July 24, 2018, 12:58 p.m.)


Review request for mesos and Benjamin Mahler.


Bugs: MESOS-9104
    https://issues.apache.org/jira/browse/MESOS-9104


Repository: mesos


Description
-------

This helper removes any resources that the framework is not
capable of receiving based on the given framework capability.


Diffs (updated)
-----

  src/master/allocator/mesos/hierarchical.hpp c1a6789f1808a57dd94ede7bbd2636031f136ea3 
  src/master/allocator/mesos/hierarchical.cpp 7b4e9db3435b51c9ce025b7b522e10db4b907ebb 


Diff: https://reviews.apache.org/r/67827/diff/4/

Changes: https://reviews.apache.org/r/67827/diff/3-4/


Testing
-------

make check


Thanks,

Meng Zhu


Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67827/
-----------------------------------------------------------

(Updated July 20, 2018, 4:30 p.m.)


Review request for mesos and Benjamin Mahler.


Changes
-------

Rebased.


Bugs: MESOS-9104
    https://issues.apache.org/jira/browse/MESOS-9104


Repository: mesos


Description
-------

This helper removes any resources that the framework is not
capable of receiving based on the given framework capability.


Diffs (updated)
-----

  src/master/allocator/mesos/hierarchical.hpp 702e7c0aa84b4b672d82c759c25a28a77c78ad50 
  src/master/allocator/mesos/hierarchical.cpp 707dd6bd0f255a64d759ce87cbf75be57d86b392 


Diff: https://reviews.apache.org/r/67827/diff/3/

Changes: https://reviews.apache.org/r/67827/diff/2-3/


Testing
-------

make check


Thanks,

Meng Zhu


Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

Posted by Meng Zhu <mz...@mesosphere.io>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67827/
-----------------------------------------------------------

(Updated July 17, 2018, 2:32 p.m.)


Review request for mesos and Benjamin Mahler.


Bugs: MESOS-8916
    https://issues.apache.org/jira/browse/MESOS-8916


Repository: mesos


Description
-------

This helper removes any resources that the framework is not
capable of receiving based on the given framework capability.


Diffs (updated)
-----

  src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
  src/master/allocator/mesos/hierarchical.cpp 1f1a73fc2308301cfed6ea1131e09fbf3393dfc7 


Diff: https://reviews.apache.org/r/67827/diff/2/

Changes: https://reviews.apache.org/r/67827/diff/1-2/


Testing
-------

make check


Thanks,

Meng Zhu


Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

Posted by Mesos Reviewbot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67827/#review205732
-----------------------------------------------------------



Patch looks great!

Reviews applied: [67773, 67444, 67777, 67825, 67826, 67827]

Passed command: export OS='ubuntu:14.04' BUILDTOOL='autotools' COMPILER='gcc' CONFIGURATION='--verbose --disable-libtool-wrappers' ENVIRONMENT='GLOG_v=1 MESOS_VERBOSE=1'; ./support/docker-build.sh

- Mesos Reviewbot


On July 4, 2018, 1:01 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> -----------------------------------------------------------
> 
> (Updated July 4, 2018, 1:01 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

Posted by Meng Zhu <mz...@mesosphere.io>.

> On July 5, 2018, 3:16 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.hpp
> > Lines 687 (patched)
> > <https://reviews.apache.org/r/67827/diff/1/?file=2049474#file2049474line687>
> >
> >     s/ based on the ..././
> >     
> >     Or this might be clearer?
> >     
> >     // Returns the subset of resources that the framework
> >     // is capable of being offered.

I want to emphasize this function only considers framework capability, things like offer filters and etc. are not considered. This is why I think without the "based on the given framework capability" there could be doubt.


> On July 5, 2018, 3:16 p.m., Benjamin Mahler wrote:
> > src/master/allocator/mesos/hierarchical.cpp
> > Lines 2660-2685 (patched)
> > <https://reviews.apache.org/r/67827/diff/1/?file=2049475#file2049475line2701>
> >
> >     Should we re-write this now to just consistently use filter()?
> >     
> >     ```
> >     Resources HierarchicalAllocatorProcess::stripIncapableResources(
> >         const Resources& resources, // Can take a const ref now?
> >         const protobuf::framework::Capabilities& frameworkCapabilities) const
> >     {
> >       return resources.filter([&](const Resource& resource) {
> >         if (!frameworkCapabilities.sharedResources &&
> >             Resources::isShared(resource)) {
> >           return false;
> >         }
> >         
> >         if (!frameworkCapabilities.revocableResources &&
> >             Resources::isRevocable(resource)) {
> >           return false;
> >         }
> >         
> >         // When reservation refinements are present, old frameworks without the
> >         // RESERVATION_REFINEMENT capability won't be able to understand the
> >         // new format. While it's possible to translate the refined reservations
> >         // into the old format by "hiding" the intermediate reservations in the
> >         // "stack", this leads to ambiguity when processing RESERVE / UNRESERVE
> >         // operations. This is due to the loss of information when we drop the
> >         // intermediate reservations. Therefore, for now we simply filter out
> >         // resources with refined reservations if the framework does not have
> >         // the capability.
> >         if (!frameworkCapabilities.reservationRefinement &&
> >             Resources::hasRefinedReservations(resource)) {
> >           return false;
> >         }
> >         
> >         return true;
> >       });
> >     }
> >     ```
> >     
> >     I think this approach avoids adding extra copyies as we add more cases?

Good point.


- Meng


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


On July 17, 2018, 2:32 p.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> -----------------------------------------------------------
> 
> (Updated July 17, 2018, 2:32 p.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp 1f1a73fc2308301cfed6ea1131e09fbf3393dfc7 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/2/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

Posted by Benjamin Mahler <bm...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67827/#review205771
-----------------------------------------------------------



Looks good, the second allocation loop is starting to look pretty succinct!

Just a few questions about the logic below


src/master/allocator/mesos/hierarchical.hpp
Lines 687 (patched)
<https://reviews.apache.org/r/67827/#comment288668>

    s/ based on the ..././
    
    Or this might be clearer?
    
    // Returns the subset of resources that the framework
    // is capable of being offered.



src/master/allocator/mesos/hierarchical.cpp
Lines 1768-1772 (original), 1768-1770 (patched)
<https://reviews.apache.org/r/67827/#comment288670>

    Hm.. do we need the if condition? Can't this just be:
    
    ```
    available -= offeredSharedResources.get(slaveId).getOrElse(Resources());
    ```



src/master/allocator/mesos/hierarchical.cpp
Line 1930 (original), 1912-1914 (patched)
<https://reviews.apache.org/r/67827/#comment288671>

    Hm.. this looks like a no-op change?



src/master/allocator/mesos/hierarchical.cpp
Lines 1998-2003 (original), 1982-1984 (patched)
<https://reviews.apache.org/r/67827/#comment288673>

    Ditto here



src/master/allocator/mesos/hierarchical.cpp
Line 2083 (original), 2044-2046 (patched)
<https://reviews.apache.org/r/67827/#comment288672>

    Ditto here



src/master/allocator/mesos/hierarchical.cpp
Lines 2660-2685 (patched)
<https://reviews.apache.org/r/67827/#comment288676>

    Should we re-write this now to just consistently use filter()?
    
    ```
    Resources HierarchicalAllocatorProcess::stripIncapableResources(
        const Resources& resources, // Can take a const ref now?
        const protobuf::framework::Capabilities& frameworkCapabilities) const
    {
      return resources.filter([&](const Resource& resource) {
        if (!frameworkCapabilities.sharedResources &&
            Resources::isShared(resource)) {
          return false;
        }
        
        if (!frameworkCapabilities.revocableResources &&
            Resources::isRevocable(resource)) {
          return false;
        }
        
        // When reservation refinements are present, old frameworks without the
        // RESERVATION_REFINEMENT capability won't be able to understand the
        // new format. While it's possible to translate the refined reservations
        // into the old format by "hiding" the intermediate reservations in the
        // "stack", this leads to ambiguity when processing RESERVE / UNRESERVE
        // operations. This is due to the loss of information when we drop the
        // intermediate reservations. Therefore, for now we simply filter out
        // resources with refined reservations if the framework does not have
        // the capability.
        if (!frameworkCapabilities.reservationRefinement &&
            Resources::hasRefinedReservations(resource)) {
          return false;
        }
        
        return true;
      });
    }
    ```
    
    I think this approach avoids adding extra copyies as we add more cases?


- Benjamin Mahler


On July 4, 2018, 1:01 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> -----------------------------------------------------------
> 
> (Updated July 4, 2018, 1:01 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>


Re: Review Request 67827: Added a helper `stripIncapableResources` in the allocator.

Posted by Mesos Reviewbot Windows <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/67827/#review205708
-----------------------------------------------------------



FAIL: Some of the unit tests failed. Please check the relevant logs.

Reviews applied: `['67444', '67777', '67825', '67826', '67827']`

Failed command: `Start-MesosCITesting`

All the build artifacts available at: http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67827

Relevant logs:

- [mesos-tests-stdout.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67827/logs/mesos-tests-stdout.log):

```
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_DestroyWhilePulling
[       OK ] DockerContainerizerTest.ROOT_DOCKER_DestroyWhilePulling (801 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_DestroyUnknownContainer
[       OK ] DockerContainerizerTest.ROOT_DOCKER_DestroyUnknownContainer (603 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_ExecutorCleanupWhenLaunchFailed
[       OK ] DockerContainerizerTest.ROOT_DOCKER_ExecutorCleanupWhenLaunchFailed (1226 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_FetchFailure
[       OK ] DockerContainerizerTest.ROOT_DOCKER_FetchFailure (798 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_DockerPullFailure
[       OK ] DockerContainerizerTest.ROOT_DOCKER_DockerPullFailure (801 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_DockerInspectDiscard
[       OK ] DockerContainerizerTest.ROOT_DOCKER_DockerInspectDiscard (1007 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_WaitUnknownContainer
[       OK ] DockerContainerizerTest.ROOT_DOCKER_WaitUnknownContainer (597 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_NoTransitionFromKillingToRunning
[       OK ] DockerContainerizerTest.ROOT_DOCKER_NoTransitionFromKillingToRunning (5561 ms)
[ RUN      ] DockerContainerizerTest.ROOT_DOCKER_DefaultDNS
[       OK ] DockerContainerizerTest.ROOT_DOCKER_DefaultDNS (5258 ms)
[----------] 24 tests from DockerContainerizerTest (93839 ms total)

[----------] 1 test from HungDockerTest
[ RUN      ] HungDockerTest.ROOT_DOCKER_InspectHungDuringPull

d:\dcos\mesos\mesos\src\tests\mock_docker.hpp(155): ERROR: this mock object (used in test HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be deleted but never is. Its address is @000000AC3FF5BB00.
d:\dcos\mesos\mesos\src\tests\containerizer\docker_containerizer_tests.cpp(5187): ERROR: this mock object (used in test HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be deleted but never is. Its address is @000000AC3FF5BD60.
d:\dcos\mesos\mesos\src\tests\mock_docker.cpp(48): ERROR: this mock object (used in test HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be deleted but never is. Its address is @0000020B480C7890.
d:\dcos\mesos\mesos\3rdparty\libprocess\include\process\gmock.hpp(235): ERROR: this mock object (used in test HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be deleted but never is. Its address is @0000020B487738B8.
d:\dcos\mesos\mesos\src\tests\mock_registrar.cpp(54): ERROR: this mock object (used in test HungDockerTest.ROOT_DOCKER_InspectHungDuringPull) should be deleted but never is. Its address is @0000020B48FC87B0.
ERROR: 5 leaked mock objects found at program exit.
```

- [mesos-tests-stderr.log](http://dcos-win.westus.cloudapp.azure.com/mesos-build/review/67827/logs/mesos-tests-stderr.log):

```
I0704 02:47:51.492888  1828 authenticatee.cpp:299] Authentication success
I0704 02:47:51.493870  8728 master.cpp:9802] Successfully authenticated principal 'test-principal' at scheduler-10e49cd3-1f13-41c6-8a87-79c1f6175935@192.10.1.6:60741
I0704 02:47:51.493870  2728 sched.cpp:501] Successfully authenticated with master master@192.10.1.6:60741
I0704 02:47:51.495057  1828 master.cpp:2927] Received SUBSCRIBE call for framework 'default' at scheduler-10e49cd3-1f13-41c6-8a87-79c1f6175935@192.10.1.6:60741
I0704 02:47:51.495057  1828 master.cpp:2234] Authorizing framework principal 'test-principal' to receive offers for roles '{ * }'
I0704 02:47:51.495887  7476 master.cpp:3008] Subscribing framework default with checkpointing disabled and capabilities [ MULTI_ROLE, RESERVATION_REFINEMENT ]
I0704 02:47:51.496978  7476 master.cpp:9993] Adding framework 2f17db8c-4e59-471f-9d62-2d3c02232b0a-0000 (default) at scheduler-10e49cd3-1f13-41c6-8a87-79c1f6175935@192.10.1.6:60741 with roles {  } suppressed
I0704 02:47:51.497874  9732 sched.cpp:749] Framework registered with 2f17db8c-4e59-471f-9d62-2d3c02232b0a-0000
I0704 02:47:51.498872 11940 hierarchical.cpp:298] Added framework 2f17db8c-4e59-471f-9d62-2d3c02232b0a-0000
E0704 02:47:51.598872  2728 slave.cpp:7289] EXIT with status 1: Failed to perform recovery: Collect failed: Failed to run 'C:\Program Files (x86)\Microsoft Visual Studio\2017\Community\VC\Auxiliary\Build\test-docker.bat -H npipe:////./pipe/docker_engine ps -a': exited with status 1; stderr=''C:\Program' is not recognized as an internal or external command,

operable program or batch file.

'
If recovery failed due to a change in configuration and you want to
keep the current agent id, you might want to change the
`--reconfiguration_policy` flag to a more permissive value.

To restart this agent with a new agent id instead, do as follows:
rm -f C:\Users\jenkins\AppData\Local\Temp\RP8OWM\meta\slaves\latest
This ensures that the agent does not recover old live executors.

If you use the Docker containerizer and think that the Docker
daemon state is broken, you can try to clear it. But be careful:
these commands will erase all containers and images from this host,
not just those started by Mesos!
docker kill $(docker ps -q)
docker rm $(docker ps -a -q)
docker rmi $(docker images -q)

Finally, restart the agent.
```

- Mesos Reviewbot Windows


On July 4, 2018, 1:01 a.m., Meng Zhu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67827/
> -----------------------------------------------------------
> 
> (Updated July 4, 2018, 1:01 a.m.)
> 
> 
> Review request for mesos and Benjamin Mahler.
> 
> 
> Bugs: MESOS-8916
>     https://issues.apache.org/jira/browse/MESOS-8916
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This helper removes any resources that the framework is not
> capable of receiving based on the given framework capability.
> 
> 
> Diffs
> -----
> 
>   src/master/allocator/mesos/hierarchical.hpp 0f6c0e96a105c64465d3f5db4ff663d8fdfe7e26 
>   src/master/allocator/mesos/hierarchical.cpp 5a6cd3d2fc5bdbaaee2d05b9be9e83d4107c749b 
> 
> 
> Diff: https://reviews.apache.org/r/67827/diff/1/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Meng Zhu
> 
>