You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by John Sirois <js...@apache.org> on 2016/02/02 17:30:57 UTC
Re: Review Request 43013: Move lifecycle documentation into separate
file
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43013/#review117408
-----------------------------------------------------------
This LGTM, some things noted are not yours so as you see fit.
docs/configuration-reference.md (line 330)
<https://reviews.apache.org/r/43013/#comment178588>
Checking my understanding - did you get this from here?: https://github.com/apache/aurora/blob/master/src/main/python/apache/aurora/executor/thermos_task_runner.py#L317-L318
docs/task-lifecycle.md (line 12)
<https://reviews.apache.org/r/43013/#comment178587>
A note that clarifies some states are missing from the diagram is probably in order as is a new issue to update the diagram unless you intend to keep https://issues.apache.org/jira/browse/AURORA-489 open for that second step.
docs/task-lifecycle.md (line 41)
<https://reviews.apache.org/r/43013/#comment178589>
s/how/that/
docs/task-lifecycle.md (line 51)
<https://reviews.apache.org/r/43013/#comment178590>
s/world. `Or/world.`, or/
docs/task-lifecycle.md (line 78)
<https://reviews.apache.org/r/43013/#comment178591>
This is out of date (If a ShellHealthChecker is configured then steps 2&3 are replaced by a new step 2), but this is also out of scope for your change here. As you see fit.
docs/task-lifecycle.md (line 113)
<https://reviews.apache.org/r/43013/#comment178592>
I'm not sure this sentence is needed. By definition the production bit, as described above, leads to production preference and pre-emption of non-production. "Important" is a bit of a loaded term.
docs/task-lifecycle.md (line 140)
<https://reviews.apache.org/r/43013/#comment178593>
Checking - the 1 hour figure comes from here?: https://github.com/apache/aurora/blob/master/src/main/java/org/apache/aurora/scheduler/reconciliation/ReconciliationModule.java#L78-L83
docs/user-guide.md
<https://reviews.apache.org/r/43013/#comment178595>
The [Job Lifecycle] could stay and be ppolated with a few sentences including a link off to the new task-lifecycle doc. Something like: Jobs have a rich lifecycle that is described here, but in day to day use, you'll be primarily concerned with launching new jobs and updating existing ones...
That would lead into the [Task Updates] and [HTTP Health Checking and Graceful Shutdown] a little more cleanly and provide a natural spot to note the more detailed docs.
- John Sirois
On Jan. 30, 2016, 1:41 p.m., Stephan Erb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43013/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2016, 1:41 p.m.)
>
>
> Review request for Aurora, Jay Buffington, John Sirois, Kevin Sweeney, and Brian Wickman.
>
>
> Bugs: AURORA-1068, AURORA-1262, AURORA-489, and AURORA-734
> https://issues.apache.org/jira/browse/AURORA-1068
> https://issues.apache.org/jira/browse/AURORA-1262
> https://issues.apache.org/jira/browse/AURORA-489
> https://issues.apache.org/jira/browse/AURORA-734
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In addition to the move, a couple of releted additions and adjustements have been made:
>
> * slight reorganization
> * documentation of missing states (THROTTELD, DRAINING)
> * custom section on reconciliation
> * remark regarding the uniqueness of an instance
> * updated documentation of the teardown of a task (HTTPLifecycleConfig and finalization_wait)
>
>
> Diffs
> -----
>
> docs/README.md 8ebc06121c1fd985027bebfab4f0c7123b66a6bb
> docs/configuration-reference.md 995f70654c89829a092e7b745a8390459244325d
> docs/task-lifecycle.md PRE-CREATION
> docs/user-guide.md df63468ddacb4a584938cd502004e47876347253
>
> Diff: https://reviews.apache.org/r/43013/diff/
>
>
> Testing
> -------
>
> Rendered version is available at https://github.com/StephanErb/aurora/blob/tasklifecycle/docs/task-lifecycle.md
>
>
> Thanks,
>
> Stephan Erb
>
>
Re: Review Request 43013: Move lifecycle documentation into separate
file
Posted by Stephan Erb <st...@dev.static-void.de>.
> On Feb. 2, 2016, 5:30 p.m., John Sirois wrote:
> > docs/configuration-reference.md, line 330
> > <https://reviews.apache.org/r/43013/diff/1/?file=1227169#file1227169line330>
> >
> > Checking my understanding - did you get this from here?: https://github.com/apache/aurora/blob/master/src/main/python/apache/aurora/executor/thermos_task_runner.py#L317-L318
I would not claim to fully grasp Thermos, but yes, that's probably it. As additional evidence we have this comment from Brian https://issues.apache.org/jira/browse/AURORA-734 and personal experience with misbehaving processes.
> On Feb. 2, 2016, 5:30 p.m., John Sirois wrote:
> > docs/task-lifecycle.md, line 140
> > <https://reviews.apache.org/r/43013/diff/1/?file=1227170#file1227170line140>
> >
> > Checking - the 1 hour figure comes from here?: https://github.com/apache/aurora/blob/master/src/main/java/org/apache/aurora/scheduler/reconciliation/ReconciliationModule.java#L78-L83
Yes.
> On Feb. 2, 2016, 5:30 p.m., John Sirois wrote:
> > docs/task-lifecycle.md, line 12
> > <https://reviews.apache.org/r/43013/diff/1/?file=1227170#file1227170line12>
> >
> > A note that clarifies some states are missing from the diagram is probably in order as is a new issue to update the diagram unless you intend to keep https://issues.apache.org/jira/browse/AURORA-489 open for that second step.
Added the note. I'll keep the linked issue open until the diagram is updated in a new review request coming soon-ish.
> On Feb. 2, 2016, 5:30 p.m., John Sirois wrote:
> > docs/task-lifecycle.md, line 41
> > <https://reviews.apache.org/r/43013/diff/1/?file=1227170#file1227170line41>
> >
> > s/how/that/
Done.
> On Feb. 2, 2016, 5:30 p.m., John Sirois wrote:
> > docs/task-lifecycle.md, line 51
> > <https://reviews.apache.org/r/43013/diff/1/?file=1227170#file1227170line51>
> >
> > s/world. `Or/world.`, or/
Done
> On Feb. 2, 2016, 5:30 p.m., John Sirois wrote:
> > docs/task-lifecycle.md, line 78
> > <https://reviews.apache.org/r/43013/diff/1/?file=1227170#file1227170line78>
> >
> > This is out of date (If a ShellHealthChecker is configured then steps 2&3 are replaced by a new step 2), but this is also out of scope for your change here. As you see fit.
AFAIK even if `ShellHealthChecker` is used for health checking, lifecycle is only controled via HTTP https://github.com/apache/aurora/blob/0d7f946f76e4600cec878e23d5b2a44702f9b4f6/src/main/python/apache/aurora/executor/http_lifecycle.py#L34
> On Feb. 2, 2016, 5:30 p.m., John Sirois wrote:
> > docs/task-lifecycle.md, line 113
> > <https://reviews.apache.org/r/43013/diff/1/?file=1227170#file1227170line113>
> >
> > I'm not sure this sentence is needed. By definition the production bit, as described above, leads to production preference and pre-emption of non-production. "Important" is a bit of a loaded term.
Dropped.
> On Feb. 2, 2016, 5:30 p.m., John Sirois wrote:
> > docs/user-guide.md, line 5
> > <https://reviews.apache.org/r/43013/diff/1/?file=1227171#file1227171line5>
> >
> > The [Job Lifecycle] could stay and be ppolated with a few sentences including a link off to the new task-lifecycle doc. Something like: Jobs have a rich lifecycle that is described here, but in day to day use, you'll be primarily concerned with launching new jobs and updating existing ones...
> >
> > That would lead into the [Task Updates] and [HTTP Health Checking and Graceful Shutdown] a little more cleanly and provide a natural spot to note the more detailed docs.
Good idea. to keep the user-guide intact for now.
My longterm goal is to get rid of the user-guide and replace it by a high-level description, a document on service discovery and a document on ports/healthchecking. But that will take more time and will not make it into 0.12.
- Stephan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/43013/#review117408
-----------------------------------------------------------
On Jan. 30, 2016, 9:41 p.m., Stephan Erb wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43013/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2016, 9:41 p.m.)
>
>
> Review request for Aurora, Jay Buffington, John Sirois, Kevin Sweeney, and Brian Wickman.
>
>
> Bugs: AURORA-1068, AURORA-1262, AURORA-489, and AURORA-734
> https://issues.apache.org/jira/browse/AURORA-1068
> https://issues.apache.org/jira/browse/AURORA-1262
> https://issues.apache.org/jira/browse/AURORA-489
> https://issues.apache.org/jira/browse/AURORA-734
>
>
> Repository: aurora
>
>
> Description
> -------
>
> In addition to the move, a couple of releted additions and adjustements have been made:
>
> * slight reorganization
> * documentation of missing states (THROTTELD, DRAINING)
> * custom section on reconciliation
> * remark regarding the uniqueness of an instance
> * updated documentation of the teardown of a task (HTTPLifecycleConfig and finalization_wait)
>
>
> Diffs
> -----
>
> docs/README.md 8ebc06121c1fd985027bebfab4f0c7123b66a6bb
> docs/configuration-reference.md 995f70654c89829a092e7b745a8390459244325d
> docs/task-lifecycle.md PRE-CREATION
> docs/user-guide.md df63468ddacb4a584938cd502004e47876347253
>
> Diff: https://reviews.apache.org/r/43013/diff/
>
>
> Testing
> -------
>
> Rendered version is available at https://github.com/StephanErb/aurora/blob/tasklifecycle/docs/task-lifecycle.md
>
>
> Thanks,
>
> Stephan Erb
>
>