Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

2014-09-24 Thread Bill Farner

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

(Updated Sept. 25, 2014, 5:03 a.m.)


Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


Changes
---

Rebase on master.


Bugs: AURORA-718
https://issues.apache.org/jira/browse/AURORA-718


Repository: aurora


Description
---

When creating an update, store only the delta between the initial and desired 
states.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/base/Numbers.java 
5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
  src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
  src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java 
b7143941feb4824aee609741e8e2c0e015eecac3 
  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
15a9e2b9acd65f6268e70fb402414f5c01c42409 
  
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
 26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
4602dbb69a675097430de1345bf6bfea0d15ceca 
  src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 
3bf81e6d05d6dc8240737fad0ad48e194ba3166c 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
7efcaa4a80b25db69f4bbaa79435e58997ea393e 
  src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 
1d0bbfef583708bfdab30802724273e4b21ecf8f 
  src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
842256a16fe2775af865ed2ee91fd719b3476577 
  src/main/python/apache/aurora/client/api/__init__.py 
bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 e1179b449c05adc56206bfef271a9d440a77e042 
  src/main/thrift/org/apache/aurora/gen/api.thrift 
7436a5ac61013f576649665d5f585fa5e1bb2a56 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 10fa31db10a80cbc05a4c79728d687766135b5e2 
  src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
f7c7205d80edac714a12c2aff0cc7db27b1909b4 
  src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
ad4040c4a58d3e6f075a60712144aa6968270a55 
  src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 
548d31580b5f2bae92900fddb80005f7d274a155 

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


Testing
---

./gradlew build -Pq


Thanks,

Bill Farner



Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

2014-09-24 Thread Bill Farner


> On Sept. 25, 2014, 12:04 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java, line 49
> > 
> >
> > Should this be a RangeSet instead?

I chose against this simply because it was non-trivial to translate 
Set -> RangeSet.  I believe it involves looping over the 
integers, and using something like:

rangeBuilder.add(Range.closedOpen(i, 
i+1).canonicalize(DiscreteDomain.integers()));

but i found that pretty unreadable.  Since i already have a Set to begin with 
and the object is quickly discarded, i didn't see much value in converting it 
to a RangeSet.


- Bill


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


On Sept. 24, 2014, 6:02 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25969/
> ---
> 
> (Updated Sept. 24, 2014, 6:02 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-718
> https://issues.apache.org/jira/browse/AURORA-718
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> When creating an update, store only the delta between the initial and desired 
> states.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 
> 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
>   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
> 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
>   src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java 
> b7143941feb4824aee609741e8e2c0e015eecac3 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 15a9e2b9acd65f6268e70fb402414f5c01c42409 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  ae0320b44b55a3630e255484ea7a881daab6a7bc 
>   src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 
> 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  348bdbfbb51800d31b67ea1b3f2632975e598306 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 
> 1d0bbfef583708bfdab30802724273e4b21ecf8f 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 842256a16fe2775af865ed2ee91fd719b3476577 
>   src/main/python/apache/aurora/client/api/__init__.py 
> bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  e1179b449c05adc56206bfef271a9d440a77e042 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> f7c7205d80edac714a12c2aff0cc7db27b1909b4 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> ad4040c4a58d3e6f075a60712144aa6968270a55 
>   
> src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 
> 548d31580b5f2bae92900fddb80005f7d274a155 
> 
> Diff: https://reviews.apache.org/r/25969/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 26015: Skip past irrelevant jobs when running cron show command.

2014-09-24 Thread Maxim Khutornenko

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

Ship it!


Ship It!

- Maxim Khutornenko


On Sept. 25, 2014, 2:03 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26015/
> ---
> 
> (Updated Sept. 25, 2014, 2:03 a.m.)
> 
> 
> Review request for Aurora, Joe Smith and Mark Chu-Carroll.
> 
> 
> Bugs: AURORA-753
> https://issues.apache.org/jira/browse/AURORA-753
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Skip past irrelevant jobs when running cron show command.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/cron.py 
> 3416c8e1932056725880f2007b60d77112759428 
>   src/test/python/apache/aurora/client/cli/test_cron.py 
> f488432cd68cc68fab8fce968e8605625ea3f56a 
> 
> Diff: https://reviews.apache.org/r/26015/diff/
> 
> 
> Testing
> ---
> 
> Reproduced issue in a test case, now passes:
> ./pants src/test/python/apache/aurora/client/cli:cron
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 26012: Use BufferedOutputStream for deflater

2014-09-24 Thread Bill Farner


> On Sept. 25, 2014, 2:23 a.m., Bill Farner wrote:
> > Ship It!

Actually - can you include a comment with a very brief explanation of the 
findings?  Wouldn't want somebody to remove the seemingly-unnecessary stream 
wrapper.


- Bill


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


On Sept. 25, 2014, 1:36 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26012/
> ---
> 
> (Updated Sept. 25, 2014, 1:36 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joe Smith, and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Use BufferedOutputStream for deflater
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> 45cf7ecf52582ba3fd7bbc8b4a981e396793da4b 
> 
> Diff: https://reviews.apache.org/r/26012/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build
> 
> Also put together a microbenchmark with a scheduler backup from my production 
> environment - using a BufferedOutputStream gets a 4sec speedup with no 
> reduction in compression ratio. ¯_(?)_/¯
> 
> # Deflater.BEST_SPEED, No buffer
> deflate: 11745ms
> total:12734ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.BEST_SPEED, buf = 512KiB
> deflate: 11015ms
> total:12001ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.BEST_SPEED, BufferedOutputStream
> deflate: 6885ms
> total:7694ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.BEST_SPEED, BufferedOutputStream at 512KiB
> deflate: 6752ms
> total:7585ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.DEFAULT_COMPRESSION
> deflate: 16664ms
> total:17219ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> # Deflater.DEFAULT_COMPRESSION, buf size = 512KiB
> deflate: 16516ms
> total:17103ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> # Deflater.DEFAULT_COMPRESSION, BufferedOutputStream
> deflate: 12241ms
> total:12788ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> # Deflater.DEFAULT_COMPRESSION, BufferedOutputStream at 512KiB
> deflate: 11548ms
> total:12108ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 26012: Use BufferedOutputStream for deflater

2014-09-24 Thread Bill Farner

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

Ship it!


Ship It!

- Bill Farner


On Sept. 25, 2014, 1:36 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26012/
> ---
> 
> (Updated Sept. 25, 2014, 1:36 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joe Smith, and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Use BufferedOutputStream for deflater
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> 45cf7ecf52582ba3fd7bbc8b4a981e396793da4b 
> 
> Diff: https://reviews.apache.org/r/26012/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build
> 
> Also put together a microbenchmark with a scheduler backup from my production 
> environment - using a BufferedOutputStream gets a 4sec speedup with no 
> reduction in compression ratio. ¯_(?)_/¯
> 
> # Deflater.BEST_SPEED, No buffer
> deflate: 11745ms
> total:12734ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.BEST_SPEED, buf = 512KiB
> deflate: 11015ms
> total:12001ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.BEST_SPEED, BufferedOutputStream
> deflate: 6885ms
> total:7694ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.BEST_SPEED, BufferedOutputStream at 512KiB
> deflate: 6752ms
> total:7585ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.DEFAULT_COMPRESSION
> deflate: 16664ms
> total:17219ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> # Deflater.DEFAULT_COMPRESSION, buf size = 512KiB
> deflate: 16516ms
> total:17103ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> # Deflater.DEFAULT_COMPRESSION, BufferedOutputStream
> deflate: 12241ms
> total:12788ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> # Deflater.DEFAULT_COMPRESSION, BufferedOutputStream at 512KiB
> deflate: 11548ms
> total:12108ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 26014: Quick fix for cron error.

2014-09-24 Thread Bill Farner

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


Fix + test is up here: https://reviews.apache.org/r/26015/

- Bill Farner


On Sept. 25, 2014, 1:57 a.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26014/
> ---
> 
> (Updated Sept. 25, 2014, 1:57 a.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: aurora-753
> https://issues.apache.org/jira/browse/aurora-753
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Damned python indent-syntax - else clause attached to "if" rather than "for".
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/cron.py 
> 3416c8e1932056725880f2007b60d77112759428 
> 
> Diff: https://reviews.apache.org/r/26014/diff/
> 
> 
> Testing
> ---
> 
> Quick fix: no additional test. I'll add a regression test when rosh hashanah 
> is over.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>



Re: Review Request 26015: Skip past irrelevant jobs when running cron show command.

2014-09-24 Thread Joe Smith

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

Ship it!


Thanks for the test case!


src/main/python/apache/aurora/client/cli/cron.py


Mainly confirming for myself- but this follows the expected behavior- if a 
job is the right name, but not a cron: INVALID_PARAMETER. If there are no jobs 
matching the name: INVALID_PARAMETER.


- Joe Smith


On Sept. 24, 2014, 7:03 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26015/
> ---
> 
> (Updated Sept. 24, 2014, 7:03 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Mark Chu-Carroll.
> 
> 
> Bugs: AURORA-753
> https://issues.apache.org/jira/browse/AURORA-753
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Skip past irrelevant jobs when running cron show command.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/cron.py 
> 3416c8e1932056725880f2007b60d77112759428 
>   src/test/python/apache/aurora/client/cli/test_cron.py 
> f488432cd68cc68fab8fce968e8605625ea3f56a 
> 
> Diff: https://reviews.apache.org/r/26015/diff/
> 
> 
> Testing
> ---
> 
> Reproduced issue in a test case, now passes:
> ./pants src/test/python/apache/aurora/client/cli:cron
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Review Request 26015: Skip past irrelevant jobs when running cron show command.

2014-09-24 Thread Bill Farner

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

Review request for Aurora, Joe Smith and Mark Chu-Carroll.


Bugs: AURORA-753
https://issues.apache.org/jira/browse/AURORA-753


Repository: aurora


Description
---

Skip past irrelevant jobs when running cron show command.


Diffs
-

  src/main/python/apache/aurora/client/cli/cron.py 
3416c8e1932056725880f2007b60d77112759428 
  src/test/python/apache/aurora/client/cli/test_cron.py 
f488432cd68cc68fab8fce968e8605625ea3f56a 

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


Testing
---

Reproduced issue in a test case, now passes:
./pants src/test/python/apache/aurora/client/cli:cron


Thanks,

Bill Farner



Re: Review Request 26012: Use BufferedOutputStream for deflater

2014-09-24 Thread David McLaughlin

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

Ship it!


Ship It!

- David McLaughlin


On Sept. 25, 2014, 1:36 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26012/
> ---
> 
> (Updated Sept. 25, 2014, 1:36 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joe Smith, and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Use BufferedOutputStream for deflater
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> 45cf7ecf52582ba3fd7bbc8b4a981e396793da4b 
> 
> Diff: https://reviews.apache.org/r/26012/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build
> 
> Also put together a microbenchmark with a scheduler backup from my production 
> environment - using a BufferedOutputStream gets a 4sec speedup with no 
> reduction in compression ratio. ¯_(?)_/¯
> 
> # Deflater.BEST_SPEED, No buffer
> deflate: 11745ms
> total:12734ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.BEST_SPEED, buf = 512KiB
> deflate: 11015ms
> total:12001ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.BEST_SPEED, BufferedOutputStream
> deflate: 6885ms
> total:7694ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.BEST_SPEED, BufferedOutputStream at 512KiB
> deflate: 6752ms
> total:7585ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.DEFAULT_COMPRESSION
> deflate: 16664ms
> total:17219ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> # Deflater.DEFAULT_COMPRESSION, buf size = 512KiB
> deflate: 16516ms
> total:17103ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> # Deflater.DEFAULT_COMPRESSION, BufferedOutputStream
> deflate: 12241ms
> total:12788ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> # Deflater.DEFAULT_COMPRESSION, BufferedOutputStream at 512KiB
> deflate: 11548ms
> total:12108ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Review Request 26014: Quick fix for cron error.

2014-09-24 Thread Mark Chu-Carroll

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

Review request for Aurora, Bill Farner and Zameer Manji.


Bugs: aurora-753
https://issues.apache.org/jira/browse/aurora-753


Repository: aurora


Description
---

Damned python indent-syntax - else clause attached to "if" rather than "for".


Diffs
-

  src/main/python/apache/aurora/client/cli/cron.py 
3416c8e1932056725880f2007b60d77112759428 

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


Testing
---

Quick fix: no additional test. I'll add a regression test when rosh hashanah is 
over.


Thanks,

Mark Chu-Carroll



Re: Review Request 26012: Use BufferedOutputStream for deflater

2014-09-24 Thread Maxim Khutornenko

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

Ship it!


Ship It!

- Maxim Khutornenko


On Sept. 25, 2014, 1:36 a.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26012/
> ---
> 
> (Updated Sept. 25, 2014, 1:36 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Joe Smith, and Bill Farner.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Use BufferedOutputStream for deflater
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
> 45cf7ecf52582ba3fd7bbc8b4a981e396793da4b 
> 
> Diff: https://reviews.apache.org/r/26012/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build
> 
> Also put together a microbenchmark with a scheduler backup from my production 
> environment - using a BufferedOutputStream gets a 4sec speedup with no 
> reduction in compression ratio. ¯_(?)_/¯
> 
> # Deflater.BEST_SPEED, No buffer
> deflate: 11745ms
> total:12734ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.BEST_SPEED, buf = 512KiB
> deflate: 11015ms
> total:12001ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.BEST_SPEED, BufferedOutputStream
> deflate: 6885ms
> total:7694ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.BEST_SPEED, BufferedOutputStream at 512KiB
> deflate: 6752ms
> total:7585ms
> compression ratio: 647270930/121616834 = 532.22%
> 
> # Deflater.DEFAULT_COMPRESSION
> deflate: 16664ms
> total:17219ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> # Deflater.DEFAULT_COMPRESSION, buf size = 512KiB
> deflate: 16516ms
> total:17103ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> # Deflater.DEFAULT_COMPRESSION, BufferedOutputStream
> deflate: 12241ms
> total:12788ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> # Deflater.DEFAULT_COMPRESSION, BufferedOutputStream at 512KiB
> deflate: 11548ms
> total:12108ms
> compression ratio: 647270930/92690409 = 698.31%
> 
> 
> Thanks,
> 
> Kevin Sweeney
> 
>



Re: Review Request 26012: Use BufferedOutputStream for deflater

2014-09-24 Thread Kevin Sweeney

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

(Updated Sept. 24, 2014, 6:36 p.m.)


Review request for Aurora, David McLaughlin, Joe Smith, and Bill Farner.


Changes
---

+jsmith


Repository: aurora


Description
---

Use BufferedOutputStream for deflater


Diffs
-

  src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
45cf7ecf52582ba3fd7bbc8b4a981e396793da4b 

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


Testing
---

./gradlew build

Also put together a microbenchmark with a scheduler backup from my production 
environment - using a BufferedOutputStream gets a 4sec speedup with no 
reduction in compression ratio. ¯_(?)_/¯

# Deflater.BEST_SPEED, No buffer
deflate: 11745ms
total:12734ms
compression ratio: 647270930/121616834 = 532.22%

# Deflater.BEST_SPEED, buf = 512KiB
deflate: 11015ms
total:12001ms
compression ratio: 647270930/121616834 = 532.22%

# Deflater.BEST_SPEED, BufferedOutputStream
deflate: 6885ms
total:7694ms
compression ratio: 647270930/121616834 = 532.22%

# Deflater.BEST_SPEED, BufferedOutputStream at 512KiB
deflate: 6752ms
total:7585ms
compression ratio: 647270930/121616834 = 532.22%

# Deflater.DEFAULT_COMPRESSION
deflate: 16664ms
total:17219ms
compression ratio: 647270930/92690409 = 698.31%

# Deflater.DEFAULT_COMPRESSION, buf size = 512KiB
deflate: 16516ms
total:17103ms
compression ratio: 647270930/92690409 = 698.31%

# Deflater.DEFAULT_COMPRESSION, BufferedOutputStream
deflate: 12241ms
total:12788ms
compression ratio: 647270930/92690409 = 698.31%

# Deflater.DEFAULT_COMPRESSION, BufferedOutputStream at 512KiB
deflate: 11548ms
total:12108ms
compression ratio: 647270930/92690409 = 698.31%


Thanks,

Kevin Sweeney



Review Request 26012: Use BufferedOutputStream for deflater

2014-09-24 Thread Kevin Sweeney

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

Review request for Aurora, David McLaughlin and Bill Farner.


Repository: aurora


Description
---

Use BufferedOutputStream for deflater


Diffs
-

  src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
45cf7ecf52582ba3fd7bbc8b4a981e396793da4b 

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


Testing
---

./gradlew build

Also put together a microbenchmark with a scheduler backup from my production 
environment - using a BufferedOutputStream gets a 4sec speedup with no 
reduction in compression ratio. ¯_(?)_/¯

# Deflater.BEST_SPEED, No buffer
deflate: 11745ms
total:12734ms
compression ratio: 647270930/121616834 = 532.22%

# Deflater.BEST_SPEED, buf = 512KiB
deflate: 11015ms
total:12001ms
compression ratio: 647270930/121616834 = 532.22%

# Deflater.BEST_SPEED, BufferedOutputStream
deflate: 6885ms
total:7694ms
compression ratio: 647270930/121616834 = 532.22%

# Deflater.BEST_SPEED, BufferedOutputStream at 512KiB
deflate: 6752ms
total:7585ms
compression ratio: 647270930/121616834 = 532.22%

# Deflater.DEFAULT_COMPRESSION
deflate: 16664ms
total:17219ms
compression ratio: 647270930/92690409 = 698.31%

# Deflater.DEFAULT_COMPRESSION, buf size = 512KiB
deflate: 16516ms
total:17103ms
compression ratio: 647270930/92690409 = 698.31%

# Deflater.DEFAULT_COMPRESSION, BufferedOutputStream
deflate: 12241ms
total:12788ms
compression ratio: 647270930/92690409 = 698.31%

# Deflater.DEFAULT_COMPRESSION, BufferedOutputStream at 512KiB
deflate: 11548ms
total:12108ms
compression ratio: 647270930/92690409 = 698.31%


Thanks,

Kevin Sweeney



Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

2014-09-24 Thread Kevin Sweeney

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

Ship it!



src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java


Should this be a RangeSet instead?


- Kevin Sweeney


On Sept. 24, 2014, 11:02 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25969/
> ---
> 
> (Updated Sept. 24, 2014, 11:02 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-718
> https://issues.apache.org/jira/browse/AURORA-718
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> When creating an update, store only the delta between the initial and desired 
> states.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 
> 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
>   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
> 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
>   src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java 
> b7143941feb4824aee609741e8e2c0e015eecac3 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 15a9e2b9acd65f6268e70fb402414f5c01c42409 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  ae0320b44b55a3630e255484ea7a881daab6a7bc 
>   src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 
> 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  348bdbfbb51800d31b67ea1b3f2632975e598306 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 
> 1d0bbfef583708bfdab30802724273e4b21ecf8f 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 842256a16fe2775af865ed2ee91fd719b3476577 
>   src/main/python/apache/aurora/client/api/__init__.py 
> bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  e1179b449c05adc56206bfef271a9d440a77e042 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> f7c7205d80edac714a12c2aff0cc7db27b1909b4 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> ad4040c4a58d3e6f075a60712144aa6968270a55 
>   
> src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 
> 548d31580b5f2bae92900fddb80005f7d274a155 
> 
> Diff: https://reviews.apache.org/r/25969/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 25963: Expose in progress and recently completed updates in UI

2014-09-24 Thread Joshua Cohen

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

Ship it!


Ship It!

- Joshua Cohen


On Sept. 24, 2014, 10:52 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25963/
> ---
> 
> (Updated Sept. 24, 2014, 10:52 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-745
> https://issues.apache.org/jira/browse/AURORA-745
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Very simple first iteration. Shows only the latest 20 updates. 
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 392b4f7bc9d5352ed536ff83eafb4175e4860aa1 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 
> befd5908e51dcac2a0fc56b12651af11273d25c0 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 
> 6434d1bc3b9f769f6f565d601fd98f99743efe8c 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js 
> 082d920348f12b214b8ca2d20c11e7b4825453ed 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> ded175a1e4b288b8978235c986db2549c22dcc7d 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 
> 4e67c4e0458cd67df7c1e523ee896b7d2e234a52 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/latestUpdates.html 
> PRE-CREATION 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25963/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew jsHint
> 
> 
> File Attachments
> 
> 
> updates page
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png
> no data updates
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>



Re: Review Request 25963: Expose in progress and recently completed updates in UI

2014-09-24 Thread David McLaughlin


> On Sept. 24, 2014, 10:23 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/latestUpdates.html, 
> > line 15
> > 
> >
> > Nit: should we parameterize this to maintain the previous distinction 
> > between in progress vs completed?

Fixed.


- David


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


On Sept. 24, 2014, 9:46 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25963/
> ---
> 
> (Updated Sept. 24, 2014, 9:46 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-745
> https://issues.apache.org/jira/browse/AURORA-745
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Very simple first iteration. Shows only the latest 20 updates. 
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 392b4f7bc9d5352ed536ff83eafb4175e4860aa1 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 
> befd5908e51dcac2a0fc56b12651af11273d25c0 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 
> 6434d1bc3b9f769f6f565d601fd98f99743efe8c 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js 
> 082d920348f12b214b8ca2d20c11e7b4825453ed 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> ded175a1e4b288b8978235c986db2549c22dcc7d 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 
> 4e67c4e0458cd67df7c1e523ee896b7d2e234a52 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/latestUpdates.html 
> PRE-CREATION 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25963/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew jsHint
> 
> 
> File Attachments
> 
> 
> updates page
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png
> no data updates
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>



Re: Review Request 25963: Expose in progress and recently completed updates in UI

2014-09-24 Thread David McLaughlin

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

(Updated Sept. 24, 2014, 10:52 p.m.)


Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.


Bugs: AURORA-745
https://issues.apache.org/jira/browse/AURORA-745


Repository: aurora


Description
---

Very simple first iteration. Shows only the latest 20 updates. 


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
392b4f7bc9d5352ed536ff83eafb4175e4860aa1 
  src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 
befd5908e51dcac2a0fc56b12651af11273d25c0 
  src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 
6434d1bc3b9f769f6f565d601fd98f99743efe8c 
  src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js 
082d920348f12b214b8ca2d20c11e7b4825453ed 
  src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
ded175a1e4b288b8978235c986db2549c22dcc7d 
  src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 
4e67c4e0458cd67df7c1e523ee896b7d2e234a52 
  src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8 
  src/main/resources/org/apache/aurora/scheduler/http/ui/latestUpdates.html 
PRE-CREATION 
  src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html 
PRE-CREATION 

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


Testing
---

./gradlew jsHint


File Attachments


updates page
  
https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png
no data updates
  
https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png


Thanks,

David McLaughlin



Re: Review Request 25972: Make thermos more cognizant of user deletions

2014-09-24 Thread Kevin Sweeney

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



src/main/python/apache/thermos/core/helper.py


nullable as well?



src/main/python/apache/thermos/core/helper.py


Logic is inverted - this log message belongs below



src/test/python/apache/thermos/core/test_helper.py


why does this need to be a contextmanager?


- Kevin Sweeney


On Sept. 23, 2014, 5:58 p.m., Brian Wickman wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25972/
> ---
> 
> (Updated Sept. 23, 2014, 5:58 p.m.)
> 
> 
> Review request for Aurora, Joe Smith and Kevin Sweeney.
> 
> 
> Bugs: AURORA-175
> https://issues.apache.org/jira/browse/AURORA-175
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is changes 1 of 2 for AURORA-175.  The second change adds an exit status 
> contract between Aurora and Thermos.
> 
> This change allows a process in the Thermos state machine to go directly from 
> WAITING -> FAILED if the user associated with that process has been deleted.  
> It also persists a 'uid' field to the Thermos RunnerHeader so that we can 
> check against process UIDs which in theory have higher fidelity than 
> usernames.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/thermos/common/ckpt.py 
> 7df179b135e896d52655667f0707850a4bc33068 
>   src/main/python/apache/thermos/core/helper.py 
> 82c68f3af424d3309db3816080465edd1ff1a87c 
>   src/main/python/apache/thermos/core/process.py 
> 4889e636c2fa7325852684cecd87a2123714144d 
>   src/main/python/apache/thermos/core/runner.py 
> 31f40713ec32c0626566899caec76f2a9985c7bf 
>   src/main/thrift/org/apache/thermos/thermos_internal.thrift 
> 0cea1057230b367c9d55b773854453cee2f9fce0 
>   src/test/python/apache/thermos/core/test_helper.py 
> 53f1e467283b45598b3f1865951c42b13579a512 
>   src/test/python/apache/thermos/core/test_process.py 
> 223393d62d85dbcd3da3c2ca0d5558e8209aa1d1 
> 
> Diff: https://reviews.apache.org/r/25972/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Brian Wickman
> 
>



Re: Review Request 26004: Add "aurora update list" and "aurora update status" commands.

2014-09-24 Thread David McLaughlin

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


What was the rationale for hiding update IDs from the user and making job key 
the parameter for update status? It's nice you can quickly see if a job key has 
an update in progress.. but what happens if you're wanting to see a recent 
deploy that is already finished or see further back?  

I think update list should return the update ids and update status should 
accept an update id. Well maybe not update status.. but update show or 
something? Something which is consistent with the job verbs would be great.

- David McLaughlin


On Sept. 24, 2014, 8:24 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26004/
> ---
> 
> (Updated Sept. 24, 2014, 8:24 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-742
> https://issues.apache.org/jira/browse/aurora-742
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add support for commands to query and display active updates being
> managed by the scheduler. Two commands are added:
> 
> * "aurora update list", which shows all active updates that are being 
> processed by the server.
> * "aurora update status", which shows detailed status information about an 
> update in-progress on the server.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/context.py 
> 102d20797816788361dfdac450aac9fb8e6fbc28 
>   src/main/python/apache/aurora/client/cli/options.py 
> c2d422ac2bc82fc387596e93040b49f722f8310f 
>   src/main/python/apache/aurora/client/cli/update.py 
> c6cb98fe6aa42310090167796c971856d3dc177f 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 4fb1623e497b9741ae2a350deb20030dd4036506 
>   src/test/python/apache/aurora/client/cli/util.py 
> a50b83c571390374975accf75e31f392dbdaaa04 
> 
> Diff: https://reviews.apache.org/r/26004/diff/
> 
> 
> Testing
> ---
> 
> Added new unit tests; all tests pass.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>



Re: Review Request 25963: Expose in progress and recently completed updates in UI

2014-09-24 Thread Joshua Cohen

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

Ship it!


Thanks for the cleanup, looks good!


src/main/resources/org/apache/aurora/scheduler/http/ui/latestUpdates.html


Nit: should we parameterize this to maintain the previous distinction 
between in progress vs completed?


- Joshua Cohen


On Sept. 24, 2014, 9:46 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25963/
> ---
> 
> (Updated Sept. 24, 2014, 9:46 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-745
> https://issues.apache.org/jira/browse/AURORA-745
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Very simple first iteration. Shows only the latest 20 updates. 
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 392b4f7bc9d5352ed536ff83eafb4175e4860aa1 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 
> befd5908e51dcac2a0fc56b12651af11273d25c0 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 
> 6434d1bc3b9f769f6f565d601fd98f99743efe8c 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js 
> 082d920348f12b214b8ca2d20c11e7b4825453ed 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> ded175a1e4b288b8978235c986db2549c22dcc7d 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 
> 4e67c4e0458cd67df7c1e523ee896b7d2e234a52 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/latestUpdates.html 
> PRE-CREATION 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25963/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew jsHint
> 
> 
> File Attachments
> 
> 
> updates page
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png
> no data updates
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>



Re: Review Request 25963: Expose in progress and recently completed updates in UI

2014-09-24 Thread David McLaughlin

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

(Updated Sept. 24, 2014, 9:46 p.m.)


Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.


Changes
---

feedback.


Bugs: AURORA-745
https://issues.apache.org/jira/browse/AURORA-745


Repository: aurora


Description
---

Very simple first iteration. Shows only the latest 20 updates. 


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
392b4f7bc9d5352ed536ff83eafb4175e4860aa1 
  src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 
befd5908e51dcac2a0fc56b12651af11273d25c0 
  src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 
6434d1bc3b9f769f6f565d601fd98f99743efe8c 
  src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js 
082d920348f12b214b8ca2d20c11e7b4825453ed 
  src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
ded175a1e4b288b8978235c986db2549c22dcc7d 
  src/main/resources/org/apache/aurora/scheduler/http/ui/js/directives.js 
4e67c4e0458cd67df7c1e523ee896b7d2e234a52 
  src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8 
  src/main/resources/org/apache/aurora/scheduler/http/ui/latestUpdates.html 
PRE-CREATION 
  src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html 
PRE-CREATION 

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


Testing
---

./gradlew jsHint


File Attachments


updates page
  
https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png
no data updates
  
https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png


Thanks,

David McLaughlin



Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-24 Thread Maxim Khutornenko

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

(Updated Sept. 24, 2014, 9:26 p.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


Changes
---

-davmclau


Bugs: AURORA-686
https://issues.apache.org/jira/browse/AURORA-686


Repository: aurora


Description
---

Getting it right required a deep refactoring of the QuotaManager. The prod 
consumption is now calculated from:
- production tasks not participating in job updates;
- unaffected production tasks from a job being updated (i.e. those that are not 
covered by the update working set);
- production tasks directly affected by the job update (max of old and new 
resources used).

Also, moved all logic back to SchedulerThriftInterface as quota checks are done 
only there now.


Diffs
-

  
src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
 3661f8487985f631e3ea437fe6430e0296376a9e 
  src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
14b0dd8f86026840d0444c128f656a144eab017d 
  src/main/java/org/apache/aurora/scheduler/quota/QuotaUtil.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/state/StateModule.java 
54b90127551c69509dbd41ee95c384dbbf1a7ee4 
  src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java 
70333737ee119ea083f0ac17a4205e7af9f4c753 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
ae0320b44b55a3630e255484ea7a881daab6a7bc 
  src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 
8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
348bdbfbb51800d31b67ea1b3f2632975e598306 
  src/main/thrift/org/apache/aurora/gen/api.thrift 
2376a5e530b12fbbebb4cfc7555656ae07795518 
  src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
8381e2126c62c40d28a325c72686d01e82bb84cf 
  src/test/java/org/apache/aurora/scheduler/state/TaskLimitValidatorTest.java 
8f18617b2052201f87bb1464314c2ee45b279276 
  
src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java 
5aebbfbc691dfac4a066cb1425d18d3fccc77090 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
  src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
40156c211a346664c0d2f174235efb2049cf3bb9 

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


Testing
---

gradle -Pq build


Thanks,

Maxim Khutornenko



Re: Review Request 26004: Add "aurora update list" and "aurora update status" commands.

2014-09-24 Thread Zameer Manji

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

Ship it!


Ship It!

- Zameer Manji


On Sept. 24, 2014, 1:24 p.m., Mark Chu-Carroll wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26004/
> ---
> 
> (Updated Sept. 24, 2014, 1:24 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Zameer Manji.
> 
> 
> Bugs: aurora-742
> https://issues.apache.org/jira/browse/aurora-742
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Add support for commands to query and display active updates being
> managed by the scheduler. Two commands are added:
> 
> * "aurora update list", which shows all active updates that are being 
> processed by the server.
> * "aurora update status", which shows detailed status information about an 
> update in-progress on the server.
> 
> 
> Diffs
> -
> 
>   src/main/python/apache/aurora/client/cli/context.py 
> 102d20797816788361dfdac450aac9fb8e6fbc28 
>   src/main/python/apache/aurora/client/cli/options.py 
> c2d422ac2bc82fc387596e93040b49f722f8310f 
>   src/main/python/apache/aurora/client/cli/update.py 
> c6cb98fe6aa42310090167796c971856d3dc177f 
>   src/test/python/apache/aurora/client/cli/test_supdate.py 
> 4fb1623e497b9741ae2a350deb20030dd4036506 
>   src/test/python/apache/aurora/client/cli/util.py 
> a50b83c571390374975accf75e31f392dbdaaa04 
> 
> Diff: https://reviews.apache.org/r/26004/diff/
> 
> 
> Testing
> ---
> 
> Added new unit tests; all tests pass.
> 
> 
> Thanks,
> 
> Mark Chu-Carroll
> 
>



Re: Review Request 25963: Expose in progress and recently completed updates in UI

2014-09-24 Thread David McLaughlin


> On Sept. 24, 2014, 4:43 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js, 
> > lines 296-305
> > 
> >
> > Can we just have a single function that takes the statuses as a 
> > parameter? If you want to maintain the simplicity of the current API we 
> > could bind a more generic function to a specific one. I.e.:
> > 
> > function getJobUpdateQuery(status) {
> >   var query = new JobUpdateQuery();
> >   query.updateStatuses = status;
> >   return query;
> > };
> > 
> > ...
> > 
> > getTerminalQuery: getJobUpdateQuery.bind(this, UPDATE_TERMINAL_STATUSES 
> > );

The API is so that services.js was solely responsible for what constitutes a 
TERMINAL_STATE, rather than leaking that code into the controllers. There are 
multiple ways to do that, of course. For now I will make the refactor you 
proposed.


> On Sept. 24, 2014, 4:43 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js, 
> > line 39
> > 
> >
> > restore the `_MS` suffix to make it explicit what the unit is here?

Fixed.


> On Sept. 24, 2014, 4:43 p.m., Joshua Cohen wrote:
> > src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html, 
> > lines 33-51
> > 
> >
> > This table and the one below for completed updates are identical (minus 
> > the data source). Does angular have partials or something similar so we can 
> > avoid replicating this?

Sure, I can make this a directive. I figured we might eventually have 
pagination/filtering/etc. in the second table but not the first, so that was 
the reasoning.. but we can cross that bridge when we come to it.


- David


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


On Sept. 23, 2014, 11:46 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25963/
> ---
> 
> (Updated Sept. 23, 2014, 11:46 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-745
> https://issues.apache.org/jira/browse/AURORA-745
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Very simple first iteration. Shows only the latest 20 updates. 
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 392b4f7bc9d5352ed536ff83eafb4175e4860aa1 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 
> befd5908e51dcac2a0fc56b12651af11273d25c0 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 
> 6434d1bc3b9f769f6f565d601fd98f99743efe8c 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js 
> 082d920348f12b214b8ca2d20c11e7b4825453ed 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> ded175a1e4b288b8978235c986db2549c22dcc7d 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25963/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew jsHint
> 
> 
> File Attachments
> 
> 
> updates page
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png
> no data updates
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>



Review Request 26004: Add "aurora update list" and "aurora update status" commands.

2014-09-24 Thread Mark Chu-Carroll

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

Review request for Aurora, David McLaughlin and Zameer Manji.


Bugs: aurora-742
https://issues.apache.org/jira/browse/aurora-742


Repository: aurora


Description
---

Add support for commands to query and display active updates being
managed by the scheduler. Two commands are added:

* "aurora update list", which shows all active updates that are being processed 
by the server.
* "aurora update status", which shows detailed status information about an 
update in-progress on the server.


Diffs
-

  src/main/python/apache/aurora/client/cli/context.py 
102d20797816788361dfdac450aac9fb8e6fbc28 
  src/main/python/apache/aurora/client/cli/options.py 
c2d422ac2bc82fc387596e93040b49f722f8310f 
  src/main/python/apache/aurora/client/cli/update.py 
c6cb98fe6aa42310090167796c971856d3dc177f 
  src/test/python/apache/aurora/client/cli/test_supdate.py 
4fb1623e497b9741ae2a350deb20030dd4036506 
  src/test/python/apache/aurora/client/cli/util.py 
a50b83c571390374975accf75e31f392dbdaaa04 

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


Testing
---

Added new unit tests; all tests pass.


Thanks,

Mark Chu-Carroll



Re: Review Request 26002: Calculate median SLA stats over sampling interval.

2014-09-24 Thread Bill Farner

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

Ship it!


Ship It!

- Bill Farner


On Sept. 24, 2014, 7:42 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26002/
> ---
> 
> (Updated Sept. 24, 2014, 7:42 p.m.)
> 
> 
> Review request for Aurora and Bill Farner.
> 
> 
> Bugs: AURORA-733
> https://issues.apache.org/jira/browse/AURORA-733
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Accepting only those events that have their triggering state event timestamp 
> within the last minute.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
> 82f1894d939a132cf0a9e8172de54845ad48a19c 
>   src/test/java/org/apache/aurora/scheduler/sla/SlaAlgorithmTest.java 
> e7755279970230be101345ac1d86bd49c77b550c 
> 
> Diff: https://reviews.apache.org/r/26002/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Review Request 26002: Calculate median SLA stats over sampling interval.

2014-09-24 Thread Maxim Khutornenko

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

Review request for Aurora and Bill Farner.


Bugs: AURORA-733
https://issues.apache.org/jira/browse/AURORA-733


Repository: aurora


Description
---

Accepting only those events that have their triggering state event timestamp 
within the last minute.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
82f1894d939a132cf0a9e8172de54845ad48a19c 
  src/test/java/org/apache/aurora/scheduler/sla/SlaAlgorithmTest.java 
e7755279970230be101345ac1d86bd49c77b550c 

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


Testing
---

gradle -Pq build


Thanks,

Maxim Khutornenko



Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

2014-09-24 Thread Maxim Khutornenko

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

Ship it!


Ship It!

- Maxim Khutornenko


On Sept. 24, 2014, 6:02 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25969/
> ---
> 
> (Updated Sept. 24, 2014, 6:02 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-718
> https://issues.apache.org/jira/browse/AURORA-718
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> When creating an update, store only the delta between the initial and desired 
> states.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 
> 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
>   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
> 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
>   src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java 
> b7143941feb4824aee609741e8e2c0e015eecac3 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 15a9e2b9acd65f6268e70fb402414f5c01c42409 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  ae0320b44b55a3630e255484ea7a881daab6a7bc 
>   src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 
> 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  348bdbfbb51800d31b67ea1b3f2632975e598306 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 
> 1d0bbfef583708bfdab30802724273e4b21ecf8f 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 842256a16fe2775af865ed2ee91fd719b3476577 
>   src/main/python/apache/aurora/client/api/__init__.py 
> bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  e1179b449c05adc56206bfef271a9d440a77e042 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> f7c7205d80edac714a12c2aff0cc7db27b1909b4 
>   src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
> ad4040c4a58d3e6f075a60712144aa6968270a55 
>   
> src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 
> 548d31580b5f2bae92900fddb80005f7d274a155 
> 
> Diff: https://reviews.apache.org/r/25969/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>



Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

2014-09-24 Thread Bill Farner

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

(Updated Sept. 24, 2014, 6:02 p.m.)


Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


Bugs: AURORA-718
https://issues.apache.org/jira/browse/AURORA-718


Repository: aurora


Description
---

When creating an update, store only the delta between the initial and desired 
states.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/base/Numbers.java 
5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
  src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
  src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java 
b7143941feb4824aee609741e8e2c0e015eecac3 
  src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
15a9e2b9acd65f6268e70fb402414f5c01c42409 
  
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
 26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
ae0320b44b55a3630e255484ea7a881daab6a7bc 
  src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 
8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
  
src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 
348bdbfbb51800d31b67ea1b3f2632975e598306 
  src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 
1d0bbfef583708bfdab30802724273e4b21ecf8f 
  src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
842256a16fe2775af865ed2ee91fd719b3476577 
  src/main/python/apache/aurora/client/api/__init__.py 
bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 e1179b449c05adc56206bfef271a9d440a77e042 
  src/main/thrift/org/apache/aurora/gen/api.thrift 
2376a5e530b12fbbebb4cfc7555656ae07795518 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
  src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java 
PRE-CREATION 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
f7c7205d80edac714a12c2aff0cc7db27b1909b4 
  src/test/java/org/apache/aurora/scheduler/updater/OneWayJobUpdaterTest.java 
ad4040c4a58d3e6f075a60712144aa6968270a55 
  src/test/java/org/apache/aurora/scheduler/updater/UpdateFactoryImplTest.java 
548d31580b5f2bae92900fddb80005f7d274a155 

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


Testing
---

./gradlew build -Pq


Thanks,

Bill Farner



Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

2014-09-24 Thread Bill Farner


> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java, 
> > line 195
> > 
> >
> > s/_get_/_fetch_ to match method name.

Fixed.


> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  lines 1405-1410
> > 
> >
> > How about moving this into a static helper in JobDiff for better 
> > readability?

Sure.


> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java,
> >  line 1415
> > 
> >
> > Suggest a more descriptive message here mentioning 
> > updateOnlyTheseInstances property name.

Done.


> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java,
> >  line 148
> > 
> >
> > Does this translate into INVALID_REQUEST in thrift? Not sure it's a 
> > good idea unless we expose a jobDiff RPC to reliably compare job configs 
> > before the update. Otherwise, it's client diff vs. server diff with a 
> > chance of mismatch and subpar client experience.
> > 
> > Should it rather be special-cased to return OK with a detail message 
> > stating a noop update?

Sounds good, changed to no-op/OK.


> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java,
> >  line 468
> > 
> >
> > Unless I am reading it wrong, I am not sure how this could even be 
> > possible now that noop instances are rejected by the Diff. Is there still a 
> > case for it?

Changing from rolling forward to rolling back will be the most common.  The 
updater will flip the instances, and skip untouched instances while walking 
backwards.
It can also happen if the scheduler fails over - in-flight instances will be 
re-evaluated, and may no-op at that point.


> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/thrift/org/apache/aurora/gen/api.thrift, line 694
> > 
> >
> > s/../.

Fixed.


> On Sept. 24, 2014, 4:58 p.m., Maxim Khutornenko wrote:
> > src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml,
> >  line 421
> > 
> >
> > Why left join here? Events without update ID would not make any sense.

Thanks!  I clearly can't be trusted when it comes to SQL.


- Bill


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


On Sept. 24, 2014, 12:44 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25969/
> ---
> 
> (Updated Sept. 24, 2014, 12:44 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-718
> https://issues.apache.org/jira/browse/AURORA-718
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> When creating an update, store only the delta between the initial and desired 
> states.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 
> 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
>   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
> 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
>   src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java 
> b7143941feb4824aee609741e8e2c0e015eecac3 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 15a9e2b9acd65f6268e70fb402414f5c01c42409 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  ae0320b44b55a3630e255484ea7a881daab6a7bc 
>   src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 
> 8f9e6d6fa0

Re: Review Request 25970: Adding support for retryable storage errors.

2014-09-24 Thread Bill Farner

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

Ship it!


Ship It!

- Bill Farner


On Sept. 24, 2014, 5:41 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25970/
> ---
> 
> (Updated Sept. 24, 2014, 5:41 p.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-187
> https://issues.apache.org/jira/browse/AURORA-187
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a new ERROR_RETRYABLE thrift response type to enable response-aware 
> retrying in scheduler_client. 
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  671dbd1b14e759f02f655909857099368255f214 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 775564e1de0bec7fcfd96ea5ee3ee4948567e58f 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> a21ab901d86f74d5bc22bfffc09029c99d3a398f 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> ec632516655bbca4782e01310518843ce8aca27e 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 527448f882c12fe9bb51ce2a0a4c330188838282 
> 
> Diff: https://reviews.apache.org/r/25970/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq build
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 25970: Adding support for retryable storage errors.

2014-09-24 Thread Maxim Khutornenko

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

(Updated Sept. 24, 2014, 5:41 p.m.)


Review request for Aurora, Mark Chu-Carroll, Bill Farner, and Brian Wickman.


Changes
---

CR comments.


Bugs: AURORA-187
https://issues.apache.org/jira/browse/AURORA-187


Repository: aurora


Description
---

Adding a new ERROR_RETRYABLE thrift response type to enable response-aware 
retrying in scheduler_client. 


Diffs (updated)
-

  
src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
 671dbd1b14e759f02f655909857099368255f214 
  src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
775564e1de0bec7fcfd96ea5ee3ee4948567e58f 
  src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
a21ab901d86f74d5bc22bfffc09029c99d3a398f 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
ec632516655bbca4782e01310518843ce8aca27e 
  src/main/thrift/org/apache/aurora/gen/api.thrift 
2376a5e530b12fbbebb4cfc7555656ae07795518 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
527448f882c12fe9bb51ce2a0a4c330188838282 

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


Testing
---

gradle -Pq build
./pants src/test/python:all


Thanks,

Maxim Khutornenko



Re: Review Request 25970: Adding support for retryable storage errors.

2014-09-24 Thread Maxim Khutornenko


> On Sept. 24, 2014, 4:58 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java,
> >  line 82
> > 
> >
> > Is there a downside if we always throw the more specific 
> > StorageException, rather than in these few cases?
> 
> Maxim Khutornenko wrote:
> Not sure I understand. Are you suggesting altering a StorageException 
> with something like isRetryable flag instead? Otherwise, we would not be able 
> to differentiate between truly retryable cases like these ones from something 
> like failed SQL insert/select.
> 
> Bill Farner wrote:
> Nope i'm saying why should this class have retryIfNotInReadyState() 
> throwing RetryableStorageException _and_ checkInState() throwing 
> StorageException?
> 
> i.e. can this diff just change checkInState to throw 
> RetryableStorageException?

Ah, I see. Sure, even though other places may not be actionable it does not 
hurt to throw a more specific one instead. Changed.


- Maxim


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


On Sept. 24, 2014, 12:29 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25970/
> ---
> 
> (Updated Sept. 24, 2014, 12:29 a.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-187
> https://issues.apache.org/jira/browse/AURORA-187
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a new ERROR_RETRYABLE thrift response type to enable response-aware 
> retrying in scheduler_client. 
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  671dbd1b14e759f02f655909857099368255f214 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 775564e1de0bec7fcfd96ea5ee3ee4948567e58f 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> a21ab901d86f74d5bc22bfffc09029c99d3a398f 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> ec632516655bbca4782e01310518843ce8aca27e 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 527448f882c12fe9bb51ce2a0a4c330188838282 
> 
> Diff: https://reviews.apache.org/r/25970/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq build
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 25970: Adding support for retryable storage errors.

2014-09-24 Thread Bill Farner


> On Sept. 24, 2014, 4:58 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java,
> >  line 82
> > 
> >
> > Is there a downside if we always throw the more specific 
> > StorageException, rather than in these few cases?
> 
> Maxim Khutornenko wrote:
> Not sure I understand. Are you suggesting altering a StorageException 
> with something like isRetryable flag instead? Otherwise, we would not be able 
> to differentiate between truly retryable cases like these ones from something 
> like failed SQL insert/select.

Nope i'm saying why should this class have retryIfNotInReadyState() throwing 
RetryableStorageException _and_ checkInState() throwing StorageException?

i.e. can this diff just change checkInState to throw RetryableStorageException?


- Bill


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


On Sept. 24, 2014, 12:29 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25970/
> ---
> 
> (Updated Sept. 24, 2014, 12:29 a.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-187
> https://issues.apache.org/jira/browse/AURORA-187
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a new ERROR_RETRYABLE thrift response type to enable response-aware 
> retrying in scheduler_client. 
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  671dbd1b14e759f02f655909857099368255f214 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 775564e1de0bec7fcfd96ea5ee3ee4948567e58f 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> a21ab901d86f74d5bc22bfffc09029c99d3a398f 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> ec632516655bbca4782e01310518843ce8aca27e 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 527448f882c12fe9bb51ce2a0a4c330188838282 
> 
> Diff: https://reviews.apache.org/r/25970/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq build
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 25970: Adding support for retryable storage errors.

2014-09-24 Thread Maxim Khutornenko


> On Sept. 24, 2014, 4:58 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java,
> >  line 82
> > 
> >
> > Is there a downside if we always throw the more specific 
> > StorageException, rather than in these few cases?

Not sure I understand. Are you suggesting altering a StorageException with 
something like isRetryable flag instead? Otherwise, we would not be able to 
differentiate between truly retryable cases like these ones from something like 
failed SQL insert/select.


- Maxim


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


On Sept. 24, 2014, 12:29 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25970/
> ---
> 
> (Updated Sept. 24, 2014, 12:29 a.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-187
> https://issues.apache.org/jira/browse/AURORA-187
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a new ERROR_RETRYABLE thrift response type to enable response-aware 
> retrying in scheduler_client. 
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  671dbd1b14e759f02f655909857099368255f214 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 775564e1de0bec7fcfd96ea5ee3ee4948567e58f 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> a21ab901d86f74d5bc22bfffc09029c99d3a398f 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> ec632516655bbca4782e01310518843ce8aca27e 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 527448f882c12fe9bb51ce2a0a4c330188838282 
> 
> Diff: https://reviews.apache.org/r/25970/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq build
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 25812: Implementing quota checking for async job updates.

2014-09-24 Thread Bill Farner

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

Ship it!


Ship It!

- Bill Farner


On Sept. 23, 2014, 11:08 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25812/
> ---
> 
> (Updated Sept. 23, 2014, 11:08 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin, Kevin Sweeney, and Bill Farner.
> 
> 
> Bugs: AURORA-686
> https://issues.apache.org/jira/browse/AURORA-686
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Getting it right required a deep refactoring of the QuotaManager. The prod 
> consumption is now calculated from:
> - production tasks not participating in job updates;
> - unaffected production tasks from a job being updated (i.e. those that are 
> not covered by the update working set);
> - production tasks directly affected by the job update (max of old and new 
> resources used).
> 
> Also, moved all logic back to SchedulerThriftInterface as quota checks are 
> done only there now.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
>  3661f8487985f631e3ea437fe6430e0296376a9e 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaManager.java 
> 14b0dd8f86026840d0444c128f656a144eab017d 
>   src/main/java/org/apache/aurora/scheduler/quota/QuotaUtil.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/state/StateModule.java 
> 54b90127551c69509dbd41ee95c384dbbf1a7ee4 
>   src/main/java/org/apache/aurora/scheduler/state/TaskLimitValidator.java 
> 70333737ee119ea083f0ac17a4205e7af9f4c753 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  ae0320b44b55a3630e255484ea7a881daab6a7bc 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 
> 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  348bdbfbb51800d31b67ea1b3f2632975e598306 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   src/test/java/org/apache/aurora/scheduler/quota/QuotaManagerImplTest.java 
> 8381e2126c62c40d28a325c72686d01e82bb84cf 
>   src/test/java/org/apache/aurora/scheduler/state/TaskLimitValidatorTest.java 
> 8f18617b2052201f87bb1464314c2ee45b279276 
>   
> src/test/java/org/apache/aurora/scheduler/storage/testing/StorageTestUtil.java
>  5aebbfbc691dfac4a066cb1425d18d3fccc77090 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java 
> 40156c211a346664c0d2f174235efb2049cf3bb9 
> 
> Diff: https://reviews.apache.org/r/25812/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 25970: Adding support for retryable storage errors.

2014-09-24 Thread Bill Farner

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



src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java


Is there a downside if we always throw the more specific StorageException, 
rather than in these few cases?


- Bill Farner


On Sept. 24, 2014, 12:29 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25970/
> ---
> 
> (Updated Sept. 24, 2014, 12:29 a.m.)
> 
> 
> Review request for Aurora, Mark Chu-Carroll, Bill Farner, and Brian Wickman.
> 
> 
> Bugs: AURORA-187
> https://issues.apache.org/jira/browse/AURORA-187
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Adding a new ERROR_RETRYABLE thrift response type to enable response-aware 
> retrying in scheduler_client. 
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/storage/CallOrderEnforcingStorage.java
>  671dbd1b14e759f02f655909857099368255f214 
>   src/main/java/org/apache/aurora/scheduler/storage/Storage.java 
> 775564e1de0bec7fcfd96ea5ee3ee4948567e58f 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/aop/LoggingInterceptor.java 
> a21ab901d86f74d5bc22bfffc09029c99d3a398f 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> ec632516655bbca4782e01310518843ce8aca27e 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 527448f882c12fe9bb51ce2a0a4c330188838282 
> 
> Diff: https://reviews.apache.org/r/25970/diff/
> 
> 
> Testing
> ---
> 
> gradle -Pq build
> ./pants src/test/python:all
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>



Re: Review Request 25969: When creating an update, store only the delta between the initial and desired states.

2014-09-24 Thread Maxim Khutornenko

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



src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java


s/_get_/_fetch_ to match method name.



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java


How about moving this into a static helper in JobDiff for better 
readability?



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java


Suggest a more descriptive message here mentioning updateOnlyTheseInstances 
property name.



src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java


Does this translate into INVALID_REQUEST in thrift? Not sure it's a good 
idea unless we expose a jobDiff RPC to reliably compare job configs before the 
update. Otherwise, it's client diff vs. server diff with a chance of mismatch 
and subpar client experience.

Should it rather be special-cased to return OK with a detail message 
stating a noop update?



src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java


Unless I am reading it wrong, I am not sure how this could even be possible 
now that noop instances are rejected by the Diff. Is there still a case for it?



src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml


Why left join here? Events without update ID would not make any sense.



src/main/thrift/org/apache/aurora/gen/api.thrift


s/../.


- Maxim Khutornenko


On Sept. 24, 2014, 12:44 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25969/
> ---
> 
> (Updated Sept. 24, 2014, 12:44 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-718
> https://issues.apache.org/jira/browse/AURORA-718
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> When creating an update, store only the delta between the initial and desired 
> states.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/base/Numbers.java 
> 5fd0814e14eee7eb827b3a3b1e6dc28ce910b7e8 
>   src/main/java/org/apache/aurora/scheduler/base/Tasks.java 
> 9664c3b0f04087ceb0451a4cc5a0b4dd6cdcee9d 
>   src/main/java/org/apache/aurora/scheduler/http/TransformationUtils.java 
> b7143941feb4824aee609741e8e2c0e015eecac3 
>   src/main/java/org/apache/aurora/scheduler/storage/ForwardingStore.java 
> 1874a3e3f9bc47104b7597fd88d77c0c36620f7d 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> cb74d62878af1efe9d58e3d4c3e5f7a84edc8de6 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 15a9e2b9acd65f6268e70fb402414f5c01c42409 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  26cc85dcb6dccd913e9a130a134fbf8df10cc08d 
>   
> src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
>  ae0320b44b55a3630e255484ea7a881daab6a7bc 
>   src/main/java/org/apache/aurora/scheduler/updater/JobDiff.java PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateController.java 
> 8f9e6d6fa00840d8ff94eb7cf1c4415eb5cc1269 
>   
> src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java
>  348bdbfbb51800d31b67ea1b3f2632975e598306 
>   src/main/java/org/apache/aurora/scheduler/updater/OneWayJobUpdater.java 
> 1d0bbfef583708bfdab30802724273e4b21ecf8f 
>   src/main/java/org/apache/aurora/scheduler/updater/UpdateFactory.java 
> 842256a16fe2775af865ed2ee91fd719b3476577 
>   src/main/python/apache/aurora/client/api/__init__.py 
> bbc8cf0314388b5b7cfe05562e8f9391fd722ac1 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  e1179b449c05adc56206bfef271a9d440a77e042 
>   src/main/thrift/org/apache/aurora/gen/api.thrift 
> 2376a5e530b12fbbebb4cfc7555656ae07795518 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  553ef4d88fafff894a11bc6fb6d5ab33d5f6646d 
>   
> src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
>  21e8a8ba5c47f549133ea339899bf0e5097ccbb9 
>   src/test/java/org/apache/aurora/scheduler/updater/JobDiffTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> f7c7205d80edac714a12c2aff0cc7db27b1909b4 
>   s

Re: Review Request 25963: Expose in progress and recently completed updates in UI

2014-09-24 Thread Joshua Cohen

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



src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js


restore the `_MS` suffix to make it explicit what the unit is here?



src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js


Can we just have a single function that takes the statuses as a parameter? 
If you want to maintain the simplicity of the current API we could bind a more 
generic function to a specific one. I.e.:

function getJobUpdateQuery(status) {
  var query = new JobUpdateQuery();
  query.updateStatuses = status;
  return query;
};

...

getTerminalQuery: getJobUpdateQuery.bind(this, UPDATE_TERMINAL_STATUSES );



src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html


This table and the one below for completed updates are identical (minus the 
data source). Does angular have partials or something similar so we can avoid 
replicating this?


- Joshua Cohen


On Sept. 23, 2014, 11:46 p.m., David McLaughlin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/25963/
> ---
> 
> (Updated Sept. 23, 2014, 11:46 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen, Maxim Khutornenko, and Bill Farner.
> 
> 
> Bugs: AURORA-745
> https://issues.apache.org/jira/browse/AURORA-745
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Very simple first iteration. Shows only the latest 20 updates. 
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 392b4f7bc9d5352ed536ff83eafb4175e4860aa1 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/css/app.css 
> befd5908e51dcac2a0fc56b12651af11273d25c0 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/index.html 
> 6434d1bc3b9f769f6f565d601fd98f99743efe8c 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/app.js 
> 082d920348f12b214b8ca2d20c11e7b4825453ed 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/controllers.js 
> ded175a1e4b288b8978235c986db2549c22dcc7d 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 
> f65cb5e69c2d1712b5f9da782ee2c7b3bc22cee8 
>   src/main/resources/org/apache/aurora/scheduler/http/ui/updateList.html 
> PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/25963/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew jsHint
> 
> 
> File Attachments
> 
> 
> updates page
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/23/33b2b9ff-3485-4bba-a8e8-2d4e6280e5ca__Screen_Shot_2014-09-23_at_4.15.06_PM.png
> no data updates
>   
> https://reviews.apache.org/media/uploaded/files/2014/09/23/5fe9c0bf-ef31-41a3-95c4-7aeb9abcb3b0__Screen_Shot_2014-09-23_at_4.45.59_PM.png
> 
> 
> Thanks,
> 
> David McLaughlin
> 
>