Re: Review Request 32371: Refine types used in QuotaManager, share more functions/predicates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32371/#review77667 --- Ship it! Ship It! - Maxim Khutornenko On March 21, 2015, 7:42 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32371/ --- (Updated March 21, 2015, 7:42 p.m.) Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- I found myself in here while doing some exploratory changes to `TaskStore`. I started by using `IAssignedTask` and `ITaskConfig` where possible, rather than `IScheduledTask`, and that snowballed into a refactor to extract common functions and predicates. Feel free to push back if you feel this reduces readability or seems like code shuffling with little gain. One upside to this outcome is the imminent post-JDK8 lambda refactor will cause this diff to be a net reduction in code. Diffs - src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 39e930c4bfe716765908c6d78b58c831b6fb341b src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java b53086169aa53d27a39a01cadf8d3c4a8ecb68de src/main/java/org/apache/aurora/scheduler/updater/Updates.java 776278cb64c7ce0419a692143e458801e3b1c37f Diff: https://reviews.apache.org/r/32371/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32352: Making preemptor asynchronous. Part 3(final) - background service.
, visit: https://reviews.apache.org/r/32352/#review77609 --- On March 21, 2015, 2:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32352/ --- (Updated March 21, 2015, 2:19 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Summary of changes: - The PreemptorImpl now only hosts slot validation and task preemption logic and requires a write transaction. - PendingTaskProcessor is called every minute to pull all available PENDING tasks and search for preemption slots. - TaskScheduler has no connection to slot search anymore. It now completely relies on periodic PreemptionService to find available slots. - preemption_delay is reduced from 10 to 3 minutes. Benchmark refactoring will be addressed separately. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 5309e8140fff411da30ee87c1b3b1a55d6fdaeeb src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 1b9d741dba7b9c2663ff119cd44adc8403c0d257 src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 84791a272f245c729706af95d70c7f1631bfe99c src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 18a2e6032ba86ff7efab4d42a4d83798a1d06b06 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java 782e751f5b05391ebeee4d947570cc174dddece2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java da7b662c3ca4040221805cba81e5ac7b64fb3df4 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 29fe156da19f3c08af00a951bb3baccf2b97a6cb src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java f5c2128e90eb5c849d68431225661d1cfa7da0cc src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 0e2e958a053e5cee280b947f7349c076e2addb45 Diff: https://reviews.apache.org/r/32352/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 32352: Making preemptor asynchronous. Part 3(final) - background service.
On March 22, 2015, 10:04 a.m., Stephan Erb wrote: src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java, line 107 https://reviews.apache.org/r/32352/diff/1/?file=902121#file902121line107 Slotes computed here might have overlapping victims. A simple way to mitigate this problem could be to properly randomize the slave traversal order in `findPreemptionSlotFor` Thanks for bringing this up! The existing algorithm is optimized to work for a single pending task in a sequence with victim preemption. By splitting apart slot search and actual preemption we definitely lose some efficiency. I am afraid randomizing slave traversal will not address the problem completely and will bring some unpredictability in execution order. We need to optimize the N(pendingTasks) x M(slaves) problem in a more structured fashion, possibly reversing the traversal from tasks-slaves to slaves-tasks to further improve efficiency (as slave count is expected to be pending task count under normal conditions). I have already left a TODO to optimize slot search for multiple pending tasks. Will file a ticket to capture all these points. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32352/#review77348 --- On March 21, 2015, 2:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32352/ --- (Updated March 21, 2015, 2:19 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Summary of changes: - The PreemptorImpl now only hosts slot validation and task preemption logic and requires a write transaction. - PendingTaskProcessor is called every minute to pull all available PENDING tasks and search for preemption slots. - TaskScheduler has no connection to slot search anymore. It now completely relies on periodic PreemptionService to find available slots. - preemption_delay is reduced from 10 to 3 minutes. Benchmark refactoring will be addressed separately. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 5309e8140fff411da30ee87c1b3b1a55d6fdaeeb src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 1b9d741dba7b9c2663ff119cd44adc8403c0d257 src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 84791a272f245c729706af95d70c7f1631bfe99c src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 18a2e6032ba86ff7efab4d42a4d83798a1d06b06 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java 782e751f5b05391ebeee4d947570cc174dddece2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java da7b662c3ca4040221805cba81e5ac7b64fb3df4 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 29fe156da19f3c08af00a951bb3baccf2b97a6cb src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java f5c2128e90eb5c849d68431225661d1cfa7da0cc src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 0e2e958a053e5cee280b947f7349c076e2addb45 Diff: https://reviews.apache.org/r/32352/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 32369: Simplify port name association.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32369/#review77417 --- Ship it! Ship It! - Maxim Khutornenko On March 21, 2015, 5:49 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32369/ --- (Updated March 21, 2015, 5:49 p.m.) Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- I realized that `StateManagerImpl` had an unnecessarily-complex function to associate assigned ports with port names, with error checking along the way. I have simplified this function and relocated it closer to where ports are assigned. Diffs - src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 93fc05ef8b4cad187b1d36d05d2b4bb509ed60e7 src/main/java/org/apache/aurora/scheduler/state/StateManager.java 50ff4ec915352ead8c03f494f38522bcdeca3617 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 8721466b935da22ac9dd491b06f5e7ddc7f913e1 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java c44ff339b94cac20f4fb7e69a8e403fd63c132ca src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 17b170261b87506078d5463a5ed55fbc1cd8 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 06a19038f99f88d28c5548055bd82b0aebb461ac Diff: https://reviews.apache.org/r/32369/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32359: Adding a configurable delay into writing a backup file.
On March 23, 2015, 6:22 p.m., Aurora ReviewBot wrote: Master (a3a35e9) is red with this patch. ./build-support/jenkins/build.sh src.test.python.apache.aurora.client.factory . SUCCESS src.test.python.apache.aurora.client.hooks.hooked_api . SUCCESS src.test.python.apache.aurora.client.hooks.non_hooked_api . SUCCESS src.test.python.apache.aurora.common.test_aurora_job_key . SUCCESS src.test.python.apache.aurora.common.test_cluster . SUCCESS src.test.python.apache.aurora.common.test_cluster_option . SUCCESS src.test.python.apache.aurora.common.test_clusters . SUCCESS src.test.python.apache.aurora.common.test_http_signaler . SUCCESS src.test.python.apache.aurora.common.test_pex_version . SUCCESS src.test.python.apache.aurora.common.test_shellify . SUCCESS src.test.python.apache.aurora.common.test_transport . SUCCESS src.test.python.apache.aurora.config.test_base . SUCCESS src.test.python.apache.aurora.config.test_constraint_parsing . SUCCESS src.test.python.apache.aurora.config.test_loader . SUCCESS src.test.python.apache.aurora.config.test_thrift . SUCCESS src.test.python.apache.aurora.executor.common.announcer . SUCCESS src.test.python.apache.aurora.executor.common.directory_sandbox . SUCCESS src.test.python.apache.aurora.executor.common.executor_detector . SUCCESS src.test.python.apache.aurora.executor.common.executor_timeout . SUCCESS src.test.python.apache.aurora.executor.common.health_checker . SUCCESS src.test.python.apache.aurora.executor.common.kill_manager . SUCCESS src.test.python.apache.aurora.executor.common.path_detector . SUCCESS src.test.python.apache.aurora.executor.common.status_checker . SUCCESS src.test.python.apache.aurora.executor.common.task_info . SUCCESS src.test.python.apache.aurora.executor.executor_base . SUCCESS src.test.python.apache.aurora.executor.executor_vars . SUCCESS src.test.python.apache.aurora.executor.gc_executor . FAILURE src.test.python.apache.aurora.executor.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . SUCCESS src.test.python.apache.thermos.cli.commands.commands . SUCCESS src.test.python.apache.thermos.cli.common . SUCCESS src.test.python.apache.thermos.cli.main . SUCCESS src.test.python.apache.thermos.common.test_pathspec . SUCCESS src.test.python.apache.thermos.core.test_runner_integration . SUCCESS src.test.python.apache.thermos.monitoring.test_disk . SUCCESS FAILURE [31m FAILURE[0m I will refresh this build result if you post a review containing @ReviewBot retry @ReviewBot retry - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32359/#review77436 --- On March 23, 2015, 6:10 p.m., Maxim Khutornenko wrote
Re: Review Request 32359: Adding a configurable delay into writing a backup file.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32359/ --- (Updated March 23, 2015, 6:10 p.m.) Review request for Aurora and Bill Farner. Changes --- Stephan's comments. Bugs: AURORA-1211 https://issues.apache.org/jira/browse/AURORA-1211 Repository: aurora Description --- Adding a configurable delay into writing a backup file. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java a6b187d888726b921733a36fcaecdab97bdef094 src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java 569648aea2ccdb57663d16b7c837fd2677694419 src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 5853c037d53e707e5df434fc661cd285ed9ecfc4 src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java ebe551739fb6b7132ce666ad9b3c5a86e90a5363 Diff: https://reviews.apache.org/r/32359/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32359: Adding a configurable delay into writing a backup file.
On March 21, 2015, 12:14 p.m., Stephan Erb wrote: src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java, line 66 https://reviews.apache.org/r/32359/diff/1/?file=902147#file902147line66 The description should state the intent of the option more explicitly. Even reading the code did not help me much. Only the description in the associated Jira ticket helps to undestand why this option is important and might need to be tuned. Thanks, added more details. Let me know if you still think it's not clear or have another alternative. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32359/#review77337 --- On March 21, 2015, 2:22 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32359/ --- (Updated March 21, 2015, 2:22 a.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1211 https://issues.apache.org/jira/browse/AURORA-1211 Repository: aurora Description --- Adding a configurable delay into writing a backup file. Diffs - src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java a6b187d888726b921733a36fcaecdab97bdef094 src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java 569648aea2ccdb57663d16b7c837fd2677694419 src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 5853c037d53e707e5df434fc661cd285ed9ecfc4 src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java ebe551739fb6b7132ce666ad9b3c5a86e90a5363 Diff: https://reviews.apache.org/r/32359/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 32353: Renaming PreemptionSlotFinder.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32353/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- IDE-driven renaming. Prerequisite for the final preemptor refactoring step. Diffs - src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 18a2e6032ba86ff7efab4d42a4d83798a1d06b06 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java 80bd13a192bda64759b5a93749ec47ddfeae504a src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 Diff: https://reviews.apache.org/r/32353/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 32359: Adding a configurable delay into writing a backup file.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32359/ --- Review request for Aurora and Bill Farner. Bugs: AURORA-1211 https://issues.apache.org/jira/browse/AURORA-1211 Repository: aurora Description --- Adding a configurable delay into writing a backup file. Diffs - src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java a6b187d888726b921733a36fcaecdab97bdef094 src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java 569648aea2ccdb57663d16b7c837fd2677694419 src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 5853c037d53e707e5df434fc661cd285ed9ecfc4 src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java ebe551739fb6b7132ce666ad9b3c5a86e90a5363 Diff: https://reviews.apache.org/r/32359/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 32352: Making preemptor asynchronous. Part 3(final) - background service.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32352/ --- Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Summary of changes: - The PreemptorImpl now only hosts slot validation and task preemption logic and requires a write transaction. - PendingTaskProcessor is called every minute to pull all available PENDING tasks and search for preemption slots. - TaskScheduler has no connection to slot search anymore. It now completely relies on periodic PreemptionService to find available slots. - preemption_delay is reduced from 10 to 3 minutes. Benchmark refactoring will be addressed separately. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 5309e8140fff411da30ee87c1b3b1a55d6fdaeeb src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 1b9d741dba7b9c2663ff119cd44adc8403c0d257 src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 84791a272f245c729706af95d70c7f1631bfe99c src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 18a2e6032ba86ff7efab4d42a4d83798a1d06b06 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java 782e751f5b05391ebeee4d947570cc174dddece2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java da7b662c3ca4040221805cba81e5ac7b64fb3df4 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 29fe156da19f3c08af00a951bb3baccf2b97a6cb src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java f5c2128e90eb5c849d68431225661d1cfa7da0cc src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 0e2e958a053e5cee280b947f7349c076e2addb45 Diff: https://reviews.apache.org/r/32352/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 32220: Making preemptor asynchronous. Part 2 - async handling.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32220/ --- (Updated March 19, 2015, 11:39 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Bill's comments. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- This diff makes preemption asynchronous wrt the scheduling loop. New flow works as follows: - TaskScheduler is unable to schedule a task and calls into PreemptorImpl.attemptPreemptionFor() to acquire a reservation. - PreemptorImpl keeps track of found preemption slots in PreemptionSlotCache, fires an async slot search request and replies back with empty result. - A search is finished and a slot (if found) is added into internal slot cache. - TaskScheduler calls into attemptPreemptionFor(), finds a preemption slot, validates a slot is still valid and preempts tasks. A reservation for a slave is created. This is still an intermediate milestone on the way to a fully independent background preemptor. Benchmark refactoring will be addressed in a separate diff. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/async/OfferManager.java 7d2cb46aa86dd4c3c6d53848725eed1542307ebd src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java e748b42eaa8f54e0cf1a0a883da4aceff3d7a3b8 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 1808b71546423dfe80ccb1902e8cebd545674a27 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java 801a6d7f3cb0e9987e2029fd8c4c89015e8d3b65 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java dbfebf99bc6028faf433a69db4308a239ff61290 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java cfbc1a039262d92481ded2733d50ac51293a5b91 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java e329358f70028f52a807cd987378cbc002af36a9 Diff: https://reviews.apache.org/r/32220/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32220: Making preemptor asynchronous. Part 2 - async handling.
On March 19, 2015, 9:23 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java, line 45 https://reviews.apache.org/r/32220/diff/2/?file=899548#file899548line45 This should refer to the parameter, not the guice binding annotation. Done. On March 19, 2015, 9:23 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java, line 87 https://reviews.apache.org/r/32220/diff/2/?file=899548#file899548line87 `synchronized` should be unnecessary, `Cache` is expected to be thread safe, and we're not doing multiple operations per method here. Yeah, I used synchronized when I was not sure about the final functionality but now it's clearly redundant. dropped. On March 19, 2015, 9:23 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java, line 124 https://reviews.apache.org/r/32220/diff/2/?file=899549#file899549line124 Validates that a previously-found Done. On March 19, 2015, 9:23 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java, line 84 https://reviews.apache.org/r/32220/diff/2/?file=899552#file899552line84 Given that you probably want a singleton PreemptionSlotCache, don't forget to bind that as such here. Good idea. Done. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32220/#review77106 --- On March 19, 2015, 12:29 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32220/ --- (Updated March 19, 2015, 12:29 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- This diff makes preemption asynchronous wrt the scheduling loop. New flow works as follows: - TaskScheduler is unable to schedule a task and calls into PreemptorImpl.attemptPreemptionFor() to acquire a reservation. - PreemptorImpl keeps track of found preemption slots in PreemptionSlotCache, fires an async slot search request and replies back with empty result. - A search is finished and a slot (if found) is added into internal slot cache. - TaskScheduler calls into attemptPreemptionFor(), finds a preemption slot, validates a slot is still valid and preempts tasks. A reservation for a slave is created. This is still an intermediate milestone on the way to a fully independent background preemptor. Benchmark refactoring will be addressed in a separate diff. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferManager.java 7d2cb46aa86dd4c3c6d53848725eed1542307ebd src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java e748b42eaa8f54e0cf1a0a883da4aceff3d7a3b8 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 1808b71546423dfe80ccb1902e8cebd545674a27 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java 801a6d7f3cb0e9987e2029fd8c4c89015e8d3b65 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java dbfebf99bc6028faf433a69db4308a239ff61290 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java cfbc1a039262d92481ded2733d50ac51293a5b91 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java e329358f70028f52a807cd987378cbc002af36a9 Diff: https://reviews.apache.org/r/32220/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32220: Making preemptor asynchronous. Part 2 - async handling.
On March 19, 2015, 9:41 p.m., Zameer Manji wrote: src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java, line 254 https://reviews.apache.org/r/32220/diff/2/?file=899549#file899549line254 This is a bit confusing to see .getSlaveId and then construct an instance of SlaveID. Perhaps preemptionSlot should return a SlaveID instance? That's what I actually started with. It quickly resulted in more places like this one (both in source and test code), so I reverted to String to have this ugliness contained in a single place. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32220/#review77112 --- On March 19, 2015, 12:29 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32220/ --- (Updated March 19, 2015, 12:29 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- This diff makes preemption asynchronous wrt the scheduling loop. New flow works as follows: - TaskScheduler is unable to schedule a task and calls into PreemptorImpl.attemptPreemptionFor() to acquire a reservation. - PreemptorImpl keeps track of found preemption slots in PreemptionSlotCache, fires an async slot search request and replies back with empty result. - A search is finished and a slot (if found) is added into internal slot cache. - TaskScheduler calls into attemptPreemptionFor(), finds a preemption slot, validates a slot is still valid and preempts tasks. A reservation for a slave is created. This is still an intermediate milestone on the way to a fully independent background preemptor. Benchmark refactoring will be addressed in a separate diff. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferManager.java 7d2cb46aa86dd4c3c6d53848725eed1542307ebd src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java e748b42eaa8f54e0cf1a0a883da4aceff3d7a3b8 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 1808b71546423dfe80ccb1902e8cebd545674a27 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java 801a6d7f3cb0e9987e2029fd8c4c89015e8d3b65 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java dbfebf99bc6028faf433a69db4308a239ff61290 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java cfbc1a039262d92481ded2733d50ac51293a5b91 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java e329358f70028f52a807cd987378cbc002af36a9 Diff: https://reviews.apache.org/r/32220/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32276: Fix error listing active updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32276/#review77152 --- Ship it! Ship It! - Maxim Khutornenko On March 20, 2015, 1:46 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32276/ --- (Updated March 20, 2015, 1:46 a.m.) Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- Fixes this error: ``` $ aurora update list devcluster/vagrant/test/http_example --status active Traceback (most recent call last): File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 272, in execute self.execute_entry(entry_point, args) File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 320, in execute_entry runner(entry_point) File /usr/local/bin/aurora/.bootstrap/_pex/pex.py, line 343, in execute_pkg_resources runner() File /usr/local/bin/aurora/apache/aurora/client/cli/client.py, line 95, in proxy_main File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 329, in execute File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 306, in _execute File /usr/local/bin/aurora/apache/aurora/client/cli/__init__.py, line 382, in execute File /usr/local/bin/aurora/apache/aurora/client/cli/update.py, line 318, in execute TypeError: unhashable type: 'set' ``` Diffs - src/main/python/apache/aurora/client/cli/update.py 2168e99a315dd2916086100589c8345cd3a2c4ff Diff: https://reviews.apache.org/r/32276/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32225: Adding preemptor jmh benchmark
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32225/ --- (Updated March 20, 2015, 12:25 a.m.) Review request for Aurora and Bill Farner. Changes --- Rebased. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description (updated) --- Adding a preemptor slot search perf benchmark. Below are the results for synchronous (Before) and asynchronous (After) preemptor. Before: ``` Benchmark Mode Cnt ScoreError Units SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100781243.004 ± 9308.450 ns/op SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 1205278.826 ± 19800.452 ns/op SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark avgt 100 77048458.974 ± 918593.702 ns/op SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100769919.326 ± 18963.264 ns/op ``` After: ``` Benchmark Mode CntScore Error Units SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark avgt 100 3062.264 ± 323.854 ns/op SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 10022135.031 ± 703.886 ns/op SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 283028.184 ± 1954.987 ns/op SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark avgt 100 3338470.414 ± 31189.009 ns/op SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 10022177.423 ± 589.332 ns/op ``` Result analysis is here: https://reviews.apache.org/r/31739/ Diffs (updated) - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java ad49effdaf700bb9d5715aa5bdd1a5d0b276f83f src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java f817ccd23644de5aa03fe42be3a5bc2b63683a9d src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java c9d10e4cec44045806ec2d75d8c158dc40d7de98 Diff: https://reviews.apache.org/r/32225/diff/ Testing --- ./gradlew jmh Thanks, Maxim Khutornenko
Re: Review Request 32171: Change update list subcommand to accept a hierarchy.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/#review77102 --- Ship it! src/main/python/apache/aurora/client/cli/update.py https://reviews.apache.org/r/32171/#comment124993 How about a special blocked group to display only the updates blocked due to lack of heartbeat? Could be quite useful for monitoring of blocked updates. src/main/python/apache/aurora/client/cli/update.py https://reviews.apache.org/r/32171/#comment124992 Move to previous line? src/main/python/apache/aurora/client/cli/update.py https://reviews.apache.org/r/32171/#comment124997 Recommend adding a phrase explaining the behavior in case multiple groups/statuses are specified. src/main/python/apache/aurora/client/cli/update.py https://reviews.apache.org/r/32171/#comment124999 While it's converted to set by thrift anyway, I'd still recommend using a set() here to explicitly assert no duplicates are ever accepted. - Maxim Khutornenko On March 19, 2015, 9:09 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/ --- (Updated March 19, 2015, 9:09 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1168 https://issues.apache.org/jira/browse/AURORA-1168 Repository: aurora Description --- NOTE: I also am proposing with this change that the output format of `aurora update list` be line-oriented and more concise (see test changes for examples). The idea is to follow up ~immediately with AURORA-1206. The proposal is for this command to let you retrieve update identifiers, and `update status` (whose name may change) will allow you to drill into updates. I'm open to input on this. Diffs - src/main/python/apache/aurora/client/cli/update.py f025d46d50592156e2455313890e981722ab63a5 src/test/python/apache/aurora/client/cli/test_supdate.py cb66439a778349fc5add4985a7395655c9e1328a Diff: https://reviews.apache.org/r/32171/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32220: Making preemptor asynchronous. Part 2 - async handling.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32220/ --- (Updated March 19, 2015, 12:29 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Dropping PreemptionSlotCache visibility. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- This diff makes preemption asynchronous wrt the scheduling loop. New flow works as follows: - TaskScheduler is unable to schedule a task and calls into PreemptorImpl.attemptPreemptionFor() to acquire a reservation. - PreemptorImpl keeps track of found preemption slots in PreemptionSlotCache, fires an async slot search request and replies back with empty result. - A search is finished and a slot (if found) is added into internal slot cache. - TaskScheduler calls into attemptPreemptionFor(), finds a preemption slot, validates a slot is still valid and preempts tasks. A reservation for a slave is created. This is still an intermediate milestone on the way to a fully independent background preemptor. Benchmark refactoring will be addressed in a separate diff. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/async/OfferManager.java 7d2cb46aa86dd4c3c6d53848725eed1542307ebd src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java e748b42eaa8f54e0cf1a0a883da4aceff3d7a3b8 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 1808b71546423dfe80ccb1902e8cebd545674a27 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java 801a6d7f3cb0e9987e2029fd8c4c89015e8d3b65 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java dbfebf99bc6028faf433a69db4308a239ff61290 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java cfbc1a039262d92481ded2733d50ac51293a5b91 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java e329358f70028f52a807cd987378cbc002af36a9 Diff: https://reviews.apache.org/r/32220/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 32225: Adding preemptor jmh benchmark
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32225/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- Adding a preemptor slot search perf benchmark. Will not apply cleanly, diffed against https://reviews.apache.org/r/32220/ Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java ad49effdaf700bb9d5715aa5bdd1a5d0b276f83f src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java dbfebf99bc6028faf433a69db4308a239ff61290 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java c9d10e4cec44045806ec2d75d8c158dc40d7de98 Diff: https://reviews.apache.org/r/32225/diff/ Testing --- ./gradlew jmh Thanks, Maxim Khutornenko
Re: Review Request 32208: Reduce loglevel for insufficient GC resources to fine
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32208/#review76919 --- Ship it! Ship It! - Maxim Khutornenko On March 18, 2015, 6:28 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32208/ --- (Updated March 18, 2015, 6:28 p.m.) Review request for Aurora. Repository: aurora Description --- Reduce loglevel for insufficient GC resources to fine Diffs - src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 31aa2bbaab3d97875493ad75c4d2c7c82ac7fa58 Diff: https://reviews.apache.org/r/32208/diff/ Testing --- ./gradlew -Pq build Thanks, Stephan Erb
Review Request 32164: Moving pending task search into PreemptorImpl
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32164/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- Addressing a TODO left after refactoring in step 1. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 9b7a3f16ad489c296d5e3513a74264a0620942a6 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java e5cc48625cf019f1b6bb85749711528bf5dee4cd src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java e11db4e0e04fe3ca089defc0304333cbb8bf7624 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 93cab3414e3cd76e02b34c439413e625037bf90c src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java f27ca62397e60b114f0c570cdfe9323a6c825645 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java 9a6e94dc941025fd272495de0c46f77b793e867c Diff: https://reviews.apache.org/r/32164/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32141/#review76774 --- Ship it! src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java https://reviews.apache.org/r/32141/#comment12 You probably want @Nullable here as well. src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java https://reviews.apache.org/r/32141/#comment124445 same here - Maxim Khutornenko On March 17, 2015, 6:55 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32141/ --- (Updated March 17, 2015, 6:55 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to inherit from it. This gives us a place to put annotations like the AuthorizingParam one introduced in this review without needing to copy-paste them when we override a new method. A future diff will use these annotations to determine which permission a method call needs by inspecting the annotated parameter. I created a new interface to enable DRY - otherwise I'd need to annotate both ForwardingThrift and SchedulerThriftInterface and keep them in sync. Diffs - src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c60d1e78e0f5c474ff0dcf6393e0c305d865bded src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java b1d633602f3fb021a026a087b91d41580798eeaf src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java f0e40b646092e96955fddc46c3a1e62a8237b00f src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java df6b53a524b005cd5fabb099fd0c08d83e3b287d src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java ee98f66de7f671018fa0a0b4894f114c7a283eda src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 3900c2228038668576cdbb37e87127246a33317c src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 7b1bf2ef8b413d2c1a08b41722a04af091305304 src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java 7e20aaa6836bd205261afe5b1244fb6af8a56356 src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java aae5cd7709abe3896c2ae06c218a0c90ca11c576 Diff: https://reviews.apache.org/r/32141/diff/ Testing --- ./gradlew build Thanks, Kevin Sweeney
Re: Review Request 32181: Only warn about insufficient GC resources when actually needed
On March 17, 2015, 11:12 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java, line 208 https://reviews.apache.org/r/32181/diff/1/?file=898407#file898407line208 This reordering will result in pulsing the hostname even when an offer is insufficient for GC thus leading to missed collections. I'd much rather suppress the warning (e.g. change it to Level.FINE) than risk a missed collection. Stephan Erb wrote: What about extracting the pulsing from `isTimeToCollect` and performing it directly in `willUse`? Given this feature is going away soon, I'd rather not invest any time in improving it unless absolutely necessary. Are there any other concerns besides warnings in scheduler log? If not, perhaps lowering the logging level is the way to go. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32181/#review76841 --- On March 17, 2015, 10:50 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32181/ --- (Updated March 17, 2015, 10:50 p.m.) Review request for Aurora. Repository: aurora Description --- Only warn about insufficient GC resources when actually needed Whenever a garbage collection run finishes, the launcher is offered the same resources again. Probably due to rounding errors, these are not considered sufficient for another run. By changing the order of time check and resource check we prevent unnecessary warnings about these small offers. Diffs - src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 31aa2bbaab3d97875493ad75c4d2c7c82ac7fa58 src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419 Diff: https://reviews.apache.org/r/32181/diff/ Testing --- ./gradlew -Pq build Thanks, Stephan Erb
Re: Review Request 32171: Change update list subcommand to accept a hierarchy.
On March 18, 2015, 12:04 a.m., Kevin Sweeney wrote: src/test/python/apache/aurora/client/cli/test_supdate.py, lines 404-406 https://reviews.apache.org/r/32171/diff/3/?file=898440#file898440line404 How would you feel about including all of the currently available data with a header row? Also instead of raw epoch time how about ISO 8601 - using the spaceless 'T' form (`2015-03-15T13:27:36+00:00`) How would you feel about including all of the currently available data with a header row? -1 on shoving all available data into a single header row. I'd rather see a clean output here and rely on `aurora update info` for drill downs. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/#review76854 --- On March 17, 2015, 11:54 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32171/ --- (Updated March 17, 2015, 11:54 p.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Bugs: AURORA-1168 https://issues.apache.org/jira/browse/AURORA-1168 Repository: aurora Description --- NOTE: I also am proposing with this change that the output format of `aurora update list` be line-oriented and more concise (see test changes for examples). The idea is to follow up ~immediately with AURORA-1206. The proposal is for this command to let you retrieve update identifiers, and `update status` (whose name may change) will allow you to drill into updates. I'm open to input on this. Diffs - src/main/python/apache/aurora/client/cli/update.py f025d46d50592156e2455313890e981722ab63a5 src/test/python/apache/aurora/client/cli/test_supdate.py cb66439a778349fc5add4985a7395655c9e1328a Diff: https://reviews.apache.org/r/32171/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32164: Moving pending task search into PreemptorImpl
On March 17, 2015, 6:05 p.m., Bill Farner wrote: I actually meant this should be outside the preemptor altogether. The preemptor is being called, and then internally deciding the caller should have not called in the first place. I claim this is odd behavior. I think it would make sense, instead, if the caller opened a transaction, and continued that transaction into the preemptor once it has deemed the task is `PENDING` and has been in that state long enough to warrant a preemption search. After the final refactoring step is implemented, the preemptor will be guided by a scheduled background worker most likely residing within the PreemptorImpl itself. So, it will be the only caller and consumer of pending tasks. I don't think it's worth purging pending task search out at this stage just to return it back shortly after. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32164/#review76753 --- On March 17, 2015, 5:19 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32164/ --- (Updated March 17, 2015, 5:19 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Addressing a TODO left after refactoring in step 1. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 9b7a3f16ad489c296d5e3513a74264a0620942a6 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java e5cc48625cf019f1b6bb85749711528bf5dee4cd src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java e11db4e0e04fe3ca089defc0304333cbb8bf7624 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 93cab3414e3cd76e02b34c439413e625037bf90c src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java f27ca62397e60b114f0c570cdfe9323a6c825645 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java 9a6e94dc941025fd272495de0c46f77b793e867c Diff: https://reviews.apache.org/r/32164/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32141: Introduce AnnotatedAuroraAdmin superclass for annotations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32141/#review76741 --- src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java https://reviews.apache.org/r/32141/#comment124360 What's the guidance behind usage of @Nullable here? Technically, the `shardsId` could come as NULL from thrift as well as `TaskQuery` in `killTasks`. Both are required for RPC operation but the latter one is annotated with @Nullable. - Maxim Khutornenko On March 17, 2015, 1:29 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32141/ --- (Updated March 17, 2015, 1:29 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- Add an AnnotatedAuroraAdmin subclass for AuroraAdmin.Iface and change code to inherit from it. This gives us a place to put annotations like the AuthorizingParam one introduced in this review without needing to copy-paste them when we override a new method. A future diff will use these annotations to determine which permission a method call needs by inspecting the annotated parameter. I created a new interface to enable DRY - otherwise I'd need to annotate both ForwardingThrift and SchedulerThriftInterface and keep them in sync. Diffs - src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c60d1e78e0f5c474ff0dcf6393e0c305d865bded src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java b1d633602f3fb021a026a087b91d41580798eeaf src/main/java/org/apache/aurora/scheduler/thrift/aop/AnnotatedAuroraAdmin.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java f0e40b646092e96955fddc46c3a1e62a8237b00f src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java df6b53a524b005cd5fabb099fd0c08d83e3b287d src/test/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptorTest.java ee98f66de7f671018fa0a0b4894f114c7a283eda src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 3900c2228038668576cdbb37e87127246a33317c src/test/java/org/apache/aurora/scheduler/thrift/aop/MockDecoratedThrift.java 7b1bf2ef8b413d2c1a08b41722a04af091305304 src/test/java/org/apache/aurora/scheduler/thrift/aop/ServerInfoInterceptorTest.java 7e20aaa6836bd205261afe5b1244fb6af8a56356 src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java aae5cd7709abe3896c2ae06c218a0c90ca11c576 Diff: https://reviews.apache.org/r/32141/diff/ Testing --- ./gradlew build Thanks, Kevin Sweeney
Re: Review Request 32106: Changed the updater to not update an instance if only the job owner changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32106/#review76568 --- src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java https://reviews.apache.org/r/32106/#comment124146 There is actually a better place to do this type of action [1]. If not done there, the update will still have to iterate over all instances and generate a lot of instance event noise. Also, the UI will show these instances in the update working set, which does not reflect the NOOP nature of this action. [1] - https://github.com/apache/incubator-aurora/blob/master/src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java - Maxim Khutornenko On March 16, 2015, 3:13 p.m., Steve Niemitz wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32106/ --- (Updated March 16, 2015, 3:13 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1200 https://issues.apache.org/jira/browse/AURORA-1200 Repository: aurora Description --- Changed the updater to not update an instance if the job owner changes Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 14753cf5ef35d5133ca5029f3a465df884756070 src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 09c147e76d7c2c130a1fdd85459c45395fee7dde Diff: https://reviews.apache.org/r/32106/diff/ Testing --- Thanks, Steve Niemitz
Re: Review Request 32078: Remove the populatedDEPRECATED thrift field.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32078/#review76632 --- Ship it! Ship It! - Maxim Khutornenko On March 16, 2015, 5:39 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32078/ --- (Updated March 16, 2015, 5:39 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-975 https://issues.apache.org/jira/browse/AURORA-975 Repository: aurora Description --- Remove the populatedDEPRECATED thrift field. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 8386ee36117c8135c7f855a496e1e887904b23dd src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 3876290c3c116ae29a0f386c899ea213efdba318 src/main/python/apache/aurora/client/api/updater.py 7fd6fdacafb7c3d2b30e1d9de2c8a48a88e0e943 src/main/python/apache/aurora/client/base.py 352d3f07148eff3c616f1f2dba861de6a023acc7 src/main/python/apache/aurora/client/cli/jobs.py d6633d93a09f5203219d680cf3780ba1f17d0969 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 3cf8dfde64d38d43e72dd1e7ccc901584b59cbe9 src/test/python/apache/aurora/client/api/test_updater.py 7f3bc28aef52ed171a24ce2a98718afb6bfb1b54 src/test/python/apache/aurora/client/cli/test_diff.py 054e6fbc1b1a56b4818b2a8f08b06e8c64bcfc10 src/test/python/apache/aurora/client/cli/test_restart.py 4f20f6bf3db8d9cae35ec6458777990c763cdbd1 src/test/python/apache/aurora/client/cli/test_update.py 9bfbbea85c6f123076fb570ea91d154dd11c2d78 src/test/python/apache/aurora/client/test_base.py 06c0b436b5104c9b3afbf80499d3bfc66e7ef2f4 Diff: https://reviews.apache.org/r/32078/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32077: Rename beta-update subcommand to update.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32077/#review76636 --- Ship it! Ship It! - Maxim Khutornenko On March 16, 2015, 5:57 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32077/ --- (Updated March 16, 2015, 5:57 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1160 https://issues.apache.org/jira/browse/AURORA-1160 Repository: aurora Description --- Rename beta-update subcommand to update. Diffs - docs/client-commands.md f0769b5db58971c345090ee584fa5fc11e2a057a docs/configuration-reference.md af0386202844a0ab706c691068d587cefc75becf docs/hooks.md 23cc207700ca1cca8ae5827345a5812ab2af9d61 examples/vagrant/upstart/aurora-scheduler.conf f85127d9d565c3c91eca914f73f28005d763234c src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c60d1e78e0f5c474ff0dcf6393e0c305d865bded src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java b1d633602f3fb021a026a087b91d41580798eeaf src/main/python/apache/aurora/client/cli/update.py cced1080655a0d6648883c16edf07f7ad75a2ca1 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 89b9ce0a4150a88e8d9099e711659b591dcdd947 src/test/python/apache/aurora/client/cli/test_supdate.py a237da92229e21253aaca488a89af2979d94ce48 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh f255f2d0a390359da4f442505fc7b8b492dc06bb Diff: https://reviews.apache.org/r/32077/diff/ Testing --- Running end-to-end tests in an existing vagrant setup, and will repeat with a fresh one. Will report back before committing. Thanks, Bill Farner
Re: Review Request 32014: Adding more logging into MaintenanceController.
On March 13, 2015, 2:30 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java, line 255 https://reviews.apache.org/r/32014/diff/1/?file=892576#file892576line255 This is a pretty weird log entry to have. Can it be done in the client instead? Maxim Khutornenko wrote: Client already has logs but that's not enough to reconstruct the behavior when maintenance stalls. We need to see both sides of the story to properly troubleshoot host draining issues. Bill Farner wrote: This one i still don't follow - AFAICT it's literally printing the response handed directly to the client, which is the only caller if i'm reading correctly. Maxim Khutornenko wrote: Having it only on the client hampers investigation when client logs are missing or incomplete. This logging completes the scheduler view of the maintenance story, which can be reconstructed any time we have a question and the client logs a long gone. Chatted with Bill offline. While it's certainly tempting to add a response logging in the scheduler, it does not align with our general approach not to. Perhaps we have a better answer with AURORA-189. I will drop this particular statement and add a more verbose status logging on the client instead. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32014/#review76332 --- On March 13, 2015, 1:23 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32014/ --- (Updated March 13, 2015, 1:23 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Also added missing test coverage. Diffs - config/legacy_untested_classes.txt 06e130fae6bdc43b5e8aff182a530eac65acca53 src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java b6f642e319b790544bc538098e7cb78b1bb49997 src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java bd031a5e0b7d2923e36ca5958c5074f40dc64848 Diff: https://reviews.apache.org/r/32014/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32014: Adding more logging into MaintenanceController.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32014/ --- (Updated March 13, 2015, 5:59 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1170 https://issues.apache.org/jira/browse/AURORA-1170 Repository: aurora Description --- Also added missing test coverage. Diffs - config/legacy_untested_classes.txt 06e130fae6bdc43b5e8aff182a530eac65acca53 src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java b6f642e319b790544bc538098e7cb78b1bb49997 src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java bd031a5e0b7d2923e36ca5958c5074f40dc64848 Diff: https://reviews.apache.org/r/32014/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32014: Adding more logging into MaintenanceController.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32014/ --- (Updated March 13, 2015, 8:05 p.m.) Review request for Aurora and Bill Farner. Changes --- Bill's comments. Bugs: AURORA-1170 https://issues.apache.org/jira/browse/AURORA-1170 Repository: aurora Description --- Also added missing test coverage. Diffs (updated) - config/legacy_untested_classes.txt 06e130fae6bdc43b5e8aff182a530eac65acca53 src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java b6f642e319b790544bc538098e7cb78b1bb49997 src/main/python/apache/aurora/admin/host_maintenance.py 8fa5182fae509493def7175635fbe1cb79fa3eec src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java bd031a5e0b7d2923e36ca5958c5074f40dc64848 src/test/python/apache/aurora/admin/test_host_maintenance.py bb586700814a96b3e83d11728b462a7765e81bc1 Diff: https://reviews.apache.org/r/32014/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32014: Adding more logging into MaintenanceController.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32014/ --- (Updated March 13, 2015, 8:06 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1170 https://issues.apache.org/jira/browse/AURORA-1170 Repository: aurora Description --- Also added missing test coverage. Diffs (updated) - config/legacy_untested_classes.txt 06e130fae6bdc43b5e8aff182a530eac65acca53 src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java b6f642e319b790544bc538098e7cb78b1bb49997 src/main/python/apache/aurora/admin/host_maintenance.py 8fa5182fae509493def7175635fbe1cb79fa3eec src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java bd031a5e0b7d2923e36ca5958c5074f40dc64848 src/test/python/apache/aurora/admin/test_host_maintenance.py bb586700814a96b3e83d11728b462a7765e81bc1 Diff: https://reviews.apache.org/r/32014/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32014: Adding more logging into MaintenanceController.
On March 13, 2015, 2:30 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java, line 255 https://reviews.apache.org/r/32014/diff/1/?file=892576#file892576line255 This is a pretty weird log entry to have. Can it be done in the client instead? Maxim Khutornenko wrote: Client already has logs but that's not enough to reconstruct the behavior when maintenance stalls. We need to see both sides of the story to properly troubleshoot host draining issues. Bill Farner wrote: This one i still don't follow - AFAICT it's literally printing the response handed directly to the client, which is the only caller if i'm reading correctly. Having it only on the client hampers investigation when client logs are missing or incomplete. This logging completes the scheduler view of the maintenance story, which can be reconstructed any time we have a question and the client logs a long gone. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32014/#review76332 --- On March 13, 2015, 1:23 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32014/ --- (Updated March 13, 2015, 1:23 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Also added missing test coverage. Diffs - config/legacy_untested_classes.txt 06e130fae6bdc43b5e8aff182a530eac65acca53 src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java b6f642e319b790544bc538098e7cb78b1bb49997 src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java bd031a5e0b7d2923e36ca5958c5074f40dc64848 Diff: https://reviews.apache.org/r/32014/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 32048: Build all components before running the end-to-end tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32048/#review76375 --- Ship it! Ship It! - Maxim Khutornenko On March 13, 2015, 3:45 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32048/ --- (Updated March 13, 2015, 3:45 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1191 https://issues.apache.org/jira/browse/AURORA-1191 Repository: aurora Description --- Build all components before running the end-to-end tests. Diffs - src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh f9f81891a0fad765255cea06bfc4f2ab1e794c6a Diff: https://reviews.apache.org/r/32048/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31821: Making preemptor asynchronous. Part 1 - extracting slot finder.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/ --- (Updated March 10, 2015, 5:26 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Bill's comments. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Extracting PreemptorSlotFinder to be reused for slot validation in later stages. The changes are very minimal and mostly around metric handling and test code. Also added missing test coverage. Diffs (updated) - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 701b9052696337766cb233c865cb9fbb4907071e src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java e093ca54521ffb9399bb97ce60f510331af70853 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictim.java 80c2023f46b63753dcec6a555dba626720a1925a src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java bddb9647493b3e7a58c40d4b477a06161c1388a2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java ae56d1e09322869eedd7a27586cd6f96edd64e0a src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 85b3874a36ed07c684f26da172952c932cff707a src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 58733bdc4dd6de29ccead5cb0a267286e8dc0656 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 891cc098cca99e84ba014b7131106ceb0b429b5f src/test/java/org/apache/aurora/scheduler/async/preemptor/ClusterStateImplTest.java 7207867813b0d096772dbc7f92fc1c76937e9831 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictimTest.java b0380b3fabb45be8ace55cfcf38ce15ef8040188 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 83680769611878886da04e1794b321aa1986e678 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 020b67187a18bba64d9b562c3a6c0969fc85d469 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31821/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31966: Add client support for including messages when changing update state.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31966/#review76243 --- Ship it! src/main/python/apache/aurora/client/api/__init__.py https://reviews.apache.org/r/31966/#comment123752 Spacing seems off here and below. src/main/python/apache/aurora/client/cli/update.py https://reviews.apache.org/r/31966/#comment123755 I'd also include a short '-m' name to make typing less tedious. src/main/python/apache/aurora/client/cli/update.py https://reviews.apache.org/r/31966/#comment123753 While not entirely consistent, the majority of our CommandOption help strings start with a capital. src/main/python/apache/aurora/client/cli/update.py https://reviews.apache.org/r/31966/#comment123754 Mind sorting them alphabetically? - Maxim Khutornenko On March 12, 2015, 1:27 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31966/ --- (Updated March 12, 2015, 1:27 a.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- More plumbing. Diffs - src/main/python/apache/aurora/client/api/__init__.py 4025781963a61821d111e6347ae02d27459fd8cd src/main/python/apache/aurora/client/cli/update.py 37cc49880eab2ac0879945714634a4938b603aae src/main/python/apache/aurora/client/hooks/hooked_api.py 60a5aad7c3a7388154654673a6669c93be716635 src/test/python/apache/aurora/api_util.py 70599555c8b13073e51adee31662df74afe67edb src/test/python/apache/aurora/client/api/test_api.py d211fb975db01e72a88312c28078e06cb622d83c src/test/python/apache/aurora/client/cli/test_supdate.py 370a46b46d8fe9499468e0792f181e79e8042b61 Diff: https://reviews.apache.org/r/31966/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31913: Added 'none' host maintenance grouping function.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31913/#review76283 --- src/test/python/apache/aurora/client/test_base.py https://reviews.apache.org/r/31913/#comment123803 Mind adding a host maintenance test instead? I am concerned this may drift over time and will no longer represent how the grouping is actually used. - Maxim Khutornenko On March 11, 2015, 2:53 a.m., David Robinson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31913/ --- (Updated March 11, 2015, 2:53 a.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1181 https://issues.apache.org/jira/browse/AURORA-1181 Repository: aurora Description --- Added 'none' host maintenance grouping function. Diffs - src/main/python/apache/aurora/client/base.py d550c8eeed91f0967e281957b71fcefb0b4cf3b8 src/test/python/apache/aurora/client/test_base.py 06c0b436b5104c9b3afbf80499d3bfc66e7ef2f4 Diff: https://reviews.apache.org/r/31913/diff/ Testing --- [drobinson@x1 aurora-github (drobinson/no_grouping)]$ ./pants test src/test/python/apache/aurora/client:base 15:43:40 00:00 [main] (To run a reporting server: ./pants server) 15:43:40 00:00 [bootstrap] 15:43:40 00:00 [setup] 15:43:40 00:00 [parse] Executing tasks in goals: bootstrap - imports - unpack-jars - deferred-sources - gen - resolve - compile - resources - test 15:43:40 00:00 [bootstrap] 15:43:40 00:00 [bootstrap-jvm-tools] 15:43:40 00:00 [imports] 15:43:40 00:00 [ivy-imports] 15:43:40 00:00 [unpack-jars] 15:43:40 00:00 [unpack-jars] 15:43:40 00:00 [deferred-sources] 15:43:40 00:00 [deferred-sources] 15:43:40 00:00 [gen] 15:43:40 00:00 [thrift] 15:43:40 00:00 [scrooge] 15:43:40 00:00 [protoc] 15:43:40 00:00 [antlr] 15:43:40 00:00 [ragel] 15:43:40 00:00 [jaxb] 15:43:40 00:00 [wire] 15:43:40 00:00 [aapt] 15:43:40 00:00 [resolve] 15:43:40 00:00 [ivy] 15:43:40 00:00 [compile] 15:43:40 00:00 [compile] 15:43:40 00:00 [jvm] 15:43:40 00:00 [jvm-compilers] 15:43:40 00:00 [resources] 15:43:40 00:00 [prepare] 15:43:40 00:00 [test] 15:43:40 00:00 [run_prep_command] 15:43:40 00:00 [test] 15:43:40 00:00 [pytest] 15:43:40 00:00 [run] == test session starts === platform linux2 -- Python 2.7.8 -- py-1.4.26 -- pytest-2.6.4 plugins: cov, timeout collected 7 items src/test/python/apache/aurora/client/test_base.py ... 7 passed in 0.10 seconds 15:43:41 00:01 [junit] 15:43:41 00:01 [specs] SUCCESS Thanks, David Robinson
Re: Review Request 31913: Added 'none' host maintenance grouping function.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31913/#review76304 --- Ship it! Thanks! - Maxim Khutornenko On March 12, 2015, 11:13 p.m., David Robinson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31913/ --- (Updated March 12, 2015, 11:13 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1181 https://issues.apache.org/jira/browse/AURORA-1181 Repository: aurora Description --- Added 'none' host maintenance grouping function. Diffs - src/main/python/apache/aurora/client/base.py d550c8eeed91f0967e281957b71fcefb0b4cf3b8 src/test/python/apache/aurora/admin/test_host_maintenance.py bb586700814a96b3e83d11728b462a7765e81bc1 Diff: https://reviews.apache.org/r/31913/diff/ Testing --- [drobinson@x1 aurora-github (drobinson/no_grouping)]$ ./pants test src/test/python/apache/aurora/client:base 15:43:40 00:00 [main] (To run a reporting server: ./pants server) 15:43:40 00:00 [bootstrap] 15:43:40 00:00 [setup] 15:43:40 00:00 [parse] Executing tasks in goals: bootstrap - imports - unpack-jars - deferred-sources - gen - resolve - compile - resources - test 15:43:40 00:00 [bootstrap] 15:43:40 00:00 [bootstrap-jvm-tools] 15:43:40 00:00 [imports] 15:43:40 00:00 [ivy-imports] 15:43:40 00:00 [unpack-jars] 15:43:40 00:00 [unpack-jars] 15:43:40 00:00 [deferred-sources] 15:43:40 00:00 [deferred-sources] 15:43:40 00:00 [gen] 15:43:40 00:00 [thrift] 15:43:40 00:00 [scrooge] 15:43:40 00:00 [protoc] 15:43:40 00:00 [antlr] 15:43:40 00:00 [ragel] 15:43:40 00:00 [jaxb] 15:43:40 00:00 [wire] 15:43:40 00:00 [aapt] 15:43:40 00:00 [resolve] 15:43:40 00:00 [ivy] 15:43:40 00:00 [compile] 15:43:40 00:00 [compile] 15:43:40 00:00 [jvm] 15:43:40 00:00 [jvm-compilers] 15:43:40 00:00 [resources] 15:43:40 00:00 [prepare] 15:43:40 00:00 [test] 15:43:40 00:00 [run_prep_command] 15:43:40 00:00 [test] 15:43:40 00:00 [pytest] 15:43:40 00:00 [run] == test session starts === platform linux2 -- Python 2.7.8 -- py-1.4.26 -- pytest-2.6.4 plugins: cov, timeout collected 7 items src/test/python/apache/aurora/client/test_base.py ... 7 passed in 0.10 seconds 15:43:41 00:01 [junit] 15:43:41 00:01 [specs] SUCCESS Thanks, David Robinson
Re: Review Request 31913: Added 'none' host maintenance grouping function.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31913/#review76307 --- Thanks, it's now on master. - Maxim Khutornenko On March 12, 2015, 11:13 p.m., David Robinson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31913/ --- (Updated March 12, 2015, 11:13 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1181 https://issues.apache.org/jira/browse/AURORA-1181 Repository: aurora Description --- Added 'none' host maintenance grouping function. Diffs - src/main/python/apache/aurora/client/base.py d550c8eeed91f0967e281957b71fcefb0b4cf3b8 src/test/python/apache/aurora/admin/test_host_maintenance.py bb586700814a96b3e83d11728b462a7765e81bc1 Diff: https://reviews.apache.org/r/31913/diff/ Testing --- [drobinson@x1 aurora-github (drobinson/no_grouping)]$ ./pants test src/test/python/apache/aurora/client:base 15:43:40 00:00 [main] (To run a reporting server: ./pants server) 15:43:40 00:00 [bootstrap] 15:43:40 00:00 [setup] 15:43:40 00:00 [parse] Executing tasks in goals: bootstrap - imports - unpack-jars - deferred-sources - gen - resolve - compile - resources - test 15:43:40 00:00 [bootstrap] 15:43:40 00:00 [bootstrap-jvm-tools] 15:43:40 00:00 [imports] 15:43:40 00:00 [ivy-imports] 15:43:40 00:00 [unpack-jars] 15:43:40 00:00 [unpack-jars] 15:43:40 00:00 [deferred-sources] 15:43:40 00:00 [deferred-sources] 15:43:40 00:00 [gen] 15:43:40 00:00 [thrift] 15:43:40 00:00 [scrooge] 15:43:40 00:00 [protoc] 15:43:40 00:00 [antlr] 15:43:40 00:00 [ragel] 15:43:40 00:00 [jaxb] 15:43:40 00:00 [wire] 15:43:40 00:00 [aapt] 15:43:40 00:00 [resolve] 15:43:40 00:00 [ivy] 15:43:40 00:00 [compile] 15:43:40 00:00 [compile] 15:43:40 00:00 [jvm] 15:43:40 00:00 [jvm-compilers] 15:43:40 00:00 [resources] 15:43:40 00:00 [prepare] 15:43:40 00:00 [test] 15:43:40 00:00 [run_prep_command] 15:43:40 00:00 [test] 15:43:40 00:00 [pytest] 15:43:40 00:00 [run] == test session starts === platform linux2 -- Python 2.7.8 -- py-1.4.26 -- pytest-2.6.4 plugins: cov, timeout collected 7 items src/test/python/apache/aurora/client/test_base.py ... 7 passed in 0.10 seconds 15:43:41 00:01 [junit] 15:43:41 00:01 [specs] SUCCESS Thanks, David Robinson
Review Request 32014: Adding more logging into MaintenanceController.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32014/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- Also added missing test coverage. Diffs - config/legacy_untested_classes.txt 06e130fae6bdc43b5e8aff182a530eac65acca53 src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java b6f642e319b790544bc538098e7cb78b1bb49997 src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java bd031a5e0b7d2923e36ca5958c5074f40dc64848 Diff: https://reviews.apache.org/r/32014/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31916: Add scheduler API support for audit messages when changing job updates.
On March 11, 2015, 12:19 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, line 48 https://reviews.apache.org/r/31916/diff/1/?file=890852#file890852line48 Do we really want to fail an operation when a message gets too long? Since it's optional anyway, I'd expect truncation could be a more resilient way to handle this. David McLaughlin wrote: I think it's better to just give a clear message telling them there is a limit. Truncation could happen in the client if needed. Maxim Khutornenko wrote: I was considering a case when some automated external service would attempt to act on an update and append some arbitrary metadata with pause/resume/abort. While not desirable, does not necessarily warrant a failure. Stopping a misbehaving update should clearly be a higher priority than enforcing a strict audit mode. Bill Farner wrote: Maybe it does warrant a failure, though. IMHO truncation would be a policy decision that the scheduler is making on behalf of the client. If the most important part of the message is after the truncation, we've made a poor choice. IDK, this whole length enforcment seems quite arbitrary to me. We don't restrict string size anywhere in thrift API or SQL schema. The only exception is TaskIdGenerator where the ID length is critical for external reasons (MESOS-691). Perhaps we should start doing it everywhere then? - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/#review76008 --- On March 11, 2015, 12:04 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/ --- (Updated March 11, 2015, 12:04 a.m.) Review request for Aurora, David McLaughlin and Joshua Cohen. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Add scheduler API support for audit messages when changing job updates. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift badb8eec51d9034fdfee79061c777927b2ba1314 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c0d48034ad6b6a91f1f58aca54544a5eddea4742 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 78024a8c257f2d370a4b4d1ba79c6eac68d81ac2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 58824888a4839b71f4efa832a6d62ff6dd946e40 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 7f5e5c2091458192d9310a81314f3c2c42b11f49 src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java eebe01b161fbebdc43e62df4836136a02c3d5fb7 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e119c494de8e81d7e2dd1f78337f08dcba3cd518 Diff: https://reviews.apache.org/r/31916/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31821: Making preemptor asynchronous. Part 1 - extracting slot finder.
On March 7, 2015, 5:30 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java, line 326 https://reviews.apache.org/r/31821/diff/1/?file=888148#file888148line326 Not yours, but it seems odd that we would call a function and internally suppress its behavior based on configuration. Seems like the caller should avoid the call if the behavior is not desired. If you agree, please TODO. Maxim Khutornenko wrote: Not sure I share your concern. This is a predicate intended to answer a simple question, which it does quite well. I doubt the alternative would gain us anything here. Bill Farner wrote: Perhaps i highlighted a line that created confusion. I'm suggesting that this predicate does not belong in this class, as it is used to determine whether to execute the 'body' of the preemptor. I suggest this predicate live in the caller. Thanks for explaining, makes sense now. Added a TODO to refactor PreemptionSlotFinder interface to accept an idle pending task instead. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/#review75637 --- On March 9, 2015, 11:34 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/ --- (Updated March 9, 2015, 11:34 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Extracting PreemptorSlotFinder to be reused for slot validation in later stages. The changes are very minimal and mostly around metric handling and test code. Also added missing test coverage. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 701b9052696337766cb233c865cb9fbb4907071e src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java e093ca54521ffb9399bb97ce60f510331af70853 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictim.java 80c2023f46b63753dcec6a555dba626720a1925a src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java bddb9647493b3e7a58c40d4b477a06161c1388a2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java ae56d1e09322869eedd7a27586cd6f96edd64e0a src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 85b3874a36ed07c684f26da172952c932cff707a src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 58733bdc4dd6de29ccead5cb0a267286e8dc0656 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 891cc098cca99e84ba014b7131106ceb0b429b5f src/test/java/org/apache/aurora/scheduler/async/preemptor/ClusterStateImplTest.java 7207867813b0d096772dbc7f92fc1c76937e9831 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictimTest.java b0380b3fabb45be8ace55cfcf38ce15ef8040188 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 83680769611878886da04e1794b321aa1986e678 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 020b67187a18bba64d9b562c3a6c0969fc85d469 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31821/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31901: Export task status reason counters whenever they are present.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31901/#review75994 --- Ship it! Ship It! - Maxim Khutornenko On March 10, 2015, 6 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31901/ --- (Updated March 10, 2015, 6 p.m.) Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- I realized that the reason values apply to more than just TASK_LOST, so the previous code would hide reasons like when a task has exceeded its memory limit. Diffs - src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java 68214f20dd7f46adec2d8f6d84e9840dc88dc0fb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java 4bbeff957050fb9d8ee81d9fc79520a6a0ac38a1 Diff: https://reviews.apache.org/r/31901/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31913: Added 'none' host maintenance grouping function.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31913/#review75997 --- src/main/python/apache/aurora/client/base.py https://reviews.apache.org/r/31913/#comment123375 This will actually fail upstream in host_maintenance.py when it tries to sort by group: ``` d = defaultdict(set) d[None].add(1) d[None].add(2) s = sorted(d, key=lambda v: v[0]) Traceback (most recent call last): File stdin, line 1, in module File stdin, line 1, in lambda TypeError: 'NoneType' object has no attribute '__getitem__' ``` Please, add a typed default grouping (e.g. ALL) and a correspondent test in test_host_maintenance.py. - Maxim Khutornenko On March 10, 2015, 10:44 p.m., David Robinson wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31913/ --- (Updated March 10, 2015, 10:44 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1181 https://issues.apache.org/jira/browse/AURORA-1181 Repository: aurora Description --- Added 'none' host maintenance grouping function. Diffs - src/main/python/apache/aurora/client/base.py d550c8eeed91f0967e281957b71fcefb0b4cf3b8 src/test/python/apache/aurora/client/test_base.py 06c0b436b5104c9b3afbf80499d3bfc66e7ef2f4 Diff: https://reviews.apache.org/r/31913/diff/ Testing --- [drobinson@x1 aurora-github (drobinson/no_grouping)]$ ./pants test src/test/python/apache/aurora/client:base 15:43:40 00:00 [main] (To run a reporting server: ./pants server) 15:43:40 00:00 [bootstrap] 15:43:40 00:00 [setup] 15:43:40 00:00 [parse] Executing tasks in goals: bootstrap - imports - unpack-jars - deferred-sources - gen - resolve - compile - resources - test 15:43:40 00:00 [bootstrap] 15:43:40 00:00 [bootstrap-jvm-tools] 15:43:40 00:00 [imports] 15:43:40 00:00 [ivy-imports] 15:43:40 00:00 [unpack-jars] 15:43:40 00:00 [unpack-jars] 15:43:40 00:00 [deferred-sources] 15:43:40 00:00 [deferred-sources] 15:43:40 00:00 [gen] 15:43:40 00:00 [thrift] 15:43:40 00:00 [scrooge] 15:43:40 00:00 [protoc] 15:43:40 00:00 [antlr] 15:43:40 00:00 [ragel] 15:43:40 00:00 [jaxb] 15:43:40 00:00 [wire] 15:43:40 00:00 [aapt] 15:43:40 00:00 [resolve] 15:43:40 00:00 [ivy] 15:43:40 00:00 [compile] 15:43:40 00:00 [compile] 15:43:40 00:00 [jvm] 15:43:40 00:00 [jvm-compilers] 15:43:40 00:00 [resources] 15:43:40 00:00 [prepare] 15:43:40 00:00 [test] 15:43:40 00:00 [run_prep_command] 15:43:40 00:00 [test] 15:43:40 00:00 [pytest] 15:43:40 00:00 [run] == test session starts === platform linux2 -- Python 2.7.8 -- py-1.4.26 -- pytest-2.6.4 plugins: cov, timeout collected 7 items src/test/python/apache/aurora/client/test_base.py ... 7 passed in 0.10 seconds 15:43:41 00:01 [junit] 15:43:41 00:01 [specs] SUCCESS Thanks, David Robinson
Re: Review Request 31916: Add scheduler API support for audit messages when changing job updates.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/#review76008 --- src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java https://reviews.apache.org/r/31916/#comment123392 Do we really want to fail an operation when a message gets too long? Since it's optional anyway, I'd expect truncation could be a more resilient way to handle this. - Maxim Khutornenko On March 11, 2015, 12:04 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/ --- (Updated March 11, 2015, 12:04 a.m.) Review request for Aurora, David McLaughlin and Joshua Cohen. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Add scheduler API support for audit messages when changing job updates. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift badb8eec51d9034fdfee79061c777927b2ba1314 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c0d48034ad6b6a91f1f58aca54544a5eddea4742 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 78024a8c257f2d370a4b4d1ba79c6eac68d81ac2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 58824888a4839b71f4efa832a6d62ff6dd946e40 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 7f5e5c2091458192d9310a81314f3c2c42b11f49 src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java eebe01b161fbebdc43e62df4836136a02c3d5fb7 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e119c494de8e81d7e2dd1f78337f08dcba3cd518 Diff: https://reviews.apache.org/r/31916/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31570: Suppressing duplicate update instance events.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31570/ --- (Updated March 11, 2015, 12:19 a.m.) Review request for Aurora, David McLaughlin and Bill Farner. Changes --- Rebased. Bugs: AURORA-1097 https://issues.apache.org/jira/browse/AURORA-1097 Repository: aurora Description --- All instance update actions are supposed to be unique. Suppressing duplicats due to pause/resume cycle. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 58824888a4839b71f4efa832a6d62ff6dd946e40 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e119c494de8e81d7e2dd1f78337f08dcba3cd518 Diff: https://reviews.apache.org/r/31570/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 31916: Add scheduler API support for audit messages when changing job updates.
On March 11, 2015, 12:19 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java, line 48 https://reviews.apache.org/r/31916/diff/1/?file=890852#file890852line48 Do we really want to fail an operation when a message gets too long? Since it's optional anyway, I'd expect truncation could be a more resilient way to handle this. David McLaughlin wrote: I think it's better to just give a clear message telling them there is a limit. Truncation could happen in the client if needed. I was considering a case when some automated external service would attempt to act on an update and append some arbitrary metadata with pause/resume/abort. While not desirable, does not necessarily warrant a failure. Stopping a misbehaving update should clearly be a higher priority than enforcing a strict audit mode. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/#review76008 --- On March 11, 2015, 12:04 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31916/ --- (Updated March 11, 2015, 12:04 a.m.) Review request for Aurora, David McLaughlin and Joshua Cohen. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Add scheduler API support for audit messages when changing job updates. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift badb8eec51d9034fdfee79061c777927b2ba1314 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java c0d48034ad6b6a91f1f58aca54544a5eddea4742 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 78024a8c257f2d370a4b4d1ba79c6eac68d81ac2 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 58824888a4839b71f4efa832a6d62ff6dd946e40 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 7f5e5c2091458192d9310a81314f3c2c42b11f49 src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java eebe01b161fbebdc43e62df4836136a02c3d5fb7 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e119c494de8e81d7e2dd1f78337f08dcba3cd518 Diff: https://reviews.apache.org/r/31916/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31814: Include messages with internal job updater state transitions.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/#review75726 --- src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java https://reviews.apache.org/r/31814/#comment122955 What's the motivation behind dropping the Aurora Updater here? - Maxim Khutornenko On March 7, 2015, 1:06 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/ --- (Updated March 7, 2015, 1:06 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Include messages with internal job updater state transitions. Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 09f2a74c9e6f6dbf79f83aa7a387587c593b9f0e src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java acdade3dca807a221b4da975d0310c91884ee752 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 27e0654bfb90f48b407edda5a0c914e595d9c552 src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 66b0e4b7a9b23e3e6c772a21f5adc39e1d1461ad src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 4db0080547d61af1511a4fb62bf88b3bbf819f1e src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e24d6bde5f3479a75522e825cce4ec6c30c117aa src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 Diff: https://reviews.apache.org/r/31814/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31525: Improving NearestFit reporting accuracy.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31525/ --- (Updated March 9, 2015, 9:59 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Rebased. Bugs: AURORA-1159 https://issues.apache.org/jira/browse/AURORA-1159 Repository: aurora Description --- Instead of relying on a fixed MAX_SCORE, vetoes are now graded by their relative weight (severity) with a dedicated constraint mismatch ranked higest and an insufficient resource mismatch - lowest. Together with break early rule in SchedulingFilter, this ensures heavier vetoes are given proper attention in the NearestFit. This simplifies `NearestFit` logic while also improving pending reason reporting accuracy and scheduling performance. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 3313bd2f9ed5a88375d88715dea5da1e9ad8b963 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java a020ce50d578fba7f32b370f448a49eb1c284147 src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java 785b5742301eec6930a431585256fefce7ec776d src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b00668423a53a8cf6f4da3456bce3354f1fd2424 src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java e0f9d8b4c3d80b70faa10612b82a034e3dae9112 Diff: https://reviews.apache.org/r/31525/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31814: Include messages with internal job updater state transitions.
On March 9, 2015, 5:50 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 291 https://reviews.apache.org/r/31814/diff/2/?file=888120#file888120line291 What's the motivation behind dropping the Aurora Updater here? Bill Farner wrote: I think the magic user value abuses a field that serves a different purpose. IMHO an API consumer should be able to programmatically determine that the scheduler independently performed an action without resorting to string matching on the magic value of a field. In this particular case, i don't think the user field adds signal to what is already present in the state enum value. I still think having a special Aurora Updater user clearly visible in the UI makes it way easier to grasp the event origin rather than trying to decipher a particular state meaning. Users may not and should not be familar with the update state diagram in order to understand the cause of the transition. This is especially true in a multi-actor environment where state transitions may come from users (pause/resume/abort), external monitoring service (pause) or the updater itself (pulse expired). - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/#review75726 --- On March 7, 2015, 1:06 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/ --- (Updated March 7, 2015, 1:06 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Include messages with internal job updater state transitions. Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 09f2a74c9e6f6dbf79f83aa7a387587c593b9f0e src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java acdade3dca807a221b4da975d0310c91884ee752 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 27e0654bfb90f48b407edda5a0c914e595d9c552 src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 66b0e4b7a9b23e3e6c772a21f5adc39e1d1461ad src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 4db0080547d61af1511a4fb62bf88b3bbf819f1e src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e24d6bde5f3479a75522e825cce4ec6c30c117aa src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 Diff: https://reviews.apache.org/r/31814/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31869: Catch only known Exception types in the client.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/#review75761 --- Ship it! src/main/python/apache/aurora/client/cli/jobs.py https://reviews.apache.org/r/31869/#comment123014 why not just context? - Maxim Khutornenko On March 9, 2015, 8:10 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31869/ --- (Updated March 9, 2015, 8:10 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-1176 https://issues.apache.org/jira/browse/AURORA-1176 Repository: aurora Description --- As indicated by some changes in tests - there were legitimate issues hiding behind this catch. After this change, the client will allow unhandled exceptions to surface in their full glory. Diffs - src/main/python/apache/aurora/client/api/__init__.py 194629f61192c1d7d5e7064e9226adf26d03e890 src/main/python/apache/aurora/client/base.py d550c8eeed91f0967e281957b71fcefb0b4cf3b8 src/main/python/apache/aurora/client/cli/__init__.py 4d9ef09749e3075b9d9e2ae1db311e60f7bdb4ee src/main/python/apache/aurora/client/cli/jobs.py 286b2182d5fe25703882f0b367739ad03d6c8fe8 src/test/python/apache/aurora/client/cli/test_api_from_cli.py b855c3c2d74125738d2106e18a9e9b0ebed6ac4b src/test/python/apache/aurora/client/cli/test_create.py 459d157155f74b6a3d140b85d3b7f0364367 src/test/python/apache/aurora/client/cli/test_kill.py 7aad34a2fe5591937c5bca890751073439e3a1a6 src/test/python/apache/aurora/client/cli/test_supdate.py 1806769426a196793481f948892f5474df8dd665 src/test/python/apache/aurora/client/cli/util.py b65970a2717a1f36767c61e5e09c980b04895f01 Diff: https://reviews.apache.org/r/31869/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31814: Include messages with internal job updater state transitions.
On March 9, 2015, 5:50 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 291 https://reviews.apache.org/r/31814/diff/2/?file=888120#file888120line291 What's the motivation behind dropping the Aurora Updater here? Bill Farner wrote: I think the magic user value abuses a field that serves a different purpose. IMHO an API consumer should be able to programmatically determine that the scheduler independently performed an action without resorting to string matching on the magic value of a field. In this particular case, i don't think the user field adds signal to what is already present in the state enum value. Maxim Khutornenko wrote: I still think having a special Aurora Updater user clearly visible in the UI makes it way easier to grasp the event origin rather than trying to decipher a particular state meaning. Users may not and should not be familar with the update state diagram in order to understand the cause of the transition. This is especially true in a multi-actor environment where state transitions may come from users (pause/resume/abort), external monitoring service (pause) or the updater itself (pulse expired). Bill Farner wrote: I still think having a special Aurora Updater user clearly visible in the UI I completely agree, but the UI should make that decision, not the scheduler. I see, sure I am fine with that. Do you propose to address it separately? If so, mind filing a ticket? - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/#review75726 --- On March 7, 2015, 1:06 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31814/ --- (Updated March 7, 2015, 1:06 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1077 https://issues.apache.org/jira/browse/AURORA-1077 Repository: aurora Description --- Include messages with internal job updater state transitions. Diffs - src/main/java/org/apache/aurora/scheduler/updater/InstanceUpdater.java 09f2a74c9e6f6dbf79f83aa7a387587c593b9f0e src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java acdade3dca807a221b4da975d0310c91884ee752 src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/main/java/org/apache/aurora/scheduler/updater/SideEffect.java 27e0654bfb90f48b407edda5a0c914e595d9c552 src/main/java/org/apache/aurora/scheduler/updater/StateEvaluator.java 66b0e4b7a9b23e3e6c772a21f5adc39e1d1461ad src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 4db0080547d61af1511a4fb62bf88b3bbf819f1e src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e24d6bde5f3479a75522e825cce4ec6c30c117aa src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 7d0a7438b4a517e5e0d44f4e99aceb1a6d19f987 Diff: https://reviews.apache.org/r/31814/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31821: Making preemptor asynchronous. Part 1 - extracting slot finder.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/ --- (Updated March 9, 2015, 11:34 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Bill's and Zameer's comments. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Extracting PreemptorSlotFinder to be reused for slot validation in later stages. The changes are very minimal and mostly around metric handling and test code. Also added missing test coverage. Diffs (updated) - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 701b9052696337766cb233c865cb9fbb4907071e src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java e093ca54521ffb9399bb97ce60f510331af70853 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictim.java 80c2023f46b63753dcec6a555dba626720a1925a src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java bddb9647493b3e7a58c40d4b477a06161c1388a2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java ae56d1e09322869eedd7a27586cd6f96edd64e0a src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 85b3874a36ed07c684f26da172952c932cff707a src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 58733bdc4dd6de29ccead5cb0a267286e8dc0656 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 891cc098cca99e84ba014b7131106ceb0b429b5f src/test/java/org/apache/aurora/scheduler/async/preemptor/ClusterStateImplTest.java 7207867813b0d096772dbc7f92fc1c76937e9831 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictimTest.java b0380b3fabb45be8ace55cfcf38ce15ef8040188 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 83680769611878886da04e1794b321aa1986e678 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 020b67187a18bba64d9b562c3a6c0969fc85d469 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31821/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31821: Making preemptor asynchronous. Part 1 - extracting slot finder.
, so we can avoid further modification of the test cases. However, in this diff please correct the line wrap style on the arguments. Zameer Manji wrote: +1 to Bill's suggestion of having `makeTask(IJobKey job, String id)`. I think having a fixture with this many options will only lead to complexity and sadness if the tests increase in scope. Agree. Dropped. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/#review75637 --- On March 7, 2015, 1:36 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/ --- (Updated March 7, 2015, 1:36 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Extracting PreemptorSlotFinder to be reused for slot validation in later stages. The changes are very minimal and mostly around metric handling and test code. Also added missing test coverage. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 701b9052696337766cb233c865cb9fbb4907071e src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java e093ca54521ffb9399bb97ce60f510331af70853 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java bddb9647493b3e7a58c40d4b477a06161c1388a2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java ae56d1e09322869eedd7a27586cd6f96edd64e0a src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 85b3874a36ed07c684f26da172952c932cff707a src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 58733bdc4dd6de29ccead5cb0a267286e8dc0656 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 891cc098cca99e84ba014b7131106ceb0b429b5f src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 83680769611878886da04e1794b321aa1986e678 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 020b67187a18bba64d9b562c3a6c0969fc85d469 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/TestUtil.java PRE-CREATION Diff: https://reviews.apache.org/r/31821/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31821: Making preemptor asynchronous. Part 1 - extracting slot finder.
On March 9, 2015, 6:59 p.m., Zameer Manji wrote: src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java, line 135 https://reviews.apache.org/r/31821/diff/1/?file=888148#file888148line135 To keep the implementation package private, could you move it to another file? It has to be public for the benchmark code. This will hopefully get dropped with the TODO I left there to DRY module usage. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/#review75743 --- On March 7, 2015, 1:36 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/ --- (Updated March 7, 2015, 1:36 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Extracting PreemptorSlotFinder to be reused for slot validation in later stages. The changes are very minimal and mostly around metric handling and test code. Also added missing test coverage. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 701b9052696337766cb233c865cb9fbb4907071e src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java e093ca54521ffb9399bb97ce60f510331af70853 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java bddb9647493b3e7a58c40d4b477a06161c1388a2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java ae56d1e09322869eedd7a27586cd6f96edd64e0a src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 85b3874a36ed07c684f26da172952c932cff707a src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 58733bdc4dd6de29ccead5cb0a267286e8dc0656 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 891cc098cca99e84ba014b7131106ceb0b429b5f src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 83680769611878886da04e1794b321aa1986e678 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 020b67187a18bba64d9b562c3a6c0969fc85d469 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/TestUtil.java PRE-CREATION Diff: https://reviews.apache.org/r/31821/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31821: Making preemptor asynchronous. Part 1 - extracting slot finder.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/#review75816 --- @ReviewBot retry - Maxim Khutornenko On March 9, 2015, 11:34 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/ --- (Updated March 9, 2015, 11:34 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Extracting PreemptorSlotFinder to be reused for slot validation in later stages. The changes are very minimal and mostly around metric handling and test code. Also added missing test coverage. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 701b9052696337766cb233c865cb9fbb4907071e src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java e093ca54521ffb9399bb97ce60f510331af70853 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictim.java 80c2023f46b63753dcec6a555dba626720a1925a src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java bddb9647493b3e7a58c40d4b477a06161c1388a2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java ae56d1e09322869eedd7a27586cd6f96edd64e0a src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 85b3874a36ed07c684f26da172952c932cff707a src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 58733bdc4dd6de29ccead5cb0a267286e8dc0656 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 891cc098cca99e84ba014b7131106ceb0b429b5f src/test/java/org/apache/aurora/scheduler/async/preemptor/ClusterStateImplTest.java 7207867813b0d096772dbc7f92fc1c76937e9831 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionVictimTest.java b0380b3fabb45be8ace55cfcf38ce15ef8040188 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 83680769611878886da04e1794b321aa1986e678 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 020b67187a18bba64d9b562c3a6c0969fc85d469 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31821/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 31821: Making preemptor asynchronous. Part 1 - extracting slot finder.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31821/ --- Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Extracting PreemptorSlotFinder to be reused for slot validation in later stages. The changes are very minimal and mostly around metric handling and test code. Also added missing test coverage. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 701b9052696337766cb233c865cb9fbb4907071e src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java e093ca54521ffb9399bb97ce60f510331af70853 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java bddb9647493b3e7a58c40d4b477a06161c1388a2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java ae56d1e09322869eedd7a27586cd6f96edd64e0a src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 85b3874a36ed07c684f26da172952c932cff707a src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 58733bdc4dd6de29ccead5cb0a267286e8dc0656 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 891cc098cca99e84ba014b7131106ceb0b429b5f src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 83680769611878886da04e1794b321aa1986e678 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 020b67187a18bba64d9b562c3a6c0969fc85d469 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/TestUtil.java PRE-CREATION Diff: https://reviews.apache.org/r/31821/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31652: Returning pending reason for all tasks in a group
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31652/ --- (Updated March 5, 2015, 6:48 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Bill's comments. Bugs: AURORA-911 https://issues.apache.org/jira/browse/AURORA-911 Repository: aurora Description --- Modifying `Vetoed` event to broadcast `TaskGroupKey` instead of task ID and storing veto reasons by TaskGroupKey in NearestFit. Depends on https://reviews.apache.org/r/31646/. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java edaf2f4f845544c13b2fb9bc77c34f6e6d96fb48 src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java c103472b9404df1c690b3a6019d64d42e15f2fed src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java c3097e49c0f6588ea765aa4fab69dd35e3d90e8b src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 53582c63ddee23e643bd4654cad2bef75dfba36d src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 13520eb5846022ed0b43b402096fe02565103aa9 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java ab7817f929bbcc96a6046043ea17921a388fdb9f src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 78a236c0f9074692b67ce18e6e03f18fe4529e02 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java ce5a62650cebab9a53743460f5a5119f62efec1c Diff: https://reviews.apache.org/r/31652/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31652: Returning pending reason for all tasks in a group
On March 3, 2015, 11:56 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java, line 237 https://reviews.apache.org/r/31652/diff/1/?file=882474#file882474line237 s/taskId/groupKey/ Good catch, fixed. On March 3, 2015, 11:56 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java, line 57 https://reviews.apache.org/r/31652/diff/1/?file=882475#file882475line57 Should this cache be renamed? No preference here, renamed. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31652/#review75096 --- On March 3, 2015, 12:58 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31652/ --- (Updated March 3, 2015, 12:58 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-911 https://issues.apache.org/jira/browse/AURORA-911 Repository: aurora Description --- Modifying `Vetoed` event to broadcast `TaskGroupKey` instead of task ID and storing veto reasons by TaskGroupKey in NearestFit. Depends on https://reviews.apache.org/r/31646/. Diffs - src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java edaf2f4f845544c13b2fb9bc77c34f6e6d96fb48 src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java c103472b9404df1c690b3a6019d64d42e15f2fed src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java c3097e49c0f6588ea765aa4fab69dd35e3d90e8b src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 53582c63ddee23e643bd4654cad2bef75dfba36d src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 13520eb5846022ed0b43b402096fe02565103aa9 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java ab7817f929bbcc96a6046043ea17921a388fdb9f src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 78a236c0f9074692b67ce18e6e03f18fe4529e02 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java ce5a62650cebab9a53743460f5a5119f62efec1c Diff: https://reviews.apache.org/r/31652/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31646: Moving GroupKey to scheduler.base.
On March 3, 2015, 11:54 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java, line 23 https://reviews.apache.org/r/31646/diff/1/?file=882425#file882425line23 s/Identifying/Identifer for/ It's not until reading this diff that i wonder why TaskGroupKey/GroupKey even exists. The best justification i have is separation of concerns from how a task is configured and how it is identified. If you agree, it would be nice to document that here. Done. On March 3, 2015, 11:54 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java, line 38 https://reviews.apache.org/r/31646/diff/1/?file=882425#file882425line38 Why the proxy method to the constructor? To highlight the immutable final purpose of its existence. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31646/#review75093 --- On March 3, 2015, 12:07 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31646/ --- (Updated March 3, 2015, 12:07 a.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-911 https://issues.apache.org/jira/browse/AURORA-911 Repository: aurora Description --- Simple IDE-driven refactoring in preparation for the AURORA-911 fix. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferManager.java 7d3713972f1df54da4c8cbaae0fca59a8a6f3d77 src/main/java/org/apache/aurora/scheduler/async/TaskGroup.java e5067e18af7c477d2927d83eacec063f3dec575a src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java 8cd6c966c9aca2e869311787fb5d5caba7439d3e src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java d0fe3e133cbec2418f31160bf8ab8adaa45bb958 src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java 0cbc71d50612323aa4d8acc33e74b879c0a76aff src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 4ee13c8e5d46ba863f4d9871884c7d494d07758d src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 87bc531d2a72f21c36ddd0c1bd3b2367826cc422 Diff: https://reviews.apache.org/r/31646/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 31739: Making task preemption asynchronous.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31739/ --- Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Reservations are now happen asynchronously with a configurable delay between a failed task scheduling and a preemption attempt. Added a new `PreemptorBenchmark` to measure preemption perf as it now happens off the main scheduling loop and thus unreachable by earlier benchmarks. Benchmark results are unsurprisingly great. The biggest winner is the PreemptorFallbackForLargeClusterBenchmark (now ClusterFullUtilizationBenchmark). Without the preemptor fallback and thanks to static veto offer filtering it's now 99.995% faster :) The lowest gain is for the limit constraint benchmark. It's the only dynamic veto type and thus is not subjected to offer filtering. Still ~82% improvement is nothing to complain about. Before: Benchmark Mode Cnt ScoreError Units SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100781243.004 ± 9308.450 ns/op SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 1205278.826 ± 19800.452 ns/op SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark avgt 100 77048458.974 ± 918593.702 ns/op SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100769919.326 ± 18963.264 ns/op After: Benchmark Mode CntScoreError Units SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 10028117.603 ±243.556 ns/op SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 348667.808 ± 2956.521 ns/op SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark avgt 100 3978.828 ±351.186 ns/op SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 10026096.782 ±412.138 ns/op SchedulingBenchmarks.PreemptorBenchmark.runBenchmark avgt 100 6054216.773 ± 105428.318 ns/op Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 3239eaa139e35e8c3acdacf6375f492de2b5bfee src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e87dda47a355654c66f6f54fb25a4d9a7f68422d src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java d0fe3e133cbec2418f31160bf8ab8adaa45bb958 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 4ee13c8e5d46ba863f4d9871884c7d494d07758d src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 87bc531d2a72f21c36ddd0c1bd3b2367826cc422 Diff: https://reviews.apache.org/r/31739/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 31739: Making task preemption asynchronous.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31739/ --- (Updated March 4, 2015, 7:30 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description (updated) --- Reservations now happen asynchronously with a configurable delay between a failed task scheduling and a preemption attempt. Added a new `PreemptorBenchmark` to measure preemption perf as it now happens off the main scheduling loop and thus unreachable by earlier benchmarks. Benchmark results are unsurprisingly great. The biggest winner is the PreemptorFallbackForLargeClusterBenchmark (now ClusterFullUtilizationBenchmark). Without the preemptor fallback and thanks to static veto offer filtering it's now 99.995% faster :) The lowest gain is for the limit constraint benchmark. It's the only dynamic veto type and thus is not subjected to offer filtering. Still ~71% improvement is nothing to complain about. Before: Benchmark Mode Cnt ScoreError Units SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100781243.004 ± 9308.450 ns/op SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 1205278.826 ± 19800.452 ns/op SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark avgt 100 77048458.974 ± 918593.702 ns/op SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100769919.326 ± 18963.264 ns/op After: Benchmark Mode CntScoreError Units SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 10028117.603 ±243.556 ns/op SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 348667.808 ± 2956.521 ns/op SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark avgt 100 3978.828 ±351.186 ns/op SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 10026096.782 ±412.138 ns/op SchedulingBenchmarks.PreemptorBenchmark.runBenchmark avgt 100 6054216.773 ± 105428.318 ns/op Perf gain summary: InsufficientResourcesSchedulingBenchmark - 96.4% LimitConstraintMismatchSchedulingBenchmark - 71% PreemptorFallbackForLargeClusterBenchmark- 99.995% ValueConstraintMismatchSchedulingBenchmark - 96.6% Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 3239eaa139e35e8c3acdacf6375f492de2b5bfee src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e87dda47a355654c66f6f54fb25a4d9a7f68422d src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java d0fe3e133cbec2418f31160bf8ab8adaa45bb958 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 4ee13c8e5d46ba863f4d9871884c7d494d07758d src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 87bc531d2a72f21c36ddd0c1bd3b2367826cc422 Diff: https://reviews.apache.org/r/31739/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Review Request 31751: Adding missing service binding into UpdaterModule.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31751/ --- Review request for Aurora and Bill Farner. Bugs: AURORA-1173 https://issues.apache.org/jira/browse/AURORA-1173 Repository: aurora Description --- Adding missing binding and a e2e test. Diffs - src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java 5733da3daeacd8cb726310e5d9933635e3993687 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 216849145fe681d05ed54ac7fbf385bb31d8cdea Diff: https://reviews.apache.org/r/31751/diff/ Testing --- ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Maxim Khutornenko
Re: Review Request 31751: Adding missing service binding into UpdaterModule.
On March 5, 2015, 1:01 a.m., Bill Farner wrote: It would be great to have coverage for this within the scheduler's tests. `JobUpdaterIT` already consumes `UpdaterModule`, pehaps this fix could be proven there? AFAICT, to truly test that binding would require wiring up the SchedulerModule and all its dependent modules to simulate a DriverRegistered event. That would trigger SchedulerLifecycle handleRegistered() and all services annotated with @SchedulerActive. IMO, not worth the increased test complexity. On March 5, 2015, 1:01 a.m., Bill Farner wrote: src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh, line 169 https://reviews.apache.org/r/31751/diff/1/?file=885238#file885238line169 Any reason you chose `stop` and `start` over `restart`? Did not notice restart command. Changed. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31751/#review75272 --- On March 5, 2015, 12:48 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31751/ --- (Updated March 5, 2015, 12:48 a.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1173 https://issues.apache.org/jira/browse/AURORA-1173 Repository: aurora Description --- Adding missing binding and a e2e test. Diffs - src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java 5733da3daeacd8cb726310e5d9933635e3993687 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 216849145fe681d05ed54ac7fbf385bb31d8cdea Diff: https://reviews.apache.org/r/31751/diff/ Testing --- ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Maxim Khutornenko
Re: Review Request 31739: Making task preemption asynchronous.
On March 4, 2015, 10:25 p.m., Stephan Erb wrote: src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java, line 269 https://reviews.apache.org/r/31739/diff/1/?file=884469#file884469line269 Is this the same single threaded scheduler used for ordinary scheduling? (Otherwise I would expect races) Yes, it is. On March 4, 2015, 10:25 p.m., Stephan Erb wrote: src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java, line 331 https://reviews.apache.org/r/31739/diff/1/?file=884467#file884467line331 By only using one task per host we don't benchmark the costly O(tasks per slave) candidate search. The latter can probably be improved significantly, I therefore think it is wise to extend the benchmark accordingly. This line is actually referring to a benchmark tester task rather than preemption candidates, which are populated in the base `buildClusterTasks()`. However, you are correct about one task per slave observation. We could potentially add a multiple tasks per slave preemption benchmark but we should probably do so when we try to optimize the preemptor algorithm itself. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31739/#review75247 --- On March 4, 2015, 7:30 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31739/ --- (Updated March 4, 2015, 7:30 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Reservations now happen asynchronously with a configurable delay between a failed task scheduling and a preemption attempt. Added a new `PreemptorBenchmark` to measure preemption perf as it now happens off the main scheduling loop and thus unreachable by earlier benchmarks. Benchmark results are unsurprisingly great. The biggest winner is the PreemptorFallbackForLargeClusterBenchmark (now ClusterFullUtilizationBenchmark). Without the preemptor fallback and thanks to static veto offer filtering it's now 99.995% faster :) The lowest gain is for the limit constraint benchmark. It's the only dynamic veto type and thus is not subjected to offer filtering. Still ~71% improvement is nothing to complain about. Before: Benchmark Mode Cnt ScoreError Units SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100781243.004 ± 9308.450 ns/op SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 1205278.826 ± 19800.452 ns/op SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark avgt 100 77048458.974 ± 918593.702 ns/op SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100769919.326 ± 18963.264 ns/op After: Benchmark Mode CntScoreError Units SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 10028117.603 ±243.556 ns/op SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 348667.808 ± 2956.521 ns/op SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark avgt 100 3978.828 ±351.186 ns/op SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 10026096.782 ±412.138 ns/op SchedulingBenchmarks.PreemptorBenchmark.runBenchmark avgt 100 6054216.773 ± 105428.318 ns/op Perf gain summary: InsufficientResourcesSchedulingBenchmark - 96.4% LimitConstraintMismatchSchedulingBenchmark - 71% PreemptorFallbackForLargeClusterBenchmark- 99.995% ValueConstraintMismatchSchedulingBenchmark - 96.6% Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 3239eaa139e35e8c3acdacf6375f492de2b5bfee src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e87dda47a355654c66f6f54fb25a4d9a7f68422d src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java d0fe3e133cbec2418f31160bf8ab8adaa45bb958 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 4ee13c8e5d46ba863f4d9871884c7d494d07758d src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 87bc531d2a72f21c36ddd0c1bd3b2367826cc422 Diff: https://reviews.apache.org/r/31739/diff/ Testing --- ./gradlew -Pq build Manual testing
Re: Review Request 31739: Making task preemption asynchronous.
On March 4, 2015, 8:17 p.m., Bill Farner wrote: Is there a reason you did not opt to implement this behind the `Preemptor` interface? Seems like if you went with that approach, `TaskScheduler` can be oblivious to the background operations. Maxim Khutornenko wrote: Trying to keep things simple. Moving it behind `Preemptor` would require sharing `Reservations` (or some equivalent feedback notificaiton) between TaskScheduler and Preemptor. Bill Farner wrote: I don't see why the data structure would need to be shared. On one call you could asynchronously kick off the work, and a subsequent call could report back the result of the previous. Maxim Khutornenko wrote: Perhaps I am missing the point but how does it correlate with TaskScheduler can be oblivious to the background operations? If there is no immediate response back from the preemptor what is responsible for getting the reservation data and when? Where does that reservation data live in this case? Chatted with Bill offline and we agreed that we should start moving towards a standalone (background worker) preemptor. That would require moving async decision making into the preemptor itself. I am going to discard this RB and start working towards what benefits us longer term. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31739/#review75226 --- On March 4, 2015, 7:30 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31739/ --- (Updated March 4, 2015, 7:30 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Reservations now happen asynchronously with a configurable delay between a failed task scheduling and a preemption attempt. Added a new `PreemptorBenchmark` to measure preemption perf as it now happens off the main scheduling loop and thus unreachable by earlier benchmarks. Benchmark results are unsurprisingly great. The biggest winner is the PreemptorFallbackForLargeClusterBenchmark (now ClusterFullUtilizationBenchmark). Without the preemptor fallback and thanks to static veto offer filtering it's now 99.995% faster :) The lowest gain is for the limit constraint benchmark. It's the only dynamic veto type and thus is not subjected to offer filtering. Still ~71% improvement is nothing to complain about. Before: Benchmark Mode Cnt ScoreError Units SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100781243.004 ± 9308.450 ns/op SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 1205278.826 ± 19800.452 ns/op SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark avgt 100 77048458.974 ± 918593.702 ns/op SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100769919.326 ± 18963.264 ns/op After: Benchmark Mode CntScoreError Units SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 10028117.603 ±243.556 ns/op SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 348667.808 ± 2956.521 ns/op SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark avgt 100 3978.828 ±351.186 ns/op SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 10026096.782 ±412.138 ns/op SchedulingBenchmarks.PreemptorBenchmark.runBenchmark avgt 100 6054216.773 ± 105428.318 ns/op Perf gain summary: InsufficientResourcesSchedulingBenchmark - 96.4% LimitConstraintMismatchSchedulingBenchmark - 71% PreemptorFallbackForLargeClusterBenchmark- 99.995% ValueConstraintMismatchSchedulingBenchmark - 96.6% Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 3239eaa139e35e8c3acdacf6375f492de2b5bfee src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java e87dda47a355654c66f6f54fb25a4d9a7f68422d src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java d0fe3e133cbec2418f31160bf8ab8adaa45bb958 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 4ee13c8e5d46ba863f4d9871884c7d494d07758d src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java
Re: Review Request 31659: Clean up end-to-end tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31659/#review75045 --- Ship it! Ship It! - Maxim Khutornenko On March 3, 2015, 8:53 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31659/ --- (Updated March 3, 2015, 8:53 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Repository: aurora Description --- Major changes: - applying DRY by not invoking everything via `vagrant ssh` (making tests easier to write and much faster to run) - pulled 'test cases' into individual functions - simplified (maybe?) setup for tests using SSH This is being done in preparation for AURORA-1157. Diffs - src/test/sh/org/apache/aurora/e2e/http/http_example.aurora 0f6423521ad2c85032b4825a5b9793dc522a3b70 src/test/sh/org/apache/aurora/e2e/http/http_example_docker.aurora 00fa2fb6c272fe01700c4696f92713f11d62230b src/test/sh/org/apache/aurora/e2e/http/http_example_docker_updated.aurora d987d637da4061cd7946074243daabee4b9a150f src/test/sh/org/apache/aurora/e2e/http/http_example_updated.aurora 072bbb76707e6696815509f21d50b6b5446241b3 src/test/sh/org/apache/aurora/e2e/test_common.sh b621975bab6f8bbf193e61360654e34337e36943 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 1d599c3f71b7e07c1cb8bee01a4d3b5cfcf92631 src/test/sh/org/apache/aurora/e2e/test_run.sh ab91b95a60f7b459b95dc7e13a29692badca5ff7 Diff: https://reviews.apache.org/r/31659/diff/ Testing --- Ran the script. Thanks, Bill Farner
Re: Review Request 31710: Fix query and timestamp display when running beta-update status.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31710/#review75089 --- Ship it! src/main/python/apache/aurora/client/cli/update.py https://reviews.apache.org/r/31710/#comment122025 You may want to extract an internal method to do this, e.g.: ``` def timestamp(time_msec): return time.ctime(time_msec / 1000) ``` - Maxim Khutornenko On March 3, 2015, 11:44 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31710/ --- (Updated March 3, 2015, 11:44 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1165 and AURORA-1166 https://issues.apache.org/jira/browse/AURORA-1165 https://issues.apache.org/jira/browse/AURORA-1166 Repository: aurora Description --- Discovered these while working on AURORA-1157 Diffs - src/main/python/apache/aurora/client/cli/update.py ee8146be0cb6e3f3139a5491c883864950f245bd src/test/python/apache/aurora/client/cli/test_supdate.py 1173650998d397a68a7fabf38201fb9feebf2977 Diff: https://reviews.apache.org/r/31710/diff/ Testing --- Thanks, Bill Farner
Review Request 31652: Returning pending reason for all tasks in a group
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31652/ --- Review request for Aurora, Bill Farner and Zameer Manji. Repository: aurora Description --- Modifying `Vetoed` event to broadcast `TaskGroupKey` instead of task ID and storing veto reasons by TaskGroupKey in NearestFit. Depends on https://reviews.apache.org/r/31646/. Diffs - src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java edaf2f4f845544c13b2fb9bc77c34f6e6d96fb48 src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java c103472b9404df1c690b3a6019d64d42e15f2fed src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java c3097e49c0f6588ea765aa4fab69dd35e3d90e8b src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 53582c63ddee23e643bd4654cad2bef75dfba36d src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java 13520eb5846022ed0b43b402096fe02565103aa9 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java ab7817f929bbcc96a6046043ea17921a388fdb9f src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 78a236c0f9074692b67ce18e6e03f18fe4529e02 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java ce5a62650cebab9a53743460f5a5119f62efec1c Diff: https://reviews.apache.org/r/31652/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31525: Improving NearestFit reporting accuracy.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31525/ --- (Updated March 2, 2015, 11:40 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Updating ticket number Bugs: AURORA-1159 https://issues.apache.org/jira/browse/AURORA-1159 Repository: aurora Description --- Instead of relying on a fixed MAX_SCORE, vetoes are now graded by their relative weight (severity) with a dedicated constraint mismatch ranked higest and an insufficient resource mismatch - lowest. Together with break early rule in SchedulingFilter, this ensures heavier vetoes are given proper attention in the NearestFit. This simplifies `NearestFit` logic while also improving pending reason reporting accuracy and scheduling performance. Diffs - src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 3313bd2f9ed5a88375d88715dea5da1e9ad8b963 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java a020ce50d578fba7f32b370f448a49eb1c284147 src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java c3097e49c0f6588ea765aa4fab69dd35e3d90e8b src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b00668423a53a8cf6f4da3456bce3354f1fd2424 src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 78a236c0f9074692b67ce18e6e03f18fe4529e02 Diff: https://reviews.apache.org/r/31525/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 31646: Moving GroupKey to scheduler.base.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31646/ --- Review request for Aurora and Bill Farner. Bugs: AURORA-911 https://issues.apache.org/jira/browse/AURORA-911 Repository: aurora Description --- Simple IDE-driven refactoring in preparation for the AURORA-911 fix. Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferManager.java 7d3713972f1df54da4c8cbaae0fca59a8a6f3d77 src/main/java/org/apache/aurora/scheduler/async/TaskGroup.java e5067e18af7c477d2927d83eacec063f3dec575a src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java 8cd6c966c9aca2e869311787fb5d5caba7439d3e src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java d0fe3e133cbec2418f31160bf8ab8adaa45bb958 src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java 0cbc71d50612323aa4d8acc33e74b879c0a76aff src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 4ee13c8e5d46ba863f4d9871884c7d494d07758d src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 87bc531d2a72f21c36ddd0c1bd3b2367826cc422 Diff: https://reviews.apache.org/r/31646/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31525: Improving NearestFit reporting accuracy.
On March 2, 2015, 7:40 p.m., Bill Farner wrote: Can you elaborate on how this change would have addressed this scenario described in AURORA-1148? Seems like the confusion of only seeing 'Insufficient RAM' is not resolved by this change alone. Correct, it's an incremental quality improvement rather than a comprehensive fix (which I am not even sure is possible given our current NearestFit approach). The idea is to NOT emit lower score (aka better fit) vetoes until the higher score vetoes are dismissed. Here is the most likely execution sequence in AURORA-1148 case: 1. non-decicated hosts are filtered out (veto reason: dedicated constraint mismatch) 2. dedicated hosts are evaluated wrt diversity constraints (veto reason: host limit constraint mismatch) 3. dedicated host resources are evaluated (veto reason: insufficient RAM/CPU/DISK) Given that every VetoType triggered short-circuits further veto analysis, users would *more* likely see a host limit constraint. What this change does not solve is that in case there are some offers satisfying diversity analysis they would pollute veto reason with Insufficient resource veto thus humpering investigation. Given our current 10 minute veto reason expiration and default offer hold time (5 minutes), users would likely see pending reason occasionally flipping from host limit mismatch to insufficient resource. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31525/#review74801 --- On Feb. 27, 2015, 9:05 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31525/ --- (Updated Feb. 27, 2015, 9:05 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-1148 https://issues.apache.org/jira/browse/AURORA-1148 Repository: aurora Description --- Instead of relying on a fixed MAX_SCORE, vetoes are now graded by their relative weight (severity) with a dedicated constraint mismatch ranked higest and an insufficient resource mismatch - lowest. Together with break early rule in SchedulingFilter, this ensures heavier vetoes are given proper attention in the NearestFit. This simplifies `NearestFit` logic while also improving pending reason reporting accuracy and scheduling performance. Diffs - src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 3313bd2f9ed5a88375d88715dea5da1e9ad8b963 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java a020ce50d578fba7f32b370f448a49eb1c284147 src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java c3097e49c0f6588ea765aa4fab69dd35e3d90e8b src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b00668423a53a8cf6f4da3456bce3354f1fd2424 src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 78a236c0f9074692b67ce18e6e03f18fe4529e02 Diff: https://reviews.apache.org/r/31525/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31525: Improving NearestFit reporting accuracy.
On March 2, 2015, 7:14 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java, line 64 https://reviews.apache.org/r/31525/diff/2/?file=880724#file880724line64 I don't think the Math.pow approach is necessary - why not make score an EnumMapVetoType, Integer (or just an EnumSetVetoType). A veto increments the counter in the bucket for its type (or sets the bit for it). Still very efficient and more type-safe. Not sure I understand the proposal. The score buckets, as currently defined, flatten out the VetoType hierarchy via non-intersecting int ranges. This abstracts out the internal VetoType relationship and simplifies the consuming side (NearestFit). The sum of all previous level veto scores should never exceed the next level score. I don't see how we can get by with EnumSetVetoType here without making NearestFit aware of VetoType hierarchy. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31525/#review74793 --- On Feb. 27, 2015, 9:05 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31525/ --- (Updated Feb. 27, 2015, 9:05 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-1148 https://issues.apache.org/jira/browse/AURORA-1148 Repository: aurora Description --- Instead of relying on a fixed MAX_SCORE, vetoes are now graded by their relative weight (severity) with a dedicated constraint mismatch ranked higest and an insufficient resource mismatch - lowest. Together with break early rule in SchedulingFilter, this ensures heavier vetoes are given proper attention in the NearestFit. This simplifies `NearestFit` logic while also improving pending reason reporting accuracy and scheduling performance. Diffs - src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 3313bd2f9ed5a88375d88715dea5da1e9ad8b963 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java a020ce50d578fba7f32b370f448a49eb1c284147 src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java c3097e49c0f6588ea765aa4fab69dd35e3d90e8b src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b00668423a53a8cf6f4da3456bce3354f1fd2424 src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 78a236c0f9074692b67ce18e6e03f18fe4529e02 Diff: https://reviews.apache.org/r/31525/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31570: Suppressing duplicate update instance events.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31570/#review74649 --- @ReviewBot retry - Maxim Khutornenko On Feb. 28, 2015, 1:37 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31570/ --- (Updated Feb. 28, 2015, 1:37 a.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-1097 https://issues.apache.org/jira/browse/AURORA-1097 Repository: aurora Description --- All instance update actions are supposed to be unique. Suppressing duplicats due to pause/resume cycle. Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java acdade3dca807a221b4da975d0310c91884ee752 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e24d6bde5f3479a75522e825cce4ec6c30c117aa Diff: https://reviews.apache.org/r/31570/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Review Request 31570: Suppressing duplicate update instance events.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31570/ --- Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-1097 https://issues.apache.org/jira/browse/AURORA-1097 Repository: aurora Description --- All instance update actions are supposed to be unique. Suppressing duplicats due to pause/resume cycle. Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java acdade3dca807a221b4da975d0310c91884ee752 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e24d6bde5f3479a75522e825cce4ec6c30c117aa Diff: https://reviews.apache.org/r/31570/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 31388: Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31388/#review74508 --- Ship it! Ship It! - Maxim Khutornenko On Feb. 27, 2015, 4:49 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31388/ --- (Updated Feb. 27, 2015, 4:49 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1093 https://issues.apache.org/jira/browse/AURORA-1093 Repository: aurora Description --- Update thrift API and internal code to use JobUpdateSummary.key rather than job key and id. Note: This change will leave the client in a broken state w.r.t. the scheduler. I will hold commiting this change until i have another review ready to go that updates the client. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 6a00ce2c30ce24851560c4d499c395194dbeed16 src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 8444d225e91c75e9a571049d28af7264b6d2d017 src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 6ade817284b4c0607fe1ff09ad61ea47768f7297 src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 4b8b00cc74d81aa5cc772a7b4e077da6a4ed2a83 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java d52c15e0a7083e0d02bbf6dbaa58d8956bc5dd6f src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 06de5384182f84f973a28e312c0dc4329f593dad src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java b9dce16fa705fc14b5af394f8edb6301c789aff4 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 56398f33206cfa1fa45601a21760c7e23e2616bf src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java d1bd3c975b2057b46597f38555c2a73d14835600 src/main/java/org/apache/aurora/scheduler/updater/Updates.java b8c919585307e27f154782ee179153165458bed7 src/main/python/apache/aurora/client/api/__init__.py 07d1b1c5405b1329a15e8900b07c2a3dba6c592f src/main/python/apache/aurora/client/cli/context.py ea64010ab20c1292fb3fd1f5eb71394b7c965743 src/main/python/apache/aurora/client/cli/update.py bfa7a27c4e283b981a4ce37d382aa117a0074ee1 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml c08f09a511e1fc631abdae50630b8b7ab10a440b src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql e6184dd2de0e86849d811ec05c168bbb4db8e8cd src/main/resources/scheduler/assets/breadcrumb.html b5f82ca120d3b76b63a61628437525cec8870015 src/main/resources/scheduler/assets/job.html 4a00a8863d676f168fbfce9f45ec4bad0244199f src/main/resources/scheduler/assets/js/controllers.js 092e7d5df2121f45f99f5a788187d52bebb7e5dd src/main/resources/scheduler/assets/js/services.js 87eef8c689ac2c2e8b6aae5c91ba7b0e3b353efd src/main/resources/scheduler/assets/latestUpdates.html 9844edadbdea1c123b1c0afea87dd94664f93f37 src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java b79b07c23e00fab524aef21481070c5f09c9fa18 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 81c8be7685854c47ed4a7c6deeffb6a0b80023ab src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java e969b972806bef9aa67729c8a35283df6145e687 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java d36418807ace4f61e8709a2827cea0e7bb8ac6b7 src/test/java/org/apache/aurora/scheduler/thrift/aop/ForwardingThrift.java 2405bb8ad379e355a7987e9bb4163e1693f18777 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 3ba6274b97393ac2228d3eac8ba1615cb57014f8 src/test/python/apache/aurora/client/api/test_api.py ff1aff2eac391f219bc7c2483a16e35f916a224c src/test/python/apache/aurora/client/cli/test_status.py 1f39aa1c50a41e174804751b4cce25b96b6aebed src/test/python/apache/aurora/client/cli/test_supdate.py 6413c0db20db5914c7b1537da2bbc6e110705dc2 Diff: https://reviews.apache.org/r/31388/diff/ Testing --- Manually :-( verified javascript changes by performing an async update in vagrant and viewing relevant pages. Thanks, Bill Farner
Re: Review Request 31508: Removing redundant scheduling loop in preemptor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31508/ --- (Updated Feb. 27, 2015, 8:23 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Changes --- Rebased. Bugs: AURORA-1156 https://issues.apache.org/jira/browse/AURORA-1156 Repository: aurora Description --- This is #1 from the attached ticket. Brings anywhere between 2% and 18% better perf in bechmark scenarios. BEFORE: ``` Benchmark Mode Samples Score Error Units o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100909677.646 ± 10103.466 ns/op o.a.a.b.SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 1332768.205 ± 16664.386 ns/op o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark avgt 100 69304405.590 ± 1536571.317 ns/op o.a.a.b.SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100870348.707 ± 16815.495 ns/op ``` AFTER: ``` Benchmark Mode Samples Score Error Units o.a.a.b.SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark avgt 100749864.522 ±6568.372 ns/op o.a.a.b.SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100 1125995.085 ± 19241.796 ns/op o.a.a.b.SchedulingBenchmarks.PreemptorFallbackForLargeClusterBenchmark.runBenchmark avgt 100 68028627.539 ± 1412569.919 ns/op o.a.a.b.SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark avgt 100766747.584 ± 13310.142 ns/op ``` Diffs (updated) - src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 833a3e0b088780e21f5f16434327c96447a25115 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 2845b3f72fca5c329a8b81ce796370ad95d94f02 Diff: https://reviews.apache.org/r/31508/diff/ Testing --- ./gradlew jmh Thanks, Maxim Khutornenko
Re: Review Request 31550: Add test coverage for MesosSchedulerImpl.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31550/#review74566 --- Ship it! Ship It! - Maxim Khutornenko On Feb. 27, 2015, 6:30 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31550/ --- (Updated Feb. 27, 2015, 6:30 p.m.) Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- This brings MesosSchedulerImpl to 100% line and branch coverage. Diffs - src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java aa8aaad975bac513ce7734b4807a2f736b493e69 src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d621586abc8b0e20ffa6bd2c39aa92bd5512c3dd Diff: https://reviews.apache.org/r/31550/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31525: Improving NearestFit reporting accuracy.
On Feb. 27, 2015, 7:30 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java, lines 110-113 https://reviews.apache.org/r/31525/diff/1/?file=879537#file879537line110 How about making score an EnumSetVetoType so that you can do a typesafe bitwise OR? These scores did not match directly to VetoType. However, your comment got me thinking it's quite easy to fix that. Created a new DEDICATED_CONSTRAINT_MISMATCH and moved all scores definitions into the VetoType enum. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31525/#review74554 --- On Feb. 27, 2015, 3:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31525/ --- (Updated Feb. 27, 2015, 3:19 a.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-1148 https://issues.apache.org/jira/browse/AURORA-1148 Repository: aurora Description --- Instead of relying on a fixed MAX_SCORE, vetoes are now graded by their relative weight (severity) with a dedicated constraint mismatch ranked higest and an insufficient resource mismatch - lowest. Together with break early rule in SchedulingFilter, this ensures heavier vetoes are given proper attention in the NearestFit. This simplifies `NearestFit` logic while also improving pending reason reporting accuracy and scheduling performance. Diffs - src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 3313bd2f9ed5a88375d88715dea5da1e9ad8b963 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java a020ce50d578fba7f32b370f448a49eb1c284147 src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java c3097e49c0f6588ea765aa4fab69dd35e3d90e8b src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b00668423a53a8cf6f4da3456bce3354f1fd2424 src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 78a236c0f9074692b67ce18e6e03f18fe4529e02 Diff: https://reviews.apache.org/r/31525/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 31493: Upgrading JMH plugin and framework versions.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31493/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- Updating jmh plugin to 0.2.0: https://github.com/melix/jmh-gradle-plugin/blob/master/CHANGELOG.txt The most notable is the fix to support `jmhVersion` attribute that allowed to bump up jmh version from 1.3.2 to current 1.6.1. JMH changelog: http://hg.openjdk.java.net/code-tools/jmh/ Among multiple improvements and stability fixes there is also https://bugs.openjdk.java.net/browse/CODETOOLS-7901127 fix that gets rid of: ``` warning: Supported source version 'RELEASE_6' from annotation processor 'org.openjdk.jmh.generators.BenchmarkProcessor' less than -source '1.7' ``` Also, bumping up max heap size as some of bechmarks OOM occasionally (noticed peak usage of 600Mb). Diffs - build.gradle 158d47ac273e75deb8cb1460281c84e85db4f248 Diff: https://reviews.apache.org/r/31493/diff/ Testing --- ./gradlew jmh Thanks, Maxim Khutornenko
Review Request 31525: Improving NearestFit reporting accuracy.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31525/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-1148 https://issues.apache.org/jira/browse/AURORA-1148 Repository: aurora Description --- Instead of relying on a fixed MAX_SCORE, vetoes are now graded by their relative weight (severity) with a dedicated constraint mismatch ranked higest and an insufficient resource mismatch - lowest. Together with break early rule in SchedulingFilter, this ensures heavier vetoes are given proper attention in the NearestFit. This simplifies `NearestFit` logic while also improving pending reason reporting accuracy and scheduling performance. Diffs - src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 3313bd2f9ed5a88375d88715dea5da1e9ad8b963 src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java a020ce50d578fba7f32b370f448a49eb1c284147 src/main/java/org/apache/aurora/scheduler/metadata/NearestFit.java c3097e49c0f6588ea765aa4fab69dd35e3d90e8b src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java b00668423a53a8cf6f4da3456bce3354f1fd2424 src/test/java/org/apache/aurora/scheduler/metadata/NearestFitTest.java 78a236c0f9074692b67ce18e6e03f18fe4529e02 Diff: https://reviews.apache.org/r/31525/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31421: Handle TASK_ERROR and TASK_STAGING states.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31421/#review74055 --- Ship it! Ship It! - Maxim Khutornenko On Feb. 25, 2015, 6:48 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31421/ --- (Updated Feb. 25, 2015, 6:48 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Bugs: AURORA-1001 https://issues.apache.org/jira/browse/AURORA-1001 Repository: aurora Description --- This is only a stop-gap to make sure we are compatible with mesos 0.22. See ticket for more detail. Diffs - src/main/java/org/apache/aurora/scheduler/base/Conversions.java 7b7650d74e32d6396ab75a413380666d5000eac3 src/test/java/org/apache/aurora/scheduler/base/ConversionsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31421/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31376: Introduce JobUpdateSummary.key field, dual write that field when it is read/received.
On Feb. 24, 2015, 8:54 p.m., Zameer Manji wrote: src/main/java/org/apache/aurora/scheduler/updater/Updates.java, line 57 https://reviews.apache.org/r/31376/diff/1/?file=874378#file874378line57 I don't think we should pass around any mutable state. I think data objects like JobUpdateSummary should be immutable and the extra complextity at call sites is worth the safety we get. I agree wrt permanent changes outside of backfill logic. This, however, is a short-term migration hack that is not supposed to be used anywhere outside of backfilling. Anyway, I am ok with the change. Just feels weird to go through all these hoops in this particular case. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31376/#review73906 --- On Feb. 24, 2015, 8:12 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31376/ --- (Updated Feb. 24, 2015, 8:12 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1093 https://issues.apache.org/jira/browse/AURORA-1093 Repository: aurora Description --- There are 3 ways a JobUpdateSummary lands in storage: - starting a job update via thrift - replaying a log record to save a job update - replaying a snapshot log record This change focuses on ensuring that `JobUpdateSummary.key` is always available for code that is currently relying on the legacy fields (`updateId` and `jobKey`) by cloning them. A follow-up change will alter all code inside the scheduler to only consume the new field (removing `Updates.getKey`). Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 6f6124a4c844a1abcf07401d80c3e50eb8b4 src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 384e9b536c3ee3edf7d90960aa51ef98948af088 src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 27981393d700c84f6aaa791f12b24d0cec28b5df src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 3e491d999dc3f9e116d3502e4c08c04bd6c6bab9 src/main/java/org/apache/aurora/scheduler/updater/Updates.java 6493d687d00a1d285777ce3c3b5dd45cd08ceb71 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 7685597bb6acafe1412b23234227adb0eddad429 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java f4c92e64c36fb0c000ab8e9199dfed2fc35dcb36 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7e5d0edefbed3f67116361da15dd4c969c67cfe8 src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 152e70489b351b5bcf06f85baddbe31259d0aef4 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 945144dcb5240c3713d909344c82a9312cd3ba5c src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 2088a8f1be04218f2a5abc7fb4b04170b37ba2d1 Diff: https://reviews.apache.org/r/31376/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.
On Feb. 23, 2015, 10:52 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/async/OfferManager.java, line 255 https://reviews.apache.org/r/30891/diff/7/?file=873086#file873086line255 s/may/will/? Changed. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/#review73703 --- On Feb. 23, 2015, 8:36 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/ --- (Updated Feb. 23, 2015, 8:36 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 3 of 4: Filtering out statically banned offers. Will not apply cleanly: diffed with https://reviews.apache.org/r/30890 as a parent. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferManager.java b241d7b22c3d1ceca127b2671eb608ae41283bf3 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 21ea7d2b9d2f8c76367d7ae985270402bb51ea26 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 5a0f7ddb7e8fa6869cbb0fdfd07c6881780c6917 src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java 7ee2bb9bec6c59ba67b65d5b1229c64aca1277ff src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 9eef52a333e09454e8fd0026371c7e64472a883d src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java b6d4d8e771c7d16a46e43c7d5c427b911f8b661d Diff: https://reviews.apache.org/r/30891/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.
On Feb. 24, 2015, 7:35 p.m., Kevin Sweeney wrote:
src/main/java/org/apache/aurora/scheduler/async/OfferManager.java, line 301
https://reviews.apache.org/r/30891/diff/7/?file=873086#file873086line301
No need to hold the intrinsic lock while logging here
Having an explicit lock will be inconsistent with the rest of this class and
will add up to clutter. Given the on-demand logging here, I don't see much
value in over-optimizing this logic.
On Feb. 24, 2015, 7:35 p.m., Kevin Sweeney wrote:
src/main/java/org/apache/aurora/scheduler/async/OfferManager.java, line 317
https://reviews.apache.org/r/30891/diff/7/?file=873086#file873086line317
Same as above - no need to hold the intrinsic lock while logging here.
Same here.
On Feb. 24, 2015, 7:35 p.m., Kevin Sweeney wrote:
src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java,
line 78
https://reviews.apache.org/r/30891/diff/7/?file=873089#file873089line78
It would be good form to reset the level in the tearDown here.
Sure, done.
On Feb. 24, 2015, 7:35 p.m., Kevin Sweeney wrote:
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java, line 119
https://reviews.apache.org/r/30891/diff/3/?file=862800#file862800line119
as written, a caller needs to know FAILURE and FAILURE_STATIC_MISMATCH
both mean failure, but it would be easy to just check for `== FAILURE` and
miss that in code reviews. Would it make sense to add `isFailure()` and
`isStaticMismatch()` here?
Also, consider fetching the VetoGroup in the constructor so that
precondition checks will fail faster
as written, a caller needs to know FAILURE and FAILURE_STATIC_MISMATCH both
mean failure, but it would be easy to just check for == FAILURE and miss that
in code reviews. Would it make sense to add isFailure() and isStaticMismatch()
here?
This would shift failure detection responsibility upstream and will open up for
invalid combinations like isFailure=False isStaticMismatch=True. I believe
enum-based approach is the more accurate and easier to reason solution in this
case.
Also, consider fetching the VetoGroup in the constructor so that precondition
checks will fail faster
`Veto.identifyGroup` does not provide any additional error handling. It's
already failing fast as the only way to create an instance with vetoes is
through this:
```
public static Assignment failure(SetVeto vetoes) {
return new Assignment(NO_TASK_INFO, MorePreconditions.checkNotBlank(vetoes));
}
```
- Maxim
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30891/#review72448
---
On Feb. 23, 2015, 8:36 p.m., Maxim Khutornenko wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30891/
---
(Updated Feb. 23, 2015, 8:36 p.m.)
Review request for Aurora, Kevin Sweeney and Bill Farner.
Bugs: AURORA-909
https://issues.apache.org/jira/browse/AURORA-909
Repository: aurora
Description
---
Offer filtering for static vetoes. Part 3 of 4: Filtering out statically
banned offers.
Will not apply cleanly: diffed with https://reviews.apache.org/r/30890 as a
parent.
Original RB: https://reviews.apache.org/r/28617/
Diffs
-
src/main/java/org/apache/aurora/scheduler/async/OfferManager.java
b241d7b22c3d1ceca127b2671eb608ae41283bf3
src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java
21ea7d2b9d2f8c76367d7ae985270402bb51ea26
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java
5a0f7ddb7e8fa6869cbb0fdfd07c6881780c6917
src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java
7ee2bb9bec6c59ba67b65d5b1229c64aca1277ff
src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java
9eef52a333e09454e8fd0026371c7e64472a883d
src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java
b6d4d8e771c7d16a46e43c7d5c427b911f8b661d
Diff: https://reviews.apache.org/r/30891/diff/
Testing
---
./gradlew -Pq build
Thanks,
Maxim Khutornenko
Re: Review Request 29943: Uptime-driven scheduler job updates
On Feb. 24, 2015, 7:30 p.m., Kevin Sweeney wrote: Is this ready for review now? It is. However, since AURORA-1041 is still in Open I am going to discard it and repost when the ticket moves into Accepted. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29943/#review73877 --- On Jan. 20, 2015, 9:12 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/29943/ --- (Updated Jan. 20, 2015, 9:12 p.m.) Review request for Aurora, Kevin Sweeney, Bill Farner, and Brian Wickman. Bugs: AURORA-1041 https://issues.apache.org/jira/browse/AURORA-1041 Repository: aurora Description --- This is the first take on implementing job uptime driven updates. In addition to the olde good batch_size, instances can now be dispatched in arbitrary sequence depending on the overall uptime (health) of the job. The uptime is specified by a tuple of **waitForUptimeMs** and **waitForUptimePercentInstances** values. An excerpt from api.thrift explaining the feature: ``` /** * The uptime-driven update throttles the number of instances being updated at any given moment * according to the job uptime calculations. The X% of instances up over Y interval invariant * is preserved over the entire job update lifetime. No new instances are dispatched for update * unless that invariant is satisfied. Instances are dispatched in their natural uptime order, * shortest uptime first. * * For example, when set as below the update will block until at least 90% of job instances are in * RUNNING state for at least 1 minute: *waitForUptimeMs = 6 *waitForUptimePercentInstances = 90 * * When using uptime-driven update, it's expected that updateGroupSize is left unset to allow job * uptime settings drive the update progress. However, if updateGroupSize is set it will be * pre-applied before SLA uptime calculations to determine the update working set. As a side * effect, the updateGroupSize results in a natural ordering of instances taken for each group * (instances within a group are still updated in a shortest uptime first order). * * For example, if set as below the number of instances being updated at any given moment will * never exceed 5 even though the uptime calculations may allow more than 5: *updateGroupSize = 5 *waitForUptimeMs = 6 *waitForUptimePercentInstances = 90 * * NOTE on update rollback: with the uptime-driven update, there is no reliable way to ensure a * graceful throttled rollback as unstable/flapping instances may never yield an acceptable uptime * to perform an uptime-coordinated rollback. As such, when rollbackOnFailure=True AND the * updateGroupSize=0 the updater will dispatch all affected instances at once. * Use rollbackOnFailure=True with caution for uptime-driven updates. */ ``` For reviewers: recommend starting with api.thrift and then proceeding to the InstanceUptimeStrategy.java that implements the core algo. TODO: - vagrant e2e test - more corner case unit test coverage in JobUpdaterIT - client warning message in case uptime specs are used with client updater - docs Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 08ba1cdf88b712de22c26c04443079282db59ef9 src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java eae79d59b445ea58f46dc9e3107c03fbd83b6a95 src/main/java/org/apache/aurora/scheduler/sla/SlaUtil.java 156b9c0a2fa0c0ec4b7220d5ec2cc40c3e59d1d6 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java ac92959f34a3b0962d6aa018dc82a5ac72ea1b34 src/main/java/org/apache/aurora/scheduler/updater/InstanceUptimeProviderImpl.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java a992938d4e12b20f81608be6bbdc24c0a211c3fd src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 27a5b9026f5ac3b3bdeb32813b10435bc3dab173 src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java b53086169aa53d27a39a01cadf8d3c4a8ecb68de src/main/java/org/apache/aurora/scheduler/updater/UpdaterModule.java 5733da3daeacd8cb726310e5d9933635e3993687 src/main/java/org/apache/aurora/scheduler/updater/strategy/FilteringStrategy.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/strategy/InstanceUptimeProvider.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/updater/strategy/InstanceUptimeStrategy.java PRE-CREATION src/main/java/org/apache
Re: Review Request 31376: Introduce JobUpdateSummary.key field, dual write that field when it is read/received.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31376/#review73899 --- Ship it! src/main/java/org/apache/aurora/scheduler/updater/Updates.java https://reviews.apache.org/r/31376/#comment120333 Would it make sense to accept/return `JobUpdateSummary`? The mutable/immutable dance is rather confusing at call sites. - Maxim Khutornenko On Feb. 24, 2015, 8:12 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31376/ --- (Updated Feb. 24, 2015, 8:12 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1093 https://issues.apache.org/jira/browse/AURORA-1093 Repository: aurora Description --- There are 3 ways a JobUpdateSummary lands in storage: - starting a job update via thrift - replaying a log record to save a job update - replaying a snapshot log record This change focuses on ensuring that `JobUpdateSummary.key` is always available for code that is currently relying on the legacy fields (`updateId` and `jobKey`) by cloning them. A follow-up change will alter all code inside the scheduler to only consume the new field (removing `Updates.getKey`). Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 6f6124a4c844a1abcf07401d80c3e50eb8b4 src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 384e9b536c3ee3edf7d90960aa51ef98948af088 src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 27981393d700c84f6aaa791f12b24d0cec28b5df src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 3e491d999dc3f9e116d3502e4c08c04bd6c6bab9 src/main/java/org/apache/aurora/scheduler/updater/Updates.java 6493d687d00a1d285777ce3c3b5dd45cd08ceb71 src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 7685597bb6acafe1412b23234227adb0eddad429 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java f4c92e64c36fb0c000ab8e9199dfed2fc35dcb36 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 7e5d0edefbed3f67116361da15dd4c969c67cfe8 src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 152e70489b351b5bcf06f85baddbe31259d0aef4 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 945144dcb5240c3713d909344c82a9312cd3ba5c src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 2088a8f1be04218f2a5abc7fb4b04170b37ba2d1 Diff: https://reviews.apache.org/r/31376/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31350: Fix clusters.patch contextmanager cleanup
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/#review73832 --- src/test/python/apache/aurora/common/test_clusters.py https://reviews.apache.org/r/31350/#comment120234 How about populating with a real cluster here and asserting it later? src/test/python/apache/aurora/common/test_clusters.py https://reviews.apache.org/r/31350/#comment120235 `with pytest.raises():` should be more compact. src/test/python/apache/aurora/common/test_clusters.py https://reviews.apache.org/r/31350/#comment120237 Please, assert the entire contents here, e.g. `assert clusters == [Cluster(...)]` - Maxim Khutornenko On Feb. 24, 2015, 2:18 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31350/ --- (Updated Feb. 24, 2015, 2:18 p.m.) Review request for Aurora. Repository: aurora Description --- Fix clusters.patch contextmanager cleanup Otherwise one failing testcase can affect the entire suite. Diffs - src/main/python/apache/aurora/common/clusters.py c4b7fefca30313b281808478bf23158a9b7fbf85 src/test/python/apache/aurora/common/test_clusters.py 1bd696e9cd28d87d0cac68b33ab043407d796b61 Diff: https://reviews.apache.org/r/31350/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/common:: Thanks, Stephan Erb
Re: Review Request 31389: Remove LiveClusterState, make CachedClusterState (now ClusterStateImpl) default.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31389/#review73954 --- Ship it! Ship It! - Maxim Khutornenko On Feb. 25, 2015, 12:01 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31389/ --- (Updated Feb. 25, 2015, 12:01 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1055 https://issues.apache.org/jira/browse/AURORA-1055 Repository: aurora Description --- Remove LiveClusterState, make CachedClusterState (now ClusterStateImpl) default. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 0351b10cbfff76f07eec15c5ed1af7bb5f39a8ce src/main/java/org/apache/aurora/scheduler/async/preemptor/CachedClusterState.java 2831103457184d2049684c1c519b6bbb3b5b3e62 src/main/java/org/apache/aurora/scheduler/async/preemptor/LiveClusterState.java 38cb17371735d959c14fc7bf9fc25b8eb814fdfe src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 2030a9413035e1ad7db52b13957ed074b211d558 src/test/java/org/apache/aurora/scheduler/async/preemptor/CachedClusterStateTest.java 7cc04dd4ea214f7e9923be51b467a2564fec18bb src/test/java/org/apache/aurora/scheduler/async/preemptor/LiveClusterStateTest.java 763f8b5f31349f291c0ede7b5d3292f6ca5b src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 83cf118c9e1d32b0de511754b4aeadd8aed33e49 Diff: https://reviews.apache.org/r/31389/diff/ Testing --- Standard tests. I removed a test case `testIgnoresThrottledTasks`, which was actually testing LiveClusterState behavior. That test case was redundant to unit testing in `ClusterStateImpl`. Thanks, Bill Farner
Re: Review Request 30895: Offer filtering for static vetoes. Part 4 of 4: Benchmarks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30895/ --- (Updated Feb. 23, 2015, 9:03 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Bill's comments. Repository: aurora Description --- Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling. Original RB: https://reviews.apache.org/r/28617/ Diffs (updated) - src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61 src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25 Diff: https://reviews.apache.org/r/30895/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31171: Saving backups asynchronously.
On Feb. 20, 2015, 12:06 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java, line 261 https://reviews.apache.org/r/31171/diff/2/?file=869995#file869995line261 It may exceed your appetite for this review, but i would actually prefer that this transaction be opened at the origin of the operation. This would mean `createSnapshot()` might be best to accept a `StoreProvider`, and allow the caller to decide whether that provider is part of a read or write transaction. If you follow this for `applySnapshot` as well, it could accept `MutableStoreProvider` and have no need to inject `Storage`. Bill Farner wrote: (fwiw this advice is in line with other recent refactors [1] to push transactions up) [1] https://reviews.apache.org/r/26899/ For posterity: SnapshotStoreImpl uses a @Volatile storage instance, which cannot be enforced at all call sites (e.g. LogStorage). - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31171/#review73225 --- On Feb. 20, 2015, 10:48 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31171/ --- (Updated Feb. 20, 2015, 10:48 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-1108 https://issues.apache.org/jira/browse/AURORA-1108 Repository: aurora Description --- Wrapped backup file handling into Runnable to handle asynchronously. Refactoring somehow triggered a findbugs warning that I had to address as well: Exceptional return value of java.io.File.delete() ignored in org.apache.aurora.scheduler.storage.backup.StorageBackup$StorageBackupImpl$1.run() Diffs - src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java 71feb5779df5738a92e587f9f66f915961f52d1d src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java a20378a01575c399c23c86aa784424fc6909c34e src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 7602d112d29454608a97ec81de14b6bf0c45df68 src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java 15fc4404fa2ace4391e4ddc7153c848bc91d43df Diff: https://reviews.apache.org/r/31171/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30895: Offer filtering for static vetoes. Part 4 of 4: Benchmarks.
On Feb. 23, 2015, 7:47 p.m., Bill Farner wrote: src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java, line 33 https://reviews.apache.org/r/30895/diff/2/?file=873059#file873059line33 This is bound to become a Texas constructor [1]. Please consider using the builder pattern instead to improve readability at the call sites. Brief docs on these would be helpful too. Specifically, `clusterUtilization` would benefit from a blurb. [1] https://github.com/twitter/commons/blob/master/src/java/com/twitter/common/styleguide.md#stay-out-of-texas I can't force myself into justifying Builder here quite yet but I am fine with improving readabilty at call sites. Done. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30895/#review73642 --- On Feb. 23, 2015, 7:34 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30895/ --- (Updated Feb. 23, 2015, 7:34 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61 src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25 Diff: https://reviews.apache.org/r/30895/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.
On Feb. 21, 2015, 12:43 a.m., Bill Farner wrote: src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java, line 80 https://reviews.apache.org/r/30891/diff/5-6/?file=867194#file867194line80 This is kinda weird, what's the motivation here? Maxim Khutornenko wrote: Test coverage for the fine-logging statements. Maxim Khutornenko wrote: Just noticed you are referring to the `setUp()`. This is making sure anything with INFO is still logged and setting to FINE does not mess up with it depending on the test execution sequence. Bill Farner wrote: How about an injected `Logger` instead? Guice actually injects this by default, and there's no good reason we aren't using it. https://github.com/google/guice/wiki/BuiltInBindings It would require modifying HostOffers to accept a Logger instance and adjusting both OfferManagerImplTest and TaskSchedulerTest to expect info(), fine() and isLoggable(). I rejected that idea to reduce the diff size. Since info level folds into fine, set the default level to Fine in setUp() instead. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/#review73355 --- On Feb. 20, 2015, 11:58 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/ --- (Updated Feb. 20, 2015, 11:58 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 3 of 4: Filtering out statically banned offers. Will not apply cleanly: diffed with https://reviews.apache.org/r/30890 as a parent. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/main/java/org/apache/aurora/scheduler/async/OfferManager.java b241d7b22c3d1ceca127b2671eb608ae41283bf3 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 21ea7d2b9d2f8c76367d7ae985270402bb51ea26 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 5a0f7ddb7e8fa6869cbb0fdfd07c6881780c6917 src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java 7ee2bb9bec6c59ba67b65d5b1229c64aca1277ff src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 9eef52a333e09454e8fd0026371c7e64472a883d src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java b6d4d8e771c7d16a46e43c7d5c427b911f8b661d Diff: https://reviews.apache.org/r/30891/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/ --- (Updated Feb. 23, 2015, 8:36 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Bill's comments. Bugs: AURORA-909 https://issues.apache.org/jira/browse/AURORA-909 Repository: aurora Description --- Offer filtering for static vetoes. Part 3 of 4: Filtering out statically banned offers. Will not apply cleanly: diffed with https://reviews.apache.org/r/30890 as a parent. Original RB: https://reviews.apache.org/r/28617/ Diffs (updated) - src/main/java/org/apache/aurora/scheduler/async/OfferManager.java b241d7b22c3d1ceca127b2671eb608ae41283bf3 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 21ea7d2b9d2f8c76367d7ae985270402bb51ea26 src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 5a0f7ddb7e8fa6869cbb0fdfd07c6881780c6917 src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java 7ee2bb9bec6c59ba67b65d5b1229c64aca1277ff src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 9eef52a333e09454e8fd0026371c7e64472a883d src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java b6d4d8e771c7d16a46e43c7d5c427b911f8b661d Diff: https://reviews.apache.org/r/30891/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30895: Offer filtering for static vetoes. Part 4 of 4: Benchmarks.
On Feb. 23, 2015, 9:59 p.m., Bill Farner wrote: src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java, line 66 https://reviews.apache.org/r/30895/diff/3/?file=873099#file873099line66 Javadoc here would be nice. I failed to elaborate on the last review, but it's not immediatly obvious what this value means. My bad, forgot to address in the previous diff. Fixed now. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30895/#review73680 --- On Feb. 23, 2015, 9:03 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30895/ --- (Updated Feb. 23, 2015, 9:03 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling. Original RB: https://reviews.apache.org/r/28617/ Diffs - src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61 src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25 Diff: https://reviews.apache.org/r/30895/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 30895: Offer filtering for static vetoes. Part 4 of 4: Benchmarks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30895/ --- (Updated Feb. 23, 2015, 10:47 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Bill's comments. Repository: aurora Description --- Offer filtering for static vetoes. Part 4 of 4: Modifying benchmarks to support preemption toggling. Original RB: https://reviews.apache.org/r/28617/ Diffs (updated) - src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java bacfbfeb237ecddf82f58679e05be012c5214e61 src/jmh/java/org/apache/aurora/benchmark/Tasks.java 1a35f9ee9e8e76def0f9bf5454cf8cbdf6a89c25 Diff: https://reviews.apache.org/r/30895/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 31334: Fixing cron update quota checking.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31334/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-1134 https://issues.apache.org/jira/browse/AURORA-1134 Repository: aurora Description --- Cron schedule updates were treated as template addition rather than differential update, requiring extra quota capacity to clear the check. Adding a new `checkCronUpdate()` QuotaManager interface method to correctly handle cron job updates. This will no apply cleanly, diffed against https://reviews.apache.org/r/31241/ Diffs - src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15 src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java b0772f73f1a21da5828660bfd7d2b1f6b15cbf74 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b Diff: https://reviews.apache.org/r/31334/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31170: Refactor existing write APIs for job updates to use IJobUpdateKey.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31170/#review73342 --- Ship it! src/main/java/org/apache/aurora/scheduler/updater/Updates.java https://reviews.apache.org/r/31170/#comment119639 JobKeys.assertValid(summary.getJobKey()) should be better here. Also, you may want to inline these checks with the JobUpdateKey construction statement. - Maxim Khutornenko On Feb. 19, 2015, 8:30 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31170/ --- (Updated Feb. 19, 2015, 8:30 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Bugs: AURORA-1093 https://issues.apache.org/jira/browse/AURORA-1093 Repository: aurora Description --- In an effort to keep the diffs manageable, phase 2 started by changing the signatures of `JobUpdateStore.Mutable` (read APIs still use just the update ID string). This change is mostly mechanical, with a few noteworthy exceptions: - SaveJobUpdateEvent and SaveJobInstanceUpdateEvent now have a JobUpdateKey field, so LogStorage dual reads and writes - JobUpdateStore.fetchUpdateKey was added to facilitate the above If we are happy with this diff, i will create relevant 0.9.0 tickets to remove the old SaveJob[Instance]UpdateEvent fields. Diffs - api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3 src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 7a349bb36991851c6936ee990b529cc8c6fbc3d7 src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 3ca150c3088d99f331ca8e84a235f25e5eb26e17 src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 629a39b824a0f606f7697d637426510b6a0a41cb src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 34c4ab5adddbb62f117497b8007bc9b70ddd4490 src/main/java/org/apache/aurora/scheduler/storage/db/JobInstanceUpdateEventMapper.java 4fb33bd6a68d1f0c7b113558fbdcce328e51dbdb src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ba1672f06425db9477d52a91b36e0b0a1756430a src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 2d275997edb57d3474a33ea7cf924e2500334234 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15 src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java d0def6ee0ad31f9dbb47fe2052aaf4ec3540b1fa src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java f39d8768e7a83089f32b036ac072c50c3e0a66bd src/main/java/org/apache/aurora/scheduler/updater/Updates.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 1cd693a07dcc1fb3136a64e49f9481078fec45a1 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 156cbc49d8492c5a0209deae11c7be77ab2e0048 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 0a5cc51967f756411ca1489d81872f863c045b6b src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplTest.java 8fc3cb865fbcd467db91f4cb828d381a02ba7595 src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 9393ad7a3e09865ae0c88b983c577a73e6782016 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java a15c409d9b9aafd55372b5f6d7e775ebbf894ac1 Diff: https://reviews.apache.org/r/31170/diff/ Testing --- Thanks, Bill Farner
Review Request 31241: Pushing transactions up in QuotaManager.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31241/ --- Review request for Aurora and Bill Farner. Repository: aurora Description --- Removing Storage from QuotaManager by pushing transaction layer up the call chain. Will no apply cleanly. Diffed against https://reviews.apache.org/r/31235/. Diffs - src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 934b92021d08ca23d95888683e9527ce37a8690a src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java ee7618979ce94631af8aaf7ab3ecb2fbfb33fc38 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 2d21a976379631d11a498e7fcfd7cb6b800f3c15 src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java b0772f73f1a21da5828660bfd7d2b1f6b15cbf74 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 2b8a9e443e1c50ba7a11556bbcaf4dc5bb694dd4 Diff: https://reviews.apache.org/r/31241/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 31171: Saving backups asynchronously.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31171/ --- (Updated Feb. 20, 2015, 10:48 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Changes --- Joshua's comments. Bugs: AURORA-1108 https://issues.apache.org/jira/browse/AURORA-1108 Repository: aurora Description --- Wrapped backup file handling into Runnable to handle asynchronously. Refactoring somehow triggered a findbugs warning that I had to address as well: Exceptional return value of java.io.File.delete() ignored in org.apache.aurora.scheduler.storage.backup.StorageBackup$StorageBackupImpl$1.run() Diffs (updated) - src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java 71feb5779df5738a92e587f9f66f915961f52d1d src/main/java/org/apache/aurora/scheduler/storage/backup/StorageBackup.java a20378a01575c399c23c86aa784424fc6909c34e src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java ea33037d86f30f0787136f34dad34b88eceb0a4d src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 7602d112d29454608a97ec81de14b6bf0c45df68 src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java 15fc4404fa2ace4391e4ddc7153c848bc91d43df Diff: https://reviews.apache.org/r/31171/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
