mesos-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Benjamin Hindman" <b...@berkeley.edu>
Subject Re: Review Request: Renamed Executor Init callback
Date Wed, 08 Feb 2012 20:11:24 GMT

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



include/mesos/executor.hpp
<https://reviews.apache.org/r/3761/#comment10826>

    Okay, I think the right thing here is to pass both SlaveID and SlaveInfo. That way we
can access the slaveInfo.hostname() and slaveInfo.public_hostname() if we need.



include/mesos/executor.hpp
<https://reviews.apache.org/r/3761/#comment10824>

    This looks like I forgot to finish this comment. I think I wanted to say: "Note, the executor
pointer must outlive the driver." Can you add that please? Thanks!



src/Makefile.am
<https://reviews.apache.org/r/3761/#comment10825>

    This has been committed in another patch.



src/exec/exec.cpp
<https://reviews.apache.org/r/3761/#comment10827>

    No need for the ':'.



src/exec/exec.cpp
<https://reviews.apache.org/r/3761/#comment10828>

    s/for:/for task



src/exec/exec.cpp
<https://reviews.apache.org/r/3761/#comment10831>

    Indentation?



src/exec/exec.cpp
<https://reviews.apache.org/r/3761/#comment10829>

    Wrap line.



src/exec/exec.cpp
<https://reviews.apache.org/r/3761/#comment10830>

    This data could be binary! Let's not print it out please.



src/exec/exec.cpp
<https://reviews.apache.org/r/3761/#comment10832>

    This is more verbose than we need to be, please remove.



src/python/native/proxy_executor.cpp
<https://reviews.apache.org/r/3761/#comment10833>

    If the rest of the file was updated to spell out the variable (i.e., sidObj => slaveIdObj),
then do the same here please.


- Benjamin


On 2012-02-05 03:34:16, Vinod Kone wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3761/
> -----------------------------------------------------------
> 
> (Updated 2012-02-05 03:34:16)
> 
> 
> Review request for mesos, Benjamin Hindman, John Sirois, and Brian Wickman.
> 
> 
> Summary
> -------
> 
> Currently, the init() callback in Executor is not consistent with the naming of our callbacks.
Like scheduler callback, this should be called registered(). 
> 
> In the near future, we are also going to add a new executor callback called re-registered()
as we implement slaves being restarted without killing executors.
> 
> As part of this, also got rid of the ExecutorArgs protobuf.
> 
> 
> Diffs
> -----
> 
>   frameworks/hadoop-0.20.2/src/contrib/mesos/src/java/org/apache/hadoop/mapred/FrameworkExecutor.java
6744082 
>   include/mesos/executor.hpp 8e651e7 
>   include/mesos/mesos.proto b855f70 
>   src/Makefile.am ac0a209 
>   src/examples/java/TestExecutor.java 6211490 
>   src/examples/long_lived_executor.cpp 6ba083b 
>   src/examples/memhog_executor.cpp 333b1d9 
>   src/examples/test_executor.cpp 9dd244e 
>   src/exec/exec.cpp 48c0d0f 
>   src/java/jni/convert.cpp 04a25dd 
>   src/java/jni/org_apache_mesos_MesosExecutorDriver.cpp b338847 
>   src/java/src/org/apache/mesos/Executor.java a05591f 
>   src/messages/messages.proto 04dc3a0 
>   src/python/native/proxy_executor.hpp 3ed9e37 
>   src/python/native/proxy_executor.cpp 30cf089 
>   src/python/src/mesos.py 026a611 
>   src/scaling/nested_exec.py 5c4edf1 
>   src/sched/sched.cpp 21d670f 
>   src/slave/slave.cpp 9b33a5a 
>   src/tests/fault_tolerance_tests.cpp 7be3d6b 
>   src/tests/master_tests.cpp a411ba5 
>   src/tests/utils.hpp 660aee9 
> 
> Diff: https://reviews.apache.org/r/3761/diff
> 
> 
> Testing
> -------
> 
> ./mesos-tests --gtest_filter = -"*Python*"
> 
> All the tests, except the SamplePythonFramework, pass. 
> 
> Looking at earlier Jiras, looks like there is a fix for python tests, that needs to be
pushed to the trunk? I'm not sure if that fixes this test fail yet.
> 
> 
> Thanks,
> 
> Vinod
> 
>


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