Review Request 26269: Remove UI from page titles.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26269/ --- Review request for Aurora and David McLaughlin. Repository: aurora Description --- Aurora UI is a weird thing to call a website. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 40617aa5f84170abe8e2a2b8bc06913f90ae8eb0 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 3a07ce7927e936d2d30b8e19b2362a073372ad7b Diff: https://reviews.apache.org/r/26269/diff/ Testing --- Thanks, Bill Farner
Review Request 26270: Make the client invocation code actually use the exit codes returned by the client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26270/ --- Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-781 https://issues.apache.org/jira/browse/aurora-781 Repository: aurora Description --- Fix multiple errors involving bindings, and fix result codes. - Cause an error to be raised by an unresolved binding in a configuration file. - Fix incorrect structuring of processed bindings. - Add a test to confirm that unbound parameters generate errors. While I'm in the code, also fix a problem where result codes aren't returned correctly to the shell. Diffs - src/main/python/apache/aurora/client/cli/client.py 0cb69448cd24372067ac845eca5862bc3d3a46a9 src/main/python/apache/aurora/client/cli/context.py 102d20797816788361dfdac450aac9fb8e6fbc28 src/main/python/apache/aurora/client/cli/options.py c2d422ac2bc82fc387596e93040b49f722f8310f src/main/python/apache/aurora/client/cli/standalone_client.py 30d8f750559b7811d66760741905fa8adf80fd1f src/test/python/apache/aurora/client/cli/test_create.py 31fa56f5edcfc97903725ab27ccc12c6a8f39ffc src/test/python/apache/aurora/client/cli/util.py a50b83c571390374975accf75e31f392dbdaaa04 Diff: https://reviews.apache.org/r/26270/diff/ Testing --- Unit tests: - verified that tests fail if the binding code is disabled, but pass with in on. - added a new unit test to check that unresolved bindings generate an error. Thanks, Mark Chu-Carroll
Re: Review Request 26269: Remove UI from page titles.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26269/#review55212 --- Ship it! Ship It! - Joshua Cohen On Oct. 2, 2014, 3:48 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26269/ --- (Updated Oct. 2, 2014, 3:48 p.m.) Review request for Aurora and David McLaughlin. Repository: aurora Description --- Aurora UI is a weird thing to call a website. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 40617aa5f84170abe8e2a2b8bc06913f90ae8eb0 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 3a07ce7927e936d2d30b8e19b2362a073372ad7b Diff: https://reviews.apache.org/r/26269/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 26269: Remove UI from page titles.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26269/#review55213 --- Ship it! Ship It! - Kevin Sweeney On Oct. 2, 2014, 8:48 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26269/ --- (Updated Oct. 2, 2014, 8:48 a.m.) Review request for Aurora and David McLaughlin. Repository: aurora Description --- Aurora UI is a weird thing to call a website. Diffs - src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 40617aa5f84170abe8e2a2b8bc06913f90ae8eb0 src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 3a07ce7927e936d2d30b8e19b2362a073372ad7b Diff: https://reviews.apache.org/r/26269/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 26270: Make the client invocation code actually use the exit codes returned by the client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26270/#review55217 --- Ship it! Ship It! - Zameer Manji On Oct. 2, 2014, 9:07 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26270/ --- (Updated Oct. 2, 2014, 9:07 a.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-781 https://issues.apache.org/jira/browse/aurora-781 Repository: aurora Description --- Fix multiple errors involving bindings, and fix result codes. - Cause an error to be raised by an unresolved binding in a configuration file. - Fix incorrect structuring of processed bindings. - Add a test to confirm that unbound parameters generate errors. While I'm in the code, also fix a problem where result codes aren't returned correctly to the shell. Diffs - src/main/python/apache/aurora/client/cli/client.py 0cb69448cd24372067ac845eca5862bc3d3a46a9 src/main/python/apache/aurora/client/cli/context.py 102d20797816788361dfdac450aac9fb8e6fbc28 src/main/python/apache/aurora/client/cli/options.py c2d422ac2bc82fc387596e93040b49f722f8310f src/main/python/apache/aurora/client/cli/standalone_client.py 30d8f750559b7811d66760741905fa8adf80fd1f src/test/python/apache/aurora/client/cli/test_create.py 31fa56f5edcfc97903725ab27ccc12c6a8f39ffc src/test/python/apache/aurora/client/cli/util.py a50b83c571390374975accf75e31f392dbdaaa04 Diff: https://reviews.apache.org/r/26270/diff/ Testing --- Unit tests: - verified that tests fail if the binding code is disabled, but pass with in on. - added a new unit test to check that unresolved bindings generate an error. Thanks, Mark Chu-Carroll
Re: Review Request 26270: Fix multiple errors involving bindings, and fix result codes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26270/ --- (Updated Oct. 2, 2014, 2:24 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Summary (updated) - Fix multiple errors involving bindings, and fix result codes. Bugs: aurora-781 https://issues.apache.org/jira/browse/aurora-781 Repository: aurora Description --- Fix multiple errors involving bindings, and fix result codes. - Cause an error to be raised by an unresolved binding in a configuration file. - Fix incorrect structuring of processed bindings. - Add a test to confirm that unbound parameters generate errors. While I'm in the code, also fix a problem where result codes aren't returned correctly to the shell. Diffs - src/main/python/apache/aurora/client/cli/client.py 0cb69448cd24372067ac845eca5862bc3d3a46a9 src/main/python/apache/aurora/client/cli/context.py 102d20797816788361dfdac450aac9fb8e6fbc28 src/main/python/apache/aurora/client/cli/options.py c2d422ac2bc82fc387596e93040b49f722f8310f src/main/python/apache/aurora/client/cli/standalone_client.py 30d8f750559b7811d66760741905fa8adf80fd1f src/test/python/apache/aurora/client/cli/test_create.py 31fa56f5edcfc97903725ab27ccc12c6a8f39ffc src/test/python/apache/aurora/client/cli/util.py a50b83c571390374975accf75e31f392dbdaaa04 Diff: https://reviews.apache.org/r/26270/diff/ Testing --- Unit tests: - verified that tests fail if the binding code is disabled, but pass with in on. - added a new unit test to check that unresolved bindings generate an error. Thanks, Mark Chu-Carroll
Re: Review Request 26232: Implementing update history pruner.
On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java,
line 85
https://reviews.apache.org/r/26232/diff/2/?file=710417#file710417line85
I'm always nervous when important behavior is embedded in something
seemingly-less-important like logging. Can you extract a variable to
separate the two?
Kind of redundant but sure, done.
On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line
140
https://reviews.apache.org/r/26232/diff/2/?file=710418#file710418line140
...last completed updates that completed less than {@code
historyPruneThreshold} ago..
Done.
On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line
147
https://reviews.apache.org/r/26232/diff/2/?file=710418#file710418line147
Keep the value rich as far down as you can, to mitigate accidental
misuse:
AmountLong, Time historyPruneThreshold
Disagree. I don't really like Amount - long -Amount - long dance that would
require. The Amount is converted into long only once now, converted into an
absolute timestamp and passed down to SQL to compare against.
On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java,
line 141
https://reviews.apache.org/r/26232/diff/2/?file=710419#file710419line141
s/result/pruned/
Sure.
On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java,
line 143
https://reviews.apache.org/r/26232/diff/2/?file=710419#file710419line143
How would you feel about making the two pruning goals distinct at the
mapper level? Does that simplfiy anything?
- get UUIDs of all updates older than pruneThreshold
- get UUIDs of the the last retainCount updates for each job
- delete above UUIDs.
I think i would find that easier to follow, at least.
This approach would require an extra SQL call (step 1) and potentially a lot
more SQL calls in step 2 as we would now deal with raw job keys not
pre-filtered for processing. The current algorithm is optimized to be less
SQL-chatty and short circuits if there are no job keys to process:
- get job keys that have updates to be deleted (older than pruneThreshold
and/or count retainCount)
- for every key above get update UUIDs and delete them.
On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java,
line 109
https://reviews.apache.org/r/26232/diff/2/?file=710420#file710420line109
Avoid repeating the method signature in text:
s/Set of u/U/
Done.
On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java,
line 121
https://reviews.apache.org/r/26232/diff/2/?file=710420#file710420line121
How about `getPruneCandidates`?
I don't think it will be clear enough as we are selecting job keys rather then
UUIDs here. How about selectJobKeysForPruning?
On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java,
line 320
https://reviews.apache.org/r/26232/diff/2/?file=710421#file710421line320
Why this rather than an explicit delete record for the affected update
UUIDs?
I thought about that but decided to stick with the current solution:
- less code to maintain: deleting a set of UUIDs would require a new
JobUpdateStore API only used by the LogStorage
- less data to store: persisting a bunch of pruned UUIDs seems redundant where
a a single prune call restores the consistency much more effectively.
On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java,
line 318
https://reviews.apache.org/r/26232/diff/2/?file=710421#file710421line318
If this comment remains, please elaborate on why it is not strictly
necessary. (i know, but a future developer might not.)
Done.
On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java,
line 45
https://reviews.apache.org/r/26232/diff/2/?file=710425#file710425line45
Can you take a stab at using FakeScheduledExecutor instead of a real
thread? I don't insist on it, but i'd like to see if that pattern works
out in other situations.
You'll have to modify FakeScheduledExecutor a bit to add support for
`scheduleAtFixedRate`, but at that point all you should have to do in the
test after `replay()` is
clock.advance(pruneInterval);
That's a fine suggestion. Moved and refactored FakeScheduledExcecutor to
support fixed rate scheduling.
-
Re: Review Request 26232: Implementing update history pruner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/ --- (Updated Oct. 2, 2014, 7:40 p.m.) Review request for Aurora, David McLaughlin and Bill Farner. Changes --- CR comments. Bugs: AURORA-743 https://issues.apache.org/jira/browse/AURORA-743 Repository: aurora Description --- The pruner runs on periodic basis and trims completed updates up to the guranteed per job retention count. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java ebae58a04e8857c5f26d4b57c27dfcda9e14c82c src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java c3abffe575e801cebec3572cf4aceac83a238b55 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java c583e085e0458835d51ebf740a3b5f01b428bb25 src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 66c91644677e7176ccf53dcfcf29a6792ec398bc src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/thrift/org/apache/aurora/gen/storage.thrift 7e502450f06abb449d06af09cc59185c6a9a2963 src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 68df0d542e41438c0844f76fc5b9ec6996a00e8d src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java e35fe23f023f5accfb2270a3634c91bec657 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 7254588b55df9a12217c8ec172abc5976892497e Diff: https://reviews.apache.org/r/26232/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 26232: Implementing update history pruner.
On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line 147 https://reviews.apache.org/r/26232/diff/2/?file=710418#file710418line147 Keep the value rich as far down as you can, to mitigate accidental misuse: AmountLong, Time historyPruneThreshold Maxim Khutornenko wrote: Disagree. I don't really like Amount - long -Amount - long dance that would require. The Amount is converted into long only once now, converted into an absolute timestamp and passed down to SQL to compare against. I'm actually suggesting you avoid the dance completely. Keep AmountLong, Time in the settings object, and only translate it to a long when you need a long. The only argument i can see against this is performance, which doesn't hold here. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/#review55186 --- On Oct. 2, 2014, 7:40 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/ --- (Updated Oct. 2, 2014, 7:40 p.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-743 https://issues.apache.org/jira/browse/AURORA-743 Repository: aurora Description --- The pruner runs on periodic basis and trims completed updates up to the guranteed per job retention count. Diffs - src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java ebae58a04e8857c5f26d4b57c27dfcda9e14c82c src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java c3abffe575e801cebec3572cf4aceac83a238b55 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java c583e085e0458835d51ebf740a3b5f01b428bb25 src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 66c91644677e7176ccf53dcfcf29a6792ec398bc src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/thrift/org/apache/aurora/gen/storage.thrift 7e502450f06abb449d06af09cc59185c6a9a2963 src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 68df0d542e41438c0844f76fc5b9ec6996a00e8d src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java e35fe23f023f5accfb2270a3634c91bec657 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 7254588b55df9a12217c8ec172abc5976892497e Diff: https://reviews.apache.org/r/26232/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 26270: Fix multiple errors involving bindings, and fix result codes.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26270/#review55239 --- Ship it! Ship It! - David McLaughlin On Oct. 2, 2014, 6:24 p.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26270/ --- (Updated Oct. 2, 2014, 6:24 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-781 https://issues.apache.org/jira/browse/aurora-781 Repository: aurora Description --- Fix multiple errors involving bindings, and fix result codes. - Cause an error to be raised by an unresolved binding in a configuration file. - Fix incorrect structuring of processed bindings. - Add a test to confirm that unbound parameters generate errors. While I'm in the code, also fix a problem where result codes aren't returned correctly to the shell. Diffs - src/main/python/apache/aurora/client/cli/client.py 0cb69448cd24372067ac845eca5862bc3d3a46a9 src/main/python/apache/aurora/client/cli/context.py 102d20797816788361dfdac450aac9fb8e6fbc28 src/main/python/apache/aurora/client/cli/options.py c2d422ac2bc82fc387596e93040b49f722f8310f src/main/python/apache/aurora/client/cli/standalone_client.py 30d8f750559b7811d66760741905fa8adf80fd1f src/test/python/apache/aurora/client/cli/test_create.py 31fa56f5edcfc97903725ab27ccc12c6a8f39ffc src/test/python/apache/aurora/client/cli/util.py a50b83c571390374975accf75e31f392dbdaaa04 Diff: https://reviews.apache.org/r/26270/diff/ Testing --- Unit tests: - verified that tests fail if the binding code is disabled, but pass with in on. - added a new unit test to check that unresolved bindings generate an error. Thanks, Mark Chu-Carroll
Re: Review Request 26232: Implementing update history pruner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/ --- (Updated Oct. 2, 2014, 7:54 p.m.) Review request for Aurora, David McLaughlin and Bill Farner. Changes --- CR comments. Bugs: AURORA-743 https://issues.apache.org/jira/browse/AURORA-743 Repository: aurora Description --- The pruner runs on periodic basis and trims completed updates up to the guranteed per job retention count. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java ebae58a04e8857c5f26d4b57c27dfcda9e14c82c src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java c3abffe575e801cebec3572cf4aceac83a238b55 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java c583e085e0458835d51ebf740a3b5f01b428bb25 src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 66c91644677e7176ccf53dcfcf29a6792ec398bc src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 src/main/thrift/org/apache/aurora/gen/storage.thrift 7e502450f06abb449d06af09cc59185c6a9a2963 src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 68df0d542e41438c0844f76fc5b9ec6996a00e8d src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java e35fe23f023f5accfb2270a3634c91bec657 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 7254588b55df9a12217c8ec172abc5976892497e Diff: https://reviews.apache.org/r/26232/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 26137: Fix help for new update command.
On Sept. 30, 2014, 2:32 a.m., Joe Smith wrote: src/main/python/apache/aurora/client/cli/update.py, line 45 https://reviews.apache.org/r/26137/diff/1/?file=708198#file708198line45 Could you update a test case to catch accessing these as properties to catch accidental regressions? David McLaughlin wrote: Piggy backing this issue to add that my ship it is pending a test for this command at least? I don't know of any way to write a single test that would always catch this. - Mark --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/#review54953 --- On Sept. 29, 2014, 11:05 a.m., Mark Chu-Carroll wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26137/ --- (Updated Sept. 29, 2014, 11:05 a.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: aurora-748 https://issues.apache.org/jira/browse/aurora-748 Repository: aurora Description --- Fix help for new update command. Diffs - src/main/python/apache/aurora/client/cli/update.py b4dd792dc12f19424c620f4d91748113e272f0c9 Diff: https://reviews.apache.org/r/26137/diff/ Testing --- manual testing. Thanks, Mark Chu-Carroll
Re: Review Request 26004: Add aurora update list and aurora update status commands.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26004/ --- (Updated Oct. 2, 2014, 4:41 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Changes --- Add the IDs to update list. Bugs: aurora-742 https://issues.apache.org/jira/browse/aurora-742 Repository: aurora Description --- Add support for commands to query and display active updates being managed by the scheduler. Two commands are added: * aurora update list, which shows all active updates that are being processed by the server. * aurora update status, which shows detailed status information about an update in-progress on the server. Diffs (updated) - src/main/python/apache/aurora/client/cli/context.py f639af7de93a069b278dc494b6f92a2f6b10de9c src/main/python/apache/aurora/client/cli/options.py 9a81a1d7d04c6648e94ff117429278317a9dbed8 src/main/python/apache/aurora/client/cli/update.py b4dd792dc12f19424c620f4d91748113e272f0c9 src/test/python/apache/aurora/client/cli/test_supdate.py 2782fee2867c6ef79349240299de082f07f7967a src/test/python/apache/aurora/client/cli/util.py e1ee884c06f3bc22bcd9e908ff61af9459a0b535 Diff: https://reviews.apache.org/r/26004/diff/ Testing --- Added new unit tests; all tests pass. Thanks, Mark Chu-Carroll
Re: Review Request 26137: Fix help for new update command.
On Sept. 29, 2014, 11:32 p.m., Joe Smith wrote: src/main/python/apache/aurora/client/cli/update.py, line 45 https://reviews.apache.org/r/26137/diff/1/?file=708198#file708198line45 Could you update a test case to catch accessing these as properties to catch accidental regressions? David McLaughlin wrote: Piggy backing this issue to add that my ship it is pending a test for this command at least? Mark Chu-Carroll wrote: I don't know of any way to write a single test that would always catch this. Rebased off your diff: [tw-172-25-132-201 aurora (yasumoto/test_update_help)]$ git diff diff --git a/src/main/python/apache/aurora/client/cli/update.py b/src/main/python/apache/aurora/client/cli/update.py index 41475a7..ef07a11 100644 --- a/src/main/python/apache/aurora/client/cli/update.py +++ b/src/main/python/apache/aurora/client/cli/update.py @@ -42,7 +42,6 @@ class StartUpdate(Verb): INSTANCES_SPEC_ARGUMENT, CONFIG_ARGUMENT ] - @property def help(self): return textwrap.dedent(\ Start a scheduler-driven rolling upgrade on a running job, using the update diff --git a/src/test/python/apache/aurora/client/cli/test_update.py b/src/test/python/apache/aurora/client/cli/test_update.py index eeed774..1a38ffe 100644 --- a/src/test/python/apache/aurora/client/cli/test_update.py +++ b/src/test/python/apache/aurora/client/cli/test_update.py @@ -23,6 +23,7 @@ from apache.aurora.client.api.quota_check import QuotaCheck from apache.aurora.client.api.scheduler_mux import SchedulerMux from apache.aurora.client.cli import EXIT_INVALID_CONFIGURATION, EXIT_OK from apache.aurora.client.cli.client import AuroraCommandLine +from apache.aurora.client.cli.update import StartUpdate from apache.aurora.client.cli.util import AuroraClientCommandTest, FakeAuroraCommandContext, IOMock from apache.aurora.config import AuroraConfig @@ -301,6 +302,10 @@ class TestUpdateCommand(AuroraClientCommandTest): 'Update completed successfully'] assert mock_err.get() == [] + def test_update_start_help(self): +start = StartUpdate() +assert 'Start a scheduler-driven rolling' in start.help + @classmethod def assert_correct_addinstance_calls(cls, api): assert api.addInstances.call_count == 20 [tw-172-25-132-201 aurora (yasumoto/test_update_help)]$ ./pants ./src/test/python/apache/aurora/client/cli:job Build operating on top level addresses: set([BuildFileAddress(/Users/jsmith/workspace/aurora/src/test/python/apache/aurora/client/cli/BUILD, job)]) test session starts = platform darwin -- Python 2.6.8 -- py-1.4.25 -- pytest-2.6.3 plugins: cov, timeout collected 61 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_open.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 ..F... == FAILURES == __ TestUpdateCommand.test_update_start_help __ self = test_update.TestUpdateCommand testMethod=test_update_start_help def test_update_start_help(self): start = StartUpdate() assert 'Start a scheduler-driven rolling' in start.help E TypeError: argument of type 'instancemethod' is not iterable src/test/python/apache/aurora/client/cli/test_update.py:307: TypeError 1 failed, 60 passed in 6.07 seconds = src.test.python.apache.aurora.client.cli.job . FAILURE [tw-172-25-132-201 aurora (yasumoto/test_update_help)]$ git diff diff --git
Review Request 26283: Adding authz check into descheduleCronJob RPC.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26283/ --- Review request for Aurora and Bill Farner. Bugs: AURORA-566 https://issues.apache.org/jira/browse/AURORA-566 Repository: aurora Description --- Adding authz check into descheduleCronJob RPC. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 23115fb64fc6c2324b416e9527b0f3952c253977 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b21dce6588d8d1d9fde85e8ab7ab1ee6e2587008 Diff: https://reviews.apache.org/r/26283/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.
On Oct. 1, 2014, 4:34 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/executor/common/announcer.py, line 228 https://reviews.apache.org/r/25974/diff/6/?file=709964#file709964line228 push this into `__start`, out of the constructor? At least on the Java side we try to avoid doing any I/O in constructors (as they're more for wiring) but instead delegate to explicit lifecycle methods like `start`. I'd rather not. Keeping it in the constructor means it stays in the from_assigned_task method code path. Moving it to the start method moves it out of that code path and really changes when we do this sort of I/O. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/#review55161 --- On Sept. 30, 2014, 5:17 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/ --- (Updated Sept. 30, 2014, 5:17 p.m.) Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman. Bugs: AURORA-728 https://issues.apache.org/jira/browse/AURORA-728 Repository: aurora Description --- Prevent initial ZK timeouts from killing the executor. In addition, prevent uncaught exceptions from killing the executor. Diffs - src/main/python/apache/aurora/executor/aurora_executor.py 79a24855b2a68271b7478395dfdadab8755c3af2 src/main/python/apache/aurora/executor/common/announcer.py c466da8d48bbc2aa227c2db157cab84665ad6602 src/test/python/apache/aurora/executor/common/test_announcer.py 4f6e200ecb1a4ea7cb45acd466a57f19d5815326 Diff: https://reviews.apache.org/r/25974/diff/ Testing --- ./pants src/test/python/apache/aurora/executor:executor-small ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.
On Oct. 1, 2014, 4:14 p.m., Brian Wickman wrote: src/test/python/apache/aurora/executor/common/test_announcer.py, line 239 https://reviews.apache.org/r/25974/diff/6/?file=709965#file709965line239 out of scope for this review, but there's a new-ish pypi project called 'zake' that allows the kazoo client to be stubbed out with a mock zk tree (while preserving certain operations like create/delete.) i'd love to explore using it here in the future. It looks really nice for this sort of testing. It is definately out of the scope of this review. If we ever need to do more work on the Announcer/Service Discovery we should first migrate to zake. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/#review55158 --- On Sept. 30, 2014, 5:17 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/ --- (Updated Sept. 30, 2014, 5:17 p.m.) Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman. Bugs: AURORA-728 https://issues.apache.org/jira/browse/AURORA-728 Repository: aurora Description --- Prevent initial ZK timeouts from killing the executor. In addition, prevent uncaught exceptions from killing the executor. Diffs - src/main/python/apache/aurora/executor/aurora_executor.py 79a24855b2a68271b7478395dfdadab8755c3af2 src/main/python/apache/aurora/executor/common/announcer.py c466da8d48bbc2aa227c2db157cab84665ad6602 src/test/python/apache/aurora/executor/common/test_announcer.py 4f6e200ecb1a4ea7cb45acd466a57f19d5815326 Diff: https://reviews.apache.org/r/25974/diff/ Testing --- ./pants src/test/python/apache/aurora/executor:executor-small ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Review Request 26288: Fixing log_response in context.py
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26288/ --- Review request for Aurora and Mark Chu-Carroll. Bugs: AURORA-786 https://issues.apache.org/jira/browse/AURORA-786 Repository: aurora Description --- Fixing log_response in context.py Diffs - src/main/python/apache/aurora/client/cli/context.py f639af7de93a069b278dc494b6f92a2f6b10de9c src/test/python/apache/aurora/client/cli/BUILD 8ce6bd5b7faa1579372fb6935180ea982af64af8 src/test/python/apache/aurora/client/cli/test_context.py PRE-CREATION Diff: https://reviews.apache.org/r/26288/diff/ Testing --- ./pants src/tests/python:all Thanks, Maxim Khutornenko
Re: Review Request 26283: Adding authz check into descheduleCronJob RPC.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26283/#review55265 --- Ship it! Ship It! - Bill Farner On Oct. 2, 2014, 9:44 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26283/ --- (Updated Oct. 2, 2014, 9:44 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-566 https://issues.apache.org/jira/browse/AURORA-566 Repository: aurora Description --- Adding authz check into descheduleCronJob RPC. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 23115fb64fc6c2324b416e9527b0f3952c253977 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b21dce6588d8d1d9fde85e8ab7ab1ee6e2587008 Diff: https://reviews.apache.org/r/26283/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 26232: Implementing update history pruner.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55266
---
Ship it!
src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java
https://reviews.apache.org/r/26232/#comment95651
if (!prunedUpdates.isEmpty()) {
LOG.info(...);
}
src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java
https://reviews.apache.org/r/26232/#comment95652
line break above
- Bill Farner
On Oct. 2, 2014, 7:54 p.m., Maxim Khutornenko wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/
---
(Updated Oct. 2, 2014, 7:54 p.m.)
Review request for Aurora, David McLaughlin and Bill Farner.
Bugs: AURORA-743
https://issues.apache.org/jira/browse/AURORA-743
Repository: aurora
Description
---
The pruner runs on periodic basis and trims completed updates up to the
guranteed per job retention count.
Diffs
-
src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java
aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef
src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java
ebae58a04e8857c5f26d4b57c27dfcda9e14c82c
src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java
c3abffe575e801cebec3572cf4aceac83a238b55
src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java
3db0114c5432a5b7d0b01d97c75494be9e3e99a3
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
c583e085e0458835d51ebf740a3b5f01b428bb25
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java
66c91644677e7176ccf53dcfcf29a6792ec398bc
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
631ab2543dadb77b7fd1cae96adc9c0bd2f94b39
src/main/thrift/org/apache/aurora/gen/storage.thrift
7e502450f06abb449d06af09cc59185c6a9a2963
src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java
011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
PRE-CREATION
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9
src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java
68df0d542e41438c0844f76fc5b9ec6996a00e8d
src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java
e35fe23f023f5accfb2270a3634c91bec657
src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java
7254588b55df9a12217c8ec172abc5976892497e
Diff: https://reviews.apache.org/r/26232/diff/
Testing
---
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 26232: Implementing update history pruner.
On Oct. 2, 2014, 10:39 p.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java,
line 89
https://reviews.apache.org/r/26232/diff/4/?file=711547#file711547line89
if (!prunedUpdates.isEmpty()) {
LOG.info(...);
}
I actually want to log in either case for better transparency. Modified to
support both cases.
On Oct. 2, 2014, 10:39 p.m., Bill Farner wrote:
src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java,
line 75
https://reviews.apache.org/r/26232/diff/4/?file=711558#file711558line75
line break above
Done.
- Maxim
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55266
---
On Oct. 2, 2014, 7:54 p.m., Maxim Khutornenko wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/
---
(Updated Oct. 2, 2014, 7:54 p.m.)
Review request for Aurora, David McLaughlin and Bill Farner.
Bugs: AURORA-743
https://issues.apache.org/jira/browse/AURORA-743
Repository: aurora
Description
---
The pruner runs on periodic basis and trims completed updates up to the
guranteed per job retention count.
Diffs
-
src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java
aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef
src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java
ebae58a04e8857c5f26d4b57c27dfcda9e14c82c
src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java
c3abffe575e801cebec3572cf4aceac83a238b55
src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java
3db0114c5432a5b7d0b01d97c75494be9e3e99a3
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
c583e085e0458835d51ebf740a3b5f01b428bb25
src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java
66c91644677e7176ccf53dcfcf29a6792ec398bc
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
631ab2543dadb77b7fd1cae96adc9c0bd2f94b39
src/main/thrift/org/apache/aurora/gen/storage.thrift
7e502450f06abb449d06af09cc59185c6a9a2963
src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java
011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
PRE-CREATION
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9
src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java
68df0d542e41438c0844f76fc5b9ec6996a00e8d
src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java
e35fe23f023f5accfb2270a3634c91bec657
src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java
7254588b55df9a12217c8ec172abc5976892497e
Diff: https://reviews.apache.org/r/26232/diff/
Testing
---
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 26232: Implementing update history pruner.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/ --- (Updated Oct. 2, 2014, 10:51 p.m.) Review request for Aurora, David McLaughlin and Bill Farner. Changes --- CR comments. Bugs: AURORA-743 https://issues.apache.org/jira/browse/AURORA-743 Repository: aurora Description --- The pruner runs on periodic basis and trims completed updates up to the guranteed per job retention count. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java ebae58a04e8857c5f26d4b57c27dfcda9e14c82c src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java c3abffe575e801cebec3572cf4aceac83a238b55 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java c583e085e0458835d51ebf740a3b5f01b428bb25 src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 66c91644677e7176ccf53dcfcf29a6792ec398bc src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 2a81a94f99f242bbe400d8428ad1f1ce38a06a86 src/main/thrift/org/apache/aurora/gen/storage.thrift 7e502450f06abb449d06af09cc59185c6a9a2963 src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java dbf0badbfcc19f40d9b9eeec22b7024d95a02884 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 68df0d542e41438c0844f76fc5b9ec6996a00e8d src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java e35fe23f023f5accfb2270a3634c91bec657 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 6758b7b56e77882c67be2e39481ff76893ad1610 Diff: https://reviews.apache.org/r/26232/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.
On Oct. 1, 2014, 4:34 p.m., Kevin Sweeney wrote: src/main/python/apache/aurora/executor/common/announcer.py, line 228 https://reviews.apache.org/r/25974/diff/6/?file=709964#file709964line228 push this into `__start`, out of the constructor? At least on the Java side we try to avoid doing any I/O in constructors (as they're more for wiring) but instead delegate to explicit lifecycle methods like `start`. Zameer Manji wrote: I'd rather not. Keeping it in the constructor means it stays in the from_assigned_task method code path. Moving it to the start method moves it out of that code path and really changes when we do this sort of I/O. is it common practice to do I/O in constructors elsewhere in this codebase? I'll defer to wickman here - Kevin --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/#review55161 --- On Sept. 30, 2014, 5:17 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/ --- (Updated Sept. 30, 2014, 5:17 p.m.) Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman. Bugs: AURORA-728 https://issues.apache.org/jira/browse/AURORA-728 Repository: aurora Description --- Prevent initial ZK timeouts from killing the executor. In addition, prevent uncaught exceptions from killing the executor. Diffs - src/main/python/apache/aurora/executor/aurora_executor.py 79a24855b2a68271b7478395dfdadab8755c3af2 src/main/python/apache/aurora/executor/common/announcer.py c466da8d48bbc2aa227c2db157cab84665ad6602 src/test/python/apache/aurora/executor/common/test_announcer.py 4f6e200ecb1a4ea7cb45acd466a57f19d5815326 Diff: https://reviews.apache.org/r/25974/diff/ Testing --- ./pants src/test/python/apache/aurora/executor:executor-small ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/ --- (Updated Oct. 2, 2014, 4:20 p.m.) Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman. Changes --- Brian's feedback. Bugs: AURORA-728 https://issues.apache.org/jira/browse/AURORA-728 Repository: aurora Description --- Prevent initial ZK timeouts from killing the executor. In addition, prevent uncaught exceptions from killing the executor. Diffs (updated) - src/main/python/apache/aurora/executor/aurora_executor.py 79a24855b2a68271b7478395dfdadab8755c3af2 src/main/python/apache/aurora/executor/common/announcer.py c466da8d48bbc2aa227c2db157cab84665ad6602 src/test/python/apache/aurora/executor/common/test_announcer.py 4f6e200ecb1a4ea7cb45acd466a57f19d5815326 Diff: https://reviews.apache.org/r/25974/diff/ Testing --- ./pants src/test/python/apache/aurora/executor:executor-small ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Re: Review Request 26232: Implementing update history pruner.
On Oct. 2, 2014, 11:16 p.m., David McLaughlin wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml, lines 468-490 https://reviews.apache.org/r/26232/diff/5/?file=713015#file713015line468 It'd be nice if this didn't just blanket delete all old updates, especially for active jobs. There are probably a certain class of service/cron that are rarely touched and it would be nice to keep around that change history. Would it add too much complexity to try and solve this? Well, that would require active task queries and implementation leaking outside DB layer. I'd rather not attempt anything like that until we move TaskStore into SQL. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/#review55272 --- On Oct. 2, 2014, 10:51 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/ --- (Updated Oct. 2, 2014, 10:51 p.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-743 https://issues.apache.org/jira/browse/AURORA-743 Repository: aurora Description --- The pruner runs on periodic basis and trims completed updates up to the guranteed per job retention count. Diffs - src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java ebae58a04e8857c5f26d4b57c27dfcda9e14c82c src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java c3abffe575e801cebec3572cf4aceac83a238b55 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java c583e085e0458835d51ebf740a3b5f01b428bb25 src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 66c91644677e7176ccf53dcfcf29a6792ec398bc src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 2a81a94f99f242bbe400d8428ad1f1ce38a06a86 src/main/thrift/org/apache/aurora/gen/storage.thrift 7e502450f06abb449d06af09cc59185c6a9a2963 src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java dbf0badbfcc19f40d9b9eeec22b7024d95a02884 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 68df0d542e41438c0844f76fc5b9ec6996a00e8d src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java e35fe23f023f5accfb2270a3634c91bec657 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 6758b7b56e77882c67be2e39481ff76893ad1610 Diff: https://reviews.apache.org/r/26232/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 26300: Sleep 10sec instead of 5sec during GC shutdown
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26300/ --- Review request for Aurora, Vinod Kone and Brian Wickman. Bugs: AURORA-788 https://issues.apache.org/jira/browse/AURORA-788 Repository: aurora Description --- Sleep 10sec instead of 5sec during GC shutdown Diffs - src/main/python/apache/aurora/executor/gc_executor.py 44eb0da984a126536f0d277da3da128089201a47 Diff: https://reviews.apache.org/r/26300/diff/ Testing --- Thanks, Kevin Sweeney
Review Request 26298: Use a less broad retry loop for RPCs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26298/ --- Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko. Repository: aurora Description --- A few times when making changes, i've found myself confused at a stalled test and spiked CPU, only to find that my test should have failed, but an exception is trapped in this retry loop. The key change here is that unknown exceptions will break the loop. Making this change pointed out what should have been a test failure in `test_transient_error`, where an exception caused by an unexpected call to `getVersion` was swallowed. I also changed the test to avoid sleeping, which was eating a fixed 10 seconds of unit test time. Diffs - src/main/python/apache/aurora/client/api/scheduler_client.py b400cb2dbdb35077fc2c4a6e161c2959a9217317 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1cbfbf86e903d890baac7d34461109f9beaff442 Diff: https://reviews.apache.org/r/26298/diff/ Testing --- ./pants src/test/python:all -vxs Thanks, Bill Farner
Re: Review Request 26298: Use a less broad retry loop for RPCs.
On Oct. 3, 2014, 12:13 a.m., Maxim Khutornenko wrote: | I also changed the test to avoid sleeping, which was eating a fixed 10 seconds of unit test time. Not really sure why it takes that long for you: $ time ./pants src/test/python/apache/aurora/client/api:scheduler_client -k test_transient_error ... real0m3.602s user0m2.679s sys 0m0.696s You're right, my own change added another sleep when a transient error is encountered: self._terminating.wait(self.RPC_RETRY_INTERVAL.as_(Time.SECONDS)) That's what was causing the delay. It's the right behavior to do this, rather than a tight retry loop, but you're correct that this change is not speeding up our tests. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26298/#review55292 --- On Oct. 3, 2014, 12:05 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26298/ --- (Updated Oct. 3, 2014, 12:05 a.m.) Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko. Repository: aurora Description --- A few times when making changes, i've found myself confused at a stalled test and spiked CPU, only to find that my test should have failed, but an exception is trapped in this retry loop. The key change here is that unknown exceptions will break the loop. Making this change pointed out what should have been a test failure in `test_transient_error`, where an exception caused by an unexpected call to `getVersion` was swallowed. I also changed the test to avoid sleeping, which was eating a fixed 10 seconds of unit test time. Diffs - src/main/python/apache/aurora/client/api/scheduler_client.py b400cb2dbdb35077fc2c4a6e161c2959a9217317 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1cbfbf86e903d890baac7d34461109f9beaff442 Diff: https://reviews.apache.org/r/26298/diff/ Testing --- ./pants src/test/python:all -vxs Thanks, Bill Farner
Re: Review Request 26298: Use a less broad retry loop for RPCs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26298/ --- (Updated Oct. 3, 2014, 12:18 a.m.) Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko. Changes --- Corrected description. Repository: aurora Description (updated) --- A few times when making changes, i've found myself confused at a stalled test and spiked CPU, only to find that my test should have failed, but an exception is trapped in this retry loop. The key change here is that unknown exceptions will break the loop. Making this change pointed out what should have been a test failure in `test_transient_error`, where an exception caused by an unexpected call to `getVersion` was swallowed. Diffs - src/main/python/apache/aurora/client/api/scheduler_client.py b400cb2dbdb35077fc2c4a6e161c2959a9217317 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1cbfbf86e903d890baac7d34461109f9beaff442 Diff: https://reviews.apache.org/r/26298/diff/ Testing --- ./pants src/test/python:all -vxs Thanks, Bill Farner
Re: Review Request 26298: Use a less broad retry loop for RPCs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26298/#review55295 --- src/main/python/apache/aurora/client/api/scheduler_client.py https://reviews.apache.org/r/26298/#comment95682 Why not just add a break into the Exception catcher instead? - Maxim Khutornenko On Oct. 3, 2014, 12:18 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26298/ --- (Updated Oct. 3, 2014, 12:18 a.m.) Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko. Repository: aurora Description --- A few times when making changes, i've found myself confused at a stalled test and spiked CPU, only to find that my test should have failed, but an exception is trapped in this retry loop. The key change here is that unknown exceptions will break the loop. Making this change pointed out what should have been a test failure in `test_transient_error`, where an exception caused by an unexpected call to `getVersion` was swallowed. Diffs - src/main/python/apache/aurora/client/api/scheduler_client.py b400cb2dbdb35077fc2c4a6e161c2959a9217317 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1cbfbf86e903d890baac7d34461109f9beaff442 Diff: https://reviews.apache.org/r/26298/diff/ Testing --- ./pants src/test/python:all -vxs Thanks, Bill Farner
Re: Review Request 26244: Add a doc about dedicated hosts.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26244/#review55296 --- Ship it! Ship It! - Kevin Sweeney On Oct. 1, 2014, 7:25 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26244/ --- (Updated Oct. 1, 2014, 7:25 p.m.) Review request for Aurora and Kevin Sweeney. Bugs: AURORA-703 https://issues.apache.org/jira/browse/AURORA-703 Repository: aurora Description --- Add a doc about dedicated hosts. Diffs - docs/deploying-aurora-scheduler.md 93ff746038df14f2abeea85acf48d02b217af522 Diff: https://reviews.apache.org/r/26244/diff/ Testing --- Rendered here: https://github.com/wfarner/incubator-aurora/blob/wfarner/doc_dedicated_machines/docs/deploying-aurora-scheduler.md Thanks, Bill Farner
Re: Review Request 26298: Use a less broad retry loop for RPCs.
On Oct. 3, 2014, 12:21 a.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/api/scheduler_client.py, line 286 https://reviews.apache.org/r/26298/diff/1/?file=713279#file713279line286 Why not just add a break into the Exception catcher instead? Actually, we are throwing there already. Wait, how is the exception is trapped in that loop? - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26298/#review55295 --- On Oct. 3, 2014, 12:18 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26298/ --- (Updated Oct. 3, 2014, 12:18 a.m.) Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko. Repository: aurora Description --- A few times when making changes, i've found myself confused at a stalled test and spiked CPU, only to find that my test should have failed, but an exception is trapped in this retry loop. The key change here is that unknown exceptions will break the loop. Making this change pointed out what should have been a test failure in `test_transient_error`, where an exception caused by an unexpected call to `getVersion` was swallowed. Diffs - src/main/python/apache/aurora/client/api/scheduler_client.py b400cb2dbdb35077fc2c4a6e161c2959a9217317 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1cbfbf86e903d890baac7d34461109f9beaff442 Diff: https://reviews.apache.org/r/26298/diff/ Testing --- ./pants src/test/python:all -vxs Thanks, Bill Farner
Re: Review Request 26300: Sleep 10sec instead of 5sec during GC shutdown
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26300/#review55299 --- Ship it! http://media.giphy.com/media/7LG6PqAubrWBa/giphy.gif - Brian Wickman On Oct. 3, 2014, 12:03 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26300/ --- (Updated Oct. 3, 2014, 12:03 a.m.) Review request for Aurora, Vinod Kone and Brian Wickman. Bugs: AURORA-788 https://issues.apache.org/jira/browse/AURORA-788 Repository: aurora Description --- Sleep 10sec instead of 5sec during GC shutdown Diffs - src/main/python/apache/aurora/executor/gc_executor.py 44eb0da984a126536f0d277da3da128089201a47 Diff: https://reviews.apache.org/r/26300/diff/ Testing --- Thanks, Kevin Sweeney
Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/#review55309 --- src/test/python/apache/aurora/executor/common/test_announcer.py https://reviews.apache.org/r/25974/#comment95688 I'm confused -- nobody calls client.connected anymore, right? In theory, this test will pass, except it will take 30-60 seconds to run since client_connect_event.wait(timeout=...) will be called with the health check timeout. - Brian Wickman On Oct. 2, 2014, 11:20 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/ --- (Updated Oct. 2, 2014, 11:20 p.m.) Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman. Bugs: AURORA-728 https://issues.apache.org/jira/browse/AURORA-728 Repository: aurora Description --- Prevent initial ZK timeouts from killing the executor. In addition, prevent uncaught exceptions from killing the executor. Diffs - src/main/python/apache/aurora/executor/aurora_executor.py 79a24855b2a68271b7478395dfdadab8755c3af2 src/main/python/apache/aurora/executor/common/announcer.py c466da8d48bbc2aa227c2db157cab84665ad6602 src/test/python/apache/aurora/executor/common/test_announcer.py 4f6e200ecb1a4ea7cb45acd466a57f19d5815326 Diff: https://reviews.apache.org/r/25974/diff/ Testing --- ./pants src/test/python/apache/aurora/executor:executor-small ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Re: Review Request 25974: Prevent initial ZK timeouts from killing the executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25974/ --- (Updated Oct. 2, 2014, 6:44 p.m.) Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman. Changes --- Brian's feedback. Bugs: AURORA-728 https://issues.apache.org/jira/browse/AURORA-728 Repository: aurora Description --- Prevent initial ZK timeouts from killing the executor. In addition, prevent uncaught exceptions from killing the executor. Diffs (updated) - src/main/python/apache/aurora/executor/aurora_executor.py 79a24855b2a68271b7478395dfdadab8755c3af2 src/main/python/apache/aurora/executor/common/announcer.py c466da8d48bbc2aa227c2db157cab84665ad6602 src/test/python/apache/aurora/executor/common/test_announcer.py 4f6e200ecb1a4ea7cb45acd466a57f19d5815326 Diff: https://reviews.apache.org/r/25974/diff/ Testing --- ./pants src/test/python/apache/aurora/executor:executor-small ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Zameer Manji
Re: Review Request 26300: Sleep 10sec instead of 5sec during GC shutdown
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26300/#review55312 --- Upon discussion with Vinod Kone and Ben Mahler this doesn't seem like the right approach - driver.stop() doesn't actually send a signal to the slave that it should not expect further communications from the executor, so the race will always exist. Upon further dissection of this code I'm questioning the need to quit after 15 minutes of inactivity. Updated patch incoming. - Kevin Sweeney On Oct. 2, 2014, 5:03 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26300/ --- (Updated Oct. 2, 2014, 5:03 p.m.) Review request for Aurora, Vinod Kone and Brian Wickman. Bugs: AURORA-788 https://issues.apache.org/jira/browse/AURORA-788 Repository: aurora Description --- Sleep 10sec instead of 5sec during GC shutdown Diffs - src/main/python/apache/aurora/executor/gc_executor.py 44eb0da984a126536f0d277da3da128089201a47 Diff: https://reviews.apache.org/r/26300/diff/ Testing --- Thanks, Kevin Sweeney
Re: Review Request 26298: Use a less broad retry loop for RPCs.
On Oct. 3, 2014, 12:21 a.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/api/scheduler_client.py, line 286 https://reviews.apache.org/r/26298/diff/1/?file=713279#file713279line286 Why not just add a break into the Exception catcher instead? Maxim Khutornenko wrote: Actually, we are throwing there already. Wait, how is the exception is trapped in that loop? I scratched my head at this for a while, and wound up with this review: https://reviews.apache.org/r/26308/ AFAICT `threading.Event` does not override `__bool__`, so those branches are never entered, and we loop until the timer is up. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26298/#review55295 --- On Oct. 3, 2014, 12:18 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26298/ --- (Updated Oct. 3, 2014, 12:18 a.m.) Review request for Aurora, Mark Chu-Carroll and Maxim Khutornenko. Repository: aurora Description --- A few times when making changes, i've found myself confused at a stalled test and spiked CPU, only to find that my test should have failed, but an exception is trapped in this retry loop. The key change here is that unknown exceptions will break the loop. Making this change pointed out what should have been a test failure in `test_transient_error`, where an exception caused by an unexpected call to `getVersion` was swallowed. Diffs - src/main/python/apache/aurora/client/api/scheduler_client.py b400cb2dbdb35077fc2c4a6e161c2959a9217317 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1cbfbf86e903d890baac7d34461109f9beaff442 Diff: https://reviews.apache.org/r/26298/diff/ Testing --- ./pants src/test/python:all -vxs Thanks, Bill Farner
Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26308/ --- (Updated Oct. 3, 2014, 2:38 a.m.) Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll. Changes --- Missed one. Repository: aurora Description --- Fixes two problems: - mocking was incorrect in `test_status_api_failure`. Turns out that Mock objects were being passed around and somehow resulted in the test case passing. - use of `threading.Event()` was broken in scheduler_client.py. I don't think it's possible to enter those branches. Diffs (updated) - src/main/python/apache/aurora/client/api/scheduler_client.py b400cb2dbdb35077fc2c4a6e161c2959a9217317 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1cbfbf86e903d890baac7d34461109f9beaff442 src/test/python/apache/aurora/client/cli/test_api_from_cli.py 78f21d2f20cf71fa2dfe0614885d44d2948decd2 Diff: https://reviews.apache.org/r/26308/diff/ Testing --- ./pants src/test/python:all -vxs Thanks, Bill Farner
Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26308/#review55319 --- src/test/python/apache/aurora/client/cli/test_api_from_cli.py https://reviews.apache.org/r/26308/#comment95691 you need to set a spec here Mock(SchedulerClient) - Joe Smith On Oct. 2, 2014, 7:38 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26308/ --- (Updated Oct. 2, 2014, 7:38 p.m.) Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll. Repository: aurora Description --- Fixes two problems: - mocking was incorrect in `test_status_api_failure`. Turns out that Mock objects were being passed around and somehow resulted in the test case passing. - use of `threading.Event()` was broken in scheduler_client.py. I don't think it's possible to enter those branches. Diffs - src/main/python/apache/aurora/client/api/scheduler_client.py b400cb2dbdb35077fc2c4a6e161c2959a9217317 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1cbfbf86e903d890baac7d34461109f9beaff442 src/test/python/apache/aurora/client/cli/test_api_from_cli.py 78f21d2f20cf71fa2dfe0614885d44d2948decd2 Diff: https://reviews.apache.org/r/26308/diff/ Testing --- ./pants src/test/python:all -vxs Thanks, Bill Farner
Re: Review Request 26308: Fix exit condition for RPC loop, fix test_status_api_failure test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26308/ --- (Updated Oct. 3, 2014, 4:52 a.m.) Review request for Aurora, Kevin Sweeney and Mark Chu-Carroll. Repository: aurora Description --- Fixes two problems: - mocking was incorrect in `test_status_api_failure`. Turns out that Mock objects were being passed around and somehow resulted in the test case passing. - use of `threading.Event()` was broken in scheduler_client.py. I don't think it's possible to enter those branches. Diffs (updated) - src/main/python/apache/aurora/client/api/scheduler_client.py b400cb2dbdb35077fc2c4a6e161c2959a9217317 src/test/python/apache/aurora/client/api/test_scheduler_client.py 1cbfbf86e903d890baac7d34461109f9beaff442 src/test/python/apache/aurora/client/cli/test_api_from_cli.py 78f21d2f20cf71fa2dfe0614885d44d2948decd2 Diff: https://reviews.apache.org/r/26308/diff/ Testing --- ./pants src/test/python:all -vxs Thanks, Bill Farner
