Re: Review Request 30647: Instrument the HealthChecker to export stats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/#review73324 --- src/main/python/apache/aurora/executor/common/health_checker.py https://reviews.apache.org/r/30647/#comment119623 nit - can you include the unit in the name? i've seen time-based things misused more often than i care to recall src/test/python/apache/aurora/executor/common/test_health_checker.py https://reviews.apache.org/r/30647/#comment119625 Is it necessary to use a real clock + real threads + sleeps in this test? I'm not confident i can confirm that this test will not be flaky with these in place. - Bill Farner On Feb. 20, 2015, 7:33 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/ --- (Updated Feb. 20, 2015, 7:33 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1062 https://issues.apache.org/jira/browse/AURORA-1062 Repository: aurora Description --- Instrument the HealthChecker to export stats. HealthChecker plugin now should export three stats: consecutive_failures: number of consecutive failures experienced (resets on success) latency: how long health checks are taking in practice snoozed: whether or not the health checker is snoozed Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 60676ba0fbd8a218fe4309f07de28e2c66d54530 src/main/python/apache/aurora/executor/common/status_checker.py 624921d68199df098ea51ee8a10815403bf58984 src/test/python/apache/aurora/executor/common/test_health_checker.py a4e215d4422e3ada7b7913eaab105fdf030695c5 src/test/python/apache/aurora/executor/test_thermos_executor.py c8fab307d17949a8157659c4b3944ec7520feb9d Diff: https://reviews.apache.org/r/30647/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/executor/common:: Thanks, Brian Wickman
Review Request 31240: Remove render partial job key method.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31240/ --- Review request for Aurora, Maxim Khutornenko and Bill Farner. Repository: aurora Description --- This continues the refactoring started in https://reviews.apache.org/r/30207/ which will allow for easier testing and mocking out of the context object. This removes more single caller methods from the global context object and moves them closer to the callers. Diffs - src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf src/main/python/apache/aurora/client/cli/options.py 36f80aa175beaf8f2dc6b6f962394643313d6405 src/test/python/apache/aurora/client/cli/util.py 95a2123e127c9811fd2305e71cfc5c7c4376f904 Diff: https://reviews.apache.org/r/31240/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/client:: Thanks, Zameer Manji
Re: Review Request 30647: Instrument the HealthChecker to export stats.
On Feb. 20, 2015, 10:30 p.m., Bill Farner wrote: src/test/python/apache/aurora/executor/common/test_health_checker.py, line 136 https://reviews.apache.org/r/30647/diff/8/?file=870602#file870602line136 Is it necessary to use a real clock + real threads + sleeps in this test? I'm not confident i can confirm that this test will not be flaky with these in place. Brian Wickman wrote: this is not a real clock. it's a fake clock that allows us to test threaded behavior: https://github.com/twitter/commons/blob/master/src/python/twitter/common/testing/clock.py Aha! Thanks for clarifying. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/#review73324 --- On Feb. 20, 2015, 11:09 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/ --- (Updated Feb. 20, 2015, 11:09 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1062 https://issues.apache.org/jira/browse/AURORA-1062 Repository: aurora Description --- Instrument the HealthChecker to export stats. HealthChecker plugin now should export three stats: consecutive_failures: number of consecutive failures experienced (resets on success) latency: how long health checks are taking in practice snoozed: whether or not the health checker is snoozed Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 60676ba0fbd8a218fe4309f07de28e2c66d54530 src/main/python/apache/aurora/executor/common/status_checker.py 624921d68199df098ea51ee8a10815403bf58984 src/test/python/apache/aurora/executor/common/test_health_checker.py a4e215d4422e3ada7b7913eaab105fdf030695c5 src/test/python/apache/aurora/executor/test_thermos_executor.py c8fab307d17949a8157659c4b3944ec7520feb9d Diff: https://reviews.apache.org/r/30647/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/executor/common:: Thanks, Brian Wickman
Re: Review Request 30749: Port GC executor to PathDetector interface
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/#review73337 --- @Reviewbot retry - Brian Wickman On Feb. 20, 2015, 7:06 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/ --- (Updated Feb. 20, 2015, 7:06 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1025 https://issues.apache.org/jira/browse/AURORA-1025 Repository: aurora Description --- This makes the GC executor detect checkpoint roots via the PathDetector interface. This paves the way to detecting checkpoint roots both from /var/run/thermos and from /var/lib/mesos/** Diffs - src/main/python/apache/aurora/executor/bin/BUILD 6530f4914736754f92ba192c1a345e4b7e4a5398 src/main/python/apache/aurora/executor/bin/gc_executor_main.py b903bcb3630a8a8d50a2008bfae532b2eb947356 src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 0752d50015b2ff936f079c4a9f2777172dc00a93 src/main/python/apache/aurora/executor/gc_executor.py dbec82ffe4e155059f3e6b1aa0e67ec4c52a9611 src/main/python/apache/thermos/bin/thermos.py 161bbdbc4de95c82e2b596e77b0eac7b920eae66 src/main/python/apache/thermos/monitoring/garbage.py 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f src/test/python/apache/aurora/executor/bin/BUILD PRE-CREATION src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/test_gc_executor.py 27dee7fa10a4141ec7e9f4440bde2dd257db1cc6 Diff: https://reviews.apache.org/r/30749/diff/ Testing --- ./pants test.pytest --no-fast src/main/python:: Thanks, Brian Wickman
Re: Review Request 30647: Instrument the HealthChecker to export stats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/#review73338 --- Ship it! Ship It! - Bill Farner On Feb. 20, 2015, 11:09 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/ --- (Updated Feb. 20, 2015, 11:09 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1062 https://issues.apache.org/jira/browse/AURORA-1062 Repository: aurora Description --- Instrument the HealthChecker to export stats. HealthChecker plugin now should export three stats: consecutive_failures: number of consecutive failures experienced (resets on success) latency: how long health checks are taking in practice snoozed: whether or not the health checker is snoozed Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 60676ba0fbd8a218fe4309f07de28e2c66d54530 src/main/python/apache/aurora/executor/common/status_checker.py 624921d68199df098ea51ee8a10815403bf58984 src/test/python/apache/aurora/executor/common/test_health_checker.py a4e215d4422e3ada7b7913eaab105fdf030695c5 src/test/python/apache/aurora/executor/test_thermos_executor.py c8fab307d17949a8157659c4b3944ec7520feb9d Diff: https://reviews.apache.org/r/30647/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/executor/common:: Thanks, Brian Wickman
Re: Review Request 31241: Pushing transactions up in QuotaManager.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31241/#review73328 --- src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java https://reviews.apache.org/r/31241/#comment119627 Would `QuotaStore.Mutable` be sufficient? That would align with LoD. - Bill Farner On Feb. 20, 2015, 10:31 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31241/ --- (Updated Feb. 20, 2015, 10:31 p.m.) 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.
On Feb. 20, 2015, 5:56 p.m., Joshua Cohen wrote: Is it worth adding a test to StorageBackupTest asserting that we write the backup asynchronously (i.e. some expectation on the mock executor service)? Bill Farner wrote: This is not a bad idea. I wouldn't venture so far to actually use multiple threads, but you could have one test case where the executor swallows the task to prove that the backup is not writen. Maxim Khutornenko wrote: This is already tested by relying on a FakeScheduledExecutor mocking the `execute()`. If it was not mocked tests would fail with: ``` java.lang.AssertionError: Unexpected method call ScheduledExecutorService.execute(org.apache.aurora.scheduler.storage.backup.StorageBackup$StorageBackupImpl$1@2c7614d6): at org.easymock.internal.MockInvocationHandler.invoke(MockInvocationHandler.java:44) at org.easymock.internal.ObjectMethodsFilter.invoke(ObjectMethodsFilter.java:94) at com.sun.proxy.$Proxy7.execute(Unknown Source) at org.apache.aurora.scheduler.storage.backup.StorageBackup$StorageBackupImpl.createSnapshot(StorageBackup.java:141) ... ``` Well, i was going a step further and suggesting we test that the executor service is used for the write as opposed to something else. Though that may be a bit too paranoid. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31171/#review73291 --- 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 30749: Port GC executor to PathDetector interface
On Feb. 20, 2015, 11:11 p.m., Brian Wickman wrote: @Reviewbot retry Is that test known to be flaky? If so, can you file a ticket? - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/#review73337 --- On Feb. 20, 2015, 7:06 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/ --- (Updated Feb. 20, 2015, 7:06 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1025 https://issues.apache.org/jira/browse/AURORA-1025 Repository: aurora Description --- This makes the GC executor detect checkpoint roots via the PathDetector interface. This paves the way to detecting checkpoint roots both from /var/run/thermos and from /var/lib/mesos/** Diffs - src/main/python/apache/aurora/executor/bin/BUILD 6530f4914736754f92ba192c1a345e4b7e4a5398 src/main/python/apache/aurora/executor/bin/gc_executor_main.py b903bcb3630a8a8d50a2008bfae532b2eb947356 src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 0752d50015b2ff936f079c4a9f2777172dc00a93 src/main/python/apache/aurora/executor/gc_executor.py dbec82ffe4e155059f3e6b1aa0e67ec4c52a9611 src/main/python/apache/thermos/bin/thermos.py 161bbdbc4de95c82e2b596e77b0eac7b920eae66 src/main/python/apache/thermos/monitoring/garbage.py 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f src/test/python/apache/aurora/executor/bin/BUILD PRE-CREATION src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/test_gc_executor.py 27dee7fa10a4141ec7e9f4440bde2dd257db1cc6 Diff: https://reviews.apache.org/r/30749/diff/ Testing --- ./pants test.pytest --no-fast src/main/python:: Thanks, Brian Wickman
Re: Review Request 31240: Remove single caller methods from AuroraCommandContext
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31240/#review73341 --- Ship it! Ship It! - Bill Farner On Feb. 20, 2015, 10:29 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31240/ --- (Updated Feb. 20, 2015, 10:29 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Repository: aurora Description --- This continues the refactoring started in https://reviews.apache.org/r/30207/ which will allow for easier testing and mocking out of the context object. This removes more single caller methods from the global context object and moves them closer to the callers. Diffs - src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf src/main/python/apache/aurora/client/cli/options.py 36f80aa175beaf8f2dc6b6f962394643313d6405 src/test/python/apache/aurora/client/cli/util.py 95a2123e127c9811fd2305e71cfc5c7c4376f904 Diff: https://reviews.apache.org/r/31240/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/client:: Thanks, Zameer Manji
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
Re: Review Request 31171: Saving backups asynchronously.
On Feb. 20, 2015, 5:56 p.m., Joshua Cohen wrote: Is it worth adding a test to StorageBackupTest asserting that we write the backup asynchronously (i.e. some expectation on the mock executor service)? Bill Farner wrote: This is not a bad idea. I wouldn't venture so far to actually use multiple threads, but you could have one test case where the executor swallows the task to prove that the backup is not writen. This is already tested by relying on a FakeScheduledExecutor mocking the `execute()`. If it was not mocked tests would fail with: ``` java.lang.AssertionError: Unexpected method call ScheduledExecutorService.execute(org.apache.aurora.scheduler.storage.backup.StorageBackup$StorageBackupImpl$1@2c7614d6): at org.easymock.internal.MockInvocationHandler.invoke(MockInvocationHandler.java:44) at org.easymock.internal.ObjectMethodsFilter.invoke(ObjectMethodsFilter.java:94) at com.sun.proxy.$Proxy7.execute(Unknown Source) at org.apache.aurora.scheduler.storage.backup.StorageBackup$StorageBackupImpl.createSnapshot(StorageBackup.java:141) ... ``` On Feb. 20, 2015, 5:56 p.m., Joshua Cohen wrote: src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java, line 75 https://reviews.apache.org/r/31171/diff/2/?file=869997#file869997line75 I think this assignment obviates the need to assign to a new FakeClock above? Good catch. Done. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31171/#review73291 --- On Feb. 19, 2015, 11:58 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31171/ --- (Updated Feb. 19, 2015, 11:58 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 31240: Remove single caller methods from AuroraCommandContext
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31240/#review73343 --- Ship it! Ship It! - Maxim Khutornenko On Feb. 20, 2015, 10:29 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31240/ --- (Updated Feb. 20, 2015, 10:29 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Repository: aurora Description --- This continues the refactoring started in https://reviews.apache.org/r/30207/ which will allow for easier testing and mocking out of the context object. This removes more single caller methods from the global context object and moves them closer to the callers. Diffs - src/main/python/apache/aurora/client/cli/context.py 51c7d24dca664e476e62f1864d095416dfab70e4 src/main/python/apache/aurora/client/cli/jobs.py 8f349c09637c16e2499e85f2dc96eb7ccffd0aaf src/main/python/apache/aurora/client/cli/options.py 36f80aa175beaf8f2dc6b6f962394643313d6405 src/test/python/apache/aurora/client/cli/util.py 95a2123e127c9811fd2305e71cfc5c7c4376f904 Diff: https://reviews.apache.org/r/31240/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/client:: Thanks, Zameer Manji
Re: Review Request 30647: Instrument the HealthChecker to export stats.
On Feb. 20, 2015, 10:30 p.m., Bill Farner wrote: src/test/python/apache/aurora/executor/common/test_health_checker.py, line 136 https://reviews.apache.org/r/30647/diff/8/?file=870602#file870602line136 Is it necessary to use a real clock + real threads + sleeps in this test? I'm not confident i can confirm that this test will not be flaky with these in place. this is not a real clock. it's a fake clock that allows us to test threaded behavior: https://github.com/twitter/commons/blob/master/src/python/twitter/common/testing/clock.py On Feb. 20, 2015, 10:30 p.m., Bill Farner wrote: src/main/python/apache/aurora/executor/common/health_checker.py, line 161 https://reviews.apache.org/r/30647/diff/8/?file=870600#file870600line161 nit - can you include the unit in the name? i've seen time-based things misused more often than i care to recall sure - Brian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/#review73324 --- On Feb. 20, 2015, 7:33 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/ --- (Updated Feb. 20, 2015, 7:33 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1062 https://issues.apache.org/jira/browse/AURORA-1062 Repository: aurora Description --- Instrument the HealthChecker to export stats. HealthChecker plugin now should export three stats: consecutive_failures: number of consecutive failures experienced (resets on success) latency: how long health checks are taking in practice snoozed: whether or not the health checker is snoozed Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 60676ba0fbd8a218fe4309f07de28e2c66d54530 src/main/python/apache/aurora/executor/common/status_checker.py 624921d68199df098ea51ee8a10815403bf58984 src/test/python/apache/aurora/executor/common/test_health_checker.py a4e215d4422e3ada7b7913eaab105fdf030695c5 src/test/python/apache/aurora/executor/test_thermos_executor.py c8fab307d17949a8157659c4b3944ec7520feb9d Diff: https://reviews.apache.org/r/30647/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/executor/common:: Thanks, Brian Wickman
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. 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. - 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 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
On Feb. 21, 2015, 12:39 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java, line 78 https://reviews.apache.org/r/31248/diff/1/?file=871337#file871337line78 Any reason to lower case these? I'd actually prefer upper cased (default) values. This would be consistent with other places where we use enums (e.g. task_store_ASSIGNED). Yeah, not sure why i did that. Done. On Feb. 21, 2015, 12:39 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java, line 216 https://reviews.apache.org/r/31248/diff/1/?file=871335#file871335line216 Should it rather be micros (given that the only current consumer accepts microseconds)? Sure. Also added a guard at the receiving side against =0 delta in case of 1ms delivery or clock skew. On Feb. 21, 2015, 12:39 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java, line 199 https://reviews.apache.org/r/31248/diff/1/?file=871335#file871335line199 Use StringBuilder instead to avoid heap churn. javac is smart enough here http://stackoverflow.com/questions/11942368/why-use-stringbuilder-explicitly-if-the-compiler-converts-string-concatenation-t - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/#review73351 --- On Feb. 21, 2015, 12:04 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- (Updated Feb. 21, 2015, 12:04 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- (Updated Feb. 21, 2015, 1:36 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31241: Pushing transactions up in QuotaManager.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31241/#review73373 --- This patch does not apply cleanly on master (e5de618), do you need to rebase? I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Feb. 21, 2015, 12:15 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31241/ --- (Updated Feb. 21, 2015, 12:15 a.m.) 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 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/#review73372 --- Ship it! Master (e5de618) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Feb. 21, 2015, 1:36 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- (Updated Feb. 21, 2015, 1:36 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
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/#review73376 --- Ship it! Master (e5de618) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot 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 31251: Fix swallowed exceptions in health check test.
On Feb. 21, 2015, 2:10 a.m., Aurora ReviewBot wrote: Master (e5de618) is red with this patch. ./build-support/jenkins/build.sh src.test.python.apache.aurora.client.cli.update . SUCCESS src.test.python.apache.aurora.client.cli.version . SUCCESS src.test.python.apache.aurora.client.config . SUCCESS 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.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 I ran gc_executor test locally a dozen times and managed to get this one: FAILURES test_gc_lifetime def test_gc_lifetime(): with
Re: Review Request 30647: Instrument the HealthChecker to export stats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/#review73384 --- This patch does not apply cleanly on master (e5de618), do you need to rebase? I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Feb. 20, 2015, 11:09 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/ --- (Updated Feb. 20, 2015, 11:09 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1062 https://issues.apache.org/jira/browse/AURORA-1062 Repository: aurora Description --- Instrument the HealthChecker to export stats. HealthChecker plugin now should export three stats: consecutive_failures: number of consecutive failures experienced (resets on success) latency: how long health checks are taking in practice snoozed: whether or not the health checker is snoozed Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 60676ba0fbd8a218fe4309f07de28e2c66d54530 src/main/python/apache/aurora/executor/common/status_checker.py 624921d68199df098ea51ee8a10815403bf58984 src/test/python/apache/aurora/executor/common/test_health_checker.py a4e215d4422e3ada7b7913eaab105fdf030695c5 src/test/python/apache/aurora/executor/test_thermos_executor.py c8fab307d17949a8157659c4b3944ec7520feb9d Diff: https://reviews.apache.org/r/30647/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/executor/common:: Thanks, Brian Wickman
Re: Review Request 30749: Port GC executor to PathDetector interface
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/#review73383 --- Ship it! Master (e5de618) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Feb. 20, 2015, 7:06 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/ --- (Updated Feb. 20, 2015, 7:06 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1025 https://issues.apache.org/jira/browse/AURORA-1025 Repository: aurora Description --- This makes the GC executor detect checkpoint roots via the PathDetector interface. This paves the way to detecting checkpoint roots both from /var/run/thermos and from /var/lib/mesos/** Diffs - src/main/python/apache/aurora/executor/bin/BUILD 6530f4914736754f92ba192c1a345e4b7e4a5398 src/main/python/apache/aurora/executor/bin/gc_executor_main.py b903bcb3630a8a8d50a2008bfae532b2eb947356 src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 0752d50015b2ff936f079c4a9f2777172dc00a93 src/main/python/apache/aurora/executor/gc_executor.py dbec82ffe4e155059f3e6b1aa0e67ec4c52a9611 src/main/python/apache/thermos/bin/thermos.py 161bbdbc4de95c82e2b596e77b0eac7b920eae66 src/main/python/apache/thermos/monitoring/garbage.py 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f src/test/python/apache/aurora/executor/bin/BUILD PRE-CREATION src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/test_gc_executor.py 27dee7fa10a4141ec7e9f4440bde2dd257db1cc6 Diff: https://reviews.apache.org/r/30749/diff/ Testing --- ./pants test.pytest --no-fast src/main/python:: Thanks, Brian Wickman
Re: Review Request 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
On Feb. 21, 2015, 12:39 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java, line 199 https://reviews.apache.org/r/31248/diff/1/?file=871335#file871335line199 Use StringBuilder instead to avoid heap churn. Bill Farner wrote: javac is smart enough here http://stackoverflow.com/questions/11942368/why-use-stringbuilder-explicitly-if-the-compiler-converts-string-concatenation-t Maxim Khutornenko wrote: This is not obvious and may be dependent on particular JVM complier version/options. I'd rather go with an explicit approach than second guess JVM internals. Bill Farner wrote: Mind if we wait to observe a problem? This seems like premature optimization in a relatively infrequent code path, i'd rather favor readabilty. Maxim Khutornenko wrote: How do you propose we observe the problem? :) It's all these unaccounted/non-obvious little fragments that may contribute to the churn. Since it logs on every task status update unconditionally, I'd rather eliminate the potential problem than deal with possible consequences. Bill Farner wrote: Stepping back, String's javadoc actually documents this behavior [1] The Java language provides special support for the string concatenation operator ( + ), and for conversion of other objects to strings. String concatenation is implemented through the StringBuilder(or StringBuffer) class and its append method. String conversions are implemented through the method toString, defined by Object and inherited by all classes in Java. For additional information on string concatenation and conversion, see Gosling, Joy, and Steele, The Java Language Specification. [1] http://docs.oracle.com/javase/7/docs/api/java/lang/String.html This is better. Though specification is still non-assertive about it: To increase the performance of repeated string concatenation, a Java compiler **may use** the StringBuffer class or a similar technique to reduce the number of intermediate String objects that are created by evaluation of an expression. http://docs.oracle.com/javase/specs/jls/se7/html/jls-15.html#jls-15.18.1 - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/#review73351 --- On Feb. 21, 2015, 1:36 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- (Updated Feb. 21, 2015, 1:36 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
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. 20, 2015, 11:58 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 30749: Port GC executor to PathDetector interface
On Feb. 20, 2015, 11:11 p.m., Brian Wickman wrote: @Reviewbot retry Bill Farner wrote: Is that test known to be flaky? If so, can you file a ticket? Brian Wickman wrote: https://reviews.apache.org/r/30647/ should deflake it. Gotcha. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/#review73337 --- On Feb. 20, 2015, 7:06 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/ --- (Updated Feb. 20, 2015, 7:06 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1025 https://issues.apache.org/jira/browse/AURORA-1025 Repository: aurora Description --- This makes the GC executor detect checkpoint roots via the PathDetector interface. This paves the way to detecting checkpoint roots both from /var/run/thermos and from /var/lib/mesos/** Diffs - src/main/python/apache/aurora/executor/bin/BUILD 6530f4914736754f92ba192c1a345e4b7e4a5398 src/main/python/apache/aurora/executor/bin/gc_executor_main.py b903bcb3630a8a8d50a2008bfae532b2eb947356 src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 0752d50015b2ff936f079c4a9f2777172dc00a93 src/main/python/apache/aurora/executor/gc_executor.py dbec82ffe4e155059f3e6b1aa0e67ec4c52a9611 src/main/python/apache/thermos/bin/thermos.py 161bbdbc4de95c82e2b596e77b0eac7b920eae66 src/main/python/apache/thermos/monitoring/garbage.py 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f src/test/python/apache/aurora/executor/bin/BUILD PRE-CREATION src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/test_gc_executor.py 27dee7fa10a4141ec7e9f4440bde2dd257db1cc6 Diff: https://reviews.apache.org/r/30749/diff/ Testing --- ./pants test.pytest --no-fast src/main/python:: Thanks, Brian Wickman
Re: Review Request 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- (Updated Feb. 21, 2015, 1:11 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31241: Pushing transactions up in QuotaManager.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31241/#review73368 --- Ship it! Ship It! - Bill Farner On Feb. 21, 2015, 12:15 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31241/ --- (Updated Feb. 21, 2015, 12:15 a.m.) 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 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- (Updated Feb. 21, 2015, 1:34 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
On Feb. 21, 2015, 12:39 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java, line 199 https://reviews.apache.org/r/31248/diff/1/?file=871335#file871335line199 Use StringBuilder instead to avoid heap churn. Bill Farner wrote: javac is smart enough here http://stackoverflow.com/questions/11942368/why-use-stringbuilder-explicitly-if-the-compiler-converts-string-concatenation-t Maxim Khutornenko wrote: This is not obvious and may be dependent on particular JVM complier version/options. I'd rather go with an explicit approach than second guess JVM internals. Bill Farner wrote: Mind if we wait to observe a problem? This seems like premature optimization in a relatively infrequent code path, i'd rather favor readabilty. How do you propose we observe the problem? :) It's all these unaccounted/non-obvious little fragments that may contribute to the churn. Since it logs on every task status update unconditionally, I'd rather eliminate the potential problem than deal with possible consequences. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/#review73351 --- On Feb. 21, 2015, 1:36 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- (Updated Feb. 21, 2015, 1:36 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
Review Request 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
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? Test coverage for the fine-logging statements. - 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
Review Request 31251: Fix swallowed exceptions in health check test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31251/ --- Review request for Aurora, Joe Smith and Zameer Manji. Repository: aurora Description --- Fix swallowed exceptions in health check test. One of the health checkers was raising uncaught exceptions on a separate thread. The Python interpreter on CI *might* behave slightly differently and fail here. To be honest I've not been able to reproduce the CI behavior, so hopefully buildbot can chime in here. Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 0d3365d395c65793b90ce96f277b7f4e4b1dcb99 src/test/python/apache/aurora/executor/common/test_health_checker.py 4e09d309d402cf678c69d033e5219e9c8e4ccaaa Diff: https://reviews.apache.org/r/31251/diff/ Testing --- Ran the following about 100 times: THERMOS_DEBUG=1 ./pants test.pytest --no-fast --options='-vs' src/test/python/apache/aurora/executor/common:health_checker Thanks, Brian Wickman
Re: Review Request 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
On Feb. 21, 2015, 12:53 a.m., Joshua Cohen wrote: Can you fill in testing done? Honest question - do you find that useful for changes like this? I find it redundant to always type `./gradlew build -Pq`, especially since the build bot will do that anyhow. On Feb. 21, 2015, 12:53 a.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java, lines 199-200 https://reviews.apache.org/r/31248/diff/1/?file=871335#file871335line199 Use StringBuilder and String.format? See my reply to Maxim's comment. javac is smart about using StringBuilder behind the scenes in cases like this. Do you find String.format more readable in cases like this? Personally i do not. On Feb. 21, 2015, 12:53 a.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java, line 44 https://reviews.apache.org/r/31248/diff/1/?file=871337#file871337line44 s/loast/lost Fixed. On Feb. 21, 2015, 12:53 a.m., Joshua Cohen wrote: src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java, line 85 https://reviews.apache.org/r/31248/diff/1/?file=871339#file871339line85 move these to previous line (here, elsewhere)? Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/#review73353 --- On Feb. 21, 2015, 1:11 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- (Updated Feb. 21, 2015, 1:11 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31241: Pushing transactions up in QuotaManager.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31241/ --- (Updated Feb. 21, 2015, 12:15 a.m.) Review request for Aurora and Bill Farner. Changes --- Bill's comments. 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 (updated) - 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 31241: Pushing transactions up in QuotaManager.
On Feb. 20, 2015, 10:37 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java, line 77 https://reviews.apache.org/r/31241/diff/1/?file=871169#file871169line77 Would `QuotaStore.Mutable` be sufficient? That would align with LoD. Done, Though we normally don't use store types in interfaces outside Storage package. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31241/#review73328 --- On Feb. 20, 2015, 10:31 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31241/ --- (Updated Feb. 20, 2015, 10:31 p.m.) 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 31170: Refactor existing write APIs for job updates to use IJobUpdateKey.
On Feb. 19, 2015, 1:11 p.m., Zameer Manji wrote: Ship it, modulo my concern over uncessarily creating an IJobUpdateKey from PruneVictim. Bill clarified the creating IJobUpdateKey from PruneVictim offline. PruneVictim needs to have the mutable types so it can be populated from storage. - Zameer --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31170/#review73199 --- On Feb. 19, 2015, 12: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, 12: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
Re: Review Request 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
On Feb. 21, 2015, 12:39 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java, line 199 https://reviews.apache.org/r/31248/diff/1/?file=871335#file871335line199 Use StringBuilder instead to avoid heap churn. Bill Farner wrote: javac is smart enough here http://stackoverflow.com/questions/11942368/why-use-stringbuilder-explicitly-if-the-compiler-converts-string-concatenation-t Maxim Khutornenko wrote: This is not obvious and may be dependent on particular JVM complier version/options. I'd rather go with an explicit approach than second guess JVM internals. Bill Farner wrote: Mind if we wait to observe a problem? This seems like premature optimization in a relatively infrequent code path, i'd rather favor readabilty. Maxim Khutornenko wrote: How do you propose we observe the problem? :) It's all these unaccounted/non-obvious little fragments that may contribute to the churn. Since it logs on every task status update unconditionally, I'd rather eliminate the potential problem than deal with possible consequences. Stepping back, String's javadoc actually documents this behavior [1] The Java language provides special support for the string concatenation operator ( + ), and for conversion of other objects to strings. String concatenation is implemented through the StringBuilder(or StringBuffer) class and its append method. String conversions are implemented through the method toString, defined by Object and inherited by all classes in Java. For additional information on string concatenation and conversion, see Gosling, Joy, and Steele, The Java Language Specification. [1] http://docs.oracle.com/javase/7/docs/api/java/lang/String.html - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/#review73351 --- On Feb. 21, 2015, 1:36 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- (Updated Feb. 21, 2015, 1:36 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31251: Fix swallowed exceptions in health check test.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31251/#review73380 --- Master (e5de618) is red with this patch. ./build-support/jenkins/build.sh src.test.python.apache.aurora.client.cli.update . SUCCESS src.test.python.apache.aurora.client.cli.version . SUCCESS src.test.python.apache.aurora.client.config . SUCCESS 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.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 - Aurora ReviewBot On Feb. 21, 2015, 1:09 a.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31251/ --- (Updated Feb. 21, 2015, 1:09 a.m.) Review request for Aurora, Joe Smith and Zameer Manji.
Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.
On Feb. 19, 2015, 10:05 p.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/async/OfferManager.java, line 295
https://reviews.apache.org/r/30891/diff/5/?file=867191#file867191line295
Some debug visibility could be _really_ helpful here.
Consider a `LOG.isLoggable(Level.FINE)` guard + `LOG.fine()` here and
in `addStaticGroupBan`.
Done.
On Feb. 19, 2015, 10:05 p.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/async/OfferManager.java, line 299
https://reviews.apache.org/r/30891/diff/5/?file=867191#file867191line299
It wouldn't hurt to ensure consistency between `staticallyBannedOffers`
and `offersById` by making sure that the offer ID exists in `offersById`
before adding to this map. It's plausible for `launchFirst` to race
removal of an offer, resulting in a memory leak in `staticallyBannedOffers`.
Done.
On Feb. 19, 2015, 10:05 p.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java, line 63
https://reviews.apache.org/r/30891/diff/5/?file=867193#file867193line63
More context would be helpful here, specifically about what a static
mismatch is. (Feel free to link to `VetoGroup#STATIC`.
Linked to VetoGroup.
On Feb. 19, 2015, 10:05 p.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java, line 117
https://reviews.apache.org/r/30891/diff/5/?file=867193#file867193line117
I find this more readable:
if (Veto.identifyGroup(vetoes) == VetoGroup.STATIC) {
return Result.FAILURE_STATIC_MISMATCH;
} else {
return Result.FAILURE;
}
Or, if you prefer ternary:
return (Veto.identifyGroup(vetoes) == VetoGroup.STATIC)
? Result.FAILURE_STATIC_MISMATCH
: Result.FAILURE;
Done.
On Feb. 19, 2015, 10:05 p.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/async/OfferManager.java, line 251
https://reviews.apache.org/r/30891/diff/5/?file=867191#file867191line251
It would be helpful to include a comment about why things land in this
map, if only to save the reader from tracing what goes in.
Also, please TODO+ticket exposing this map via a debug endpoint.
Done and done.
- Maxim
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/30891/#review73208
---
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 30749: Port GC executor to PathDetector interface
On Feb. 10, 2015, 7:12 p.m., Joshua Cohen wrote: src/main/python/apache/thermos/monitoring/garbage.py, line 120 https://reviews.apache.org/r/30749/diff/9/?file=859049#file859049line120 Are we no longer worried about the scenario where the checkpoint root is $HOME or /? The checkpoint root could be set to this but only maliciously. On Feb. 10, 2015, 7:12 p.m., Joshua Cohen wrote: src/test/python/apache/aurora/executor/test_gc_executor.py, line 180 https://reviews.apache.org/r/30749/diff/9/?file=859053#file859053line180 s/'fake_root'/FAKE_ROOT fixed On Feb. 10, 2015, 7:12 p.m., Joshua Cohen wrote: src/test/python/apache/aurora/executor/test_gc_executor.py, lines 210-211 https://reviews.apache.org/r/30749/diff/9/?file=859053#file859053line210 Am I missing something or is this a duplicate of the make_task defined above? totally a dupe, deleted. On Feb. 10, 2015, 7:12 p.m., Joshua Cohen wrote: src/test/python/apache/aurora/executor/test_gc_executor.py, line 254 https://reviews.apache.org/r/30749/diff/9/?file=859053#file859053line254 nit: maybe give task_id a default value in make_task instead of having to always pass in the same placeholder value? Nah, because in all of these assertions we actually need to know the task id so it wouldn't make sense to omit from make_task(). - Brian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/#review71833 --- On Feb. 20, 2015, 7:06 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/ --- (Updated Feb. 20, 2015, 7:06 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1025 https://issues.apache.org/jira/browse/AURORA-1025 Repository: aurora Description --- This makes the GC executor detect checkpoint roots via the PathDetector interface. This paves the way to detecting checkpoint roots both from /var/run/thermos and from /var/lib/mesos/** Diffs - src/main/python/apache/aurora/executor/bin/BUILD 6530f4914736754f92ba192c1a345e4b7e4a5398 src/main/python/apache/aurora/executor/bin/gc_executor_main.py b903bcb3630a8a8d50a2008bfae532b2eb947356 src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 0752d50015b2ff936f079c4a9f2777172dc00a93 src/main/python/apache/aurora/executor/gc_executor.py dbec82ffe4e155059f3e6b1aa0e67ec4c52a9611 src/main/python/apache/thermos/bin/thermos.py 161bbdbc4de95c82e2b596e77b0eac7b920eae66 src/main/python/apache/thermos/monitoring/garbage.py 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f src/test/python/apache/aurora/executor/bin/BUILD PRE-CREATION src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/test_gc_executor.py 27dee7fa10a4141ec7e9f4440bde2dd257db1cc6 Diff: https://reviews.apache.org/r/30749/diff/ Testing --- ./pants test.pytest --no-fast src/main/python:: Thanks, Brian Wickman
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/#review73355 --- src/test/java/org/apache/aurora/scheduler/async/OfferManagerImplTest.java https://reviews.apache.org/r/30891/#comment119653 This is kinda weird, what's the motivation here? - Bill Farner 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 31170: Refactor existing write APIs for job updates to use IJobUpdateKey.
On Feb. 20, 2015, 11:36 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/updater/Updates.java, line 43 https://reviews.apache.org/r/31170/diff/2/?file=869688#file869688line43 JobKeys.assertValid(summary.getJobKey()) should be better here. Also, you may want to inline these checks with the JobUpdateKey construction statement. Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31170/#review73342 --- 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
Re: Review Request 31171: Saving backups asynchronously.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31171/#review73334 --- Ship it! Ship It! - Bill Farner 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 31171: Saving backups asynchronously.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31171/#review73291 --- Is it worth adding a test to StorageBackupTest asserting that we write the backup asynchronously (i.e. some expectation on the mock executor service)? src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java https://reviews.apache.org/r/31171/#comment119559 I think this assignment obviates the need to assign to a new FakeClock above? - Joshua Cohen On Feb. 19, 2015, 11:58 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31171/ --- (Updated Feb. 19, 2015, 11:58 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 30749: Port GC executor to PathDetector interface
On Feb. 10, 2015, 7:19 p.m., Joe Smith wrote: src/main/python/apache/aurora/executor/gc_executor.py, line 73 https://reviews.apache.org/r/30749/diff/9/?file=859047#file859047line73 Some documentation would go a long way here. I assume this is the way to go from `task_id` - `path_of_the_checkpoint_stream_on_disk` ? Added some commentary. On Feb. 10, 2015, 7:19 p.m., Joe Smith wrote: src/main/python/apache/aurora/executor/gc_executor.py, line 129 https://reviews.apache.org/r/30749/diff/9/?file=859047#file859047line129 s/PathDetector/path_detector, I think whoops, thanks. On Feb. 10, 2015, 7:19 p.m., Joe Smith wrote: src/main/python/apache/aurora/executor/gc_executor.py, line 135 https://reviews.apache.org/r/30749/diff/9/?file=859047#file859047line135 ``` :param task: an instance of a task to retrieve checkpoint path :type task: RootedTask instance ``` Does it make sense to pydoc a private method? i'll add it but i also don't want to necessarily set a precedent. On Feb. 10, 2015, 7:19 p.m., Joe Smith wrote: src/main/python/apache/thermos/monitoring/garbage.py, line 31 https://reviews.apache.org/r/30749/diff/9/?file=859049#file859049line31 Mind adding a docstring here and for the `__init__` ? done On Feb. 10, 2015, 7:19 p.m., Joe Smith wrote: src/main/python/apache/thermos/monitoring/garbage.py, line 173 https://reviews.apache.org/r/30749/diff/9/?file=859049#file859049line173 Everytime I see a namedtuple I cringe a bit (since I really only see them as a poor replacement for spec'd mocks) but if you don't mind adding docs here that'd be ~alright. This code is on the chopping block, so I wouldn't lament the general practices here too much. - Brian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/#review71831 --- On Feb. 9, 2015, 11:16 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/ --- (Updated Feb. 9, 2015, 11:16 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1025 https://issues.apache.org/jira/browse/AURORA-1025 Repository: aurora Description --- This makes the GC executor detect checkpoint roots via the PathDetector interface. This paves the way to detecting checkpoint roots both from /var/run/thermos and from /var/lib/mesos/** Diffs - src/main/python/apache/aurora/executor/bin/BUILD 1fa1e9e839bf28093b4e9ded403a761cf4bf5f44 src/main/python/apache/aurora/executor/bin/gc_executor_main.py b903bcb3630a8a8d50a2008bfae532b2eb947356 src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 0752d50015b2ff936f079c4a9f2777172dc00a93 src/main/python/apache/aurora/executor/gc_executor.py 43b415bc6c5177be24ec036cc32ae7cbd87fc70f src/main/python/apache/thermos/bin/thermos.py 161bbdbc4de95c82e2b596e77b0eac7b920eae66 src/main/python/apache/thermos/monitoring/garbage.py 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f src/test/python/apache/aurora/executor/bin/BUILD PRE-CREATION src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/test_gc_executor.py b1bbc89a822302d8ea12324eb767631326639ebb Diff: https://reviews.apache.org/r/30749/diff/ Testing --- ./pants test.pytest --no-fast src/main/python:: Thanks, Brian Wickman
Re: Review Request 30749: Port GC executor to PathDetector interface
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/#review73303 --- Master (e0e3f2e) is red with this patch. ./build-support/jenkins/build.sh src.test.python.apache.aurora.client.cli.sla . SUCCESS src.test.python.apache.aurora.client.cli.supdate . SUCCESS src.test.python.apache.aurora.client.cli.task . SUCCESS src.test.python.apache.aurora.client.cli.update . SUCCESS src.test.python.apache.aurora.client.cli.version . SUCCESS src.test.python.apache.aurora.client.config . SUCCESS 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 . FAILURE src.test.python.apache.aurora.executor.common.path_detector . 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.status_manager . SUCCESS src.test.python.apache.aurora.executor.thermos_task_runner . 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 - Aurora ReviewBot On Feb. 20, 2015, 7:06 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/ --- (Updated Feb. 20, 2015, 7:06 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji.
Re: Review Request 30749: Port GC executor to PathDetector interface
On Feb. 20, 2015, 11:11 p.m., Brian Wickman wrote: @Reviewbot retry Bill Farner wrote: Is that test known to be flaky? If so, can you file a ticket? https://reviews.apache.org/r/30647/ should deflake it. - Brian --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/#review73337 --- On Feb. 20, 2015, 7:06 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30749/ --- (Updated Feb. 20, 2015, 7:06 p.m.) Review request for Aurora, Joshua Cohen and Zameer Manji. Bugs: AURORA-1025 https://issues.apache.org/jira/browse/AURORA-1025 Repository: aurora Description --- This makes the GC executor detect checkpoint roots via the PathDetector interface. This paves the way to detecting checkpoint roots both from /var/run/thermos and from /var/lib/mesos/** Diffs - src/main/python/apache/aurora/executor/bin/BUILD 6530f4914736754f92ba192c1a345e4b7e4a5398 src/main/python/apache/aurora/executor/bin/gc_executor_main.py b903bcb3630a8a8d50a2008bfae532b2eb947356 src/main/python/apache/aurora/executor/bin/thermos_executor_main.py 0752d50015b2ff936f079c4a9f2777172dc00a93 src/main/python/apache/aurora/executor/gc_executor.py dbec82ffe4e155059f3e6b1aa0e67ec4c52a9611 src/main/python/apache/thermos/bin/thermos.py 161bbdbc4de95c82e2b596e77b0eac7b920eae66 src/main/python/apache/thermos/monitoring/garbage.py 69bf8e4c2e2e5f85f6b822fbe45f828d61814d7f src/test/python/apache/aurora/executor/bin/BUILD PRE-CREATION src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/bin/test_thermos_executor_entry_point.py PRE-CREATION src/test/python/apache/aurora/executor/test_gc_executor.py 27dee7fa10a4141ec7e9f4440bde2dd257db1cc6 Diff: https://reviews.apache.org/r/30749/diff/ Testing --- ./pants test.pytest --no-fast src/main/python:: Thanks, Brian Wickman
Re: Review Request 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/#review73351 --- src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java https://reviews.apache.org/r/31248/#comment119647 Use StringBuilder instead to avoid heap churn. src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java https://reviews.apache.org/r/31248/#comment119648 Should it rather be micros (given that the only current consumer accepts microseconds)? src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java https://reviews.apache.org/r/31248/#comment119646 Any reason to lower case these? I'd actually prefer upper cased (default) values. This would be consistent with other places where we use enums (e.g. task_store_ASSIGNED). - Maxim Khutornenko On Feb. 21, 2015, 12:04 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- (Updated Feb. 21, 2015, 12:04 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 31248: Export stats for source and reason for LOST tasks, and status delivery latency.
On Feb. 21, 2015, 12:39 a.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java, line 199 https://reviews.apache.org/r/31248/diff/1/?file=871335#file871335line199 Use StringBuilder instead to avoid heap churn. Bill Farner wrote: javac is smart enough here http://stackoverflow.com/questions/11942368/why-use-stringbuilder-explicitly-if-the-compiler-converts-string-concatenation-t This is not obvious and may be dependent on particular JVM complier version/options. I'd rather go with an explicit approach than second guess JVM internals. - Maxim --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/#review73351 --- On Feb. 21, 2015, 1:11 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31248/ --- (Updated Feb. 21, 2015, 1:11 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1028 https://issues.apache.org/jira/browse/AURORA-1028 Repository: aurora Description --- Export stats for source and reason for LOST tasks, and status delivery latency. Diffs - src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 1d8f0128732756db74576ee669f6a2718fecc105 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java ffc30bb548706df7bec9e1502242890e9b5eb942 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java 59ad9e65589c421cefb76f265446fa2885e6198c src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java d02c6b32841d5d39c5780e7a044079a38729effb src/test/java/org/apache/aurora/scheduler/mesos/TaskStatusStatsTest.java PRE-CREATION Diff: https://reviews.apache.org/r/31248/diff/ Testing --- Thanks, Bill Farner
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/ --- (Updated Feb. 21, 2015, 1:43 a.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 (updated) - 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 31235: Refactoring CronJobManager interface.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31235/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora Description --- Removing job store proxy methods from CronJobManager and dropping job manager concept from the storage. The job manager is long gone but we still carry around the job manager ID. Despite the diff's size the changes are mostly mechanical: replacing CronJobManager occurrences with direct job store access. Diffs - api/src/main/thrift/org/apache/aurora/gen/storage.thrift 3798797bbd4a7f26b78bfd63e2d275cbec60cab3 src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java b8830d187de27533307a8a2e9be6385f5d3e2289 src/main/java/org/apache/aurora/scheduler/cron/CronJobManager.java fc6e4432d64e625a583d8c8a130d99e066fd232c src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java efea7ad9f6efc99b600d071c3c20063b6bc4b211 src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java c5cb8cae43c86ae2378a0bef7688d400aa188e57 src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java 64d9486fade3b03b8db936fe60790ea0858212a9 src/main/java/org/apache/aurora/scheduler/http/StructDump.java 063170ac869dafc161c73f735b33f4cbe8e03ab6 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/JobStore.java ad0d67a27628f46dedda2ae4e0e61025dff1e1fd src/main/java/org/apache/aurora/scheduler/storage/Storage.java 83a59ce5d5b2754b1d31354280eca31922d73cdd src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 52377bca8060720dc4bec884c911182c3e77bc52 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/storage/mem/MemJobStore.java c24a32e57b34a1fc41681fda9bdb4de38ed8896b 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/SchedulerLifecycleTest.java 97ecb742d6e0418890f875394ded8d9fdae2b1c2 src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java c99135ab9c55a42ab51f18cc5ea127b498f0721f src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 915d7c8294b3d8262021da1c30324f55d8413ff9 src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java 701536fff8948ef233523e114e45043992175891 src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java b0772f73f1a21da5828660bfd7d2b1f6b15cbf74 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 493150b062eddf2581a048a9e13826205b8f2c15 src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java 15fc4404fa2ace4391e4ddc7153c848bc91d43df 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/mem/MemJobStoreTest.java 2e0c4151f58a28b695c13a392bae126857d4c4b6 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 06c8faa9de4d0ac8389dbf07d4e81934b503761b Diff: https://reviews.apache.org/r/31235/diff/ Testing --- ./gradlew -Pq build ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Maxim Khutornenko
Re: Review Request 30647: Instrument the HealthChecker to export stats.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/#review73308 --- Ship it! Master (e0e3f2e) is green with this patch. ./build-support/jenkins/build.sh I will refresh this build result if you post a review containing @ReviewBot retry - Aurora ReviewBot On Feb. 20, 2015, 7:33 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30647/ --- (Updated Feb. 20, 2015, 7:33 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1062 https://issues.apache.org/jira/browse/AURORA-1062 Repository: aurora Description --- Instrument the HealthChecker to export stats. HealthChecker plugin now should export three stats: consecutive_failures: number of consecutive failures experienced (resets on success) latency: how long health checks are taking in practice snoozed: whether or not the health checker is snoozed Diffs - src/main/python/apache/aurora/executor/common/health_checker.py 60676ba0fbd8a218fe4309f07de28e2c66d54530 src/main/python/apache/aurora/executor/common/status_checker.py 624921d68199df098ea51ee8a10815403bf58984 src/test/python/apache/aurora/executor/common/test_health_checker.py a4e215d4422e3ada7b7913eaab105fdf030695c5 src/test/python/apache/aurora/executor/test_thermos_executor.py c8fab307d17949a8157659c4b3944ec7520feb9d Diff: https://reviews.apache.org/r/30647/diff/ Testing --- ./pants test.pytest --no-fast src/test/python/apache/aurora/executor/common:: Thanks, Brian Wickman
