Review Request 68110: Update URLS for git repositories

2018-07-30 Thread Jordan Ly
- CONTRIBUTING.md daf95bc94befcf425da2fd32fffcdaec93f3706f docs/development/committers-guide.md 2650f19d057cd17b62b80833dc4a53f7f5398edf ui/package.json 567dd78a359ec4a9f167689648decb108a6247cf Diff: https://reviews.apache.org/r/68110/diff/1/ Testing --- Thanks, Jordan Ly

Re: Review Request 68071: Prune updates that have no surviving job keys in the TaskStore

2018-07-26 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68071/#review206520 --- Ship it! Ship It! - Jordan Ly On July 26, 2018, 10:02 p.m

Review Request 68047: Add size metric for memory stores, add MemSchedulerStoreTest

2018-07-25 Thread Jordan Ly
and ensure the new metrics are being recorded. Thanks, Jordan Ly

Re: Review Request 67967: Unhandled exception should not strand runner in STARTING state.

2018-07-18 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67967/#review206206 --- Ship it! Ship It! - Jordan Ly On July 18, 2018, 8:27 p.m

Re: Review Request 67696: Enable SLA-aware updates

2018-07-17 Thread Jordan Ly
events occuring concurrently. Scheduler stability did not seem to be affected. Thanks, Jordan Ly

Re: Review Request 67696: Enable SLA-aware updates

2018-07-17 Thread Jordan Ly
before being evaluated for the update, then we will kill A' and update the configuration Lots of moving parts for updates so hopefully what I said makes sense :/ - Jordan --- This is an automatically generated e-mail

Re: Review Request 67696: Enable SLA-aware updates

2018-07-17 Thread Jordan Ly
-aware instance update events occuring concurrently. Scheduler stability did not seem to be affected. Thanks, Jordan Ly

Re: Review Request 67696: Enable SLA-aware updates

2018-07-16 Thread Jordan Ly
/67696/diff/8-9/ Testing --- Added unit tests, `./gradlew test`. Tested at scale with over 10,000 SLA-aware instance update events occuring concurrently. Scheduler stability did not seem to be affected. Thanks, Jordan Ly

Re: Review Request 67696: Enable SLA-aware updates

2018-07-12 Thread Jordan Ly
; > Drop `_` here and everywhere. `SLA_CHECKING` sounds like a valid event > > that is definied in code. > > Jordan Ly wrote: > Can you elaborate on this? Did you mean drop `_MESSAGE`? I added > `_MESSAGE` because it is not a true "event" in the sense tha

Re: Review Request 67696: Enable SLA-aware updates

2018-07-12 Thread Jordan Ly
/diff/7-8/ Testing --- Added unit tests, `./gradlew test`. Tested at scale with over 10,000 SLA-aware instance update events occuring concurrently. Scheduler stability did not seem to be affected. Thanks, Jordan Ly

Re: Review Request 67696: Enable SLA-aware updates

2018-07-09 Thread Jordan Ly
/ Testing --- Added unit tests, `./gradlew test`. Tested at scale with over 10,000 SLA-aware instance update events occuring concurrently. Scheduler stability did not seem to be affected. Thanks, Jordan Ly

Re: Review Request 67696: Enable SLA-aware updates

2018-07-09 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67696/#review205879 --- @ReviewBot retry - Jordan Ly On July 9, 2018, 11:12 p.m

Re: Review Request 67696: Enable SLA-aware updates

2018-07-09 Thread Jordan Ly
://reviews.apache.org/r/67696/diff/5-6/ Testing --- Added unit tests, `./gradlew test`. Tested at scale with over 10,000 SLA-aware instance update events occuring concurrently. Scheduler stability did not seem to be affected. Thanks, Jordan Ly

Re: Review Request 67696: Enable SLA-aware updates

2018-07-09 Thread Jordan Ly
t; Drop `_` here and everywhere. `SLA_CHECKING` sounds like a valid event > > that is definied in code. Can you elaborate on this? Did you mean drop `_MESSAGE`? I added `_MESSAGE` because it is not a true "event" in the sense that it piggybacks on `INSTANCE_UPDATING` or `INSTANCE_ROLLING_BACK` with a message showing progress. - Jordan -

Re: Review Request 67696: Enable SLA-aware updates

2018-07-08 Thread Jordan Ly
://reviews.apache.org/r/67696/diff/4-5/ Testing --- Added unit tests, `./gradlew test`. Tested at scale with over 10,000 SLA-aware instance update events occuring concurrently. Scheduler stability did not seem to be affected. Thanks, Jordan Ly

Re: Review Request 67696: Enable SLA-aware updates

2018-07-03 Thread Jordan Ly
--- Added unit tests, `./gradlew test`. Tested at scale with over 10,000 SLA-aware instance update events occuring concurrently. Scheduler stability did not seem to be affected. Thanks, Jordan Ly

Re: Review Request 67696: Enable SLA-aware updates

2018-07-03 Thread Jordan Ly
/2-3/ Testing --- Added unit tests, `./gradlew test`. Tested at scale with over 10,000 SLA-aware instance update events occuring concurrently. Scheduler stability did not seem to be affected. Thanks, Jordan Ly

Re: Review Request 67696: Enable SLA-aware updates

2018-06-21 Thread Jordan Ly
-aware instance update events occuring concurrently. Scheduler stability did not seem to be affected. Thanks, Jordan Ly

Review Request 67696: Enable SLA-aware updates

2018-06-21 Thread Jordan Ly
Diff: https://reviews.apache.org/r/67696/diff/1/ Testing --- Added unit tests, `./gradlew test`. Tested at scale with over 10,000 SLA-aware instance update events occuring concurrently. Scheduler stability did not seem to be affected. Thanks, Jordan Ly

Re: Review Request 67657: Introduce a `countdown-ms` param in Coordinator request.

2018-06-20 Thread Jordan Ly
metadata like countdown-ms inside the query param of a POST request. If we add more metadata, we could soon have very long query strings. Overall, I don't have a strong preference either way. I would probably err towards adding a metadata field to the json body, but no blocking concerns. - Jordan Ly

Re: Review Request 67639: Export count-down to forceful Maintenace as a metric.

2018-06-19 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67639/#review205005 --- Ship it! Ship It! - Jordan Ly On June 19, 2018, 1:21 a.m

Re: Review Request 67638: Export number of tasks lost per dedicated role.

2018-06-18 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67638/#review204965 --- Ship it! Ship It! - Jordan Ly On June 18, 2018, 11:20 p.m

Re: Review Request 67613: Close AsyncHttpClient on scheduler shutdown.

2018-06-15 Thread Jordan Ly
> On June 15, 2018, 6:03 p.m., Jordan Ly wrote: > > Oops missed one thing: > > > > You need to add a scheduler active binding in the module: > > ``` > > SchedulerServicesModule.addSchedulerActiveServiceBinding(binder()) > > .to([SOMETHING].cla

Re: Review Request 67613: Close AsyncHttpClient on scheduler shutdown.

2018-06-15 Thread Jordan Ly
active binding in the module: ``` SchedulerServicesModule.addSchedulerActiveServiceBinding(binder()) .to([SOMETHING].class); ``` - Jordan Ly On June 15, 2018, 6:02 p.m., Santhosh Kumar Shanmugham wrote

Re: Review Request 67613: Close AsyncHttpClient on scheduler shutdown.

2018-06-15 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67613/#review204856 --- Ship it! Ship It! - Jordan Ly On June 15, 2018, 5:58 p.m

Re: Review Request 66716: Enable `Tasks` to specify their own custom maintenance SLA.

2018-05-25 Thread Jordan Ly
gt; nit: why double dashes? src/main/java/org/apache/aurora/scheduler/sla/SlaManager.java Lines 384 (patched) <https://reviews.apache.org/r/66716/#comment286264> Would it be useful to add a log message here for forcing through SLA requirements? - Jordan Ly On May 25, 2018, 12

Re: Review Request 66716: Enable `Tasks` to specify their own custom maintenance SLA.

2018-05-23 Thread Jordan Ly
/state/MaintenanceController.java Lines 453 (patched) <https://reviews.apache.org/r/66716/#comment285990> Remove `ISlaPolicy.build` src/test/java/org/apache/aurora/scheduler/sla/SlaManagerTest.java Lines 1018 (patched) <https://reviews.apache.org/r/66716/#comment285997> `Thread.sleep(15

Re: Review Request 67141: Introduce structs to enable specifying custom SLA.

2018-05-21 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67141/#review203525 --- @ReviewBot retry - Jordan Ly On May 17, 2018, 2:07 a.m

Re: Review Request 67219: Fix flaky Webhook test by ensuring proper error condition

2018-05-21 Thread Jordan Ly
/apache/aurora/scheduler/events/WebhookTest.java 3e10c57e00ba12725310bd50bd55743bec95a77b Diff: https://reviews.apache.org/r/67219/diff/5/ Changes: https://reviews.apache.org/r/67219/diff/4-5/ Testing --- `./gradlew test` passes. Repeated AuroraBot tests. Thanks, Jordan Ly

Re: Review Request 67219: Fix flaky Webhook test by ensuring proper error condition

2018-05-21 Thread Jordan Ly
(updated) --- `./gradlew test` passes. Repeated AuroraBot tests. Thanks, Jordan Ly

Re: Review Request 67219: [IGNORE] testing flaky webhook test

2018-05-18 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67219/#review203458 --- @ReviewBot retry - Jordan Ly On May 18, 2018, 6:43 p.m

Review Request 67219: [IGNORE] testing flaky webhook test

2018-05-18 Thread Jordan Ly
--- Thanks, Jordan Ly

Re: Review Request 67141: Introduce structs to enable specifying custom SLA.

2018-05-16 Thread Jordan Ly
we use a feature toggle here operators can enable the backwards > > incompatible change after they have vetted the release (i.e. once they are > > sure they don't need to do a rollback for unrelated issues). > > > > We can then simply enable the feature toggle

Re: Review Request 67141: Introduce structs to enable specifying custom SLA.

2018-05-16 Thread Jordan Ly
> On May 16, 2018, 5:02 p.m., Jordan Ly wrote: > > Mostly LGTM. > > > > This patch only allows for a better rollback story if you are developing > > from HEAD? You might have to cut a release after this patch so there is > > -1/+1 version compatabili

Re: Review Request 67141: Introduce structs to enable specifying custom SLA.

2018-05-16 Thread Jordan Ly
torage/durability/WriteRecorder.java Lines 266-267 (patched) <https://reviews.apache.org/r/67141/#comment285339> Same as above. - Jordan Ly On May 15, 2018, 9:15 p.m., Santhosh Kumar Shanmugham wrote: > > --- > Thi

Re: Review Request 67141: Introduce structs to enable specifying custom SLA.

2018-05-16 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/67141/#review203242 --- @ReviewBot retry - Jordan Ly On May 15, 2018, 9:15 p.m

Re: Review Request 66716: [WIP] Enable `Tasks` to specify their own custom maintenance SLA.

2018-05-10 Thread Jordan Ly
.py Line 170 (original), 171 (patched) <https://reviews.apache.org/r/66716/#comment284880> nit: newline src/main/python/apache/aurora/executor/executor_vars.py Line 65 (original), 65 (patched) <https://reviews.apache.org/r/66716/#comment28488

Re: Review Request 66716: [WIP] Enable `Tasks` to specify their own custom maintenance SLA.

2018-05-07 Thread Jordan Ly
d into a list or joined into a string. This will print out an object reference. - Jordan Ly On May 1, 2018, 9:19 p.m., Santhosh Kumar Shanmugham wrote: > > --- > This is an automatically generated e-mail. To reply, visit: >

Re: Review Request 66716: [WIP] Enable `Tasks` to specify their own custom maintenance SLA.

2018-05-03 Thread Jordan Ly
omment284152> I think this (+ its dependencies and options) may be better suited for the `SlaModule`. src/main/java/org/apache/aurora/scheduler/state/StateModule.java Lines 131-140 (patched) <https://reviews.apache.org/r/66716/#comment284153> I would create a `MaintenanceModule` and mo

Re: Review Request 66922: Changing Vagrant requirements to latest version for launching our local dev box.

2018-05-03 Thread Jordan Ly
:/ Vagrantfile Lines 20-21 (original), 20-21 (patched) <https://reviews.apache.org/r/66922/#comment284109> Should we need to update (or remove) this comment? - Jordan Ly On May 2, 2018, 10:08 p.m., Renan DelValle

Re: Review Request 66806: Breakdown resource stats by role

2018-04-26 Thread Jordan Ly
y role. ``` Not a huge deal though src/main/java/org/apache/aurora/scheduler/stats/TaskStatCalculator.java Lines 64 (patched) <https://reviews.apache.org/r/66806/#comment283681> rename from `test` to something more descriptive - Jordan Ly On April 26, 2018, 5:51 p.m., David McLa

Re: Review Request 66716: [WIP] Enable `Tasks` to specify their own custom maintenance SLA.

2018-04-20 Thread Jordan Ly
utes an action provided by the caller. I am thinking about this in terms of my proposed SLA-aware updates -- I would be able to utilize this interface as well. src/main/java/org/apache/aurora/scheduler/storage/HostMaintenanceStore.java Lines 22-28 (patched) <https://reviews.apache.org/r/66716/#c

Review Request 66573: Add initial interval before searching for preemption slots

2018-04-11 Thread Jordan Ly
interval. Thanks, Jordan Ly

Review Request 66570: Remove flaky test/assertion in PendingTaskProcessorTest

2018-04-11 Thread Jordan Ly
0bd8d214f49aeb09721cd33776ef213e9cf00347 Diff: https://reviews.apache.org/r/66570/diff/1/ Testing --- `./gradlew test` passes. Manually forcing `JOB_A` (the condition that caused the error before) to get searched first passes. Thanks, Jordan Ly

Re: Review Request 66536: Add more preemption metrics (jobs preempted, preemptors) and logging statements

2018-04-11 Thread Jordan Ly
at scale. Thanks, Jordan Ly

Review Request 66536: Add more preemption metrics (jobs preempted, preemptors) and logging statements

2018-04-10 Thread Jordan Ly
/ Testing --- Added unit tests, `./gradlew test` passes. Manually ensured new metrics are exported. Tested at scale. Thanks, Jordan Ly

Review Request 66199: Remove unused LOST_LOCK_MESSAGE variable in JobUpdateControllerImpl

2018-03-21 Thread Jordan Ly
://reviews.apache.org/r/66199/diff/1/ Testing --- `./gradlew test` Thanks, Jordan Ly

Re: Review Request 66192: [WIP] Variable group size updates

2018-03-21 Thread Jordan Ly
batch size specified to the variable strategy should behave the same as the current implementation). - Jordan Ly On March 21, 2018, 2:10 a.m., Renan DelValle wrote: > > --- > This is an automatically generated e-mail. To rep

Re: Review Request 66190: Fix 'PreemptorSlotSearchBenchmark', remove 'isProduction' references in benchmark

2018-03-21 Thread Jordan Ly
> On March 21, 2018, 12:10 a.m., Jordan Ly wrote: > > src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java > > Lines 386-387 (original), 386-387 (patched) > > <https://reviews.apache.org/r/66190/diff/1/?file=1984290#file1984290line387> > > > &g

Re: Review Request 66190: Fix 'PreemptorSlotSearchBenchmark', remove 'isProduction' references in benchmark

2018-03-21 Thread Jordan Ly
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark:·stack 1000 thrpt NaN --- ``` Benchmark is slower as it is doing more (correct) work. Thanks, Jordan Ly

Re: Review Request 66190: Fix 'PreemptorSlotSearchBenchmark', remove 'isProduction' references in benchmark

2018-03-20 Thread Jordan Ly
--- ``` Benchmark is slower as it is doing more (correct) work. Thanks, Jordan Ly

Re: Review Request 66190: Fix 'PreemptorSlotSearchBenchmark', remove 'isProduction' references

2018-03-20 Thread Jordan Ly
s the number of hosts we look at when doing preemptions. - Jordan Ly On March 21, 2018, 12:04 a.m., Jordan Ly wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://re

Review Request 66190: Fix 'PreemptorSlotSearchBenchmark', remove 'isProduction' references

2018-03-20 Thread Jordan Ly
--- ``` Benchmark is slower as it is doing more (correct) work. Thanks, Jordan Ly

Re: Review Request 66136: Switch Thermos to lazy log formatting

2018-03-19 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66136/#review199447 --- Ship it! Ship It! - Jordan Ly On March 19, 2018, 2:55 p.m

Re: Review Request 66136: Switch Thermos to lazy log formatting

2018-03-19 Thread Jordan Ly
8, 2:55 p.m., Stephan Erb wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66136/ > --- > > (Updated Ma

Re: Review Request 65769: Remove unused module in RecoveryTool, move TaskTestUtil to test folder

2018-03-17 Thread Jordan Ly
o reply, visit: https://reviews.apache.org/r/65769/#review199333 --- On Feb. 23, 2018, 6:04 p.m., Jordan Ly wrote: > > --- > This is an automatically generated e-mail. To reply, visit:

Re: Review Request 65896: Persist scheduler/observer logs to /var/log/aurora/[FILE].log

2018-03-14 Thread Jordan Ly
ver[3129]: Bottle v0.11.6 server starting up (using CherryPyServer())... Mar 05 00:36:18 aurora aurora-observer[3129]: Listening on http://192.168.33.7:1338/ Mar 05 00:36:18 aurora aurora-observer[3129]: Hit Ctrl-C to quit. ``` Thanks, Jordan Ly

Re: Review Request 65769: Remove unused module in RecoveryTool, move TaskTestUtil to test folder

2018-03-14 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65769/#review199219 --- bump - Jordan Ly On Feb. 23, 2018, 6:04 p.m., Jordan Ly wrote

Review Request 66074: Refactor ClusterState to more appropriate package, move binding to StateModule

2018-03-14 Thread Jordan Ly
/test/java/org/apache/aurora/scheduler/preemptor/PendingTaskProcessorTest.java 35e9348e6ab0c14e6ae20af4076358d7e12681c7 Diff: https://reviews.apache.org/r/66074/diff/1/ Testing --- Manually tested state endpoint returns jobs when preemptor is both on and off. Thanks, Jordan Ly

Re: Review Request 65941: Avoid scheduling on the same host the ancestor of a task recently failed on

2018-03-07 Thread Jordan Ly
> On March 7, 2018, 6:48 p.m., David McLaughlin wrote: > > So what happens if there are two bad hosts? :) > > Jordan Ly wrote: > This does not scale past n=1 > > We can make this more generic by getting the list of hosts the task has > previously fa

Re: Review Request 65941: Avoid scheduling on the same host the ancestor of a task recently failed on

2018-03-07 Thread Jordan Ly
iew198803 --- On March 7, 2018, 5:50 a.m., Jordan Ly wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://revie

Review Request 65941: Avoid scheduling on the same host the ancestor of a task recently failed on

2018-03-06 Thread Jordan Ly
/TaskAssignerImplTest.java 864538b6730d7318385494818276ba370124b8e9 Diff: https://reviews.apache.org/r/65941/diff/1/ Testing --- `./gradlew test` Benchmarks and live-cluster testing coming soon. Thanks, Jordan Ly

Review Request 65896: Persist scheduler/observer logs to /var/log/aurora/[FILE].log

2018-03-04 Thread Jordan Ly
]: Bottle v0.11.6 server starting up (using CherryPyServer())... Mar 05 00:36:18 aurora aurora-observer[3129]: Listening on http://192.168.33.7:1338/ Mar 05 00:36:18 aurora aurora-observer[3129]: Hit Ctrl-C to quit. ``` Thanks, Jordan Ly

Re: Review Request 65873: Upgrade RBT to 0.7.11

2018-03-01 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65873/#review198498 --- Ship it! Any notable changes? - Jordan Ly On March 2, 2018

Review Request 65769: Remove unused module in RecoveryTool, move TaskTestUtil to test folder

2018-02-23 Thread Jordan Ly
: https://reviews.apache.org/r/65769/diff/1/ Testing --- `./gradlew test` `./gradlew jmh` end-to-end tests pass. Thanks, Jordan Ly

Review Request 65761: Use StandardCharset instead of Charset.forName in ApiModule

2018-02-22 Thread Jordan Ly
--- `./gradlew test` Manually verified Firefox, Chrome, and Safari. Thanks, Jordan Ly

Re: Review Request 65690: Make charset parsing in HTTP headers case insensitve

2018-02-16 Thread Jordan Ly
ne has a way to confirm please let me know :) - Jordan Ly On Feb. 16, 2018, 8:59 p.m., Jordan Ly wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.

Review Request 65690: Make charset parsing in HTTP headers case insensitve

2018-02-16 Thread Jordan Ly
bfd117b4988c35c6b3f95508b2924bbd19b0c692 Diff: https://reviews.apache.org/r/65690/diff/1/ Testing --- `./gradlew test` Manual testing in Safari, Chrome, and Firefox. Thanks, Jordan Ly

Re: Review Request 65680: Fix cron id collision bug by avoiding state in Quartz jobs

2018-02-16 Thread Jordan Ly
s.apache.org/r/65680/diff/2/ Changes: https://reviews.apache.org/r/65680/diff/1-2/ Testing --- Repro'd bug using steps above. Validated code changes solved issue. Added small change in unit test to ensure state is now preserved in the Quartz job context. `./gradlew test` Thanks, Jordan Ly

Review Request 65680: Fix cron id collision bug by avoiding state in Quartz jobs

2018-02-15 Thread Jordan Ly
ted code changes solved issue. Added small change in unit test to ensure state is now preserved in the Quartz job context. `./gradlew test` Thanks, Jordan Ly

Re: Review Request 65648: Do not reschedule a PARTITIONED task if it was in KILLING state

2018-02-14 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65648/#review197574 --- Ship it! Ship It! - Jordan Ly On Feb. 14, 2018, 5:27 a.m

Re: Review Request 65598: Disable pytest-fast mode as a workaround for failing health checker tests

2018-02-14 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65598/#review197488 --- Ship it! Ship It! - Jordan Ly On Feb. 12, 2018, 8:36 a.m

Review Request 65650: Add GPG key for jorda...@apache.org

2018-02-14 Thread Jordan Ly
: aurora Description --- Add GPG key for jorda...@apache.org Diffs - KEYS 843e955e8f69e4d36b61df66aa20a1c914cb77c9 Diff: https://reviews.apache.org/r/65650/diff/1/ Testing --- Thanks, Jordan Ly

Re: Review Request 65649: Adding support for a Thrift JSON request which defines UTF-8 as the charset for the Content-Type in the Request Headers

2018-02-14 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65649/#review197487 --- Ship it! Ship It! - Jordan Ly On Feb. 14, 2018, 5:05 a.m

Re: Review Request 65433: Update Javascript Thrift to 0.10

2018-02-12 Thread Jordan Ly
correctly but inspecting the `thrift.js` file being return showed that the version was still `0.91`. I had to do a `git clean -fdx` and destroy/bring up the vagrant cluster again in order to reproduce the error above. - Jordan Ly On Jan. 31, 2018, 10:11 a.m., Stephan Erb wrote

Re: Review Request 65537: Use overflow to prevent overlapping config summary tables

2018-02-06 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65537/#review196936 --- Ship it! Ship It! - Jordan Ly On Feb. 6, 2018, 10:11 p.m

Re: Review Request 65434: Ensure primary_port warning respects announcer portmap

2018-01-31 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65434/#review196603 --- Ship it! Ship It! - Jordan Ly On Jan. 31, 2018, 10:57 a.m

Re: Review Request 65303: Improve performance of MemTaskStore queries

2018-01-31 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65303/#review196592 --- Ship it! Ship It! - Jordan Ly On Jan. 31, 2018, 6:12 p.m

Re: Review Request 65338: Fix error handling logic for launch failures

2018-01-25 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65338/#review196275 --- Ship it! Ship It! - Jordan Ly On Jan. 25, 2018, 6:21 p.m

Re: Review Request 65281: Support PARTITIONED state in SLA calculations

2018-01-23 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65281/#review196046 --- Ship it! Nice test for future cases. - Jordan Ly On Jan. 23

Re: Review Request 65233: Allow for injection of custom OfferSets, removed OfferOrder and OfferSelector

2018-01-19 Thread Jordan Ly
ld be a module, right? Yep! Good catch, changed. - Jordan --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/65233/#review195867 --- On Ja

Re: Review Request 65233: Allow for injection of custom OfferSets, removed OfferOrder and OfferSelector

2018-01-19 Thread Jordan Ly
--- `./gradlew test` `./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh` Testing injection of a custom `OfferSet` onto a test cluster. Thanks, Jordan Ly

Re: Review Request 65233: Allow for injection of custom OfferSets, removed OfferOrder and OfferSelector

2018-01-19 Thread Jordan Ly
s that the number of tasks per agent can > > shift drastically, sometimes even up to a point where the Thermos observer > > is completely unusable). > > Jordan Ly wrote: > Some context for the new interface: > > For performance, it is helpful to control the data struc

Review Request 65225: Allow custom data structures in HostOffers injected via Guice modules

2018-01-18 Thread Jordan Ly
1e36b2c3094e99f05aa4a4f098a48df2293b4320 Diff: https://reviews.apache.org/r/65225/diff/1/ Testing --- TODO: I am planning on adding tests for `FilterableCollection`, just wanted to put this out there for quick comments. I've done some ad-hoc testing of injecting custom collection types. Thanks, Jordan Ly

Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-09 Thread Jordan Ly
/apache/aurora/scheduler/scheduling/TaskAssignerImplTest.java Lines 369-372 (original), 356-358 (patched) <https://reviews.apache.org/r/64954/#comment274180> nit: can you just expect 0 - Jordan Ly On Jan. 9, 2018, 6:32 p.m., Bill Farner

Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-04 Thread Jordan Ly
duling/TaskSchedulerImpl.java Lines 152-158 (original), 141-144 (patched) <https://reviews.apache.org/r/64954/#comment273872> Is this check effectively the same as the check in `fetchTasks`? Unless `ids` can be empty... - Jordan Ly On Jan. 4, 2018, 7:08 p.m., Bill

Re: Review Request 64954: Refactor scheduling code to split matching and assigning phases

2018-01-04 Thread Jordan Ly
- On Jan. 4, 2018, 7:08 p.m., Bill Farner wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/64954/ > --- > > (Updated Jan. 4, 2018, 7:08

Re: Review Request 64519: Add a test to detect incompatible storage changes

2018-01-03 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64519/#review194729 --- Ship it! This LGTM! - Jordan Ly On Dec. 12, 2017, 1:35 a.m

Re: Review Request 64629: Use java.util.Optional throughout

2017-12-15 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64629/#review193946 --- Ship it! Ship It! - Jordan Ly On Dec. 14, 2017, 11:09 p.m

Re: Review Request 64625: Add a storage recovery tool

2017-12-15 Thread Jordan Ly
t; > (Updated Dec. 14, 2017, 10:05 p.m.) > > > Review request for Aurora, Jordan Ly and Santhosh Kumar Shanmugham. > > > Repository: aurora > > > Description > --- > > This tool was originally intended as a migration path between Persistenc

Re: Review Request 64286: Recover snapshots via the Op stream

2017-12-13 Thread Jordan Ly
> On Dec. 14, 2017, 2:07 a.m., Jordan Ly wrote: > > src/main/java/org/apache/aurora/scheduler/storage/backup/TemporaryStorage.java > > Line 88 (original), 90 (patched) > > <https://reviews.apache.org/r/64286/diff/3/?file=1914592#file1914592line90> > > &g

Re: Review Request 64286: Recover snapshots via the Op stream

2017-12-13 Thread Jordan Ly
rc/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java Lines 100 (patched) <https://reviews.apache.org/r/64286/#comment272393> Do we need to map edit -> op? - Jordan Ly On Dec.

Re: Review Request 64523: Attempt #2 to fix flaky Webhook test

2017-12-12 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64523/#review193610 --- @ReviewBot retry - Jordan Ly On Dec. 12, 2017, 7:15 p.m

Re: Review Request 64523: Attempt #2 to fix flaky Webhook test

2017-12-12 Thread Jordan Ly
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/64523/#review193595 --- @ReviewBot retry - Jordan Ly On Dec. 12, 2017, 7:15 p.m

Re: Review Request 64523: Attempt #2 to fix flaky Webhook test

2017-12-12 Thread Jordan Ly
and the unknown cause of the actual errors. Maybe we run the reviewbot on this repo repeatedly? Obviously not the most scientifically sound solution. EDIT 12/12: I tested this by putting a long sleep in `onThrowable` which causes the issue in master and is fixed with this patch. Thanks, Jordan

Re: Review Request 64523: Attempt #2 to fix flaky Webhook test

2017-12-12 Thread Jordan Ly
rowable t) { > > +try { > > + Thread.sleep(1000); > > +} catch (InterruptedException e) { > > +} > > errorsCounter.incrementAndGet(); > > LOG.error("Error sending a Webhook event", t); > > ``` > > > > If i

Re: Review Request 64523: Attempt #2 to fix flaky Webhook test

2017-12-11 Thread Jordan Ly
/aurora/scheduler/events/WebhookTest.java > 1b5d2d02535345edfe6cf04d18d00434393f800b > > > Diff: https://reviews.apache.org/r/64523/diff/1/ > > > Testing > --- > > This change seems pretty hard to test considering the differences in > environment and the unknown cause of the actual errors. Maybe we run the > reviewbot on this repo repeatedly? Obviously not the most scientifically > sound solution. > > > Thanks, > > Jordan Ly > >

Review Request 64523: Attempt #2 to fix flaky Webhook test

2017-12-11 Thread Jordan Ly
/64523/diff/1/ Testing --- This change seems pretty hard to test considering the differences in environment and the unknown cause of the actual errors. Maybe we run the reviewbot on this repo repeatedly? Obviously not the most scientifically sound solution. Thanks, Jordan Ly

Re: Review Request 64459: Deprecated Ops re-added, perform no-op instead of throwing an exception

2017-12-08 Thread Jordan Ly
To reply, visit: https://reviews.apache.org/r/64459/#review193287 ------- On Dec. 8, 2017, 8:08 p.m., Jordan Ly wrote: > > --- > This is an automatically generated e-mail.

Re: Review Request 64286: Recover snapshots via the Op stream

2017-12-08 Thread Jordan Ly
pshot failures propogate, do they kill the scheduler or just the service? If everything stays up, can we increment a counter for failed snapshots? We probably don't want to kill the service if a snapshot fails since that means it will never try to snapshot again. - Jordan Ly On Dec.

  1   2   3   >