Re: Review Request 55105: AURORA-1870 Add finer grained timings to the Snapshot process

2017-02-22 Thread David McLaughlin

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55105/#review166465
---


Ship it!




Ship It!

- David McLaughlin


On Jan. 28, 2017, 9:15 p.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55105/
> ---
> 
> (Updated Jan. 28, 2017, 9:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Stephan Erb.
> 
> 
> Bugs: AURORA-1870
> https://issues.apache.org/jira/browse/AURORA-1870
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1870 Add finer grained timings to the Snapshot process
> 
> I gave up on `@Timed` interceptor approach because major refactoring is 
> required in order to have snapshot fields instantiated by Guice through 
> `Provider` or `@Provides` interfaces. The abstract class approach is much 
> easier/cleaner.
> 
> 
> Diffs
> -
> 
>   commons/src/main/java/org/apache/aurora/common/stats/SlidingStats.java 
> f7a5ae41e307627fc55157758e9b7cdd861c3268 
>   commons/src/test/java/org/apache/aurora/common/stats/SlidingStatsTest.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 7aa111ec14696ae40f518c42f3c7f45d8ab0e94c 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java
>  f56a1624c7188da175ad3e6de323c3442802f2ea 
> 
> Diff: https://reviews.apache.org/r/55105/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ curl localhost:8081/vars | grep snapshot_restore
>   % Total% Received % Xferd  Average Speed   TimeTime Time  
> Current
>  Dload  Upload   Total   SpentLeft  Speed
>   0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 0
> snapshot_restore_crons_events 1
> snapshot_restore_crons_events_per_sec 0.0
> snapshot_restore_crons_nanos_per_event 0.0
> snapshot_restore_crons_nanos_total 73648
> snapshot_restore_crons_nanos_total_per_sec 0.0
> snapshot_restore_dbscript_events 1
> snapshot_restore_dbscript_events_per_sec 0.0
> snapshot_restore_dbscript_nanos_per_event 0.0
> snapshot_restore_dbscript_nanos_total 1148842021
> snapshot_restore_dbscript_nanos_total_per_sec 0.0
> snapshot_restore_hosts_events 1
> snapshot_restore_hosts_events_per_sec 0.0
> snapshot_restore_hosts_nanos_per_event 0.0
> snapshot_restore_hosts_nanos_total 76166
> snapshot_restore_hosts_nanos_total_per_sec 0.0
> snapshot_restore_job_updates_events 1
> snapshot_restore_job_updates_events_per_sec 0.0
> snapshot_restore_job_updates_nanos_per_event 0.0
> snapshot_restore_job_updates_nanos_total 49482
> snapshot_restore_job_updates_nanos_total_per_sec 0.0
> snapshot_restore_locks_events 1
> snapshot_restore_locks_events_per_sec 0.0
> snapshot_restore_locks_nanos_per_event 0.0
> snapshot_restore_locks_nanos_total 125084
> snapshot_restore_locks_nanos_total_per_sec 0.0
> snapshot_restore_quota_events 1
> snapshot_restore_quota_events_per_sec 0.0
> snapshot_restore_quota_nanos_per_event 0.0
> snapshot_restore_quota_nanos_total 52305
> snapshot_restore_quota_nanos_total_per_sec 0.0
> snapshot_restore_scheduler_metadata_events 1
> snapshot_restore_scheduler_metadata_events_per_sec 0.0
> snapshot_restore_scheduler_metadata_nanos_per_event 0.0
> snapshot_restore_scheduler_metadata_nanos_total 70816
> snapshot_restore_scheduler_metadata_nanos_total_per_sec 0.0
> snapshot_restore_tasks_events 1
> snapshot_restore_tasks_events_per_sec 0.0
> snapshot_restore_tasks_nanos_per_event 0.0
> snapshot_restore_tasks_nanos_total 91377
> snapshot_restore_tasks_nanos_total_per_sec 0.0
> ```
> 
> ```
> $ aurora_admin scheduler_snapshot devcluster
>  INFO] Response from scheduler: OK (message: )
>  
> $ curl localhost:8081/vars | grep snapshot_save
>   % Total% Received % Xferd  Average Speed   TimeTime Time  
> Current
>  Dload  Upload   Total   SpentLeft  Speed
> 100 482260 482260 0  3266k  0 --:--:-- --:--:-- --:--:-- 3363k
> snapshot_save_crons_events 1
> snapshot_save_crons_events_per_sec 0.0
> snapshot_save_crons_nanos_per_event 0.0
> snapshot_save_crons_nanos_total 466181
> snapshot_save_crons_nanos_total_per_sec 0.0
> snapshot_save_dbscript_events 1
> snapshot_save_dbscript_events_per_sec 0.0
> snapshot_save_dbscript_nanos_per_event 0.0
> snapshot_save_dbscript_nanos_total 18201542
> snapshot_save_dbscript_nanos_total_per_sec 0.0
> snapshot_save_hosts_events 1
> snapshot_save_hosts_events_per_sec 0.0
> snapshot_save_hosts_nanos_per_event 0.0
> snapshot_save_hosts_nanos_total 1286180
> snapshot_save_hosts_nanos_total_per_sec 0.0
> snapshot_save_job_updates_events 1
> snapshot_save_job_updates_events_per_sec 0.0
> 

Re: Review Request 56691: [Patch 2/2] RFC for 2nd patch implements OfferReconciler for dynamic reservations proposal

2017-02-22 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56691/#review166455
---


Ship it!




Master (98eb99a) is green with this patch.
  ./build-support/jenkins/build.sh

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Feb. 22, 2017, 10:35 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56691/
> ---
> 
> (Updated Feb. 22, 2017, 10:35 p.m.)
> 
> 
> Review request for Aurora, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1816
> https://issues.apache.org/jira/browse/AURORA-1816
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Second patch that implements mechanism for unreserving previously reserved 
> resources for dynamic reservations proposal.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeOfferManager.java 
> 6f2ca35c5d83dde29c24865b4826d4932e96da80 
>   src/main/java/org/apache/aurora/scheduler/HostOffer.java 
> 23f0600d64e1e15f4856f397e839e3d1c87f3b96 
>   src/main/java/org/apache/aurora/scheduler/base/InstanceKeys.java 
> b12ac83168401c15fb1d30179ea8e4816f09cd3d 
>   src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
> 0136afb8f6049a6d88cd42b5e3f17d61fcd629d5 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 
> 0637eb7f85125cf70b588d56fa7dc88130947837 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> 8c000cb0626bd34f6f30e23fe2b3a045f2b44e35 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferReconciler.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> 202cae96ffc5b49e638b973a273f7983137b5baf 
>   src/test/java/org/apache/aurora/scheduler/http/OffersTest.java 
> 30699596a1c95199df7504f62c5c18cab1be1c6c 
>   src/test/java/org/apache/aurora/scheduler/offers/HostOffers.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 49d4e82cc03144b80292fe43066a6cc4d7aed88f 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferReconcilerTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/56691/diff/
> 
> 
> Testing
> ---
> 
> Ran locally on vagrant.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>



Re: Review Request 56691: [Patch 2/2] RFC for 2nd patch implements OfferReconciler for dynamic reservations proposal

2017-02-22 Thread Dmitriy Shirchenko

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56691/#review166450
---



@ReviewBot retry

- Dmitriy Shirchenko


On Feb. 22, 2017, 10:35 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56691/
> ---
> 
> (Updated Feb. 22, 2017, 10:35 p.m.)
> 
> 
> Review request for Aurora, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1816
> https://issues.apache.org/jira/browse/AURORA-1816
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Second patch that implements mechanism for unreserving previously reserved 
> resources for dynamic reservations proposal.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeOfferManager.java 
> 6f2ca35c5d83dde29c24865b4826d4932e96da80 
>   src/main/java/org/apache/aurora/scheduler/HostOffer.java 
> 23f0600d64e1e15f4856f397e839e3d1c87f3b96 
>   src/main/java/org/apache/aurora/scheduler/base/InstanceKeys.java 
> b12ac83168401c15fb1d30179ea8e4816f09cd3d 
>   src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
> 0136afb8f6049a6d88cd42b5e3f17d61fcd629d5 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 
> 0637eb7f85125cf70b588d56fa7dc88130947837 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> 8c000cb0626bd34f6f30e23fe2b3a045f2b44e35 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferReconciler.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> 202cae96ffc5b49e638b973a273f7983137b5baf 
>   src/test/java/org/apache/aurora/scheduler/http/OffersTest.java 
> 30699596a1c95199df7504f62c5c18cab1be1c6c 
>   src/test/java/org/apache/aurora/scheduler/offers/HostOffers.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 49d4e82cc03144b80292fe43066a6cc4d7aed88f 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferReconcilerTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/56691/diff/
> 
> 
> Testing
> ---
> 
> Ran locally on vagrant.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>



Re: Review Request 56690: [Patch 1/2] RFC for first patch implementing scheduling changes for Dynamic Reservations

2017-02-22 Thread Stephan Erb

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56690/#review166430
---



This is only a very partial review. My brain was not capable of more today :)


src/main/java/org/apache/aurora/scheduler/TierInfo.java (lines 66 - 68)


Copy paste error



src/main/java/org/apache/aurora/scheduler/TierInfo.java (lines 74 - 81)


A mutable `TierConfig` makes the code very hard to reason about, especially 
in the context of multi-threading. 

Do you see a way to keep the `TierConfig` immutable at runtime?



src/main/java/org/apache/aurora/scheduler/TierManager.java (lines 100 - 105)


In addition to my concerns about the mutable tier config, the `copyOf` 
might come with a performance tax.

`getTier` is on a hot code path and I therefore recently rewrote the 
`checkArgument` here to only construct the String if the argument is invalid. 
This yielded a significant perf improvement in our scheduling benchmarks.



src/main/java/org/apache/aurora/scheduler/base/InstanceKeys.java (lines 51 - 63)


`IAssignedTask` has a `task` which in turn has a jobkey. There should 
therfore be no need to parse this from the Mesos `TaskInfo`.



src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java (lines 
405 - 406)


Please add a doc string. I may not have completely understood why this is 
absolutely needed. 

We should have a very good reason to break the existing interface.



src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
(lines 99 - 100)


To short-circuit both loops this should probably be a return.



src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java (line 166)


Please make sure you always use the built-in formatting of the logger 
methods. Otherwise we will assemble strings in memory, only to be discarded by 
the logger afterwards :-)



src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java (line 208)


Same here (and in some other places)



src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java (lines 210 - 
228)


This reads like as if there is a more general concept of "soft-constraints" 
waiting to be discovered. In other words: We want constraints that are lifted 
once we cannot satisfy them for a certain amount of time. See 
https://issues.apache.org/jira/browse/AURORA-173 for details.

I imagine the TaskAssigner could keep its current behavior and only assign 
a task if there is no veto for it. It is the responsibility of an out-of-band 
mechanism to ensure we either don't generate vetos for expired 
soft-constraints, or we have a way to check if a generated veto is already 
expired or not. 

(One very rudimentary implementation could probably look similar to 
`TaskTimeout.java` but for the `PENDING` state. However there are most likely 
also other/better ways to do this)



src/main/resources/org/apache/aurora/scheduler/tiers.json (lines 29 - 33)


Shoudn't reservations be always backed by quota to prevent abuse?


- Stephan Erb


On Feb. 15, 2017, 2:44 a.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56690/
> ---
> 
> (Updated Feb. 15, 2017, 2:44 a.m.)
> 
> 
> Review request for Aurora, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1819
> https://issues.apache.org/jira/browse/AURORA-1819
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is an RFC (without tests) for dynamic reservations proposal. If there is 
> consensus on the approach, I will add tests. This patch was also tested 
> locally and works as expected.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> f2296a9d7a88be7e43124370edecfe64415df00f 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeOfferManager.java 
> 6f2ca35c5d83dde29c24865b4826d4932e96da80 
>   src/main/java/org/apache/aurora/scheduler/TaskVars.java 
> 676dfd9f9d7ee0633c05424f788fd0ab116976bb 
>   src/main/java/org/apache/aurora/scheduler/TierInfo.java 
> c45b949ae7946fc92d7e62f94696ddc4f0790cfa 
>   src/main/java/org/apache/aurora/scheduler/TierManager.java 
> 

Re: Review Request 56691: [Patch 2/2] RFC for 2nd patch implements OfferReconciler for dynamic reservations proposal

2017-02-22 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56691/#review166435
---



Master (98eb99a) is red with this patch.
  ./build-support/jenkins/build.sh

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
:commons:generateThriftJava
:commons:classesThriftNote: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:commons-args:compileJava
:commons-args:processResources
:commons-args:classes
:commons-args:jar
:commons:compileJavaNote: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/commons/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.1
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/commons/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor
Note: Some input files use or override a deprecated API.
Note: Recompile with -Xlint:deprecation for details.
Note: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:commons:generateThriftResources
:commons:processResources
:commons:classes
:commons:jar
:compileJava/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java:74:
 Note: Wrote forwarder 
org.apache.aurora.scheduler.storage.log.WriteAheadStorageForwarder
@Forward({
^
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/org/apache/aurora/common/args/apt/cmdline.arg.info.txt.2
Note: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/META-INF/compiler/resource-mappings/org.apache.aurora.common.args.apt.CmdLineProcessor
Java HotSpot(TM) 64-Bit Server VM warning: INFO: 
os::commit_memory(0x0006cdc0, 164626432, 0) failed; error='Cannot 
allocate memory' (errno=12)
#
# There is insufficient memory for the Java Runtime Environment to continue.
# Native memory allocation (mmap) failed to map 164626432 bytes for committing 
reserved memory.
# An error report file with more information is saved as:
# /home/jenkins/jenkins-slave/workspace/AuroraBot/hs_err_pid21381.log


I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Feb. 22, 2017, 10:35 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56691/
> ---
> 
> (Updated Feb. 22, 2017, 10:35 p.m.)
> 
> 
> Review request for Aurora, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1816
> https://issues.apache.org/jira/browse/AURORA-1816
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Second patch that implements mechanism for unreserving previously reserved 
> resources for dynamic reservations proposal.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeOfferManager.java 
> 6f2ca35c5d83dde29c24865b4826d4932e96da80 
>   src/main/java/org/apache/aurora/scheduler/HostOffer.java 
> 23f0600d64e1e15f4856f397e839e3d1c87f3b96 
>   src/main/java/org/apache/aurora/scheduler/base/InstanceKeys.java 
> b12ac83168401c15fb1d30179ea8e4816f09cd3d 
>   src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
> 0136afb8f6049a6d88cd42b5e3f17d61fcd629d5 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 
> 0637eb7f85125cf70b588d56fa7dc88130947837 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> 8c000cb0626bd34f6f30e23fe2b3a045f2b44e35 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferReconciler.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> 202cae96ffc5b49e638b973a273f7983137b5baf 
>   src/test/java/org/apache/aurora/scheduler/http/OffersTest.java 
> 30699596a1c95199df7504f62c5c18cab1be1c6c 
>   src/test/java/org/apache/aurora/scheduler/offers/HostOffers.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 49d4e82cc03144b80292fe43066a6cc4d7aed88f 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferReconcilerTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/56691/diff/
> 
> 
> Testing
> ---
> 
> Ran locally on vagrant.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>



Re: Review Request 56691: [Patch 2/2] RFC for 2nd patch implements OfferReconciler for dynamic reservations proposal

2017-02-22 Thread Dmitriy Shirchenko

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56691/#review166434
---



@ReviewBot retry

- Dmitriy Shirchenko


On Feb. 22, 2017, 10:35 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56691/
> ---
> 
> (Updated Feb. 22, 2017, 10:35 p.m.)
> 
> 
> Review request for Aurora, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1816
> https://issues.apache.org/jira/browse/AURORA-1816
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Second patch that implements mechanism for unreserving previously reserved 
> resources for dynamic reservations proposal.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeOfferManager.java 
> 6f2ca35c5d83dde29c24865b4826d4932e96da80 
>   src/main/java/org/apache/aurora/scheduler/HostOffer.java 
> 23f0600d64e1e15f4856f397e839e3d1c87f3b96 
>   src/main/java/org/apache/aurora/scheduler/base/InstanceKeys.java 
> b12ac83168401c15fb1d30179ea8e4816f09cd3d 
>   src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
> 0136afb8f6049a6d88cd42b5e3f17d61fcd629d5 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 
> 0637eb7f85125cf70b588d56fa7dc88130947837 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> 8c000cb0626bd34f6f30e23fe2b3a045f2b44e35 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferReconciler.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> 202cae96ffc5b49e638b973a273f7983137b5baf 
>   src/test/java/org/apache/aurora/scheduler/http/OffersTest.java 
> 30699596a1c95199df7504f62c5c18cab1be1c6c 
>   src/test/java/org/apache/aurora/scheduler/offers/HostOffers.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 49d4e82cc03144b80292fe43066a6cc4d7aed88f 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferReconcilerTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/56691/diff/
> 
> 
> Testing
> ---
> 
> Ran locally on vagrant.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>



Re: Review Request 56691: [Patch 2/2] RFC for 2nd patch implements OfferReconciler for dynamic reservations proposal

2017-02-22 Thread Aurora ReviewBot

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56691/#review166432
---



This patch does not apply cleanly against RB#56690 (98eb99a), do you need to 
rebase?

I will refresh this build result if you post a review containing "@ReviewBot 
retry"

- Aurora ReviewBot


On Feb. 22, 2017, 10:19 p.m., Dmitriy Shirchenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56691/
> ---
> 
> (Updated Feb. 22, 2017, 10:19 p.m.)
> 
> 
> Review request for Aurora, Mehrdad Nurolahzade, Stephan Erb, and Zameer Manji.
> 
> 
> Bugs: AURORA-1816
> https://issues.apache.org/jira/browse/AURORA-1816
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Second patch that implements mechanism for unreserving previously reserved 
> resources for dynamic reservations proposal.
> 
> 
> Diffs
> -
> 
>   src/jmh/java/org/apache/aurora/benchmark/fakes/FakeOfferManager.java 
> 6f2ca35c5d83dde29c24865b4826d4932e96da80 
>   src/main/java/org/apache/aurora/scheduler/HostOffer.java 
> 23f0600d64e1e15f4856f397e839e3d1c87f3b96 
>   src/main/java/org/apache/aurora/scheduler/base/InstanceKeys.java 
> b12ac83168401c15fb1d30179ea8e4816f09cd3d 
>   src/main/java/org/apache/aurora/scheduler/base/JobKeys.java 
> 0136afb8f6049a6d88cd42b5e3f17d61fcd629d5 
>   src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java 
> 0637eb7f85125cf70b588d56fa7dc88130947837 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferManager.java 
> 8c000cb0626bd34f6f30e23fe2b3a045f2b44e35 
>   src/main/java/org/apache/aurora/scheduler/offers/OfferReconciler.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java 
> 202cae96ffc5b49e638b973a273f7983137b5baf 
>   src/test/java/org/apache/aurora/scheduler/http/OffersTest.java 
> 30699596a1c95199df7504f62c5c18cab1be1c6c 
>   src/test/java/org/apache/aurora/scheduler/offers/HostOffers.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferManagerImplTest.java 
> 49d4e82cc03144b80292fe43066a6cc4d7aed88f 
>   src/test/java/org/apache/aurora/scheduler/offers/OfferReconcilerTest.java 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/56691/diff/
> 
> 
> Testing
> ---
> 
> Ran locally on vagrant.
> 
> 
> Thanks,
> 
> Dmitriy Shirchenko
> 
>



Re: Review Request 55105: AURORA-1870 Add finer grained timings to the Snapshot process

2017-02-22 Thread Stephan Erb

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/55105/#review166427
---


Ship it!




LGTM.

- Stephan Erb


On Jan. 28, 2017, 10:15 p.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/55105/
> ---
> 
> (Updated Jan. 28, 2017, 10:15 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Stephan Erb.
> 
> 
> Bugs: AURORA-1870
> https://issues.apache.org/jira/browse/AURORA-1870
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> AURORA-1870 Add finer grained timings to the Snapshot process
> 
> I gave up on `@Timed` interceptor approach because major refactoring is 
> required in order to have snapshot fields instantiated by Guice through 
> `Provider` or `@Provides` interfaces. The abstract class approach is much 
> easier/cleaner.
> 
> 
> Diffs
> -
> 
>   commons/src/main/java/org/apache/aurora/common/stats/SlidingStats.java 
> f7a5ae41e307627fc55157758e9b7cdd861c3268 
>   commons/src/test/java/org/apache/aurora/common/stats/SlidingStatsTest.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImpl.java 
> 7aa111ec14696ae40f518c42f3c7f45d8ab0e94c 
>   
> src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotStoreImplIT.java
>  f56a1624c7188da175ad3e6de323c3442802f2ea 
> 
> Diff: https://reviews.apache.org/r/55105/diff/
> 
> 
> Testing
> ---
> 
> ```
> $ curl localhost:8081/vars | grep snapshot_restore
>   % Total% Received % Xferd  Average Speed   TimeTime Time  
> Current
>  Dload  Upload   Total   SpentLeft  Speed
>   0 00 00 0  0  0 --:--:-- --:--:-- --:--:-- 0
> snapshot_restore_crons_events 1
> snapshot_restore_crons_events_per_sec 0.0
> snapshot_restore_crons_nanos_per_event 0.0
> snapshot_restore_crons_nanos_total 73648
> snapshot_restore_crons_nanos_total_per_sec 0.0
> snapshot_restore_dbscript_events 1
> snapshot_restore_dbscript_events_per_sec 0.0
> snapshot_restore_dbscript_nanos_per_event 0.0
> snapshot_restore_dbscript_nanos_total 1148842021
> snapshot_restore_dbscript_nanos_total_per_sec 0.0
> snapshot_restore_hosts_events 1
> snapshot_restore_hosts_events_per_sec 0.0
> snapshot_restore_hosts_nanos_per_event 0.0
> snapshot_restore_hosts_nanos_total 76166
> snapshot_restore_hosts_nanos_total_per_sec 0.0
> snapshot_restore_job_updates_events 1
> snapshot_restore_job_updates_events_per_sec 0.0
> snapshot_restore_job_updates_nanos_per_event 0.0
> snapshot_restore_job_updates_nanos_total 49482
> snapshot_restore_job_updates_nanos_total_per_sec 0.0
> snapshot_restore_locks_events 1
> snapshot_restore_locks_events_per_sec 0.0
> snapshot_restore_locks_nanos_per_event 0.0
> snapshot_restore_locks_nanos_total 125084
> snapshot_restore_locks_nanos_total_per_sec 0.0
> snapshot_restore_quota_events 1
> snapshot_restore_quota_events_per_sec 0.0
> snapshot_restore_quota_nanos_per_event 0.0
> snapshot_restore_quota_nanos_total 52305
> snapshot_restore_quota_nanos_total_per_sec 0.0
> snapshot_restore_scheduler_metadata_events 1
> snapshot_restore_scheduler_metadata_events_per_sec 0.0
> snapshot_restore_scheduler_metadata_nanos_per_event 0.0
> snapshot_restore_scheduler_metadata_nanos_total 70816
> snapshot_restore_scheduler_metadata_nanos_total_per_sec 0.0
> snapshot_restore_tasks_events 1
> snapshot_restore_tasks_events_per_sec 0.0
> snapshot_restore_tasks_nanos_per_event 0.0
> snapshot_restore_tasks_nanos_total 91377
> snapshot_restore_tasks_nanos_total_per_sec 0.0
> ```
> 
> ```
> $ aurora_admin scheduler_snapshot devcluster
>  INFO] Response from scheduler: OK (message: )
>  
> $ curl localhost:8081/vars | grep snapshot_save
>   % Total% Received % Xferd  Average Speed   TimeTime Time  
> Current
>  Dload  Upload   Total   SpentLeft  Speed
> 100 482260 482260 0  3266k  0 --:--:-- --:--:-- --:--:-- 3363k
> snapshot_save_crons_events 1
> snapshot_save_crons_events_per_sec 0.0
> snapshot_save_crons_nanos_per_event 0.0
> snapshot_save_crons_nanos_total 466181
> snapshot_save_crons_nanos_total_per_sec 0.0
> snapshot_save_dbscript_events 1
> snapshot_save_dbscript_events_per_sec 0.0
> snapshot_save_dbscript_nanos_per_event 0.0
> snapshot_save_dbscript_nanos_total 18201542
> snapshot_save_dbscript_nanos_total_per_sec 0.0
> snapshot_save_hosts_events 1
> snapshot_save_hosts_events_per_sec 0.0
> snapshot_save_hosts_nanos_per_event 0.0
> snapshot_save_hosts_nanos_total 1286180
> snapshot_save_hosts_nanos_total_per_sec 0.0
> snapshot_save_job_updates_events 1
> snapshot_save_job_updates_events_per_sec 0.0
> snapshot_save_job_updates_nanos_per_event 

Re: Review Request 56935: Fix for unnecessary object serializations

2017-02-22 Thread Mehrdad Nurolahzade

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56935/
---

(Updated Feb. 22, 2017, 11:34 a.m.)


Review request for Aurora, David McLaughlin and Zameer Manji.


Changes
---

Updated description


Repository: aurora


Description (updated)
---

Methodology: I attached a profiler (YourKit) to Aurora in one of test clusters 
(where very minimal workload was being applied to the scheduler). Looking at 
object allocation rates, `Webhook` and `ResourceType` stood out. Based on the 
two cases above, I looked itno `requireNonNull()` and `LOG.xxx()` methods in 
the code base to find potentially misbehaving statements.

This patch provides a fix for some unnecessary object serilizations that happen 
on high frequency execution paths and contribute to scheduler's high object 
creation rate. Due to low scheduler workload at the time observation, this list 
is not exhaustive.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
3e8e38abe29766f6fcf08707fba5df402c96a257 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
17301036e28d95ba90b3e4d8840d8a5641e49c46 
  src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
0d639f66db456858278b0485c91c40975c3b45ac 
  src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
c88428412d69e4202e7cceb1b608dc1809a9ccc0 
  src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
a649a6e3d2f2d0aeaf6d7ac704ed24911c310a1e 
  src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
d89e715b1b08faf95f8b5788c9c28cbbb33af093 
  src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
9bb319bb04bb386d9792c3cc0017b039e8f25044 

Diff: https://reviews.apache.org/r/56935/diff/


Testing
---

```./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh```


Thanks,

Mehrdad Nurolahzade



Re: Review Request 56935: Fix for unnecessary object serializations

2017-02-22 Thread Mehrdad Nurolahzade


> On Feb. 22, 2017, 10:27 a.m., Zameer Manji wrote:
> > Could you please add to the RB description on what methodology you used to 
> > determine this and what are the results of this patch?
> > 
> > 
> > Further, do you have any ideas on how we can prevent regressions?

Short-term, we need to invest in profiling Aurora internally. Long-term, 
integrating with an observability/monitoring platform like 
[TwitterServer](https://twitter.github.io/twitter-server/Admin.html#profiling) 
would help with providing better visibility into heap/GC.


- Mehrdad


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56935/#review166387
---


On Feb. 22, 2017, 11:34 a.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56935/
> ---
> 
> (Updated Feb. 22, 2017, 11:34 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Methodology: I attached a profiler (YourKit) to Aurora in one of test 
> clusters (where very minimal workload was being applied to the scheduler). 
> Looking at object allocation rates, `Webhook` and `ResourceType` stood out. 
> Based on the two cases above, I looked itno `requireNonNull()` and 
> `LOG.xxx()` methods in the code base to find potentially misbehaving 
> statements.
> 
> This patch provides a fix for some unnecessary object serilizations that 
> happen on high frequency execution paths and contribute to scheduler's high 
> object creation rate. Due to low scheduler workload at the time observation, 
> this list is not exhaustive.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> 3e8e38abe29766f6fcf08707fba5df402c96a257 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> 17301036e28d95ba90b3e4d8840d8a5641e49c46 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 0d639f66db456858278b0485c91c40975c3b45ac 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
> c88428412d69e4202e7cceb1b608dc1809a9ccc0 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> a649a6e3d2f2d0aeaf6d7ac704ed24911c310a1e 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> d89e715b1b08faf95f8b5788c9c28cbbb33af093 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> 9bb319bb04bb386d9792c3cc0017b039e8f25044 
> 
> Diff: https://reviews.apache.org/r/56935/diff/
> 
> 
> Testing
> ---
> 
> ```./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>



Re: Review Request 56935: Fix for unnecessary object serializations

2017-02-22 Thread Mehrdad Nurolahzade


> On Feb. 22, 2017, 10:07 a.m., Reza Motamedi wrote:
> > Mehrdad, do you also have some stats on how much these changes reduced the 
> > object creation rate?

The object allocation rate dropped from of 25 M/s on average to 15-20 M/s. But, 
as I indicated above these numbers are not representative as the workload on 
test cluster is very low.


- Mehrdad


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56935/#review166385
---


On Feb. 22, 2017, 11:34 a.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56935/
> ---
> 
> (Updated Feb. 22, 2017, 11:34 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Methodology: I attached a profiler (YourKit) to Aurora in one of test 
> clusters (where very minimal workload was being applied to the scheduler). 
> Looking at object allocation rates, `Webhook` and `ResourceType` stood out. 
> Based on the two cases above, I looked itno `requireNonNull()` and 
> `LOG.xxx()` methods in the code base to find potentially misbehaving 
> statements.
> 
> This patch provides a fix for some unnecessary object serilizations that 
> happen on high frequency execution paths and contribute to scheduler's high 
> object creation rate. Due to low scheduler workload at the time observation, 
> this list is not exhaustive.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> 3e8e38abe29766f6fcf08707fba5df402c96a257 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> 17301036e28d95ba90b3e4d8840d8a5641e49c46 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 0d639f66db456858278b0485c91c40975c3b45ac 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
> c88428412d69e4202e7cceb1b608dc1809a9ccc0 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> a649a6e3d2f2d0aeaf6d7ac704ed24911c310a1e 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> d89e715b1b08faf95f8b5788c9c28cbbb33af093 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> 9bb319bb04bb386d9792c3cc0017b039e8f25044 
> 
> Diff: https://reviews.apache.org/r/56935/diff/
> 
> 
> Testing
> ---
> 
> ```./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>



Re: Review Request 56935: Fix for unnecessary object serializations

2017-02-22 Thread Reza Motamedi

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56935/#review166385
---



Mehrdad, do you also have some stats on how much these changes reduced the 
object creation rate?

- Reza Motamedi


On Feb. 22, 2017, 5:23 p.m., Mehrdad Nurolahzade wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56935/
> ---
> 
> (Updated Feb. 22, 2017, 5:23 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This patch provides a fix for some unnecessary object serilizations that 
> happen on high frequency execution paths and contribute to scheduler's high 
> object creation rate.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/events/Webhook.java 
> 3e8e38abe29766f6fcf08707fba5df402c96a257 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImpl.java 
> 17301036e28d95ba90b3e4d8840d8a5641e49c46 
>   src/main/java/org/apache/aurora/scheduler/mesos/MesosTaskFactory.java 
> 0d639f66db456858278b0485c91c40975c3b45ac 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceType.java 
> c88428412d69e4202e7cceb1b608dc1809a9ccc0 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> a649a6e3d2f2d0aeaf6d7ac704ed24911c310a1e 
>   src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java 
> d89e715b1b08faf95f8b5788c9c28cbbb33af093 
>   src/test/java/org/apache/aurora/scheduler/mesos/MesosSchedulerImplTest.java 
> 9bb319bb04bb386d9792c3cc0017b039e8f25044 
> 
> Diff: https://reviews.apache.org/r/56935/diff/
> 
> 
> Testing
> ---
> 
> ```./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh```
> 
> 
> Thanks,
> 
> Mehrdad Nurolahzade
> 
>



Re: Review Request 56395: Change Resource Validation in ConfigurationManager so that it validates the Resource Set instead of deprecated fields

2017-02-22 Thread Stephan Erb

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/56395/#review166376
---




src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 (lines 344 - 345)


I belive this will not work as expected if a resource type is missing. 
`getResource` throws `IllegalArgumentException` but users of the this class 
expect we throw `TaskDescriptionException`.

My recommendation would be:
* leave `getResource` in the `ThriftBackFill` class. It is scrape code that 
we will remove soon anyway.
* Leverage the already existing `ResourceManager` methods here instead: 
`quantityOf(getTaskResources(config, CPUS))`


- Stephan Erb


On Feb. 7, 2017, 6:02 p.m., Nicolás Donatucci wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/56395/
> ---
> 
> (Updated Feb. 7, 2017, 6:02 p.m.)
> 
> 
> Review request for Aurora, Stephan Erb and Zameer Manji.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The Resource validation in ConfigurationManager is now done against the 
> Resource set instead of the NumCpus, RamMb and DiskMb fields.
> 
> To do this I moved the GetResource method from the ThriftBackfill to the 
> ResourceManager so it can be used in the ConfigurationManager without 
> exposing it on the ThriftBackfill.
> 
> Related Issue: Aurora-1707
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  80f0aebd6de0c2d7c30a58ec09702d1c6d519787 
>   src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java 
> 9aa263a9cfae03a9a0c5bc7fe3a1405397d3009c 
>   src/main/java/org/apache/aurora/scheduler/storage/log/ThriftBackfill.java 
> c8838438ef5aa94fbcc407101e7f04abc75cd96e 
> 
> Diff: https://reviews.apache.org/r/56395/diff/
> 
> 
> Testing
> ---
> 
> src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Nicolás Donatucci
> 
>