-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/29736/
-----------------------------------------------------------
(Updated Feb. 5, 2015, 1:40 a.m.)
Review request for mesos, Adam B, Benjamin Hindman, Ben Mahler, Jie Yu, and
Vinod Kone.
Repository: mesos
Description (updated)
-------
1. A `Resources::Filter` constructs off of a predicate and the operator () has
a single implementation:
```cpp
Resources result;
foreach (const Resource& resource, resources) {
if (pred(resource)) {
result += resource;
}
}
return result;
```
Advantages of this approach:
a. No new class definition is necessary to create a new filter.
- We can use static functions to create a new filter inline. e.g.
Filter(persistentVolume) or Filter(empty)
- We can use lambdas (later on) to create a new filter inline.
Filter([](const Resource& resource) { ... })
- We don't need to write the same for loop with different predicates for
every new filter.
b. The resulting set of resources are guaranteed to consist only of resources
that existed in the original resources.
- The only guarantee we enforce with a derived filter class that inherits
from a base filter class is
that we take Resources and return Resources. Nothing enforces that the
resulting resources must only be
consisted of existing resources.
2. We can naturally compose filters with logical operators.
Suppose we want a filter that returns resources that are reserved for a
particular `ReservationType`, and
a filter that returns persistent volumes. It may look something like this in
the old approach.
```cpp
// Returns resources that are reserved for a particular ReservationType.
struct ReservedFilter : Filter
{
explicit ReservedFilter(const Resource::ReservationType& _reservationType)
: reservationType(_reservationType) {}
virtual Resources apply(const Resources& resources) const {
Resources result;
foreach (const Resource& resource, resources) {
if (isReserved(resource, reservationType)) {
result += resource;
}
}
return result;
}
Resource::ReservationType reservationType;
}
// Returns persistent volumes.
struct PersistentVolumeFilter : Filter
{
virtual Resources apply(const Resources& resources) const {
Resources result;
foreach (const Resource& resource, resources) {
if (isPersistentVolume(resource)) {
result += resource;
}
}
return result;
}
}
```
and we can use them like so:
```cpp
ReservedFilter(Resource::DYNAMIC).apply(resources);
PersistentVolumeFilter().apply(resources);
```
But how about when we want to now create a filter that returns resources that
are persistent volumes **and**
reserved for a particular `ReservationType`? Ideally we would compose these 2
existing filters somehow,
but we end up having to duplicate most of the code from each filter and write a
new filter definition
(or perform both filters and intersect the results, yikes.):
```cpp
// Returns persistent volumes that are reserved for a particular
ReservationType.
struct ReservedPersistentVolumeFilter : Filter
{
explicit ReservedFilter(const Resource::ReservationType& _reservationType)
: reservationType(_reservationType) {}
virtual Resources apply(const Resources& resources) const {
Resources result;
foreach (const Resource& resource, resources) {
if (reserved(resource, reservationType) && persistentVolume(resource)) {
result += resource;
}
}
return result;
}
Resource::ReservationType reservationType;
}
```
and it would be used like so:
```cpp
ReservedPersistentVolumeFilter(Resource::DYNAMIC).apply(resources);
```
Here's what the above looks like in the new design.
```cpp
class Filter
{
public:
// Useful predefined filters.
static Filter reserved(const Resource::ReservationType& reservationType)
{
return Filter(lambda::bind(Resources::reserved, lambda::_1,
reservationType));
}
static Filter persistentVolume()
{
return Filter(Resources::persistentVolume);
}
/* Generic Filter definition */
};
```
These can be used like so:
```cpp
Filter::reserved(Resource::DYNAMIC)(resources);
Filter::persistentVolume()(resources);
```
With logical operators, we can compose them inline like so:
```cpp
Filter filter =
Filter::reserved(Resource::DYNAMIC) && Filter::persistentVolume();
Resources filtered = filter(resources);
```
If this composition is common, then we can always add another static function
under `Filter`:
```cpp
class Filter
{
static Filter reserved(const Resource::ReservationType& reservationType);
static Filter persistentVolume();
static Filter reservedPersistentVolume(const Resource::ReservationType&
reservationType)
{
return Filter::reserved(Resource::DYNAMIC) && Filter::persistentVolume();
}
/* ... */
};
```
At which point we can use it like so:
```cpp
Filter::reservedPersistentVolume(Resource::DYNAMIC)(resources);
```
Diffs
-----
include/mesos/resources.hpp 3b57568c10233a0c692787de6464f21af5eaadf4
src/common/resources.cpp 308b8860fec3db5bfab3925423afda037bfbc4e7
src/master/hierarchical_allocator_process.hpp
6b4489276d9f72f3bd99066bf7e48dba5ebe537e
src/tests/hierarchical_allocator_tests.cpp
f44d9e98d6d9db9621f5361cddb6134f90277180
src/tests/resources_tests.cpp 4744e872b082553046ecc0e344403754ee685842
Diff: https://reviews.apache.org/r/29736/diff/
Testing
-------
make check
Thanks,
Michael Park
