Re: Review Request 17823: Add a utility to list missing shipits.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17823/#review34508 --- Ship it! Ship It! - Brian Wickman On Feb. 13, 2014, 10:59 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17823/ --- (Updated Feb. 13, 2014, 10:59 p.m.) Review request for Aurora and Brian Wickman. Repository: aurora Description --- Add a utility to list missing shipits. This is a tool to assist committers in finding review requests that require their attention (i.e. when the request is addressed to you explicitly and missing a public shipit from you). It uses the ReviewBoard API via ./rbt api-get (thus avoiding the need to wire in RB creds). Diffs - build-support/tools/list-missing-shipits PRE-CREATION Diff: https://reviews.apache.org/r/17823/diff/ Testing --- ~aurora git aurora/. kts/missing-shipits % ./build-support/list-missing-shipits kevints https://reviews.apache.org/r/17818/ jfarrellAURORA-145:Test dependencies leak into distribution https://reviews.apache.org/r/17785/ wfarner Ignore THROTTLED tasks when looking for tasks associated with a slave. https://reviews.apache.org/r/17562/ mansu Added cluster name to page title. https://reviews.apache.org/r/17303/ mansu Updated getJobs API to return task stats and latest task config https://reviews.apache.org/r/17347/ mansu Added non-prod consumption info to getQuota call. https://reviews.apache.org/r/17042/ weingartAdd a web interface to the zookeeper instance. https://reviews.apache.org/r/17026/ jonboulle update release tag script to support patch versions as well as minor versions ~aurora git aurora/. kts/missing-shipits % ./build-support/list-missing-shipits kevints | wc -l 7 Thanks, Kevin Sweeney
Re: Review Request 17877: AURORA-200: set_quota should die if given invalid arguments
On Feb. 11, 2014, 4:33 p.m., Mark Chu-Carroll wrote:
src/main/python/apache/aurora/client/commands/admin.py, line 174
https://reviews.apache.org/r/17877/diff/1/?file=481150#file481150line174
Die expects a message string, not an exception object. die('Invalid
unit specification') would end up with a better error message.
Oh yeah, I can see the the problem with not passing a string. I feel like the
default exception message provides a better explanation of what was wrong with
the argument though:
./pants py src/main/python/apache/aurora/client/bin:aurora_admin set_quota
example role 10 a b
FATAL] Invalid size: Amount did not have a valid base: a. Valid bases: kb gb g
mb tb k m t
- Dan
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17877/#review34193
---
On Feb. 10, 2014, 7:17 p.m., Dan Norris wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17877/
---
(Updated Feb. 10, 2014, 7:17 p.m.)
Review request for Aurora, Jonathan Boulle and Mark Chu-Carroll.
Bugs: AURORA-200
https://issues.apache.org/jira/browse/AURORA-200
Repository: aurora
Description
---
AURORA-200: set_quota should die if given invalid arguments
Diffs
-
src/main/python/apache/aurora/client/commands/admin.py
45686aec8d69f0dfa1d649a92a19ca87bd315823
Diff: https://reviews.apache.org/r/17877/diff/
Testing
---
./gradlew clean build ./pants src/test/python:all -vxs along with manual
testing w/ bad arguments.
Thanks,
Dan Norris
Re: Review Request 17752: Add task noun, supporting run and ssh verbs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17752/#review34515 --- src/main/python/apache/aurora/client/cli/options.py https://reviews.apache.org/r/17752/#comment64612 Minor nit: I think the original version of this was --user. Also since we're mirroring the -L form for creating tunnels, consider adding the '-l' short form for --ssh_user. src/main/python/apache/aurora/client/cli/task.py https://reviews.apache.org/r/17752/#comment64613 any reason why '-t' short form is removed? for me '-t50' is almost muscle memory. - Brian Wickman On Feb. 12, 2014, 2:16 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17752/ --- (Updated Feb. 12, 2014, 2:16 a.m.) Review request for Aurora, Bill Farner and Brian Wickman. Bugs: aurora-124 https://issues.apache.org/jira/browse/aurora-124 Repository: aurora Description --- Add task noun, supporting run and ssh verbs. Diffs - src/main/python/apache/aurora/client/cli/BUILD f5c9ae75d7a26883d90b9bd8ac9c80cd436d03a1 src/main/python/apache/aurora/client/cli/client.py e416d3879cc5943ec5ab3931a8b1aedb85a379a9 src/main/python/apache/aurora/client/cli/jobs.py caff6d824d9f43cc9eb0c738ea6bb489321d4669 src/main/python/apache/aurora/client/cli/options.py 5d6eba2abc55954fddb0c8bc1fe4de4f6d089962 src/main/python/apache/aurora/client/cli/task.py PRE-CREATION src/test/python/apache/aurora/client/cli/BUILD c106b97a0902da5cd791b18117da3198bfb43b8c src/test/python/apache/aurora/client/cli/test_task_run.py PRE-CREATION Diff: https://reviews.apache.org/r/17752/diff/ Testing --- [sun-wukong incubator-aurora (tasknoun)]$ ./pants src/test/python/apache/aurora/client/cli:all Build operating on targets: OrderedSet([PythonTestSuite(src/test/python/apache/aurora/client/cli/BUILD:all)]) = test session starts == platform darwin -- Python 2.7.5 -- py-1.4.20 -- pytest-2.5.2 collected 25 items src/test/python/apache/aurora/client/cli/test_cancel_update.py .. src/test/python/apache/aurora/client/cli/test_create.py src/test/python/apache/aurora/client/cli/test_diff.py ... src/test/python/apache/aurora/client/cli/test_kill.py . src/test/python/apache/aurora/client/cli/test_restart.py ... src/test/python/apache/aurora/client/cli/test_status.py . src/test/python/apache/aurora/client/cli/test_update.py ... == 25 passed in 1.37 seconds === = test session starts == platform darwin -- Python 2.7.5 -- py-1.4.20 -- pytest-2.5.2 collected 2 items src/test/python/apache/aurora/client/cli/test_task_run.py .. === 2 passed in 0.30 seconds === src.test.python.apache.aurora.client.cli.job . SUCCESS src.test.python.apache.aurora.client.cli.task . SUCCESS Thanks, Mark Chu-Carroll
Re: Review Request 17948: Implement help message generation for the noun/verb framework.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17948/#review34509 --- src/main/python/apache/aurora/client/cli/__init__.py https://reviews.apache.org/r/17948/#comment64606 minor nit -- we usually prefer never to shadow builtins even though in this case it's harmless. i'd s/str/msg/ just to be consistent with our practices elsewhere. src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/17948/#comment64609 I'm not sure how I feel about the help property returning manually split doc lines. It probably makes more sense to just return a docstring style string -- which you can then always do .splitlines() on. (Alternately, you can do isinstance(str) elif isinstance(list) I suppose.) At the end of the day, if you end up on smaller or larger terminals, you'll probably end up using the textwrap module to change the line breaks here programmatically anyway. - Brian Wickman On Feb. 11, 2014, 3:31 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/17948/ --- (Updated Feb. 11, 2014, 3:31 p.m.) Review request for Aurora, David Robinson and Brian Wickman. Bugs: aurora-202 https://issues.apache.org/jira/browse/aurora-202 Repository: aurora Description --- Implement help message generation for the noun/verb framework. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 9eb5c52097f0e75567977ba2d4fe10932d227a4b src/main/python/apache/aurora/client/cli/client.py e416d3879cc5943ec5ab3931a8b1aedb85a379a9 src/main/python/apache/aurora/client/cli/jobs.py caff6d824d9f43cc9eb0c738ea6bb489321d4669 src/main/python/apache/aurora/client/cli/options.py 5d6eba2abc55954fddb0c8bc1fe4de4f6d089962 src/main/python/apache/aurora/client/cli/quota.py a7bcfbe0100fe8e400abda3710519e3e5029c477 src/test/python/apache/aurora/client/cli/BUILD c106b97a0902da5cd791b18117da3198bfb43b8c src/test/python/apache/aurora/client/cli/test_help.py PRE-CREATION Diff: https://reviews.apache.org/r/17948/diff/ Testing --- Added unit tests of new functions; verified that all client tests pass. Thanks, Mark Chu-Carroll
Re: Review Request 18042: Adding sla get_task_up_count command.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18042/ --- (Updated Feb. 14, 2014, 6 p.m.) Review request for Aurora, Mark Chu-Carroll and Brian Wickman. Changes --- CR comments. Bugs: AURORA-206 https://issues.apache.org/jira/browse/AURORA-206 Repository: aurora Description --- Implementing api and cli logic for the get_task_up_count command. Diffs (updated) - src/main/python/apache/aurora/client/api/BUILD 69229a87698ed7e99957cd74364bb2ed6b93c6a3 src/main/python/apache/aurora/client/api/__init__.py 8bc05e20e34aa3266c2a9c5f24a1cfec8adaf24d src/main/python/apache/aurora/client/api/sla.py PRE-CREATION src/main/python/apache/aurora/client/cli/BUILD f5c9ae75d7a26883d90b9bd8ac9c80cd436d03a1 src/main/python/apache/aurora/client/cli/client.py e416d3879cc5943ec5ab3931a8b1aedb85a379a9 src/main/python/apache/aurora/client/cli/options.py 5d6eba2abc55954fddb0c8bc1fe4de4f6d089962 src/main/python/apache/aurora/client/cli/sla.py PRE-CREATION src/test/python/apache/aurora/client/api/BUILD 9833535bd6a25d6409abe584056152e1ac84548e src/test/python/apache/aurora/client/api/test_sla.py PRE-CREATION src/test/python/apache/aurora/client/cli/BUILD c106b97a0902da5cd791b18117da3198bfb43b8c src/test/python/apache/aurora/client/cli/test_sla.py PRE-CREATION Diff: https://reviews.apache.org/r/18042/diff/ Testing --- ./pants src/test/python/apache/aurora/client:all Thanks, Maxim Khutornenko
Re: Review Request 18141: Prepare and launch GC executor tasks asynchronously.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18141/#review34542 --- src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java https://reviews.apache.org/r/18141/#comment64654 This comment seems more appropriate for the SchedulerModule where the list is created. src/main/java/org/apache/aurora/scheduler/periodic/GcExecutorLauncher.java https://reviews.apache.org/r/18141/#comment64655 Any particular reason you have two makeGcTask overloads? Is it just to make unit tests easier? src/main/java/org/apache/aurora/scheduler/periodic/GcExecutorLauncher.java https://reviews.apache.org/r/18141/#comment64656 With the async execution there is a potential of leaking offers in case task creation starts failing for any reason. Would it make sense to have a separate stat counter (e.g. scheduler_gc_offers_used) to help troubleshooting such cases? Any difference between it and the scheduler_gc_tasks_created would be a red flag. src/main/java/org/apache/aurora/scheduler/periodic/GcExecutorLauncher.java https://reviews.apache.org/r/18141/#comment64657 s/public// src/main/java/org/apache/aurora/scheduler/periodic/GcExecutorLauncher.java https://reviews.apache.org/r/18141/#comment64658 @Override - Maxim Khutornenko On Feb. 14, 2014, 8:09 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18141/ --- (Updated Feb. 14, 2014, 8:09 p.m.) Review request for Aurora, Deprecated Use kevints and Maxim Khutornenko. Bugs: AURORA-214 https://issues.apache.org/jira/browse/AURORA-214 Repository: aurora Description --- This change also alters the signature to and renames TaskLauncher#createTask, since no remaining TaskLaunchers return a task. The signature change resulted in removing MesosSchedulerImpl#fitsInOffer, which turned out to be redundant in practice (GcExecutorLauncher did this internally). Diffs - src/main/java/org/apache/aurora/scheduler/MesosSchedulerImpl.java 70ac62e6ba3566a8789f1c7b2042b8a94f5f3c02 src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 92399cc7a38f0ddce8338d127fb7b579606f2571 src/main/java/org/apache/aurora/scheduler/TaskLauncher.java 96a3adee537aa87402a39dba090c6bbebe6afb1f src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 010776efe1620c13bbcff611f9c1c3272d6c776f src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e7a5a8377f4c8a42012e7ecb118c597a2804da40 src/main/java/org/apache/aurora/scheduler/periodic/GcExecutorLauncher.java f0d4fbcc411dcb3642c21f51f65c89ad24c3400a src/test/java/org/apache/aurora/scheduler/MesosSchedulerImplTest.java 92c77d53f09c39e710bd1bb3277f32d1e144e62f src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java ecf4f90c7d635f912f90fb367dbaa11401048052 src/test/java/org/apache/aurora/scheduler/periodic/GcExecutorLauncherTest.java 98f5aa141ecca7475274d84f50c750db6f2908b5 Diff: https://reviews.apache.org/r/18141/diff/ Testing --- $ ./gradlew build Thanks, Bill Farner
Re: Review Request 18149: Cron predictor conformance test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18149/#review34555 --- Ship it! This is great, thanks!! - Bill Farner On Feb. 14, 2014, 11:43 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/18149/ --- (Updated Feb. 14, 2014, 11:43 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-132 https://issues.apache.org/jira/browse/AURORA-132 Repository: aurora Description --- Cron predictor conformance test. Quartz doesn't support standard cron syntax (as described in crontab(5)[1]) and deviates [2] in a lot of ways that would lock us into it. Some of the ways it varies include: * '00' is not a valid minute * day-of-week is 1-indexed instead of 0-indexed * it supports a seconds field, which is an unrealistic constraint for aurora to deliver * the common * * * * * pattern is invalid Rather than adopt Quartz's syntax I propose introducing a translation layer that will allow users to write in the more familiar crontab syntax. This change adds test data that can be used as a conformance test - a valid CronPredictor implementation must both accept the same syntax and make the same predictions as those referenced here. In a future review I will introduce a Quartz cron subsystem that passes this test. [1] http://unixhelp.ed.ac.uk/CGI/man-cgi?crontab+5 [2] http://quartz-scheduler.org/documentation/quartz-2.2.x/tutorials/tutorial-lesson-06 Diffs - src/main/java/org/apache/aurora/scheduler/cron/testing/AbstractCronIT.java d8c40222809f525ce485a002873f1fe9dedc2748 src/main/resources/org/apache/aurora/scheduler/cron/testing/cron-schedule-predictions.json PRE-CREATION Diff: https://reviews.apache.org/r/18149/diff/ Testing --- ./gradlew build Thanks, Kevin Sweeney
Re: Review Request 18157: Compute task host attribute aggregates once when scheduling tasks.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18157/#review34561
---
Ship it!
src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java
https://reviews.apache.org/r/18157/#comment64685
Double space.
src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java
https://reviews.apache.org/r/18157/#comment64686
Having a comment deciphering key/value here would be helpful.
src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java
https://reviews.apache.org/r/18157/#comment64690
Typo.
src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java
https://reviews.apache.org/r/18157/#comment64689
Would it make sense to move getHostAttributes() outside of the Supplier
initialization? I find it hard to follow the logic as it is.
- Maxim Khutornenko
On Feb. 15, 2014, 12:21 a.m., Bill Farner wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/18157/
---
(Updated Feb. 15, 2014, 12:21 a.m.)
Review request for Aurora, Suman Karumuri and Maxim Khutornenko.
Repository: aurora
Description
---
Addresses TODO from CachedJobState:
33 * TODO(wfarner): Take this caching one step further and don't store
tasks here, but instead store
34 * pre-computed aggregates of host attributes. For example, translate
each IScheduledTask into
35 * the HostAttributes of the machine it resides on, and count the number
of times each attribute is
36 * seen. This could be represented in a preMapString,
MultisetString/pre, where
37 * the outer key is the attribute name, and the inner key is attribute
values. So, for a job with
38 * two tasks on the same rack but different hosts, you could have this
aggregate:
39 * pre
40 * { host: {hostA: 1, hostB: 1},
41 * rack: {rack1: 2}
42 * }
43 * /pre
The bulk of this change is with renaming/rewriting CachedJobState (now called
AttributeAggregate) and consuming from AttributeFilter. The rest is ripple
in wiring.
I expect this will make scheduling tasks in large jobs less expensive, since
we would previously iterate through all tasks multiple times for each offer.
Diffs
-
src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java
e7a5a8377f4c8a42012e7ecb118c597a2804da40
src/main/java/org/apache/aurora/scheduler/async/Preemptor.java
7893e55f83c3285b37a4e20e88a7bd31728253f3
src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java
61385c6bbcddc6fd03498190708c9e49892d0859
src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
d2be7ace46ca59612368ef70239877a4a1ee72fb
src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java
3e74135b1dcff2c42bd10b558e3699e70fd7c6be
src/main/java/org/apache/aurora/scheduler/filter/CachedJobState.java
337a5e7c045b3338056d17541d6aec0b2d84c9cb
src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java
c2b9f047b199aa24849c3a9eb1d8176f13fade07
src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java
ddb34cfc2c476ad6266e2d04a626c391f33d2592
src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java
93355c3e9a38bb581d9080a492ec786509158d96
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java
3154ef04e211e0078144e14091e0ceb1470231b1
src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java
dc371e0b0ef66c254c6db5b274ddf81ccfbd9ff3
src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java
94647832c01cbf0b2d8d44f7e40fb1b5cc68fd91
src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java
4112d6133ad26344696237c180f7cc045da4f4f2
src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
4ac059cf0eab2370938dc388dbfd86ffb086a248
src/test/java/org/apache/aurora/scheduler/filter/AttributeAggregateTest.java
PRE-CREATION
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
56de3aae4e08138a66a587e2a43d64a2310079f1
src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java
f0fdd571f273be8dad1f9410b0e70e3a11e8133b
Diff: https://reviews.apache.org/r/18157/diff/
Testing
---
./gradlew build
Thanks,
Bill Farner
