Re: Review Request 48082: AURORA-1624 Make 'tier' required and remove support for 'production' flag in Job configuration - New thrift API for retrieving tier configuration
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48082/ --- (Updated June 7, 2016, 4:11 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Changes --- (1) Applied suggested refactorings (2) Updated release notes Repository: aurora Description --- AURORA-1624 Make 'tier' required and remove support for 'production' flag in Job configuration - New thrift API for retrieving tier configuration Diffs (updated) - RELEASE-NOTES.md 7d47cf63a5529b43ca06cfb0b9e171a90a56f7f8 api/src/main/thrift/org/apache/aurora/gen/api.thrift ed94f249d85ac0e438213924c777cf7c029a119a src/main/java/org/apache/aurora/scheduler/TierInfo.java ac8901fe90d57c541829247fa9fa0895eb019e87 src/main/java/org/apache/aurora/scheduler/TierManager.java b96189be0ada1623665c2bff2550c1d72d7bc3dd src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java e431b58c933886f46c095240704d3eb0ceea2d80 src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 0d4f04403ec20c21b7cfacd706557cd191579f0a src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4d032b96d55dd0b92fab338220e66628e38cbb11 src/main/resources/org/apache/aurora/scheduler/tiers.json f724c5ad03a1315bc55dac35d98fdef45625e017 src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 95174bb7454a9e7c075ebaa6a4f84bf55fbc2652 src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 6b4e7a0d5e64d0632c66273ad516f737b2ef4a92 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 2122f744089adaee24328a634a2f786d1ef9720f Diff: https://reviews.apache.org/r/48082/diff/ Testing --- Manual/Explorative: Invoked from CLI-side to see if the new API call works as intended Integration: ./build-support/jenkins/build.sh E2E: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Mehrdad Nurolahzade
Re: Review Request 48082: AURORA-1624 New thrift API for retrieving tier configuration
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48082/ --- (Updated June 7, 2016, 8:17 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Changes --- (1) Applied suggested refactoring (2) Updated release notes Summary (updated) - AURORA-1624 New thrift API for retrieving tier configuration Repository: aurora Description (updated) --- AURORA-1624 Make 'tier' required and remove support for 'production' flag in Job configuration (1) New thrift API for retrieving tier configuration (2) `production` field in `TaskConfig` thrift struct is deprecated (will be removed in next release) (3) `default` property in `tiers.json` to specify the name of the default tier Diffs (updated) - RELEASE-NOTES.md 96bc3ca4bfd85c3e307f186ab24252c297ba336c api/src/main/thrift/org/apache/aurora/gen/api.thrift ed94f249d85ac0e438213924c777cf7c029a119a src/main/java/org/apache/aurora/scheduler/TierInfo.java ac8901fe90d57c541829247fa9fa0895eb019e87 src/main/java/org/apache/aurora/scheduler/TierManager.java b96189be0ada1623665c2bff2550c1d72d7bc3dd src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java e431b58c933886f46c095240704d3eb0ceea2d80 src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 0d4f04403ec20c21b7cfacd706557cd191579f0a src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4d032b96d55dd0b92fab338220e66628e38cbb11 src/main/resources/org/apache/aurora/scheduler/tiers.json f724c5ad03a1315bc55dac35d98fdef45625e017 src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 95174bb7454a9e7c075ebaa6a4f84bf55fbc2652 src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 6b4e7a0d5e64d0632c66273ad516f737b2ef4a92 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 2122f744089adaee24328a634a2f786d1ef9720f Diff: https://reviews.apache.org/r/48082/diff/ Testing --- Manual/Explorative: Invoked from CLI-side to see if the new API call works as intended Integration: ./build-support/jenkins/build.sh E2E: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Mehrdad Nurolahzade
Re: Review Request 48082: AURORA-1624 New thrift API for retrieving tier configuration
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48082/ --- (Updated June 7, 2016, 8:40 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Changes --- +ticket Bugs: AURORA-1624 https://issues.apache.org/jira/browse/AURORA-1624 Repository: aurora Description --- AURORA-1624 Make 'tier' required and remove support for 'production' flag in Job configuration (1) New thrift API for retrieving tier configuration (2) `production` field in `TaskConfig` thrift struct is deprecated (will be removed in next release) (3) `default` property in `tiers.json` to specify the name of the default tier Diffs - RELEASE-NOTES.md 96bc3ca4bfd85c3e307f186ab24252c297ba336c api/src/main/thrift/org/apache/aurora/gen/api.thrift ed94f249d85ac0e438213924c777cf7c029a119a src/main/java/org/apache/aurora/scheduler/TierInfo.java ac8901fe90d57c541829247fa9fa0895eb019e87 src/main/java/org/apache/aurora/scheduler/TierManager.java b96189be0ada1623665c2bff2550c1d72d7bc3dd src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java e431b58c933886f46c095240704d3eb0ceea2d80 src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 0d4f04403ec20c21b7cfacd706557cd191579f0a src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4d032b96d55dd0b92fab338220e66628e38cbb11 src/main/resources/org/apache/aurora/scheduler/tiers.json f724c5ad03a1315bc55dac35d98fdef45625e017 src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 95174bb7454a9e7c075ebaa6a4f84bf55fbc2652 src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 6b4e7a0d5e64d0632c66273ad516f737b2ef4a92 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 2122f744089adaee24328a634a2f786d1ef9720f Diff: https://reviews.apache.org/r/48082/diff/ Testing --- Manual/Explorative: Invoked from CLI-side to see if the new API call works as intended Integration: ./build-support/jenkins/build.sh E2E: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Mehrdad Nurolahzade
Re: Review Request 48082: AURORA-1624 Make 'tier' required and remove support for 'production' flag in Job configuration - New thrift API for retrieving tier configuration
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48082/ --- (Updated June 6, 2016, 7:13 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Changes --- (1) Applied suggested refactorings (2) Raised AURORA-1708 for deprecation of 'production' flag Repository: aurora Description --- AURORA-1624 Make 'tier' required and remove support for 'production' flag in Job configuration - New thrift API for retrieving tier configuration Diffs (updated) - api/src/main/thrift/org/apache/aurora/gen/api.thrift ed94f249d85ac0e438213924c777cf7c029a119a src/main/java/org/apache/aurora/scheduler/TierManager.java b96189be0ada1623665c2bff2550c1d72d7bc3dd src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java e431b58c933886f46c095240704d3eb0ceea2d80 src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 0d4f04403ec20c21b7cfacd706557cd191579f0a src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4d032b96d55dd0b92fab338220e66628e38cbb11 src/main/resources/org/apache/aurora/scheduler/tiers.json f724c5ad03a1315bc55dac35d98fdef45625e017 src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 95174bb7454a9e7c075ebaa6a4f84bf55fbc2652 src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 6b4e7a0d5e64d0632c66273ad516f737b2ef4a92 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 2122f744089adaee24328a634a2f786d1ef9720f Diff: https://reviews.apache.org/r/48082/diff/ Testing --- Manual/Explorative: Invoked from CLI-side to see if the new API call works as intended Integration: ./build-support/jenkins/build.sh E2E: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Mehrdad Nurolahzade
Re: Review Request 48559: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - Backfill portion
/ResourceManagerTest.java 14ac54713acf69fc40807b8cf7345b7a043e1ad9 src/test/java/org/apache/aurora/scheduler/scheduling/RescheduleCalculatorImplTest.java 2d34729e8f371703ba250b8e82d82c8a2504fcac src/test/java/org/apache/aurora/scheduler/scheduling/TaskSchedulerImplTest.java fba427bd327e7f63b640c8b8753bfdeec3ee31e7 src/test/java/org/apache/aurora/scheduler/sla/SlaTestUtil.java 78f440f7546de9ed6842cb51db02b3bddc9a74ff src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java 94f5ca565476f62d72879837a0e7dafabcf30432 src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java fbcc4003f7647580b859f4dc08a16c3e471a8f18 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java b4d27f69ad5d4cce03da9f04424dc35d30e8af29 src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java c5193238db5cb65373f7cb69e6b38b2e3b051dae src/test/java/org/apache/aurora/scheduler/stats/ResourceCounterTest.java 19c1f8e949e31a2338a5cbbbf15228230377a9f5 src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java c316e497a34a45c7ada2ca83a1115e826c0f572f src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java b1593f682f48ea66339bc2372de3e4f14e40be32 src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java e870087e3d47906559410ff76515457f4ff99ff5 src/test/java/org/apache/aurora/scheduler/storage/db/DbAttributeStoreTest.java f47f4a8a492fb43bacd909dc520256ed028531dd src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java 08530397ff75081bde6f07f9d53317b5486e0da4 src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java 3e5296e40ba63dc06a4720f1ff2c1ff046613ea2 src/test/java/org/apache/aurora/scheduler/storage/log/LogManagerTest.java 73440517cddda643c0b84cc04cb8463cdea2da28 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java be1132b439948104458efdc82a6bbee43c20c4fd src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java e0cf602ead1530301b09eff60287b8fa48be63e8 src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java 0a2cd3d5b01c389f99fca169227aac35436d474b src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 13726cc11ab09cd4995233d9d31811b97b065275 src/test/java/org/apache/aurora/scheduler/storage/mem/StorageTransactionTest.java 25f34e2bc26c6d4754c1591fad7f2165dd465d32 src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 4f8158546f3eba8f79d653ad7a30f83d66cbce83 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 6f355d6e66c05651fa9b13356dd81b45bee52adc src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ecdc62ae3b21b73b6a6af80bb9855867a7e965e0 src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java a54d169caebfc211035386f64169ecd983e378d9 src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java 56c94b5caf414861212f673a27b84d46c07332e6 src/test/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriberTest.java 36f2c657c05a87e78a11a1b0be5779dfd6511ee5 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e157c0dfde5efc418448e138aa008ade742fe816 src/test/java/org/apache/aurora/scheduler/updater/KillTaskTest.java e5935f65924e7d9a2491cac8f4c1f575ec657776 Diff: https://reviews.apache.org/r/48559/diff/ Testing --- Manual under Vagrant: - Deployed old scheduler (with tier backfill support), created a job without tier, upgraded scheduler, noticed that tier has been backfilled - Tried the above scenario with both -use_beta_db_task_store=true and -use_beta_db_task_store=false configuration flags - Verified that if tier is already set it would not be altered - Verified that it works both when production = 'true' and production = 'false' End to End: ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ... *** OK (All tests passed) *** mesos-master start/running, process 26886 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real17m53.514s user0m1.443s sys 0m0.624s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 48559: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - Backfill portion
/aurora/scheduler/sla/SlaTestUtil.java 78f440f7546de9ed6842cb51db02b3bddc9a74ff src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java 94f5ca565476f62d72879837a0e7dafabcf30432 src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java fbcc4003f7647580b859f4dc08a16c3e471a8f18 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java b4d27f69ad5d4cce03da9f04424dc35d30e8af29 src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java c5193238db5cb65373f7cb69e6b38b2e3b051dae src/test/java/org/apache/aurora/scheduler/stats/ResourceCounterTest.java 19c1f8e949e31a2338a5cbbbf15228230377a9f5 src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java c316e497a34a45c7ada2ca83a1115e826c0f572f src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java b1593f682f48ea66339bc2372de3e4f14e40be32 src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java e870087e3d47906559410ff76515457f4ff99ff5 src/test/java/org/apache/aurora/scheduler/storage/db/DbAttributeStoreTest.java f47f4a8a492fb43bacd909dc520256ed028531dd src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java 08530397ff75081bde6f07f9d53317b5486e0da4 src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java 3e5296e40ba63dc06a4720f1ff2c1ff046613ea2 src/test/java/org/apache/aurora/scheduler/storage/log/LogManagerTest.java 73440517cddda643c0b84cc04cb8463cdea2da28 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java be1132b439948104458efdc82a6bbee43c20c4fd src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java e0cf602ead1530301b09eff60287b8fa48be63e8 src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java 0a2cd3d5b01c389f99fca169227aac35436d474b src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java 13726cc11ab09cd4995233d9d31811b97b065275 src/test/java/org/apache/aurora/scheduler/storage/mem/StorageTransactionTest.java 25f34e2bc26c6d4754c1591fad7f2165dd465d32 src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 4f8158546f3eba8f79d653ad7a30f83d66cbce83 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 6f355d6e66c05651fa9b13356dd81b45bee52adc src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ecdc62ae3b21b73b6a6af80bb9855867a7e965e0 src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java a54d169caebfc211035386f64169ecd983e378d9 src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java 56c94b5caf414861212f673a27b84d46c07332e6 src/test/java/org/apache/aurora/scheduler/updater/JobUpdateEventSubscriberTest.java 36f2c657c05a87e78a11a1b0be5779dfd6511ee5 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java e157c0dfde5efc418448e138aa008ade742fe816 src/test/java/org/apache/aurora/scheduler/updater/KillTaskTest.java e5935f65924e7d9a2491cac8f4c1f575ec657776 Diff: https://reviews.apache.org/r/48559/diff/ Testing --- Manual under Vagrant: - Deployed old scheduler (with tier backfill support), created a job without tier, upgraded scheduler, noticed that tier has been backfilled - Tried the above scenario with both -use_beta_db_task_store=true and -use_beta_db_task_store=false configuration flags - Verified that if tier is already set it would not be altered - Verified that it works both when production = 'true' and production = 'false' End to End: ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ... *** OK (All tests passed) *** mesos-master start/running, process 26886 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real17m53.514s user0m1.443s sys 0m0.624s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 48559: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - Backfill portion
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48559/#review137553 --- @ReviewBot retry - Mehrdad Nurolahzade On June 14, 2016, 10:12 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48559/ > --- > > (Updated June 14, 2016, 10:12 a.m.) > > > Review request for Aurora and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - Backfill portion > > > Diffs > - > > src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java > f4f8d0037751c9c2096747264c19f6292461b308 > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java > 65f5edce74077f52e98f110fcd17b2f12d673f81 > src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java > 6fd9ee21cf8e0e42e73a68cdf2d231d581278aae > src/jmh/java/org/apache/aurora/benchmark/Tasks.java > e548a09d94b1c6d550f9beec2b7120b64d576f20 > src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java > 7497fb96a578cff64b6484c587e29ec464969e84 > src/main/java/org/apache/aurora/scheduler/TierManager.java > af54cab73a80a5120b1a77fd985dfbaf568d786c > src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java > 3ea0992eb0a9930a4db9eb4b7fcab82689495c1f > > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java > 0e9562020c298e685e6c2efd18933818b03a5000 > > src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java > d08873c88f159eb65b582840b48b7ff604862c31 > > src/main/java/org/apache/aurora/scheduler/storage/db/migration/V006_PopulateTierField.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java > c981a05e61cb053a05144c702c9ffafeb0af8260 > > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java > 8eed1fc680b0c4fb27d8a353b7f804ae09058156 > src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java > 0a307fe8d8238c23a526d5c3ee500e1de0761703 > src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java > 999ef064737c3d8a3d7610b40c13736f51742edd > src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java > 05cd78f4c7c7d8dd6eeb6f2f9a3e8f7a167f274d > src/test/java/org/apache/aurora/scheduler/TierManagerTest.java > d4b71f8dbb674384ccbbd9e76f510d127e480e32 > src/test/java/org/apache/aurora/scheduler/TierModuleTest.java > 58d95dcdf31bc920ca1f8822baccc6c37b66e739 > src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java > 8c8c793813e84bf7ef741f9b6b4ae1e759be1b82 > src/test/java/org/apache/aurora/scheduler/base/JobsTest.java > 13f656f241a8a9a3d339f4053f165070c2669ef3 > src/test/java/org/apache/aurora/scheduler/base/TasksTest.java > 935622bef38cc3d399e8b5b1db84fa21c79c78e1 > > src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java > 2e322d217fc9dc75c51b57607a5547745206fb9f > > src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java > 5c64ff2994e200b3453603ac5470e8e152cebc55 > src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java > 1c0a3fa84874d7bc185b78f13d2664cb4d8dd72f > > src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java > 81440f5689f9538a4c7a9e6700bf03ca89c4ba85 > src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java > 3c5ecd698557cafdf8eeacdc472589a379018896 > src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java > 488eefd14c3e67a41a75c809397c8d19f83cc08a > src/test/java/org/apache/aurora/scheduler/http/MaintenanceTest.java > f94b58b77b7c6ce824914af7e1147e73ad5a7eed > src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java > ccef4ab930a7b7e1c10d611f0852aa65de82e726 > > src/test/java/org/apache/aurora/scheduler/mesos/MesosTaskFactoryImplTest.java > 58785bfa37ff214f26e9f94d836e6df40e411c3b > > src/test/java/org/apache/aurora/scheduler/preemptor/PreemptionVictimFilterTest.java > 7eb1714d14581a6ab25e85d36a1f3e973380c536 > > src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java > 99c27e8012f10a67ce5f1b84d258e7a5608995c7 > src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java > 2e97a3361feaed71e4f39cbd27cf5afb7d919e31 > > sr
Re: Review Request 48559: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - Backfill portion
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48559/#review137538 --- src/main/java/org/apache/aurora/scheduler/storage/db/migration/V006_PopulateTierField.java (lines 22 - 23) <https://reviews.apache.org/r/48559/#comment202685> Hardcoding tier names here can be problematic if someone decides to customize their tier configuration file. I tried looking up tier names dynamically through ```TierManager``` but then realized that migration scripts are instantiated through reflection by ibatis and Guice injection did not work on them (or I don't know enough about Guice to make it work). Any suggestions? - Mehrdad Nurolahzade On June 14, 2016, 9:46 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48559/ > --- > > (Updated June 14, 2016, 9:46 a.m.) > > > Review request for Aurora and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - Backfill portion > > > Diffs > - > > src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java > f4f8d0037751c9c2096747264c19f6292461b308 > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java > 65f5edce74077f52e98f110fcd17b2f12d673f81 > src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java > 6fd9ee21cf8e0e42e73a68cdf2d231d581278aae > src/jmh/java/org/apache/aurora/benchmark/Tasks.java > e548a09d94b1c6d550f9beec2b7120b64d576f20 > src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java > 7497fb96a578cff64b6484c587e29ec464969e84 > src/main/java/org/apache/aurora/scheduler/TierManager.java > af54cab73a80a5120b1a77fd985dfbaf568d786c > src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java > 3ea0992eb0a9930a4db9eb4b7fcab82689495c1f > > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java > 0e9562020c298e685e6c2efd18933818b03a5000 > > src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java > d08873c88f159eb65b582840b48b7ff604862c31 > > src/main/java/org/apache/aurora/scheduler/storage/db/migration/V006_PopulateTierField.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java > c981a05e61cb053a05144c702c9ffafeb0af8260 > > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java > 8eed1fc680b0c4fb27d8a353b7f804ae09058156 > src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java > 0a307fe8d8238c23a526d5c3ee500e1de0761703 > src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java > 999ef064737c3d8a3d7610b40c13736f51742edd > src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java > 05cd78f4c7c7d8dd6eeb6f2f9a3e8f7a167f274d > src/test/java/org/apache/aurora/scheduler/TierManagerTest.java > d4b71f8dbb674384ccbbd9e76f510d127e480e32 > src/test/java/org/apache/aurora/scheduler/TierModuleTest.java > 58d95dcdf31bc920ca1f8822baccc6c37b66e739 > src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java > 8c8c793813e84bf7ef741f9b6b4ae1e759be1b82 > src/test/java/org/apache/aurora/scheduler/base/JobsTest.java > 13f656f241a8a9a3d339f4053f165070c2669ef3 > src/test/java/org/apache/aurora/scheduler/base/TasksTest.java > 935622bef38cc3d399e8b5b1db84fa21c79c78e1 > > src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java > 2e322d217fc9dc75c51b57607a5547745206fb9f > > src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java > 5c64ff2994e200b3453603ac5470e8e152cebc55 > src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java > 1c0a3fa84874d7bc185b78f13d2664cb4d8dd72f > > src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java > 81440f5689f9538a4c7a9e6700bf03ca89c4ba85 > src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java > 3c5ecd698557cafdf8eeacdc472589a379018896 > src/test/java/org/apache/aurora/scheduler/events/WebhookTest.java > 488eefd14c3e67a41a75c809397c8d19f83cc08a > src/test/java/org/apache/aurora/scheduler/http/MaintenanceTest.java > f94b58b77b7c6ce824914af7e1147e73ad5a7eed > src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java > ccef4ab930a7b7e1c10d611f0852aa65de82e726 > > src/tes
Review Request 48082: AURORA-1624 Make 'tier' required and remove support for 'production' flag in Job configuration - New thrift API for retrieving tier configuration
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48082/ --- Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Repository: aurora Description --- AURORA-1624 Make 'tier' required and remove support for 'production' flag in Job configuration - New thrift API for retrieving tier configuration Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift ed94f249d85ac0e438213924c777cf7c029a119a src/main/java/org/apache/aurora/scheduler/TierManager.java b96189be0ada1623665c2bff2550c1d72d7bc3dd src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java e431b58c933886f46c095240704d3eb0ceea2d80 src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 0d4f04403ec20c21b7cfacd706557cd191579f0a src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 4d032b96d55dd0b92fab338220e66628e38cbb11 src/main/resources/org/apache/aurora/scheduler/tiers.json f724c5ad03a1315bc55dac35d98fdef45625e017 src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 95174bb7454a9e7c075ebaa6a4f84bf55fbc2652 src/test/java/org/apache/aurora/scheduler/TierModuleTest.java 6b4e7a0d5e64d0632c66273ad516f737b2ef4a92 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 2122f744089adaee24328a634a2f786d1ef9720f Diff: https://reviews.apache.org/r/48082/diff/ Testing --- Manual/Explorative: Invoked from CLI-side to see if the new API call works as intended Integration: ./build-support/jenkins/build.sh E2E: ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Mehrdad Nurolahzade
Re: Review Request 48559: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - Backfill portion
> On June 15, 2016, 10:41 a.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java, line 14 > > <https://reviews.apache.org/r/48559/diff/3/?file=1418765#file1418765line14> > > > > Not sure why this class has been moved but if you want to keep it > > please carve it out into a separate patch. Combining functional changes > > with moving files around inflates the diff size and makes it harder to > > understand the commit in retrospect. I can revert this package move refactoring. This was required by the refactorings to ```TaskTestUtil```. I can alternatively make the constructor of ```TierManagerImpl``` public (instead of moving them both to the same package). > On June 15, 2016, 10:41 a.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/storage/db/migration/V006_PopulateTierField.java, > > lines 50-52 > > <https://reviews.apache.org/r/48559/diff/3/?file=1418768#file1418768line50> > > > > This should be unnecessary as zero is the default value for > > 'production'. Isn't this required to resolve the inconsistencies between the choice of ```production``` and ```tier``` in already scheduled jobs? For example, if tier has been set to ```revocable``` and production to ```true``` in a job submitted to the old scheduler, shouldn't we resolve this by resetting ```production``` to ```false```? > On June 15, 2016, 10:41 a.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/TierManager.java, lines 125-127 > > <https://reviews.apache.org/r/48559/diff/3/?file=1418764#file1418764line125> > > > > Reiterating my previous comment: can this (and other places checking > > for tier presence) be simplified now that the tier is a required field? This logic is used by ```ThriftBackfill``` to set ```tier``` when it is not already set. Shouldn't this be removed in the next release when we drop the back fill logic? - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48559/#review137763 --- On June 14, 2016, 10:12 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48559/ > --- > > (Updated June 14, 2016, 10:12 a.m.) > > > Review request for Aurora and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - Backfill portion > > > Diffs > - > > src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java > f4f8d0037751c9c2096747264c19f6292461b308 > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java > 65f5edce74077f52e98f110fcd17b2f12d673f81 > src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java > 6fd9ee21cf8e0e42e73a68cdf2d231d581278aae > src/jmh/java/org/apache/aurora/benchmark/Tasks.java > e548a09d94b1c6d550f9beec2b7120b64d576f20 > src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java > 7497fb96a578cff64b6484c587e29ec464969e84 > src/main/java/org/apache/aurora/scheduler/TierManager.java > af54cab73a80a5120b1a77fd985dfbaf568d786c > src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java > 3ea0992eb0a9930a4db9eb4b7fcab82689495c1f > > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java > 0e9562020c298e685e6c2efd18933818b03a5000 > > src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java > d08873c88f159eb65b582840b48b7ff604862c31 > > src/main/java/org/apache/aurora/scheduler/storage/db/migration/V006_PopulateTierField.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java > c981a05e61cb053a05144c702c9ffafeb0af8260 > > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java > 8eed1fc680b0c4fb27d8a353b7f804ae09058156 > src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java > 0a307fe8d8238c23a526d5c3ee500e1de0761703 > src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java > 999ef064737c3d8a3d7610b40c13736f51742edd > src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java > 05cd78f4c7c7d8dd6eeb6f2f9a3e8f7a167f274d > src/test/java/org/apache/aurora/scheduler/TierManagerTest.java > d4b71f8dbb674384ccbbd9e76f510d127e480e32 > src/test/java/org/apache/auro
Re: Review Request 48559: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - Backfill portion
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48559/ --- (Updated June 17, 2016, 12:46 p.m.) Review request for Aurora and Maxim Khutornenko. Changes --- - Moved ```TaskTestUtil``` back to ```org.apache.aurora.scheduler.base``` package - Moved back fill related logic from ```TierManagerImpl``` to ```ThriftBackfill``` - Other suggested refactorings Note: This change set still does not include client changes. Repository: aurora Description --- AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - Backfill portion Diffs (updated) - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 65f5edce74077f52e98f110fcd17b2f12d673f81 src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 6fd9ee21cf8e0e42e73a68cdf2d231d581278aae src/main/java/org/apache/aurora/scheduler/TierManager.java af54cab73a80a5120b1a77fd985dfbaf568d786c src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 3ea0992eb0a9930a4db9eb4b7fcab82689495c1f src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 0e9562020c298e685e6c2efd18933818b03a5000 src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java d08873c88f159eb65b582840b48b7ff604862c31 src/main/java/org/apache/aurora/scheduler/storage/db/migration/V006_PopulateTierField.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java c981a05e61cb053a05144c702c9ffafeb0af8260 src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 8eed1fc680b0c4fb27d8a353b7f804ae09058156 src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java 0a307fe8d8238c23a526d5c3ee500e1de0761703 src/test/java/org/apache/aurora/scheduler/TierManagerTest.java d4b71f8dbb674384ccbbd9e76f510d127e480e32 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 8c8c793813e84bf7ef741f9b6b4ae1e759be1b82 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 2e322d217fc9dc75c51b57607a5547745206fb9f src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java e870087e3d47906559410ff76515457f4ff99ff5 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java be1132b439948104458efdc82a6bbee43c20c4fd src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java e0cf602ead1530301b09eff60287b8fa48be63e8 src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java 0a2cd3d5b01c389f99fca169227aac35436d474b src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 4f8158546f3eba8f79d653ad7a30f83d66cbce83 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 6f355d6e66c05651fa9b13356dd81b45bee52adc src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ecdc62ae3b21b73b6a6af80bb9855867a7e965e0 src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java a54d169caebfc211035386f64169ecd983e378d9 Diff: https://reviews.apache.org/r/48559/diff/ Testing --- Manual under Vagrant: - Deployed old scheduler (with tier backfill support), created a job without tier, upgraded scheduler, noticed that tier has been backfilled - Tried the above scenario with both -use_beta_db_task_store=true and -use_beta_db_task_store=false configuration flags - Verified that if tier is already set it would not be altered - Verified that it works both when production = 'true' and production = 'false' End to End: ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ... *** OK (All tests passed) *** mesos-master start/running, process 26886 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real17m53.514s user0m1.443s sys 0m0.624s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
> On June 21, 2016, 4:02 p.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/config/thrift.py, lines 231-258 > > <https://reviews.apache.org/r/49048/diff/1/?file=1427035#file1427035line231> > > > > A better place for this would be `get_job_config()` in > > apache/aurora/client/cli/context.py. This way a config would always get > > populated/backfilled on loading. > > > > Any validaiton/deprecation warnings should go to > > apache/aurora/client/config.py. Yes, you are right shifting to ```context``` causes the thermos executor dependency problem to go away, I'll move it. I initially had deprecation logic in ```config.py``` but then realized thrift conversion logic (that causes backfill) happens before this validation so pushed it up hierarchy. Now, I can move back to ```config.py``` again. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review138964 ------- On June 21, 2016, 4:09 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated June 21, 2016, 4:09 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/config/__init__.py > 65923be1cb8b88139b8eab0ac5b75428972d3cb1 > src/main/python/apache/aurora/config/thrift.py > 3539469d243638c0acd08bf0859d0ce858d8977c > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_inspect.py > fedc16b3d4e9fb7d6f5f0dc34ad7a1837e34baea > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > src/test/python/apache/aurora/config/__init__.py PRE-CREATION > src/test/python/apache/aurora/config/test_base.py > b354f0804ce70682e8ecb9fb3a3d8fe736fd1cc5 > src/test/python/apache/aurora/config/test_thrift.py > e213184739167e01f3614c20a809af39b3a6b3d6 > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > --- > > Solution fails end to end test, this is WIP intended to receive feedback > > I seem to have introduced a dependency between thermos executor and client > api that is causing the executor fail with the following sample error log: > ``` > cat > /var/lib/mesos/slaves/c8fd5700-d2ad-4249-b705-94d40451681b-S0/frameworks/c8fd5700-d2ad-4249-b705-94d40451681b-0001/executors/thermos-www-data-prod-hello-0-f4543715-21f2-402c-9a75-656cb90693b8/runs/19621370-6b72-4c9e-8569-d11c6ca67456/stderr > > I0621 21:29:00.888162 23895 fetcher.cpp:424] Fetcher Info: > {"cache_directory":"\/tmp\/mesos\/fetch\/slaves\/c8fd5700-d2ad-4249-b705-94d40451681b-S0\/root","items":[{"action":"BYPASS_CACHE","uri":{"executable":true,"extract":true,"value":"\/home\/vagrant\/aurora\/dist\/thermos_executor.pex"}}],"sandbox_directory":"\/var\/lib\/mesos\/slaves\/c8fd5700-d2ad-4249-b705-94d40451681b-S0\/frameworks\/c8fd5700-d2ad-4249-b705-94d40451681b-0001\/executors\/thermos-www-data-prod-hello-0-f4543715-21f2-402c-9a75-656cb90693b8\/runs\/19621370-6b72-4c9e-8569-d11c6ca67456","user":"root"} > I0621 21:29:00.889114 23895 fetcher.cpp:379] Fetching URI > '/home/vagrant/aurora/dist/thermos_executor.pex' > I0621 21:29:00.889127 23895 fetcher.cpp:250] Fetching directly into the > sandbox directory > I0621 21:29:00.889137 23895 fetcher.cpp:187] Fetching URI > '/home/vagrant/aurora/dist/thermos_executor.pex' > I0621 21:29:0
Re: Review Request 48559: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - Backfill portion
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48559/ --- (Updated June 20, 2016, 9:47 a.m.) Review request for Aurora and Maxim Khutornenko. Changes --- Applied suggested refactorings Repository: aurora Description --- AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - Backfill portion Diffs (updated) - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 65f5edce74077f52e98f110fcd17b2f12d673f81 src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 6fd9ee21cf8e0e42e73a68cdf2d231d581278aae src/main/java/org/apache/aurora/scheduler/TierManager.java af54cab73a80a5120b1a77fd985dfbaf568d786c src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 3ea0992eb0a9930a4db9eb4b7fcab82689495c1f src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java 0e9562020c298e685e6c2efd18933818b03a5000 src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java d08873c88f159eb65b582840b48b7ff604862c31 src/main/java/org/apache/aurora/scheduler/storage/db/migration/V006_PopulateTierField.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java c981a05e61cb053a05144c702c9ffafeb0af8260 src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 8eed1fc680b0c4fb27d8a353b7f804ae09058156 src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java 0a307fe8d8238c23a526d5c3ee500e1de0761703 src/test/java/org/apache/aurora/scheduler/TierManagerTest.java d4b71f8dbb674384ccbbd9e76f510d127e480e32 src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 8c8c793813e84bf7ef741f9b6b4ae1e759be1b82 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java 2e322d217fc9dc75c51b57607a5547745206fb9f src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java e870087e3d47906559410ff76515457f4ff99ff5 src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java be1132b439948104458efdc82a6bbee43c20c4fd src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java e0cf602ead1530301b09eff60287b8fa48be63e8 src/test/java/org/apache/aurora/scheduler/storage/log/ThriftBackfillTest.java 0a2cd3d5b01c389f99fca169227aac35436d474b src/test/java/org/apache/aurora/scheduler/thrift/Fixtures.java 4f8158546f3eba8f79d653ad7a30f83d66cbce83 src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java 6f355d6e66c05651fa9b13356dd81b45bee52adc src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java ecdc62ae3b21b73b6a6af80bb9855867a7e965e0 src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java a54d169caebfc211035386f64169ecd983e378d9 Diff: https://reviews.apache.org/r/48559/diff/ Testing --- Manual under Vagrant: - Deployed old scheduler (with tier backfill support), created a job without tier, upgraded scheduler, noticed that tier has been backfilled - Tried the above scenario with both -use_beta_db_task_store=true and -use_beta_db_task_store=false configuration flags - Verified that if tier is already set it would not be altered - Verified that it works both when production = 'true' and production = 'false' End to End: ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ... *** OK (All tests passed) *** mesos-master start/running, process 26886 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real17m53.514s user0m1.443s sys 0m0.624s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 48559: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - Backfill portion
> On June 15, 2016, 10:41 a.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/storage/db/migration/V006_PopulateTierField.java, > > lines 50-52 > > <https://reviews.apache.org/r/48559/diff/3/?file=1418768#file1418768line50> > > > > This should be unnecessary as zero is the default value for > > 'production'. > > Mehrdad Nurolahzade wrote: > Isn't this required to resolve the inconsistencies between the choice of > ```production``` and ```tier``` in already scheduled jobs? > For example, if tier has been set to ```revocable``` and production to > ```true``` in a job submitted to the old scheduler, shouldn't we resolve this > by resetting ```production``` to ```false```? Actually, thinking twice about this, do we even need a downgrade script? I mean, we are already modifying ```production``` based on choice of ```tier``` for scheduled jobs, so it should not be a concern when downgrading scheduler, right? - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48559/#review137763 ----------- On June 14, 2016, 10:12 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48559/ > --- > > (Updated June 14, 2016, 10:12 a.m.) > > > Review request for Aurora and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - Backfill portion > > > Diffs > - > > src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java > f4f8d0037751c9c2096747264c19f6292461b308 > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java > 65f5edce74077f52e98f110fcd17b2f12d673f81 > src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java > 6fd9ee21cf8e0e42e73a68cdf2d231d581278aae > src/jmh/java/org/apache/aurora/benchmark/Tasks.java > e548a09d94b1c6d550f9beec2b7120b64d576f20 > src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java > 7497fb96a578cff64b6484c587e29ec464969e84 > src/main/java/org/apache/aurora/scheduler/TierManager.java > af54cab73a80a5120b1a77fd985dfbaf568d786c > src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java > 3ea0992eb0a9930a4db9eb4b7fcab82689495c1f > > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java > 0e9562020c298e685e6c2efd18933818b03a5000 > > src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java > d08873c88f159eb65b582840b48b7ff604862c31 > > src/main/java/org/apache/aurora/scheduler/storage/db/migration/V006_PopulateTierField.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java > c981a05e61cb053a05144c702c9ffafeb0af8260 > > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java > 8eed1fc680b0c4fb27d8a353b7f804ae09058156 > src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java > 0a307fe8d8238c23a526d5c3ee500e1de0761703 > src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java > 999ef064737c3d8a3d7610b40c13736f51742edd > src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java > 05cd78f4c7c7d8dd6eeb6f2f9a3e8f7a167f274d > src/test/java/org/apache/aurora/scheduler/TierManagerTest.java > d4b71f8dbb674384ccbbd9e76f510d127e480e32 > src/test/java/org/apache/aurora/scheduler/TierModuleTest.java > 58d95dcdf31bc920ca1f8822baccc6c37b66e739 > src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java > 8c8c793813e84bf7ef741f9b6b4ae1e759be1b82 > src/test/java/org/apache/aurora/scheduler/base/JobsTest.java > 13f656f241a8a9a3d339f4053f165070c2669ef3 > src/test/java/org/apache/aurora/scheduler/base/TasksTest.java > 935622bef38cc3d399e8b5b1db84fa21c79c78e1 > > src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java > 2e322d217fc9dc75c51b57607a5547745206fb9f > > src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java > 5c64ff2994e200b3453603ac5470e8e152cebc55 > src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java > 1c0a3fa84874d7bc185b78f13d2664cb4d8dd72f > > src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java > 81440f5689f9538a4c7a9e6700bf03ca89c4ba85 > src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTes
Review Request 48796: AURORA-1458 Add tier into the UI "show config" summary
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48796/ --- Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- AURORA-1458 Add tier into the UI "show config" summary Diffs - src/main/resources/scheduler/assets/configSummary.html 86a87ab710312969c31802492d856b04f07c276d src/main/resources/scheduler/assets/js/services.js b3a0a994707972a9a19a7f4079dad67fe3136c1c Diff: https://reviews.apache.org/r/48796/diff/ Testing --- ``` ./build-support/jenkins/build.sh + date Thu Jun 16 08:57:08 PDT 2016 + ./gradlew -Pq clean build ... 09:03:14 00:03 [complete] SUCCESS ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 48559: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - Backfill portion
> On June 15, 2016, 10:41 a.m., Maxim Khutornenko wrote: > > src/main/java/org/apache/aurora/scheduler/storage/db/migration/V006_PopulateTierField.java, > > lines 50-52 > > <https://reviews.apache.org/r/48559/diff/3/?file=1418768#file1418768line50> > > > > This should be unnecessary as zero is the default value for > > 'production'. > > Mehrdad Nurolahzade wrote: > Isn't this required to resolve the inconsistencies between the choice of > ```production``` and ```tier``` in already scheduled jobs? > For example, if tier has been set to ```revocable``` and production to > ```true``` in a job submitted to the old scheduler, shouldn't we resolve this > by resetting ```production``` to ```false```? > > Mehrdad Nurolahzade wrote: > Actually, thinking twice about this, do we even need a downgrade script? > I mean, we are already modifying ```production``` based on choice of > ```tier``` for scheduled jobs, so it should not be a concern when downgrading > scheduler, right? > > Maxim Khutornenko wrote: > This is a matter of consistency with `ThriftBackfill` and while not > breaking functionality, could be confusing to have something like 'preferred > & prod=false' after a rollback. > > The 'revocable & prod=true' was the invalid combination in the first > place, so having rollback fixing consistency here feels right. So, keep it as is? - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/48559/#review137763 --- On June 14, 2016, 10:12 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/48559/ > --- > > (Updated June 14, 2016, 10:12 a.m.) > > > Review request for Aurora and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - Backfill portion > > > Diffs > - > > src/jmh/java/org/apache/aurora/benchmark/JobUpdates.java > f4f8d0037751c9c2096747264c19f6292461b308 > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java > 65f5edce74077f52e98f110fcd17b2f12d673f81 > src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java > 6fd9ee21cf8e0e42e73a68cdf2d231d581278aae > src/jmh/java/org/apache/aurora/benchmark/Tasks.java > e548a09d94b1c6d550f9beec2b7120b64d576f20 > src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java > 7497fb96a578cff64b6484c587e29ec464969e84 > src/main/java/org/apache/aurora/scheduler/TierManager.java > af54cab73a80a5120b1a77fd985dfbaf568d786c > src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java > 3ea0992eb0a9930a4db9eb4b7fcab82689495c1f > > src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java > 0e9562020c298e685e6c2efd18933818b03a5000 > > src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java > d08873c88f159eb65b582840b48b7ff604862c31 > > src/main/java/org/apache/aurora/scheduler/storage/db/migration/V006_PopulateTierField.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java > c981a05e61cb053a05144c702c9ffafeb0af8260 > > src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java > 8eed1fc680b0c4fb27d8a353b7f804ae09058156 > src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java > 0a307fe8d8238c23a526d5c3ee500e1de0761703 > src/test/java/org/apache/aurora/codec/ThriftBinaryCodecTest.java > 999ef064737c3d8a3d7610b40c13736f51742edd > src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java > 05cd78f4c7c7d8dd6eeb6f2f9a3e8f7a167f274d > src/test/java/org/apache/aurora/scheduler/TierManagerTest.java > d4b71f8dbb674384ccbbd9e76f510d127e480e32 > src/test/java/org/apache/aurora/scheduler/TierModuleTest.java > 58d95dcdf31bc920ca1f8822baccc6c37b66e739 > src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java > 8c8c793813e84bf7ef741f9b6b4ae1e759be1b82 > src/test/java/org/apache/aurora/scheduler/base/JobsTest.java > 13f656f241a8a9a3d339f4053f165070c2669ef3 > src/test/java/org/apache/aurora/scheduler/base/TasksTest.java > 935622bef38cc3d399e8b5b1db84fa21c79c78e1 > > src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java > 2e322d217fc9dc
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review139304 --- src/main/python/apache/aurora/config/__init__.py (line 15) <https://reviews.apache.org/r/49048/#comment204470> This is left over from refactoring, will remove. - Mehrdad Nurolahzade On June 23, 2016, 3:35 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated June 23, 2016, 3:35 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 > src/main/python/apache/aurora/client/config.py > 2fc12559016d406c347adb416a5166cca31c961e > src/main/python/apache/aurora/config/__init__.py > 65923be1cb8b88139b8eab0ac5b75428972d3cb1 > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_context.py > 204ca092adad8bf43c5032a02f61bf303fb0b2fc > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > --- > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > *** OK (All tests passed) *** > > mesos-master start/running, process 26868 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 19m46.324s > user 0m1.496s > sys 0m0.774s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/ --- (Updated June 23, 2016, 3:35 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Changes --- - Refactor/Moved backfill logic from ```thrift.py``` to ```context.py``` - Refactor/Moved deprecation warning logic to ```config.py``` Repository: aurora Description --- AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes Diffs (updated) - src/main/python/apache/aurora/client/api/__init__.py 68baf8fdb90cd26100159401c46c9963c24332b3 src/main/python/apache/aurora/client/cli/context.py 9b1511801d031ff48b81c25688a55cb586b8ac66 src/main/python/apache/aurora/client/config.py 2fc12559016d406c347adb416a5166cca31c961e src/main/python/apache/aurora/config/__init__.py 65923be1cb8b88139b8eab0ac5b75428972d3cb1 src/test/python/apache/aurora/client/cli/test_command_hooks.py 2130f1fa71be02a004cdf8e476a270c81a7105d3 src/test/python/apache/aurora/client/cli/test_context.py 204ca092adad8bf43c5032a02f61bf303fb0b2fc src/test/python/apache/aurora/client/cli/test_create.py 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c src/test/python/apache/aurora/client/cli/test_cron.py f3c522ed94a2d774865811ceb546bf9df083c14f src/test/python/apache/aurora/client/cli/test_plugins.py a545fece5e2b3e0017a61e1be9ac478372b1f34d src/test/python/apache/aurora/client/cli/test_restart.py 967d560e5c7eb0ed85b215fb11d9751b8666acb5 src/test/python/apache/aurora/client/cli/util.py 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e src/test/python/apache/aurora/client/test_config.py b1a3c1865819899ef19173be0f861783a2631d0a Diff: https://reviews.apache.org/r/49048/diff/ Testing (updated) --- ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh *** OK (All tests passed) *** mesos-master start/running, process 26868 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real19m46.324s user0m1.496s sys 0m0.774s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
> On June 27, 2016, 11:31 a.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/client/cli/context.py, lines 129-130 > > <https://reviews.apache.org/r/49048/diff/3/?file=1431153#file1431153line129> > > > > Can you simplify this as: > > ``` > > return not to_bool(tier.settings['preemptible']) and not > > to_bool(tier.settings['revocable']) > > ``` > > > > and falback to `tier_configurations.defaultTierName` in the next()? > > > > Also, I'd change the function name to something less generic, like > > `production_tier_filter`. > > Mehrdad Nurolahzade wrote: > Do you mean something like the following? (this is breaking my tests) > ``` > def production_tier_filter(tier): > return not to_bool(tier.settings['preemptible']) and not > to_bool(tier.settings['revocable']) > > task = config.job().taskConfig > if task.tier is None: > backfill_args = { > 'tier': String( > next( > (t.name for t in tier_configurations.tiers if > production_tier_filter(t)), > tier_configurations.defaultTierName)) > } > else: > backfill_args = { > 'production': Boolean( > next( > (not to_bool(t.settings['preemptible']) for t in > tier_configurations.tiers if > production_tier_filter(t)), > task.production)) > } > ``` > > Maxim Khutornenko wrote: > I presume this is due to the "else" block not accounting for the actual > `task.tier` value? Yes, and the if block also relies on ```task.production``` value. I am not sure if I am following your suggested simplification of ```production_tier_filter``` and using it in both if and else blocks: ``` def production_tier_filter(tier): return not to_bool(tier.settings['preemptible']) and not to_bool(tier.settings['revocable']) ``` - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review139597 --- On June 27, 2016, 9:02 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated June 27, 2016, 9:02 a.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 > src/main/python/apache/aurora/client/config.py > 2fc12559016d406c347adb416a5166cca31c961e > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_context.py > 204ca092adad8bf43c5032a02f61bf303fb0b2fc > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > --- > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > *** OK (All tests passed) *** > > mesos-master start/running, process 26868 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 19m46.324s > user 0m1.496s > sys 0m0.774s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
> On June 27, 2016, 11:31 a.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/client/config.py, line 118 > > <https://reviews.apache.org/r/49048/diff/3/?file=1431154#file1431154line118> > > > > Please, add a link to our docs for more info: > > http://aurora.apache.org/documentation/latest/reference/configuration/#job-objects > > > > Also, release notes need to be updated to clearly state this > > deprecation route. We added an entry to ```RELEASE-NOTES.md``` for 0.14.0 under "Deprecations and removals" with a previous JIRA, do we need anything beyond that? ``` - Deprecated `production` field in `TaskConfig` thrift struct. Use `tier` field to specify task scheduling and resource handling behavior. ``` - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review139597 ------- On June 27, 2016, 9:02 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated June 27, 2016, 9:02 a.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 > src/main/python/apache/aurora/client/config.py > 2fc12559016d406c347adb416a5166cca31c961e > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_context.py > 204ca092adad8bf43c5032a02f61bf303fb0b2fc > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > --- > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > *** OK (All tests passed) *** > > mesos-master start/running, process 26868 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 19m46.324s > user 0m1.496s > sys 0m0.774s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
> On June 27, 2016, 11:31 a.m., Maxim Khutornenko wrote: > > src/main/python/apache/aurora/client/cli/context.py, lines 129-130 > > <https://reviews.apache.org/r/49048/diff/3/?file=1431153#file1431153line129> > > > > Can you simplify this as: > > ``` > > return not to_bool(tier.settings['preemptible']) and not > > to_bool(tier.settings['revocable']) > > ``` > > > > and falback to `tier_configurations.defaultTierName` in the next()? > > > > Also, I'd change the function name to something less generic, like > > `production_tier_filter`. Do you mean something like the following? (this is breaking my tests) ``` def production_tier_filter(tier): return not to_bool(tier.settings['preemptible']) and not to_bool(tier.settings['revocable']) task = config.job().taskConfig if task.tier is None: backfill_args = { 'tier': String( next( (t.name for t in tier_configurations.tiers if production_tier_filter(t)), tier_configurations.defaultTierName)) } else: backfill_args = { 'production': Boolean( next( (not to_bool(t.settings['preemptible']) for t in tier_configurations.tiers if production_tier_filter(t)), task.production)) } ``` - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review139597 ------- On June 27, 2016, 9:02 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated June 27, 2016, 9:02 a.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 > src/main/python/apache/aurora/client/config.py > 2fc12559016d406c347adb416a5166cca31c961e > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_context.py > 204ca092adad8bf43c5032a02f61bf303fb0b2fc > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > --- > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > *** OK (All tests passed) *** > > mesos-master start/running, process 26868 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 19m46.324s > user 0m1.496s > sys 0m0.774s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 47550: AURORA-1492 Improve "aurora update start" command output
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47550/ --- (Updated May 23, 2016, 9:38 p.m.) Review request for Aurora. Changes --- (1) Refactored test_supdate.py and test_diff.py to use mock.patch() (2) Cleeaned up unused test fixtures in test_diff_formatter.py Repository: aurora Description --- AURORA-1492 Improve "aurora update start" command output Diffs (updated) - src/main/python/apache/aurora/client/cli/diff_formatter.py PRE-CREATION src/main/python/apache/aurora/client/cli/jobs.py e8bc38aaff42579419130b116389d9b9e09122a9 src/main/python/apache/aurora/client/cli/update.py eb7e9b0799bb25af652a0d5bde231cfca5bc1510 src/test/python/apache/aurora/client/cli/test_diff.py b9e91cf3be6ebd6d99e588ba2a2df304e42fd832 src/test/python/apache/aurora/client/cli/test_diff_formatter.py PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 2135ca988ce2f260262145a2168849acd8d14874 Diff: https://reviews.apache.org/r/47550/diff/ Testing --- Automated: ./pants test.pytest --no-fast src/test/python:: Manual: using vagrant e2e: src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Style: ./build-support/hooks/pre-commit Example output: aurora update start devcluster/www-data/prod/hello ./aurora/examples/jobs/updated_hello_world.aurora INFO] Starting update for: hello Job update has started. View your update progress at http://aurora.local:8081/scheduler/www-data/prod/hello/update/df5ba4d7-6a32-42ca-8453-1b172f61a23b This job update will: add instances: [2-3] update instances: [0-1] Thanks, Mehrdad Nurolahzade
Review Request 47550: AURORA-1492 Improve "aurora update start" command output
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47550/ --- Review request for Aurora. Repository: aurora Description --- AURORA-1492 Improve "aurora update start" command output Diffs - src/main/python/apache/aurora/client/cli/update.py eb7e9b0799bb25af652a0d5bde231cfca5bc1510 src/test/python/apache/aurora/client/cli/test_supdate.py 2135ca988ce2f260262145a2168849acd8d14874 Diff: https://reviews.apache.org/r/47550/diff/ Testing --- Automated: ./pants test.pytest --no-fast src/test/python:: Manual: using vagrant e2e: src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Style: ./build-support/hooks/pre-commit Thanks, Mehrdad Nurolahzade
Re: Review Request 47550: AURORA-1492 Improve "aurora update start" command output
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47550/#review133807 --- src/main/python/apache/aurora/client/cli/update.py (line 162) <https://reviews.apache.org/r/47550/#comment198434> Copy & pasted from DiffCommand in jobs.py Any suggestions for how to avoid duplicating this code snippet (and the next one) across the two commands? src/main/python/apache/aurora/client/cli/update.py (line 203) <https://reviews.apache.org/r/47550/#comment198436> Copy & pasted from DiffCommand.execute() in jobs.py src/main/python/apache/aurora/client/cli/update.py (line 205) <https://reviews.apache.org/r/47550/#comment198437> What should we do if the api.job_update_diff() call fails? - Mehrdad Nurolahzade On May 18, 2016, 8:55 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47550/ > --- > > (Updated May 18, 2016, 8:55 p.m.) > > > Review request for Aurora. > > > Repository: aurora > > > Description > --- > > AURORA-1492 Improve "aurora update start" command output > > > Diffs > - > > src/main/python/apache/aurora/client/cli/update.py > eb7e9b0799bb25af652a0d5bde231cfca5bc1510 > src/test/python/apache/aurora/client/cli/test_supdate.py > 2135ca988ce2f260262145a2168849acd8d14874 > > Diff: https://reviews.apache.org/r/47550/diff/ > > > Testing > --- > > Automated: ./pants test.pytest --no-fast src/test/python:: > Manual: using vagrant > e2e: src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > Style: ./build-support/hooks/pre-commit > > Example output: > > aurora update start devcluster/www-data/prod/hello > ./aurora/examples/jobs/updated_hello_world.aurora > INFO] Starting update for: hello > Job update has started. View your update progress at > http://aurora.local:8081/scheduler/www-data/prod/hello/update/df5ba4d7-6a32-42ca-8453-1b172f61a23b > This job update will: > add instances: [2-3] > update instances: [0-1] > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 47550: AURORA-1492 Improve "aurora update start" command output
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47550/ --- (Updated May 18, 2016, 8:55 p.m.) Review request for Aurora. Repository: aurora Description --- AURORA-1492 Improve "aurora update start" command output Diffs - src/main/python/apache/aurora/client/cli/update.py eb7e9b0799bb25af652a0d5bde231cfca5bc1510 src/test/python/apache/aurora/client/cli/test_supdate.py 2135ca988ce2f260262145a2168849acd8d14874 Diff: https://reviews.apache.org/r/47550/diff/ Testing (updated) --- Automated: ./pants test.pytest --no-fast src/test/python:: Manual: using vagrant e2e: src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Style: ./build-support/hooks/pre-commit Example output: aurora update start devcluster/www-data/prod/hello ./aurora/examples/jobs/updated_hello_world.aurora INFO] Starting update for: hello Job update has started. View your update progress at http://aurora.local:8081/scheduler/www-data/prod/hello/update/df5ba4d7-6a32-42ca-8453-1b172f61a23b This job update will: add instances: [2-3] update instances: [0-1] Thanks, Mehrdad Nurolahzade
Re: Review Request 47550: AURORA-1492 Improve "aurora update start" command output
> On May 19, 2016, 4:31 p.m., David McLaughlin wrote: > > src/test/python/apache/aurora/client/cli/test_supdate.py, lines 223-226 > > <https://reviews.apache.org/r/47550/diff/1/?file=1387328#file1387328line223> > > > > Can you add tests to show what happens when the instance ranges will be > > empty? Test scenario test_show_job_update_diff_no_change in test_diff_formatter.py has been added to cover this. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47550/#review133966 ------- On May 23, 2016, 9:38 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47550/ > --- > > (Updated May 23, 2016, 9:38 p.m.) > > > Review request for Aurora. > > > Repository: aurora > > > Description > --- > > AURORA-1492 Improve "aurora update start" command output > > > Diffs > - > > src/main/python/apache/aurora/client/cli/diff_formatter.py PRE-CREATION > src/main/python/apache/aurora/client/cli/jobs.py > e8bc38aaff42579419130b116389d9b9e09122a9 > src/main/python/apache/aurora/client/cli/update.py > eb7e9b0799bb25af652a0d5bde231cfca5bc1510 > src/test/python/apache/aurora/client/cli/test_diff.py > b9e91cf3be6ebd6d99e588ba2a2df304e42fd832 > src/test/python/apache/aurora/client/cli/test_diff_formatter.py > PRE-CREATION > src/test/python/apache/aurora/client/cli/test_supdate.py > 2135ca988ce2f260262145a2168849acd8d14874 > > Diff: https://reviews.apache.org/r/47550/diff/ > > > Testing > --- > > Automated: ./pants test.pytest --no-fast src/test/python:: > Manual: using vagrant > e2e: src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > Style: ./build-support/hooks/pre-commit > > Example output: > > aurora update start devcluster/www-data/prod/hello > ./aurora/examples/jobs/updated_hello_world.aurora > INFO] Starting update for: hello > Job update has started. View your update progress at > http://aurora.local:8081/scheduler/www-data/prod/hello/update/df5ba4d7-6a32-42ca-8453-1b172f61a23b > This job update will: > add instances: [2-3] > update instances: [0-1] > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 47550: AURORA-1492 Improve "aurora update start" command output
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47550/ --- (Updated May 20, 2016, 10:24 p.m.) Review request for Aurora. Changes --- (1) Added test scenario asked by David McLaughlin (2) Fixed test scenario names (3) Removed unnecessary test fixtures Repository: aurora Description --- AURORA-1492 Improve "aurora update start" command output Diffs (updated) - src/main/python/apache/aurora/client/cli/diff_formatter.py PRE-CREATION src/main/python/apache/aurora/client/cli/jobs.py e8bc38aaff42579419130b116389d9b9e09122a9 src/main/python/apache/aurora/client/cli/update.py eb7e9b0799bb25af652a0d5bde231cfca5bc1510 src/test/python/apache/aurora/client/cli/test_diff.py b9e91cf3be6ebd6d99e588ba2a2df304e42fd832 src/test/python/apache/aurora/client/cli/test_diff_formatter.py PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 2135ca988ce2f260262145a2168849acd8d14874 Diff: https://reviews.apache.org/r/47550/diff/ Testing --- Automated: ./pants test.pytest --no-fast src/test/python:: Manual: using vagrant e2e: src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Style: ./build-support/hooks/pre-commit Example output: aurora update start devcluster/www-data/prod/hello ./aurora/examples/jobs/updated_hello_world.aurora INFO] Starting update for: hello Job update has started. View your update progress at http://aurora.local:8081/scheduler/www-data/prod/hello/update/df5ba4d7-6a32-42ca-8453-1b172f61a23b This job update will: add instances: [2-3] update instances: [0-1] Thanks, Mehrdad Nurolahzade
Re: Review Request 47550: AURORA-1492 Improve "aurora update start" command output
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47550/ --- (Updated May 20, 2016, 10:04 p.m.) Review request for Aurora. Repository: aurora Description --- AURORA-1492 Improve "aurora update start" command output Diffs (updated) - src/main/python/apache/aurora/client/cli/diff_formatter.py PRE-CREATION src/main/python/apache/aurora/client/cli/jobs.py e8bc38aaff42579419130b116389d9b9e09122a9 src/main/python/apache/aurora/client/cli/update.py eb7e9b0799bb25af652a0d5bde231cfca5bc1510 src/test/python/apache/aurora/client/cli/test_diff.py b9e91cf3be6ebd6d99e588ba2a2df304e42fd832 src/test/python/apache/aurora/client/cli/test_diff_formatter.py PRE-CREATION src/test/python/apache/aurora/client/cli/test_supdate.py 2135ca988ce2f260262145a2168849acd8d14874 Diff: https://reviews.apache.org/r/47550/diff/ Testing --- Automated: ./pants test.pytest --no-fast src/test/python:: Manual: using vagrant e2e: src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Style: ./build-support/hooks/pre-commit Example output: aurora update start devcluster/www-data/prod/hello ./aurora/examples/jobs/updated_hello_world.aurora INFO] Starting update for: hello Job update has started. View your update progress at http://aurora.local:8081/scheduler/www-data/prod/hello/update/df5ba4d7-6a32-42ca-8453-1b172f61a23b This job update will: add instances: [2-3] update instances: [0-1] Thanks, Mehrdad Nurolahzade
Re: Review Request 47550: AURORA-1492 Improve "aurora update start" command output
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47550/#review134018 --- src/main/python/apache/aurora/client/cli/diff_formatter.py (line 145) <https://reviews.apache.org/r/47550/#comment198676> This is gonna break for update command (which does not supply local_tasks), so it needs to be something like: self.diff_no_update_details([] if local_tasks is None else local_tasks) - Mehrdad Nurolahzade On May 19, 2016, 7:28 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/47550/ > --- > > (Updated May 19, 2016, 7:28 p.m.) > > > Review request for Aurora. > > > Repository: aurora > > > Description > --- > > AURORA-1492 Improve "aurora update start" command output > > > Diffs > - > > src/main/python/apache/aurora/client/cli/diff_formatter.py PRE-CREATION > src/main/python/apache/aurora/client/cli/jobs.py > e8bc38aaff42579419130b116389d9b9e09122a9 > src/main/python/apache/aurora/client/cli/update.py > eb7e9b0799bb25af652a0d5bde231cfca5bc1510 > src/test/python/apache/aurora/client/cli/test_supdate.py > 2135ca988ce2f260262145a2168849acd8d14874 > > Diff: https://reviews.apache.org/r/47550/diff/ > > > Testing > --- > > Automated: ./pants test.pytest --no-fast src/test/python:: > Manual: using vagrant > e2e: src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > Style: ./build-support/hooks/pre-commit > > Example output: > > aurora update start devcluster/www-data/prod/hello > ./aurora/examples/jobs/updated_hello_world.aurora > INFO] Starting update for: hello > Job update has started. View your update progress at > http://aurora.local:8081/scheduler/www-data/prod/hello/update/df5ba4d7-6a32-42ca-8453-1b172f61a23b > This job update will: > add instances: [2-3] > update instances: [0-1] > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 47550: AURORA-1492 Improve "aurora update start" command output
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/47550/ --- (Updated May 19, 2016, 7:28 p.m.) Review request for Aurora. Changes --- (1) Refactored duplicated code in updates.py and jobs.py to diff_formatter.py (2) The diff messages are now displayed before the api.start_job_update() call is made (3) Fixed broken tests and added a couple of new scenarios Repository: aurora Description --- AURORA-1492 Improve "aurora update start" command output Diffs (updated) - src/main/python/apache/aurora/client/cli/diff_formatter.py PRE-CREATION src/main/python/apache/aurora/client/cli/jobs.py e8bc38aaff42579419130b116389d9b9e09122a9 src/main/python/apache/aurora/client/cli/update.py eb7e9b0799bb25af652a0d5bde231cfca5bc1510 src/test/python/apache/aurora/client/cli/test_supdate.py 2135ca988ce2f260262145a2168849acd8d14874 Diff: https://reviews.apache.org/r/47550/diff/ Testing --- Automated: ./pants test.pytest --no-fast src/test/python:: Manual: using vagrant e2e: src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Style: ./build-support/hooks/pre-commit Example output: aurora update start devcluster/www-data/prod/hello ./aurora/examples/jobs/updated_hello_world.aurora INFO] Starting update for: hello Job update has started. View your update progress at http://aurora.local:8081/scheduler/www-data/prod/hello/update/df5ba4d7-6a32-42ca-8453-1b172f61a23b This job update will: add instances: [2-3] update instances: [0-1] Thanks, Mehrdad Nurolahzade
Review Request 50432: AURORA-1741 Fix pystachio binding bug introduced by AURORA-1710
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50432/ --- Review request for Aurora, David McLaughlin and Maxim Khutornenko. Bugs: AURORA-1741 https://issues.apache.org/jira/browse/AURORA-1741 Repository: aurora Description --- AURORA-1741 Fix pystachio binding bug introduced by AURORA-1710 Diffs - src/main/python/apache/aurora/client/config.py 96cd9ddf77cac449d68537cd652bca03ef87d75d Diff: https://reviews.apache.org/r/50432/diff/ Testing --- ``` ./build-support/jenkins/build.sh ``` Thanks, Mehrdad Nurolahzade
Review Request 50052: AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50052/ --- Review request for Aurora, Joshua Cohen and Stephan Erb. Bugs: AURORA-1736 https://issues.apache.org/jira/browse/AURORA-1736 Repository: aurora Description --- AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint Diffs - src/main/java/org/apache/aurora/scheduler/http/Offers.java 80f082410896a50d86c7886736caf79581f5051c Diff: https://reviews.apache.org/r/50052/diff/ Testing --- Manual, Jenkins, and end_to_end Thanks, Mehrdad Nurolahzade
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/ --- (Updated July 25, 2016, 8:30 a.m.) Review request for Aurora, Joshua Cohen and Stephan Erb. Changes --- - Deprecation note in release notes - Rebased Bugs: AURORA-1710 https://issues.apache.org/jira/browse/AURORA-1710 Repository: aurora Description --- AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes Diffs (updated) - RELEASE-NOTES.md ca98d7ab802114e025f6021d33bc251704c59088 src/main/python/apache/aurora/client/api/__init__.py 68baf8fdb90cd26100159401c46c9963c24332b3 src/main/python/apache/aurora/client/cli/context.py 9b1511801d031ff48b81c25688a55cb586b8ac66 src/main/python/apache/aurora/client/config.py 2fc12559016d406c347adb416a5166cca31c961e src/test/python/apache/aurora/client/cli/test_command_hooks.py 2130f1fa71be02a004cdf8e476a270c81a7105d3 src/test/python/apache/aurora/client/cli/test_context.py 204ca092adad8bf43c5032a02f61bf303fb0b2fc src/test/python/apache/aurora/client/cli/test_create.py 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c src/test/python/apache/aurora/client/cli/test_cron.py f3c522ed94a2d774865811ceb546bf9df083c14f src/test/python/apache/aurora/client/cli/test_plugins.py a545fece5e2b3e0017a61e1be9ac478372b1f34d src/test/python/apache/aurora/client/cli/test_restart.py 967d560e5c7eb0ed85b215fb11d9751b8666acb5 src/test/python/apache/aurora/client/cli/util.py 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e src/test/python/apache/aurora/client/test_config.py b1a3c1865819899ef19173be0f861783a2631d0a Diff: https://reviews.apache.org/r/49048/diff/ Testing --- ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh *** OK (All tests passed) *** mesos-master start/running, process 26868 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real19m46.324s user0m1.496s sys 0m0.774s ``` Thanks, Mehrdad Nurolahzade
Review Request 50530: AURORA-1656 Document tier concept
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50530/ --- Review request for Aurora, Maxim Khutornenko and Stephan Erb. Bugs: AURORA-1656 https://issues.apache.org/jira/browse/AURORA-1656 Repository: aurora Description --- AURORA-1656 Document tier concept Diffs - docs/features/multitenancy.md 62bcd535d3bf39c9ea7e6d5958f6ae8ba0867c0d docs/reference/configuration.md 64c076d862453545652fbaa2d3e29f284ddd164d Diff: https://reviews.apache.org/r/50530/diff/ Testing --- Thanks, Mehrdad Nurolahzade
Re: Review Request 50432: AURORA-1741 Fix pystachio binding bug introduced by AURORA-1710
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50432/ --- (Updated July 29, 2016, 2 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Changes --- Added unbound moustache bindings to test cases Bugs: AURORA-1741 https://issues.apache.org/jira/browse/AURORA-1741 Repository: aurora Description --- AURORA-1741 Fix pystachio binding bug introduced by AURORA-1710 Diffs (updated) - src/test/python/apache/aurora/client/test_config.py 4742fa28e3156e5b20791b80f2db8392f7f2f4bf Diff: https://reviews.apache.org/r/50432/diff/ Testing --- ``` ./build-support/jenkins/build.sh ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 50432: AURORA-1741 Fix pystachio binding bug introduced by AURORA-1710
> On July 29, 2016, 2:03 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/client/config.py, line 138 > > <https://reviews.apache.org/r/50432/diff/1/?file=1452163#file1452163line138> > > > > Isn't this undoing the bug fix to compare w/ `Empty` instead of `None`? You are right, not sure why this is happening. But that's not part of my new change-set, should be review board mix up! - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50432/#review144191 --- On July 29, 2016, 2 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50432/ > --- > > (Updated July 29, 2016, 2 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Bugs: AURORA-1741 > https://issues.apache.org/jira/browse/AURORA-1741 > > > Repository: aurora > > > Description > --- > > AURORA-1741 Fix pystachio binding bug introduced by AURORA-1710 > > > Diffs > - > > src/test/python/apache/aurora/client/test_config.py > 4742fa28e3156e5b20791b80f2db8392f7f2f4bf > > Diff: https://reviews.apache.org/r/50432/diff/ > > > Testing > --- > > ``` > ./build-support/jenkins/build.sh > > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50530: AURORA-1656 Document tier concept
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50530/#review144167 --- Can we git this merged? - Mehrdad Nurolahzade On July 27, 2016, 3:41 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50530/ > --- > > (Updated July 27, 2016, 3:41 p.m.) > > > Review request for Aurora, Maxim Khutornenko and Stephan Erb. > > > Bugs: AURORA-1656 > https://issues.apache.org/jira/browse/AURORA-1656 > > > Repository: aurora > > > Description > --- > > AURORA-1656 Document tier concept > > > Diffs > - > > docs/features/multitenancy.md 62bcd535d3bf39c9ea7e6d5958f6ae8ba0867c0d > docs/reference/configuration.md 64c076d862453545652fbaa2d3e29f284ddd164d > > Diff: https://reviews.apache.org/r/50530/diff/ > > > Testing > --- > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50432: AURORA-1741 Fix pystachio binding bug introduced by AURORA-1710
> On July 29, 2016, 2:03 p.m., Joshua Cohen wrote: > > src/main/python/apache/aurora/client/config.py, line 138 > > <https://reviews.apache.org/r/50432/diff/1/?file=1452163#file1452163line138> > > > > Isn't this undoing the bug fix to compare w/ `Empty` instead of `None`? > > Mehrdad Nurolahzade wrote: > You are right, not sure why this is happening. > > But that's not part of my new change-set, should be review board mix up! I'll create a new RB. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50432/#review144191 ------- On July 29, 2016, 2 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50432/ > --- > > (Updated July 29, 2016, 2 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Bugs: AURORA-1741 > https://issues.apache.org/jira/browse/AURORA-1741 > > > Repository: aurora > > > Description > --- > > AURORA-1741 Fix pystachio binding bug introduced by AURORA-1710 > > > Diffs > - > > src/test/python/apache/aurora/client/test_config.py > 4742fa28e3156e5b20791b80f2db8392f7f2f4bf > > Diff: https://reviews.apache.org/r/50432/diff/ > > > Testing > --- > > ``` > ./build-support/jenkins/build.sh > > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50617: AURORA-1741 Added missing test cases
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50617/#review144212 --- Props to Joshua for finding a problem with these test scenarios. I'll sibmit a new version shortly. - Mehrdad Nurolahzade On July 29, 2016, 2:21 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50617/ > --- > > (Updated July 29, 2016, 2:21 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Bugs: AURORA-1741 > https://issues.apache.org/jira/browse/AURORA-1741 > > > Repository: aurora > > > Description > --- > > AURORA-1741 Added missing test cases > > > Diffs > - > > src/test/python/apache/aurora/client/test_config.py > 4742fa28e3156e5b20791b80f2db8392f7f2f4bf > > Diff: https://reviews.apache.org/r/50617/diff/ > > > Testing > --- > > `./pants test src/test/python/apache/aurora/client/cli:cli` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50617: AURORA-1741 Added missing test cases
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50617/ --- (Updated July 29, 2016, 3:46 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Changes --- Sorry, forgot to commit :) Bugs: AURORA-1741 https://issues.apache.org/jira/browse/AURORA-1741 Repository: aurora Description --- AURORA-1741 Added missing test cases Diffs (updated) - src/test/python/apache/aurora/client/test_config.py 4742fa28e3156e5b20791b80f2db8392f7f2f4bf Diff: https://reviews.apache.org/r/50617/diff/ Testing --- `./pants test src/test/python/apache/aurora/client/cli:cli` Thanks, Mehrdad Nurolahzade
Re: Review Request 50617: AURORA-1741 Added missing test cases
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50617/ --- (Updated July 29, 2016, 3:44 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Changes --- Now using a truely unbound mustache like `{{_unbound_}}` instead of `{{thermos.ports[http]}}` that would not throw `ThermosTaskValidator.assert_all_refs_bound()` off on a `config.job()` call. Bugs: AURORA-1741 https://issues.apache.org/jira/browse/AURORA-1741 Repository: aurora Description --- AURORA-1741 Added missing test cases Diffs (updated) - src/test/python/apache/aurora/client/test_config.py 4742fa28e3156e5b20791b80f2db8392f7f2f4bf Diff: https://reviews.apache.org/r/50617/diff/ Testing --- `./pants test src/test/python/apache/aurora/client/cli:cli` Thanks, Mehrdad Nurolahzade
Review Request 50617: AURORA-1741 Added missing test cases
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50617/ --- Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1741 https://issues.apache.org/jira/browse/AURORA-1741 Repository: aurora Description --- AURORA-1741 Added missing test cases Diffs - src/test/python/apache/aurora/client/test_config.py 4742fa28e3156e5b20791b80f2db8392f7f2f4bf Diff: https://reviews.apache.org/r/50617/diff/ Testing --- `./pants test src/test/python/apache/aurora/client/cli:cli` Thanks, Mehrdad Nurolahzade
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/ --- (Updated July 25, 2016, 9:25 a.m.) Review request for Aurora, Joshua Cohen and Stephan Erb. Changes --- Reworded deprecation note in release notes Bugs: AURORA-1710 https://issues.apache.org/jira/browse/AURORA-1710 Repository: aurora Description --- AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes Diffs (updated) - RELEASE-NOTES.md ca98d7ab802114e025f6021d33bc251704c59088 src/main/python/apache/aurora/client/api/__init__.py 68baf8fdb90cd26100159401c46c9963c24332b3 src/main/python/apache/aurora/client/cli/context.py 9b1511801d031ff48b81c25688a55cb586b8ac66 src/main/python/apache/aurora/client/config.py 2fc12559016d406c347adb416a5166cca31c961e src/test/python/apache/aurora/client/cli/test_command_hooks.py 2130f1fa71be02a004cdf8e476a270c81a7105d3 src/test/python/apache/aurora/client/cli/test_context.py 204ca092adad8bf43c5032a02f61bf303fb0b2fc src/test/python/apache/aurora/client/cli/test_create.py 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c src/test/python/apache/aurora/client/cli/test_cron.py f3c522ed94a2d774865811ceb546bf9df083c14f src/test/python/apache/aurora/client/cli/test_plugins.py a545fece5e2b3e0017a61e1be9ac478372b1f34d src/test/python/apache/aurora/client/cli/test_restart.py 967d560e5c7eb0ed85b215fb11d9751b8666acb5 src/test/python/apache/aurora/client/cli/util.py 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e src/test/python/apache/aurora/client/test_config.py b1a3c1865819899ef19173be0f861783a2631d0a Diff: https://reviews.apache.org/r/49048/diff/ Testing --- ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh *** OK (All tests passed) *** mesos-master start/running, process 26868 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real19m46.324s user0m1.496s sys 0m0.774s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
> On July 25, 2016, 8:55 a.m., Joshua Cohen wrote: > > RELEASE-NOTES.md, line 20 > > <https://reviews.apache.org/r/49048/diff/7-8/?file=1440195#file1440195line20> > > > > Given that tier names are configurable by Aurora operators, should we > > word this differently? Better? The job configuration flag `production` is now deprecated. To achieve the same scheduling behavior that `production=true` used to provide, users should elect a `tier` for the job with attributes `preemptible=false` and `revocable=false`. For example, the `preferred` tier in the default tier configuation file (`tiers.json`) matches the above criteria. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review143387 --- On July 25, 2016, 8:30 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated July 25, 2016, 8:30 a.m.) > > > Review request for Aurora, Joshua Cohen and Stephan Erb. > > > Bugs: AURORA-1710 > https://issues.apache.org/jira/browse/AURORA-1710 > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > RELEASE-NOTES.md ca98d7ab802114e025f6021d33bc251704c59088 > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 > src/main/python/apache/aurora/client/config.py > 2fc12559016d406c347adb416a5166cca31c961e > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_context.py > 204ca092adad8bf43c5032a02f61bf303fb0b2fc > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > --- > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > *** OK (All tests passed) *** > > mesos-master start/running, process 26868 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 19m46.324s > user 0m1.496s > sys 0m0.774s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50432: AURORA-1741 Fix pystachio binding bug introduced by AURORA-1710
> On July 26, 2016, 7:36 a.m., Joshua Cohen wrote: > > I know this is already landed, but it would be nice to follow this up with > > a test case if possible? > > Mehrdad Nurolahzade wrote: > The logic has two test cases: > (https://github.com/apache/aurora/blob/380307ac1878cb71bc86d2ccd7887192161254cf/src/test/python/apache/aurora/client/test_config.py#L228-L243). > > Joshua Cohen wrote: > Yes, but they weren't failing though? It seems they were not sufficient > to catch this problem. Alright, I'll add a test case with moustache variables (to prove that it works now with bindings). - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50432/#review143525 --- On July 25, 2016, 7:36 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50432/ > --- > > (Updated July 25, 2016, 7:36 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Bugs: AURORA-1741 > https://issues.apache.org/jira/browse/AURORA-1741 > > > Repository: aurora > > > Description > --- > > AURORA-1741 Fix pystachio binding bug introduced by AURORA-1710 > > > Diffs > - > > src/main/python/apache/aurora/client/config.py > 96cd9ddf77cac449d68537cd652bca03ef87d75d > > Diff: https://reviews.apache.org/r/50432/diff/ > > > Testing > --- > > ``` > ./build-support/jenkins/build.sh > > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50530: AURORA-1656 Document tier concept
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50530/ --- (Updated July 27, 2016, 3:41 p.m.) Review request for Aurora, Maxim Khutornenko and Stephan Erb. Changes --- Marked `production` as deprecated in `Job` object reference table Bugs: AURORA-1656 https://issues.apache.org/jira/browse/AURORA-1656 Repository: aurora Description --- AURORA-1656 Document tier concept Diffs (updated) - docs/features/multitenancy.md 62bcd535d3bf39c9ea7e6d5958f6ae8ba0867c0d docs/reference/configuration.md 64c076d862453545652fbaa2d3e29f284ddd164d Diff: https://reviews.apache.org/r/50530/diff/ Testing --- Thanks, Mehrdad Nurolahzade
Re: Review Request 50902: AURORA-1656 Fix broken links in tier documentation
> On Aug. 10, 2016, 10:36 a.m., Maxim Khutornenko wrote: > > The easiest way to validate doc changes like this is to push your branch to > > your github remote and post a link here for review. Care to give it a try? Will do. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50902/#review145364 --- On Aug. 8, 2016, 10:13 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50902/ > --- > > (Updated Aug. 8, 2016, 10:13 a.m.) > > > Review request for Aurora and Maxim Khutornenko. > > > Bugs: AURORA-1656 > https://issues.apache.org/jira/browse/AURORA-1656 > > > Repository: aurora > > > Description > --- > > Just realized that tier documentation (that I recently touched for > AURORA-1656) has broken links that I had not notice. > > > Diffs > - > > docs/features/multitenancy.md 417c433665bf9e32416bde426078883931783c1c > docs/reference/configuration.md 16b31be5bf3021eb2f646e7a4361d5b614bd1164 > > Diff: https://reviews.apache.org/r/50902/diff/ > > > Testing > --- > > Manual testing > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50902: AURORA-1656 Fix broken links in tier documentation
> On Aug. 10, 2016, 10:36 a.m., Maxim Khutornenko wrote: > > The easiest way to validate doc changes like this is to push your branch to > > your github remote and post a link here for review. Care to give it a try? > > Mehrdad Nurolahzade wrote: > Will do. Here is the github fork: https://github.com/nurolahzade/aurora - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50902/#review145364 --- On Aug. 8, 2016, 10:13 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50902/ > --- > > (Updated Aug. 8, 2016, 10:13 a.m.) > > > Review request for Aurora and Maxim Khutornenko. > > > Bugs: AURORA-1656 > https://issues.apache.org/jira/browse/AURORA-1656 > > > Repository: aurora > > > Description > --- > > Just realized that tier documentation (that I recently touched for > AURORA-1656) has broken links that I had not notice. > > > Diffs > - > > docs/features/multitenancy.md 417c433665bf9e32416bde426078883931783c1c > docs/reference/configuration.md 16b31be5bf3021eb2f646e7a4361d5b614bd1164 > > Diff: https://reviews.apache.org/r/50902/diff/ > > > Testing > --- > > Manual testing > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50052: AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint
> On July 17, 2016, 3:15 p.m., Stephan Erb wrote: > > Should we consider adopting `protobuf-java-util`'s JsonFormatter rather > > than implementing this on our own? > > > > * > > https://github.com/google/protobuf/blob/master/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java > > * > > http://mvnrepository.com/artifact/com.google.protobuf/protobuf-java-util/3.0.0-beta-3 Looking into protobuf util ... - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50052/#review142524 ------- On July 15, 2016, 1:15 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50052/ > --- > > (Updated July 15, 2016, 1:15 p.m.) > > > Review request for Aurora, Joshua Cohen and Stephan Erb. > > > Bugs: AURORA-1736 > https://issues.apache.org/jira/browse/AURORA-1736 > > > Repository: aurora > > > Description > --- > > AURORA-1736 Display reservations and persistent volumes in /offers debug http > endpoint > > > Diffs > - > > config/legacy_untested_classes.txt 1ea2183ab20cc5c6bca147bcea4e5c708d576b62 > src/main/java/org/apache/aurora/scheduler/http/Offers.java > 80f082410896a50d86c7886736caf79581f5051c > src/test/java/org/apache/aurora/scheduler/http/OffersTest.java PRE-CREATION > > Diff: https://reviews.apache.org/r/50052/diff/ > > > Testing > --- > > Manual, Jenkins, and end_to_end > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50052: AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint
> On July 17, 2016, 3:15 p.m., Stephan Erb wrote: > > Should we consider adopting `protobuf-java-util`'s JsonFormatter rather > > than implementing this on our own? > > > > * > > https://github.com/google/protobuf/blob/master/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java > > * > > http://mvnrepository.com/artifact/com.google.protobuf/protobuf-java-util/3.0.0-beta-3 > > Mehrdad Nurolahzade wrote: > Looking into protobuf util ... I can see that we already have [jackson-datatype-protobuf](https://github.com/HubSpot/jackson-datatype-protobuf) library in the project that can be used to create a JSON serializer from protobuf. We can probably get this task done using it. However, ```protobuf-java-util``` seems to have a larger and more active community. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50052/#review142524 ----------- On July 15, 2016, 1:15 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50052/ > --- > > (Updated July 15, 2016, 1:15 p.m.) > > > Review request for Aurora, Joshua Cohen and Stephan Erb. > > > Bugs: AURORA-1736 > https://issues.apache.org/jira/browse/AURORA-1736 > > > Repository: aurora > > > Description > --- > > AURORA-1736 Display reservations and persistent volumes in /offers debug http > endpoint > > > Diffs > - > > config/legacy_untested_classes.txt 1ea2183ab20cc5c6bca147bcea4e5c708d576b62 > src/main/java/org/apache/aurora/scheduler/http/Offers.java > 80f082410896a50d86c7886736caf79581f5051c > src/test/java/org/apache/aurora/scheduler/http/OffersTest.java PRE-CREATION > > Diff: https://reviews.apache.org/r/50052/diff/ > > > Testing > --- > > Manual, Jenkins, and end_to_end > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50052: AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint
> On July 17, 2016, 3:15 p.m., Stephan Erb wrote: > > Should we consider adopting `protobuf-java-util`'s JsonFormatter rather > > than implementing this on our own? > > > > * > > https://github.com/google/protobuf/blob/master/java/util/src/main/java/com/google/protobuf/util/JsonFormat.java > > * > > http://mvnrepository.com/artifact/com.google.protobuf/protobuf-java-util/3.0.0-beta-3 > > Mehrdad Nurolahzade wrote: > Looking into protobuf util ... > > Mehrdad Nurolahzade wrote: > I can see that we already have > [jackson-datatype-protobuf](https://github.com/HubSpot/jackson-datatype-protobuf) > library in the project that can be used to create a JSON serializer from > protobuf. We can probably get this task done using it. However, > ```protobuf-java-util``` seems to have a larger and more active community. I spent some time trying to get ```protobuf-java-util``` to work. However, it seems to a very light-weight utility designed to creat one-to-one mapping from protobuf to JSON. Controls to exclude or give special treatment to fields in the mapping are non-existant which makes the resulting generated JSON look significantly different from what we are serving today. Then I looked into ```jackson-datatype-protobuf``` and I found a great degree of flexibility in it to control generated JSON that I did not see in ```protobuf-java-util```. To generate JSON output: ``` @GET @Produces(MediaType.APPLICATION_JSON) public Response getOffers() throws JsonProcessingException { ObjectMapper mapper = new ObjectMapper() .registerModule(new ProtobufModule()) .setPropertyNamingStrategy(PropertyNamingStrategy.CAMEL_CASE_TO_LOWER_CASE_WITH_UNDERSCORES) .setSerializationInclusion(JsonInclude.Include.NON_NULL); return Response.ok(mapper.writeValueAsString(offerManager.getOffers())).build(); } ``` However, the resulting JSON output is different from what we used to serve (see the attachments). If this change is acceptable (and won't break anything else) I can use it as-is. Otherwise, I have to write code to customize the output (filter out certain fields, change rendering, etc.) which I feel somehow defeats the purpose of this refactoring. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50052/#review142524 ------- On July 15, 2016, 1:15 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50052/ > --- > > (Updated July 15, 2016, 1:15 p.m.) > > > Review request for Aurora, Joshua Cohen and Stephan Erb. > > > Bugs: AURORA-1736 > https://issues.apache.org/jira/browse/AURORA-1736 > > > Repository: aurora > > > Description > --- > > AURORA-1736 Display reservations and persistent volumes in /offers debug http > endpoint > > > Diffs > - > > config/legacy_untested_classes.txt 1ea2183ab20cc5c6bca147bcea4e5c708d576b62 > src/main/java/org/apache/aurora/scheduler/http/Offers.java > 80f082410896a50d86c7886736caf79581f5051c > src/test/java/org/apache/aurora/scheduler/http/OffersTest.java PRE-CREATION > > Diff: https://reviews.apache.org/r/50052/diff/ > > > Testing > --- > > Manual, Jenkins, and end_to_end > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50052: AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50052/ --- (Updated July 18, 2016, 12:32 p.m.) Review request for Aurora, Joshua Cohen and Stephan Erb. Bugs: AURORA-1736 https://issues.apache.org/jira/browse/AURORA-1736 Repository: aurora Description --- AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint Diffs - config/legacy_untested_classes.txt 1ea2183ab20cc5c6bca147bcea4e5c708d576b62 src/main/java/org/apache/aurora/scheduler/http/Offers.java 80f082410896a50d86c7886736caf79581f5051c src/test/java/org/apache/aurora/scheduler/http/OffersTest.java PRE-CREATION Diff: https://reviews.apache.org/r/50052/diff/ Testing --- Manual, Jenkins, and end_to_end File Attachments (updated) CURRENT https://reviews.apache.org/media/uploaded/files/2016/07/18/1de4c357-c932-4c84-962f-4209a5b679bc__offers-old.json MODIFIED https://reviews.apache.org/media/uploaded/files/2016/07/18/50faae5c-45ac-45af-aa3c-d68ca09d2e72__offers-new.json Thanks, Mehrdad Nurolahzade
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review142681 --- @ReviewBot retry - Mehrdad Nurolahzade On July 8, 2016, 4:15 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated July 8, 2016, 4:15 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Bugs: AURORA-1710 > https://issues.apache.org/jira/browse/AURORA-1710 > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > RELEASE-NOTES.md 29d224d5596eb060356f1343cf347db79b1ad687 > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 > src/main/python/apache/aurora/client/config.py > 2fc12559016d406c347adb416a5166cca31c961e > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_context.py > 204ca092adad8bf43c5032a02f61bf303fb0b2fc > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > --- > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > *** OK (All tests passed) *** > > mesos-master start/running, process 26868 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 19m46.324s > user 0m1.496s > sys 0m0.774s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50052: AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50052/ --- (Updated July 18, 2016, 7:12 p.m.) Review request for Aurora, Joshua Cohen and Stephan Erb. Bugs: AURORA-1736 https://issues.apache.org/jira/browse/AURORA-1736 Repository: aurora Description --- AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint Diffs - config/legacy_untested_classes.txt 1ea2183ab20cc5c6bca147bcea4e5c708d576b62 src/main/java/org/apache/aurora/scheduler/http/Offers.java 80f082410896a50d86c7886736caf79581f5051c src/test/java/org/apache/aurora/scheduler/http/OffersTest.java PRE-CREATION Diff: https://reviews.apache.org/r/50052/diff/ Testing --- Manual, Jenkins, and end_to_end File Attachments (updated) CURRENT https://reviews.apache.org/media/uploaded/files/2016/07/18/1de4c357-c932-4c84-962f-4209a5b679bc__offers-old.json NEW https://reviews.apache.org/media/uploaded/files/2016/07/19/799bcd1f-f9c8-4b6e-bbaa-ce8022b1dac1__offers-new.json Thanks, Mehrdad Nurolahzade
Re: Review Request 50052: AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50052/ --- (Updated July 18, 2016, 7:11 p.m.) Review request for Aurora, Joshua Cohen and Stephan Erb. Changes --- Rendering JSON using ```jackson-datatype-protobuf``` library Bugs: AURORA-1736 https://issues.apache.org/jira/browse/AURORA-1736 Repository: aurora Description --- AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint Diffs (updated) - config/legacy_untested_classes.txt 1ea2183ab20cc5c6bca147bcea4e5c708d576b62 src/main/java/org/apache/aurora/scheduler/http/Offers.java 80f082410896a50d86c7886736caf79581f5051c src/test/java/org/apache/aurora/scheduler/http/OffersTest.java PRE-CREATION Diff: https://reviews.apache.org/r/50052/diff/ Testing --- Manual, Jenkins, and end_to_end File Attachments CURRENT https://reviews.apache.org/media/uploaded/files/2016/07/18/1de4c357-c932-4c84-962f-4209a5b679bc__offers-old.json MODIFIED https://reviews.apache.org/media/uploaded/files/2016/07/18/50faae5c-45ac-45af-aa3c-d68ca09d2e72__offers-new.json Thanks, Mehrdad Nurolahzade
Re: Review Request 50052: AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint
> On July 19, 2016, 2:13 p.m., Dmitriy Shirchenko wrote: > > File Attachment: NEW - offers-new.json > > <https://reviews.apache.org/r/50052/#fcomment103> > > > > Why is there a double nested labels: labels array under reservation? Is > > this just a carry over from how Aurora gets offers from Mesos? > > > > So there is a key `labels` and then underneath is there's another key > > `labels` whose value is an array. Seems like one of them is not necessary. > > > > ```json > > reservation: { > > labels: { > > labels: [ > > { > > key: "job", > > value: "devcluster/www-data/prod/hello" > > } > > ] > > } > > } > > ``` This is a question we have to put to Mesos people, it's a weired design. I can confirm that it is not due to our rendering of their protobuf structures. To verify I played with their ```src/tests/reservation_endpoints_tests.cpp``` tests and dumped their ```ReservationInfo``` structures and noticed the same weired nest behavior. Also, if you try to set labels through supplied http endpoints ```/reserve``` you would notice the call would fail unless you supply nested labels: ``` curl -i \ -d slaveId="d091a635-33c8-4409-989f-9bef16e36f34-S0" \ -d resources='[ { "name": "disk", "type": "SCALAR", "scalar": { "value": 128 }, "role": "aurora-role", "reservation": { "labels": { "labels" : [{"key": "job", "value": "devcluster/www-data/prod/hello"}] } } } ]' \ -X POST http://192.168.33.7:5050/master/reserve ``` - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50052/#review142830 --- On July 18, 2016, 7:12 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50052/ > --- > > (Updated July 18, 2016, 7:12 p.m.) > > > Review request for Aurora, Joshua Cohen and Stephan Erb. > > > Bugs: AURORA-1736 > https://issues.apache.org/jira/browse/AURORA-1736 > > > Repository: aurora > > > Description > --- > > AURORA-1736 Display reservations and persistent volumes in /offers debug http > endpoint > > > Diffs > - > > config/legacy_untested_classes.txt 1ea2183ab20cc5c6bca147bcea4e5c708d576b62 > src/main/java/org/apache/aurora/scheduler/http/Offers.java > 80f082410896a50d86c7886736caf79581f5051c > src/test/java/org/apache/aurora/scheduler/http/OffersTest.java PRE-CREATION > > Diff: https://reviews.apache.org/r/50052/diff/ > > > Testing > --- > > Manual, Jenkins, and end_to_end > > > File Attachments > > > CURRENT > > https://reviews.apache.org/media/uploaded/files/2016/07/18/1de4c357-c932-4c84-962f-4209a5b679bc__offers-old.json > NEW > > https://reviews.apache.org/media/uploaded/files/2016/07/19/799bcd1f-f9c8-4b6e-bbaa-ce8022b1dac1__offers-new.json > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
> On July 19, 2016, 3:27 p.m., Stephan Erb wrote: > > src/main/python/apache/aurora/client/cli/context.py, line 143 > > <https://reviews.apache.org/r/49048/diff/7/?file=1440197#file1440197line143> > > > > Side stepping the `get_config` factory and re-creating the > > `AnnotatedAuroraConfig` does not seem optimal in my eyes. > > > > Have you considered using Aurora's binding helper concept instead? Idea > > would be: > > > > * change the default of tier from `None` to `{{aurora.default_tier}}` > > (or something similar) > > * register a binding helper > > https://github.com/apache/aurora/blob/528198ecbf4adde22988f6073b043e3da049486d/src/main/python/apache/aurora/client/binding_helper.py#L33 > > > > * let the helper automatically resolve the binding via a call to the > > scheduler whenever the user has not set a custom tier > > > > Backfilling of the production flag could be moved to the scheduler side. Thanks for pointing out binding helpers, did not know about them. The backfilling of production/tier attributes is already happening on the scheduler side. We did that as part of the same ticket and it has already been merged to master, see [rb 48559](https://reviews.apache.org/r/48559). To idea here is to have the backfill logic repeated on the client side so that if the new client is used against the old scheduler it would still correctly backfill tier and production settings. Perhaps we can throw away this client side backfilling logic in the future and allow it to be done entirely on the scheduler side. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review142828 --- On July 8, 2016, 4:15 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated July 8, 2016, 4:15 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Bugs: AURORA-1710 > https://issues.apache.org/jira/browse/AURORA-1710 > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > RELEASE-NOTES.md 29d224d5596eb060356f1343cf347db79b1ad687 > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 > src/main/python/apache/aurora/client/config.py > 2fc12559016d406c347adb416a5166cca31c961e > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_context.py > 204ca092adad8bf43c5032a02f61bf303fb0b2fc > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > --- > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > *** OK (All tests passed) *** > > mesos-master start/running, process 26868 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 19m46.324s > user 0m1.496s > sys 0m0.774s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
> On July 19, 2016, 3:27 p.m., Stephan Erb wrote: > > src/test/python/apache/aurora/client/cli/test_cron.py, line 111 > > <https://reviews.apache.org/r/49048/diff/7/?file=1440202#file1440202line111> > > > > As an example of many similar test changes: > > > > The additional mock calls are obscuring the original test intend. A > > simple workaround would be to already set a `tier` in the job. > > `get_tier_config` would then no longer need to be called. Your suggested workaround does not work. If you read through `_get_config_with_production_and_tier()` you would notice that the call on `api.get_tier_configs()` is made whether or not `tier` is already set. The idea is even when `tier` is set, we need to ensure that the selection of `tier` matches that of `production` flag. For example, `tier=preemptible` and `production=true` should result in `production` being revised to `false`. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review142828 ----------- On July 20, 2016, 10:56 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated July 20, 2016, 10:56 a.m.) > > > Review request for Aurora, Joshua Cohen and Stephan Erb. > > > Bugs: AURORA-1710 > https://issues.apache.org/jira/browse/AURORA-1710 > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > RELEASE-NOTES.md 29d224d5596eb060356f1343cf347db79b1ad687 > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 > src/main/python/apache/aurora/client/config.py > 2fc12559016d406c347adb416a5166cca31c961e > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_context.py > 204ca092adad8bf43c5032a02f61bf303fb0b2fc > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > --- > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > *** OK (All tests passed) *** > > mesos-master start/running, process 26868 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 19m46.324s > user 0m1.496s > sys 0m0.774s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
> On July 19, 2016, 3:27 p.m., Stephan Erb wrote: > > RELEASE-NOTES.md, line 13 > > <https://reviews.apache.org/r/49048/diff/7/?file=1440195#file1440195line13> > > > > Maybe add here explicitly that `production` is deprecated and that > > people should use `tier='preferred'` instead. > > > > Some people are only skimming the deprecation sections when updating, > > so they might miss it otherwise. This was added to release notes under `Deprecations and removals` section for 0.14.0 release when scheduler-side backfill work was done: ``` - Deprecated `production` field in `TaskConfig` thrift struct. Use `tier` field to specify task scheduling and resource handling behavior. ``` Would you still suggest we add a deprecation entry to 0.16.0 release note? > On July 19, 2016, 3:27 p.m., Stephan Erb wrote: > > src/main/python/apache/aurora/client/config.py, line 122 > > <https://reviews.apache.org/r/49048/diff/7/?file=1440198#file1440198line122> > > > > That comment reminds me we should have solid user facing docs. This > > includes: > > > > * a note that `production` is deprecated in favor of tier in > > `reference/configuration.md` > > * according update of `features/multitenancy.md#preemption` > > * a user facing description of the job tier. Most people have never > > heard of it, so we need to explain it to them. Probably a new subsection in > > `features/multitenancy.md` is a good place to do this. We can probably track documentation changes under [AURORA-1656](https://issues.apache.org/jira/browse/AURORA-1656), allowing this change-set to be limited to code changes only. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review142828 --- On July 8, 2016, 4:15 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated July 8, 2016, 4:15 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Bugs: AURORA-1710 > https://issues.apache.org/jira/browse/AURORA-1710 > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > RELEASE-NOTES.md 29d224d5596eb060356f1343cf347db79b1ad687 > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 > src/main/python/apache/aurora/client/config.py > 2fc12559016d406c347adb416a5166cca31c961e > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_context.py > 204ca092adad8bf43c5032a02f61bf303fb0b2fc > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > --- > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > *** OK (All tests passed) *** > > mesos-master start/running, process 26868 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 19m46.324s > user 0m1.496s > sys 0m0.774s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50052: AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50052/ --- (Updated July 20, 2016, 9:26 a.m.) Review request for Aurora, Joshua Cohen and Stephan Erb. Changes --- Updated release notes Bugs: AURORA-1736 https://issues.apache.org/jira/browse/AURORA-1736 Repository: aurora Description --- AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint Diffs (updated) - RELEASE-NOTES.md 29d224d5596eb060356f1343cf347db79b1ad687 config/legacy_untested_classes.txt 1ea2183ab20cc5c6bca147bcea4e5c708d576b62 src/main/java/org/apache/aurora/scheduler/http/Offers.java 80f082410896a50d86c7886736caf79581f5051c src/test/java/org/apache/aurora/scheduler/http/OffersTest.java PRE-CREATION Diff: https://reviews.apache.org/r/50052/diff/ Testing --- Manual, Jenkins, and end_to_end File Attachments CURRENT https://reviews.apache.org/media/uploaded/files/2016/07/18/1de4c357-c932-4c84-962f-4209a5b679bc__offers-old.json NEW https://reviews.apache.org/media/uploaded/files/2016/07/19/799bcd1f-f9c8-4b6e-bbaa-ce8022b1dac1__offers-new.json Thanks, Mehrdad Nurolahzade
Re: Review Request 50052: AURORA-1736 Display reservations and persistent volumes in /offers debug http endpoint
> On July 14, 2016, 7:34 p.m., Joshua Cohen wrote: > > How do you feel about adding a unit test for `getOffers` that verifies all > > this new logic? I feel good. :) > On July 14, 2016, 7:34 p.m., Joshua Cohen wrote: > > src/main/java/org/apache/aurora/scheduler/http/Offers.java, line 89 > > <https://reviews.apache.org/r/50052/diff/1/?file=1444281#file1444281line89> > > > > Is there any reason that all of these `*_TO_BEAN` functions return an > > Object rather than `Map<String, ?>` (since that's what the root `TO_BEAN` > > function returns) Correct, going to refactor all. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50052/#review142326 ----------- On July 14, 2016, 4:08 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50052/ > --- > > (Updated July 14, 2016, 4:08 p.m.) > > > Review request for Aurora, Joshua Cohen and Stephan Erb. > > > Bugs: AURORA-1736 > https://issues.apache.org/jira/browse/AURORA-1736 > > > Repository: aurora > > > Description > --- > > AURORA-1736 Display reservations and persistent volumes in /offers debug http > endpoint > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/http/Offers.java > 80f082410896a50d86c7886736caf79581f5051c > > Diff: https://reviews.apache.org/r/50052/diff/ > > > Testing > --- > > Manual, Jenkins, and end_to_end > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
> On June 28, 2016, 10:02 a.m., Aurora ReviewBot wrote: > > Master (73dd2a8) is red with this patch. > > ./build-support/jenkins/build.sh > > > > > > src/test/python/apache/thermos/common/test_planner.py::test_planner_empty > > [32mPASSED[0m > > > > src/test/python/apache/thermos/common/test_planner.py::test_planner_unordered > > [32mPASSED[0m > > > > src/test/python/apache/thermos/common/test_planner.py::test_planner_ordered > > [32mPASSED[0m > > > > src/test/python/apache/thermos/common/test_planner.py::test_planner_mixed > > [32mPASSED[0m > > > > src/test/python/apache/thermos/common/test_planner.py::test_planner_unsatisfiables > > [32mPASSED[0m > > > > FAILURES > > > > test_get_config_with_production_and_tier_is_preemptible > > > > [1mdef > > test_get_config_with_production_and_tier_is_preemptible():[0m > > [1m context = FakeAuroraCommandContext()[0m > > [1m > > context.set_options(create_mock_options())[0m > > [1m with > > CLUSTERS.patch(AuroraClientCommandTest.TEST_CLUSTERS.values()):[0m > > [1mapi = > > context.get_api(TEST_CLUSTER.name)[0m > > [1mapi.get_tier_configs.return_value = > > AuroraClientCommandTest.get_mock_tier_configurations()[0m > > [1mwith temporary_file() as fp:[0m > > [1m fp.write(create_test_config())[0m > > [1m fp.flush()[0m > > [1m config = > > context.get_job_config(AuroraClientCommandTest.TEST_JOBKEY, fp.name)[0m > > [1m assert not > > config.job().taskConfig.production[0m > > [1m> assert config.job().taskConfig.tier == > > AuroraClientCommandTest.PREEMPTIBLE_TIER.name[0m > > [1m[31mE assert 'revocable' == > > 'preemptible'[0m > > [1m[31mE - revocable[0m > > [1m[31mE + preemptible[0m > > > > > > src/test/python/apache/aurora/client/cli/test_context.py:110: AssertionError > > -- Captured stderr call -- > > INFO:root:OK > > INFO] OK > > INFO] OK > > INFO] OK > > generated xml file: > > /home/jenkins/jenkins-slave/workspace/AuroraBot/dist/test-results/415337499eb72578eab327a6487c1f5c9452b3d6.xml > > > > [1m[31m 1 failed, 667 passed, 6 skipped, 1 warnings > > in 198.50 seconds [0m > > > > FAILURE > > > > [32m > >Waiting for background workers to finish.[0m > > 17:01:40 04:08 [complete][31m > >FAILURE[0m > > > > > > I will refresh this build result if you post a review containing > > "@ReviewBot retry" This is passing locally, not sure why it's breaking on the server. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review139819 --- On June 28, 2016, 10:28 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated June 28, 2016, 10:28 a.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 &g
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/ --- (Updated June 28, 2016, 10:28 a.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Changes --- Applied suggested refactoring Repository: aurora Description --- AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes Diffs (updated) - RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec src/main/python/apache/aurora/client/api/__init__.py 68baf8fdb90cd26100159401c46c9963c24332b3 src/main/python/apache/aurora/client/cli/context.py 9b1511801d031ff48b81c25688a55cb586b8ac66 src/main/python/apache/aurora/client/config.py 2fc12559016d406c347adb416a5166cca31c961e src/test/python/apache/aurora/client/cli/test_command_hooks.py 2130f1fa71be02a004cdf8e476a270c81a7105d3 src/test/python/apache/aurora/client/cli/test_context.py 204ca092adad8bf43c5032a02f61bf303fb0b2fc src/test/python/apache/aurora/client/cli/test_create.py 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c src/test/python/apache/aurora/client/cli/test_cron.py f3c522ed94a2d774865811ceb546bf9df083c14f src/test/python/apache/aurora/client/cli/test_plugins.py a545fece5e2b3e0017a61e1be9ac478372b1f34d src/test/python/apache/aurora/client/cli/test_restart.py 967d560e5c7eb0ed85b215fb11d9751b8666acb5 src/test/python/apache/aurora/client/cli/util.py 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e src/test/python/apache/aurora/client/test_config.py b1a3c1865819899ef19173be0f861783a2631d0a Diff: https://reviews.apache.org/r/49048/diff/ Testing --- ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh *** OK (All tests passed) *** mesos-master start/running, process 26868 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real19m46.324s user0m1.496s sys 0m0.774s ``` Thanks, Mehrdad Nurolahzade
Review Request 49334: AURORA-1725 Expose tier configurations as a debug page
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49334/ --- Review request for Aurora, David McLaughlin and Maxim Khutornenko. Repository: aurora Description --- AURORA-1725 Expose tier configurations as a debug page Diffs - config/legacy_untested_classes.txt 1ea2183ab20cc5c6bca147bcea4e5c708d576b62 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 496cd5590d6042d30e7bd0410a6c40d804c15023 src/main/java/org/apache/aurora/scheduler/http/Tiers.java PRE-CREATION src/main/resources/scheduler/assets/index.html eca27e081950a66b743f79ae7ae3d08e4625d2c8 src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java 561b134152113a50ff495cef28d2396b56d02f1c Diff: https://reviews.apache.org/r/49334/diff/ Testing --- Comments: - Logged the work done so far (new ```/tiers``` debug page) to this ticket and kept AURORA-1710 for modifications to scheduler UI. - Added comments to AURORA-1710 to clarify the suggested approach for implementation. Testing: ``` ./build-support/jenkins/build.sh ... *** OK (All tests passed) *** ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ... mesos-master start/running, process 18776 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real15m5.795s user0m1.318s sys 0m0.533s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 49334: AURORA-1725 Expose tier configurations as a debug page
> On June 28, 2016, 1:53 p.m., David McLaughlin wrote: > > config/legacy_untested_classes.txt, line 44 > > <https://reviews.apache.org/r/49334/diff/1/?file=1432053#file1432053line44> > > > > Isn't this a fairly easy class to test? > > Mehrdad Nurolahzade wrote: > I wrote a test for it initially but then I ran into a few problems > interesting problems but then eventually gave up. Seems like many other http > endpoints have remained untested for a similar reason. I can get behind > finding a solution for this testing problem though. > > The ```getTiers()``` call uses ```MessageBodyWriter``` to serialize the > ```Map```. The elements in that map can be ordered randomly (depending on map > implementation) which makes String equality difficult. That was the easier > problem to solve, I used ```LinkedHashMap``` in test fixtures to force > determinstic order of elements in the map. > > Then, I ran into the problem of Jackson ordering ```TierInfo``` > properties in undeterminstic order. That is, a tier can be serialized as > ```"perferred":{"preemptible":false,"revocable":false}``` or > ```"perferred":{"revocable":false,"preemptible":false}```. A quick Google > search suggested that application of @JasonPropertyOrder on ```TierInfo``` > can result in deterministic property serialization order but it did not work > for me (potentially because of mocking but I am not sure). > > Anyways, after burning some time here I decided to give up. Let me know > if you have any suggestion for testing. Found a simple solution using GSON. Will update this review board shortly. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49334/#review139857 --- On June 28, 2016, 1:42 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49334/ > --- > > (Updated June 28, 2016, 1:42 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > AURORA-1725 Expose tier configurations as a debug page > > > Diffs > - > > config/legacy_untested_classes.txt 1ea2183ab20cc5c6bca147bcea4e5c708d576b62 > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java > 496cd5590d6042d30e7bd0410a6c40d804c15023 > src/main/java/org/apache/aurora/scheduler/http/Tiers.java PRE-CREATION > src/main/resources/scheduler/assets/index.html > eca27e081950a66b743f79ae7ae3d08e4625d2c8 > src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java > 561b134152113a50ff495cef28d2396b56d02f1c > > Diff: https://reviews.apache.org/r/49334/diff/ > > > Testing > --- > > Comments: > - Logged the work done so far (new ```/tiers``` debug page) to this ticket > and kept AURORA-1710 for modifications to scheduler UI. > - Added comments to AURORA-1710 to clarify the suggested approach for > implementation. > > Testing: > ``` > ./build-support/jenkins/build.sh > ... > > *** OK (All tests passed) *** > > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > ... > mesos-master start/running, process 18776 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 15m5.795s > user 0m1.318s > sys 0m0.533s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 49334: AURORA-1725 Expose tier configurations as a debug page
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49334/ --- (Updated June 28, 2016, 3:04 p.m.) Review request for Aurora, David McLaughlin and Maxim Khutornenko. Changes --- Added unit test Repository: aurora Description --- AURORA-1725 Expose tier configurations as a debug page Diffs (updated) - src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java 4089b79da8079243703eead884e80bcf736f8b29 src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 496cd5590d6042d30e7bd0410a6c40d804c15023 src/main/java/org/apache/aurora/scheduler/http/Tiers.java PRE-CREATION src/main/resources/scheduler/assets/index.html eca27e081950a66b743f79ae7ae3d08e4625d2c8 src/test/java/org/apache/aurora/scheduler/TierManagerTest.java 7d9b3330b8c2b7a87b4dc3adfff94e40ef25d294 src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java 561b134152113a50ff495cef28d2396b56d02f1c src/test/java/org/apache/aurora/scheduler/http/TiersTest.java PRE-CREATION Diff: https://reviews.apache.org/r/49334/diff/ Testing --- Comments: - Logged the work done so far (new ```/tiers``` debug page) to this ticket and kept AURORA-1710 for modifications to scheduler UI. - Added comments to AURORA-1710 to clarify the suggested approach for implementation. Testing: ``` ./build-support/jenkins/build.sh ... *** OK (All tests passed) *** ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ... mesos-master start/running, process 18776 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real15m5.795s user0m1.318s sys 0m0.533s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/ --- (Updated June 28, 2016, 4:57 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1710 https://issues.apache.org/jira/browse/AURORA-1710 Repository: aurora Description --- AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes Diffs - RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec src/main/python/apache/aurora/client/api/__init__.py 68baf8fdb90cd26100159401c46c9963c24332b3 src/main/python/apache/aurora/client/cli/context.py 9b1511801d031ff48b81c25688a55cb586b8ac66 src/main/python/apache/aurora/client/config.py 2fc12559016d406c347adb416a5166cca31c961e src/test/python/apache/aurora/client/cli/test_command_hooks.py 2130f1fa71be02a004cdf8e476a270c81a7105d3 src/test/python/apache/aurora/client/cli/test_context.py 204ca092adad8bf43c5032a02f61bf303fb0b2fc src/test/python/apache/aurora/client/cli/test_create.py 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c src/test/python/apache/aurora/client/cli/test_cron.py f3c522ed94a2d774865811ceb546bf9df083c14f src/test/python/apache/aurora/client/cli/test_plugins.py a545fece5e2b3e0017a61e1be9ac478372b1f34d src/test/python/apache/aurora/client/cli/test_restart.py 967d560e5c7eb0ed85b215fb11d9751b8666acb5 src/test/python/apache/aurora/client/cli/util.py 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e src/test/python/apache/aurora/client/test_config.py b1a3c1865819899ef19173be0f861783a2631d0a Diff: https://reviews.apache.org/r/49048/diff/ Testing --- ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh *** OK (All tests passed) *** mesos-master start/running, process 26868 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real19m46.324s user0m1.496s sys 0m0.774s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 49334: AURORA-1725 Expose tier configurations as a debug page
> On June 28, 2016, 1:53 p.m., David McLaughlin wrote: > > config/legacy_untested_classes.txt, line 44 > > <https://reviews.apache.org/r/49334/diff/1/?file=1432053#file1432053line44> > > > > Isn't this a fairly easy class to test? I wrote a test for it initially but then I ran into a few problems interesting problems but then eventually gave up. Seems like many other http endpoints have remained untested for a similar reason. I can get behind finding a solution for this testing problem though. The ```getTiers()``` call uses ```MessageBodyWriter``` to serialize the ```Map```. The elements in that map can be ordered randomly (depending on map implementation) which makes String equality difficult. That was the easier problem to solve, I used ```LinkedHashMap``` in test fixtures to force determinstic order of elements in the map. Then, I ran into the problem of Jackson ordering ```TierInfo``` properties in undeterminstic order. That is, a tier can be serialized as ```"perferred":{"preemptible":false,"revocable":false}``` or ```"perferred":{"revocable":false,"preemptible":false}```. A quick Google search suggested that application of @JasonPropertyOrder on ```TierInfo``` can result in deterministic property serialization order but it did not work for me (potentially because of mocking but I am not sure). Anyways, after burning some time here I decided to give up. Let me know if you have any suggestion for testing. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49334/#review139857 ------- On June 28, 2016, 1:42 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49334/ > --- > > (Updated June 28, 2016, 1:42 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Repository: aurora > > > Description > --- > > AURORA-1725 Expose tier configurations as a debug page > > > Diffs > - > > config/legacy_untested_classes.txt 1ea2183ab20cc5c6bca147bcea4e5c708d576b62 > src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java > 496cd5590d6042d30e7bd0410a6c40d804c15023 > src/main/java/org/apache/aurora/scheduler/http/Tiers.java PRE-CREATION > src/main/resources/scheduler/assets/index.html > eca27e081950a66b743f79ae7ae3d08e4625d2c8 > src/test/java/org/apache/aurora/scheduler/http/AbstractJettyTest.java > 561b134152113a50ff495cef28d2396b56d02f1c > > Diff: https://reviews.apache.org/r/49334/diff/ > > > Testing > --- > > Comments: > - Logged the work done so far (new ```/tiers``` debug page) to this ticket > and kept AURORA-1710 for modifications to scheduler UI. > - Added comments to AURORA-1710 to clarify the suggested approach for > implementation. > > Testing: > ``` > ./build-support/jenkins/build.sh > ... > > *** OK (All tests passed) *** > > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > ... > mesos-master start/running, process 18776 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 15m5.795s > user 0m1.318s > sys 0m0.533s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
> On June 28, 2016, 8:39 p.m., Joshua Cohen wrote: > > Should this be merged with upstream now that 0.15 is released? - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review139923 --- On June 28, 2016, 4:57 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated June 28, 2016, 4:57 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Bugs: AURORA-1710 > https://issues.apache.org/jira/browse/AURORA-1710 > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 > src/main/python/apache/aurora/client/config.py > 2fc12559016d406c347adb416a5166cca31c961e > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_context.py > 204ca092adad8bf43c5032a02f61bf303fb0b2fc > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > --- > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > *** OK (All tests passed) *** > > mesos-master start/running, process 26868 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 19m46.324s > user 0m1.496s > sys 0m0.774s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/ --- (Updated July 8, 2016, 1:12 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Changes --- Rebased to master Bugs: AURORA-1710 https://issues.apache.org/jira/browse/AURORA-1710 Repository: aurora Description --- AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes Diffs (updated) - RELEASE-NOTES.md 29d224d5596eb060356f1343cf347db79b1ad687 src/main/python/apache/aurora/client/api/__init__.py 68baf8fdb90cd26100159401c46c9963c24332b3 src/main/python/apache/aurora/client/cli/context.py 9b1511801d031ff48b81c25688a55cb586b8ac66 src/main/python/apache/aurora/client/config.py 2fc12559016d406c347adb416a5166cca31c961e src/test/python/apache/aurora/client/cli/test_command_hooks.py 2130f1fa71be02a004cdf8e476a270c81a7105d3 src/test/python/apache/aurora/client/cli/test_context.py 204ca092adad8bf43c5032a02f61bf303fb0b2fc src/test/python/apache/aurora/client/cli/test_create.py 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c src/test/python/apache/aurora/client/cli/test_cron.py f3c522ed94a2d774865811ceb546bf9df083c14f src/test/python/apache/aurora/client/cli/test_plugins.py a545fece5e2b3e0017a61e1be9ac478372b1f34d src/test/python/apache/aurora/client/cli/test_restart.py 967d560e5c7eb0ed85b215fb11d9751b8666acb5 src/test/python/apache/aurora/client/cli/util.py 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e src/test/python/apache/aurora/client/test_config.py b1a3c1865819899ef19173be0f861783a2631d0a Diff: https://reviews.apache.org/r/49048/diff/ Testing --- ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh *** OK (All tests passed) *** mesos-master start/running, process 26868 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real19m46.324s user0m1.496s sys 0m0.774s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/ --- (Updated July 8, 2016, 4:15 p.m.) Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Changes --- The logic was originally correct but then I modified it based on review comments without noticing that the refactoring has broken the logic. Bugs: AURORA-1710 https://issues.apache.org/jira/browse/AURORA-1710 Repository: aurora Description --- AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes Diffs (updated) - RELEASE-NOTES.md 29d224d5596eb060356f1343cf347db79b1ad687 src/main/python/apache/aurora/client/api/__init__.py 68baf8fdb90cd26100159401c46c9963c24332b3 src/main/python/apache/aurora/client/cli/context.py 9b1511801d031ff48b81c25688a55cb586b8ac66 src/main/python/apache/aurora/client/config.py 2fc12559016d406c347adb416a5166cca31c961e src/test/python/apache/aurora/client/cli/test_command_hooks.py 2130f1fa71be02a004cdf8e476a270c81a7105d3 src/test/python/apache/aurora/client/cli/test_context.py 204ca092adad8bf43c5032a02f61bf303fb0b2fc src/test/python/apache/aurora/client/cli/test_create.py 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c src/test/python/apache/aurora/client/cli/test_cron.py f3c522ed94a2d774865811ceb546bf9df083c14f src/test/python/apache/aurora/client/cli/test_plugins.py a545fece5e2b3e0017a61e1be9ac478372b1f34d src/test/python/apache/aurora/client/cli/test_restart.py 967d560e5c7eb0ed85b215fb11d9751b8666acb5 src/test/python/apache/aurora/client/cli/util.py 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e src/test/python/apache/aurora/client/test_config.py b1a3c1865819899ef19173be0f861783a2631d0a Diff: https://reviews.apache.org/r/49048/diff/ Testing --- ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh *** OK (All tests passed) *** mesos-master start/running, process 26868 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real19m46.324s user0m1.496s sys 0m0.774s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
> On June 28, 2016, 8:39 p.m., Joshua Cohen wrote: > > > > Mehrdad Nurolahzade wrote: > Should this be merged with upstream now that 0.15 is released? > > Joshua Cohen wrote: > Yes, we can land this now. Would you mind rebasing? Then I can commit it. Alright, running tests locally now, going to push shortly. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review139923 --- On June 28, 2016, 4:57 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated June 28, 2016, 4:57 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Bugs: AURORA-1710 > https://issues.apache.org/jira/browse/AURORA-1710 > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > RELEASE-NOTES.md af2061c7605c12a066778bd99ec1a3857bee6dec > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 > src/main/python/apache/aurora/client/config.py > 2fc12559016d406c347adb416a5166cca31c961e > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_context.py > 204ca092adad8bf43c5032a02f61bf303fb0b2fc > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > --- > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > *** OK (All tests passed) *** > > mesos-master start/running, process 26868 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 19m46.324s > user 0m1.496s > sys 0m0.774s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 49048: AURORA-1710 Make 'tier' required and remove support for 'production' flag in Job configuration - CLI changes
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49048/#review141698 --- @ReviewBot retry - Mehrdad Nurolahzade On July 8, 2016, 4:15 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49048/ > --- > > (Updated July 8, 2016, 4:15 p.m.) > > > Review request for Aurora, Joshua Cohen and Maxim Khutornenko. > > > Bugs: AURORA-1710 > https://issues.apache.org/jira/browse/AURORA-1710 > > > Repository: aurora > > > Description > --- > > AURORA-1710 Make 'tier' required and remove support for 'production' flag in > Job configuration - CLI changes > > > Diffs > - > > RELEASE-NOTES.md 29d224d5596eb060356f1343cf347db79b1ad687 > src/main/python/apache/aurora/client/api/__init__.py > 68baf8fdb90cd26100159401c46c9963c24332b3 > src/main/python/apache/aurora/client/cli/context.py > 9b1511801d031ff48b81c25688a55cb586b8ac66 > src/main/python/apache/aurora/client/config.py > 2fc12559016d406c347adb416a5166cca31c961e > src/test/python/apache/aurora/client/cli/test_command_hooks.py > 2130f1fa71be02a004cdf8e476a270c81a7105d3 > src/test/python/apache/aurora/client/cli/test_context.py > 204ca092adad8bf43c5032a02f61bf303fb0b2fc > src/test/python/apache/aurora/client/cli/test_create.py > 8c27e2b340bb0a5fb5bcb44ef94d433e7f92c76c > src/test/python/apache/aurora/client/cli/test_cron.py > f3c522ed94a2d774865811ceb546bf9df083c14f > src/test/python/apache/aurora/client/cli/test_plugins.py > a545fece5e2b3e0017a61e1be9ac478372b1f34d > src/test/python/apache/aurora/client/cli/test_restart.py > 967d560e5c7eb0ed85b215fb11d9751b8666acb5 > src/test/python/apache/aurora/client/cli/util.py > 7b4558ec7f0fb0fd2902591bc6a90dc15051dd6e > src/test/python/apache/aurora/client/test_config.py > b1a3c1865819899ef19173be0f861783a2631d0a > > Diff: https://reviews.apache.org/r/49048/diff/ > > > Testing > --- > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > *** OK (All tests passed) *** > > mesos-master start/running, process 26868 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 19m46.324s > user 0m1.496s > sys 0m0.774s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 50819: Use update_job instead of creating new config object when modifying.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/50819/#review144823 --- Ship it! Ship It! - Mehrdad Nurolahzade On Aug. 4, 2016, 2:10 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/50819/ > --- > > (Updated Aug. 4, 2016, 2:10 p.m.) > > > Review request for Aurora and Mehrdad Nurolahzade. > > > Repository: aurora > > > Description > --- > > Use update_job instead of creating new config object when modifying. This > avoids losing any state (e.g. metadata) attached to the config. > > > Diffs > - > > src/main/python/apache/aurora/client/cli/context.py > 7bbfb5e815b2408b8ddaaeeb9911a5bd1b019a27 > > Diff: https://reviews.apache.org/r/50819/diff/ > > > Testing > --- > > $ ./pants test.pytest src/test/python/apache/aurora/client:: > > 14:07:19 00:00 [main] >(To run a reporting server: ./pants server) > 14:07:19 00:00 [setup] > 14:07:19 00:00 [parse] >Executing tasks in goals: test > 14:07:19 00:00 [test] > 14:07:19 00:00 [test-prep-command] > 14:07:19 00:00 [test] > 14:07:19 00:00 [pytest] > 14:07:19 00:00 [run] > > 14:07:19 00:00 [chroot]== test session starts > === > platform darwin -- Python 2.7.10 -- py-1.4.31 -- > pytest-2.6.4 > plugins: timeout, cov > collected 339 items > > > src/test/python/apache/aurora/client/cli/test_config_noun.py ... > src/test/python/apache/aurora/client/cli/test_context.py > > src/test/python/apache/aurora/client/cli/test_version.py > . > src/test/python/apache/aurora/client/cli/test_quota.py > . > src/test/python/apache/aurora/client/cli/test_plugins.py > . > src/test/python/apache/aurora/client/cli/test_client.py > .. > src/test/python/apache/aurora/client/cli/test_sla.py > . > src/test/python/apache/aurora/client/cli/test_open.py > . > src/test/python/apache/aurora/client/cli/test_supdate.py > . > src/test/python/apache/aurora/client/cli/test_restart.py > .. > src/test/python/apache/aurora/client/cli/test_status.py > . > src/test/python/apache/aurora/client/cli/test_add.py > src/test/python/apache/aurora/client/cli/test_diff.py .. > src/test/python/apache/aurora/client/cli/test_cron.py > .. > > src/test/python/apache/aurora/client/cli/test_command_hooks.py .. > src/test/python/apache/aurora/client/cli/test_options.py > .. > src/test/python/apache/aurora/client/cli/test_task.py > . > src/test/python/apache/aurora/client/cli/test_create.py > .. > src/test/python/apache/aurora/client/cli/test_kill.py > . > src/test/python/apache/aurora/client/cli/test_inspect.py > ... > > src/test/python/apache/aurora/client/cli/test_api_from_cli.py .. > > src/test/python/apache/aurora/client/cli/test_diff_formatter.py . > > src/test/python/apache/aurora/client/api/test_updater_util.py ... > > src/test/python/apache/aurora/client/api/test_job_monitor.py .. > src/test/python/apache/aurora/client/api/test_api.py > .. > > src/test/python/apache/aurora/client/api/test_task_util.py . > > src/test/python/apache/aurora/client/api/test_restarter.py .. > > src/test/python/apache/aurora/client/api/test_instance_watcher.py .. > > src/test/python/apache/aurora/client/api/test_scheduler_client.py > . > src/test/python/apache/aurora/client/api/test_sla.py > .. >
Re: Review Request 56131: Suppress role deprecation warning as replacement is not yet ready.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56131/#review163701 --- Ship it! Ship It! - Mehrdad Nurolahzade On Jan. 31, 2017, 10:41 a.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56131/ > --- > > (Updated Jan. 31, 2017, 10:41 a.m.) > > > Review request for Aurora, Mehrdad Nurolahzade, Santhosh Kumar Shanmugham, > and Stephan Erb. > > > Repository: aurora > > > Description > --- > > The role field was prematurely deprecated in the Mesos project. > https://github.com/apache/mesos/blob/master/include/mesos/mesos.proto#L257 > > Suppress deprecation warnings. > > > Diffs > - > > > src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java > 49877682e501d0af76f6e2583b59e93b1bd90137 > > Diff: https://reviews.apache.org/r/56131/diff/ > > > Testing > --- > > > Thanks, > > David McLaughlin > >
Re: Review Request 56265: Move Aurora to v1 Protobufs.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56265/#review164267 --- Ship it! Should we chase this patch with a source-wide refactoring from `slave` to `agent`? - Mehrdad Nurolahzade On Feb. 3, 2017, 11:38 a.m., Zameer Manji wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56265/ > --- > > (Updated Feb. 3, 2017, 11:38 a.m.) > > > Review request for Aurora, Mehrdad Nurolahzade and Stephan Erb. > > > Bugs: AURORA-1886 > https://issues.apache.org/jira/browse/AURORA-1886 > > > Repository: aurora > > > Description > --- > > This is the first step in moving Aurora to the V1 API from Mesos. This patch > moves most of the code to v1 Protobufs. This means all peices of code that do > not interact with Mesos now handle only v1 Protobufs. > > Classes that interact with Mesos directly are: > > * `org.apache.aurora.scheduler.mesos.SchedulerDriverService` > * `org.apache.aurora.scheduler.mesos.MesosSchedulerImpl` > * `org.apache.aurora.scheduler.mesos.DriverFactoryImpl` > > These classes handle unversioned Protobufs and use the `ProtosConversion` > class > to convert them to v1 Protobufs that can be safely passed to the rest of the > code. > > > Diffs > - > > src/jmh/java/org/apache/aurora/benchmark/Offers.java > 144f47ea07222d8a972f311b5eaf407fcc502a21 > src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java > a649239c80e031c6d4e3d0770a5d4728f897a94b > src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java > 9674c76120cf7b748ac4c8ace1da84483442996e > src/jmh/java/org/apache/aurora/benchmark/fakes/FakeOfferManager.java > fbd24ea4a58e28c14a343170de137c0e0ae437a2 > src/main/java/org/apache/aurora/scheduler/HostOffer.java > ad30bf978ae5aa278fa9b5e01294c43892b08762 > src/main/java/org/apache/aurora/scheduler/SchedulerModule.java > 2ec3967ddb1d470cf681de062a6400f647978185 > src/main/java/org/apache/aurora/scheduler/TaskStatusHandler.java > 3e132ee8595b9b771adcc9580af213c1a4439e69 > src/main/java/org/apache/aurora/scheduler/TaskStatusHandlerImpl.java > 6afafe875fc31982bc1afcebe40e4953172a7984 > src/main/java/org/apache/aurora/scheduler/base/Conversions.java > d08b6cf1c7f9e49fefd3b560aeba9bb4a53a20fc > src/main/java/org/apache/aurora/scheduler/base/Numbers.java > 1b278e29555cf4dbaba63b7b87e8b21ae78f9786 > src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java > aba73005019c13ac943be9c53ac58c8ce5bfba94 > > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorConfig.java > 32bafb2b3ef4ea64fa0a5e3ffc43b47361fd3358 > > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorModule.java > 0d6a8c9acfbf4a0838fa580da09ce4c50a1b0761 > > src/main/java/org/apache/aurora/scheduler/configuration/executor/ExecutorSettingsLoader.java > 18ff2e2baebe1784bc31bc7f3a685282a2bed915 > > src/main/java/org/apache/aurora/scheduler/configuration/executor/Executors.java > 6ac2d006c45393592e3e8b4ed94f99500f6518b2 > src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java > 70b5470b9dad1af838b5222cae5ac86487e2f2e4 > > src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java > 36c69be9ccf152c5d74466a08ddca8029b4a13c9 > src/main/java/org/apache/aurora/scheduler/mesos/Driver.java > bb208ea7f77402a07aef162930288219595054a2 > src/main/java/org/apache/aurora/scheduler/mesos/DriverFactory.java > 92d8924aa182ae272fd394ce68dcec0bf35a118d > src/main/java/org/apache/aurora/scheduler/mesos/DriverFactoryImpl.java > a5f5e9940c467b7f8584254b4bfefa3b5d059abe > src/main/java/org/apache/aurora/scheduler/mesos/DriverSettings.java > 85d471ff88618665a377182b5b1c208278e3ccea > src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java > 7b2614f3f2b552235cf93ca285bcf9c999457e46 > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java > 3d5c3bd139206e970811aa95bd74b78987bb9cfe > src/main/java/org/apache/aurora/scheduler/mesos/ProtosConversion.java > PRE-CREATION > src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java > c89be79f5610a56477fbbd4ae8b2475a682456b6 > src/main/java/org/apache/aurora/scheduler/mesos/TaskStatusStats.java > 5573638f0a18ae6cdd84f648160d0e63c27f7510 > src/main/java/org/apache/aurora/scheduler/mesos/TestExecutorSettings.java > fe5441190
Re: Review Request 56361: Add additional tests for the conversion of TaskStatus.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56361/#review164519 --- Ship it! Ship It! - Mehrdad Nurolahzade On Feb. 6, 2017, 6:44 p.m., Zameer Manji wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56361/ > --- > > (Updated Feb. 6, 2017, 6:44 p.m.) > > > Review request for Aurora and Mehrdad Nurolahzade. > > > Repository: aurora > > > Description > --- > > This adds additional testing for the `ProtosConversions` class, ensuring > there is the correct conversion between `SlaveID` and `AgentID`. > > > Diffs > - > > src/test/java/org/apache/aurora/scheduler/mesos/ProtosConversionTest.java > 86e065c62cf2bc7ee173a377efa7b57b41918e40 > > Diff: https://reviews.apache.org/r/56361/diff/ > > > Testing > --- > > > Thanks, > > Zameer Manji > >
Re: Review Request 56361: Add additional tests for the conversion of TaskStatus.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56361/#review164517 --- @ReviewBot retry - Mehrdad Nurolahzade On Feb. 6, 2017, 6:44 p.m., Zameer Manji wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56361/ > --- > > (Updated Feb. 6, 2017, 6:44 p.m.) > > > Review request for Aurora and Mehrdad Nurolahzade. > > > Repository: aurora > > > Description > --- > > This adds additional testing for the `ProtosConversions` class, ensuring > there is the correct conversion between `SlaveID` and `AgentID`. > > > Diffs > - > > src/test/java/org/apache/aurora/scheduler/mesos/ProtosConversionTest.java > 86e065c62cf2bc7ee173a377efa7b57b41918e40 > > Diff: https://reviews.apache.org/r/56361/diff/ > > > Testing > --- > > > Thanks, > > Zameer Manji > >
Re: Review Request 55357: AURORA-1867 Consider reserving for multiple tasks per preemption round
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55357/ --- (Updated Jan. 24, 2017, 8:42 a.m.) Review request for Aurora, David McLaughlin, Joshua Cohen, Stephan Erb, and Zameer Manji. Changes --- - Corrected spelling mistake - Fixed bug in unit test Bugs: AURORA-1867 https://issues.apache.org/jira/browse/AURORA-1867 Repository: aurora Description --- To be fair, PendingTaskProcessor interleaves tasks from different groups. However, this fairness comes at the price of increasing reservation time. Even if reservations are being made for the same task group, the processor would still restart iterating through slaves for each task instance. This results in reevaluating all slaves already rejected in a previous search before it finds a new viable candidate. This patch improves `PendingTaskProcessor` performance by reducing slave search/evaluation time, at the cost of reduced fairness. `PendingTaskProcessor` now does reservation for a configurable maximum of _N_ candidates per task group in each iteration over the list of slaves. Diffs (updated) - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java fa37236e68657b539b182519b9d46d96d5b0953a src/main/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessor.java f59f3fd8959b1ba3726b55a2943fb9228a049ac5 src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorMetrics.java 67822cafbe89f4798b4ea6da3856663cc4872798 src/main/java/org/apache/aurora/scheduler/preemptor/PreemptorModule.java 23d1c120657d5cb9d294a80c63e8a04512d361ca src/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java d11ae5883f2a00dca4c4b36f0ab58ea95c7ecb2e src/test/java/org/apache/aurora/scheduler/preemptor/PreemptorModuleTest.java 67b6d69e3ddd1028dfe9ff451b171cd888674920 Diff: https://reviews.apache.org/r/55357/diff/ Testing --- As is, the cluster setup in our existing preemption benchmark does not reflect the improvements resulting from this patch. Currently, all existing victims can be preempted, therefore all `PendingTaskProcessor` has to is look at the next slave. ``` BEFORE Benchmark (numPendingTasks) Mode Cnt Score Error Units SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1 thrpt 10 75.386 ± 2.984 ops/s SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 74.584 ± 2.598 ops/s SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 79.731 ± 2.182 ops/s SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 66.386 ± 1.833 ops/s AFTER Benchmark (numPendingTasks) Mode Cnt Score Error Units SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1 thrpt 10 78.266 ± 3.290 ops/s SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 76.743 ± 2.073 ops/s SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 75.343 ± 1.943 ops/s SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 68.284 ± 2.413 ops/s ``` I need to further imprpve the cluster setup for this benchmark to reflect the improvements in the patch. A more representative cluster setup would be one in which only a subset of potential victims pass `PreemptionVictimFilter.filterPreemptionVictims()` test. Thanks, Mehrdad Nurolahzade
Re: Review Request 55089: AURORA-1826 Expose Thrift server request workload stats
> On Jan. 25, 2017, 3:11 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > line 427 > > <https://reviews.apache.org/r/55089/diff/2/?file=1593704#file1593704line427> > > > > This annotation can be very missleading when adding a new API endpoint. > > If you add it to a method it will only be tracked if one has edited in a > > completly different part of the code. Would it be possible to write > > something like the following? > > > > ``` > > @ThriftWorkload(result -> > > result.getJobSummaryResult().getSummariesSize()) > > ``` I like it! Let me try. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55089/#review163042 ----------- On Dec. 29, 2016, 9:58 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55089/ > --- > > (Updated Dec. 29, 2016, 9:58 p.m.) > > > Review request for Aurora, David McLaughlin and Stephan Erb. > > > Bugs: AURORA-1826 > https://issues.apache.org/jira/browse/AURORA-1826 > > > Repository: aurora > > > Description > --- > > AURORA-1826 Expose Thrift server request workload stats > > > Diffs > - > > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 16b1b52f8691d978a9ec1bf7aa0c9716b3484cf0 > src/main/java/org/apache/aurora/scheduler/thrift/aop/Measured.java > PRE-CREATION > > src/main/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptor.java > d57f910d8f9bbe5c24aec960e88d03702bc353da > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > b28cd2489a52041a8e7e53f298fad8d8cd29406f > > src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java > 9c40ec51c28c8c57365dc21c3cd7391a3894784c > > Diff: https://reviews.apache.org/r/55089/diff/ > > > Testing > --- > > ``` > curl 192.168.33.7:8081/vars | grep thrift_workload > % Total% Received % Xferd Average Speed TimeTime Time > Current > Dload Upload Total SpentLeft Speed > 100 413340 413340 0 3695k 0 --:--:-- --:--:-- --:--:-- 4036k > thrift_workload_addInstances 0 > thrift_workload_createJob 0 > thrift_workload_createOrUpdateCronTemplate 0 > thrift_workload_drainHosts 0 > thrift_workload_endMaintenance 0 > thrift_workload_getConfigSummary 0 > thrift_workload_getJobSummary 0 > thrift_workload_getJobUpdateDetails 0 > thrift_workload_getJobUpdateSummaries 0 > thrift_workload_getJobs 0 > thrift_workload_getPendingReason 0 > thrift_workload_getRoleSummary 0 > thrift_workload_getTaskStatus 0 > thrift_workload_getTasksWithoutConfigs 0 > thrift_workload_killTasks 0 > thrift_workload_maintenanceStatus 0 > thrift_workload_restartShards 0 > thrift_workload_rewriteConfigs 0 > thrift_workload_startJobUpdate 0 > thrift_workload_startMaintenance 0 > ``` > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > ... > > *** OK (All tests passed) *** > > mesos-master start/running, process 2359 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 28m58.389s > user 0m1.508s > sys 0m0.820s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Review Request 56048: Preemption performance improvement and new metrics release notes entry
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56048/ --- Review request for Aurora and Stephan Erb. Repository: aurora Description --- Preemption performance improvement and new metrics release notes entry Diffs - RELEASE-NOTES.md 7d01c90610e2cddf0f0629af669fd3bdb2afa7c5 Diff: https://reviews.apache.org/r/56048/diff/ Testing --- Thanks, Mehrdad Nurolahzade
Re: Review Request 55089: AURORA-1826 Expose Thrift server request workload stats
> On Jan. 27, 2017, 3:37 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptor.java, > > lines 65-72 > > <https://reviews.apache.org/r/55089/diff/3/?file=1616764#file1616764line65> > > > > Can `invocation.proceed();` ever return `null`? > > > > If not, then we could move the new counter code from the `finally` to > > the `try` block and eliminate the `response != null` guard. `response != null` guard is a necessary protection from the corner case when `invocation.proceed()` throws an exception. > On Jan. 27, 2017, 3:37 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptor.java, > > line 51 > > <https://reviews.apache.org/r/55089/diff/3/?file=1616764#file1616764line51> > > > > ... should we use `scheduler_` as a prefix here as well? Done. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55089/#review163351 ------- On Jan. 26, 2017, 2:07 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55089/ > --- > > (Updated Jan. 26, 2017, 2:07 p.m.) > > > Review request for Aurora, David McLaughlin and Stephan Erb. > > > Bugs: AURORA-1826 > https://issues.apache.org/jira/browse/AURORA-1826 > > > Repository: aurora > > > Description > --- > > AURORA-1826 Expose Thrift server request workload stats > > > Diffs > - > > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 16b1b52f8691d978a9ec1bf7aa0c9716b3484cf0 > > src/main/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptor.java > d57f910d8f9bbe5c24aec960e88d03702bc353da > src/main/java/org/apache/aurora/scheduler/thrift/aop/ThriftWorkload.java > PRE-CREATION > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > b28cd2489a52041a8e7e53f298fad8d8cd29406f > > src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java > 9c40ec51c28c8c57365dc21c3cd7391a3894784c > > Diff: https://reviews.apache.org/r/55089/diff/ > > > Testing > --- > > ``` > curl 192.168.33.7:8081/vars | grep thrift_workload > % Total% Received % Xferd Average Speed TimeTime Time > Current > Dload Upload Total SpentLeft Speed > 100 413340 413340 0 3695k 0 --:--:-- --:--:-- --:--:-- 4036k > thrift_workload_addInstances 0 > thrift_workload_createJob 0 > thrift_workload_createOrUpdateCronTemplate 0 > thrift_workload_drainHosts 0 > thrift_workload_endMaintenance 0 > thrift_workload_getConfigSummary 0 > thrift_workload_getJobSummary 0 > thrift_workload_getJobUpdateDetails 0 > thrift_workload_getJobUpdateSummaries 0 > thrift_workload_getJobs 0 > thrift_workload_getPendingReason 0 > thrift_workload_getRoleSummary 0 > thrift_workload_getTaskStatus 0 > thrift_workload_getTasksWithoutConfigs 0 > thrift_workload_killTasks 0 > thrift_workload_maintenanceStatus 0 > thrift_workload_restartShards 0 > thrift_workload_rewriteConfigs 0 > thrift_workload_startJobUpdate 0 > thrift_workload_startMaintenance 0 > ``` > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > ... > > *** OK (All tests passed) *** > > mesos-master start/running, process 2359 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 28m58.389s > user 0m1.508s > sys 0m0.820s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 55089: AURORA-1826 Expose Thrift server request workload stats
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55089/ --- (Updated Jan. 28, 2017, 1:05 p.m.) Review request for Aurora, David McLaughlin and Stephan Erb. Changes --- Added `scheduler` prefix to stat name. Bugs: AURORA-1826 https://issues.apache.org/jira/browse/AURORA-1826 Repository: aurora Description --- AURORA-1826 Expose Thrift server request workload stats Diffs (updated) - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 16b1b52f8691d978a9ec1bf7aa0c9716b3484cf0 src/main/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptor.java d57f910d8f9bbe5c24aec960e88d03702bc353da src/main/java/org/apache/aurora/scheduler/thrift/aop/ThriftWorkload.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b28cd2489a52041a8e7e53f298fad8d8cd29406f src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java 9c40ec51c28c8c57365dc21c3cd7391a3894784c Diff: https://reviews.apache.org/r/55089/diff/ Testing --- ``` curl 192.168.33.7:8081/vars | grep thrift_workload % Total% Received % Xferd Average Speed TimeTime Time Current Dload Upload Total SpentLeft Speed 100 413340 413340 0 3695k 0 --:--:-- --:--:-- --:--:-- 4036k thrift_workload_addInstances 0 thrift_workload_createJob 0 thrift_workload_createOrUpdateCronTemplate 0 thrift_workload_drainHosts 0 thrift_workload_endMaintenance 0 thrift_workload_getConfigSummary 0 thrift_workload_getJobSummary 0 thrift_workload_getJobUpdateDetails 0 thrift_workload_getJobUpdateSummaries 0 thrift_workload_getJobs 0 thrift_workload_getPendingReason 0 thrift_workload_getRoleSummary 0 thrift_workload_getTaskStatus 0 thrift_workload_getTasksWithoutConfigs 0 thrift_workload_killTasks 0 thrift_workload_maintenanceStatus 0 thrift_workload_restartShards 0 thrift_workload_rewriteConfigs 0 thrift_workload_startJobUpdate 0 thrift_workload_startMaintenance 0 ``` ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ... *** OK (All tests passed) *** mesos-master start/running, process 2359 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real28m58.389s user0m1.508s sys 0m0.820s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 55089: AURORA-1826 Expose Thrift server request workload stats
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55089/ --- (Updated Jan. 28, 2017, 1:11 p.m.) Review request for Aurora, David McLaughlin and Stephan Erb. Bugs: AURORA-1826 https://issues.apache.org/jira/browse/AURORA-1826 Repository: aurora Description (updated) --- This patch introduces a number of stats that measure the workload generated by Thrift server requests. Current Thrift server stats expose the number and timing of requests received by the server. However, they fail to reflect the size of the requests. This is limiting us in having an accurate view of the workload handled by the scheduler. For example, every call to `restartShards()` is recorded as one event despite the fact that a request might only restart one shard while another request might seek to restart 1K shards. Diffs - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 16b1b52f8691d978a9ec1bf7aa0c9716b3484cf0 src/main/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptor.java d57f910d8f9bbe5c24aec960e88d03702bc353da src/main/java/org/apache/aurora/scheduler/thrift/aop/ThriftWorkload.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b28cd2489a52041a8e7e53f298fad8d8cd29406f src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java 9c40ec51c28c8c57365dc21c3cd7391a3894784c Diff: https://reviews.apache.org/r/55089/diff/ Testing --- ``` curl 192.168.33.7:8081/vars | grep thrift_workload % Total% Received % Xferd Average Speed TimeTime Time Current Dload Upload Total SpentLeft Speed 100 413340 413340 0 3695k 0 --:--:-- --:--:-- --:--:-- 4036k thrift_workload_addInstances 0 thrift_workload_createJob 0 thrift_workload_createOrUpdateCronTemplate 0 thrift_workload_drainHosts 0 thrift_workload_endMaintenance 0 thrift_workload_getConfigSummary 0 thrift_workload_getJobSummary 0 thrift_workload_getJobUpdateDetails 0 thrift_workload_getJobUpdateSummaries 0 thrift_workload_getJobs 0 thrift_workload_getPendingReason 0 thrift_workload_getRoleSummary 0 thrift_workload_getTaskStatus 0 thrift_workload_getTasksWithoutConfigs 0 thrift_workload_killTasks 0 thrift_workload_maintenanceStatus 0 thrift_workload_restartShards 0 thrift_workload_rewriteConfigs 0 thrift_workload_startJobUpdate 0 thrift_workload_startMaintenance 0 ``` ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ... *** OK (All tests passed) *** mesos-master start/running, process 2359 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real28m58.389s user0m1.508s sys 0m0.820s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 55089: AURORA-1826 Expose Thrift server request workload stats
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55089/ --- (Updated Jan. 26, 2017, 2:07 p.m.) Review request for Aurora, David McLaughlin and Stephan Erb. Changes --- Moved the workload mapping logic from `ThriftStatsExporterInterceptor` to the annotation definition site itself, hoping that this might provide more visibility. Bugs: AURORA-1826 https://issues.apache.org/jira/browse/AURORA-1826 Repository: aurora Description --- AURORA-1826 Expose Thrift server request workload stats Diffs (updated) - src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 16b1b52f8691d978a9ec1bf7aa0c9716b3484cf0 src/main/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptor.java d57f910d8f9bbe5c24aec960e88d03702bc353da src/main/java/org/apache/aurora/scheduler/thrift/aop/ThriftWorkload.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b28cd2489a52041a8e7e53f298fad8d8cd29406f src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java 9c40ec51c28c8c57365dc21c3cd7391a3894784c Diff: https://reviews.apache.org/r/55089/diff/ Testing --- ``` curl 192.168.33.7:8081/vars | grep thrift_workload % Total% Received % Xferd Average Speed TimeTime Time Current Dload Upload Total SpentLeft Speed 100 413340 413340 0 3695k 0 --:--:-- --:--:-- --:--:-- 4036k thrift_workload_addInstances 0 thrift_workload_createJob 0 thrift_workload_createOrUpdateCronTemplate 0 thrift_workload_drainHosts 0 thrift_workload_endMaintenance 0 thrift_workload_getConfigSummary 0 thrift_workload_getJobSummary 0 thrift_workload_getJobUpdateDetails 0 thrift_workload_getJobUpdateSummaries 0 thrift_workload_getJobs 0 thrift_workload_getPendingReason 0 thrift_workload_getRoleSummary 0 thrift_workload_getTaskStatus 0 thrift_workload_getTasksWithoutConfigs 0 thrift_workload_killTasks 0 thrift_workload_maintenanceStatus 0 thrift_workload_restartShards 0 thrift_workload_rewriteConfigs 0 thrift_workload_startJobUpdate 0 thrift_workload_startMaintenance 0 ``` ``` ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ... *** OK (All tests passed) *** mesos-master start/running, process 2359 + RETCODE=0 + restore_netrc + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc + true Connection to 127.0.0.1 closed. real28m58.389s user0m1.508s sys 0m0.820s ``` Thanks, Mehrdad Nurolahzade
Re: Review Request 55089: AURORA-1826 Expose Thrift server request workload stats
> On Jan. 25, 2017, 3:11 p.m., Stephan Erb wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > line 427 > > <https://reviews.apache.org/r/55089/diff/2/?file=1593704#file1593704line427> > > > > This annotation can be very missleading when adding a new API endpoint. > > If you add it to a method it will only be tracked if one has edited in a > > completly different part of the code. Would it be possible to write > > something like the following? > > > > ``` > > @ThriftWorkload(result -> > > result.getJobSummaryResult().getSummariesSize()) > > ``` > > Mehrdad Nurolahzade wrote: > I like it! Let me try. Unfortunately, due to language limitations, the elegant lambda expression approach you suggested is not possible. Annotation attributes have to be compile-time constat expressions. Read more here: http://docs.oracle.com/javase/specs/jls/se8/html/jls-9.html#jls-9.7.1-300 I moved the workload mapping logic from `ThriftStatsExporterInterceptor` to the annotation definition site itself, hoping that this might provide more visibility. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55089/#review163042 --- On Dec. 29, 2016, 9:58 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55089/ > --- > > (Updated Dec. 29, 2016, 9:58 p.m.) > > > Review request for Aurora, David McLaughlin and Stephan Erb. > > > Bugs: AURORA-1826 > https://issues.apache.org/jira/browse/AURORA-1826 > > > Repository: aurora > > > Description > --- > > AURORA-1826 Expose Thrift server request workload stats > > > Diffs > - > > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 16b1b52f8691d978a9ec1bf7aa0c9716b3484cf0 > src/main/java/org/apache/aurora/scheduler/thrift/aop/Measured.java > PRE-CREATION > > src/main/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptor.java > d57f910d8f9bbe5c24aec960e88d03702bc353da > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > b28cd2489a52041a8e7e53f298fad8d8cd29406f > > src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java > 9c40ec51c28c8c57365dc21c3cd7391a3894784c > > Diff: https://reviews.apache.org/r/55089/diff/ > > > Testing > --- > > ``` > curl 192.168.33.7:8081/vars | grep thrift_workload > % Total% Received % Xferd Average Speed TimeTime Time > Current > Dload Upload Total SpentLeft Speed > 100 413340 413340 0 3695k 0 --:--:-- --:--:-- --:--:-- 4036k > thrift_workload_addInstances 0 > thrift_workload_createJob 0 > thrift_workload_createOrUpdateCronTemplate 0 > thrift_workload_drainHosts 0 > thrift_workload_endMaintenance 0 > thrift_workload_getConfigSummary 0 > thrift_workload_getJobSummary 0 > thrift_workload_getJobUpdateDetails 0 > thrift_workload_getJobUpdateSummaries 0 > thrift_workload_getJobs 0 > thrift_workload_getPendingReason 0 > thrift_workload_getRoleSummary 0 > thrift_workload_getTaskStatus 0 > thrift_workload_getTasksWithoutConfigs 0 > thrift_workload_killTasks 0 > thrift_workload_maintenanceStatus 0 > thrift_workload_restartShards 0 > thrift_workload_rewriteConfigs 0 > thrift_workload_startJobUpdate 0 > thrift_workload_startMaintenance 0 > ``` > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > ... > > *** OK (All tests passed) *** > > mesos-master start/running, process 2359 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 28m58.389s > user 0m1.508s > sys 0m0.820s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 55089: AURORA-1826 Expose Thrift server request workload stats
> On Jan. 24, 2017, 5:35 p.m., Reza Motamedi wrote: > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, > > line 510 > > <https://reviews.apache.org/r/55089/diff/2/?file=1593704#file1593704line510> > > > > Should this be something like an atomic long? {{tasksKilled}} is a local variable in method scope (not a field bound to a singleton object on the heap); there would be no concurrent access to it. Read more here: http://stackoverflow.com/questions/12825847/why-are-local-variables-thread-safe-in-java - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55089/#review162893 --- On Dec. 29, 2016, 9:58 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/55089/ > --- > > (Updated Dec. 29, 2016, 9:58 p.m.) > > > Review request for Aurora, David McLaughlin and Stephan Erb. > > > Bugs: AURORA-1826 > https://issues.apache.org/jira/browse/AURORA-1826 > > > Repository: aurora > > > Description > --- > > AURORA-1826 Expose Thrift server request workload stats > > > Diffs > - > > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > 16b1b52f8691d978a9ec1bf7aa0c9716b3484cf0 > src/main/java/org/apache/aurora/scheduler/thrift/aop/Measured.java > PRE-CREATION > > src/main/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptor.java > d57f910d8f9bbe5c24aec960e88d03702bc353da > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > b28cd2489a52041a8e7e53f298fad8d8cd29406f > > src/test/java/org/apache/aurora/scheduler/thrift/aop/ThriftStatsExporterInterceptorTest.java > 9c40ec51c28c8c57365dc21c3cd7391a3894784c > > Diff: https://reviews.apache.org/r/55089/diff/ > > > Testing > --- > > ``` > curl 192.168.33.7:8081/vars | grep thrift_workload > % Total% Received % Xferd Average Speed TimeTime Time > Current > Dload Upload Total SpentLeft Speed > 100 413340 413340 0 3695k 0 --:--:-- --:--:-- --:--:-- 4036k > thrift_workload_addInstances 0 > thrift_workload_createJob 0 > thrift_workload_createOrUpdateCronTemplate 0 > thrift_workload_drainHosts 0 > thrift_workload_endMaintenance 0 > thrift_workload_getConfigSummary 0 > thrift_workload_getJobSummary 0 > thrift_workload_getJobUpdateDetails 0 > thrift_workload_getJobUpdateSummaries 0 > thrift_workload_getJobs 0 > thrift_workload_getPendingReason 0 > thrift_workload_getRoleSummary 0 > thrift_workload_getTaskStatus 0 > thrift_workload_getTasksWithoutConfigs 0 > thrift_workload_killTasks 0 > thrift_workload_maintenanceStatus 0 > thrift_workload_restartShards 0 > thrift_workload_rewriteConfigs 0 > thrift_workload_startJobUpdate 0 > thrift_workload_startMaintenance 0 > ``` > > ``` > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > > ... > > *** OK (All tests passed) *** > > mesos-master start/running, process 2359 > + RETCODE=0 > + restore_netrc > + mv /home/vagrant/.netrc.bak /home/vagrant/.netrc > + true > Connection to 127.0.0.1 closed. > > real 28m58.389s > user 0m1.508s > sys 0m0.820s > ``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 56935: Fix for unnecessary object serializations
> On Feb. 22, 2017, 10:07 a.m., Reza Motamedi wrote: > > Mehrdad, do you also have some stats on how much these changes reduced the > > object creation rate? The object allocation rate dropped from of 25 M/s on average to 15-20 M/s. But, as I indicated above these numbers are not representative as the workload on test cluster is very low. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56935/#review166385 --- On Feb. 22, 2017, 11:34 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56935/ > --- > > (Updated Feb. 22, 2017, 11:34 a.m.) > > > Review request for Aurora, David McLaughlin and Zameer Manji. > > > Repository: aurora > > > Description > --- > > Methodology: I attached a profiler (YourKit) to Aurora in one of test > clusters (where very minimal workload was being applied to the scheduler). > Looking at object allocation rates, `Webhook` and `ResourceType` stood out. > Based on the two cases above, I looked itno `requireNonNull()` and > `LOG.xxx()` methods in the code base to find potentially misbehaving > statements. > > This patch provides a fix for some unnecessary object serilizations that > happen on high frequency execution paths and contribute to scheduler's high > object creation rate. Due to low scheduler workload at the time observation, > this list is not exhaustive. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/events/Webhook.java > 3e8e38abe29766f6fcf08707fba5df402c96a257 > src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java > 17301036e28d95ba90b3e4d8840d8a5641e49c46 > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java > 0d639f66db456858278b0485c91c40975c3b45ac > src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java > c88428412d69e4202e7cceb1b608dc1809a9ccc0 > src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java > a649a6e3d2f2d0aeaf6d7ac704ed24911c310a1e > src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java > d89e715b1b08faf95f8b5788c9c28cbbb33af093 > src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java > 9bb319bb04bb386d9792c3cc0017b039e8f25044 > > Diff: https://reviews.apache.org/r/56935/diff/ > > > Testing > --- > > ```./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 56935: Fix for unnecessary object serializations
> On Feb. 22, 2017, 10:27 a.m., Zameer Manji wrote: > > Could you please add to the RB description on what methodology you used to > > determine this and what are the results of this patch? > > > > > > Further, do you have any ideas on how we can prevent regressions? Short-term, we need to invest in profiling Aurora internally. Long-term, integrating with an observability/monitoring platform like [TwitterServer](https://twitter.github.io/twitter-server/Admin.html#profiling) would help with providing better visibility into heap/GC. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56935/#review166387 --- On Feb. 22, 2017, 11:34 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56935/ > --- > > (Updated Feb. 22, 2017, 11:34 a.m.) > > > Review request for Aurora, David McLaughlin and Zameer Manji. > > > Repository: aurora > > > Description > --- > > Methodology: I attached a profiler (YourKit) to Aurora in one of test > clusters (where very minimal workload was being applied to the scheduler). > Looking at object allocation rates, `Webhook` and `ResourceType` stood out. > Based on the two cases above, I looked itno `requireNonNull()` and > `LOG.xxx()` methods in the code base to find potentially misbehaving > statements. > > This patch provides a fix for some unnecessary object serilizations that > happen on high frequency execution paths and contribute to scheduler's high > object creation rate. Due to low scheduler workload at the time observation, > this list is not exhaustive. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/events/Webhook.java > 3e8e38abe29766f6fcf08707fba5df402c96a257 > src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java > 17301036e28d95ba90b3e4d8840d8a5641e49c46 > src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java > 0d639f66db456858278b0485c91c40975c3b45ac > src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java > c88428412d69e4202e7cceb1b608dc1809a9ccc0 > src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java > a649a6e3d2f2d0aeaf6d7ac704ed24911c310a1e > src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java > d89e715b1b08faf95f8b5788c9c28cbbb33af093 > src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java > 9bb319bb04bb386d9792c3cc0017b039e8f25044 > > Diff: https://reviews.apache.org/r/56935/diff/ > > > Testing > --- > > ```./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh``` > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 56935: Fix for unnecessary object serializations
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56935/ --- (Updated Feb. 22, 2017, 11:34 a.m.) Review request for Aurora, David McLaughlin and Zameer Manji. Changes --- Updated description Repository: aurora Description (updated) --- Methodology: I attached a profiler (YourKit) to Aurora in one of test clusters (where very minimal workload was being applied to the scheduler). Looking at object allocation rates, `Webhook` and `ResourceType` stood out. Based on the two cases above, I looked itno `requireNonNull()` and `LOG.xxx()` methods in the code base to find potentially misbehaving statements. This patch provides a fix for some unnecessary object serilizations that happen on high frequency execution paths and contribute to scheduler's high object creation rate. Due to low scheduler workload at the time observation, this list is not exhaustive. Diffs - src/main/java/org/apache/aurora/scheduler/events/Webhook.java 3e8e38abe29766f6fcf08707fba5df402c96a257 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 17301036e28d95ba90b3e4d8840d8a5641e49c46 src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 0d639f66db456858278b0485c91c40975c3b45ac src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java c88428412d69e4202e7cceb1b608dc1809a9ccc0 src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java a649a6e3d2f2d0aeaf6d7ac704ed24911c310a1e src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java d89e715b1b08faf95f8b5788c9c28cbbb33af093 src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 9bb319bb04bb386d9792c3cc0017b039e8f25044 Diff: https://reviews.apache.org/r/56935/diff/ Testing --- ```./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh``` Thanks, Mehrdad Nurolahzade
Review Request 56575: AURORA-1837 Improve task history pruning
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/ --- Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and Stephan Erb. Bugs: AURORA-1837 https://issues.apache.org/jira/browse/AURORA-1837 Repository: aurora Description --- This patch addressed efficiency issues in the current implementation of `TaskHistoryPruner`. The new design is similar to that of `JobUpdateHistoryPruner`: (a) Instead of registering a `DelayExecutor` run upon terminal task state transitions, it runs on preconfigured intervals, finds all terminal state tasks that meet pruning criteria and deletes them. (b) Makes the initial task history pruning delay configurable so that it does not hamper scheduler upon start. The new design addressed the following two efficiecy problems: 1. Upon scheduler restart/failure, the in-memory state of task history pruning scheduled with `DelayExecutor` is lost. `TaskHistoryPruner` learns about these dead tasks upon restart when log is replayed. These expired tasks are picked up by the second call to `executor.execute()` that performs job level pruning immediately (i.e., without delay). Hence, most task history pruning happens after scheduler restarts and can severely hamper scheduler performance (or cause consecutive fail-overs on test clusters when we put load test on scheduler). 2. Expired tasks can be picked up for pruning multiple times. The asynchronous nature of `BatchWorker` which used to process task deletions introduces some delay between delete enqueue and delete execution. As a result, tasks already queued for deletion in a previous evaluation round might get picked up, evaluated and enqueued for deletion again. This is evident in `tasks_pruned` metric which reflects numbers much higher than the actual number of expired tasks deleted. Diffs - src/main/java/org/apache/aurora/scheduler/base/Query.java c76b365f43eb6a3b9b0b63a879b43eb04dcd8fac src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 735199ac1ab343c24471890aa330d6635c26 src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java f77849498ff23616f1d56d133eb218f837ac3413 src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 14e4040e0b94e96f77068b41454311fa3bf53573 Diff: https://reviews.apache.org/r/56575/diff/ Testing --- Manual testing under Vagrant Thanks, Mehrdad Nurolahzade
Re: Review Request 56629: Expose task pruning endpoint in aurora_admin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56629/#review165434 --- src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java (line 1135) <https://reviews.apache.org/r/56629/#comment237283> We are skipping task delete through `StateManager` which publishes `PubsubEvent.TasksDeleted` to subscribers like `TaskGroups` and `JobUpdateController`. In the case of `JobUpdateController` the dependency does not seem to matter. Did you verify ignoring the `TaskGroups` dependency is safe? - Mehrdad Nurolahzade On Feb. 13, 2017, 6:01 p.m., David McLaughlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56629/ > --- > > (Updated Feb. 13, 2017, 6:01 p.m.) > > > Review request for Aurora, Santhosh Kumar Shanmugham, Stephan Erb, and Zameer > Manji. > > > Repository: aurora > > > Description > --- > > Expose task pruning endpoint in aurora_admin. Useful for scale testing in > order to 'clean up' after a test run, but also useful in production if you > have a bad actor inflating the size of your task index. > > > Diffs > - > > api/src/main/thrift/org/apache/aurora/gen/api.thrift > 6205c2ed1e2d5898a322de5bc2bcba6b2c0cd4b1 > > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java > a211483caf59755b1f9b286b58f183e00db6eee6 > src/main/python/apache/aurora/admin/admin.py > 070c348d2ca5db1edecf832efd9aa5481bddaa4b > src/main/python/apache/aurora/client/api/__init__.py > 1250ccd16f906eec08c159df3300d1b94b306d8e > > src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java > 0cdd9829417bb3ef0215278a9458a3dd78a49a20 > src/test/python/apache/aurora/admin/test_admin.py > 66abade378d3974dbf031a7bbed9cd6fc4f22d5f > > Diff: https://reviews.apache.org/r/56629/diff/ > > > Testing > --- > > Ran various combinations of task pruning via aurora_admin in Vagrant. Works > as expected. > > Also: > > ./pants test src/test/python/apache/aurora/admin:: > > > Thanks, > > David McLaughlin > >
Re: Review Request 56575: AURORA-1837 Improve task history pruning
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/#review165419 --- After testing this patch on our test clusters, I can see that this implementation is generating lots of garbage that causes heap pressure. This is happening because filtering takes place in memory. Hence, even if no tasks are to be pruned, still lots of garbage is generated due to filtering. To avoid in-memory filtering, I am going to experiment with moving the pruning logic into `TaskStore`; similar to how it is currently done in `JobUpdateStore.pruneHistory()`. - Mehrdad Nurolahzade On Feb. 13, 2017, 9:30 a.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56575/ > --- > > (Updated Feb. 13, 2017, 9:30 a.m.) > > > Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and > Stephan Erb. > > > Bugs: AURORA-1837 > https://issues.apache.org/jira/browse/AURORA-1837 > > > Repository: aurora > > > Description > --- > > This patch addressed efficiency issues in the current implementation of > `TaskHistoryPruner`. The new design is similar to that of > `JobUpdateHistoryPruner`: (a) Instead of registering a `DelayExecutor` run > upon terminal task state transitions, it runs on preconfigured intervals, > finds all terminal state tasks that meet pruning criteria and deletes them. > (b) Makes the initial task history pruning delay configurable so that it does > not hamper scheduler upon start. > > The new design addressed the following two efficiecy problems: > > 1. Upon scheduler restart/failure, the in-memory state of task history > pruning scheduled with `DelayExecutor` is lost. `TaskHistoryPruner` learns > about these dead tasks upon restart when log is replayed. These expired tasks > are picked up by the second call to `executor.execute()` that performs job > level pruning immediately (i.e., without delay). Hence, most task history > pruning happens after scheduler restarts and can severely hamper scheduler > performance (or cause consecutive fail-overs on test clusters when we put > load test on scheduler). > > 2. Expired tasks can be picked up for pruning multiple times. The > asynchronous nature of `BatchWorker` which used to process task deletions > introduces some delay between delete enqueue and delete execution. As a > result, tasks already queued for deletion in a previous evaluation round > might get picked up, evaluated and enqueued for deletion again. This is > evident in `tasks_pruned` metric which reflects numbers much higher than the > actual number of expired tasks deleted. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java > 735199ac1ab343c24471890aa330d6635c26 > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java > f77849498ff23616f1d56d133eb218f837ac3413 > > src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java > 14e4040e0b94e96f77068b41454311fa3bf53573 > > Diff: https://reviews.apache.org/r/56575/diff/ > > > Testing > --- > > Manual testing under Vagrant > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 56575: AURORA-1837 Improve task history pruning
> On Feb. 12, 2017, 12:14 a.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > line 62 > > <https://reviews.apache.org/r/56575/diff/1/?file=1630791#file1630791line62> > > > > It is worthwhile to note that we are moving from a workload that was > > spread over a duration to a bursty instanteous workload (saw-tooth like), > > which can potentially make the situation worse by causing a thundering-herd > > at regular intervals. That's a valid concern; testing can better clarify this. I agree that the existing algorithm offers a better best/average case behavior (due to its scheduled pruning strategy). However, I still think the worst case behavior of this implementation is better for two reasons (1) every task/job is evaluated only once and (2) first prune after restart is similar to other prunes and is not burstier. The burst can better be tamed by reducing the pruning interval (e.g., 5 minutes). I believe the key to get this bursty workload under control is extending `org.apache.aurora.scheduler.base.Query` abstraction. If we add something like `.limit(int)` then we can control the max volume of tasks retrieved == load to be processed == garbage to be collected. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/#review165260 ----------- On Feb. 11, 2017, 3:12 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56575/ > --- > > (Updated Feb. 11, 2017, 3:12 p.m.) > > > Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and > Stephan Erb. > > > Bugs: AURORA-1837 > https://issues.apache.org/jira/browse/AURORA-1837 > > > Repository: aurora > > > Description > --- > > This patch addressed efficiency issues in the current implementation of > `TaskHistoryPruner`. The new design is similar to that of > `JobUpdateHistoryPruner`: (a) Instead of registering a `DelayExecutor` run > upon terminal task state transitions, it runs on preconfigured intervals, > finds all terminal state tasks that meet pruning criteria and deletes them. > (b) Makes the initial task history pruning delay configurable so that it does > not hamper scheduler upon start. > > The new design addressed the following two efficiecy problems: > > 1. Upon scheduler restart/failure, the in-memory state of task history > pruning scheduled with `DelayExecutor` is lost. `TaskHistoryPruner` learns > about these dead tasks upon restart when log is replayed. These expired tasks > are picked up by the second call to `executor.execute()` that performs job > level pruning immediately (i.e., without delay). Hence, most task history > pruning happens after scheduler restarts and can severely hamper scheduler > performance (or cause consecutive fail-overs on test clusters when we put > load test on scheduler). > > 2. Expired tasks can be picked up for pruning multiple times. The > asynchronous nature of `BatchWorker` which used to process task deletions > introduces some delay between delete enqueue and delete execution. As a > result, tasks already queued for deletion in a previous evaluation round > might get picked up, evaluated and enqueued for deletion again. This is > evident in `tasks_pruned` metric which reflects numbers much higher than the > actual number of expired tasks deleted. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/base/Query.java > c76b365f43eb6a3b9b0b63a879b43eb04dcd8fac > src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java > 735199ac1ab343c24471890aa330d6635c26 > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java > f77849498ff23616f1d56d133eb218f837ac3413 > > src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java > 14e4040e0b94e96f77068b41454311fa3bf53573 > > Diff: https://reviews.apache.org/r/56575/diff/ > > > Testing > --- > > Manual testing under Vagrant > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 56575: AURORA-1837 Improve task history pruning
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/#review165599 --- Looking at it as is, I'm not sure if there is much value to be gained from pushing this down to `TaskStore`. Do you see any value in pursuing this idea any further? Or shall I restore it to previous state? - Mehrdad Nurolahzade On Feb. 14, 2017, 3:38 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56575/ > --- > > (Updated Feb. 14, 2017, 3:38 p.m.) > > > Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and > Stephan Erb. > > > Bugs: AURORA-1837 > https://issues.apache.org/jira/browse/AURORA-1837 > > > Repository: aurora > > > Description > --- > > This patch addressed efficiency issues in the current implementation of > `TaskHistoryPruner`. The new design is similar to that of > `JobUpdateHistoryPruner`: (a) Instead of registering a `DelayExecutor` run > upon terminal task state transitions, it runs on preconfigured intervals, > finds all terminal state tasks that meet pruning criteria and deletes them. > (b) Makes the initial task history pruning delay configurable so that it does > not hamper scheduler upon start. > > The new design addressed the following two efficiecy problems: > > 1. Upon scheduler restart/failure, the in-memory state of task history > pruning scheduled with `DelayExecutor` is lost. `TaskHistoryPruner` learns > about these dead tasks upon restart when log is replayed. These expired tasks > are picked up by the second call to `executor.execute()` that performs job > level pruning immediately (i.e., without delay). Hence, most task history > pruning happens after scheduler restarts and can severely hamper scheduler > performance (or cause consecutive fail-overs on test clusters when we put > load test on scheduler). > > 2. Expired tasks can be picked up for pruning multiple times. The > asynchronous nature of `BatchWorker` which used to process task deletions > introduces some delay between delete enqueue and delete execution. As a > result, tasks already queued for deletion in a previous evaluation round > might get picked up, evaluated and enqueued for deletion again. This is > evident in `tasks_pruned` metric which reflects numbers much higher than the > actual number of expired tasks deleted. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java > 735199ac1ab343c24471890aa330d6635c26 > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java > f77849498ff23616f1d56d133eb218f837ac3413 > src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java > 1094a122fe836e53d0481ee5c097447f1e91fa0a > src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java > a649a6e3d2f2d0aeaf6d7ac704ed24911c310a1e > src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java > d89e715b1b08faf95f8b5788c9c28cbbb33af093 > > src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java > 14e4040e0b94e96f77068b41454311fa3bf53573 > > src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java > 02719c312294b58525c1fddd3ed096a9b1cef601 > > Diff: https://reviews.apache.org/r/56575/diff/ > > > Testing > --- > > Manual testing under Vagrant > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 56575: AURORA-1837 Improve task history pruning
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/ --- (Updated Feb. 14, 2017, 3:38 p.m.) Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and Stephan Erb. Changes --- WIP: pushing prunable history selection logic down to `TaskStore`. - Done: `MemTaskStore` - ToDo: `DbTaskStore` Bugs: AURORA-1837 https://issues.apache.org/jira/browse/AURORA-1837 Repository: aurora Description --- This patch addressed efficiency issues in the current implementation of `TaskHistoryPruner`. The new design is similar to that of `JobUpdateHistoryPruner`: (a) Instead of registering a `DelayExecutor` run upon terminal task state transitions, it runs on preconfigured intervals, finds all terminal state tasks that meet pruning criteria and deletes them. (b) Makes the initial task history pruning delay configurable so that it does not hamper scheduler upon start. The new design addressed the following two efficiecy problems: 1. Upon scheduler restart/failure, the in-memory state of task history pruning scheduled with `DelayExecutor` is lost. `TaskHistoryPruner` learns about these dead tasks upon restart when log is replayed. These expired tasks are picked up by the second call to `executor.execute()` that performs job level pruning immediately (i.e., without delay). Hence, most task history pruning happens after scheduler restarts and can severely hamper scheduler performance (or cause consecutive fail-overs on test clusters when we put load test on scheduler). 2. Expired tasks can be picked up for pruning multiple times. The asynchronous nature of `BatchWorker` which used to process task deletions introduces some delay between delete enqueue and delete execution. As a result, tasks already queued for deletion in a previous evaluation round might get picked up, evaluated and enqueued for deletion again. This is evident in `tasks_pruned` metric which reflects numbers much higher than the actual number of expired tasks deleted. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 735199ac1ab343c24471890aa330d6635c26 src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java f77849498ff23616f1d56d133eb218f837ac3413 src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java 1094a122fe836e53d0481ee5c097447f1e91fa0a src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java a649a6e3d2f2d0aeaf6d7ac704ed24911c310a1e src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java d89e715b1b08faf95f8b5788c9c28cbbb33af093 src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 14e4040e0b94e96f77068b41454311fa3bf53573 src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java 02719c312294b58525c1fddd3ed096a9b1cef601 Diff: https://reviews.apache.org/r/56575/diff/ Testing --- Manual testing under Vagrant Thanks, Mehrdad Nurolahzade
Re: Review Request 56575: AURORA-1837 Improve task history pruning
> On Feb. 14, 2017, 4:02 p.m., Mehrdad Nurolahzade wrote: > > Looking at it as is, I'm not sure if there is much value to be gained from > > pushing this down to `TaskStore`. > > Do you see any value in pursuing this idea any further? Or shall I restore > > it to previous state? > > Zameer Manji wrote: > I think there is value in moving operations like this down to storage. > First, it mirrors the update store for pruning old updates. > Second, I think we can benefit from more precise storage operations. It > allows us to leverage the power of DbTaskStore (ie we don't need to hydrate a > full object for deletion) and ensures that callers don't do excessive work. Right, that was the motivation. However the way task deletion is handled now, it needs to go through `StateManager` to (a) verify transition to `DELETED` is allowrd and (b) publish `TaskDeleted` events to other subscribers like `TaskGroups`, `TaskVars`, and so on. For this to work at storage level, like it's done for job updates, we need to address its rather inefficient dependency on `TaskManager`. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/#review165599 ------- On Feb. 14, 2017, 3:38 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56575/ > --- > > (Updated Feb. 14, 2017, 3:38 p.m.) > > > Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and > Stephan Erb. > > > Bugs: AURORA-1837 > https://issues.apache.org/jira/browse/AURORA-1837 > > > Repository: aurora > > > Description > --- > > This patch addressed efficiency issues in the current implementation of > `TaskHistoryPruner`. The new design is similar to that of > `JobUpdateHistoryPruner`: (a) Instead of registering a `DelayExecutor` run > upon terminal task state transitions, it runs on preconfigured intervals, > finds all terminal state tasks that meet pruning criteria and deletes them. > (b) Makes the initial task history pruning delay configurable so that it does > not hamper scheduler upon start. > > The new design addressed the following two efficiecy problems: > > 1. Upon scheduler restart/failure, the in-memory state of task history > pruning scheduled with `DelayExecutor` is lost. `TaskHistoryPruner` learns > about these dead tasks upon restart when log is replayed. These expired tasks > are picked up by the second call to `executor.execute()` that performs job > level pruning immediately (i.e., without delay). Hence, most task history > pruning happens after scheduler restarts and can severely hamper scheduler > performance (or cause consecutive fail-overs on test clusters when we put > load test on scheduler). > > 2. Expired tasks can be picked up for pruning multiple times. The > asynchronous nature of `BatchWorker` which used to process task deletions > introduces some delay between delete enqueue and delete execution. As a > result, tasks already queued for deletion in a previous evaluation round > might get picked up, evaluated and enqueued for deletion again. This is > evident in `tasks_pruned` metric which reflects numbers much higher than the > actual number of expired tasks deleted. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java > 735199ac1ab343c24471890aa330d6635c26 > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java > f77849498ff23616f1d56d133eb218f837ac3413 > src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java > 1094a122fe836e53d0481ee5c097447f1e91fa0a > src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java > a649a6e3d2f2d0aeaf6d7ac704ed24911c310a1e > src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java > d89e715b1b08faf95f8b5788c9c28cbbb33af093 > > src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java > 14e4040e0b94e96f77068b41454311fa3bf53573 > > src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java > 02719c312294b58525c1fddd3ed096a9b1cef601 > > Diff: https://reviews.apache.org/r/56575/diff/ > > > Testing > --- > > Manual testing under Vagrant > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 56575: AURORA-1837 Improve task history pruning
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/#review165282 --- To further manage task history pruning heap/workload pressure, we can introduce a new configuation parameter in `PruningModule` for specifying the max number of tasks to be pruned per round. It can default to -1 (unlimited). - Mehrdad Nurolahzade On Feb. 12, 2017, 2:49 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56575/ > --- > > (Updated Feb. 12, 2017, 2:49 p.m.) > > > Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and > Stephan Erb. > > > Bugs: AURORA-1837 > https://issues.apache.org/jira/browse/AURORA-1837 > > > Repository: aurora > > > Description > --- > > This patch addressed efficiency issues in the current implementation of > `TaskHistoryPruner`. The new design is similar to that of > `JobUpdateHistoryPruner`: (a) Instead of registering a `DelayExecutor` run > upon terminal task state transitions, it runs on preconfigured intervals, > finds all terminal state tasks that meet pruning criteria and deletes them. > (b) Makes the initial task history pruning delay configurable so that it does > not hamper scheduler upon start. > > The new design addressed the following two efficiecy problems: > > 1. Upon scheduler restart/failure, the in-memory state of task history > pruning scheduled with `DelayExecutor` is lost. `TaskHistoryPruner` learns > about these dead tasks upon restart when log is replayed. These expired tasks > are picked up by the second call to `executor.execute()` that performs job > level pruning immediately (i.e., without delay). Hence, most task history > pruning happens after scheduler restarts and can severely hamper scheduler > performance (or cause consecutive fail-overs on test clusters when we put > load test on scheduler). > > 2. Expired tasks can be picked up for pruning multiple times. The > asynchronous nature of `BatchWorker` which used to process task deletions > introduces some delay between delete enqueue and delete execution. As a > result, tasks already queued for deletion in a previous evaluation round > might get picked up, evaluated and enqueued for deletion again. This is > evident in `tasks_pruned` metric which reflects numbers much higher than the > actual number of expired tasks deleted. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java > 735199ac1ab343c24471890aa330d6635c26 > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java > f77849498ff23616f1d56d133eb218f837ac3413 > > src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java > 14e4040e0b94e96f77068b41454311fa3bf53573 > > Diff: https://reviews.apache.org/r/56575/diff/ > > > Testing > --- > > Manual testing under Vagrant > > > Thanks, > > Mehrdad Nurolahzade > >
Re: Review Request 56575: AURORA-1837 Improve task history pruning
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/ --- (Updated Feb. 12, 2017, 2:49 p.m.) Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and Stephan Erb. Changes --- Now limiting retrieved terminated tasks to a job scope to (1) decrease heap pressure, and (2) eliminate full-scan of `MemTaskStore` Bugs: AURORA-1837 https://issues.apache.org/jira/browse/AURORA-1837 Repository: aurora Description --- This patch addressed efficiency issues in the current implementation of `TaskHistoryPruner`. The new design is similar to that of `JobUpdateHistoryPruner`: (a) Instead of registering a `DelayExecutor` run upon terminal task state transitions, it runs on preconfigured intervals, finds all terminal state tasks that meet pruning criteria and deletes them. (b) Makes the initial task history pruning delay configurable so that it does not hamper scheduler upon start. The new design addressed the following two efficiecy problems: 1. Upon scheduler restart/failure, the in-memory state of task history pruning scheduled with `DelayExecutor` is lost. `TaskHistoryPruner` learns about these dead tasks upon restart when log is replayed. These expired tasks are picked up by the second call to `executor.execute()` that performs job level pruning immediately (i.e., without delay). Hence, most task history pruning happens after scheduler restarts and can severely hamper scheduler performance (or cause consecutive fail-overs on test clusters when we put load test on scheduler). 2. Expired tasks can be picked up for pruning multiple times. The asynchronous nature of `BatchWorker` which used to process task deletions introduces some delay between delete enqueue and delete execution. As a result, tasks already queued for deletion in a previous evaluation round might get picked up, evaluated and enqueued for deletion again. This is evident in `tasks_pruned` metric which reflects numbers much higher than the actual number of expired tasks deleted. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java 735199ac1ab343c24471890aa330d6635c26 src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java f77849498ff23616f1d56d133eb218f837ac3413 src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java 14e4040e0b94e96f77068b41454311fa3bf53573 Diff: https://reviews.apache.org/r/56575/diff/ Testing --- Manual testing under Vagrant Thanks, Mehrdad Nurolahzade
Re: Review Request 56575: AURORA-1837 Improve task history pruning
> On Feb. 12, 2017, 2:59 p.m., Mehrdad Nurolahzade wrote: > > To further manage task history pruning heap/workload pressure, we can > > introduce a new configuation parameter in `PruningModule` for specifying > > the max number of tasks to be pruned per round. It can default to -1 > > (unlimited). We can also consider adding sleep cycles (e.g., `Thread.sleep(10)`) in the job iteration loop to further slow down cpu/heap pressure. - Mehrdad --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/#review165282 --- On Feb. 12, 2017, 2:49 p.m., Mehrdad Nurolahzade wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/56575/ > --- > > (Updated Feb. 12, 2017, 2:49 p.m.) > > > Review request for Aurora, David McLaughlin, Santhosh Kumar Shanmugham, and > Stephan Erb. > > > Bugs: AURORA-1837 > https://issues.apache.org/jira/browse/AURORA-1837 > > > Repository: aurora > > > Description > --- > > This patch addressed efficiency issues in the current implementation of > `TaskHistoryPruner`. The new design is similar to that of > `JobUpdateHistoryPruner`: (a) Instead of registering a `DelayExecutor` run > upon terminal task state transitions, it runs on preconfigured intervals, > finds all terminal state tasks that meet pruning criteria and deletes them. > (b) Makes the initial task history pruning delay configurable so that it does > not hamper scheduler upon start. > > The new design addressed the following two efficiecy problems: > > 1. Upon scheduler restart/failure, the in-memory state of task history > pruning scheduled with `DelayExecutor` is lost. `TaskHistoryPruner` learns > about these dead tasks upon restart when log is replayed. These expired tasks > are picked up by the second call to `executor.execute()` that performs job > level pruning immediately (i.e., without delay). Hence, most task history > pruning happens after scheduler restarts and can severely hamper scheduler > performance (or cause consecutive fail-overs on test clusters when we put > load test on scheduler). > > 2. Expired tasks can be picked up for pruning multiple times. The > asynchronous nature of `BatchWorker` which used to process task deletions > introduces some delay between delete enqueue and delete execution. As a > result, tasks already queued for deletion in a previous evaluation round > might get picked up, evaluated and enqueued for deletion again. This is > evident in `tasks_pruned` metric which reflects numbers much higher than the > actual number of expired tasks deleted. > > > Diffs > - > > src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java > 735199ac1ab343c24471890aa330d6635c26 > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java > f77849498ff23616f1d56d133eb218f837ac3413 > > src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java > 14e4040e0b94e96f77068b41454311fa3bf53573 > > Diff: https://reviews.apache.org/r/56575/diff/ > > > Testing > --- > > Manual testing under Vagrant > > > Thanks, > > Mehrdad Nurolahzade > >