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 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors
Date Tue, 06 Oct 2015 16:38:52 GMT


> On Oct. 6, 2015, 6:35 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2420
> > <https://reviews.apache.org/r/38877/diff/2/?file=1087669#file1087669line2420>
> >
> >     Should use return here directly instead of break?

What is the advantage of one over the other here ? I was being consistent with `(re-)registerExecutor(...)`
functions in the same file.


> On Oct. 6, 2015, 6:35 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 4244
> > <https://reviews.apache.org/r/38877/diff/2/?file=1087669#file1087669line4244>
> >
> >     I just suggest if it possible to change it like this pattern.
> >     
> >     ```
> >     if (executor->pid.isSome() && executor->pid.get()) {
> >     // Normal executor
> >     } else if (executor->pid.isNone()) {
> >     // Http executor
> >     } else {
> >     // Other cases
> >     }
> >     ```
> >     
> >     My concern is `else` maybe have more exception cases in the future, assume the
only case in `else` is http executor seems not always match.

Good suggestion. Fixed.


> On Oct. 6, 2015, 6:35 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2545
> > <https://reviews.apache.org/r/38877/diff/2/?file=1087669#file1087669line2545>
> >
> >     When execute is in unexpected state, we keep http connection open and not close
it?

We are not leaking any descriptors. The OS would reclaim these after the crash on this line.


- Anand


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


On Sept. 30, 2015, 6:06 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> -----------------------------------------------------------
> 
> (Updated Sept. 30, 2015, 6:06 a.m.)
> 
> 
> Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.
> 
> 
> Bugs: MESOS-3515
>     https://issues.apache.org/jira/browse/MESOS-3515
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This change adds the functionality for executors to `Subscribe` via the `api/v1/executor`
endpoint. It also stores a marker file as part of the `Subscribe` call if framework `checkpointing`
is enabled. This can then be used by the agent when recovering to wait for reconnecting back
with the executor.
> 
> Since `Call::Update` is in progress as part of MESOS-3476. I have added a `CHECK` if
a executor tries to send a list of unacknowledged tasks as part of the `Subscribe` call.
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
>   src/slave/slave.hpp 18be4f8188ad34ef4d0aa4b5eba241053d071476 
>   src/slave/slave.cpp d1c9977feeb30ad43586a4560eed155865d27a6c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


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