Review Request 32806: Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32806/ --- Review request for Aurora, Joshua Cohen and Kevin Sweeney. Repository: aurora Description --- Remove use of LocalServiceRegistry, simplify plumbing of HTTP address. Diffs - src/main/java/org/apache/aurora/scheduler/app/AppModule.java 5f6a019e4d6401e1efd075b72c049fa245cc0d0a src/main/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverrides.java 6d92ae3c8ec46e7964e81e507a2f2a7f2db68cfd src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 2af009d3d9ec44a70659225d0c18de9fda3a6f7a src/main/java/org/apache/aurora/scheduler/http/HttpService.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 50f377587ac05dbb72063ea02502e6d980148d3e src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java e03009c12de5a09761c1f444c6439ef3144b0b17 src/test/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverridesTest.java 21fd027976f75acc427c6d9265a7c7a91895d53d src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java c3e40d88fe7ee1a447d1d61980b69bd1b46881e7 src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java 7f80757cb40af7dde042f1d39355eadf2b3b1aee src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java c5c5f789de6bf7693520081d0c1acc5165603242 src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 76cb691e6d7d4fada3a18fde73aceed7039bcaa4 Diff: https://reviews.apache.org/r/32806/diff/ Testing --- Test suite + end-to-end tests. Thanks, Bill Farner
Re: Review Request 33103: Skip known flaky tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33103/ --- (Updated April 13, 2015, 7:44 p.m.) Review request for Aurora, Joshua Cohen and Brian Wickman. Repository: aurora Description --- We've had very few legitimate build breakages over the last few months, these tests are a major source of illegitimate breakage. Diffs (updated) - src/test/python/apache/aurora/executor/common/test_announcer.py 6b782778e52394de3744b43003226dac3f65169e src/test/python/apache/aurora/executor/test_gc_executor.py 0f0b4a3781295c2a6640d272f9c8b0e8f78cc8a8 src/test/python/apache/aurora/executor/test_thermos_task_runner.py 6b24bbb2ab7ca16f97961aabeed945b61e5b5908 src/test/python/apache/thermos/core/test_staged_kill.py b52fcba3c7174a5e4d413b7b01f2fa7d95d21ee1 Diff: https://reviews.apache.org/r/33103/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33103: Skip known flaky tests.
On April 13, 2015, 3:32 p.m., Brian Wickman wrote: Actually could you also mark test_integration_quitquitquit as well? I was trying to address it in https://reviews.apache.org/r/32221/diff/# but don't feel particularly confident about that approach. Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33103/#review79882 --- On April 11, 2015, 5:16 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33103/ --- (Updated April 11, 2015, 5:16 p.m.) Review request for Aurora, Joshua Cohen and Brian Wickman. Repository: aurora Description --- We've had very few legitimate build breakages over the last few months, these tests are a major source of illegitimate breakage. Diffs - src/test/python/apache/aurora/executor/common/test_announcer.py 6b782778e52394de3744b43003226dac3f65169e src/test/python/apache/aurora/executor/test_gc_executor.py 0f0b4a3781295c2a6640d272f9c8b0e8f78cc8a8 src/test/python/apache/aurora/executor/test_thermos_task_runner.py 6b24bbb2ab7ca16f97961aabeed945b61e5b5908 src/test/python/apache/thermos/core/test_staged_kill.py b52fcba3c7174a5e4d413b7b01f2fa7d95d21ee1 Diff: https://reviews.apache.org/r/33103/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33105: Return Iterable from TaskStore.fetchTasks to allow for streaming.
On April 13, 2015, 11:06 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/async/RescheduleCalculator.java, lines 149-152 https://reviews.apache.org/r/33105/diff/1/?file=924537#file924537line149 Inline this as FluentIterable.from(...).first() `first()` changes behavior in that it will succeed if there are multiple items in the `Iterable`. Are you comfortable with that? On April 13, 2015, 11:06 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java, line 37 https://reviews.apache.org/r/33105/diff/1/?file=924545#file924545line37 Auditing the callers it looks like we might also want a `OptionalIScheduledTask fetchTask(String taskId);`. There are several API-level changes to be made. For now i would like to start with pure API compatibility to avoid pulling any more on a very long thread. On April 13, 2015, 11:06 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/async/TaskHistoryPruner.java, line 160 https://reviews.apache.org/r/33105/diff/1/?file=924538#file924538line160 This is potentially very expensive to this caller if the underlying implementation is not a collection - drop a TODO to expose a COUNT() interface? I agree. My goal for right now is to minimally change APIs and re-assess callers for needed functions a bit further down the road. I'd like to avoid dropping TODOs around, as performance will become the forcing function for these changes. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33105/#review79962 --- On April 11, 2015, 6:22 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33105/ --- (Updated April 11, 2015, 6:22 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Repository: aurora Description --- This allows the store implementation to stream if it wishes, which we might want to take advantage of when the store is backed by a database. One thing worth noting is that this change is a good example of why using wide types at call sites is important. There were many callers of TaskStore.fetchTasks that were already assigning as `Iterable`, and they were immune to this refactor. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java ce87344dc18818faa7a1a0298143dc81fff7 src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 1da35c05fb22c8fa0227a91df6a19f1b8f1aae92 src/main/java/org/apache/aurora/scheduler/async/KillRetry.java 3bb80ec99ed1f5c1d87edb59f3c9502ab6f7c706 src/main/java/org/apache/aurora/scheduler/async/RescheduleCalculator.java 0cf7fb40e5640d0ad07a7fc46c57548a3a385872 src/main/java/org/apache/aurora/scheduler/async/TaskHistoryPruner.java 985a3196ad809c7ccf30ba87a1f10cc3dacd5f49 src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 3b5dcf819228297b3840cb01788c3085759f3c4a src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java ed82ae99f23d5a7f1634261205cbe5339fe4cec8 src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 09be4dc8d6318ccfdf10397585ef7989221afd7f src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java b6a7b4aca841025c790f4b282f0490ff5bb23b2f src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java a8e3b14b3f8147f60871f9dfa84616425636e10a src/main/java/org/apache/aurora/scheduler/storage/Storage.java 6180a36255253542197234ec610eea01366135bc src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java b76c937281971a505db4b352b80cbb7ccfcafcca src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java 38764e5e7484e2fe152460ac7920be9a5799b85d src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java 2102adb3844fc1501ff9d672a2db78a69513231b src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java 7aef1ca786fea521256f69f6a9e7a978649a7195 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java b7d3874e9f597d93c9f623773a3a0289c2b76d6d src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java c5643d9d99fc46d55fd6c48161230139fb7f12b8 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 88c0163170ebc25995d9ef8b1543335a4322bb8e src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java bcd1b4e854f5ea227268c73156bc97c7c937c1de src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
Re: Review Request 33106: Simplify AttributeAggregate.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33106/ --- (Updated April 14, 2015, 4 a.m.) Review request for Aurora and Maxim Khutornenko. Changes --- Reintroduced `Supplier`. Repository: aurora Description (updated) --- Use `Multiset` instead of `AtomicLongMap`, introduce a convenience constant for unit tests to use. Diffs (updated) - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java ce87344dc18818faa7a1a0298143dc81fff7 src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java ed82ae99f23d5a7f1634261205cbe5339fe4cec8 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java c5643d9d99fc46d55fd6c48161230139fb7f12b8 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 88c0163170ebc25995d9ef8b1543335a4322bb8e src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java bcd1b4e854f5ea227268c73156bc97c7c937c1de src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 7e2d1c54362b33cc3507a4bc3e3ccc02ca29bd6f src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java b80e558f18b817814e4768b13ff94aa816d28543 src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java 61cea326ababcd6242a3c5a6dcf8d0b3ca7fbdd6 src/test/java/org/apache/aurora/scheduler/filter/AttributeAggregateTest.java 4b565769d79862326efcb31be694f95f333c89c6 src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java d06b89cc319aa7b4479124cbb2cb224cdb662e05 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java aca0234e037e85202d182affa2c0e988c6cfc854 Diff: https://reviews.apache.org/r/33106/diff/ Testing (updated) --- Thanks, Bill Farner
Re: Review Request 32597: Improving async preemptor efficiency.
On April 17, 2015, 5:59 p.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java,
lines 142-144
https://reviews.apache.org/r/32597/diff/5/?file=931268#file931268line142
```
SetString allSlaves = Sets.newHashSet(Iterables.concat(
slavesToOffers.keySet(),
slavesToActiveTasks.keySet()));
```
Maxim Khutornenko wrote:
Not opposed to the change but how is this better exactly? The way it's
currently written will iterate over all available slaves only once whereas
the proposed change will have to do it twice (at least the way I read it).
Are you assuming that `Iterables.concat` eagerly iterates over the inputs? If
so, that is false:
http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/collect/Iterables.html#concat(java.lang.Iterable...)
I prefer the snippet i've posted since it uses a single statement for a single
logical entity. Of course, if that has the side-effect of signifcantly worse
performance, it is not worth it.
On April 17, 2015, 5:59 p.m., Bill Farner wrote:
src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java,
line 96
https://reviews.apache.org/r/32597/diff/5/?file=931276#file931276line96
Should have brought this up in a previous review, but i'm uncomfortable
with using a mock to control the behavior of a data structure. It really
feels like the test knows way too much about the internals of the class at
this point, given that this is internally-managed state. Is there any
particular reason we shouldn't use a concrete `BiCache` instance here?
Maxim Khutornenko wrote:
Refactored to use concrete instance.
Thanks!!
On April 17, 2015, 5:59 p.m., Bill Farner wrote:
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java,
line 184
https://reviews.apache.org/r/32597/diff/5/?file=931268#file931268line184
It's probably not worth changing code, but might be worth noting that
this breaks round-robin, since the position is reset.
Maxim Khutornenko wrote:
Not sure what you mean. The whole reason to have a consuming iterator
here is to ensure the position is not reset when unsatisfiable groups are
removed.
Aha, thanks - it slipped my mind that the consuming iterator and removeAll were
affecting the same data. You're right.
- Bill
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review80480
---
On April 21, 2015, 1:12 a.m., Maxim Khutornenko wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/
---
(Updated April 21, 2015, 1:12 a.m.)
Review request for Aurora, Bill Farner and Zameer Manji.
Bugs: AURORA-1219
https://issues.apache.org/jira/browse/AURORA-1219
Repository: aurora
Description
---
This is addressing the preemption efficiency loss due to making preemptor
async. Summary:
- Implemented fair task processing by evaluating pending task groups in
round-robin fashion (e.g.: {G1:3, G2:2} - {G1, G2, G1, G2, G1}) against all
available slaves.
- Moved relevant functionality from PreemptionSlotFinder (now
PreemptionVictimFilter) into PendingTaskProcessor.
The bulk of new functionality is in PendingTaskIterator and
PendingTaskProcessor. The rest is refactoring to adjust to the new traversal
approach.
Diffs
-
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
00919b7910704c5025465e1071378a978e5e60a3
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java
f16f964f56f0f9da523950891293083f1bd86780
src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java
5200811ec8fe6fedf42ae2712f29051a8c0bf4a9
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java
dc7eb4421ff305dca32f36c83605c2864fea8b11
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java
7cea881a8c3c11142bd04b3c794cd86a310b15e7
src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java
6af3949b85297043640edccc1a490906c0fcff6c
src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
7bb1e7a9f27088636d6549c089b1d079dfeaf2ee
src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java
8a9a3b7d9686e29632f4e267f591cdb19826e0e7
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
64283fab8c61b841007d7c0a02b083b3067bc78d
src
Re: Review Request 33374: Resuming blocked updates on restart.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33374/#review81046 --- Ship it! Ship It! - Bill Farner On April 20, 2015, 10:19 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33374/ --- (Updated April 20, 2015, 10:19 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1285 https://issues.apache.org/jira/browse/AURORA-1285 Repository: aurora Description --- Changing the `systemResume()` to properly re-initialize `PulseState` for all active coordinated updates. Diffs - src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java ac15217fa9eb63e7292d23702e37c6fd194fbaed src/main/java/org/apache/aurora/scheduler/updater/JobUpdateStateMachine.java 74e915ce678855aa2a2ae44ff59cfba259447796 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java dd4c94097161477e8161fb7e440e4c41e2a61f21 Diff: https://reviews.apache.org/r/33374/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 33403: Upgrade to virtualenv 12.1.1
/rbt.venv/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/connection.py, line 238, in connect ssl_version=resolved_ssl_version) File /home/wfarner/code/aurora/build-support/rbt.venv/local/lib/python2.7/site-packages/pip/_vendor/requests/packages/urllib3/util/ssl_.py, line 253, in ssl_wrap_socket return context.wrap_socket(sock, server_hostname=server_hostname) File /usr/lib/python2.7/ssl.py, line 350, in wrap_socket _context=self) File /usr/lib/python2.7/ssl.py, line 563, in __init__ self.close() File /usr/lib/python2.7/ssl.py, line 758, in close if self._makefile_refs 1: AttributeError: 'SSLSocket' object has no attribute '_makefile_refs' ``` Diffs - build-support/virtualenv 1cdfc7ff95fb4d2f748325610664f8912366bd2a Diff: https://reviews.apache.org/r/33403/diff/ Testing (updated) --- End-to-end tests pass. Thanks, Bill Farner
Re: Review Request 32597: Improving async preemptor efficiency.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review81030
---
Ship it!
LGTM overall, but i'd like to converge on the `Sets.newHashSet` usage before
this lands.
- Bill Farner
On April 21, 2015, 1:12 a.m., Maxim Khutornenko wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/
---
(Updated April 21, 2015, 1:12 a.m.)
Review request for Aurora, Bill Farner and Zameer Manji.
Bugs: AURORA-1219
https://issues.apache.org/jira/browse/AURORA-1219
Repository: aurora
Description
---
This is addressing the preemption efficiency loss due to making preemptor
async. Summary:
- Implemented fair task processing by evaluating pending task groups in
round-robin fashion (e.g.: {G1:3, G2:2} - {G1, G2, G1, G2, G1}) against all
available slaves.
- Moved relevant functionality from PreemptionSlotFinder (now
PreemptionVictimFilter) into PendingTaskProcessor.
The bulk of new functionality is in PendingTaskIterator and
PendingTaskProcessor. The rest is refactoring to adjust to the new traversal
approach.
Diffs
-
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
00919b7910704c5025465e1071378a978e5e60a3
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java
PRE-CREATION
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java
f16f964f56f0f9da523950891293083f1bd86780
src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java
5200811ec8fe6fedf42ae2712f29051a8c0bf4a9
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java
dc7eb4421ff305dca32f36c83605c2864fea8b11
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java
7cea881a8c3c11142bd04b3c794cd86a310b15e7
src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java
6af3949b85297043640edccc1a490906c0fcff6c
src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
7bb1e7a9f27088636d6549c089b1d079dfeaf2ee
src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java
8a9a3b7d9686e29632f4e267f591cdb19826e0e7
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
64283fab8c61b841007d7c0a02b083b3067bc78d
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java
eed2de99a145dd2124b7f2b4d401214f1d8adf2e
Diff: https://reviews.apache.org/r/32597/diff/
Testing
---
./gradlew -Pq build
Manual testing in vagrant
Thanks,
Maxim Khutornenko
Re: Review Request 33366: Fix the path to download stdout/stderr
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33366/#review81055 --- Ship it! I'd like to note that our best practice is to use `mock.patch` sparingly, as it tends to suggest poorly-factored code and results in tests that can be difficult to reason about. However, since the observer may be overhauled in the not-too-distant future, a refactoring would not have been wise and i believe this is an appropriate use of patching. - Bill Farner On April 21, 2015, 7:53 p.m., Bhuvan Arumugam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33366/ --- (Updated April 21, 2015, 7:53 p.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1135 https://issues.apache.org/jira/browse/AURORA-1135 Repository: aurora Description --- Download url for stdout/stderr broken Make it relative to sandbox. Testing Done: ./pants test.pytest --no-fast --options='--verbose' src/test/python:: Bugs closed: AURORA-1135 Diffs - src/main/python/apache/thermos/observer/http/file_browser.py 2f48594a87724b84bad7e149eb4d63c71ea954ee src/test/python/apache/thermos/observer/http/BUILD PRE-CREATION src/test/python/apache/thermos/observer/http/test_file_browser.py PRE-CREATION Diff: https://reviews.apache.org/r/33366/diff/ Testing --- Thanks, Bhuvan Arumugam
Re: Review Request 33279: Add SQL tables needed for a datbase-backed task store.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33279/#review81512 --- Ping - Maxim, can you check my replies above? - Bill Farner On April 16, 2015, 10 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33279/ --- (Updated April 16, 2015, 10 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Bugs: AURORA-556 https://issues.apache.org/jira/browse/AURORA-556 Repository: aurora Description --- Add SQL tables needed for a datbase-backed task store. Diffs - src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ed73ef7a25537f145910dba7a1985805b3979173 Diff: https://reviews.apache.org/r/33279/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33279: Add SQL tables needed for a datbase-backed task store.
On April 20, 2015, 5:50 p.m., Maxim Khutornenko wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 179 https://reviews.apache.org/r/33279/diff/1/?file=932067#file932067line179 What are your plans on retaining the user part of the Identity struct (AURORA-749)? If you are planning to address it later a follow up TODO here would be nice to avoid dropping this issue off the radar. My plan is to avoid changing behavior with the task store, so i did not have any intention of removing fields. On April 20, 2015, 5:50 p.m., Maxim Khutornenko wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql, line 200 https://reviews.apache.org/r/33279/diff/1/?file=932067#file932067line200 Isn't this too restrictive? Currently, we can still have something like 'rack':'limit:5' and 'rack':'abc'. This constraint will now allow configs like that any longer. Maxim Khutornenko wrote: s/now/not FWIW you actually can't do that due to AURORA-199. I'm indifferent. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33279/#review80750 --- On April 16, 2015, 10 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33279/ --- (Updated April 16, 2015, 10 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Bugs: AURORA-556 https://issues.apache.org/jira/browse/AURORA-556 Repository: aurora Description --- Add SQL tables needed for a datbase-backed task store. Diffs - src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql ed73ef7a25537f145910dba7a1985805b3979173 Diff: https://reviews.apache.org/r/33279/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33366: download url for stdout/stderr broken
On April 20, 2015, 6:14 p.m., Aurora ReviewBot wrote: Master (b18dc44) is green with this patch. ./build-support/jenkins/build.sh However, it appears that it might lack test coverage. I will refresh this build result if you post a review containing @ReviewBot retry Bhuvan Arumugam wrote: @ReviewBot retry Unfortunately the bot will not read comments on a reply, only the text in a separate review. However, a retry won't help in this case since the bot has rightly flagged that the patch changes src/main but not src/test. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33366/#review80771 --- On April 20, 2015, 5:57 p.m., Bhuvan Arumugam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33366/ --- (Updated April 20, 2015, 5:57 p.m.) Review request for Aurora. Bugs: AURORA-1135 https://issues.apache.org/jira/browse/AURORA-1135 Repository: aurora Description --- Fix the path to download stdout/stderr. Make it relative to sandbox. Testing Done: ./pants test.pytest --no-fast --options='--verbose' src/test/python:: Bugs closed: AURORA-1135 Diffs - src/main/python/apache/thermos/observer/http/file_browser.py 2f48594a87724b84bad7e149eb4d63c71ea954ee Diff: https://reviews.apache.org/r/33366/diff/ Testing --- Thanks, Bhuvan Arumugam
Re: Review Request 33366: download url for stdout/stderr broken
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33366/#review80772 --- Thanks for the fix! A few requests: - Please add test coverage, even if rudimentary. - Please lift the first line of your description to the Summary, Fix the path to download stdout/stderr. The summary becomes the first line of the commit message, so it's ideal for it to explain the fix rather than state the problem. - Bill Farner On April 20, 2015, 5:57 p.m., Bhuvan Arumugam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33366/ --- (Updated April 20, 2015, 5:57 p.m.) Review request for Aurora. Bugs: AURORA-1135 https://issues.apache.org/jira/browse/AURORA-1135 Repository: aurora Description --- Fix the path to download stdout/stderr. Make it relative to sandbox. Testing Done: ./pants test.pytest --no-fast --options='--verbose' src/test/python:: Bugs closed: AURORA-1135 Diffs - src/main/python/apache/thermos/observer/http/file_browser.py 2f48594a87724b84bad7e149eb4d63c71ea954ee Diff: https://reviews.apache.org/r/33366/diff/ Testing --- Thanks, Bhuvan Arumugam
Re: Review Request 33317: Upgrade to pystachio 0.8.0
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33317/#review80805 --- Ship it! Ship It! - Bill Farner On April 20, 2015, 7:17 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33317/ --- (Updated April 20, 2015, 7:17 p.m.) Review request for Aurora, Brian Brazil and Joshua Cohen. Bugs: AURORA-1268 https://issues.apache.org/jira/browse/AURORA-1268 Repository: aurora Description --- Add pystachio 0.8.0 requirement. Diffs - 3rdparty/python/requirements.txt c23f98ca69197c400598c45b88c7c6415ffaf566 src/test/python/apache/aurora/config/test_base.py 66480db134cea97454d00e004bc3a1847cc5410f Diff: https://reviews.apache.org/r/33317/diff/ Testing --- ./pants test.pytest --no-fast --options='-v' src/test/python:: Thanks, Brian Wickman
Review Request 33612: Add a task store implementation that uses a relational database.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33612/ --- Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko. Bugs: AURORA-556 https://issues.apache.org/jira/browse/AURORA-556 Repository: aurora Description --- Add a task store implementation that uses a relational database. The meat of this review is in `DbTaskStore`, `TaskConfigManager`, and the mapper xml files. Some supporting actors include files under views/, which serve DB business objects. I suggest reviewers start by skimming `DbTaskStore` and digesting the comments in there. There are some known loose ends here, most notably being continued performance enhancements and cleanup of relations in different tables. I've included several relevant TODOs, ~all of which must be addressed before this becomes the default task store. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 0182ecb3942cfa2d9dd21138779815f4500339be examples/vagrant/upstart/aurora-scheduler.conf 82ad42fd0a626672dca80a5362fc07d804b3e412 src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java ed1171d52655fef643330f34913c256f77300fa2 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 3d19831ea0eb85032172b096ac87e126701aa218 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 94ce5c3499ced1b63abf19787acc21b2cd4d0c75 src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java c8df88b77b9a2017c48bdc0c9a0477927ba0b179 src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 1a6c3f21d4fcb476539f588facecd8ef37332837 src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/shims/ContainerShim.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/shims/TaskConstraintShim.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/ScheduleStatusTypeHandler.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java 4d0c10d60037a3310226a6fd8936b84ae4135e7e src/main/java/org/apache/aurora/scheduler/storage/db/views/AssignedPort.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/ScheduledTaskWrapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/TaskConfigRow.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/TaskLink.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/mem/InMemStoresModule.java 21f7d4db821930d2c5b52648e1996aa1ef12a85c src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f76f9a988669dab598e9d19f849018c6f55ce03e src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml PRE-CREATION src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml PRE-CREATION src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java afb7db8eefa63b84d370877742870acdec58899c src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java e3b13407cb6875489e50cf93212845eab7aacb89 src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java f18619a27eeb2aea8dcf01e54c23ed7d1c7d3d87 src/test/java/org/apache/aurora/scheduler/storage/mem/StorageTransactionTest.java bad9eb56b33c3e649c3b173e83d9c30da8f9317d Diff: https://reviews.apache.org/r/33612/diff/ Testing --- Unit tests and end-to-end tests, both using the new task store by default with this change. Thanks, Bill Farner
Review Request 33659: Add support for shorthand names of security realm modules.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33659/ --- Review request for Aurora and Kevin Sweeney. Bugs: AURORA-1290 https://issues.apache.org/jira/browse/AURORA-1290 Repository: aurora Description --- I initially went down the path of a custom `Parser` that extended `ModuleParser`, but it turns out that doesn't work. Parsers are identified by type, and a specific parser on the `@CmdLine` arg would have to reimplement the guts of `SetParser`. As a result, i decided it was more sane to bake the shorthand list in our canonical parser of modules. Diffs - docs/security.md db2e92495661800ef513334568810f16fcf513e1 examples/vagrant/upstart/aurora-scheduler-kerberos.conf ef502b7dcc48c716f71ab5ce920084917564f6ff src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java c96821683b4569977d6d2b8ed657b0625bdd1903 Diff: https://reviews.apache.org/r/33659/diff/ Testing --- TODO(wfarner): Verify in end-to-end tests. Thanks, Bill Farner
Re: Review Request 33659: Add support for shorthand names of security realm modules.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33659/ --- (Updated April 29, 2015, 4:45 a.m.) Review request for Aurora and Kevin Sweeney. Bugs: AURORA-1290 https://issues.apache.org/jira/browse/AURORA-1290 Repository: aurora Description --- I initially went down the path of a custom `Parser` that extended `ModuleParser`, but it turns out that doesn't work. Parsers are identified by type, and a specific parser on the `@CmdLine` arg would have to reimplement the guts of `SetParser`. As a result, i decided it was more sane to bake the shorthand list in our canonical parser of modules. Diffs - docs/security.md db2e92495661800ef513334568810f16fcf513e1 examples/vagrant/upstart/aurora-scheduler-kerberos.conf ef502b7dcc48c716f71ab5ce920084917564f6ff src/main/java/org/apache/aurora/scheduler/http/api/security/ModuleParser.java c96821683b4569977d6d2b8ed657b0625bdd1903 Diff: https://reviews.apache.org/r/33659/diff/ Testing (updated) --- End-to-end tests pass. Thanks, Bill Farner
Re: Review Request 33537: Document Aurora security features.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33537/#review81826 --- Ship it! Ship It! - Bill Farner On April 27, 2015, 11:24 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33537/ --- (Updated April 27, 2015, 11:24 p.m.) Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-817 https://issues.apache.org/jira/browse/AURORA-817 Repository: aurora Description --- SSIA A rendered version is available on github as https://github.com/kevints/aurora/blob/kts/security-docs/docs/security.md - feel free to send pull requests to fix typos, etc, from the markdown editor there and I will incorporate them into this review. Diffs - docs/deploying-aurora-scheduler.md 70d6e3d5a74cab808b327574c79f5b794aa63c28 docs/security.md PRE-CREATION Diff: https://reviews.apache.org/r/33537/diff/ Testing --- Thanks, Kevin Sweeney
Re: Review Request 33658: Remove redundant enable_api_security argument.
On April 29, 2015, 6:13 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java, line 150 https://reviews.apache.org/r/33658/diff/2/?file=944645#file944645line150 : Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33658/#review81990 --- On April 29, 2015, 9:24 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33658/ --- (Updated April 29, 2015, 9:24 p.m.) Review request for Aurora and Kevin Sweeney. Bugs: AURORA-1291 https://issues.apache.org/jira/browse/AURORA-1291 Repository: aurora Description --- Remove redundant enable_api_security argument. Diffs - docs/security.md db2e92495661800ef513334568810f16fcf513e1 examples/vagrant/upstart/aurora-scheduler-kerberos.conf ef502b7dcc48c716f71ab5ce920084917564f6ff examples/vagrant/upstart/aurora-scheduler.conf 82ad42fd0a626672dca80a5362fc07d804b3e412 src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java ca8e23b57a2fc17ca72102b64c70bb21b47bb860 src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java a91c4a2e911e24d4d4d5a9b8eb6544491affd890 Diff: https://reviews.apache.org/r/33658/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33456: Adding logging threadpool executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33456/#review81725 --- src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java https://reviews.apache.org/r/33456/#comment132156 Have you considered creating factory methods that apply decorators to ExecutorServices? That would potentially save this code from the combinatoric explosion we seem to be heading towards. ForwardingExecutorService [1] could be helpful to minimize the code to implement decorators. [1] http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/util/concurrent/ForwardingExecutorService.html - Bill Farner On April 22, 2015, 10:58 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33456/ --- (Updated April 22, 2015, 10:58 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Our async EventBus is using a regular executor thus potentially hiding unhandled errors. Diffs - src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java f657e057b5bbff69971876e104ff0e47b2dc4faa src/main/java/org/apache/aurora/scheduler/events/PubsubEventModule.java 3a4d40adc1abe170b5b80644db9f079751d8a9bf src/test/java/org/apache/aurora/scheduler/base/AsyncUtilTest.java e990f528aac768b5c9b829c9544045a831e094fe Diff: https://reviews.apache.org/r/33456/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 33537: Document Aurora security features.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33537/#review81722 --- Overall LGTM. Can you link to this page from `deploying-aurora-scheduler.md`? docs/security.md https://reviews.apache.org/r/33537/#comment132148 A table of contents would be nice, and match other files. docs/security.md https://reviews.apache.org/r/33537/#comment132147 The merits of strong passwords is not lost on me, but i don't think it should be the job of this document. Noting that these are plaintext/raw is good, but beyond that i think it's out of scope. docs/security.md https://reviews.apache.org/r/33537/#comment132152 Is an equivalent change to clusters.json needed for basic auth? Even if not, it's worth calling out. docs/security.md https://reviews.apache.org/r/33537/#comment132149 Markdown didn't like the format example here. - Bill Farner On April 24, 2015, 10:40 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33537/ --- (Updated April 24, 2015, 10:40 p.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-817 https://issues.apache.org/jira/browse/AURORA-817 Repository: aurora Description --- SSIA A rendered version is available on github as https://github.com/kevints/aurora/blob/kts/security-docs/docs/security.md - feel free to send pull requests to fix typos, etc, from the markdown editor there and I will incorporate them into this review. Diffs - docs/security.md PRE-CREATION Diff: https://reviews.apache.org/r/33537/diff/ Testing --- Thanks, Kevin Sweeney
Review Request 33600: Extract an abstract base test for TaskStore implementations.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33600/ --- Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- Another step towards a DB task store implementation. I renamed `MemTaskStoreTest` to `InMemTaskStoreTest` to prove that the delta here is small. Otherwise, git will not treat the bulk of the change as a rename. Diffs - src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java fbdbb05b76f588a1e024c3f965013da4c7f1f982 src/main/java/org/apache/aurora/scheduler/storage/mem/InMemStoresModule.java 88fd006fc1d5b55ad957f2f232682d61c74e76b7 src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java 688a02f8c827d5185e35ae2ec919dd0ae5b958ec Diff: https://reviews.apache.org/r/33600/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33243: Use explicit status update acknowledgements.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33243/#review81753 --- Ship it! Ship It! - Bill Farner On April 27, 2015, 11:14 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33243/ --- (Updated April 27, 2015, 11:14 p.m.) Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-1228 https://issues.apache.org/jira/browse/AURORA-1228 Repository: aurora Description --- The scheduler now explicitly acknowledges updates. I left the structure as is, per Bill's suggestion. Diffs - src/main/java/org/apache/aurora/scheduler/mesos/DriverFactoryImpl.java db7aa74c2dfca3d520d978d7837d1f2639e6c05c src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java f40746b5712f21e551c06eeb1aa379ebdcdbc693 src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 4ef23ea6ebd2c9587a8356507fcb49a36b9de219 Diff: https://reviews.apache.org/r/33243/diff/ Testing --- Ran the unit tests and the end-to-end test. Thanks, Ben Mahler
Re: Review Request 33458: Implementing PendingTaskProcessor benchmark.
On April 28, 2015, 12:37 a.m., Bill Farner wrote:
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java, line 243
https://reviews.apache.org/r/33458/diff/1/?file=940120#file940120line243
I'm not too fond of this, it points out that `BenchmarkSettings` is
being used for distinct purposes, and may become difficult to reason about.
Can you accomplish this by saving the tasks from
`PreemptorSlotSearchBenchmark` directly, since it's the oddball here?
Maxim Khutornenko wrote:
I actually like how BenchmarkSettings.getTasks() fits the count
supported by the Tasks.Builder. There is no reason to restrict benchmark runs
to just a single test task. How about generalizing this to support multiple
tasks instead? E.g.:
```
@Benchmark
public boolean runBenchmark() {
boolean result = false;
for (IScheduledTask task : settings.getTasks()) {
result =
taskScheduler.schedule(task.getAssignedTask().getTaskId());
}
return result;
}
```
This crossed my mind as well, i would indeed prefer that.
- Bill
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33458/#review81759
---
On April 23, 2015, 12:24 a.m., Maxim Khutornenko wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/33458/
---
(Updated April 23, 2015, 12:24 a.m.)
Review request for Aurora and Bill Farner.
Repository: aurora
Description
---
Adjusting preemptor benchmark to test the correct functionality.
```
Benchmark
(numPendingTasks) Mode Cnt Score Error Units
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
1 thrpt 10 52.245 ± 3.140 ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
10 thrpt 10 49.447 ± 1.606 ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
100 thrpt 10 49.996 ± 2.105 ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark
1000 thrpt 10 48.325 ± 1.710 ops/s
```
Diffs
-
src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java
8f43bd7068f3647fb2df22c60e913367924e2262
src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java
12f42a92a28c68fcc55bf2f0b130ffed8c4d4879
src/jmh/java/org/apache/aurora/benchmark/Tasks.java
b4da057d658586f5e96c871a836011cb7a08d5d9
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
4427115ceee0e5f3ca32462e3cfb2ad2f0ece913
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java
156bac2d8cf941bcda0474ef75c7eaec74a39a3a
Diff: https://reviews.apache.org/r/33458/diff/
Testing
---
./gradlew jmh
Thanks,
Maxim Khutornenko
Re: Review Request 33611: Add benchmarks for fetching tasks over the API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33611/#review81763 --- This is as far as i went towards getting some basic performance coverage for the database-backed task store. Since it will be enabled with a toggle, i stopped here so the initial diff is only required to pass functional tests, and we can then iterate on performance. - Bill Farner On April 28, 2015, 12:57 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33611/ --- (Updated April 28, 2015, 12:57 a.m.) Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- Add a benchmark for unscoped API task queries. This change also makes it possible to run specific benchmarks. I'm also sneaking in a cleanup - removing unnecessary parameter to `DbModule.testModule()`. Diffs - build.gradle 470d11ee0ab9f21e92d3796ab29e038cd9cce0dc src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 6ec0e14355850c8859f675d8fb2bc56fb64cb8b8 src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java a4abbd8129c5e4799a07a1d0134d0232313b0eb8 src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java d6ca430662d456df847ac53dbd6e5632c2c936dd src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java eae17707cc420793884519b579b79652b886a696 src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java 2014b73a44cfdb198f738cb110399d62c396fa1f src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 010e75f05449f618017cb40fd3298e7147a1d2c8 Diff: https://reviews.apache.org/r/33611/diff/ Testing --- Thanks, Bill Farner
Review Request 33611: Add benchmarks for fetching tasks over the API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33611/ --- Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- Add a benchmark for unscoped API task queries. This change also makes it possible to run specific benchmarks. I'm also sneaking in a cleanup - removing unnecessary parameter to `DbModule.testModule()`. Diffs - build.gradle 470d11ee0ab9f21e92d3796ab29e038cd9cce0dc src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 6ec0e14355850c8859f675d8fb2bc56fb64cb8b8 src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java a4abbd8129c5e4799a07a1d0134d0232313b0eb8 src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java d6ca430662d456df847ac53dbd6e5632c2c936dd src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java eae17707cc420793884519b579b79652b886a696 src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java 2014b73a44cfdb198f738cb110399d62c396fa1f src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 010e75f05449f618017cb40fd3298e7147a1d2c8 Diff: https://reviews.apache.org/r/33611/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33608: Added a status update throughput benchmark.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33608/#review82163 --- src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java https://reviews.apache.org/r/33608/#comment132861 I should have asked the first time around - what's the thought process behind including this? Given that this is a benchmark, it seems only to place a ceiling on throughput. - Bill Farner On April 30, 2015, 12:35 a.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33608/ --- (Updated April 30, 2015, 12:35 a.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1283 https://issues.apache.org/jira/browse/AURORA-1283 Repository: aurora Description --- In order to justify doing asynchronous batch acknowledgements and to better understand status update throughput, this introduces a benchmark. Note that this assumes that status update processing is not synchronous, so that the benchmark doesn't need to be updated for AURORA-1228. Diffs - src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/fakes/FakeOfferManager.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java c54619f7cd617b48069363173dcf63b6254b4095 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java d7d659bb13f085ff06291ef0033572f8bdf29874 Diff: https://reviews.apache.org/r/33608/diff/ Testing --- Ran the benchmarks against the existing code and some pending code I have for AURORA-1228 to demonstrate the improvement. The end to end tests are broken, appears to be unrelated to my change from what I can tell. Thanks, Ben Mahler
Re: Review Request 33705: Don't retry API requests that fail with auth errors.
On April 30, 2015, 7:15 p.m., Joshua Cohen wrote: src/main/python/apache/aurora/common/transport.py, line 64 https://reviews.apache.org/r/33705/diff/1/?file=945910#file945910line64 Is this meant to indicate that it should be a function that returns a Session? Is this a standard that's documented somewhere? Yes, that's what it indicates. I made this change to address warnings that PyCharm was showing, since the type hint was incorrect. I couldn't find details on whether this is more broad than PyCharm, but docs are here: https://www.jetbrains.com/pycharm/help/type-hinting-in-pycharm.html#d269326e143 - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33705/#review82172 --- On April 30, 2015, 12:12 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33705/ --- (Updated April 30, 2015, 12:12 a.m.) Review request for Aurora, Joshua Cohen and Kevin Sweeney. Bugs: AURORA-1248 https://issues.apache.org/jira/browse/AURORA-1248 Repository: aurora Description --- The only thing i really don't care for in this patch is that `add_auth_error_handler` alters a 'private' field, but i was unable to come up with an alternative that wouldn't seem to spiral into a refactor. Advice/opinions solicited. Diffs - src/main/python/apache/aurora/client/api/scheduler_client.py 3f9c691056ee50103a7857bea43160c75388f2aa src/main/python/apache/aurora/client/cli/__init__.py 10d943215282a83ebe0cadac1592848d797c3e79 src/main/python/apache/aurora/client/cli/context.py adbffc4aa07c242691e87e0fcfc9f5b840999d1d src/main/python/apache/aurora/common/transport.py eefe8f7b50e0e0040e28ff47b226e91316f1911e src/test/python/apache/aurora/client/api/test_scheduler_client.py 9319728d3a95b158cb734f4cbc6be97187d43def src/test/python/apache/aurora/client/cli/test_context.py d63f060ea6cf747792f27499da901dd7cecae596 src/test/python/apache/aurora/common/test_transport.py e4f93ef9dd6bf0d54166bdf6afb247fad871ea10 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh cc7cdee42b7c03dd1f121963129d94e67cb9e2a2 Diff: https://reviews.apache.org/r/33705/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33739: Enable GC executor to gc STARTING tasks which don't exist on the host
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33739/#review82321 --- Ship it! Ship It! - Bill Farner On May 1, 2015, 9:32 p.m., Zeke Harris wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33739/ --- (Updated May 1, 2015, 9:32 p.m.) Review request for Aurora, Bill Farner and Brian Wickman. Repository: aurora Description --- Enable GC executor to cause the scheduler to kill tasks that it thinks should be STARTING but which don't exist on the host. Diffs - src/main/python/apache/aurora/executor/gc_executor.py a7776b599f5fc028ec2ce7712856e080381e84a6 src/test/python/apache/aurora/executor/test_gc_executor.py 856483d9ef7e89a03bb0780d81510bf60928219a Diff: https://reviews.apache.org/r/33739/diff/ Testing --- $ ./pants test src/test/python/apache/aurora/executor:gc_executor ... SUCCESS Thanks, Zeke Harris
Review Request 33784: Symlink upstart configurations in vagrant rather than copying.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33784/ --- Review request for Aurora and Kevin Sweeney. Bugs: AURORA-1204 https://issues.apache.org/jira/browse/AURORA-1204 Repository: aurora Description --- Symlink upstart configurations in vagrant rather than copying. Diffs - examples/vagrant/provision-dev-cluster.sh e7fdd4b7f5bdfcbe16f8ace1cef0dba00376aa81 Diff: https://reviews.apache.org/r/33784/diff/ Testing --- TODO(wfarner): Report back after verifying end-to-end tests pass for a fresh vagrant machine. Thanks, Bill Farner
Re: Review Request 33608: Added a status update throughput benchmark.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33608/#review82048 --- src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java https://reviews.apache.org/r/33608/#comment132687 If anything, you should expose Scheduler.class to hide the implementation. This should allow you to revert changes in MesosSchedulerImpl.java. - Bill Farner On April 29, 2015, 6:12 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33608/ --- (Updated April 29, 2015, 6:12 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1283 https://issues.apache.org/jira/browse/AURORA-1283 Repository: aurora Description --- In order to justify doing asynchronous batch acknowledgements and to better understand status update throughput, this introduces a benchmark. Note that this assumes that status update processing is not synchronous, so that the benchmark doesn't need to be updated for AURORA-1228. Diffs - src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/fakes/FakeOfferManager.java PRE-CREATION src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java c54619f7cd617b48069363173dcf63b6254b4095 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverModule.java d7d659bb13f085ff06291ef0033572f8bdf29874 Diff: https://reviews.apache.org/r/33608/diff/ Testing --- Ran the benchmarks against the existing code and some pending code I have for AURORA-1228 to demonstrate the improvement. The end to end tests are broken, appears to be unrelated to my change from what I can tell. Thanks, Ben Mahler
Review Request 33705: Don't retry API requests that fail with auth errors.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33705/ --- Review request for Aurora, Joshua Cohen and Kevin Sweeney. Bugs: AURORA-1248 https://issues.apache.org/jira/browse/AURORA-1248 Repository: aurora Description --- The only thing i really don't care for in this patch is that `add_auth_error_handler` alters a 'private' field, but i was unable to come up with an alternative that wouldn't seem to spiral into a refactor. Advice/opinions solicited. Diffs - src/main/python/apache/aurora/client/api/scheduler_client.py 3f9c691056ee50103a7857bea43160c75388f2aa src/main/python/apache/aurora/client/cli/__init__.py 10d943215282a83ebe0cadac1592848d797c3e79 src/main/python/apache/aurora/client/cli/context.py adbffc4aa07c242691e87e0fcfc9f5b840999d1d src/main/python/apache/aurora/common/transport.py eefe8f7b50e0e0040e28ff47b226e91316f1911e src/test/python/apache/aurora/client/api/test_scheduler_client.py 9319728d3a95b158cb734f4cbc6be97187d43def src/test/python/apache/aurora/client/cli/test_context.py d63f060ea6cf747792f27499da901dd7cecae596 src/test/python/apache/aurora/common/test_transport.py e4f93ef9dd6bf0d54166bdf6afb247fad871ea10 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh cc7cdee42b7c03dd1f121963129d94e67cb9e2a2 Diff: https://reviews.apache.org/r/33705/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33458: Implementing PendingTaskProcessor benchmark.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33458/#review81850 --- Ship it! Ship It! - Bill Farner On April 28, 2015, 5:24 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33458/ --- (Updated April 28, 2015, 5:24 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Adjusting preemptor benchmark to test the correct functionality. ``` Benchmark (numPendingTasks) Mode Cnt Score Error Units SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1 thrpt 10 52.245 ± 3.140 ops/s SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 49.447 ± 1.606 ops/s SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 49.996 ± 2.105 ops/s SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 48.325 ± 1.710 ops/s ``` Diffs - src/jmh/java/org/apache/aurora/benchmark/BenchmarkSettings.java 8f43bd7068f3647fb2df22c60e913367924e2262 src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 372addc6f6288ebf7c3dbf1e26902c2b62b69669 src/jmh/java/org/apache/aurora/benchmark/Tasks.java b4da057d658586f5e96c871a836011cb7a08d5d9 src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java 4427115ceee0e5f3ca32462e3cfb2ad2f0ece913 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 156bac2d8cf941bcda0474ef75c7eaec74a39a3a Diff: https://reviews.apache.org/r/33458/diff/ Testing --- ./gradlew jmh Thanks, Maxim Khutornenko
Re: Review Request 33611: Add benchmarks for fetching tasks over the API.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33611/ --- (Updated April 28, 2015, 6:16 p.m.) Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- Add a benchmark for unscoped API task queries. This change also makes it possible to run specific benchmarks. I'm also sneaking in a cleanup - removing unnecessary parameter to `DbModule.testModule()`. Diffs (updated) - build.gradle 470d11ee0ab9f21e92d3796ab29e038cd9cce0dc src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 6ec0e14355850c8859f675d8fb2bc56fb64cb8b8 src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java a4abbd8129c5e4799a07a1d0134d0232313b0eb8 src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java d6ca430662d456df847ac53dbd6e5632c2c936dd src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java eae17707cc420793884519b579b79652b886a696 src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java 2014b73a44cfdb198f738cb110399d62c396fa1f src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 010e75f05449f618017cb40fd3298e7147a1d2c8 Diff: https://reviews.apache.org/r/33611/diff/ Testing --- Thanks, Bill Farner
Review Request 33677: Remove dead code related to Java executor.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33677/ --- Review request for Aurora and Brian Wickman. Bugs: AURORA-195 https://issues.apache.org/jira/browse/AURORA-195 Repository: aurora Description --- Remove dead code related to Java executor. Diffs - src/main/python/apache/aurora/client/api/command_runner.py 2d181fa5fa9ce378518acd83871b02cdbc198201 Diff: https://reviews.apache.org/r/33677/diff/ Testing --- Thanks, Bill Farner
Review Request 33676: Always require slave checkpointing.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33676/ --- Review request for Aurora and Kevin Sweeney. Bugs: AURORA-716 https://issues.apache.org/jira/browse/AURORA-716 Repository: aurora Description --- Always require slave checkpointing. Diffs - src/main/java/org/apache/aurora/scheduler/mesos/CommandLineDriverSettingsModule.java d0994203b5650f44ca2eb32e1e2aa61875163854 Diff: https://reviews.apache.org/r/33676/diff/ Testing --- Thanks, Bill Farner
Review Request 33869: Fix inconsistency in MemTaskStore secondary indices.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33869/ --- Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1305 https://issues.apache.org/jira/browse/AURORA-1305 Repository: aurora Description --- In practice this amounts to little more than a space leak, but a bug nonetheless. Diffs - src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 9c76fa58b52bacd182e87cc4ebebad6c19356a55 src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java e3b13407cb6875489e50cf93212845eab7aacb89 src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java be57c5ee9f0218ed9fabb54b3579ba03477c8930 src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java d1f4026f2b3d1cba542f96cecc0fe490e9ec8132 Diff: https://reviews.apache.org/r/33869/diff/ Testing --- Thanks, Bill Farner
Review Request 33959: Add update watch and update start --watch flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33959/ --- Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-1239 https://issues.apache.org/jira/browse/AURORA-1239 Repository: aurora Description --- Add update watch and update start --watch flag. Diffs - src/main/python/apache/aurora/client/api/__init__.py 0ae1d9fef0a7b190a51e8734c2abd71ecf3c5a32 src/main/python/apache/aurora/client/cli/update.py 8296db5434276910fc4f8470c7a962a8e8e0a9c2 src/test/python/apache/aurora/client/cli/test_supdate.py 6fb1f71b7dc787c090fce7b8e7c51baee862f336 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 501d111f87dbaff8b6f22ffc67a9720b4dad Diff: https://reviews.apache.org/r/33959/diff/ Testing --- Unit tests + end-to-end tests. Thanks, Bill Farner
Re: Review Request 33959: Add update watch and update start --watch flag.
On May 8, 2015, 12:04 a.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/update.py, line 122 https://reviews.apache.org/r/33959/diff/1/?file=952678#file952678line122 Suggest --wait-until for consistency with job create command. `--wait-until` accepts states, while this is a flag. IMHO the name should match the behavior. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33959/#review82942 --- On May 7, 2015, 11:58 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33959/ --- (Updated May 7, 2015, 11:58 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-1239 https://issues.apache.org/jira/browse/AURORA-1239 Repository: aurora Description --- Add update watch and update start --watch flag. Diffs - src/main/python/apache/aurora/client/api/__init__.py 0ae1d9fef0a7b190a51e8734c2abd71ecf3c5a32 src/main/python/apache/aurora/client/cli/update.py 8296db5434276910fc4f8470c7a962a8e8e0a9c2 src/test/python/apache/aurora/client/cli/test_supdate.py 6fb1f71b7dc787c090fce7b8e7c51baee862f336 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 501d111f87dbaff8b6f22ffc67a9720b4dad Diff: https://reviews.apache.org/r/33959/diff/ Testing --- Unit tests + end-to-end tests. Thanks, Bill Farner
Re: Review Request 33959: Add update watch and update start --watch flag.
On May 8, 2015, 12:04 a.m., Maxim Khutornenko wrote: src/main/python/apache/aurora/client/cli/update.py, line 122 https://reviews.apache.org/r/33959/diff/1/?file=952678#file952678line122 Suggest --wait-until for consistency with job create command. Bill Farner wrote: `--wait-until` accepts states, while this is a flag. IMHO the name should match the behavior. Maxim Khutornenko wrote: How about --wait then? This would still align well with `job create` and even shorter to type :) I'd be okay with that. Before i make the change - reviewers, what do you think? - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33959/#review82942 --- On May 7, 2015, 11:58 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33959/ --- (Updated May 7, 2015, 11:58 p.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Bugs: AURORA-1239 https://issues.apache.org/jira/browse/AURORA-1239 Repository: aurora Description --- Add update watch and update start --watch flag. Diffs - src/main/python/apache/aurora/client/api/__init__.py 0ae1d9fef0a7b190a51e8734c2abd71ecf3c5a32 src/main/python/apache/aurora/client/cli/update.py 8296db5434276910fc4f8470c7a962a8e8e0a9c2 src/test/python/apache/aurora/client/cli/test_supdate.py 6fb1f71b7dc787c090fce7b8e7c51baee862f336 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 501d111f87dbaff8b6f22ffc67a9720b4dad Diff: https://reviews.apache.org/r/33959/diff/ Testing --- Unit tests + end-to-end tests. Thanks, Bill Farner
Review Request 33962: Remove often-redundant Error executing command prefix from client output.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33962/ --- Review request for Aurora, Joshua Cohen and Maxim Khutornenko. Bugs: AURORA-1310 https://issues.apache.org/jira/browse/AURORA-1310 Repository: aurora Description --- See summary. Diffs - src/main/python/apache/aurora/client/cli/__init__.py 1208846b9202b600ada63c1cebfa95bf3e30b824 src/main/python/apache/aurora/client/cli/update.py 8296db5434276910fc4f8470c7a962a8e8e0a9c2 src/test/python/apache/aurora/client/cli/test_create.py 4de0ecba763af03a0b96900567a7c53853e548db src/test/python/apache/aurora/client/cli/test_kill.py 09734a7981b0f41abd0c8667caf2b121a3205853 src/test/python/apache/aurora/client/cli/test_supdate.py 6fb1f71b7dc787c090fce7b8e7c51baee862f336 Diff: https://reviews.apache.org/r/33962/diff/ Testing --- Unit tests. Thanks, Bill Farner
Re: Review Request 33959: Add update watch and update start --watch flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33959/ --- (Updated May 8, 2015, 12:44 a.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Changes --- neglected to git-add a file. Bugs: AURORA-1239 https://issues.apache.org/jira/browse/AURORA-1239 Repository: aurora Description --- Add update watch and update start --watch flag. Diffs (updated) - src/main/python/apache/aurora/client/api/__init__.py 0ae1d9fef0a7b190a51e8734c2abd71ecf3c5a32 src/main/python/apache/aurora/client/cli/update.py 8296db5434276910fc4f8470c7a962a8e8e0a9c2 src/test/python/apache/aurora/client/cli/test_supdate.py 6fb1f71b7dc787c090fce7b8e7c51baee862f336 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 501d111f87dbaff8b6f22ffc67a9720b4dad Diff: https://reviews.apache.org/r/33959/diff/ Testing --- Unit tests + end-to-end tests. Thanks, Bill Farner
Re: Review Request 33959: Add update watch and update start --watch flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33959/ --- (Updated May 8, 2015, 1:04 a.m.) Review request for Aurora, Kevin Sweeney and Zameer Manji. Changes --- rebase Bugs: AURORA-1239 https://issues.apache.org/jira/browse/AURORA-1239 Repository: aurora Description --- Add update watch and update start --watch flag. Diffs (updated) - src/main/python/apache/aurora/client/api/__init__.py 0ae1d9fef0a7b190a51e8734c2abd71ecf3c5a32 src/main/python/apache/aurora/client/cli/update.py 7bd1eb56e07f8d67e9bf62a0d46713cbaa91aa76 src/test/python/apache/aurora/client/cli/test_supdate.py 968d456927de8044b8a8c5ca365464a8fedd3711 src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh 501d111f87dbaff8b6f22ffc67a9720b4dad Diff: https://reviews.apache.org/r/33959/diff/ Testing --- Unit tests + end-to-end tests. Thanks, Bill Farner
Review Request 33905: Updgrade to gradle 2.4.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33905/ --- Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1306 https://issues.apache.org/jira/browse/AURORA-1306 Repository: aurora Description --- The code changes you see are to fix new checks introduced in the update findbugs plugin. Diffs - build.gradle 4fe4a1848742470e2f77e33c260d570acf3125ef buildSrc/gradle.properties 345242611f5fa646d82f8eaf2b05d78573e1434f config/findbugs/excludeFilter.xml 328e75c8b49696a45f113f5d28d6214f2e878ec3 gradle/wrapper/gradle-wrapper.properties 49c346d0697c648e0a91e9d27eecb02543a299a2 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 3d19831ea0eb85032172b096ac87e126701aa218 src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 40e0c840d761e7f4e30406c7f6b2666044dd0f37 src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java ebe551739fb6b7132ce666ad9b3c5a86e90a5363 Diff: https://reviews.apache.org/r/33905/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33869: Fix inconsistency in MemTaskStore secondary indices.
On May 6, 2015, 3:39 p.m., Maxim Khutornenko wrote: Mind explaining explaining the bug for everyone's benefit? Maxim Khutornenko wrote: Never mind, found the answer in the attached ticket. Hard to notice index.remove(key, Tasks.id(task)); bundled with unrelated stats refactoring. Sorry, that was not intentional - just the easiest way i saw to add a test case. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33869/#review82683 --- On May 5, 2015, 11:16 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33869/ --- (Updated May 5, 2015, 11:16 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1305 https://issues.apache.org/jira/browse/AURORA-1305 Repository: aurora Description --- In practice this amounts to little more than a space leak, but a bug nonetheless. Diffs - src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 9c76fa58b52bacd182e87cc4ebebad6c19356a55 src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java e3b13407cb6875489e50cf93212845eab7aacb89 src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java be57c5ee9f0218ed9fabb54b3579ba03477c8930 src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java d1f4026f2b3d1cba542f96cecc0fe490e9ec8132 Diff: https://reviews.apache.org/r/33869/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33612: Add a task store implementation that uses a relational database.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33612/ --- (Updated May 5, 2015, 6:21 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko. Changes --- Fixed copy-paste error + rebase. Bugs: AURORA-556 https://issues.apache.org/jira/browse/AURORA-556 Repository: aurora Description --- Add a task store implementation that uses a relational database. The meat of this review is in `DbTaskStore`, `TaskConfigManager`, and the mapper xml files. Some supporting actors include files under views/, which serve DB business objects. I suggest reviewers start by skimming `DbTaskStore` and digesting the comments in there. There are some known loose ends here, most notably being continued performance enhancements and cleanup of relations in different tables. I've included several relevant TODOs, ~all of which must be addressed before this becomes the default task store. Diffs (updated) - api/src/main/thrift/org/apache/aurora/gen/api.thrift 0182ecb3942cfa2d9dd21138779815f4500339be examples/vagrant/upstart/aurora-scheduler.conf cc4864c4d954d58e9bfaaaf5fc5afc8d0e032e78 src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java ed1171d52655fef643330f34913c256f77300fa2 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 3d19831ea0eb85032172b096ac87e126701aa218 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 94ce5c3499ced1b63abf19787acc21b2cd4d0c75 src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java c8df88b77b9a2017c48bdc0c9a0477927ba0b179 src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 1a6c3f21d4fcb476539f588facecd8ef37332837 src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/shims/ContainerShim.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/shims/TaskConstraintShim.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/ScheduleStatusTypeHandler.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java 4d0c10d60037a3310226a6fd8936b84ae4135e7e src/main/java/org/apache/aurora/scheduler/storage/db/views/AssignedPort.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/ScheduledTaskWrapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/TaskConfigRow.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/TaskLink.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/mem/InMemStoresModule.java 21f7d4db821930d2c5b52648e1996aa1ef12a85c src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f76f9a988669dab598e9d19f849018c6f55ce03e src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml PRE-CREATION src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml PRE-CREATION src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java afb7db8eefa63b84d370877742870acdec58899c src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java e3b13407cb6875489e50cf93212845eab7aacb89 src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java f18619a27eeb2aea8dcf01e54c23ed7d1c7d3d87 src/test/java/org/apache/aurora/scheduler/storage/mem/StorageTransactionTest.java bad9eb56b33c3e649c3b173e83d9c30da8f9317d Diff: https://reviews.apache.org/r/33612/diff/ Testing --- Unit tests and end-to-end tests, both using the new task store by default with this change. Thanks, Bill Farner
Review Request 33994: Change RC verification script to use a temp dir.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33994/ --- Review request for Aurora and Kevin Sweeney. Repository: aurora Description --- This sidesteps the too-long-shebang problem (see AURORA-1309), which remains and is likely to regress. Diffs - build-support/release/verify-release-candidate a0f717957d285c40ada3cd3b8a14f0dc11b6df1d Diff: https://reviews.apache.org/r/33994/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 34046: Upgade h2 to 1.4.187.
On May 11, 2015, 5:59 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 84 https://reviews.apache.org/r/34046/diff/1/?file=955485#file955485line84 Link to upstream ticket would be good here. AFAICT there is not an issue tracking this. H2 operates in a very single-developer sense, so there's not much in the way of formal project management for big milestones. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34046/#review83242 --- On May 11, 2015, 5:58 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34046/ --- (Updated May 11, 2015, 5:58 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Bugs: AURORA-1311 https://issues.apache.org/jira/browse/AURORA-1311 Repository: aurora Description --- Upgade h2 to 1.4.187. Diffs - build.gradle 829b81392d6eaf9f1eb7ae1ee89ff60b4fb32731 src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java c8df88b77b9a2017c48bdc0c9a0477927ba0b179 Diff: https://reviews.apache.org/r/34046/diff/ Testing --- I ran the benchmarks before and after this change. In nearly every case, performance is marginally better (though the difference is generally within 1 stddev). Note, however, that i needed to bump the heap size on the benchmark suite. This is because my first run encountered an OOM on one microbenchmark after the change. I have no evidence whether this is because the new H2 is more memory-hungry, or if that happens occasionally and it was poor luck. Benchmark results before (old H2 version): ``` SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark 551189.355 ± 14341.947 ops/s SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark 51502.506 ± 975.321 ops/s SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark 4427.202 ± 162.303 ops/s SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 52.291 ± 1.402 ops/s SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 50.159 ± 1.907 ops/s SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 52.891 ± 0.643 ops/s SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark 50.845 ± 0.689 ops/s SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark 51955.901 ± 1420.778 ops/s StatusUpdateBenchmark.runBenchmark 0.176 ± 0.019 ops/s StatusUpdateBenchmark.runBenchmark 0.038 ± 0.001 ops/s StatusUpdateBenchmark.runBenchmark 0.010 ± 0.001 ops/s ThriftApiBenchmarks.GetAllTasksBenchmark.run 5 2025.121 ± 30.865 ThriftApiBenchmarks.GetAllTasksBenchmark.run 5 115.166 ± 10.361 ThriftApiBenchmarks.GetAllTasksBenchmark.run 5 9.490 ± 4.451 ThriftApiBenchmarks.GetAllTasksBenchmark.run 5 0.241 ± 0.277 ThriftApiBenchmarks.GetAllTasksBenchmark.run 5 2010.467 ± 161.806 ThriftApiBenchmarks.GetAllTasksBenchmark.run 5 2009.343 ± 172.806 ThriftApiBenchmarks.GetAllTasksBenchmark.run 5 2006.227 ± 86.458 ThriftApiBenchmarks.GetAllTasksBenchmark.run 5 2008.623 ± 14.901 ThriftApiBenchmarks.GetAllTasksBenchmark.run 5 4368.957 ± 53.245 ThriftApiBenchmarks.GetAllTasksBenchmark.run 5 3932.150 ± 319.144 ThriftApiBenchmarks.GetAllTasksBenchmark.run 5 2069.126 ± 33.426 ThriftApiBenchmarks.GetAllTasksBenchmark.run 5 224.580 ± 10.336 ThriftApiBenchmarks.GetAllTasksBenchmark.run 5 19.286 ± 0.713 ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 5 6287.064 ± 156.134 ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 5 533.524 ± 101.918 ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 5 12.988 ± 0.856 ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 5 2.309 ± 0.105 ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 5 5581.662 ± 366.345 ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 5 5487.590 ± 179.228 ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 5 5337.478 ± 113.176 ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 5 5499.892 ± 166.960 ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 5 13288.963 ± 661.267 ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 5 11349.901 ± 422.637 ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 5 5984.420 ± 454.136 ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 5 995.175 ± 84.046 ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 5 23.697 ± 1.261 UpdateStoreBenchmarks.JobDetailsBenchmark.run 325.116 ± 48.465 ops/s UpdateStoreBenchmarks.JobDetailsBenchmark.run 71.977 ± 3.475 ops/s UpdateStoreBenchmarks.JobDetailsBenchmark.run 37.514 ± 2.345 ops/s ``` Benchmark results after (new H2 version): ``` SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark 551152.813
Re: Review Request 33689: Updated scheduler to process status updates asynchronously in batches.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33689/#review83304 --- Ship it! Overall LGTM. I'd like to see the move away from the polling loop if possible. See comment below. src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java https://reviews.apache.org/r/33689/#comment134239 This comment was slightly misleading, and probably belongs down near `continue;`, to communicate that the timeout was met and we are going to try again if the service is still running. However, i don't see why you can't get access to the thread. You could get it from `Thread.currentThread()` here, and use an `AtomicReference` to keep a ref. src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java https://reviews.apache.org/r/33689/#comment134246 The histogram would be interesting, but possibly overkill. We should be able to get good signal with stats we already have - number of status updates vs number of log writes - Bill Farner On May 11, 2015, 6:55 p.m., Ben Mahler wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33689/ --- (Updated May 11, 2015, 6:55 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-1228 https://issues.apache.org/jira/browse/AURORA-1228 Repository: aurora Description --- Now the processing of status updates is done asynchronously with batching to insulate throughput from the expensive storage resource. Updates are placed into a queue and consumed by another thread. If many updates arrive while we're storing a batch of updates, these will be processed together in batch rather than individually. Diffs - src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 7bb64dd913f0fe2fede95d50a061043dbb794ab4 src/jmh/java/org/apache/aurora/benchmark/fakes/FakeDriver.java 45de15a57baf7a2f7d437b590935714e28777f35 src/main/java/org/apache/aurora/scheduler/SchedulerModule.java d3ac176e9402a33fd2074b0737313458120da9e2 src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 0ce9c9d4cf75f9add260f285115b1d60786ded57 src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 4d589a33a2933b0cb6caf85abfae45c5e635c3ce src/main/java/org/apache/aurora/scheduler/mesos/Driver.java c7e45a89ceaa2c310feb610091eec0b04187860e src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 9b8ab7c1027731f9d3f6cae77b85272ea63354d4 src/main/java/org/apache/aurora/scheduler/mesos/SchedulerDriverService.java da2d5df2e053e6e1b8fb08d6813dff9eac9777f8 src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 422d5a9a42310979752eb7282658316c2b772419 src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java abdeee49858fc439c27911c4eb544bf8e8c931d4 Diff: https://reviews.apache.org/r/33689/diff/ Testing --- Ran the benchmark to confirm that this improves status update throughput substantially: Before: Around 100 updates per second for a 5ms storage latency. Much worse for higher latencies. After: Around 4k-5k updates per second for a 5ms storage latency, down to 3k updates per second for 100ms storage latency. Updated unit tests for the new invariants: * TaskLaunchers are responsible for acknowledging updates. * UserTaskLauncher processes updates asynchronously. Thanks, Ben Mahler
Re: Review Request 34015: Improve client update json output to consistently use last_modified
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34015/#review83331 --- Ship it! Ship It! - Bill Farner On May 11, 2015, 10:19 p.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34015/ --- (Updated May 11, 2015, 10:19 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1316 https://issues.apache.org/jira/browse/AURORA-1316 Repository: aurora Description --- Improve client update json output to consistently use last_modified Diffs - src/main/python/apache/aurora/client/cli/update.py 58f0db0988911858156aa086ba9fea8ecfe99143 src/test/python/apache/aurora/client/cli/test_supdate.py 158d3305c5ce5f1de5e3f4f771f318cff9bf7914 Diff: https://reviews.apache.org/r/34015/diff/ Testing --- $ ./pants test.pytest --no-fast ./src/test/python/apache/aurora/client:all src.test.python.apache.aurora.client.api.api . SUCCESS src.test.python.apache.aurora.client.api.instance_watcher . SUCCESS src.test.python.apache.aurora.client.api.job_monitor . SUCCESS src.test.python.apache.aurora.client.api.mux . SUCCESS src.test.python.apache.aurora.client.api.quota_check . SUCCESS src.test.python.apache.aurora.client.api.restarter . SUCCESS src.test.python.apache.aurora.client.api.scheduler_client . SUCCESS src.test.python.apache.aurora.client.api.sla . SUCCESS src.test.python.apache.aurora.client.api.task_util . SUCCESS src.test.python.apache.aurora.client.api.updater . SUCCESS src.test.python.apache.aurora.client.api.updater_util . SUCCESS src.test.python.apache.aurora.client.base . SUCCESS src.test.python.apache.aurora.client.binding_helper . SUCCESS src.test.python.apache.aurora.client.cli.api . SUCCESS src.test.python.apache.aurora.client.cli.client . SUCCESS src.test.python.apache.aurora.client.cli.command_hooks . SUCCESS src.test.python.apache.aurora.client.cli.config . SUCCESS src.test.python.apache.aurora.client.cli.context . SUCCESS src.test.python.apache.aurora.client.cli.cron . SUCCESS src.test.python.apache.aurora.client.cli.inspect . SUCCESS src.test.python.apache.aurora.client.cli.job . SUCCESS src.test.python.apache.aurora.client.cli.plugins . SUCCESS src.test.python.apache.aurora.client.cli.quota . SUCCESS src.test.python.apache.aurora.client.cli.sla . SUCCESS src.test.python.apache.aurora.client.cli.supdate . SUCCESS src.test.python.apache.aurora.client.cli.task . SUCCESS src.test.python.apache.aurora.client.cli.update . SUCCESS src.test.python.apache.aurora.client.cli.version . SUCCESS src.test.python.apache.aurora.client.config . SUCCESS src.test.python.apache.aurora.client.hooks.hooked_api . SUCCESS src.test.python.apache.aurora.client.hooks.non_hooked_api . SUCCESS Thanks, Joe Smith
Re: Review Request 33612: Add a task store implementation that uses a relational database.
On May 7, 2015, 10:22 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java, lines 102-106 https://reviews.apache.org/r/33612/diff/2/?file=950462#file950462line102 It'd be nice if this was extracted out to a higher level so it was reusable across DB store impls without needing to be repeated in each method. Maybe replace the usage of the TimedInterceptor with a new DBTimedInterceptor or some such? (If you think this is a good idea I'm fine with just dropping a TODO and revisiting this in a follow up). Bill Farner wrote: Unless you feel strongly, i would like to punt. I'm not too keen on building out abstractions for multiple store implementations, since multiple implementations is intended to be temporary. Joshua Cohen wrote: I think it will be very useful in the future to have a generic mechanism for identifying slow queries, but I don't feel strongly that it needs to be part of this change (other than the fact that you've implemented it in one place). I'm fine with a ticket tracking the change for the future if you agree that it would be good to have? I'm not sure i follow - you mean slow queries _in a task store_, or slow method execution? - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33612/#review82915 --- On May 9, 2015, 5:53 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33612/ --- (Updated May 9, 2015, 5:53 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko. Bugs: AURORA-556 https://issues.apache.org/jira/browse/AURORA-556 Repository: aurora Description --- Add a task store implementation that uses a relational database. The meat of this review is in `DbTaskStore`, `TaskConfigManager`, and the mapper xml files. Some supporting actors include files under views/, which serve DB business objects. I suggest reviewers start by skimming `DbTaskStore` and digesting the comments in there. There are some known loose ends here, most notably being continued performance enhancements and cleanup of relations in different tables. I've included several relevant TODOs, ~all of which must be addressed before this becomes the default task store. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 0182ecb3942cfa2d9dd21138779815f4500339be examples/vagrant/upstart/aurora-scheduler.conf cc4864c4d954d58e9bfaaaf5fc5afc8d0e032e78 src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 88d27dd729fd004d1510917a031591addba51816 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 239c61625bc49e53be918c59056f071b3b6b86b9 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java ea5600725d5dd84d21ca8d37b560c6d41541d016 src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java c8df88b77b9a2017c48bdc0c9a0477927ba0b179 src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 1a6c3f21d4fcb476539f588facecd8ef37332837 src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/shims/ContainerShim.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/shims/TaskConstraintShim.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/ScheduleStatusTypeHandler.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java 4d0c10d60037a3310226a6fd8936b84ae4135e7e src/main/java/org/apache/aurora/scheduler/storage/db/views/AssignedPort.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/ScheduledTaskWrapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/TaskConfigRow.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/TaskLink.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/mem/InMemStoresModule.java 21f7d4db821930d2c5b52648e1996aa1ef12a85c src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f76f9a988669dab598e9d19f849018c6f55ce03e src/main/resources/org/apache/aurora/scheduler/storage/db
Re: Review Request 34020: Normalize SLA stat names
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34020/#review83302 --- As Maxim pointed out on the ticket, there is a compatibility concern that should be addressed. If you don't mind, i'd like to conclude that discussion before reviewing. - Bill Farner On May 9, 2015, 9:54 p.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34020/ --- (Updated May 9, 2015, 9:54 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1282 https://issues.apache.org/jira/browse/AURORA-1282 Repository: aurora Description --- Normalize SLA stat names This is going to change these metrics from `sla_role/env/job_job_uptime_50.00_sec` to `sla_role_env_job_job_uptime_50_00_sec` Diffs - src/main/java/org/apache/aurora/scheduler/sla/MetricCalculator.java 82f36d5ca15df18bdc8ebbbd868d3394db38e603 src/test/java/org/apache/aurora/scheduler/sla/MetricCalculatorTest.java cb98834e925793fc116814371548a30470830164 Diff: https://reviews.apache.org/r/34020/diff/ Testing --- [tw-mbp-jsmith aurora (stats_normalize)]$ ./gradlew test BUILD SUCCESSFUL Thanks, Joe Smith
Re: Review Request 33612: Add a task store implementation that uses a relational database.
On May 7, 2015, 10:22 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, lines 207-208 https://reviews.apache.org/r/33612/diff/2/?file=950460#file950460line207 Doesn't need to be javadoc. Filled out with a comment. On May 7, 2015, 10:22 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java, lines 102-106 https://reviews.apache.org/r/33612/diff/2/?file=950462#file950462line102 It'd be nice if this was extracted out to a higher level so it was reusable across DB store impls without needing to be repeated in each method. Maybe replace the usage of the TimedInterceptor with a new DBTimedInterceptor or some such? (If you think this is a good idea I'm fine with just dropping a TODO and revisiting this in a follow up). Unless you feel strongly, i would like to punt. I'm not too keen on building out abstractions for multiple store implementations, since multiple implementations is intended to be temporary. On May 7, 2015, 10:22 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java, lines 169-174 https://reviews.apache.org/r/33612/diff/2/?file=950462#file950462line169 Would it make sense to use a LoadingCache for this to simplify it a bit, rather than explicitly inserting on a cache miss, we could rely on the LoadingCache to either return the existing config id or insert one and return the new id? I went down this path because of the runtime checks offered by `BiMap`, but you've convinced me that it's not worth it. Switched to LoadingCache, thanks for the suggestion! On May 7, 2015, 10:22 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java, line 264 https://reviews.apache.org/r/33612/diff/2/?file=950462#file950462line264 nit: kill else Done. On May 7, 2015, 10:22 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java, line 127 https://reviews.apache.org/r/33612/diff/2/?file=950463#file950463line127 This seems like it may be important to have (without it we may leave stale data behind)? Are you comfortable shipping this without properly cleaning up? I'm not comfortable using it in production as-is, but wanted to draw the line somewhere w.r.t. patch size. I'd also like to have a scoped discussion about what our strategy should be here, since it's a pattern that applies to several other tables. The review addressing this TODO will be a great forum for that discussion. On May 7, 2015, 10:22 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java, line 30 https://reviews.apache.org/r/33612/diff/2/?file=950465#file950465line30 I'm totally unclear on our guidelines for javadoc. Usually on interface methods we'd require javadoc, does that not hold here for some reason since it's a MyBatis mapper? Added docs. TBH i'm not certain they're valuable in this context, but we have them on other mappers so i will choose to conform. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33612/#review82915 --- On May 5, 2015, 6:21 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33612/ --- (Updated May 5, 2015, 6:21 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko. Bugs: AURORA-556 https://issues.apache.org/jira/browse/AURORA-556 Repository: aurora Description --- Add a task store implementation that uses a relational database. The meat of this review is in `DbTaskStore`, `TaskConfigManager`, and the mapper xml files. Some supporting actors include files under views/, which serve DB business objects. I suggest reviewers start by skimming `DbTaskStore` and digesting the comments in there. There are some known loose ends here, most notably being continued performance enhancements and cleanup of relations in different tables. I've included several relevant TODOs, ~all of which must be addressed before this becomes the default task store. Diffs - api/src/main/thrift/org/apache/aurora/gen/api.thrift 0182ecb3942cfa2d9dd21138779815f4500339be examples/vagrant/upstart/aurora-scheduler.conf cc4864c4d954d58e9bfaaaf5fc5afc8d0e032e78 src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java ed1171d52655fef643330f34913c256f77300fa2 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 3d19831ea0eb85032172b096ac87e126701aa218
Re: Review Request 33997: Improve GPG key validation for release verification script
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33997/#review83119 --- Ship it! Ship It! - Bill Farner On May 9, 2015, 12:11 a.m., Joe Smith wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33997/ --- (Updated May 9, 2015, 12:11 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Improve GPG key validation for release verification script Diffs - build-support/release/verify-release-candidate f96d7c9239895e86d715ecec3c47b9ba828dbc30 Diff: https://reviews.apache.org/r/33997/diff/ Testing --- Thanks, Joe Smith
Re: Review Request 33612: Add a task store implementation that uses a relational database.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33612/ --- (Updated May 12, 2015, 9:17 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko. Changes --- Address comments + rebase Bugs: AURORA-556 https://issues.apache.org/jira/browse/AURORA-556 Repository: aurora Description --- Add a task store implementation that uses a relational database. The meat of this review is in `DbTaskStore`, `TaskConfigManager`, and the mapper xml files. Some supporting actors include files under views/, which serve DB business objects. I suggest reviewers start by skimming `DbTaskStore` and digesting the comments in there. There are some known loose ends here, most notably being continued performance enhancements and cleanup of relations in different tables. I've included several relevant TODOs, ~all of which must be addressed before this becomes the default task store. Diffs (updated) - api/src/main/thrift/org/apache/aurora/gen/api.thrift 0182ecb3942cfa2d9dd21138779815f4500339be examples/vagrant/upstart/aurora-scheduler.conf cc4864c4d954d58e9bfaaaf5fc5afc8d0e032e78 src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 88d27dd729fd004d1510917a031591addba51816 src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 239c61625bc49e53be918c59056f071b3b6b86b9 src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java ea5600725d5dd84d21ca8d37b560c6d41541d016 src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 8859ca47088896a1814321147c6b4c31828b3db9 src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 1a6c3f21d4fcb476539f588facecd8ef37332837 src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/DbUtil.java 7b4067cfe10d8aa1e9e0e8bfbcd0e83c9beb4e7a src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/shims/ContainerShim.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/shims/TaskConstraintShim.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/ScheduleStatusTypeHandler.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java 4d0c10d60037a3310226a6fd8936b84ae4135e7e src/main/java/org/apache/aurora/scheduler/storage/db/views/AssignedPort.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/ScheduledTaskWrapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/TaskConfigRow.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/TaskLink.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/mem/InMemStoresModule.java 21f7d4db821930d2c5b52648e1996aa1ef12a85c src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml f76f9a988669dab598e9d19f849018c6f55ce03e src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml PRE-CREATION src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml PRE-CREATION src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java afb7db8eefa63b84d370877742870acdec58899c src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 6a6ff27d8d0f1e537a74c1df981b97e5d8b2f2e8 src/test/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStoreTest.java 7d856d020da854c125c037f01357e81de93895e1 src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java 8f139fc987a98ef0d7f2969720134729411b8b84 src/test/java/org/apache/aurora/scheduler/storage/mem/StorageTransactionTest.java bad9eb56b33c3e649c3b173e83d9c30da8f9317d Diff: https://reviews.apache.org/r/33612/diff/ Testing --- Unit tests and end-to-end tests, both using the new task store by default with this change. Thanks, Bill Farner
Re: Review Request 33612: Add a task store implementation that uses a relational database.
On May 12, 2015, 8:09 p.m., Kevin Sweeney wrote: api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 265 https://reviews.apache.org/r/33612/diff/2/?file=950455#file950455line265 This seems unrelated to the description in this diff. It is related, as we don't have a `getContainer()` method to use in the diff without this change (or a fix to the linked bug). On May 12, 2015, 8:09 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 131 https://reviews.apache.org/r/33612/diff/2/?file=950460#file950460line131 Presumably eager cleanup is expensive performance-wise. If that's the case would you mind calling that out explicitly in the comment? Are you assuming i added a close delay because of perf? If so, that's not the reason - please see the previous comment rounds with Maxim. On May 12, 2015, 8:09 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java, line 154 https://reviews.apache.org/r/33612/diff/2/?file=950462#file950462line154 Is there an explicit design choice driving an DELETE+INSERT combination rather than an UPDATE+INSERT? If so can you call it out? If not can you drop a TODO to evaluate the tradeoffs of the switch? This was discussed in previous rounds with Maxim, and a TODO exists on line 148. Please let me know if there's something more you're after. On May 12, 2015, 8:09 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java, lines 259-262 https://reviews.apache.org/r/33612/diff/2/?file=950462#file950462line259 Same question as above here - why DELETE+INSERT instead of UPDATE? ^^ On May 12, 2015, 8:09 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/storage/db/shims/ContainerShim.java, line 24 https://reviews.apache.org/r/33612/diff/2/?file=950466#file950466line24 This will lock in our thrift version when these become final and seems brittle (easy to add a new field to Container and have it compile fine) - can you file a ticket to investigate alternatives to this pattern? To be honest, i feel as though i already exhausted the options before getting here. AFAICT the alternatives are non-trivial: change the thrift codegen and upgrade, or avoid use of thrift for database model objects. I won't coerce you against filing a ticket, but i'd prefer not to since i doubt it will be addressed independently. On May 12, 2015, 8:09 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java, line 77 https://reviews.apache.org/r/33612/diff/2/?file=950462#file950462line77 Pull this CmdLine arg up to a module? Ok On May 12, 2015, 8:09 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java, line 99 https://reviews.apache.org/r/33612/diff/2/?file=950462#file950462line99 Inject a Clock here? Ok On May 12, 2015, 8:09 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/storage/db/shims/ContainerShim.java, line 27 https://reviews.apache.org/r/33612/diff/2/?file=950466#file950466line27 `isSet(_Fields.DOCKER)` Done. On May 12, 2015, 8:09 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/storage/db/shims/ContainerShim.java, line 36 https://reviews.apache.org/r/33612/diff/2/?file=950466#file950466line36 Use isSet Done. On May 12, 2015, 8:09 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/storage/db/shims/TaskConstraintShim.java, line 36 https://reviews.apache.org/r/33612/diff/2/?file=950467#file950467line36 isSet Done. On May 12, 2015, 8:09 p.m., Kevin Sweeney wrote: src/main/java/org/apache/aurora/scheduler/storage/db/shims/TaskConstraintShim.java, line 27 https://reviews.apache.org/r/33612/diff/2/?file=950467#file950467line27 isSet Done. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33612/#review82623 --- On May 9, 2015, 5:53 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33612/ --- (Updated May 9, 2015, 5:53 p.m.) Review request for Aurora, Joshua Cohen, Kevin Sweeney, and Maxim Khutornenko. Bugs: AURORA-556 https://issues.apache.org/jira/browse/AURORA-556 Repository: aurora Description --- Add a task store implementation that uses a relational database. The meat of this review is in `DbTaskStore`, `TaskConfigManager`, and the mapper xml files. Some supporting actors include files under views/, which serve DB
Re: Review Request 34121: Disable state transition logging for unknown tasks.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34121/#review83481 --- What's the motivation here? Seems like attempted state transitions for unknown tasks should be transient and/or represent a bug. In those cases, i would assume we definitely want logging. Is there somthing i'm overlooking? src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java https://reviews.apache.org/r/34121/#comment134492 This is pretty weird behavior - 'no logging unless you ask really nicely'. Rather than the enableLogging flag, how about we let the caller pass a Logger, and for the finest-only logging, we have a logger that has its level set appropriately? - Bill Farner On May 12, 2015, 8:40 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34121/ --- (Updated May 12, 2015, 8:40 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Quieting down state transition logging in preparation for task state reconciliation. Diffs - src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 2a943cf51d0a41260ada6965cea5f55db4c3f846 src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 4a7ca6267ae6c4c062f941c78a4a3496d110e8c2 src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java afb7db8eefa63b84d370877742870acdec58899c src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java b30a0737bb0d60cd8b58f7be0fff5db20f808347 Diff: https://reviews.apache.org/r/34121/diff/ Testing --- ./gradlew -Pq build ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh Thanks, Maxim Khutornenko
Review Request 34126: Use JDK 8 language features.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34126/ --- Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Bugs: AURORA-274 https://issues.apache.org/jira/browse/AURORA-274 Repository: aurora Description --- Two code changes were required: - a unit test made an assumption about iteration order over a `HashSet` - a new compiler warning showed up on possible dereference of a null pointer Note the first introduction of a lambda as a PoC while modifying a unit test. Diffs - NEWS PRE-CREATION build.gradle ec9b52206ce602d0f4a0a6d4b7fe55d055a787dc examples/vagrant/provision-dev-cluster.sh 2c5ce970446f8849b66d7035b8b10b253827db83 src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 218ae0da80f9129107da17004f23098b9507b33d src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java d1301e0ea3c255563709048247772648052e8041 Diff: https://reviews.apache.org/r/34126/diff/ Testing --- ``` ./build-support/jenkins/build.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh # TODO(wfarner): Report back, still executing. ``` Thanks, Bill Farner
Re: Review Request 34126: Use JDK 8 language features.
On May 12, 2015, 11:14 p.m., Aurora ReviewBot wrote: Master (c1b0dce) is red with this patch. ./build-support/jenkins/build.sh make[4]: Entering directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.9.1/tutorial' ../compiler/cpp/thrift --gen html -r ../tutorial/tutorial.thrift make[4]: Leaving directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.9.1/tutorial' make[3]: Leaving directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.9.1/tutorial' Making all in tutorial make[3]: Entering directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.9.1/tutorial' make[4]: Entering directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.9.1/tutorial' ../compiler/cpp/thrift --gen html -r ../tutorial/tutorial.thrift make[4]: Leaving directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.9.1/tutorial' make[3]: Leaving directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.9.1/tutorial' make[3]: Entering directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.9.1' make[3]: Leaving directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.9.1' make[2]: Leaving directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.9.1' make[1]: Leaving directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift/thrift-0.9.1' make: Leaving directory `/home/jenkins/jenkins-slave/workspace/AuroraBot/build-support/thrift' :api:classesThriftNote: Some input files use unchecked or unsafe operations. Note: Recompile with -Xlint:unchecked for details. :api:checkPython :api:generateThriftEntitiesJava :api:classesThriftEntities :api:compileJava UP-TO-DATE :api:generateThriftResources :api:processResources UP-TO-DATE :api:classes :api:jar :compileJava FAILED FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':compileJava'. invalid source release: 1.8 * Try: Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. BUILD FAILED Total time: 1 mins 22.51 secs I will refresh this build result if you post a review containing @ReviewBot retry Note: this is expected while we straddle the line between 7 and 8. After this lands, i will update the ReviewBot build to use 8. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34126/#review83515 --- On May 12, 2015, 10:54 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34126/ --- (Updated May 12, 2015, 10:54 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Bugs: AURORA-274 https://issues.apache.org/jira/browse/AURORA-274 Repository: aurora Description --- Two code changes were required: - a unit test made an assumption about iteration order over a `HashSet` - a new compiler warning showed up on possible dereference of a null pointer Note the first introduction of a lambda as a PoC while modifying a unit test. Diffs - NEWS PRE-CREATION build.gradle ec9b52206ce602d0f4a0a6d4b7fe55d055a787dc examples/vagrant/provision-dev-cluster.sh 2c5ce970446f8849b66d7035b8b10b253827db83 src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 218ae0da80f9129107da17004f23098b9507b33d src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java d1301e0ea3c255563709048247772648052e8041 Diff: https://reviews.apache.org/r/34126/diff/ Testing --- ``` ./build-support/jenkins/build.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh # TODO(wfarner): Report back, still executing. ``` Thanks, Bill Farner
Re: Review Request 34126: Use JDK 8 language features.
On May 12, 2015, 11:08 p.m., Kevin Sweeney wrote: examples/vagrant/provision-dev-cluster.sh, line 33 https://reviews.apache.org/r/34126/diff/1/?file=957021#file957021line33 comment is a lie now, but it seems safe to remove as the update-alternatives line is self-explanatory Thanks, removed. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34126/#review83513 --- On May 12, 2015, 11:41 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34126/ --- (Updated May 12, 2015, 11:41 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Bugs: AURORA-274 https://issues.apache.org/jira/browse/AURORA-274 Repository: aurora Description --- Two code changes were required: - a unit test made an assumption about iteration order over a `HashSet` - a new compiler warning showed up on possible dereference of a null pointer Note the first introduction of a lambda as a PoC while modifying a unit test. Diffs - NEWS PRE-CREATION build.gradle ec9b52206ce602d0f4a0a6d4b7fe55d055a787dc examples/vagrant/provision-dev-cluster.sh 2c5ce970446f8849b66d7035b8b10b253827db83 src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 218ae0da80f9129107da17004f23098b9507b33d src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java d1301e0ea3c255563709048247772648052e8041 Diff: https://reviews.apache.org/r/34126/diff/ Testing --- ``` ./build-support/jenkins/build.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ``` Thanks, Bill Farner
Re: Review Request 34126: Use JDK 8 language features.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34126/ --- (Updated May 12, 2015, 11:42 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Bugs: AURORA-274 https://issues.apache.org/jira/browse/AURORA-274 Repository: aurora Description --- Two code changes were required: - a unit test made an assumption about iteration order over a `HashSet` - a new compiler warning showed up on possible dereference of a null pointer Note the first introduction of a lambda as a PoC while modifying a unit test. Diffs (updated) - NEWS PRE-CREATION build.gradle ec9b52206ce602d0f4a0a6d4b7fe55d055a787dc examples/vagrant/provision-dev-cluster.sh 2c5ce970446f8849b66d7035b8b10b253827db83 src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java 218ae0da80f9129107da17004f23098b9507b33d src/test/java/org/apache/aurora/scheduler/storage/backup/StorageBackupTest.java d1301e0ea3c255563709048247772648052e8041 Diff: https://reviews.apache.org/r/34126/diff/ Testing --- ``` ./build-support/jenkins/build.sh ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh ``` Thanks, Bill Farner
Re: Review Request 34148: Enhancing the StateManager.changeState result.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34148/#review83623 --- Ship it! src/main/java/org/apache/aurora/scheduler/state/StateChangeResult.java https://reviews.apache.org/r/34148/#comment134627 s/Describes t/T/ src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java https://reviews.apache.org/r/34148/#comment134628 IMHO use of nested ternary reduces readability. Can you break the top-level out into if/else? - Bill Farner On May 13, 2015, 1:43 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34148/ --- (Updated May 13, 2015, 1:43 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Adding more details into task state change result to facilitate task reconciliation data collection. Diffs - src/main/java/org/apache/aurora/scheduler/async/TaskTimeout.java 90e614958dfa992921e05cff86ddcc434efdd112 src/main/java/org/apache/aurora/scheduler/state/StateChangeResult.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/state/StateManager.java 71bfefb8cff3e9ad1fa9566ba55c0e3541fb01f3 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java d87bb3818ae950125a54ff63d2ba52bfc67e6708 src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 4a7ca6267ae6c4c062f941c78a4a3496d110e8c2 src/main/java/org/apache/aurora/scheduler/state/TransitionResult.java 874c554f84ea9290aa0d3874241da1b23dd453a7 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 160db129578365e0dd67d3354d98497f567dd621 src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java f17c43475a09bf0bbbcc49a3b372484e7937c27f src/test/java/org/apache/aurora/scheduler/async/TaskThrottlerTest.java a637101d0f01865dc2b3f0ee00aca81d0fbf0490 src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java 88fc172be6c24fefb6f708ce757487082542 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 32d18a9b8af2ac04b0f82fe866c3eed7e923584d src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java 831803f0bf8000bd88fe870b6151ceca59c620fa src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java 7b101bc2bb5f6f1854187aa33406049a94fbb2fd src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 15e4d388795b2ab2723373a73b419878b6346456 src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java afbca61cb6a4b0a81346c496fa21077bda3c13de src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 1ac1a2838a374383d3190d1fc5b4782d03f1d826 src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 4e7ff3b3b2e12b43df157b1af6548db306c141da Diff: https://reviews.apache.org/r/34148/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 34169: Add a type witness to satisfy JDK 8U11 compiler.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34169/ --- Review request for Aurora and Maxim Khutornenko. Repository: aurora Description --- This may give us a green build until we can get the JDK8 upgraded on jenkins machines. Diffs - src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java c9153e1539e5f71258b64fff19f438fe492d9600 Diff: https://reviews.apache.org/r/34169/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 34148: Enhancing the StateManager.changeState result.
On May 13, 2015, 1:53 a.m., Aurora ReviewBot wrote: Master (bf7f9b7) is red with this patch. ./build-support/jenkins/build.sh :distZip :assemble :compileJmhJavaNote: /home/jenkins/jenkins-slave/workspace/AuroraBot/src/jmh/java/org/apache/aurora/benchmark/fakes/FakeSchedulerDriver.java uses or overrides a deprecated API. Note: Recompile with -Xlint:deprecation for details. :processJmhResources UP-TO-DATE :jmhClasses :checkstyleJmh :jsHint :checkstyleMain :compileTestJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java:81: error: method addAction in interface ShutdownRegistry cannot be applied to given types; shutdownRegistry.addAction(capture(shutdownCommand)); ^ required: T#1 found: ExceptionalCommandCAP#1 reason: inference variable T#2 has incompatible bounds equality constraints: ExceptionalCommand? upper bounds: ExceptionalCommandCAP#2,T#1,Object where T#1,E,T#2 are type-variables: T#1 extends ExceptionalCommandE declared in method E,T#1addAction(T#1) E extends Exception declared in method E,T#1addAction(T#1) T#2 extends Object declared in method T#2capture(CaptureT#2) where CAP#1,CAP#2 are fresh type-variables: CAP#1 extends Exception from capture of ? CAP#2 extends Exception from capture of ? 1 error FAILED FAILURE: Build failed with an exception. * What went wrong: Execution failed for task ':compileTestJava'. Compilation failed; see the compiler error output for details. * Try: Run with --stacktrace option to get the stack trace. Run with --info or --debug option to get more log output. BUILD FAILED Total time: 1 mins 51.943 secs I will refresh this build result if you post a review containing @ReviewBot retry Maxim Khutornenko wrote: Review bot still using JDK 7? I encountered this with JDK ~8U11 (which is the latest JDK 8 installed on the jenkins machines), but saw it go away with 8U45. This is on msater, so iterating here for a fix: https://reviews.apache.org/r/34169/ - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34148/#review83545 --- On May 13, 2015, 1:43 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/34148/ --- (Updated May 13, 2015, 1:43 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Adding more details into task state change result to facilitate task reconciliation data collection. Diffs - src/main/java/org/apache/aurora/scheduler/async/TaskTimeout.java 90e614958dfa992921e05cff86ddcc434efdd112 src/main/java/org/apache/aurora/scheduler/state/StateChangeResult.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/state/StateManager.java 71bfefb8cff3e9ad1fa9566ba55c0e3541fb01f3 src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java d87bb3818ae950125a54ff63d2ba52bfc67e6708 src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 4a7ca6267ae6c4c062f941c78a4a3496d110e8c2 src/main/java/org/apache/aurora/scheduler/state/TransitionResult.java 874c554f84ea9290aa0d3874241da1b23dd453a7 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 160db129578365e0dd67d3354d98497f567dd621 src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 32432322753799562d671db39c0d7fa308d962ff src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java f17c43475a09bf0bbbcc49a3b372484e7937c27f src/test/java/org/apache/aurora/scheduler/async/TaskThrottlerTest.java a637101d0f01865dc2b3f0ee00aca81d0fbf0490 src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java 88fc172be6c24fefb6f708ce757487082542 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java 32d18a9b8af2ac04b0f82fe866c3eed7e923584d src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java 831803f0bf8000bd88fe870b6151ceca59c620fa src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java 7b101bc2bb5f6f1854187aa33406049a94fbb2fd src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 15e4d388795b2ab2723373a73b419878b6346456 src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java afbca61cb6a4b0a81346c496fa21077bda3c13de src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 1ac1a2838a374383d3190d1fc5b4782d03f1d826 src/test
Review Request 33273: Add a specific storage routine for bulk loading data.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33273/ --- Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Repository: aurora Description --- This behavior is primarily for DbStorage, which will benefit from a signal that a mutate transaction is a bulk data load, so that it can relax transactional guarantees in the interest of recovery speed. Diffs - src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java 07d81e428342b35be019aa2046c7f1554393527f src/main/java/org/apache/aurora/scheduler/storage/Storage.java 972a3c1f094dc62ebf1d35074485441181d7fe0d src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java 526df103883fd720cb6c00f8dff1bb3cf7cead38 src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 63b5b1f87ba12edbc5ad2546189dff420409d645 src/main/java/org/apache/aurora/scheduler/storage/mem/MemStorage.java dafe1c4f7c184c41dba0360f876dd8381f4aeb59 src/test/java/org/apache/aurora/scheduler/app/local/FakeNonVolatileStorage.java 3336f8cddb2a7a6e8c9e4bc6664708342ab97979 src/test/java/org/apache/aurora/scheduler/storage/db/DbStorageTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java cb6ba25c18878b751fcaffe15977b1ca10c74d65 Diff: https://reviews.apache.org/r/33273/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33200: Require non-default primitive values in StorageEntityUtil, extract a task factory utility.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33200/#review80362 --- Ping? - Bill Farner On April 15, 2015, 12:04 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33200/ --- (Updated April 15, 2015, 12:04 a.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Repository: aurora Description --- This is a ripple from changing StorageEntityUtil.assertFullyPopulated to require that all primitive fields are using non-default values. This tightens up the validation that a round-tripped object is not silently dropping field values. Diffs - src/jmh/java/org/apache/aurora/benchmark/Tasks.java 23827ab92367f3310c67d213bc8848cbd565234b src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java b0cced750fe767f6067d0de291677f417c543919 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 858069ea8d7393bd82260693ca8499e9569b50ef src/test/java/org/apache/aurora/scheduler/base/JobsTest.java a170fb655a4d1877c66b3e27e272647b989e5409 src/test/java/org/apache/aurora/scheduler/base/TaskTestUtil.java b8b9c1b8cdf7b641976c583a71fdd9b0c14e6e5f src/test/java/org/apache/aurora/scheduler/base/TasksTest.java 2866b20c1a88f6012a6f1c8eec33e450c3510f80 src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 7234fd3dfceec4d916807559d5b6604db0864d46 src/test/java/org/apache/aurora/scheduler/stats/ResourceCounterTest.java e5f3adfa61da665edb063b01b727f75455b0ad02 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 90a05baa2a1d4fcd73d24d539a0453d6917ac2f6 src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 5853c037d53e707e5df434fc661cd285ed9ecfc4 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1f505153f36b355718c778af230a6db4168505a5 src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java 83b238f3c0d218f35db098cf444c08d72798a7a3 src/test/java/org/apache/aurora/scheduler/storage/testing/StorageEntityUtil.java b26ddd39f7d972bba41aa72283d0b0626ed0fe64 src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 3f1f72bcf214b8ab9b3d4528ff0eb403c82db00c src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java dd4c94097161477e8161fb7e440e4c41e2a61f21 src/test/java/org/apache/aurora/scheduler/updater/TaskUtil.java 0e67f91536ff89c07da9be82049719c854aa3d62 Diff: https://reviews.apache.org/r/33200/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33200: Require non-default primitive values in StorageEntityUtil, extract a task factory utility.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33200/ --- (Updated April 16, 2015, 9:56 p.m.) Review request for Aurora, Maxim Khutornenko and Zameer Manji. Repository: aurora Description (updated) --- This is a ripple from changing StorageEntityUtil.assertFullyPopulated to require that all primitive fields are using non-default values. This tightens up the validation that a round-tripped object is not silently dropping field values. To centralize this, many tests that had their own custom factory methods have been changed to use `TaskTestUtil`. Diffs - src/jmh/java/org/apache/aurora/benchmark/Tasks.java 23827ab92367f3310c67d213bc8848cbd565234b src/main/java/org/apache/aurora/scheduler/base/TaskTestUtil.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java b0cced750fe767f6067d0de291677f417c543919 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 858069ea8d7393bd82260693ca8499e9569b50ef src/test/java/org/apache/aurora/scheduler/base/JobsTest.java a170fb655a4d1877c66b3e27e272647b989e5409 src/test/java/org/apache/aurora/scheduler/base/TaskTestUtil.java b8b9c1b8cdf7b641976c583a71fdd9b0c14e6e5f src/test/java/org/apache/aurora/scheduler/base/TasksTest.java 2866b20c1a88f6012a6f1c8eec33e450c3510f80 src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 7234fd3dfceec4d916807559d5b6604db0864d46 src/test/java/org/apache/aurora/scheduler/stats/ResourceCounterTest.java e5f3adfa61da665edb063b01b727f75455b0ad02 src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 90a05baa2a1d4fcd73d24d539a0453d6917ac2f6 src/test/java/org/apache/aurora/scheduler/storage/backup/RecoveryTest.java 5853c037d53e707e5df434fc661cd285ed9ecfc4 src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 1f505153f36b355718c778af230a6db4168505a5 src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java 83b238f3c0d218f35db098cf444c08d72798a7a3 src/test/java/org/apache/aurora/scheduler/storage/testing/StorageEntityUtil.java b26ddd39f7d972bba41aa72283d0b0626ed0fe64 src/test/java/org/apache/aurora/scheduler/updater/InstanceUpdaterTest.java 3f1f72bcf214b8ab9b3d4528ff0eb403c82db00c src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java dd4c94097161477e8161fb7e440e4c41e2a61f21 src/test/java/org/apache/aurora/scheduler/updater/TaskUtil.java 0e67f91536ff89c07da9be82049719c854aa3d62 Diff: https://reviews.apache.org/r/33200/diff/ Testing --- Thanks, Bill Farner
Review Request 33103: Skip known flaky tests.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33103/ --- Review request for Aurora, Joshua Cohen and Brian Wickman. Repository: aurora Description --- We've had very few legitimate build breakages over the last few months, these tests are a major source of illegitimate breakage. Diffs - src/test/python/apache/aurora/executor/common/test_announcer.py 6b782778e52394de3744b43003226dac3f65169e src/test/python/apache/aurora/executor/test_gc_executor.py 0f0b4a3781295c2a6640d272f9c8b0e8f78cc8a8 src/test/python/apache/aurora/executor/test_thermos_task_runner.py 6b24bbb2ab7ca16f97961aabeed945b61e5b5908 src/test/python/apache/thermos/core/test_staged_kill.py b52fcba3c7174a5e4d413b7b01f2fa7d95d21ee1 Diff: https://reviews.apache.org/r/33103/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 32990: Improving JobUpdateDetails fetch performance.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32990/#review79562 --- Ship it! Ship It! - Bill Farner On April 9, 2015, 7:15 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32990/ --- (Updated April 9, 2015, 7:15 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1263 https://issues.apache.org/jira/browse/AURORA-1263 Repository: aurora Description --- Using event collections with select avoids unnecessary row explosion and provdes subsecond execution time for a large job update that used to take half a minute. Before: ``` Benchmark (instances) Mode Cnt Score Error Units UpdateStoreBenchmarks.JobDetailsBenchmark.run 1000 thrpt5 15.595 ± 1.056 ops/s UpdateStoreBenchmarks.JobDetailsBenchmark.run 5000 thrpt5 3.143 ± 0.259 ops/s UpdateStoreBenchmarks.JobDetailsBenchmark.run1 thrpt5 1.539 ± 0.075 ops/s ``` After: ``` Benchmark (instances) Mode Cnt ScoreError Units UpdateStoreBenchmarks.JobDetailsBenchmark.run 1000 thrpt5 278.405 ± 57.311 ops/s UpdateStoreBenchmarks.JobDetailsBenchmark.run 5000 thrpt5 65.386 ± 6.455 ops/s UpdateStoreBenchmarks.JobDetailsBenchmark.run1 thrpt5 33.353 ± 1.974 ops/s ``` Diffs - src/jmh/java/org/apache/aurora/benchmark/UpdateStoreBenchmarks.java PRE-CREATION src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 4536cd35ffc231a5d2f6c1b5bf2aaa084ce3ad1d Diff: https://reviews.apache.org/r/32990/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 32806: Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32806/ --- (Updated April 9, 2015, 4:45 a.m.) Review request for Aurora, Joshua Cohen and Kevin Sweeney. Repository: aurora Description --- Remove use of LocalServiceRegistry, simplify plumbing of HTTP address. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/app/AppModule.java 5f6a019e4d6401e1efd075b72c049fa245cc0d0a src/main/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverrides.java 6d92ae3c8ec46e7964e81e507a2f2a7f2db68cfd src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 2af009d3d9ec44a70659225d0c18de9fda3a6f7a src/main/java/org/apache/aurora/scheduler/http/HttpService.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 50f377587ac05dbb72063ea02502e6d980148d3e src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java e03009c12de5a09761c1f444c6439ef3144b0b17 src/test/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverridesTest.java 21fd027976f75acc427c6d9265a7c7a91895d53d src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java c3e40d88fe7ee1a447d1d61980b69bd1b46881e7 src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java 7f80757cb40af7dde042f1d39355eadf2b3b1aee src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java c5c5f789de6bf7693520081d0c1acc5165603242 src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 45a23fdc58ca7475a805c549463e87ddaa915e74 Diff: https://reviews.apache.org/r/32806/diff/ Testing --- Test suite + end-to-end tests. Thanks, Bill Farner
Re: Review Request 32990: Improving JobUpdateDetails fetch performance.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32990/#review79466 --- Ship it! Great work! - Bill Farner On April 8, 2015, 9:45 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32990/ --- (Updated April 8, 2015, 9:45 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1263 https://issues.apache.org/jira/browse/AURORA-1263 Repository: aurora Description --- Using event collections with select avoids unnecessary row explosion and provdes subsecond execution time for a large job update that used to take half a minute. Diffs - src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml 4536cd35ffc231a5d2f6c1b5bf2aaa084ce3ad1d Diff: https://reviews.apache.org/r/32990/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 32861: Increase executor resource epsilon to match the minimum required by Mesos
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32861/#review79480 --- Ship it! Ship It! - Bill Farner On April 8, 2015, 7:35 a.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32861/ --- (Updated April 8, 2015, 7:35 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1246 https://issues.apache.org/jira/browse/AURORA-1246 Repository: aurora Description --- Increase executor resource epsilon to match the minimum required by Mesos. Mesos issues warnings for executors with 32MB RAM. By increasing the epsilon in Aurora, we shift some resources from the task to the executor. However, the total resources per container remains constant. Diffs - src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java f55d1f4fff4724b0fb651b6f0dc10d2e8f62beff src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 8ae68653427c0bdbf33751d5ed4b74afcaf98a7b Diff: https://reviews.apache.org/r/32861/diff/ Testing --- ./gradlew -Pq build Thanks, Stephan Erb
Review Request 33026: Revert Make health check configurable
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33026/ --- Review request for Aurora, Brian Brazil and Kevin Sweeney. Bugs: AURORA-1266 https://issues.apache.org/jira/browse/AURORA-1266 Repository: aurora Description --- This reverts commit 27a602d2c9efdd1cd2591c9c754f086c04ad0eb9. Diffs - docs/configuration-reference.md fb753ead94fcd1ed76352306391d356652934a4e src/main/python/apache/aurora/common/http_signaler.py 531f1fecbcbf8015175c7f1cb1e1c4d3e7d1268a src/main/python/apache/aurora/config/schema/base.py ec9f983564516afe542ab277d987d4d391f87e45 src/main/python/apache/aurora/executor/common/health_checker.py 03fdf0afef120c365c6ffad09e152780eed7e351 src/test/python/apache/aurora/common/test_http_signaler.py c6a21708eb997f213f4cca038891a84f6c5218d4 src/test/python/apache/aurora/executor/common/test_health_checker.py 27c71711d52f757ed1552db4accda671a6bdafdd Diff: https://reviews.apache.org/r/33026/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 33317: Upgrade to pystachio 0.8.0
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33317/#review80496 --- Can you include a test case proving that AURORA-1268 is fixed? - Bill Farner On April 17, 2015, 6:18 p.m., Brian Wickman wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33317/ --- (Updated April 17, 2015, 6:18 p.m.) Review request for Aurora, Brian Brazil and Joshua Cohen. Bugs: AURORA-1268 https://issues.apache.org/jira/browse/AURORA-1268 Repository: aurora Description --- Add pystachio 0.8.0 requirement. Diffs - 3rdparty/python/requirements.txt c23f98ca69197c400598c45b88c7c6415ffaf566 Diff: https://reviews.apache.org/r/33317/diff/ Testing --- ./pants test.pytest --no-fast --options='-v' src/test/python:: Thanks, Brian Wickman
Re: Review Request 32597: Improving async preemptor efficiency.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/#review80480
---
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
https://reviews.apache.org/r/32597/#comment130383
```
SetString allSlaves = Sets.newHashSet(Iterables.concat(
slavesToOffers.keySet(),
slavesToActiveTasks.keySet()));
```
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
https://reviews.apache.org/r/32597/#comment130384
Reservations are made for at most one task group per slave.
This reads as though you might make multiple reservations on a slave, so
long as they are from different task groups.
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
https://reviews.apache.org/r/32597/#comment130385
neither of the slaves what does that mean?
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
https://reviews.apache.org/r/32597/#comment130388
It's probably not worth changing code, but might be worth noting that this
breaks round-robin, since the position is reset.
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
https://reviews.apache.org/r/32597/#comment130393
Given that this is all private/local code, consider
`HashMultiset.create(Iterable)` here and avoid the immutable copy - mutable
copy dance.
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
https://reviews.apache.org/r/32597/#comment130404
Less efficient when there are many task groups, but i find this easier to
read:
```
ListTaskGroupKey instructions = Lists.newLinkedList();
SetTaskGroupKey keys = ImmutableSet.copyOf(groups.elementSet());
while (!groups.isEmpty()) {
for (TaskGroupKey key : keys) {
if (groups.contains(key)) {
instructions.add(key);
// Removes a single instance of key.
groups.remove(key);
}
}
}
```
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionProposal.java
https://reviews.apache.org/r/32597/#comment130405
a small doc would be nice here
src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java
https://reviews.apache.org/r/32597/#comment130406
`slaveId`
https://google-styleguide.googlecode.com/svn/trunk/javaguide.html#s5.3-camel-case
src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java
https://reviews.apache.org/r/32597/#comment130413
Should have brought this up in a previous review, but i'm uncomfortable
with using a mock to control the behavior of a data structure. It really feels
like the test knows way too much about the internals of the class at this
point, given that this is internally-managed state. Is there any particular
reason we shouldn't use a concrete `BiCache` instance here?
src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java
https://reviews.apache.org/r/32597/#comment130414
`@Nullable String slaveId`
src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java
https://reviews.apache.org/r/32597/#comment130415
I'd like to avoid randomness in tests, as trivial as it may seem here.
It's reasonable to assume that calling `makeTask()` with identical arguments
should at least produce an `equals()` object. I'd rather see a task id
argument to make that true.
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
https://reviews.apache.org/r/32597/#comment130417
I don't think anyObject() is what you want, that's for matching arguments.
This returns `null`.
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
https://reviews.apache.org/r/32597/#comment130427
Related to the `OfferManager.andReturn` mock above, you could return a
constant there, and expect the same constant here. Then you get to drop all
the `eq()`s.
- Bill Farner
On April 16, 2015, 1:39 a.m., Maxim Khutornenko wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32597/
---
(Updated April 16, 2015, 1:39 a.m.)
Review request for Aurora, Bill Farner and Zameer Manji.
Bugs: AURORA-1219
https://issues.apache.org/jira/browse/AURORA-1219
Repository: aurora
Description
---
This is addressing the preemption efficiency loss due to making preemptor
async. Summary:
- Implemented fair task processing by evaluating pending task groups in
round-robin fashion (e.g.: {G1:3, G2:2} - {G1, G2, G1, G2, G1}) against all
Re: Review Request 33202: Fixing benchmarks.
On April 15, 2015, 3:07 a.m., Bill Farner wrote: src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java, line 135 https://reviews.apache.org/r/33202/diff/1/?file=929440#file929440line135 IMHO the empty string is a wart, and emphasizes my point about letting the consumer decide if they want stats. I won't harp on it, but i think this has made the code less reusable. Maxim Khutornenko wrote: This is hardly a good example though as we always want stats in production. I will be happy to reconsider should we have a real counter case. So far, every time we deal with caches we always want more visibility. This probably makes a good case for a decorating implementation of an interface. No urgency, of course...but something to keep in mind as things evolve. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33202/#review80148 --- On April 15, 2015, 12:32 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33202/ --- (Updated April 15, 2015, 12:32 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Benchmark module configuration got broken by recent refactoring. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 011350540d4f5091ff5f8a3c646f40f3f63357fd Diff: https://reviews.apache.org/r/33202/diff/ Testing --- ./gradlew jmh Thanks, Maxim Khutornenko
Re: Review Request 33184: Implement missing != operator for AuroraJobKey
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33184/#review80079 --- src/test/python/apache/aurora/common/test_aurora_job_key.py https://reviews.apache.org/r/33184/#comment129848 Can you test the inverse as well - keys that are not equal? - Bill Farner On April 14, 2015, 6:51 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/33184/ --- (Updated April 14, 2015, 6:51 p.m.) Review request for Aurora. Repository: aurora Description --- Implement missing \!= operator for AuroraJobKey From the Python docu of __ne__: There are no implied relationships among the comparison operators. The truth of x==y does not imply that x!=y is false. Accordingly, when defining __eq__(), one should also define __ne__() so that the operators will behave as expected. Diffs - src/main/python/apache/aurora/common/aurora_job_key.py 88896c6c41235f7d313f0d77682acdca7cce src/test/python/apache/aurora/common/test_aurora_job_key.py 7d8d58b541f16d63495f46b58bd928e8f53f66d1 Diff: https://reviews.apache.org/r/33184/diff/ Testing --- ./pants test.pytest --no-fast --options=-v src/test/python/apache/aurora/common:: Thanks, Stephan Erb
Re: Review Request 32830: Fix header levels in monitoring.md
On April 3, 2015, 7:40 p.m., Stephan Erb wrote: Thinking about it, we might as well drop all headers 'Description', 'Alerting' and 'Triage'. By just keeping their content around, the resulting document should be much simpler to read. Example of the current state: https://github.com/apache/aurora/blob/master/docs/monitoring.md Thanks for the touch-up! I'm fine with the additional proposed change. If you add that to the diff i'll commit. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32830/#review78822 --- On April 3, 2015, 7:09 p.m., Stephan Erb wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32830/ --- (Updated April 3, 2015, 7:09 p.m.) Review request for Aurora. Repository: aurora Description --- Fix header levels in monitoring.md Diffs - docs/monitoring.md 8aee66915f40d39368b57acd2b9e328e52b8cdbe Diff: https://reviews.apache.org/r/32830/diff/ Testing --- Thanks, Stephan Erb
Re: Review Request 32329: Extract job key from RPC parameters
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32329/#review78551 --- Ship it! src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetter.java https://reviews.apache.org/r/32329/#comment127489 please doc src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java https://reviews.apache.org/r/32329/#comment127491 I think you could simplify a decent amount of this if you use `getMethod` instead of `getDeclaredMethod` to do hierarchy walking for you. src/main/java/org/apache/aurora/scheduler/http/api/security/ThriftFieldGetter.java https://reviews.apache.org/r/32329/#comment127492 formatting is off. i noticed intellij likes to do this particular formatting when you extract an inner class. src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java https://reviews.apache.org/r/32329/#comment127496 revert? src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java https://reviews.apache.org/r/32329/#comment127399 make this a constant src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java https://reviews.apache.org/r/32329/#comment127398 remove semicolon, remove newline above - Bill Farner On March 28, 2015, 12:02 a.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32329/ --- (Updated March 28, 2015, 12:02 a.m.) Review request for Aurora, Joshua Cohen and Bill Farner. Bugs: AURORA-1187 https://issues.apache.org/jira/browse/AURORA-1187 Repository: aurora Description --- Apologies for the large diff, this wound up needing to input validation at the AOP layer. Probably the best place to start reading this diff is ApiSecurityIT to see the feature this patch enables. Diffs - config/pmd/custom.xml 521fd500146eb2e45f8e77c5c3c0cce330fedabb src/main/java/org/apache/aurora/scheduler/http/api/ApiBeta.java 827e85b6cac8bd52359610bbc2002973a769705c src/main/java/org/apache/aurora/scheduler/http/api/ApiModule.java 2408cd1f9af5f109a339f5c78134465cb117f7fc src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java ec6a02c4086ee0d5a7529083030d978ea889f677 src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizingParam.java 808987939b2c4a850e488dc033b50b0178e95ba0 src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/FieldGetters.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptor.java 4e341e05c34b1be38f0040c26b671a0cc797a771 src/main/java/org/apache/aurora/scheduler/http/api/security/ThriftFieldGetter.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 5588d1793d6713ee4581ac9f938d9a8689acb315 src/main/java/org/apache/aurora/scheduler/thrift/aop/AopModule.java bdd2185f3a7a94b39bcec3c73455e970d87f0c6a src/test/java/org/apache/aurora/scheduler/http/api/ApiBetaTest.java cafd10f6b705568588c1b92644b482003242fe2e src/test/java/org/apache/aurora/scheduler/http/api/ApiIT.java ed284f46ac8f01bd6d9e317f995f16d6e666a68d src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 76cb691e6d7d4fada3a18fde73aceed7039bcaa4 src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthenticatingThriftInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroAuthorizingParamInterceptorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/http/api/security/ShiroThriftInterceptorTest.java d2ba2730c4509dc9a636fd32e9244b0d7fa2884f src/test/java/org/apache/aurora/scheduler/http/api/security/ThriftFieldGetterTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 1f24e7d47e1f777ffef19a73d01171fcacd31cdb src/test/java/org/apache/aurora/scheduler/thrift/aop/AopModuleTest.java d20c9da3c4944ec8c50fe8d48b7e459ff1c7082b Diff: https://reviews.apache.org/r/32329/diff/ Testing --- ./gradlew -Pq build Thanks, Kevin Sweeney
Re: Review Request 32352: Making preemptor asynchronous. Part 3(final) - background service.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32352/#review78603 --- Ship it! Ship It! - Bill Farner On March 21, 2015, 2:19 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/32352/ --- (Updated March 21, 2015, 2:19 a.m.) Review request for Aurora, Bill Farner and Zameer Manji. Bugs: AURORA-1158 https://issues.apache.org/jira/browse/AURORA-1158 Repository: aurora Description --- Summary of changes: - The PreemptorImpl now only hosts slot validation and task preemption logic and requires a write transaction. - PendingTaskProcessor is called every minute to pull all available PENDING tasks and search for preemption slots. - TaskScheduler has no connection to slot search anymore. It now completely relies on periodic PreemptionService to find available slots. - preemption_delay is reduced from 10 to 3 minutes. Benchmark refactoring will be addressed separately. Diffs - src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 5309e8140fff411da30ee87c1b3b1a55d6fdaeeb src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 1b9d741dba7b9c2663ff119cd44adc8403c0d257 src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java 4ca36e5fe2cfd326f4d4f37f70dbcd0060109e73 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java 84bcdc57d798aca229d4f184cae065ec4dcf8fc5 src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 84791a272f245c729706af95d70c7f1631bfe99c src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImpl.java 18a2e6032ba86ff7efab4d42a4d83798a1d06b06 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorMetrics.java 782e751f5b05391ebeee4d947570cc174dddece2 src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 7034a07eae1f94d7a0bbccdf8146cf3ed0a5424e src/main/java/org/apache/aurora/scheduler/filter/AttributeAggregate.java da7b662c3ca4040221805cba81e5ac7b64fb3df4 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 29fe156da19f3c08af00a951bb3baccf2b97a6cb src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java f5c2128e90eb5c849d68431225661d1cfa7da0cc src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java d17c4fb513afdb7d8ef6d7c2b0aef86c1f47c082 src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModuleTest.java 0e2e958a053e5cee280b947f7349c076e2addb45 Diff: https://reviews.apache.org/r/32352/diff/ Testing --- ./gradlew -Pq build Manual testing in vagrant. Thanks, Maxim Khutornenko
Re: Review Request 35672: DbTaskStore perf: optimize queries scoped to a task ID.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35672/ --- (Updated June 22, 2015, 4:51 p.m.) Review request for Aurora and Maxim Khutornenko. Changes --- Added TODO. Bugs: AURORA-1298 https://issues.apache.org/jira/browse/AURORA-1298 Repository: aurora Description --- DbTaskStore perf: optimize queries scoped to a task ID. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java cceac8ab14243c7806c48cf5a8d4c1175d7004b8 src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 9b30b01e3a9ecdf368910a7270f0d6fed911b2de src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java 8270407f97f44991dcfa47263c4287c58ac558f1 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 4b67f6ba03d299ed3de73bb5ea69d949364835b3 src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml 7c27f374b1143c82131c19448c3236f3dfb96667 Diff: https://reviews.apache.org/r/35672/diff/ Testing --- Scheduling benchmark results with MemTaskStore: ``` ClusterFullUtilizationBenchmark.runBenchmark N/A thrpt 10 541318.648 ± 25645.908 ops/s InsufficientResourcesSchedulingBenchmark.runBenchmark N/A thrpt 10 57782.604 ± 1896.717 ops/s LimitConstraintMismatchSchedulingBenchmark.runBenchmarkN/A thrpt 104040.842 ±42.494 ops/s PreemptorSlotSearchBenchmark.runBenchmark1 thrpt 10 55.713 ± 1.078 ops/s PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 55.160 ± 1.434 ops/s PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 54.352 ± 3.150 ops/s PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 53.149 ± 1.026 ops/s ValueConstraintMismatchSchedulingBenchmark.runBenchmarkN/A thrpt 10 56678.838 ± 1818.328 ops/s ``` With DbTaskStore before this change: ``` ClusterFullUtilizationBenchmark.runBenchmark N/A thrpt 10 42070.261 ± 846.323 ops/s InsufficientResourcesSchedulingBenchmark.runBenchmark N/A thrpt 10 19909.569 ± 516.249 ops/s LimitConstraintMismatchSchedulingBenchmark.runBenchmarkN/A thrpt 10 2956.306 ± 24.422 ops/s PreemptorSlotSearchBenchmark.runBenchmark1 thrpt 10 54.233 ± 1.455 ops/s PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 54.970 ± 1.027 ops/s PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 53.516 ± 1.172 ops/s PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 45.404 ± 2.238 ops/s ValueConstraintMismatchSchedulingBenchmark.runBenchmarkN/A thrpt 10 16391.602 ± 752.267 ops/s ``` With DbTaskStore after this change: ``` ClusterFullUtilizationBenchmark.runBenchmark N/A thrpt 10 149589.434 ± 3553.202 ops/s InsufficientResourcesSchedulingBenchmark.runBenchmark N/A thrpt 10 29268.634 ± 1072.366 ops/s LimitConstraintMismatchSchedulingBenchmark.runBenchmarkN/A thrpt 103187.037 ± 25.248 ops/s PreemptorSlotSearchBenchmark.runBenchmark1 thrpt 10 56.504 ±0.575 ops/s PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 54.710 ±1.396 ops/s PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 54.777 ±1.244 ops/s PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 45.155 ±1.602 ops/s ValueConstraintMismatchSchedulingBenchmark.runBenchmarkN/A thrpt 10 23351.751 ± 509.439 ops/s ``` While we lack formal acceptance criteria for DbTaskStore on these benchmarks, i believe these should be considered acceptable. Thanks, Bill Farner
Re: Review Request 35587: Suppress task reconciliation status update logging.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/#review88806 --- Ship it! Ship It! - Bill Farner On June 19, 2015, 10:07 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/ --- (Updated June 19, 2015, 10:07 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Suppress task reconciliation status update logging. Diffs - src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java f233d5a181bb1f43fbbfe657dbda2cf975aa6895 src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java f0f9ac392973a276028aee3e06517a6e6d960bb6 Diff: https://reviews.apache.org/r/35587/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 35760: Removing GcExecutorLauncher code.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35760/#review89007 --- Ship it! LGTM so long as end-to-end tests come up green. docs/storage-config.md (line 103) https://reviews.apache.org/r/35760/#comment141602 Consider a significantly higher number as the advice to make it clear, e.g. 365days. src/main/java/org/apache/aurora/scheduler/SchedulerModule.java (line 104) https://reviews.apache.org/r/35760/#comment141603 while you're here, remove this newline - Bill Farner On June 23, 2015, 2:24 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35760/ --- (Updated June 23, 2015, 2:24 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1334 https://issues.apache.org/jira/browse/AURORA-1334 Repository: aurora Description --- Jave side of changes for removing gc executor support. TaskLauncher refactoring will be addressed in a separate diff. Diffs - config/legacy_untested_classes.txt d2f3ca50d32abf945f58b5fb67cd27c0b127822d debian/aurora-scheduler.default 47fea5b4e6dadab9704c6ea5141268a91803fdc2 debian/aurora-scheduler.init 2ed0159d36d4be08f2c355dc1c21f36192a819f4 debian/aurora-scheduler.upstart ed7fe0917147d9c36a0828cc701da902142f72ba docs/storage-config.md 971bc1673f8be800f0d081a44018fc17a5ed025a examples/scheduler/scheduler-local.sh 6253d505f9851aea613bc95c15c313b03b57af11 examples/vagrant/upstart/aurora-scheduler-kerberos.conf 414539b1917b5d33c577f1539575934c7f7c8167 examples/vagrant/upstart/aurora-scheduler.conf f4b867cbbcdbcc792518c2f90807834e47dce253 src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 6edec22aba135136c4ce4066b9535f23de077db7 src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 8bcac6c4f93eaca7250d43ab81a72102b91bf836 src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java f2ef70ddc5a859811f0e6c2ade62e115639c1654 src/test/java/org/apache/aurora/ProtobufsTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java d2ec944ca12456c23eb54c9be8b1e6756f35e7f1 src/test/java/org/apache/aurora/scheduler/base/CommandUtilTest.java ec43a44d9cbd0f303e4833953cdb65664d5ed569 Diff: https://reviews.apache.org/r/35760/diff/ Testing --- Thanks, Maxim Khutornenko
Re: Review Request 35761: Refactoring TaskLauncher.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35761/#review89008 --- LGTM overall, will give a ship once the rebase comes out green. src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java (line 164) https://reviews.apache.org/r/35761/#comment141605 Accessing the Impl suggests an encapsulagion problem. Can you use the interface instead? src/main/java/org/apache/aurora/scheduler/TaskStatusUpdateManager.java (line 21) https://reviews.apache.org/r/35761/#comment141604 How about `TaskStatusReceiver` or `TaskStatusHandler`? src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java (line 176) https://reviews.apache.org/r/35761/#comment141608 Please include a comment explaining the rationale behind the default. - Bill Farner On June 23, 2015, 2:29 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35761/ --- (Updated June 23, 2015, 2:29 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1334 https://issues.apache.org/jira/browse/AURORA-1334 Repository: aurora Description --- Replacing TaskLauncher abstraction with TaskStatusUpdateManager. Will not apply cleanly. Depends on https://reviews.apache.org/r/35760/. Diffs - src/jmh/java/org/apache/aurora/benchmark/StatusUpdateBenchmark.java 4c63cc7af2fb5702fa649ce2cd4ee619139223e2 src/main/java/org/apache/aurora/scheduler/SchedulerModule.java 6edec22aba135136c4ce4066b9535f23de077db7 src/main/java/org/apache/aurora/scheduler/TaskLauncher.java cd55a6ee7424873c3e615d95422c9ecab6442f46 src/main/java/org/apache/aurora/scheduler/TaskStatusUpdateManager.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 6bfbf0c76399d569ac762a7b433a6c576bf0ee87 src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 8bcac6c4f93eaca7250d43ab81a72102b91bf836 src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 4f7a1bec8c32aef4b5f435c760f69de13888ea9d src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 126001a90c819f9b3bfd448a060102bcef8b2a35 src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java e4e158721e0bdc7ed8a02eea24f4915ab937b9c7 Diff: https://reviews.apache.org/r/35761/diff/ Testing --- Thanks, Maxim Khutornenko
Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.
On June 23, 2015, 6:58 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java, line 133 https://reviews.apache.org/r/35793/diff/2/?file=990668#file990668line133 Any particular reason you've settled on an explicit removal call as opposed to a dedicated java trigger (1)? Triggers certainly have their pitfals but they may provide a cleaner solution and let us migrate easier to pure SQL triggers when H2 implements support for them (2) (1) - http://www.h2database.com/html/features.html#triggers (2) - http://www.h2database.com/html/roadmap.html Biggest issue is coupling to h2. H2 only supports triggers written in java, implementing their API. This would present a pretty big hurdle for using a different vendor. On June 23, 2015, 6:58 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java, line 136 https://reviews.apache.org/r/35793/diff/2/?file=990668#file990668line136 A stat counter recording a successful removal would be nice here and below. That would help us monitoring cleanup health. What's the shelf life of that? This seems like something we won't want to sprinkle in each of the many forthcoming places we need to do this, and doesn't seem like we'd actually be able to alert on it. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35793/#review89022 --- On June 23, 2015, 6:28 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35793/ --- (Updated June 23, 2015, 6:28 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1298 https://issues.apache.org/jira/browse/AURORA-1298 Repository: aurora Description --- DbTaskStore: delete unreferenced job keys. Diffs - src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 30e3469a9f69091b929a9243f84036fa2fdd0539 src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 3ada6286e6ef6e3302802b74eec6c46dd582dc10 src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 7ee001f9c019a1e7b669ae5cec6088bf974a3746 src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 8258fb102b7f5fca9635143ebaed542d43abeb9f src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 24cf52680b69e23f5ccbbcada0606975b0405d5b src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 63a784f843eb7edf9a13c623e5355169c7e8623b src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java dda988d03634f8de582cf2b8ccdeb433c3e3de0c Diff: https://reviews.apache.org/r/35793/diff/ Testing --- Note that this removes a defensive branch wherein we checked for inbound config references before attempting to delete. With this change, we proactively delete and count on foreign key constraints to prevent deletion of rows that are still referenced. I propose we adopt this as our pattern for handling shared references, as it seems to be the most sane approach available. A gotcha with this case is that i do not believe mybatis provides a vendor-neutral approach to identify a consistency violation, and the best signal is a generic `PersistenceException`. This is unfortunate since we can't distinguish between a hopeless query with invalid syntax, a network disconnection, or the anticipated case of a consistency violation. Thanks, Bill Farner
Review Request 35793: DbTaskStore: delete unreferenced job keys.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35793/ --- Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1298 https://issues.apache.org/jira/browse/AURORA-1298 Repository: aurora Description --- DbTaskStore: delete unreferenced job keys. Diffs - src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 30e3469a9f69091b929a9243f84036fa2fdd0539 src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 3ada6286e6ef6e3302802b74eec6c46dd582dc10 src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 7ee001f9c019a1e7b669ae5cec6088bf974a3746 src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 8258fb102b7f5fca9635143ebaed542d43abeb9f src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 24cf52680b69e23f5ccbbcada0606975b0405d5b src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 63a784f843eb7edf9a13c623e5355169c7e8623b src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java dda988d03634f8de582cf2b8ccdeb433c3e3de0c Diff: https://reviews.apache.org/r/35793/diff/ Testing --- Note that this removes a defensive branch wherein we checked for inbound config references before attempting to delete. With this change, we proactively delete and count on foreign key constraints to prevent deletion of rows that are still referenced. I propose we adopt this as our pattern for handling shared references, as it seems to be the most sane approach available. A gotcha with this case is that i do not believe mybatis provides a vendor-neutral approach to identify a consistency violation, and the best signal is a generic `PersistenceException`. This is unfortunate since we can't distinguish between a hopeless query with invalid syntax, a network disconnection, or the anticipated case of a consistency violation. Thanks, Bill Farner
Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35793/ --- (Updated June 23, 2015, 6:28 p.m.) Review request for Aurora and Maxim Khutornenko. Changes --- Remove stale TODO. Bugs: AURORA-1298 https://issues.apache.org/jira/browse/AURORA-1298 Repository: aurora Description --- DbTaskStore: delete unreferenced job keys. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 30e3469a9f69091b929a9243f84036fa2fdd0539 src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 3ada6286e6ef6e3302802b74eec6c46dd582dc10 src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 7ee001f9c019a1e7b669ae5cec6088bf974a3746 src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 8258fb102b7f5fca9635143ebaed542d43abeb9f src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 24cf52680b69e23f5ccbbcada0606975b0405d5b src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 63a784f843eb7edf9a13c623e5355169c7e8623b src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java dda988d03634f8de582cf2b8ccdeb433c3e3de0c Diff: https://reviews.apache.org/r/35793/diff/ Testing --- Note that this removes a defensive branch wherein we checked for inbound config references before attempting to delete. With this change, we proactively delete and count on foreign key constraints to prevent deletion of rows that are still referenced. I propose we adopt this as our pattern for handling shared references, as it seems to be the most sane approach available. A gotcha with this case is that i do not believe mybatis provides a vendor-neutral approach to identify a consistency violation, and the best signal is a generic `PersistenceException`. This is unfortunate since we can't distinguish between a hopeless query with invalid syntax, a network disconnection, or the anticipated case of a consistency violation. Thanks, Bill Farner
Re: Review Request 35672: DbTaskStore perf: optimize queries scoped to a task ID.
On June 19, 2015, 11:50 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java, line 127 https://reviews.apache.org/r/35672/diff/1/?file=988557#file988557line127 Curious, why special treating whitespacing here and not in other places? This is a bulk move of the predicate, so i don't have a good answer. Happy to leave a TODO/ticket to re-evaluate, but i'd like to avoid bundling that with this change. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35672/#review88605 --- On June 19, 2015, 10:43 p.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35672/ --- (Updated June 19, 2015, 10:43 p.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-1298 https://issues.apache.org/jira/browse/AURORA-1298 Repository: aurora Description --- DbTaskStore perf: optimize queries scoped to a task ID. Diffs - src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java cceac8ab14243c7806c48cf5a8d4c1175d7004b8 src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 9b30b01e3a9ecdf368910a7270f0d6fed911b2de src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java 8270407f97f44991dcfa47263c4287c58ac558f1 src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 4b67f6ba03d299ed3de73bb5ea69d949364835b3 src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml 7c27f374b1143c82131c19448c3236f3dfb96667 Diff: https://reviews.apache.org/r/35672/diff/ Testing --- Scheduling benchmark results with MemTaskStore: ``` ClusterFullUtilizationBenchmark.runBenchmark N/A thrpt 10 541318.648 ± 25645.908 ops/s InsufficientResourcesSchedulingBenchmark.runBenchmark N/A thrpt 10 57782.604 ± 1896.717 ops/s LimitConstraintMismatchSchedulingBenchmark.runBenchmarkN/A thrpt 104040.842 ±42.494 ops/s PreemptorSlotSearchBenchmark.runBenchmark1 thrpt 10 55.713 ± 1.078 ops/s PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 55.160 ± 1.434 ops/s PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 54.352 ± 3.150 ops/s PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 53.149 ± 1.026 ops/s ValueConstraintMismatchSchedulingBenchmark.runBenchmarkN/A thrpt 10 56678.838 ± 1818.328 ops/s ``` With DbTaskStore before this change: ``` ClusterFullUtilizationBenchmark.runBenchmark N/A thrpt 10 42070.261 ± 846.323 ops/s InsufficientResourcesSchedulingBenchmark.runBenchmark N/A thrpt 10 19909.569 ± 516.249 ops/s LimitConstraintMismatchSchedulingBenchmark.runBenchmarkN/A thrpt 10 2956.306 ± 24.422 ops/s PreemptorSlotSearchBenchmark.runBenchmark1 thrpt 10 54.233 ± 1.455 ops/s PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 54.970 ± 1.027 ops/s PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 53.516 ± 1.172 ops/s PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 45.404 ± 2.238 ops/s ValueConstraintMismatchSchedulingBenchmark.runBenchmarkN/A thrpt 10 16391.602 ± 752.267 ops/s ``` With DbTaskStore after this change: ``` ClusterFullUtilizationBenchmark.runBenchmark N/A thrpt 10 149589.434 ± 3553.202 ops/s InsufficientResourcesSchedulingBenchmark.runBenchmark N/A thrpt 10 29268.634 ± 1072.366 ops/s LimitConstraintMismatchSchedulingBenchmark.runBenchmarkN/A thrpt 103187.037 ± 25.248 ops/s PreemptorSlotSearchBenchmark.runBenchmark1 thrpt 10 56.504 ±0.575 ops/s PreemptorSlotSearchBenchmark.runBenchmark 10 thrpt 10 54.710 ±1.396 ops/s PreemptorSlotSearchBenchmark.runBenchmark 100 thrpt 10 54.777 ±1.244 ops/s PreemptorSlotSearchBenchmark.runBenchmark 1000 thrpt 10 45.155 ±1.602 ops/s ValueConstraintMismatchSchedulingBenchmark.runBenchmarkN/A thrpt 10 23351.751 ± 509.439 ops/s ``` While we lack formal acceptance
Re: Review Request 35812: Remove enable_legacy_constraints flag.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35812/#review89373 --- Ship it! NEWS (line 5) https://reviews.apache.org/r/35812/#comment141954 ``` The scheduler command line argument enable_legacy_constraints has been removed, and the scheduler no longer automatically injects 'host' and 'rack' constraints for production services. ``` - Bill Farner On June 23, 2015, 11:58 p.m., Zameer Manji wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35812/ --- (Updated June 23, 2015, 11:58 p.m.) Review request for Aurora, Maxim Khutornenko and Bill Farner. Bugs: AURORA-1074 https://issues.apache.org/jira/browse/AURORA-1074 Repository: aurora Description --- Remove the enable_legacy_constraints flag and associated behaviour. Diffs - NEWS a17f0e7c08fd30a0b2db6814a1c755111307228b src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java b77b0ebbf303778e528b16ff3db1aa4e76f1 src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java abbd23dd3ee4382565ce846eb035e2aa502badae src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 38ef412a6d29dfef7b305e00cf44522818303965 Diff: https://reviews.apache.org/r/35812/diff/ Testing --- ./gradlew build -Pq Thanks, Zameer Manji
Re: Review Request 35813: Removing GC executor code.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35813/#review89372 --- Ship it! 25 files changed, 2 insertions(+), 2054 deletions(-) Nice! 2 requests: - Can you confirm that end-to-end tests still pass? - Can you add to this patch a note in NEWS under 0.9.0 that the GC executor has been removed? - Bill Farner On June 24, 2015, 12:22 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35813/ --- (Updated June 24, 2015, 12:22 a.m.) Review request for Aurora, Bill Farner and Brian Wickman. Bugs: AURORA-1333 https://issues.apache.org/jira/browse/AURORA-1333 Repository: aurora Description --- Removing GC executor code. Diffs - api/src/main/thrift/org/apache/aurora/gen/BUILD fe3f83b6a7680985dce01efe2d54ccc4b0c2c482 api/src/main/thrift/org/apache/aurora/gen/internal_rpc.thrift a2c230fa9b5f648c4674042411cbe46fb8bb4faa debian/aurora-executor.install 8efb1308caf64a23bed4b580de4b86e7982539e8 debian/rules 6ba18cef7fbf0989507d630a1041cdf958742617 docs/test-resource-generation.md 335586d64757f1e6293a89f14c1c3d578321eac6 examples/vagrant/aurorabuild.sh 5eb171cf45ffee1287f3ac039ab8cf3db6991a97 src/main/python/apache/aurora/executor/BUILD cbb2f5f7b5daa936db71cf8c0aac8ddb2002060b src/main/python/apache/aurora/executor/bin/BUILD 0fbb0f1ee63499d9ce36150ae5e68fcc8a9e src/main/python/apache/aurora/executor/bin/gc_executor_main.py 8093717266f8620ebc6ef4c028ac8c87ab8d22be src/main/python/apache/aurora/executor/gc_executor.py d4392faf50f8c72f08f951962913248045d7fcb5 src/main/python/apache/thermos/cli/commands/BUILD 1dae8c981bd750807ddd1b6071e232ff2697537d src/main/python/apache/thermos/cli/commands/gc.py 23d9ff4d2048b4f2d80ea62c54e58e8d768e11c0 src/main/python/apache/thermos/cli/main.py f20f612790550b77ee3dc969c37317b014a64972 src/main/python/apache/thermos/core/BUILD efb68e84cf547cb9505a8caf5b47be394dee5145 src/main/python/apache/thermos/core/helper.py 8cd32948663a5d5a1e975e1661b78de701710436 src/main/python/apache/thermos/core/inspector.py 4fe8aa31215a12b9a53e885697b4dd4e78c1f35f src/main/python/apache/thermos/monitoring/BUILD 633dd95f9d193b1f377ab5d6cdfcdca7bdaa610f src/main/python/apache/thermos/monitoring/garbage.py aa5a2729ae6c94b6a270d97425767ccee121e588 src/test/python/apache/aurora/executor/BUILD f415ecc77022b34f053c35272d004e133803d702 src/test/python/apache/aurora/executor/bin/BUILD 2caab2aec136ede9b51ce3bdd0d139270024ba48 src/test/python/apache/aurora/executor/bin/test_gc_executor_entry_point.py d4c1d572663039eb742f70de1e06d708eb0b769a src/test/python/apache/aurora/executor/test_gc_executor.py 17d359054d1fc33f79a7612162064abd335ccf81 src/test/python/apache/thermos/cli/commands/test_import.py 74d9a32cf85a9e49cfbc596a7d6d44393df14e7a src/test/python/apache/thermos/monitoring/BUILD 89030d0e25e8eb3f4d4eec6a0d0a0fc3dfd43481 src/test/python/apache/thermos/monitoring/test_garbage.py 4309c46a3af5f12c8eb3192e3156348fa7c0db23 Diff: https://reviews.apache.org/r/35813/diff/ Testing --- Thanks, Maxim Khutornenko
Review Request 35901: Add a DbCronJobStore implementation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35901/ --- Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-415 https://issues.apache.org/jira/browse/AURORA-415 Repository: aurora Description --- This pulled a longer thread than i had hoped, but rest assured that the diff is exaggerated due to some code moves. I'll leave comments in specific places of note. Diffs - src/main/java/org/apache/aurora/scheduler/storage/db/CronJobMapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/DbCronJobStore.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 8bfb076a80face2703045800cf5530f37d64269f src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java bb61542f4043847b1c8c92ff1b4a0ecfcfc88973 src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java a5acb3ae2ca5ecfd7d470fcd02de6091d66a1694 src/main/java/org/apache/aurora/scheduler/storage/db/InsertResult.java 44dc8f5e3dcc91e80a03d980c5d8ae0db65c8b89 src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 46fa940b9cdf0544fa33b76e87ad21045e8bcdbb src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/CronCollisionPolicyTypeHandler.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java 0a519be65f90cb730f6bda1e6d7b019f0f15252b src/main/java/org/apache/aurora/scheduler/storage/db/views/CronJobWrapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/ScheduledTaskWrapper.java b89e7b5463cdb9ff9d1f9106dda0c0c4908225ca src/main/java/org/apache/aurora/scheduler/storage/mem/InMemStoresModule.java 35c83b9b8838a00693c8ebc96e496886ca249ed1 src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java f9e9e89a52c9bce3dd7e5a727498a7c87c26a56c src/main/resources/org/apache/aurora/scheduler/storage/db/CronJobMapper.xml PRE-CREATION src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml 80ff3472c768cd116770e887b68e70d2ea3c9a8d src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e9660b732bdc97117ee467fc124e5a014f176c67 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 0f77db7397866c008d32ea6f55c5bb577fe6468f src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java b9e16578b27de2985d24c25aae507b3540fcd3ff src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 863e9c998c97506759a5526135a33a161a8fb75f src/test/java/org/apache/aurora/scheduler/cron/quartz/QuartzTestUtil.java 2d74b326831fbe22fa6045610ca8d714cd64779e src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 5ad0de7c6a648f5eb6408eea7bcd789c25d55f88 src/test/java/org/apache/aurora/scheduler/storage/db/DbCronJobStoreTest.java PRE-CREATION src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java 31feaea7ba74350fc199333a2384419ec05f1816 src/test/java/org/apache/aurora/scheduler/storage/mem/InMemTaskStoreTest.java 2ed748383f269217860b80c831a3521facba83ba src/test/java/org/apache/aurora/scheduler/storage/mem/MemCronJobStoreTest.java 58256afafd12e5de234755969605861891af4daf Diff: https://reviews.apache.org/r/35901/diff/ Testing --- Thanks, Bill Farner
Re: Review Request 35901: Add a DbCronJobStore implementation.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35901/#review89464 --- Adding reviewer notes. src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java https://reviews.apache.org/r/35901/#comment142098 This check is made superfluous now that `ScheduledTaskWrapper` extends `InsertResult`. src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java https://reviews.apache.org/r/35901/#comment142099 This moved to `TaskConfigManager`. src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java https://reviews.apache.org/r/35901/#comment142100 Also moved to `TaskConfigManager`. src/main/resources/org/apache/aurora/scheduler/storage/db/CronJobMapper.xml (line 26) https://reviews.apache.org/r/35901/#comment142101 We don't actually fill an unset cronCollisionPolicy with the default, but handle `null` in the consuming code. I opted to maintain this behavior. src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java (line 72) https://reviews.apache.org/r/35901/#comment142102 This test validated bad data coming from storage, which is no longer possible. src/test/java/org/apache/aurora/scheduler/storage/AbstractCronJobStoreTest.java (line 43) https://reviews.apache.org/r/35901/#comment142103 The test cases in here are unchanged from the original, but the delta in the file causes git to not track it as a move. src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java (line 49) https://reviews.apache.org/r/35901/#comment142104 This test case relates to AURORA-749 [1]. While we are not ready to remove the `Identity` struct as that ticket calls for, we are now at a release where we can expect `JobConfiguration.taskConfig.job` to be set. I opted to change behavior here to avoid an if/else in the mapper code. [1] https://issues.apache.org/jira/browse/AURORA-749 - Bill Farner On June 26, 2015, 2:13 a.m., Bill Farner wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35901/ --- (Updated June 26, 2015, 2:13 a.m.) Review request for Aurora and Maxim Khutornenko. Bugs: AURORA-415 https://issues.apache.org/jira/browse/AURORA-415 Repository: aurora Description --- This pulled a longer thread than i had hoped, but rest assured that the diff is exaggerated due to some code moves. I'll leave comments in specific places of note. Diffs - src/main/java/org/apache/aurora/scheduler/storage/db/CronJobMapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/DbCronJobStore.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 8bfb076a80face2703045800cf5530f37d64269f src/main/java/org/apache/aurora/scheduler/storage/db/DbStorage.java bb61542f4043847b1c8c92ff1b4a0ecfcfc88973 src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java a5acb3ae2ca5ecfd7d470fcd02de6091d66a1694 src/main/java/org/apache/aurora/scheduler/storage/db/InsertResult.java 44dc8f5e3dcc91e80a03d980c5d8ae0db65c8b89 src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 46fa940b9cdf0544fa33b76e87ad21045e8bcdbb src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/CronCollisionPolicyTypeHandler.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/typehandlers/TypeHandlers.java 0a519be65f90cb730f6bda1e6d7b019f0f15252b src/main/java/org/apache/aurora/scheduler/storage/db/views/CronJobWrapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/views/ScheduledTaskWrapper.java b89e7b5463cdb9ff9d1f9106dda0c0c4908225ca src/main/java/org/apache/aurora/scheduler/storage/mem/InMemStoresModule.java 35c83b9b8838a00693c8ebc96e496886ca249ed1 src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java f9e9e89a52c9bce3dd7e5a727498a7c87c26a56c src/main/resources/org/apache/aurora/scheduler/storage/db/CronJobMapper.xml PRE-CREATION src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml 80ff3472c768cd116770e887b68e70d2ea3c9a8d src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml e9660b732bdc97117ee467fc124e5a014f176c67 src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 0f77db7397866c008d32ea6f55c5bb577fe6468f src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java b9e16578b27de2985d24c25aae507b3540fcd3ff src/test/java/org/apache/aurora/scheduler/cron/quartz/CronIT.java 863e9c998c97506759a5526135a33a161a8fb75f src/test
Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35793/ --- (Updated June 24, 2015, 10:46 p.m.) Review request for Aurora and Maxim Khutornenko. Changes --- Moved row cleanup to a periodic garbage collection routine. Bugs: AURORA-1298 https://issues.apache.org/jira/browse/AURORA-1298 Repository: aurora Description --- DbTaskStore: delete unreferenced job keys. Diffs (updated) - src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java c31446c447c3385a4763b8a516827988e46cc480 src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java 11c9c4ada400d51fc83e9e0de03108456be15fdf src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java 335d7a95e797fe940e71b10da44cbd97edea69ac src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java 2dc3034a5e4389588b5f796ff8dfb06dbc9939b8 src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 30e3469a9f69091b929a9243f84036fa2fdd0539 src/main/java/org/apache/aurora/scheduler/storage/db/GarbageCollectedTableMapper.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/JobKeyMapper.java afdaa49d6cd5c135f6e4ddda2b6a45d189560e09 src/main/java/org/apache/aurora/scheduler/storage/db/LockKeyMapper.java 5e9ba823d08d2b46342e7722c9df5f2a349c97cf src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java PRE-CREATION src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 3ada6286e6ef6e3302802b74eec6c46dd582dc10 src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 7ee001f9c019a1e7b669ae5cec6088bf974a3746 src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml f5829ac063272123995193caef5151e0d52d435b src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml 8258fb102b7f5fca9635143ebaed542d43abeb9f src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 24cf52680b69e23f5ccbbcada0606975b0405d5b src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 63a784f843eb7edf9a13c623e5355169c7e8623b src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java dda988d03634f8de582cf2b8ccdeb433c3e3de0c src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java PRE-CREATION Diff: https://reviews.apache.org/r/35793/diff/ Testing --- Note that this removes a defensive branch wherein we checked for inbound config references before attempting to delete. With this change, we proactively delete and count on foreign key constraints to prevent deletion of rows that are still referenced. I propose we adopt this as our pattern for handling shared references, as it seems to be the most sane approach available. A gotcha with this case is that i do not believe mybatis provides a vendor-neutral approach to identify a consistency violation, and the best signal is a generic `PersistenceException`. This is unfortunate since we can't distinguish between a hopeless query with invalid syntax, a network disconnection, or the anticipated case of a consistency violation. Thanks, Bill Farner
Re: Review Request 35587: Suppress task reconciliation status update logging.
On June 18, 2015, 4:42 p.m., Bill Farner wrote: src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java, line 314 https://reviews.apache.org/r/35587/diff/1/?file=986660#file986660line314 What's being tested here? If i'm reading correctly, this test case would pass before the patch. Maxim Khutornenko wrote: This is mostly for test coverage similarly to the testOfferFirstAcceptsFineLogging above. Asserting a message would require replacing current anonymous logger with a mock and deeper refactoring. Do you think it's worth it here? You could pass a mock/stub `Logger` and expect the behavior (if you do, please be vague with expectations to avoid coupling to log message strings, etc). If that's not to your liking, you could punt. - Bill --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/#review88388 --- On June 18, 2015, 5:04 p.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/ --- (Updated June 18, 2015, 5:04 p.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Suppress task reconciliation status update logging. Diffs - src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java f233d5a181bb1f43fbbfe657dbda2cf975aa6895 src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java f0f9ac392973a276028aee3e06517a6e6d960bb6 Diff: https://reviews.apache.org/r/35587/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 35587: Suppress task reconciliation status update logging.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/#review88388 --- src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java (line 198) https://reviews.apache.org/r/35587/#comment140903 A bit more context will help future readers. You could mention that reconciliation is expected to result in many no-op status updates. src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java (line 314) https://reviews.apache.org/r/35587/#comment140904 What's being tested here? If i'm reading correctly, this test case would pass before the patch. - Bill Farner On June 18, 2015, 1:02 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35587/ --- (Updated June 18, 2015, 1:02 a.m.) Review request for Aurora and Bill Farner. Repository: aurora Description --- Suppress task reconciliation status update logging. Diffs - src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java f233d5a181bb1f43fbbfe657dbda2cf975aa6895 src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java f0f9ac392973a276028aee3e06517a6e6d960bb6 Diff: https://reviews.apache.org/r/35587/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Re: Review Request 35639: Filtering explicit reconciliation tasks by SLAVE_ASSIGNED_STATES.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35639/#review88477 --- Ship it! Ship It! - Bill Farner On June 19, 2015, 1:21 a.m., Maxim Khutornenko wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35639/ --- (Updated June 19, 2015, 1:21 a.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1361 https://issues.apache.org/jira/browse/AURORA-1361 Repository: aurora Description --- Filtering explicit reconciliation tasks by SLAVE_ASSIGNED_STATES. Diffs - src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java ccdbc029f3ce2ef51b777bfc06ee31ba78254d8a src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java 9ed1dba04667df52ba1c1538709a043308828763 Diff: https://reviews.apache.org/r/35639/diff/ Testing --- ./gradlew -Pq build Thanks, Maxim Khutornenko
Review Request 35630: DbTaskStore perf: add a task store API to list task job keys.
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35630/
---
Review request for Aurora and Maxim Khutornenko.
Bugs: AURORA-1298
https://issues.apache.org/jira/browse/AURORA-1298
Repository: aurora
Description
---
DbTaskStore perf: add a task store API to list task job keys.
Diffs
-
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
5242eba2e1a5b35ba3d321f96e2c6670afed4c11
src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
2768e6eafa51f6df78073715a7caf10bb7adba25
src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
76f65dab5a5cb8d97ada962a06a7d0e191739119
src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java
9903675c9d5ee34aebbc52c85fbf02929422a3bf
src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
3a9de60d7e743634815b83587bd61814d883bf86
src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
30e579c46168b64bdc4ba2416d49cf34b4e5c5f6
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml
c3ab3eb9bd6289f73db13e7a7978b5d47621bb52
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
0f3a1a0ffde16f629422caf868fadb59be70e418
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
a854ed3d99af08fb46e436eb1bc00d8fb312278e
Diff: https://reviews.apache.org/r/35630/diff/
Testing
---
GetRoleSummaryBenchmark results using MemTaskStore:
```
{roles: 1} 6234.156 ± 41.303
{roles: 10} 448.273 ± 130.781
{roles: 100} 13.768 ± 2.186
{roles: 500} 2.017 ± 3.046
{jobs: 1} 7252.866 ± 295.430
{jobs: 10} 6034.267 ± 256.936
{jobs: 100} 6081.036 ± 272.341
{jobs: 500} 6108.709 ± 59.118
{instances: 1} 14591.270 ± 2589.012
{instances: 10} 11790.114 ± 1595.596
{instances: 100} 6332.318 ± 177.585
{instances: 1000} 1078.874 ± 25.261
{instances: 1} 23.874 ± 0.911
```
Results using DbTaskStore with this patch:
```
{roles: 1} 450002.057 ± 54941.670
{roles: 10} 24.857 ± 5335.829
{roles: 100} 12377.483 ± 800.637
{roles: 500} 2388.166 ± 101.003
{jobs: 1} 431646.943 ± 34172.295
{jobs: 10} 424383.191 ± 19939.399
{jobs: 100} 417483.093 ± 16342.521
{jobs: 500} 392792.143 ± 27064.583
{instances: 1} 454656.616 ± 21980.375
{instances: 10} 460502.356 ± 19182.511
{instances: 100} 444265.855 ± 28006.036
{instances: 1000} 419068.664 ± 49878.394
{instances: 1} 404739.919 ± 13857.501
```
Seems like we're able to take pretty good advantage of caching in myBatis
and/or H2, at which point performance mostly varies with the response size.
Thanks,
Bill Farner
Re: Review Request 35627: Explicitly bind SessionContext.
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35627/#review88440 --- Ship it! src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java (line 40) https://reviews.apache.org/r/35627/#comment140957 Given how non-obvious this was, it deserves a comment to indicate why this is necessary. - Bill Farner On June 18, 2015, 9:40 p.m., Kevin Sweeney wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35627/ --- (Updated June 18, 2015, 9:40 p.m.) Review request for Aurora and Bill Farner. Bugs: AURORA-1352 https://issues.apache.org/jira/browse/AURORA-1352 Repository: aurora Description --- Explicitly bind SessionContext. Discussion on bugs like this and a potential solution (enabling `requireExplicitBindings`) are here: https://github.com/google/guice/issues/740 Diffs - src/main/java/org/apache/aurora/auth/UnsecureAuthModule.java c89ff0fed60af04f04177e9cdfba6a24b62c2e97 src/main/java/org/apache/aurora/auth/UnsecureSessionContext.java 57132ac480702c93934ada198475203b0648ad6a src/test/java/org/apache/aurora/auth/UnsecureSessionContextTest.java 0a842cb9cdd266690b2d3103126e831fe07b1735 src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh 4d6043a402a761fe44239e33b83c1c8872fe7068 Diff: https://reviews.apache.org/r/35627/diff/ Testing --- Ran kerberos e2e test. Thanks, Kevin Sweeney
Re: Review Request 35630: DbTaskStore perf: add a task store API to list task job keys.
On June 18, 2015, 11:08 p.m., Maxim Khutornenko wrote:
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml,
line 167
https://reviews.apache.org/r/35630/diff/1/?file=987636#file987636line167
Is DISTINCT necessary given the Set result type?
Functionally it's not necessary, but why not dedupe as low in the stack as we
can? If the database is on a separate host, for example, we save on the
network.
- Bill
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35630/#review88452
---
On June 18, 2015, 10:39 p.m., Bill Farner wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35630/
---
(Updated June 18, 2015, 10:39 p.m.)
Review request for Aurora and Maxim Khutornenko.
Bugs: AURORA-1298
https://issues.apache.org/jira/browse/AURORA-1298
Repository: aurora
Description
---
DbTaskStore perf: add a task store API to list task job keys.
Diffs
-
src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java
5242eba2e1a5b35ba3d321f96e2c6670afed4c11
src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java
2768e6eafa51f6df78073715a7caf10bb7adba25
src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
76f65dab5a5cb8d97ada962a06a7d0e191739119
src/main/java/org/apache/aurora/scheduler/storage/db/TaskMapper.java
9903675c9d5ee34aebbc52c85fbf02929422a3bf
src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java
3a9de60d7e743634815b83587bd61814d883bf86
src/main/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImpl.java
30e579c46168b64bdc4ba2416d49cf34b4e5c5f6
src/main/resources/org/apache/aurora/scheduler/storage/db/TaskMapper.xml
c3ab3eb9bd6289f73db13e7a7978b5d47621bb52
src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
0f3a1a0ffde16f629422caf868fadb59be70e418
src/test/java/org/apache/aurora/scheduler/thrift/ReadOnlySchedulerImplTest.java
a854ed3d99af08fb46e436eb1bc00d8fb312278e
Diff: https://reviews.apache.org/r/35630/diff/
Testing
---
GetRoleSummaryBenchmark results using MemTaskStore:
```
{roles: 1} 6234.156 ± 41.303
{roles: 10} 448.273 ± 130.781
{roles: 100} 13.768 ± 2.186
{roles: 500} 2.017 ± 3.046
{jobs: 1} 7252.866 ± 295.430
{jobs: 10} 6034.267 ± 256.936
{jobs: 100} 6081.036 ± 272.341
{jobs: 500} 6108.709 ± 59.118
{instances: 1} 14591.270 ± 2589.012
{instances: 10} 11790.114 ± 1595.596
{instances: 100} 6332.318 ± 177.585
{instances: 1000} 1078.874 ± 25.261
{instances: 1} 23.874 ± 0.911
```
Results using DbTaskStore with this patch:
```
{roles: 1} 450002.057 ± 54941.670
{roles: 10} 24.857 ± 5335.829
{roles: 100} 12377.483 ± 800.637
{roles: 500} 2388.166 ± 101.003
{jobs: 1} 431646.943 ± 34172.295
{jobs: 10} 424383.191 ± 19939.399
{jobs: 100} 417483.093 ± 16342.521
{jobs: 500} 392792.143 ± 27064.583
{instances: 1} 454656.616 ± 21980.375
{instances: 10} 460502.356 ± 19182.511
{instances: 100} 444265.855 ± 28006.036
{instances: 1000} 419068.664 ± 49878.394
{instances: 1} 404739.919 ± 13857.501
```
Seems like we're able to take pretty good advantage of caching in myBatis
and/or H2, at which point performance mostly varies with the response size.
Thanks,
Bill Farner
