You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@aurora.apache.org by Matthew Jeffryes <mj...@twitter.com> on 2014/08/16 01:26:18 UTC
Review Request 24752: combine finalization_wait when combining tasks
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24752/
-----------------------------------------------------------
Review request for Aurora, Bill Farner and Brian Wickman.
Repository: aurora
Description
-------
Problem: combine_task and concat_task helpers lose the finalization_wait information from all but the first task
Resolution: use sum/max of finialization_wait values in concat_task/combine_task
Diffs
-----
src/main/python/apache/thermos/config/schema_helpers.py 3feef02d110c154eb92533ccd597a6d517d074a2
src/test/python/apache/thermos/config/test_schema.py 85b04c0e4d9c29dd85b576b0c296f613879b31a7
Diff: https://reviews.apache.org/r/24752/diff/
Testing
-------
./pants src/test/python:all -vxs
Thanks,
Matthew Jeffryes
Re: Review Request 24752: combine finalization_wait when combining
tasks
Posted by Brian Wickman <wi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24752/#review50997
-----------------------------------------------------------
src/main/python/apache/thermos/config/schema_helpers.py
<https://reviews.apache.org/r/24752/#comment88877>
``reduce`` is removed in python 3.x. you could either do:
```python
try:
return max(map(cls.safe_get, waits))
except ValueError:
return 0
```
or create the intermediate list:
```python
waits = [cls.safe_get(wait) for wait in waits]
return max(waits) if waits else 0
```
both would be considered idiomatic.
- Brian Wickman
On Aug. 15, 2014, 11:26 p.m., Matthew Jeffryes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24752/
> -----------------------------------------------------------
>
> (Updated Aug. 15, 2014, 11:26 p.m.)
>
>
> Review request for Aurora, Bill Farner and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Problem: combine_task and concat_task helpers lose the finalization_wait information from all but the first task
> Resolution: use sum/max of finialization_wait values in concat_task/combine_task
>
>
> Diffs
> -----
>
> src/main/python/apache/thermos/config/schema_helpers.py 3feef02d110c154eb92533ccd597a6d517d074a2
> src/test/python/apache/thermos/config/test_schema.py 85b04c0e4d9c29dd85b576b0c296f613879b31a7
>
> Diff: https://reviews.apache.org/r/24752/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python:all -vxs
>
>
> Thanks,
>
> Matthew Jeffryes
>
>
Re: Review Request 24752: combine finalization_wait when combining
tasks
Posted by Brian Wickman <wi...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24752/#review54144
-----------------------------------------------------------
Ship it!
Sorry for taking so long to get to this review. The code looks good and I'll merge to master.
This comes with a catch -- a catch that we should document and file some tickets against.
The current state of affairs is that the default finalization is 30 seconds. Thermos supports something called the "preemption notice" which roughly translates to "how long we'll allow this task to finalize before we kill everything no matter what." The idea of preemption notice is that you may have somebody who needs the resources with a higher priority, but is only willing to wait a fixed amount of time for its predecessor to clean up. (More altruistic preemptors will make this value high if they don't need the resources immediately, e.g. a high priority batch job, but a revenue-critical task could very well make this 0 seconds and give you no opportunity to finalize.)
We never integrated preemption notice into Aurora because the use-case never materialized (also, for many, preemption is fairly rare.) Instead Thermos just uses a default of 60 seconds. This means that no matter how big you make the finalization wait, it can never be longer than 60 seconds before Thermos kills everything, so keep that in mind before you use this feature heavily.
- Brian Wickman
On Aug. 28, 2014, 3:27 p.m., Matthew Jeffryes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24752/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2014, 3:27 p.m.)
>
>
> Review request for Aurora, Bill Farner and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Problem: combine_task and concat_task helpers lose the finalization_wait information from all but the first task
> Resolution: use sum/max of finialization_wait values in concat_task/combine_task
>
>
> Diffs
> -----
>
> src/main/python/apache/thermos/config/schema_helpers.py 3feef02
> src/test/python/apache/thermos/config/test_schema.py 85b04c0
>
> Diff: https://reviews.apache.org/r/24752/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python:all -vxs
>
>
> Thanks,
>
> Matthew Jeffryes
>
>
Re: Review Request 24752: combine finalization_wait when combining
tasks
Posted by Bill Farner <wf...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24752/#review52088
-----------------------------------------------------------
Ping, Brian?
- Bill Farner
On Aug. 28, 2014, 3:27 p.m., Matthew Jeffryes wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24752/
> -----------------------------------------------------------
>
> (Updated Aug. 28, 2014, 3:27 p.m.)
>
>
> Review request for Aurora, Bill Farner and Brian Wickman.
>
>
> Repository: aurora
>
>
> Description
> -------
>
> Problem: combine_task and concat_task helpers lose the finalization_wait information from all but the first task
> Resolution: use sum/max of finialization_wait values in concat_task/combine_task
>
>
> Diffs
> -----
>
> src/main/python/apache/thermos/config/schema_helpers.py 3feef02
> src/test/python/apache/thermos/config/test_schema.py 85b04c0
>
> Diff: https://reviews.apache.org/r/24752/diff/
>
>
> Testing
> -------
>
> ./pants src/test/python:all -vxs
>
>
> Thanks,
>
> Matthew Jeffryes
>
>
Re: Review Request 24752: combine finalization_wait when combining
tasks
Posted by Matthew Jeffryes <mj...@twitter.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24752/
-----------------------------------------------------------
(Updated Aug. 28, 2014, 3:27 p.m.)
Review request for Aurora, Bill Farner and Brian Wickman.
Changes
-------
use an initial value with max to obviate the use of reduce.
Repository: aurora
Description
-------
Problem: combine_task and concat_task helpers lose the finalization_wait information from all but the first task
Resolution: use sum/max of finialization_wait values in concat_task/combine_task
Diffs (updated)
-----
src/main/python/apache/thermos/config/schema_helpers.py 3feef02
src/test/python/apache/thermos/config/test_schema.py 85b04c0
Diff: https://reviews.apache.org/r/24752/diff/
Testing
-------
./pants src/test/python:all -vxs
Thanks,
Matthew Jeffryes