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/12/16 19:28:40 UTC

Review Request 41454: Added initial draft of executor HTTP API user doc.

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

Review request for mesos and Vinod Kone.


Repository: mesos


Description
-------

This is an initial draft for the user doc of Executor HTTP API. Some details might change due to the pending executor library review chain: https://reviews.apache.org/r/41275


Diffs
-----

  docs/executor-http-api.md PRE-CREATION 
  docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 

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


Testing
-------

https://gist.github.com/hatred/d35fdaa667203bf34a78


Thanks,

Anand Mazumdar


Re: Review Request 41454: Added initial draft of executor HTTP API user doc.

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

> On Jan. 5, 2016, 12:20 a.m., Vinod Kone wrote:
> > docs/executor-http-api.md, lines 52-70
> > <https://reviews.apache.org/r/41454/diff/3/?file=1167954#file1167954line52>
> >
> >     looking at just the JSON, it's not very clear that `tasks` and `updates` correspond to unacknowledged tasks and updates. we should probably rename these fields as `unacknowledged_tasks` and `unacknowledged_updates`. what do you think?

sgtm, modified the name of the fields as per our discussion.


> On Jan. 5, 2016, 12:20 a.m., Vinod Kone wrote:
> > docs/executor-http-api.md, line 131
> > <https://reviews.apache.org/r/41454/diff/3/?file=1167954#file1167954line131>
> >
> >     Is the retry by executor necessary? The current driver doesn't do retries for example. Are you anticipating a scenario where the executor is connected but messages are lost?

My bad, Fixed. This got copied over verbatim from the corresponding scheduler docs.


> On Jan. 5, 2016, 12:20 a.m., Vinod Kone wrote:
> > docs/executor-http-api.md, line 343
> > <https://reviews.apache.org/r/41454/diff/3/?file=1167954#file1167954line343>
> >
> >     what's the relation between MESOS_RETRY_INTERVAL_MAX and the duration after which the agent deems recovery process complete (EXECUTOR_REREGISTER_TIMEOUT)? AFAICT, currently the former is set to 15 min and the latter is 2s.

As per our discussion, I modified the name of the variable to `EXECUTOR_REGISTRATION_BACKOFF_MAX`. This variable is equal in value to `EXECUTOR_REREGISTER_TIMEOUT`. 

It essentially denotes that upon agent recovery, the agent would wait for `EXECUTOR_REREGISTER_TIMEOUT` before killing the executor and marking the recovery process as complete. Hence, the executor should retry atleast once within `EXECUTOR_REGISTRATION_BACKOFF_MAX` to avoid being killed.

Also, let me know if this new name looks good to you.


- Anand


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


On Jan. 5, 2016, 9:28 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41454/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2016, 9:28 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4177
>     https://issues.apache.org/jira/browse/MESOS-4177
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is an initial draft for the user doc of Executor HTTP API. Some details might change due to the pending executor library review chain: https://reviews.apache.org/r/41275
> 
> 
> Diffs
> -----
> 
>   docs/executor-http-api.md PRE-CREATION 
>   docs/home.md d929838206817a6c49cc2343b4de82fa085da682 
> 
> Diff: https://reviews.apache.org/r/41454/diff/
> 
> 
> Testing
> -------
> 
> https://gist.github.com/hatred/d35fdaa667203bf34a78
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 41454: Added initial draft of executor HTTP API user doc.

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



docs/executor-http-api.md (line 14)
<https://reviews.apache.org/r/41454/#comment173138>

    put 4 spaces so that you get a shaded block around this. see https://github.com/apache/mesos/blob/master/docs/scheduler-http-api.md for reference.



docs/executor-http-api.md (line 30)
<https://reviews.apache.org/r/41454/#comment173139>

    s/Alternatively/Additonally/



docs/executor-http-api.md (line 31)
<https://reviews.apache.org/r/41454/#comment173140>

    "The executor is expected to keep running...". This sentence should be at the top of these two sub points. Also, the executor should keep running only if checkpointing is enabled right?
    
    "These status update messages...". How about:
    "The executor is expected to keep track of status updates not acknowledged by the agent via the ACKNOWLEDGE events."



docs/executor-http-api.md (line 32)
<https://reviews.apache.org/r/41454/#comment173141>

    there should be no network intermediaries between an agent and an executor!
    
    can you just re-use the comment in exec.cpp above `tasks` map instead?



docs/executor-http-api.md (lines 52 - 70)
<https://reviews.apache.org/r/41454/#comment173142>

    looking at just the JSON, it's not very clear that `tasks` and `updates` correspond to unacknowledged tasks and updates. we should probably rename these fields as `unacknowledged_tasks` and `unacknowledged_updates`. what do you think?



docs/executor-http-api.md (line 131)
<https://reviews.apache.org/r/41454/#comment173144>

    Is the retry by executor necessary? The current driver doesn't do retries for example. Are you anticipating a scenario where the executor is connected but messages are lost?



docs/executor-http-api.md (line 171)
<https://reviews.apache.org/r/41454/#comment173145>

    s/message to the executor/message to the scheduler/



docs/executor-http-api.md (line 211)
<https://reviews.apache.org/r/41454/#comment173147>

    s/messages/tasks/



docs/executor-http-api.md (line 247)
<https://reviews.apache.org/r/41454/#comment173148>

    s/the resources/the task resources/



docs/executor-http-api.md (line 278)
<https://reviews.apache.org/r/41454/#comment173149>

    Mention that `data` is raw bytes encoded in Base64?



docs/executor-http-api.md (line 293)
<https://reviews.apache.org/r/41454/#comment173150>

    Also mention that TASK_LOST updates will be sent by the slave for any active tasks.



docs/executor-http-api.md (lines 332 - 333)
<https://reviews.apache.org/r/41454/#comment173192>

    didn't we agree to rename these?



docs/executor-http-api.md (line 335)
<https://reviews.apache.org/r/41454/#comment173193>

    s/all agent/all the agent's/



docs/executor-http-api.md (line 343)
<https://reviews.apache.org/r/41454/#comment173195>

    what's the relation between MESOS_RETRY_INTERVAL_MAX and the duration after which the agent deems recovery process complete (EXECUTOR_REREGISTER_TIMEOUT)? AFAICT, currently the former is set to 15 min and the latter is 2s.



docs/executor-http-api.md (line 350)
<https://reviews.apache.org/r/41454/#comment173194>

    s/then kill/then forcefully kill/
    
    s/in forcefully/in/


- Vinod Kone


On Dec. 17, 2015, 1:07 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41454/
> -----------------------------------------------------------
> 
> (Updated Dec. 17, 2015, 1:07 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4177
>     https://issues.apache.org/jira/browse/MESOS-4177
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is an initial draft for the user doc of Executor HTTP API. Some details might change due to the pending executor library review chain: https://reviews.apache.org/r/41275
> 
> 
> Diffs
> -----
> 
>   docs/executor-http-api.md PRE-CREATION 
>   docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 
> 
> Diff: https://reviews.apache.org/r/41454/diff/
> 
> 
> Testing
> -------
> 
> https://gist.github.com/hatred/d35fdaa667203bf34a78
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 41454: Added initial draft of executor HTTP API user doc.

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



docs/executor-http-api.md (line 33)
<https://reviews.apache.org/r/41454/#comment173447>

    Can you replace this with:
    
    * **Unacknowledged Tasks**:
    
    The executor is expected to maintain a list of tasks that have not been acknowledged by the agent. A task is considered acknowledged if atleast one of the status updates for this task is acknowledged by the slave.



docs/executor-http-api.md (line 334)
<https://reviews.apache.org/r/41454/#comment173452>

    lets call this
    
    MESOS_RECOVERY_TIMEOUT like what we currently have in the driver. I think it makes it obvious that this is the same as --recovery_timeout set on the agent.



docs/executor-http-api.md (line 335)
<https://reviews.apache.org/r/41454/#comment173453>

    Lets keep the MESOS prefix like you did for all the above. Sorry for the run around.
    
    MESOS_SUBSCRIPTION_BACKOFF_MAX


- Vinod Kone


On Jan. 5, 2016, 9:28 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41454/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2016, 9:28 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4177
>     https://issues.apache.org/jira/browse/MESOS-4177
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is an initial draft for the user doc of Executor HTTP API. Some details might change due to the pending executor library review chain: https://reviews.apache.org/r/41275
> 
> 
> Diffs
> -----
> 
>   docs/executor-http-api.md PRE-CREATION 
>   docs/home.md d929838206817a6c49cc2343b4de82fa085da682 
> 
> Diff: https://reviews.apache.org/r/41454/diff/
> 
> 
> Testing
> -------
> 
> https://gist.github.com/hatred/d35fdaa667203bf34a78
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 41454: Added initial draft of executor HTTP API user doc.

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

Ship it!


Ship It!

- Vinod Kone


On Jan. 6, 2016, 12:55 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41454/
> -----------------------------------------------------------
> 
> (Updated Jan. 6, 2016, 12:55 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4177
>     https://issues.apache.org/jira/browse/MESOS-4177
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is an initial draft for the user doc of Executor HTTP API. Some details might change due to the pending executor library review chain: https://reviews.apache.org/r/41275
> 
> 
> Diffs
> -----
> 
>   docs/executor-http-api.md PRE-CREATION 
>   docs/home.md d929838206817a6c49cc2343b4de82fa085da682 
> 
> Diff: https://reviews.apache.org/r/41454/diff/
> 
> 
> Testing
> -------
> 
> https://gist.github.com/hatred/d35fdaa667203bf34a78
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 41454: Added initial draft of executor HTTP API user doc.

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

(Updated Jan. 6, 2016, 12:55 a.m.)


Review request for mesos and Vinod Kone.


Changes
-------

Review comments from Vinod


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


Repository: mesos


Description
-------

This is an initial draft for the user doc of Executor HTTP API. Some details might change due to the pending executor library review chain: https://reviews.apache.org/r/41275


Diffs (updated)
-----

  docs/executor-http-api.md PRE-CREATION 
  docs/home.md d929838206817a6c49cc2343b4de82fa085da682 

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


Testing
-------

https://gist.github.com/hatred/d35fdaa667203bf34a78


Thanks,

Anand Mazumdar


Re: Review Request 41454: Added initial draft of executor HTTP API user doc.

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

(Updated Jan. 5, 2016, 9:28 p.m.)


Review request for mesos and Vinod Kone.


Changes
-------

Review comments from Vinod


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


Repository: mesos


Description
-------

This is an initial draft for the user doc of Executor HTTP API. Some details might change due to the pending executor library review chain: https://reviews.apache.org/r/41275


Diffs (updated)
-----

  docs/executor-http-api.md PRE-CREATION 
  docs/home.md d929838206817a6c49cc2343b4de82fa085da682 

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


Testing
-------

https://gist.github.com/hatred/d35fdaa667203bf34a78


Thanks,

Anand Mazumdar


Re: Review Request 41454: Added initial draft of executor HTTP API user doc.

Posted by Mesos ReviewBot <re...@mesos.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/41454/#review110876
-----------------------------------------------------------


Patch looks great!

Reviews applied: [41454]

Passed command: export OS=ubuntu:14.04;export CONFIGURATION="--verbose";export COMPILER=gcc; ./support/docker_build.sh

- Mesos ReviewBot


On Dec. 17, 2015, 1:07 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41454/
> -----------------------------------------------------------
> 
> (Updated Dec. 17, 2015, 1:07 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4177
>     https://issues.apache.org/jira/browse/MESOS-4177
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is an initial draft for the user doc of Executor HTTP API. Some details might change due to the pending executor library review chain: https://reviews.apache.org/r/41275
> 
> 
> Diffs
> -----
> 
>   docs/executor-http-api.md PRE-CREATION 
>   docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 
> 
> Diff: https://reviews.apache.org/r/41454/diff/
> 
> 
> Testing
> -------
> 
> https://gist.github.com/hatred/d35fdaa667203bf34a78
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 41454: Added initial draft of executor HTTP API user doc.

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

(Updated Dec. 17, 2015, 1:07 a.m.)


Review request for mesos and Vinod Kone.


Changes
-------

Review comments


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


Repository: mesos


Description
-------

This is an initial draft for the user doc of Executor HTTP API. Some details might change due to the pending executor library review chain: https://reviews.apache.org/r/41275


Diffs (updated)
-----

  docs/executor-http-api.md PRE-CREATION 
  docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 

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


Testing
-------

https://gist.github.com/hatred/d35fdaa667203bf34a78


Thanks,

Anand Mazumdar


Re: Review Request 41454: Added initial draft of executor HTTP API user doc.

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

> On Dec. 17, 2015, 12:32 a.m., Guangya Liu wrote:
> > docs/executor-http-api.md, line 65
> > <https://reviews.apache.org/r/41454/diff/2/?file=1167046#file1167046line65>
> >
> >     Does the "\" relly needed?

Escaping makes it valid JSON, no ?


> On Dec. 17, 2015, 12:32 a.m., Guangya Liu wrote:
> > docs/executor-http-api.md, line 103
> > <https://reviews.apache.org/r/41454/diff/2/?file=1167046#file1167046line103>
> >
> >     ditto

Ditto as above.


- Anand


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


On Dec. 17, 2015, 1:07 a.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41454/
> -----------------------------------------------------------
> 
> (Updated Dec. 17, 2015, 1:07 a.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4177
>     https://issues.apache.org/jira/browse/MESOS-4177
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is an initial draft for the user doc of Executor HTTP API. Some details might change due to the pending executor library review chain: https://reviews.apache.org/r/41275
> 
> 
> Diffs
> -----
> 
>   docs/executor-http-api.md PRE-CREATION 
>   docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 
> 
> Diff: https://reviews.apache.org/r/41454/diff/
> 
> 
> Testing
> -------
> 
> https://gist.github.com/hatred/d35fdaa667203bf34a78
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 41454: Added initial draft of executor HTTP API user doc.

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



docs/executor-http-api.md (line 65)
<https://reviews.apache.org/r/41454/#comment170804>

    Does the "\" relly needed?



docs/executor-http-api.md (line 103)
<https://reviews.apache.org/r/41454/#comment170805>

    ditto



docs/executor-http-api.md (line 131)
<https://reviews.apache.org/r/41454/#comment170808>

    What about 
    
    see ACKNOWLEDGE in the Events section below for the semantics.



docs/executor-http-api.md (line 160)
<https://reviews.apache.org/r/41454/#comment170810>

    Not a UUID


- Guangya Liu


On Dec. 16, 2015, 8:40 p.m., Anand Mazumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/41454/
> -----------------------------------------------------------
> 
> (Updated Dec. 16, 2015, 8:40 p.m.)
> 
> 
> Review request for mesos and Vinod Kone.
> 
> 
> Bugs: MESOS-4177
>     https://issues.apache.org/jira/browse/MESOS-4177
> 
> 
> Repository: mesos
> 
> 
> Description
> -------
> 
> This is an initial draft for the user doc of Executor HTTP API. Some details might change due to the pending executor library review chain: https://reviews.apache.org/r/41275
> 
> 
> Diffs
> -----
> 
>   docs/executor-http-api.md PRE-CREATION 
>   docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 
> 
> Diff: https://reviews.apache.org/r/41454/diff/
> 
> 
> Testing
> -------
> 
> https://gist.github.com/hatred/d35fdaa667203bf34a78
> 
> 
> Thanks,
> 
> Anand Mazumdar
> 
>


Re: Review Request 41454: Added initial draft of executor HTTP API user doc.

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

(Updated Dec. 16, 2015, 8:40 p.m.)


Review request for mesos and Vinod Kone.


Changes
-------

Fixed character encoding


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


Repository: mesos


Description
-------

This is an initial draft for the user doc of Executor HTTP API. Some details might change due to the pending executor library review chain: https://reviews.apache.org/r/41275


Diffs (updated)
-----

  docs/executor-http-api.md PRE-CREATION 
  docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 

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


Testing
-------

https://gist.github.com/hatred/d35fdaa667203bf34a78


Thanks,

Anand Mazumdar


Re: Review Request 41454: Added initial draft of executor HTTP API user doc.

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

(Updated Dec. 16, 2015, 7:38 p.m.)


Review request for mesos and Vinod Kone.


Changes
-------

Added the corresponding JIRA link


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


Repository: mesos


Description
-------

This is an initial draft for the user doc of Executor HTTP API. Some details might change due to the pending executor library review chain: https://reviews.apache.org/r/41275


Diffs
-----

  docs/executor-http-api.md PRE-CREATION 
  docs/home.md a01612ec8347eb8a7a9277b829365b6c3a1fe9e3 

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


Testing
-------

https://gist.github.com/hatred/d35fdaa667203bf34a78


Thanks,

Anand Mazumdar