Review Request 26688: Fix errors in help rendering:

2014-10-14 Thread Mark Chu-Carroll

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

Review request for Aurora, David McLaughlin and Joshua Cohen.


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


Repository: aurora


Description
---

- Put plugin-generated options into the correct order.
- Include the option-name in the detailed help list.
- Add missing metavars.


Diffs
-

  src/main/python/apache/aurora/client/cli/__init__.py 
da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8 
  src/main/python/apache/aurora/client/cli/options.py 
dc76c25b90acb9610e40b939e65c3cabf032649f 
  src/main/python/apache/aurora/client/cli/standalone_client.py 
20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1 
  src/test/python/apache/aurora/client/cli/test_help.py 
f73c8a3778b7d118ea2865f213b442a607fb4a7d 

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


Testing
---


Thanks,

Mark Chu-Carroll



Re: Review Request 26688: Fix errors in help rendering:

2014-10-14 Thread Joshua Cohen

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


This looks good to me, just a couple of questions about the tests. Speaking of, 
can you update testing done and add an example of the new help output?

Also, David's on vacation, so you'll want to add someone else to this review.


src/test/python/apache/aurora/client/cli/test_help.py
https://reviews.apache.org/r/26688/#comment96897

Are option names guaranteed to be unique? If not this test could 
potentially pass if any help output contains a plugin option name, not 
necessarily the help output for the command to which the plugin was registered.

It's also possible for a plugin option name to appear in the help for 
another option, and not on its own, which would cause this test to succeed even 
if the plugin options themselves are not properly displayed?

I guess what I'm getting at is would it be better to test for more than 
just the appearance of a string at any point in the output?

(This may be based on incomplete understanding of how the client registers 
commands/options).



src/test/python/apache/aurora/client/cli/test_help.py
https://reviews.apache.org/r/26688/#comment96898

Are there any other varieties of metavars? could we have int metavars that 
are similarly unset, resulting in help output that just says int rather than 
str?


- Joshua Cohen


On Oct. 14, 2014, 3:07 p.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26688/
 ---
 
 (Updated Oct. 14, 2014, 3:07 p.m.)
 
 
 Review request for Aurora, David McLaughlin and Joshua Cohen.
 
 
 Bugs: aurora-831
 https://issues.apache.org/jira/browse/aurora-831
 
 
 Repository: aurora
 
 
 Description
 ---
 
 - Put plugin-generated options into the correct order.
 - Include the option-name in the detailed help list.
 - Add missing metavars.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8 
   src/main/python/apache/aurora/client/cli/options.py 
 dc76c25b90acb9610e40b939e65c3cabf032649f 
   src/main/python/apache/aurora/client/cli/standalone_client.py 
 20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1 
   src/test/python/apache/aurora/client/cli/test_help.py 
 f73c8a3778b7d118ea2865f213b442a607fb4a7d 
 
 Diff: https://reviews.apache.org/r/26688/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 26688: Fix errors in help rendering:

2014-10-14 Thread Mark Chu-Carroll

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



src/test/python/apache/aurora/client/cli/test_help.py
https://reviews.apache.org/r/26688/#comment96900

Yes, they are guaranteed to be unique. The argparse framework that this is 
built on checks uniqueness, and raises an exception if there's any ambiguity.



src/test/python/apache/aurora/client/cli/test_help.py
https://reviews.apache.org/r/26688/#comment96902

Anything can have an unset metavar, but most of the time, that isn't a 
problem. The default metavar for an int is int - but in help strings, that 
works well.

For example, you'll often see [--port=int]. That's good, and I don't think 
we want that to be an error. Saying that the value is expected to be int is 
telling you something essential about the value expected for the parameter.

str is different, because it's the default type value for any structured 
input - pathnames, config names, tunnel descriptors, instance lists, usernames, 
etc. Saying str doesn't tell you anything meaningful about what's expected.


- Mark Chu-Carroll


On Oct. 14, 2014, 11:07 a.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26688/
 ---
 
 (Updated Oct. 14, 2014, 11:07 a.m.)
 
 
 Review request for Aurora, David McLaughlin and Joshua Cohen.
 
 
 Bugs: aurora-831
 https://issues.apache.org/jira/browse/aurora-831
 
 
 Repository: aurora
 
 
 Description
 ---
 
 - Put plugin-generated options into the correct order.
 - Include the option-name in the detailed help list.
 - Add missing metavars.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8 
   src/main/python/apache/aurora/client/cli/options.py 
 dc76c25b90acb9610e40b939e65c3cabf032649f 
   src/main/python/apache/aurora/client/cli/standalone_client.py 
 20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1 
   src/test/python/apache/aurora/client/cli/test_help.py 
 f73c8a3778b7d118ea2865f213b442a607fb4a7d 
 
 Diff: https://reviews.apache.org/r/26688/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 26688: Fix errors in help rendering:

2014-10-14 Thread Mark Chu-Carroll


 On Oct. 14, 2014, 1 p.m., Mark Chu-Carroll wrote:
  src/test/python/apache/aurora/client/cli/test_help.py, line 75
  https://reviews.apache.org/r/26688/diff/1/?file=720844#file720844line75
 
  Yes, they are guaranteed to be unique. The argparse framework that this 
  is built on checks uniqueness, and raises an exception if there's any 
  ambiguity.
 
 Joshua Cohen wrote:
 What about the second question (could an option name appearing elsewhere 
 in the help output cause this test to improperly pass)?

I don't think so - that would require *every* command to have option names that 
were superstrings of every option in the plugins.


- Mark


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


On Oct. 14, 2014, 11:07 a.m., Mark Chu-Carroll wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26688/
 ---
 
 (Updated Oct. 14, 2014, 11:07 a.m.)
 
 
 Review request for Aurora, David McLaughlin and Joshua Cohen.
 
 
 Bugs: aurora-831
 https://issues.apache.org/jira/browse/aurora-831
 
 
 Repository: aurora
 
 
 Description
 ---
 
 - Put plugin-generated options into the correct order.
 - Include the option-name in the detailed help list.
 - Add missing metavars.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/cli/__init__.py 
 da9d5b6ba4d22ba1f444341b97bbcfaf7889a4a8 
   src/main/python/apache/aurora/client/cli/options.py 
 dc76c25b90acb9610e40b939e65c3cabf032649f 
   src/main/python/apache/aurora/client/cli/standalone_client.py 
 20f4d7ef43ba336a2b6d02cbf5656c97bdfa2ea1 
   src/test/python/apache/aurora/client/cli/test_help.py 
 f73c8a3778b7d118ea2865f213b442a607fb4a7d 
 
 Diff: https://reviews.apache.org/r/26688/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Mark Chu-Carroll
 




Re: Review Request 26431: Moving post_drain script execution into host_maintenance.py

2014-10-14 Thread Maxim Khutornenko

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


Mark?

- Maxim Khutornenko


On Oct. 8, 2014, 11:46 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26431/
 ---
 
 (Updated Oct. 8, 2014, 11:46 p.m.)
 
 
 Review request for Aurora, Joe Smith and Mark Chu-Carroll.
 
 
 Bugs: AURORA-806
 https://issues.apache.org/jira/browse/AURORA-806
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Moving post_drain script functionality into host_maintenance.py to support 
 per batch execution.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/admin/host_maintenance.py 
 9c2a9f77109791da574e1624d27b6b7096a2678e 
   src/main/python/apache/aurora/client/commands/maintenance.py 
 e465d973e9f764076e18491e1691d44303c0f388 
   src/test/python/apache/aurora/admin/test_host_maintenance.py 
 40228df59e43bc6034f2dc651c166a0c4b78aea8 
 
 Diff: https://reviews.apache.org/r/26431/diff/
 
 
 Testing
 ---
 
 ./pants src/test/python:all
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 26478: Add a flag to deduplicate storage snapshots

2014-10-14 Thread Bill Farner

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


Ping?  Any progress here?

- Bill Farner


On Oct. 9, 2014, 2:39 a.m., Kevin Sweeney wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26478/
 ---
 
 (Updated Oct. 9, 2014, 2:39 a.m.)
 
 
 Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji.
 
 
 Bugs: AURORA-722
 https://issues.apache.org/jira/browse/AURORA-722
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Add a new format for deduplicated storage snapshots. Microbenchmarks show a 
 10x deduplication ratio on Twitter's production snapshots.
 
 This format is backwards-incompatible, so this patch introduces a flag to 
 control its use (defaulting off).
 
 This only changes the format used to write to the replicated log (where time 
 is of the essence since all writes are done holding the global storage lock) 
 - the format of backups written to disk is unchanged, as backups don't hold 
 the lock.
 
 
 Diffs
 -
 
   build.gradle 2e1bf78d7797f17afd51a94a22eff80e00aba464 
   src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
 65e986eaa2c4193431ca048425a1ed3ab60f5882 
   src/main/java/org/apache/aurora/scheduler/storage/log/EntrySerializer.java 
 7239a6a5eb5479e395e16423c83fdf80a77e5a83 
   src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java 
 4b50e2069407dc263b4fc93f1827d3a8836253bf 
   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
 f806297d1d0700155c976743f936b2b8a3a390fb 
   src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
 769348e6b8a5c701734afff391b1c77de35222c6 
   
 src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java
  PRE-CREATION 
   src/main/java/org/apache/aurora/scheduler/storage/log/StreamManager.java 
 22db80eaf34fe736fa5a3a9289836c9ac9e59906 
   
 src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java 
 e5cfbf5cf43bf5bbc38c42fe685a7e9f0d03af2a 
   src/main/thrift/org/apache/aurora/gen/storage.thrift 
 5350ec945fbe028ee4641683815a068ce00b5efc 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogManagerTest.java 
 39729b374fe4e383f9b5ada7d016923766df9af7 
   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
 7a8c3b882633376a1bf6a78616d55aaa7401d13f 
   
 src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicatorImplTest.java
  PRE-CREATION 
 
 Diff: https://reviews.apache.org/r/26478/diff/
 
 
 Testing
 ---
 
 ./gradlew -Pq build
 
 
 Thanks,
 
 Kevin Sweeney
 




Review Request 26711: Fix bad function call in thermos_task_runner.

2014-10-14 Thread Bill Farner

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

Review request for Aurora, Joshua Cohen and Kevin Sweeney.


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


Repository: aurora


Description
---

Fix bad function call in thermos_task_runner.


Diffs
-

  src/main/python/apache/aurora/executor/thermos_task_runner.py 
4e688449767af40fd5c43c963310a3ebe1e780c4 

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


Testing
---

./pants build --timeout=60 src/test/python:all -vxs


Thanks,

Bill Farner



Re: Review Request 26711: Fix bad function call in thermos_task_runner.

2014-10-14 Thread Joshua Cohen

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

Ship it!


Ship It!

- Joshua Cohen


On Oct. 14, 2014, 10:53 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26711/
 ---
 
 (Updated Oct. 14, 2014, 10:53 p.m.)
 
 
 Review request for Aurora, Joshua Cohen and Kevin Sweeney.
 
 
 Bugs: AURORA-836
 https://issues.apache.org/jira/browse/AURORA-836
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Fix bad function call in thermos_task_runner.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/executor/thermos_task_runner.py 
 4e688449767af40fd5c43c963310a3ebe1e780c4 
 
 Diff: https://reviews.apache.org/r/26711/diff/
 
 
 Testing
 ---
 
 ./pants build --timeout=60 src/test/python:all -vxs
 
 
 Thanks,
 
 Bill Farner
 




Review Request 26714: Remove use of the getVersion RPC from the client.

2014-10-14 Thread Bill Farner

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

Review request for Aurora, Joshua Cohen and Maxim Khutornenko.


Repository: aurora


Description
---

Remove use of the getVersion RPC from the client.


Diffs
-

  src/main/python/apache/aurora/client/api/scheduler_client.py 
7f1c82bdbca427d1a09271b1e22f77f66da8e767 
  src/test/python/apache/aurora/client/api/test_restarter.py 
f1bf545a1aa1ab36f05fb0c6ea2ac7e4b1677932 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
d78e7dca28d67997bc6c98cff619ab94a257c7dc 
  src/test/python/apache/aurora/client/api/test_updater.py 
e8eaa9e6aa5fb3bc52a7195c26d9bd8294256780 
  src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
a2b28ba23961284ba60358af54726e0386dd69b6 
  src/test/python/apache/aurora/client/fake_scheduler_proxy.py 
12e70e9be9e3cf707f760ccd314c79825924c8bb 

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


Testing
---

./build-support/jenkins/build.sh


Thanks,

Bill Farner



Re: Review Request 26664: Deprecating SANDBOX_DELETED task state.

2014-10-14 Thread Bill Farner

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



src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java
https://reviews.apache.org/r/26664/#comment96975

Isn't this unsafe?  Seems like we need to treat the state the same in 
0.6.0, and then actually change behavior and remove in 0.7.0.


- Bill Farner


On Oct. 13, 2014, 11:22 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26664/
 ---
 
 (Updated Oct. 13, 2014, 11:22 p.m.)
 
 
 Review request for Aurora, Joshua Cohen and Bill Farner.
 
 
 Bugs: AURORA-751
 https://issues.apache.org/jira/browse/AURORA-751
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Dropping the SANDBOX_DELETED from the scheduler.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
 cfab57896f9c76754ba3b42742504fb7e7a2cf79 
   src/main/java/org/apache/aurora/scheduler/base/Jobs.java 
 9ba83fa93409de7c6254bd8e7cc27e6bc10186e0 
   src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
 0f6731106c53420b92e60b9faf26c3614bd7ae00 
   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
 86a8eb57ce7074f71d5212b34defe4320a5c430d 
   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6 
   src/main/python/apache/aurora/executor/gc_executor.py 
 9b40adaaa3634a451b4047915485e0e97d8f7914 
   src/main/resources/scheduler/assets/js/controllers.js 
 7e9037ee921b009dc2b7c5adcf057bedebb01632 
   src/main/resources/scheduler/assets/js/filters.js 
 7e8ca8408628d6f658da4267eb763e8fb4cb68c9 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 8794731f4b3f1033588bdfa33c292e4796319a2a 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 606c4434b7158220ccf1403b6deac939021fee31 
   src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 
 f2d153f446247032ad9d8d173fb70870dbfdcca1 
   src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 
 53d2c6bb78ad08a84639c1ecd48ba64d17c3f9fc 
   src/test/java/org/apache/aurora/scheduler/base/JobsTest.java 
 f3b6ff0aa72c873d6b3cf3a1d327033c791ff6b9 
   src/test/java/org/apache/aurora/scheduler/sla/SlaAlgorithmTest.java 
 eccf0c757d1e6addcd7619120f96ffa5f1ac38b5 
   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
 cdd29ea2b6fc92b967571028d299260556e16d42 
   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
 cb3254728a697a963b92baff31b02dafa2be0039 
   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
 80646a685ea918d80efafc5773e5805000a9c012 
 
 Diff: https://reviews.apache.org/r/26664/diff/
 
 
 Testing
 ---
 
 ./gradle -Pq build
 Verified UI in vagrant.
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 26664: Deprecating SANDBOX_DELETED task state.

2014-10-14 Thread Bill Farner


 On Oct. 14, 2014, 11:26 p.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java, 
  line 183
  https://reviews.apache.org/r/26664/diff/1/?file=719801#file719801line183
 
  Isn't this unsafe?  Seems like we need to treat the state the same in 
  0.6.0, and then actually change behavior and remove in 0.7.0.

Sorry for the delayed/brief review - stopped short at the compatibility issue.  
We could move forward with some behavioral changes within the scheduler, so 
long as we rewrite `SANDBOX_DELETED` into something else (probably just 
deleting that `TaskEvent` and re-surfacing the previous).


- Bill


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


On Oct. 13, 2014, 11:22 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26664/
 ---
 
 (Updated Oct. 13, 2014, 11:22 p.m.)
 
 
 Review request for Aurora, Joshua Cohen and Bill Farner.
 
 
 Bugs: AURORA-751
 https://issues.apache.org/jira/browse/AURORA-751
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Dropping the SANDBOX_DELETED from the scheduler.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
 cfab57896f9c76754ba3b42742504fb7e7a2cf79 
   src/main/java/org/apache/aurora/scheduler/base/Jobs.java 
 9ba83fa93409de7c6254bd8e7cc27e6bc10186e0 
   src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
 0f6731106c53420b92e60b9faf26c3614bd7ae00 
   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
 86a8eb57ce7074f71d5212b34defe4320a5c430d 
   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6 
   src/main/python/apache/aurora/executor/gc_executor.py 
 9b40adaaa3634a451b4047915485e0e97d8f7914 
   src/main/resources/scheduler/assets/js/controllers.js 
 7e9037ee921b009dc2b7c5adcf057bedebb01632 
   src/main/resources/scheduler/assets/js/filters.js 
 7e8ca8408628d6f658da4267eb763e8fb4cb68c9 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 8794731f4b3f1033588bdfa33c292e4796319a2a 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 606c4434b7158220ccf1403b6deac939021fee31 
   src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 
 f2d153f446247032ad9d8d173fb70870dbfdcca1 
   src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 
 53d2c6bb78ad08a84639c1ecd48ba64d17c3f9fc 
   src/test/java/org/apache/aurora/scheduler/base/JobsTest.java 
 f3b6ff0aa72c873d6b3cf3a1d327033c791ff6b9 
   src/test/java/org/apache/aurora/scheduler/sla/SlaAlgorithmTest.java 
 eccf0c757d1e6addcd7619120f96ffa5f1ac38b5 
   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
 cdd29ea2b6fc92b967571028d299260556e16d42 
   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
 cb3254728a697a963b92baff31b02dafa2be0039 
   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
 80646a685ea918d80efafc5773e5805000a9c012 
 
 Diff: https://reviews.apache.org/r/26664/diff/
 
 
 Testing
 ---
 
 ./gradle -Pq build
 Verified UI in vagrant.
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 26664: Deprecating SANDBOX_DELETED task state.

2014-10-14 Thread Maxim Khutornenko


 On Oct. 14, 2014, 11:26 p.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java, 
  line 183
  https://reviews.apache.org/r/26664/diff/1/?file=719801#file719801line183
 
  Isn't this unsafe?  Seems like we need to treat the state the same in 
  0.6.0, and then actually change behavior and remove in 0.7.0.
 
 Bill Farner wrote:
 Sorry for the delayed/brief review - stopped short at the compatibility 
 issue.  We could move forward with some behavioral changes within the 
 scheduler, so long as we rewrite `SANDBOX_DELETED` into something else 
 (probably just deleting that `TaskEvent` and re-surfacing the previous).

Yeah, that's exactly what is happening in StorageBackfill.java.


- Maxim


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


On Oct. 13, 2014, 11:22 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26664/
 ---
 
 (Updated Oct. 13, 2014, 11:22 p.m.)
 
 
 Review request for Aurora, Joshua Cohen and Bill Farner.
 
 
 Bugs: AURORA-751
 https://issues.apache.org/jira/browse/AURORA-751
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Dropping the SANDBOX_DELETED from the scheduler.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
 cfab57896f9c76754ba3b42742504fb7e7a2cf79 
   src/main/java/org/apache/aurora/scheduler/base/Jobs.java 
 9ba83fa93409de7c6254bd8e7cc27e6bc10186e0 
   src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
 0f6731106c53420b92e60b9faf26c3614bd7ae00 
   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
 86a8eb57ce7074f71d5212b34defe4320a5c430d 
   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6 
   src/main/python/apache/aurora/executor/gc_executor.py 
 9b40adaaa3634a451b4047915485e0e97d8f7914 
   src/main/resources/scheduler/assets/js/controllers.js 
 7e9037ee921b009dc2b7c5adcf057bedebb01632 
   src/main/resources/scheduler/assets/js/filters.js 
 7e8ca8408628d6f658da4267eb763e8fb4cb68c9 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 8794731f4b3f1033588bdfa33c292e4796319a2a 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 606c4434b7158220ccf1403b6deac939021fee31 
   src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 
 f2d153f446247032ad9d8d173fb70870dbfdcca1 
   src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 
 53d2c6bb78ad08a84639c1ecd48ba64d17c3f9fc 
   src/test/java/org/apache/aurora/scheduler/base/JobsTest.java 
 f3b6ff0aa72c873d6b3cf3a1d327033c791ff6b9 
   src/test/java/org/apache/aurora/scheduler/sla/SlaAlgorithmTest.java 
 eccf0c757d1e6addcd7619120f96ffa5f1ac38b5 
   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
 cdd29ea2b6fc92b967571028d299260556e16d42 
   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
 cb3254728a697a963b92baff31b02dafa2be0039 
   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
 80646a685ea918d80efafc5773e5805000a9c012 
 
 Diff: https://reviews.apache.org/r/26664/diff/
 
 
 Testing
 ---
 
 ./gradle -Pq build
 Verified UI in vagrant.
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 26424: Disable requests http connection logging.

2014-10-14 Thread Bill Farner

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

Ship it!


Sorry for the delay on this, i didn't realize i was on the hook to commit.

Now on master:
$ git log -1
commit 72fed752fcdbacf0f29e7eaef56c177f5ec9fcf0
Author: Joshua Cohen jco...@twopensource.com
Date:   Tue Oct 14 16:32:03 2014 -0700

Disable requests http connection logging.

Bugs closed: AURORA-770

Reviewed at https://reviews.apache.org/r/26424/

- Bill Farner


On Oct. 8, 2014, 4:54 p.m., Joshua Cohen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26424/
 ---
 
 (Updated Oct. 8, 2014, 4:54 p.m.)
 
 
 Review request for Aurora, Joe Smith and Bill Farner.
 
 
 Bugs: AURORA-770
 https://issues.apache.org/jira/browse/AURORA-770
 
 
 Repository: aurora
 
 
 Description
 ---
 
 .
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/common/transport.py 
 6f7c355d725b5e537cc4ae471170eaa8431da326 
   src/test/python/apache/aurora/common/test_transport.py 
 c722eae2d04dec90e9c772f49c578184a2bdf76c 
 
 Diff: https://reviews.apache.org/r/26424/diff/
 
 
 Testing
 ---
 
 Added hokey test, ran hokey test.
 
 Also verified the lack of http connection logs when running client commands 
 directly.
 
 
 Thanks,
 
 Joshua Cohen
 




Review Request 26716: Use JobKey when getting roles to authenticate against.

2014-10-14 Thread Zameer Manji

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

Review request for Aurora.


Repository: aurora


Description
---

Use JobKey when getting roles to authenticate against.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/base/Query.java 
d8572bb21a92025e7a51cf18d5bdf00fc1281078 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
a43e5d7748c22d60f56f03a8a3d52949faebeff2 
  src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java 
3b61ff3c237de3ec5224a239d1756ce8d7093a52 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 0d51f7dc367081f72090736e36605bf363f3395e 

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


Testing
---


Thanks,

Zameer Manji



Re: Review Request 26716: Use JobKey when getting roles to authenticate against.

2014-10-14 Thread Zameer Manji

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

(Updated Oct. 14, 2014, 5:23 p.m.)


Review request for Aurora, Kevin Sweeney and Bill Farner.


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


Repository: aurora


Description
---

Use JobKey when getting roles to authenticate against.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/base/Query.java 
d8572bb21a92025e7a51cf18d5bdf00fc1281078 
  
src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java 
a43e5d7748c22d60f56f03a8a3d52949faebeff2 
  src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java 
3b61ff3c237de3ec5224a239d1756ce8d7093a52 
  
src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
 0d51f7dc367081f72090736e36605bf363f3395e 

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


Testing (updated)
---

./gradlew clean test


Thanks,

Zameer Manji



Re: Review Request 26664: Deprecating SANDBOX_DELETED task state.

2014-10-14 Thread Bill Farner


 On Oct. 14, 2014, 11:26 p.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java, 
  line 183
  https://reviews.apache.org/r/26664/diff/1/?file=719801#file719801line183
 
  Isn't this unsafe?  Seems like we need to treat the state the same in 
  0.6.0, and then actually change behavior and remove in 0.7.0.
 
 Bill Farner wrote:
 Sorry for the delayed/brief review - stopped short at the compatibility 
 issue.  We could move forward with some behavioral changes within the 
 scheduler, so long as we rewrite `SANDBOX_DELETED` into something else 
 (probably just deleting that `TaskEvent` and re-surfacing the previous).
 
 Maxim Khutornenko wrote:
 Yeah, that's exactly what is happening in StorageBackfill.java.

:sigh: i swore i scanned and even did a ctrl-f for good measure :-/
Returning for another look.


- Bill


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


On Oct. 13, 2014, 11:22 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26664/
 ---
 
 (Updated Oct. 13, 2014, 11:22 p.m.)
 
 
 Review request for Aurora, Joshua Cohen and Bill Farner.
 
 
 Bugs: AURORA-751
 https://issues.apache.org/jira/browse/AURORA-751
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Dropping the SANDBOX_DELETED from the scheduler.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
 cfab57896f9c76754ba3b42742504fb7e7a2cf79 
   src/main/java/org/apache/aurora/scheduler/base/Jobs.java 
 9ba83fa93409de7c6254bd8e7cc27e6bc10186e0 
   src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
 0f6731106c53420b92e60b9faf26c3614bd7ae00 
   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
 86a8eb57ce7074f71d5212b34defe4320a5c430d 
   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6 
   src/main/python/apache/aurora/executor/gc_executor.py 
 9b40adaaa3634a451b4047915485e0e97d8f7914 
   src/main/resources/scheduler/assets/js/controllers.js 
 7e9037ee921b009dc2b7c5adcf057bedebb01632 
   src/main/resources/scheduler/assets/js/filters.js 
 7e8ca8408628d6f658da4267eb763e8fb4cb68c9 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 8794731f4b3f1033588bdfa33c292e4796319a2a 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 606c4434b7158220ccf1403b6deac939021fee31 
   src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 
 f2d153f446247032ad9d8d173fb70870dbfdcca1 
   src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 
 53d2c6bb78ad08a84639c1ecd48ba64d17c3f9fc 
   src/test/java/org/apache/aurora/scheduler/base/JobsTest.java 
 f3b6ff0aa72c873d6b3cf3a1d327033c791ff6b9 
   src/test/java/org/apache/aurora/scheduler/sla/SlaAlgorithmTest.java 
 eccf0c757d1e6addcd7619120f96ffa5f1ac38b5 
   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
 cdd29ea2b6fc92b967571028d299260556e16d42 
   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
 cb3254728a697a963b92baff31b02dafa2be0039 
   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
 80646a685ea918d80efafc5773e5805000a9c012 
 
 Diff: https://reviews.apache.org/r/26664/diff/
 
 
 Testing
 ---
 
 ./gradle -Pq build
 Verified UI in vagrant.
 
 
 Thanks,
 
 Maxim Khutornenko
 




Re: Review Request 26664: Deprecating SANDBOX_DELETED task state.

2014-10-14 Thread Bill Farner

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

Ship it!



src/main/python/apache/aurora/executor/gc_executor.py
https://reviews.apache.org/r/26664/#comment97005

More context?  It's not clear what you mean here.



src/main/resources/scheduler/assets/js/controllers.js
https://reviews.apache.org/r/26664/#comment97007

Can this flag just be removed now?


- Bill Farner


On Oct. 13, 2014, 11:22 p.m., Maxim Khutornenko wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26664/
 ---
 
 (Updated Oct. 13, 2014, 11:22 p.m.)
 
 
 Review request for Aurora, Joshua Cohen and Bill Farner.
 
 
 Bugs: AURORA-751
 https://issues.apache.org/jira/browse/AURORA-751
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Dropping the SANDBOX_DELETED from the scheduler.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java 
 cfab57896f9c76754ba3b42742504fb7e7a2cf79 
   src/main/java/org/apache/aurora/scheduler/base/Jobs.java 
 9ba83fa93409de7c6254bd8e7cc27e6bc10186e0 
   src/main/java/org/apache/aurora/scheduler/sla/SlaAlgorithm.java 
 0f6731106c53420b92e60b9faf26c3614bd7ae00 
   src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java 
 86a8eb57ce7074f71d5212b34defe4320a5c430d 
   src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java 
 8c20ab6f2bebf1d1c0f91fed3f1e48361cdf45d6 
   src/main/python/apache/aurora/executor/gc_executor.py 
 9b40adaaa3634a451b4047915485e0e97d8f7914 
   src/main/resources/scheduler/assets/js/controllers.js 
 7e9037ee921b009dc2b7c5adcf057bedebb01632 
   src/main/resources/scheduler/assets/js/filters.js 
 7e8ca8408628d6f658da4267eb763e8fb4cb68c9 
   src/main/thrift/org/apache/aurora/gen/api.thrift 
 8794731f4b3f1033588bdfa33c292e4796319a2a 
   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
 606c4434b7158220ccf1403b6deac939021fee31 
   src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java 
 f2d153f446247032ad9d8d173fb70870dbfdcca1 
   src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 
 53d2c6bb78ad08a84639c1ecd48ba64d17c3f9fc 
   src/test/java/org/apache/aurora/scheduler/base/JobsTest.java 
 f3b6ff0aa72c873d6b3cf3a1d327033c791ff6b9 
   src/test/java/org/apache/aurora/scheduler/sla/SlaAlgorithmTest.java 
 eccf0c757d1e6addcd7619120f96ffa5f1ac38b5 
   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java 
 cdd29ea2b6fc92b967571028d299260556e16d42 
   src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java 
 cb3254728a697a963b92baff31b02dafa2be0039 
   src/test/java/org/apache/aurora/scheduler/storage/StorageBackfillTest.java 
 80646a685ea918d80efafc5773e5805000a9c012 
 
 Diff: https://reviews.apache.org/r/26664/diff/
 
 
 Testing
 ---
 
 ./gradle -Pq build
 Verified UI in vagrant.
 
 
 Thanks,
 
 Maxim Khutornenko
 




Review Request 26720: Limit the amount of work done while bootstrapping thrift.

2014-10-14 Thread Bill Farner

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

Review request for Aurora, Kevin Sweeney and Zameer Manji.


Repository: aurora


Description
---

Limit the amount of work done while bootstrapping thrift.


Diffs
-

  build-support/thrift/Makefile 2b62b15ee946862b2bea6a4df2b7fdec2be1426a 

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


Testing
---

Through a single trial on my workstation:

Before this patch:
$ git clean -fdx
$ time ./gradlew build  /dev/null
real2m2.406s
user5m14.128s
sys 0m12.222s

After this patch:
$ git clean -fdx
$ time ./gradlew build  /dev/null
real1m30.926s
user5m18.393s
sys 0m11.148s


Thanks,

Bill Farner



Re: Review Request 26720: Limit the amount of work done while bootstrapping thrift.

2014-10-14 Thread Kevin Sweeney

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

Ship it!



build-support/thrift/Makefile
https://reviews.apache.org/r/26720/#comment97016

Not your issue but this line appears impossible to execute.


- Kevin Sweeney


On Oct. 14, 2014, 5:59 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26720/
 ---
 
 (Updated Oct. 14, 2014, 5:59 p.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Zameer Manji.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Limit the amount of work done while bootstrapping thrift.
 
 
 Diffs
 -
 
   build-support/thrift/Makefile 2b62b15ee946862b2bea6a4df2b7fdec2be1426a 
 
 Diff: https://reviews.apache.org/r/26720/diff/
 
 
 Testing
 ---
 
 Through a single trial on my workstation:
 
 Before this patch:
 $ git clean -fdx
 $ time ./gradlew build  /dev/null
 real  2m2.406s
 user  5m14.128s
 sys   0m12.222s
 
 After this patch:
 $ git clean -fdx
 $ time ./gradlew build  /dev/null
 real  1m30.926s
 user  5m18.393s
 sys   0m11.148s
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 26716: Use JobKey when getting roles to authenticate against.

2014-10-14 Thread Bill Farner

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

Ship it!


Ship It!

- Bill Farner


On Oct. 15, 2014, 12:23 a.m., Zameer Manji wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26716/
 ---
 
 (Updated Oct. 15, 2014, 12:23 a.m.)
 
 
 Review request for Aurora, Kevin Sweeney and Bill Farner.
 
 
 Bugs: AURORA-837
 https://issues.apache.org/jira/browse/AURORA-837
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Use JobKey when getting roles to authenticate against.
 
 
 Diffs
 -
 
   src/main/java/org/apache/aurora/scheduler/base/Query.java 
 d8572bb21a92025e7a51cf18d5bdf00fc1281078 
   
 src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
  a43e5d7748c22d60f56f03a8a3d52949faebeff2 
   src/test/java/org/apache/aurora/scheduler/storage/mem/MemTaskStoreTest.java 
 3b61ff3c237de3ec5224a239d1756ce8d7093a52 
   
 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
  0d51f7dc367081f72090736e36605bf363f3395e 
 
 Diff: https://reviews.apache.org/r/26716/diff/
 
 
 Testing
 ---
 
 ./gradlew clean test
 
 
 Thanks,
 
 Zameer Manji
 




Re: Review Request 26714: Remove use of the getVersion RPC from the client.

2014-10-14 Thread Maxim Khutornenko

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


Should we rather do it in 0.7.0 as deprecation warning suggests? If it's 
dropped now the client will be unable to talk to 0.5.0 scheduler.

- Maxim Khutornenko


On Oct. 14, 2014, 11:22 p.m., Bill Farner wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26714/
 ---
 
 (Updated Oct. 14, 2014, 11:22 p.m.)
 
 
 Review request for Aurora, Joshua Cohen and Maxim Khutornenko.
 
 
 Repository: aurora
 
 
 Description
 ---
 
 Remove use of the getVersion RPC from the client.
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/client/api/scheduler_client.py 
 7f1c82bdbca427d1a09271b1e22f77f66da8e767 
   src/test/python/apache/aurora/client/api/test_restarter.py 
 f1bf545a1aa1ab36f05fb0c6ea2ac7e4b1677932 
   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
 d78e7dca28d67997bc6c98cff619ab94a257c7dc 
   src/test/python/apache/aurora/client/api/test_updater.py 
 e8eaa9e6aa5fb3bc52a7195c26d9bd8294256780 
   src/test/python/apache/aurora/client/cli/test_api_from_cli.py 
 a2b28ba23961284ba60358af54726e0386dd69b6 
   src/test/python/apache/aurora/client/fake_scheduler_proxy.py 
 12e70e9be9e3cf707f760ccd314c79825924c8bb 
 
 Diff: https://reviews.apache.org/r/26714/diff/
 
 
 Testing
 ---
 
 ./build-support/jenkins/build.sh
 
 
 Thanks,
 
 Bill Farner
 




Re: Review Request 26478: Add a flag to deduplicate storage snapshots

2014-10-14 Thread Kevin Sweeney

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

(Updated Oct. 14, 2014, 6:32 p.m.)


Review request for Aurora, David McLaughlin, Bill Farner, and Zameer Manji.


Changes
---

Documentation


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


Repository: aurora


Description
---

Add a new format for deduplicated storage snapshots. Microbenchmarks show a 10x 
deduplication ratio on Twitter's production snapshots.

This format is backwards-incompatible, so this patch introduces a flag to 
control its use (defaulting off).

This only changes the format used to write to the replicated log (where time is 
of the essence since all writes are done holding the global storage lock) - the 
format of backups written to disk is unchanged, as backups don't hold the lock.


Diffs (updated)
-

  config/legacy_untested_classes.txt 3af99867eb25a7e44bb3520e82b1def125bd6e15 
  docs/scheduler-storage.md PRE-CREATION 
  src/main/java/org/apache/aurora/codec/ThriftBinaryCodec.java 
65e986eaa2c4193431ca048425a1ed3ab60f5882 
  src/main/java/org/apache/aurora/scheduler/storage/log/EntrySerializer.java 
7239a6a5eb5479e395e16423c83fdf80a77e5a83 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java 
4b50e2069407dc263b4fc93f1827d3a8836253bf 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java 
f806297d1d0700155c976743f936b2b8a3a390fb 
  src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java 
769348e6b8a5c701734afff391b1c77de35222c6 
  
src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/log/StreamManager.java 
22db80eaf34fe736fa5a3a9289836c9ac9e59906 
  src/main/java/org/apache/aurora/scheduler/storage/log/StreamManagerImpl.java 
e5cfbf5cf43bf5bbc38c42fe685a7e9f0d03af2a 
  src/main/thrift/org/apache/aurora/gen/storage.thrift 
5350ec945fbe028ee4641683815a068ce00b5efc 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogManagerTest.java 
39729b374fe4e383f9b5ada7d016923766df9af7 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
7a8c3b882633376a1bf6a78616d55aaa7401d13f 
  
src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicatorImplTest.java
 PRE-CREATION 

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


Testing
---

./gradlew -Pq build


Thanks,

Kevin Sweeney



Re: Review Request 26478: Add a flag to deduplicate storage snapshots

2014-10-14 Thread Kevin Sweeney


 On Oct. 9, 2014, 8:30 a.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/storage/log/EntrySerializer.java, 
  line 86
  https://reviews.apache.org/r/26478/diff/2/?file=716376#file716376line86
 
  Method interceptors should work fine for package-priviate methods [1].  
  Just for my understanding - are you going with protected because 
  package-private class + protected method is slightly more restrictive?
  
  [1] https://github.com/google/guice/wiki/AOP#limitations

I'm going with protected because it's more indicative of why it's visible at 
all (I want guice's dynamic proxy subclass to override it and add timing 
information). It's also more restrictive than package-private in this context.


 On Oct. 9, 2014, 8:30 a.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java,
   line 61
  https://reviews.apache.org/r/26478/diff/2/?file=716380#file716380line61
 
  Tasks.SCHEDULED_TO_INFO

This operates on the mutable structures, SCHEDULED_TO_INFO operates on the 
immutable ones.


 On Oct. 9, 2014, 8:30 a.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java,
   line 71
  https://reviews.apache.org/r/26478/diff/2/?file=716380#file716380line71
 
  Do you have numbers on how much time this routine saves when compared 
  to a full deep copy and unsetting the field you're trying to clear?  Unless 
  it's a significant contributor to overall snapshot performance, i'd prefer 
  the more concise code of the latter approach.
  
  My hunch is that this one might save O(100 ms), but the ones below are 
  noise and not worth the code.

I don't have data for this specific optimization - my gut is that we should 
avoid deepCopy on Snapshots due to them respresenting essentially the entire 
scheduler heap. Happy to remove if you think it's not warranted.


 On Oct. 9, 2014, 8:30 a.m., Bill Farner wrote:
  src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicatorImplTest.java,
   line 84
  https://reviews.apache.org/r/26478/diff/2/?file=716386#file716386line84
 
  The assertion error will be much more useful if you do 
  `assertEquals(emptySet, b)` rather than `assertEquals(n, b.size())`.

It's actually `null` here, but `.size()` in thrift reports `0` for `null`. 
Changed to explicit `assertNull`.


 On Oct. 9, 2014, 8:30 a.m., Bill Farner wrote:
  src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicatorImplTest.java,
   line 89
  https://reviews.apache.org/r/26478/diff/2/?file=716386#file716386line89
 
  Isn't this check redundant to the one below?

not quite, since the latter is making a Set out of a List, and therefore 
potentially doing deduplication for me.


 On Oct. 9, 2014, 8:30 a.m., Bill Farner wrote:
  src/test/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicatorImplTest.java,
   line 98
  https://reviews.apache.org/r/26478/diff/2/?file=716386#file716386line98
 
  Can you try for `assertEquals` here as well, with an expected list of 
  `DeduplicatedScheduledTask` objects?  It will catch classes of bugs missed 
  with this check (e.g. extra entries in the `partialTasks` list), and make 
  it easier to diagnose a failed test.

Since the input is a Snapshot (SetScheduledTask) it's not possible to know 
the expected order of the output list ahead-of-time. Added an explicit check 
against extra entries.


 On Oct. 9, 2014, 8:30 a.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java,
   line 80
  https://reviews.apache.org/r/26478/diff/2/?file=716379#file716379line80
 
  First sentence contains mixed thoughts for an odd sentence, rewrite?

Rewrote.


 On Oct. 9, 2014, 8:30 a.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/storage/log/LogStorageModule.java,
   line 81
  https://reviews.apache.org/r/26478/diff/2/?file=716379#file716379line81
 
  Please make this more verbose to explain what exactly is backwards in 
  compatible.  Don't worry about being too wordy.
  
  I'd also like to see a TODO to remove this flag, since it should become 
  the default.  Perhaps this should be included in the 0.7.0 deprecations 
  list.

Moved to separate doc.


 On Oct. 9, 2014, 8:30 a.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java,
   line 110
  https://reviews.apache.org/r/26478/diff/2/?file=716380#file716380line110
 
  Inline this on :114.

done


 On Oct. 9, 2014, 8:30 a.m., Bill Farner wrote:
  src/main/java/org/apache/aurora/scheduler/storage/log/SnapshotDeduplicator.java,
   line 116
  https://reviews.apache.org/r/26478/diff/2/?file=716380#file716380line116
 
  It would be nice to see a brief comment here to give the reader an 
  overview of what's going on in the routine.

Added.


 On Oct. 9, 2014, 8:30 a.m., Bill Farner wrote:
  

Re: Review Request 26424: Disable requests http connection logging.

2014-10-14 Thread Joshua Cohen


 On Oct. 15, 2014, 12:02 a.m., Bill Farner wrote:
  Sorry for the delay on this, i didn't realize i was on the hook to commit.
  
  Now on master:
  $ git log -1
  commit 72fed752fcdbacf0f29e7eaef56c177f5ec9fcf0
  Author: Joshua Cohen jco...@twopensource.com
  Date:   Tue Oct 14 16:32:03 2014 -0700
  
  Disable requests http connection logging.
  
  Bugs closed: AURORA-770
  
  Reviewed at https://reviews.apache.org/r/26424/

D'oh, I had lost track of this, I thought it had already shipped :/.


- Joshua


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


On Oct. 8, 2014, 4:54 p.m., Joshua Cohen wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviews.apache.org/r/26424/
 ---
 
 (Updated Oct. 8, 2014, 4:54 p.m.)
 
 
 Review request for Aurora, Joe Smith and Bill Farner.
 
 
 Bugs: AURORA-770
 https://issues.apache.org/jira/browse/AURORA-770
 
 
 Repository: aurora
 
 
 Description
 ---
 
 .
 
 
 Diffs
 -
 
   src/main/python/apache/aurora/common/transport.py 
 6f7c355d725b5e537cc4ae471170eaa8431da326 
   src/test/python/apache/aurora/common/test_transport.py 
 c722eae2d04dec90e9c772f49c578184a2bdf76c 
 
 Diff: https://reviews.apache.org/r/26424/diff/
 
 
 Testing
 ---
 
 Added hokey test, ran hokey test.
 
 Also verified the lack of http connection logs when running client commands 
 directly.
 
 
 Thanks,
 
 Joshua Cohen