mesos-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vinod Kone <vinodk...@gmail.com>
Subject Re: Review Request 45067: Updated the long-lived-framework example.
Date Fri, 01 Apr 2016 21:05:44 GMT

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




src/examples/long_lived_framework.cpp (line 51)
<https://reviews.apache.org/r/45067/#comment189655>

    I think you should add a comment here about what the high level behavior of this scheduler
is. For example that this schedule tries to pick one slave and launch as many tasks on it
as possible, using a single multi-task executor. And that if the slave or executor fails,
it picks another slave and repeats the process.



src/examples/long_lived_framework.cpp (line 64)
<https://reviews.apache.org/r/45067/#comment189650>

    Any particular reason you are using "cout" instead of LOG? It would be great to have timestamps,
esp if you are planning to run this in a test cluster.



src/examples/long_lived_framework.cpp (line 86)
<https://reviews.apache.org/r/45067/#comment189653>

    Can we use "foreach(const Offer& offer, offers)" here?



src/examples/long_lived_framework.cpp (line 87)
<https://reviews.apache.org/r/45067/#comment189652>

    why do you need a vector here? AFAICT, you only ever call launchTasks with a single task.
If that's the case just pass '{task}' to driver.launchTasks().



src/examples/long_lived_framework.cpp (line 92)
<https://reviews.apache.org/r/45067/#comment189651>

    s/offer/offer is/



src/examples/long_lived_framework.cpp (lines 104 - 150)
<https://reviews.apache.org/r/45067/#comment189656>

    This is still a bit hard to follow. 
    
    How about:
    
    ```
    foreach (const Offer& offer : offers) {
    
      if (slaveID.isNone() {
        // No active executor running in the cluster. Launch a new task with executor.
      
        if (offer.resources < (task + executor).resources()) {
           // Not enough resources. Decline.
           decline();
        } else {
           launch();
        }
      } else if (slaveID == offer.slaveID() {
        // Offer from the same slave that has an active executor. Launch more tasks on that
executor.
        
        if (offer.resources < task.executors()) {
          // Noe enough resources. Decline.
          decline();
        } else {
          launch();
        }
      } else {
        // Offer from a slave different from the slave that has an active executor. Decline.
        decline();
      }
    }
    
    ```



src/examples/long_lived_framework.cpp (line 177)
<https://reviews.apache.org/r/45067/#comment189649>

    log a message here.



src/examples/long_lived_framework.cpp (line 186)
<https://reviews.apache.org/r/45067/#comment189648>

    log a message here.


- Vinod Kone


On March 29, 2016, 7:08 p.m., Joseph Wu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/45067/
> -----------------------------------------------------------
> 
> (Updated March 29, 2016, 7:08 p.m.)
> 
> 
> Review request for mesos, Greg Mann, Artem Harutyunyan, Kevin Klues, and Vinod Kone.
> 
> 
> Bugs: MESOS-5062
>     https://issues.apache.org/jira/browse/MESOS-5062
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This gives the example `long-lived-framework` enough options to run outside of the build
environment.
> 
> This also updates:
> 
> * The style of the framework code.
> * Gives the `ExecutorInfo` some resources (needed for some cgroups isolators).
> * Restricts the framework to one agent.  Otherwise, it would grab a small chunk of every
machine in the cluster.
> * Adds filters for declined offers.
> 
> 
> Diffs
> -----
> 
>   src/examples/long_lived_framework.cpp ef498d63bc5f0a8deb46d71edd85a76a1d38fdd0 
> 
> Diff: https://reviews.apache.org/r/45067/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> Ran this on the master node on a Mesos cluster:
> ```
> ./long-lived-framework --master=zk://localhost:2181/mesos --executor_uri="https://s3.amazonaws.com/url/to/long-lived-executor"
--executor_command="LD_LIBRARY_PATH=/path/to/libmesos && ./long-lived-executor"
> ```
> 
> 
> Thanks,
> 
> Joseph Wu
> 
>


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