mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Anand Mazumdar <mazumdar.an...@gmail.com>
Subject Re: Review Request 48094: Implemented GET_ROLES Call in v1 master API.
Date Tue, 07 Jun 2016 21:48:23 GMT

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




include/mesos/v1/master.proto (lines 273 - 285)
<https://reviews.apache.org/r/48094/#comment201607>

    Let's move the `Role` message to `v1/mesos.proto`.
    
    Something like:
    
    ```cpp
    /**
     * Describes a Role. Roles can be used to specify that certain resources are reserved
for the use of one or more frameworks.
    */ 
    message Role {
      optional string name = 1 [default = "*"];
      optional double weight = 2 [default = 1.0];
      repeated string frameworks = 3;
      repeated Resource resources = 4;
    }
    ```



include/mesos/v1/master.proto (lines 277 - 281)
<https://reviews.apache.org/r/48094/#comment201599>

    hmm, this is only going to work for these 4 scalar resource types. There might be other
resources that might be used by the registered framework e.g., ports that are not scalar.
Read my earlier comment on how to deal with this.



src/internal/evolve.cpp (lines 456 - 465)
<https://reviews.apache.org/r/48094/#comment201624>

    Getting `evolve` to work is pretty tricky here. Let's consider an example:
    ```
    {
      "roles": [
        {
          "frameworks": [
            "50709431-4e3c-4abd-9812-0068b7c77f54-0000"
          ],
          "name": "*",
          "resources": {
            "cpus": 0.102,
            "disk": 0,
            "gpus": 0,
            "mem": 34,
            "port": [31000-32000]
          },
          "weight": 1
        }
      ]
    }
    ```
    
    In the above output, we have already lost information on the type of resource i.e. `SCALAR`
or `RANGES` etc (e.g, port). It should be possible to write some boiler plate code to infer
this information but I don't think it's worth the effort. I am fine with duplicating the code
in `_roles()` and directly using the automated protobuf->JSON conversion in `getRoles()`.
Makes sense?



src/tests/api_tests.cpp (lines 187 - 205)
<https://reviews.apache.org/r/48094/#comment201634>

    hmm .. `/roles` just returns `0.0` for all resources if nothing is allocated/no registered
frameworks.
    
    Can we do the following:
    
    - start a framework, then wait for an offer.
    - We can then hit the `/roles` endpoint to see if the `Resource` that we used to start
the agent match the response of `/roles`. Makes sense?


- Anand Mazumdar


On May 31, 2016, 9:37 p.m., Abhishek Dasgupta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/48094/
> -----------------------------------------------------------
> 
> (Updated May 31, 2016, 9:37 p.m.)
> 
> 
> Review request for mesos, Anand Mazumdar and Vinod Kone.
> 
> 
> Bugs: MESOS-5494
>     https://issues.apache.org/jira/browse/MESOS-5494
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> Implemented GET_ROLES Call in v1 master API.
> 
> 
> Diffs
> -----
> 
>   include/mesos/v1/master.proto bcbdd4308ab1963289f59edc8c3cd7695afa72ab 
>   src/internal/evolve.cpp 4f25f1a892b4834fc4d79b6fd1bd0edd458b7a58 
>   src/master/http.cpp c8d2f46d9e0ad8a99a6ebffc6a3d5d852cee0616 
>   src/master/master.hpp eeeccdfdfd296c2a484764e887564f2e065cfd14 
>   src/tests/api_tests.cpp 0b6bfadae3a969b3cd1a48e5095e64c953193543 
> 
> Diff: https://reviews.apache.org/r/48094/diff/
> 
> 
> Testing
> -------
> 
> On Ubuntu 16.04:
> 
> sudo GTEST_FILTER="MasterAPITest.*" make -j2 check
> 
> 
> Thanks,
> 
> Abhishek Dasgupta
> 
>


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