You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@mesos.apache.org by Anand Mazumdar <ma...@gmail.com> on 2015/09/30 05:40:06 UTC

Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

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

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


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Guangya Liu <gy...@gmail.com>.

> On 九月 30, 2015, 5:41 a.m., Guangya Liu wrote:
> > src/slave/slave.cpp, line 2387
> > <https://reviews.apache.org/r/38877/diff/1/?file=1087521#file1087521line2387>
> >
> >     How to handle the case if executor is in RECOVRING state? The logic is different for register and reregister with RECOVERING state.
> 
> Anand Mazumdar wrote:
>     I am assuming you were referring to the Agent state. We should never be in `state == RECOVERY` when we make a `Subscribe` call. 
>     
>     This should already have been handled before here : https://github.com/apache/mesos/blob/master/src/slave/http.cpp#L218
>     
>     We want the `CHECK` to fail if we ever land up here and the agent is still recovering !

Got it, thanks!


- Guangya


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


On 九月 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 九月 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
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.

> On Sept. 30, 2015, 5:41 a.m., Guangya Liu wrote:
> > src/slave/slave.cpp, line 2385
> > <https://reviews.apache.org/r/38877/diff/1/?file=1087521#file1087521line2385>
> >
> >     What about using "Received subscription request for HTTP executor xxx" to keep consistent with scheduler log?

Yeah the other `Agent` functions used the 'Got ...' lingo. Modified it now.


> On Sept. 30, 2015, 5:41 a.m., Guangya Liu wrote:
> > src/slave/slave.cpp, line 2387
> > <https://reviews.apache.org/r/38877/diff/1/?file=1087521#file1087521line2387>
> >
> >     How to handle the case if executor is in RECOVRING state? The logic is different for register and reregister with RECOVERING state.

I am assuming you were referring to the Agent state. We should never be in `state == RECOVERY` when we make a `Subscribe` call. 

This should already have been handled before here : https://github.com/apache/mesos/blob/master/src/slave/http.cpp#L218

We want the `CHECK` to fail if we ever land up here and the agent is still recovering !


- Anand


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


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
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/#review101077
-----------------------------------------------------------



src/slave/slave.hpp (line 169)
<https://reviews.apache.org/r/38877/#comment158394>

    Do you mind switch the first two parameters as following, this can highlight that this is an http call.
    
    void subscribe(
        HttpConnection http,
        const executor::Call::Subscribe& subscribe,
        Framework* framework,
        Executor* executor);



src/slave/slave.cpp (line 2385)
<https://reviews.apache.org/r/38877/#comment158395>

    What about using "Received subscription request for HTTP executor xxx" to keep consistent with scheduler log?



src/slave/slave.cpp (line 2387)
<https://reviews.apache.org/r/38877/#comment158398>

    How to handle the case if executor is in RECOVRING state? The logic is different for register and reregister with RECOVERING state.


- Guangya Liu


On 九月 30, 2015, 3:40 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> -----------------------------------------------------------
> 
> (Updated 九月 30, 2015, 3:40 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
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/#review108903
-----------------------------------------------------------

Ship it!


Ship It!

- Vinod Kone


On Dec. 2, 2015, 10:41 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2015, 10:41 p.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.
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp c3247f17e9faed32a46d3ab9ee83c399cd2c8d5e 
>   src/slave/slave.hpp 5ee133ae52998d05c8afabb2ade7095e363c75c5 
>   src/slave/slave.cpp 9055f2a789cb19f3579c15a379ea505dfef0578c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/
-----------------------------------------------------------

(Updated Dec. 2, 2015, 10:41 p.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
-------

Review comments from Vinod


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.


Diffs (updated)
-----

  src/slave/http.cpp c3247f17e9faed32a46d3ab9ee83c399cd2c8d5e 
  src/slave/slave.hpp 5ee133ae52998d05c8afabb2ade7095e363c75c5 
  src/slave/slave.cpp 9055f2a789cb19f3579c15a379ea505dfef0578c 

Diff: https://reviews.apache.org/r/38877/diff/


Testing
-------

make check


Thanks,

Anand Mazumdar


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.

> On Dec. 2, 2015, 10:24 p.m., Vinod Kone wrote:
> > I see a very basic test in the next review. Are you planning to write more comprehensive tests? Is that plan to templatize slave recovery tests for both pid based and http executors?

For now, I added just a basic test for testing the subscribe->subscribed workflow but I would surely be adding more tests around recovery. Currently, the existing Slave recovery tests use the `Executor/ExecutorDriver` to do all the heavy lifting. Until, have a HTTP based executor library it won't be that easy to templatize the slave recovery tests. Hence, for now, I would go ahead and add some basic tests in `src/tests/executor_http_apt_tests.cpp`. Does that sound reasonable to you ?


> On Dec. 2, 2015, 10:24 p.m., Vinod Kone wrote:
> > src/slave/slave.cpp, lines 2414-2415
> > <https://reviews.apache.org/r/38877/diff/10/?file=1148138#file1148138line2414>
> >
> >     I see that 'RECOVERING' is a possible state in `registerExecutor()`. Is that not possible for http executors?

Yes, the agent cannot be in `state == RECOVERING` for HTTP based executors since we already check it inside the `executor(...)` call back here:
https://github.com/apache/mesos/blob/master/src/slave/http.cpp#L218

Hence, it should be fine to fail the assert and crash here.


- Anand


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


On Dec. 2, 2015, 10:41 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2015, 10:41 p.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.
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp c3247f17e9faed32a46d3ab9ee83c399cd2c8d5e 
>   src/slave/slave.hpp 5ee133ae52998d05c8afabb2ade7095e363c75c5 
>   src/slave/slave.cpp 9055f2a789cb19f3579c15a379ea505dfef0578c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Vinod Kone <vi...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/#review108391
-----------------------------------------------------------


I see a very basic test in the next review. Are you planning to write more comprehensive tests? Is that plan to templatize slave recovery tests for both pid based and http executors?


src/slave/slave.cpp (lines 2414 - 2415)
<https://reviews.apache.org/r/38877/#comment167839>

    I see that 'RECOVERING' is a possible state in `registerExecutor()`. Is that not possible for http executors?



src/slave/slave.cpp (line 2450)
<https://reviews.apache.org/r/38877/#comment167841>

    s/retry/retried/



src/slave/slave.cpp (line 2454)
<https://reviews.apache.org/r/38877/#comment167843>

    i don't think 'in lieu of' is correct here. just remove the second part starting from "in lieu of...".



src/slave/slave.cpp (line 2475)
<https://reviews.apache.org/r/38877/#comment167844>

    s/http/HTTP/


- Vinod Kone


On Nov. 30, 2015, 3:56 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2015, 3:56 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.
> 
> 
> Diffs
> -----
> 
>   src/slave/http.cpp c3247f17e9faed32a46d3ab9ee83c399cd2c8d5e 
>   src/slave/slave.hpp 5ee133ae52998d05c8afabb2ade7095e363c75c5 
>   src/slave/slave.cpp 9055f2a789cb19f3579c15a379ea505dfef0578c 
> 
> Diff: https://reviews.apache.org/r/38877/diff/
> 
> 
> Testing
> -------
> 
> make check
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/
-----------------------------------------------------------

(Updated Nov. 30, 2015, 3:56 a.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
-------

Since MESOS-3476 is resolved now. Updated the TODO for it.


Bugs: MESOS-3515
    https://issues.apache.org/jira/browse/MESOS-3515


Repository: mesos


Description (updated)
-------

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.


Diffs (updated)
-----

  src/slave/http.cpp c3247f17e9faed32a46d3ab9ee83c399cd2c8d5e 
  src/slave/slave.hpp 5ee133ae52998d05c8afabb2ade7095e363c75c5 
  src/slave/slave.cpp 9055f2a789cb19f3579c15a379ea505dfef0578c 

Diff: https://reviews.apache.org/r/38877/diff/


Testing
-------

make check


Thanks,

Anand Mazumdar


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/
-----------------------------------------------------------

(Updated Nov. 10, 2015, 2:27 p.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
-------

Updated based on BenM's review comments on r38876


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 (updated)
-----

  src/slave/http.cpp ce48a0584ab18a8d95dd02619f62df18b2276639 
  src/slave/slave.hpp ec2dfa99e6b553e2bcd82d12db915ae8625075a1 
  src/slave/slave.cpp 535adc3b17d5af3fe811a8e2505f126a28212dbf 

Diff: https://reviews.apache.org/r/38877/diff/


Testing
-------

make check


Thanks,

Anand Mazumdar


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/
-----------------------------------------------------------

(Updated Nov. 7, 2015, 1:07 a.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
-------

Modified Deps


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 80bda34e2b344b96d5d2f9c1fecd36046f67ab49 
  src/slave/slave.hpp d81a9c4d09d3f6be417120e412324258f19604a3 
  src/slave/slave.cpp e9f2d1bf7978450f0cd471bdb5606305092fc98a 

Diff: https://reviews.apache.org/r/38877/diff/


Testing
-------

make check


Thanks,

Anand Mazumdar


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/
-----------------------------------------------------------

(Updated Oct. 23, 2015, 1:39 a.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
-------

Rebase


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 (updated)
-----

  src/slave/http.cpp 80bda34e2b344b96d5d2f9c1fecd36046f67ab49 
  src/slave/slave.hpp d81a9c4d09d3f6be417120e412324258f19604a3 
  src/slave/slave.cpp e9f2d1bf7978450f0cd471bdb5606305092fc98a 

Diff: https://reviews.apache.org/r/38877/diff/


Testing
-------

make check


Thanks,

Anand Mazumdar


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/
-----------------------------------------------------------

(Updated Oct. 22, 2015, 8:48 p.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
-------

Rebased


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 (updated)
-----

  src/slave/http.cpp 80bda34e2b344b96d5d2f9c1fecd36046f67ab49 
  src/slave/slave.hpp d81a9c4d09d3f6be417120e412324258f19604a3 
  src/slave/slave.cpp e9f2d1bf7978450f0cd471bdb5606305092fc98a 

Diff: https://reviews.apache.org/r/38877/diff/


Testing
-------

make check


Thanks,

Anand Mazumdar


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/
-----------------------------------------------------------

(Updated Oct. 22, 2015, 5:53 a.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
-------

Rebased


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 (updated)
-----

  src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
  src/slave/slave.hpp d6e417b1ddde44a557e1e7fb3d22b4fd5bd0d526 
  src/slave/slave.cpp cb2d715d100bcfc76d80663ad13504bd278f1200 

Diff: https://reviews.apache.org/r/38877/diff/


Testing
-------

make check


Thanks,

Anand Mazumdar


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/
-----------------------------------------------------------

(Updated Oct. 20, 2015, 7:06 p.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
-------

Not send back a ShutDown() to the executor upon a retry subscribe request but just close the existing connection as per discussion with BenM.


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 (updated)
-----

  src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
  src/slave/slave.hpp d6e417b1ddde44a557e1e7fb3d22b4fd5bd0d526 
  src/slave/slave.cpp cb2d715d100bcfc76d80663ad13504bd278f1200 

Diff: https://reviews.apache.org/r/38877/diff/


Testing
-------

make check


Thanks,

Anand Mazumdar


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/
-----------------------------------------------------------

(Updated Oct. 16, 2015, 6:43 p.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
-------

Instead of directly querying the HTTP marker file in the agent code. Added a indirection to populate it via `RunState` as the PID workflow does


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 (updated)
-----

  src/slave/http.cpp f9cf7bbe81b7fe9637de9a8d66329c16a7e1a89b 
  src/slave/slave.hpp d6e417b1ddde44a557e1e7fb3d22b4fd5bd0d526 
  src/slave/slave.cpp cb2d715d100bcfc76d80663ad13504bd278f1200 

Diff: https://reviews.apache.org/r/38877/diff/


Testing
-------

make check


Thanks,

Anand Mazumdar


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/
-----------------------------------------------------------

(Updated Oct. 6, 2015, 4:39 p.m.)


Review request for mesos, Ben Mahler, Isabel Jimenez, and Vinod Kone.


Changes
-------

Moved `HTTP` executor checks outside else.


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 (updated)
-----

  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


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.

> On Oct. 6, 2015, 1:04 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2389
> > <https://reviews.apache.org/r/38877/diff/2/?file=1087669#file1087669line2389>
> >
> >     I think just print a number is not easy to understand when troubleshoot error happens.

Can you elaborate a bit more on this ?


- Anand


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


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
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by haosdent huang <ha...@gmail.com>.

> On Oct. 6, 2015, 1:04 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2389
> > <https://reviews.apache.org/r/38877/diff/2/?file=1087669#file1087669line2389>
> >
> >     I think just print a number is not easy to understand when troubleshoot error happens.
> 
> Anand Mazumdar wrote:
>     Can you elaborate a bit more on this ?

never mind, I just now found other places also just print `state`.


- haosdent


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


On Oct. 6, 2015, 4:39 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2015, 4:39 p.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
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/#review101568
-----------------------------------------------------------



src/slave/slave.cpp (line 96)
<https://reviews.apache.org/r/38877/#comment159002>

    Should put it before `using mesos::slave::QoSController;`?



src/slave/slave.cpp (line 2389)
<https://reviews.apache.org/r/38877/#comment159003>

    I think just print a number is not easy to understand when troubleshoot error happens.


- haosdent huang


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
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.

> 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
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/#review101615
-----------------------------------------------------------



src/slave/slave.cpp (line 2420)
<https://reviews.apache.org/r/38877/#comment159050>

    Should use return here directly instead of break?



src/slave/slave.cpp (line 2545)
<https://reviews.apache.org/r/38877/#comment159051>

    When execute is in unexpected state, we keep http connection open and not close it?



src/slave/slave.cpp (line 4244)
<https://reviews.apache.org/r/38877/#comment159052>

    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.


- haosdent huang


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
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Guangya Liu <gy...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/#review101085
-----------------------------------------------------------

Ship it!


Ship It!

- Guangya Liu


On 九月 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 九月 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
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.

> On Oct. 6, 2015, 6:20 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2479
> > <https://reviews.apache.org/r/38877/diff/2/?file=1087669#file1087669line2479>
> >
> >     Seems this line is incomplete.

It was intentionally left this way to avoid warning as error issues. This would be fixed when `MESOS-3476` is resolved. (Read comments before)


- Anand


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


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
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by haosdent huang <ha...@gmail.com>.

> On Oct. 6, 2015, 6:20 a.m., haosdent huang wrote:
> > src/slave/slave.cpp, line 2479
> > <https://reviews.apache.org/r/38877/diff/2/?file=1087669#file1087669line2479>
> >
> >     Seems this line is incomplete.
> 
> Anand Mazumdar wrote:
>     It was intentionally left this way to avoid warning as error issues. This would be fixed when `MESOS-3476` is resolved. (Read comments before)

got it, thx for explanation. I did not realize compiler would show error when update not used.


- haosdent


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


On Oct. 6, 2015, 4:39 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/38877/
> -----------------------------------------------------------
> 
> (Updated Oct. 6, 2015, 4:39 p.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
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by haosdent huang <ha...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/#review101614
-----------------------------------------------------------



src/slave/slave.cpp (line 2479)
<https://reviews.apache.org/r/38877/#comment159049>

    Seems this line is incomplete.


- haosdent huang


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
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Isabel Jimenez <co...@isabeljimenez.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/38877/#review101565
-----------------------------------------------------------

Ship it!


Ship It!

- Isabel Jimenez


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
> 
>


Re: Review Request 38877: Added functionality for Subscribe/Subscribed workflow for HTTP executors

Posted by Anand Mazumdar <ma...@gmail.com>.
-----------------------------------------------------------
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.


Changes
-------

Review comments from gyliu + Minor Cleanups


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 (updated)
-----

  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