date:20150402

Re: Review Request 32597: Improving async preemptor efficiency.

2015-04-02 Thread Bill Farner


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



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java


`{@link TaskGroupKey}`



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java


Javadoc does not preserve this formatting.  You'll want to wrap in a 
`{@code xxx}`

or ``.



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java


What's the motivation behind the ordering?  It also seems somewhat 
contradictory along with the class doc "fair evaluation order of all task 
groups regardless of their task count".



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java


s/java.lang//, here and elsewhere



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java


I can't think of a downside if the outer loop were over pending tasks 
rather than slaves.  The upside would be that we could avoid the 
`AttributeAggregate` cachine added in this diff.

Is there something i'm not thinking about?

*note* If you act on this comment, the `Iterable` comment is no longer 
relevant.



src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java


Could this be simplified by making `PeekingTaskIterator` an `Iterable`, and 
have each `Iterator` pass through the groups exactly once?  i.e. perhaps a 
circular buffer isn't the right model here, since you're having to work around 
it.

This would also allow the code here to be ignorant of `TaskGroupKey`, since 
you could support `Iterator#remove()`.


- Bill Farner


On April 2, 2015, 11:39 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> ---
> 
> (Updated April 2, 2015, 11:39 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
> https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is addressing the preemption efficiency loss due to making preemptor 
> async. Summary:
> - Implemented fair task processing by evaluating pending tasks in the order 
> of their TaskGroupKeys (largest count first)
> - Inverted the traversal of pending tasks to better reuse loop invariants. 
> Every slave is sized up against all pending tasks until a match is found
> - Moved relevant functionality from PreemptionSlotFinder (now 
> PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and 
> PendingTaskProcessor. The rest is refactoring to adjust to the new traversal 
> approach.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
>  67ad5d7f9909bc892301c19586561b6cdbfe79e6 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java
>  b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java
>  427c0de205c28540b217e817bdbab10b4af5aee4 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 
> 77617ec9a2bcd062d5b80cd2b4c58dc80514 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java
>  1092c055588363794b37a965fb2f17a6e50d92d7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 
> 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java
>  bcd1b4e854f5ea227268c73156bc97c7c937c1de 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java
>  80bd13a192bda64759b5a93749ec47ddfeae504a 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
>  281f4e02650727aa5d0a35a09dcf0eb823ad1b50 
>   
> src/test/java/org/apache/aurora

Re: Review Request 32806: Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.

2015-04-02 Thread Aurora ReviewBot


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


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

:check
:build
:api:assemble
:api:compileTestJava UP-TO-DATE
:api:processTestResources UP-TO-DATE
:api:testClasses UP-TO-DATE
:api:test UP-TO-DATE
:api:check UP-TO-DATE
:api:build
:buildSrc:compileJava UP-TO-DATE
:buildSrc:processResources UP-TO-DATE
:buildSrc:classes UP-TO-DATE
:buildSrc:jar
:buildSrc:assemble
:buildSrc:compileTestJava UP-TO-DATE
:buildSrc:processTestResources UP-TO-DATE
:buildSrc:testClasses UP-TO-DATE
:buildSrc:test UP-TO-DATE
:buildSrc:check UP-TO-DATE
:buildSrc:build

BUILD SUCCESSFUL

Total time: 4 mins 3.517 secs
+ export PIP_DEFAULT_TIMEOUT=60
+ PIP_DEFAULT_TIMEOUT=60
+ mkdir -p third_party
+ pip install -d third_party -r /dev/fd/63
++ grep -v mesos.native 3rdparty/python/requirements.txt
Downloading/unpacking bottle==0.11.6 (from -r /dev/fd/63 (line 1))
  Saved ./third_party/bottle-0.11.6-py2.py3-none-any.whl
Downloading/unpacking CherryPy==3.6.0 (from -r /dev/fd/63 (line 2))

pip can't proceed with requirement 'CherryPy==3.6.0 (from -r /dev/fd/63 (line 
2))' due to a pre-existing build directory.
 location: /tmp/user/2395/pip_build_jenkins/CherryPy
This is likely due to a previous installation that failed.
pip is being responsible and not assuming it can delete this.
Please delete it and try again.

Storing debug log for failure in /home/jenkins/.pip/pip.log


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

- Aurora ReviewBot


On April 3, 2015, 3:06 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32806/
> ---
> 
> (Updated April 3, 2015, 3:06 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Kevin Sweeney.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 5f6a019e4d6401e1efd075b72c049fa245cc0d0a 
>   
> src/main/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverrides.java
>  6d92ae3c8ec46e7964e81e507a2f2a7f2db68cfd 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 2af009d3d9ec44a70659225d0c18de9fda3a6f7a 
>   src/main/java/org/apache/aurora/scheduler/http/HttpService.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 50f377587ac05dbb72063ea02502e6d980148d3e 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
> e03009c12de5a09761c1f444c6439ef3144b0b17 
>   
> src/test/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverridesTest.java
>  21fd027976f75acc427c6d9265a7c7a91895d53d 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> c3e40d88fe7ee1a447d1d61980b69bd1b46881e7 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java 
> 7f80757cb40af7dde042f1d39355eadf2b3b1aee 
>   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
> c5c5f789de6bf7693520081d0c1acc5165603242 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
>  76cb691e6d7d4fada3a18fde73aceed7039bcaa4 
> 
> Diff: https://reviews.apache.org/r/32806/diff/
> 
> 
> Testing
> ---
> 
> Test suite + end-to-end tests.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 32806: Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.

2015-04-02 Thread Bill Farner


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

(Updated April 3, 2015, 3:06 a.m.)


Review request for Aurora, Joshua Cohen and Kevin Sweeney.


Repository: aurora


Description
---

Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
5f6a019e4d6401e1efd075b72c049fa245cc0d0a 
  
src/main/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverrides.java
 6d92ae3c8ec46e7964e81e507a2f2a7f2db68cfd 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
2af009d3d9ec44a70659225d0c18de9fda3a6f7a 
  src/main/java/org/apache/aurora/scheduler/http/HttpService.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
50f377587ac05dbb72063ea02502e6d980148d3e 
  src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
e03009c12de5a09761c1f444c6439ef3144b0b17 
  
src/test/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverridesTest.java
 21fd027976f75acc427c6d9265a7c7a91895d53d 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
  src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
c3e40d88fe7ee1a447d1d61980b69bd1b46881e7 
  src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java 
7f80757cb40af7dde042f1d39355eadf2b3b1aee 
  src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
c5c5f789de6bf7693520081d0c1acc5165603242 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
76cb691e6d7d4fada3a18fde73aceed7039bcaa4 

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


Testing
---

Test suite + end-to-end tests.


Thanks,

Bill Farner

Re: Review Request 32806: Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.

2015-04-02 Thread Aurora ReviewBot


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


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

:api:classesThriftNote: Some input files use unchecked or unsafe operations.
Note: Recompile with -Xlint:unchecked for details.

:api:checkPython
:api:generateThriftEntitiesJava
:api:classesThriftEntities
:api:compileJava UP-TO-DATE
:api:generateThriftResources
:api:processResources UP-TO-DATE
:api:classes
:api:jar
:compileJavaNote: Writing 
file:/home/jenkins/jenkins-slave/workspace/AuroraBot/dist/classes/main/com/twitter/common/args/apt/cmdline.arg.info.txt.2

:processResources
:classes
:jar
:startScripts
:distTar
:distZip
:assemble
:compileJmhJava
:processJmhResources UP-TO-DATE
:jmhClasses
:checkstyleJmh
:jsHint
:checkstyleMain[ant:checkstyle] 
/home/jenkins/jenkins-slave/workspace/AuroraBot/src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java:327:
 'if' child have incorrect indentation level 9, expected level should be 8.
 FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':checkstyleMain'.
> Checkstyle rule violations were found. See the report at: 
> file:///home/jenkins/jenkins-slave/workspace/AuroraBot/dist/reports/checkstyle/main.xml

* Try:
Run with --stacktrace option to get the stack trace. Run with --info or --debug 
option to get more log output.

BUILD FAILED

Total time: 1 mins 34.926 secs


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

- Aurora ReviewBot


On April 3, 2015, 2:49 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32806/
> ---
> 
> (Updated April 3, 2015, 2:49 a.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Kevin Sweeney.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
> 5f6a019e4d6401e1efd075b72c049fa245cc0d0a 
>   
> src/main/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverrides.java
>  6d92ae3c8ec46e7964e81e507a2f2a7f2db68cfd 
>   src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
> 2af009d3d9ec44a70659225d0c18de9fda3a6f7a 
>   src/main/java/org/apache/aurora/scheduler/http/HttpService.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
> 50f377587ac05dbb72063ea02502e6d980148d3e 
>   src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
> e03009c12de5a09761c1f444c6439ef3144b0b17 
>   
> src/test/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverridesTest.java
>  21fd027976f75acc427c6d9265a7c7a91895d53d 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
>   src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
> c3e40d88fe7ee1a447d1d61980b69bd1b46881e7 
>   src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java 
> 7f80757cb40af7dde042f1d39355eadf2b3b1aee 
>   src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
> c5c5f789de6bf7693520081d0c1acc5165603242 
>   
> src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java
>  76cb691e6d7d4fada3a18fde73aceed7039bcaa4 
> 
> Diff: https://reviews.apache.org/r/32806/diff/
> 
> 
> Testing
> ---
> 
> Test suite + end-to-end tests.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 32806: Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.

2015-04-02 Thread Bill Farner


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

Review request for Aurora, Joshua Cohen and Kevin Sweeney.


Repository: aurora


Description
---

Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/app/AppModule.java 
5f6a019e4d6401e1efd075b72c049fa245cc0d0a 
  
src/main/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverrides.java
 6d92ae3c8ec46e7964e81e507a2f2a7f2db68cfd 
  src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java 
2af009d3d9ec44a70659225d0c18de9fda3a6f7a 
  src/main/java/org/apache/aurora/scheduler/http/HttpService.java PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/http/JettyServerModule.java 
50f377587ac05dbb72063ea02502e6d980148d3e 
  src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java 
e03009c12de5a09761c1f444c6439ef3144b0b17 
  
src/test/java/org/apache/aurora/scheduler/app/LocalServiceRegistryWithOverridesTest.java
 21fd027976f75acc427c6d9265a7c7a91895d53d 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
  src/test/java/org/apache/aurora/scheduler/http/JettyServerModuleTest.java 
c3e40d88fe7ee1a447d1d61980b69bd1b46881e7 
  src/test/java/org/apache/aurora/scheduler/http/LeaderRedirectTest.java 
7f80757cb40af7dde042f1d39355eadf2b3b1aee 
  src/test/java/org/apache/aurora/scheduler/http/ServletFilterTest.java 
c5c5f789de6bf7693520081d0c1acc5165603242 
  
src/test/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityIT.java 
76cb691e6d7d4fada3a18fde73aceed7039bcaa4 

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


Testing
---

Test suite + end-to-end tests.


Thanks,

Bill Farner

Re: Review Request 32802: Add a benchmark for getRoleSummary.

2015-04-02 Thread Bill Farner


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

(Updated April 3, 2015, 2:19 a.m.)


Review request for Aurora and Maxim Khutornenko.


Repository: aurora


Description
---

The first of several benchmarks to exercise the thrift API.  The real 
motivation here is to have performance data to assess the impact of migrating 
the task store to H2.  I'm targeting the `getRoleSummary` RPC specifically here 
as i consider it the likely candidate to suffer.

One 'trick' i employed here was JSON for input parameters.  I took this 
approach since i really wanted to explore specific axes of growth without the 
full outer product of each individual parameters.

Some changes along the way:
- upgraded to jmh 1.7.1
- moved some JMH run configuration parameters out of the gradle config and into 
benchmark classes.  This allows us to define run configurations for each 
microbenchmark
- changed the benchmark mode for existing and new tests to throughput.  We had 
several benchmarks whose iteration time was measuring in hundreds of 
microseconds, which is too high resolution to capture performance variation.  
By running iterations for a set amount of time and counting iterations 
accomplished, we can capture more reliable data.


Diffs
-

  build.gradle 66dbdfde830c81c9b2291d2b8391f1ccac94b485 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
5309e8140fff411da30ee87c1b3b1a55d6fdaeeb 
  src/jmh/java/org/apache/aurora/benchmark/Tasks.java 
491c6871ba7a4c76a4eb084afb2627a3f93db8df 
  src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
c65e0218211b797ad50e2ac62e71136f967e5f5d 

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


Testing (updated)
---

FYI numbers and stddev are not useful in the data captured below, as my machine 
was doing other intensive things while running this.  Output is more useful as 
an example of how the data and run configurations are presented.

```
# Run complete. Total time: 00:18:51

Benchmark  
(testConfiguration)   Mode  Cnt   Score   Error  Units
SchedulingBenchmarks.ClusterFullUtilizationBenchmark.runBenchmark   
   N/A  thrpt   10  499824.589 ± 86328.152  ops/s
SchedulingBenchmarks.InsufficientResourcesSchedulingBenchmark.runBenchmark  
   N/A  thrpt   10   40482.762 ±  5394.306  ops/s
SchedulingBenchmarks.LimitConstraintMismatchSchedulingBenchmark.runBenchmark
   N/A  thrpt   102933.958 ±   577.571  ops/s
SchedulingBenchmarks.PreemptorSlotSearchBenchmark.runBenchmark  
   N/A  thrpt   10 220.079 ±28.943  ops/s
SchedulingBenchmarks.ValueConstraintMismatchSchedulingBenchmark.runBenchmark
   N/A  thrpt   10   41948.230 ±  1906.106  ops/s
ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 
  {"roles": 1}  thrpt51978.614 ±   736.704  ops/s
ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 
 {"roles": 10}  thrpt5 143.465 ±85.579  ops/s
ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 
{"roles": 100}  thrpt5   4.003 ± 2.742  ops/s
ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 
{"roles": 500}  thrpt5   1.964 ± 0.569  ops/s
ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 
   {"jobs": 1}  thrpt54073.564 ±  3343.602  ops/s
ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 
  {"jobs": 10}  thrpt54401.662 ±  3943.372  ops/s
ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 
 {"jobs": 100}  thrpt54327.726 ±   726.900  ops/s
ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 
 {"jobs": 500}  thrpt52473.799 ±  5396.188  ops/s
ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 
  {"instances": 1}  thrpt57351.543 ± 13368.279  ops/s
ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 
 {"instances": 10}  thrpt59496.478 ±  3066.478  ops/s
ThriftApiBenchmarks.GetRoleSummaryBenchmark.run 
{"instances": 100}  thrpt54640.480 ±  3307.831  ops/s
ThriftApiBenchmarks.GetRoleSummaryBenchmark.run
{"instances": 1000}  thrpt5 429.841 ±   191.521  ops/s
ThriftApiBenchmarks.GetRoleSummaryBenchmark.run   
{"instances": 1}  thrpt5  23.327 ± 7.011  ops/s
```


Thanks,

Bill Farner

Re: Review Request 32802: Add a benchmark for getRoleSummary.

2015-04-02 Thread Aurora ReviewBot


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


This patch does not apply cleanly on master (a7b95d9), do you need to rebase?

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

- Aurora ReviewBot


On April 3, 2015, 1:43 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32802/
> ---
> 
> (Updated April 3, 2015, 1:43 a.m.)
> 
> 
> Review request for Aurora and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The first of several benchmarks to exercise the thrift API.  The real 
> motivation here is to have performance data to assess the impact of migrating 
> the task store to H2.  I'm targeting the `getRoleSummary` RPC specifically 
> here as i consider it the likely candidate to suffer.
> 
> One 'trick' i employed here was JSON for input parameters.  I took this 
> approach since i really wanted to explore specific axes of growth without the 
> full outer product of each individual parameters.
> 
> Some changes along the way:
> - upgraded to jmh 1.7.1
> - moved some JMH run configuration parameters out of the gradle config and 
> into benchmark classes.  This allows us to define run configurations for each 
> microbenchmark
> - changed the benchmark mode for existing and new tests to throughput.  We 
> had several benchmarks whose iteration time was measuring in hundreds of 
> microseconds, which is too high resolution to capture performance variation.  
> By running iterations for a set amount of time and counting iterations 
> accomplished, we can capture more reliable data.
> 
> 
> Diffs
> -
> 
>   build.gradle 66dbdfde830c81c9b2291d2b8391f1ccac94b485 
>   src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
> 5309e8140fff411da30ee87c1b3b1a55d6fdaeeb 
>   src/jmh/java/org/apache/aurora/benchmark/Tasks.java 
> 491c6871ba7a4c76a4eb084afb2627a3f93db8df 
>   src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
> c65e0218211b797ad50e2ac62e71136f967e5f5d 
> 
> Diff: https://reviews.apache.org/r/32802/diff/
> 
> 
> Testing
> ---
> 
> Will post jmh output shortly.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Review Request 32802: Add a benchmark for getRoleSummary.

2015-04-02 Thread Bill Farner


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

Review request for Aurora and Maxim Khutornenko.


Repository: aurora


Description
---

The first of several benchmarks to exercise the thrift API.  The real 
motivation here is to have performance data to assess the impact of migrating 
the task store to H2.  I'm targeting the `getRoleSummary` RPC specifically here 
as i consider it the likely candidate to suffer.

One 'trick' i employed here was JSON for input parameters.  I took this 
approach since i really wanted to explore specific axes of growth without the 
full outer product of each individual parameters.

Some changes along the way:
- upgraded to jmh 1.7.1
- moved some JMH run configuration parameters out of the gradle config and into 
benchmark classes.  This allows us to define run configurations for each 
microbenchmark
- changed the benchmark mode for existing and new tests to throughput.  We had 
several benchmarks whose iteration time was measuring in hundreds of 
microseconds, which is too high resolution to capture performance variation.  
By running iterations for a set amount of time and counting iterations 
accomplished, we can capture more reliable data.


Diffs
-

  build.gradle 66dbdfde830c81c9b2291d2b8391f1ccac94b485 
  src/jmh/java/org/apache/aurora/benchmark/SchedulingBenchmarks.java 
5309e8140fff411da30ee87c1b3b1a55d6fdaeeb 
  src/jmh/java/org/apache/aurora/benchmark/Tasks.java 
491c6871ba7a4c76a4eb084afb2627a3f93db8df 
  src/jmh/java/org/apache/aurora/benchmark/ThriftApiBenchmarks.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/thrift/ThriftModule.java 
c65e0218211b797ad50e2ac62e71136f967e5f5d 

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


Testing
---

Will post jmh output shortly.


Thanks,

Bill Farner

Re: Review Request 32597: Improving async preemptor efficiency.

2015-04-02 Thread Aurora ReviewBot


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


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

:check
:build
:api:assemble
:api:compileTestJava UP-TO-DATE
:api:processTestResources UP-TO-DATE
:api:testClasses UP-TO-DATE
:api:test UP-TO-DATE
:api:check UP-TO-DATE
:api:build
:buildSrc:compileJava UP-TO-DATE
:buildSrc:processResources UP-TO-DATE
:buildSrc:classes UP-TO-DATE
:buildSrc:jar
:buildSrc:assemble
:buildSrc:compileTestJava UP-TO-DATE
:buildSrc:processTestResources UP-TO-DATE
:buildSrc:testClasses UP-TO-DATE
:buildSrc:test UP-TO-DATE
:buildSrc:check UP-TO-DATE
:buildSrc:build

BUILD SUCCESSFUL

Total time: 4 mins 1.746 secs
+ export PIP_DEFAULT_TIMEOUT=60
+ PIP_DEFAULT_TIMEOUT=60
+ mkdir -p third_party
+ pip install -d third_party -r /dev/fd/63
++ grep -v mesos.native 3rdparty/python/requirements.txt
Downloading/unpacking bottle==0.11.6 (from -r /dev/fd/63 (line 1))
  Saved ./third_party/bottle-0.11.6-py2.py3-none-any.whl
Downloading/unpacking CherryPy==3.6.0 (from -r /dev/fd/63 (line 2))

pip can't proceed with requirement 'CherryPy==3.6.0 (from -r /dev/fd/63 (line 
2))' due to a pre-existing build directory.
 location: /tmp/user/2395/pip_build_jenkins/CherryPy
This is likely due to a previous installation that failed.
pip is being responsible and not assuming it can delete this.
Please delete it and try again.

Storing debug log for failure in /home/jenkins/.pip/pip.log


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

- Aurora ReviewBot


On April 2, 2015, 11:39 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32597/
> ---
> 
> (Updated April 2, 2015, 11:39 p.m.)
> 
> 
> Review request for Aurora, Bill Farner and Zameer Manji.
> 
> 
> Bugs: AURORA-1219
> https://issues.apache.org/jira/browse/AURORA-1219
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> This is addressing the preemption efficiency loss due to making preemptor 
> async. Summary:
> - Implemented fair task processing by evaluating pending tasks in the order 
> of their TaskGroupKeys (largest count first)
> - Inverted the traversal of pending tasks to better reuse loop invariants. 
> Every slave is sized up against all pending tasks until a match is found
> - Moved relevant functionality from PreemptionSlotFinder (now 
> PreemptionVictimFilter) into PendingTaskProcessor.
> 
> The bulk of new functionality is in PendingTaskIterator and 
> PendingTaskProcessor. The rest is refactoring to adjust to the new traversal 
> approach.
> 
> 
> Diffs
> -
> 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
>  67ad5d7f9909bc892301c19586561b6cdbfe79e6 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java 
> PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java
>  b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java
>  427c0de205c28540b217e817bdbab10b4af5aee4 
>   src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 
> 77617ec9a2bcd062d5b80cd2b4c58dc80514 
>   
> src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java
>  1092c055588363794b37a965fb2f17a6e50d92d7 
>   src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 
> 6af3949b85297043640edccc1a490906c0fcff6c 
>   src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
> d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java
>  bcd1b4e854f5ea227268c73156bc97c7c937c1de 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java
>  80bd13a192bda64759b5a93749ec47ddfeae504a 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
>  281f4e02650727aa5d0a35a09dcf0eb823ad1b50 
>   
> src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java
>  b80e558f18b817814e4768b13ff94aa816d28543 
> 
> Diff: https://reviews.apache.org/r/32597/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> Manual testing in vagrant
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

2015-04-02 Thread Maxim Khutornenko


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

(Updated April 2, 2015, 11:39 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


Changes
---

Rebased.


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


Repository: aurora


Description
---

This is addressing the preemption efficiency loss due to making preemptor 
async. Summary:
- Implemented fair task processing by evaluating pending tasks in the order of 
their TaskGroupKeys (largest count first)
- Inverted the traversal of pending tasks to better reuse loop invariants. 
Every slave is sized up against all pending tasks until a match is found
- Moved relevant functionality from PreemptionSlotFinder (now 
PreemptionVictimFilter) into PendingTaskProcessor.

The bulk of new functionality is in PendingTaskIterator and 
PendingTaskProcessor. The rest is refactoring to adjust to the new traversal 
approach.

Will not apply cleanly. Diffed against https://reviews.apache.org/r/32352/


Diffs (updated)
-

  
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
 67ad5d7f9909bc892301c19586561b6cdbfe79e6 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java
 b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 
  
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java
 427c0de205c28540b217e817bdbab10b4af5aee4 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 
77617ec9a2bcd062d5b80cd2b4c58dc80514 
  
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 
1092c055588363794b37a965fb2f17a6e50d92d7 
  src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 
6af3949b85297043640edccc1a490906c0fcff6c 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
  
src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java
 bcd1b4e854f5ea227268c73156bc97c7c937c1de 
  
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java
 80bd13a192bda64759b5a93749ec47ddfeae504a 
  
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 
  
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java
 b80e558f18b817814e4768b13ff94aa816d28543 

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


Testing
---

./gradlew -Pq build
Manual testing in vagrant


Thanks,

Maxim Khutornenko

Re: Review Request 32597: Improving async preemptor efficiency.

2015-04-02 Thread Maxim Khutornenko


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

(Updated April 2, 2015, 11:39 p.m.)


Review request for Aurora, Bill Farner and Zameer Manji.


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


Repository: aurora


Description (updated)
---

This is addressing the preemption efficiency loss due to making preemptor 
async. Summary:
- Implemented fair task processing by evaluating pending tasks in the order of 
their TaskGroupKeys (largest count first)
- Inverted the traversal of pending tasks to better reuse loop invariants. 
Every slave is sized up against all pending tasks until a match is found
- Moved relevant functionality from PreemptionSlotFinder (now 
PreemptionVictimFilter) into PendingTaskProcessor.

The bulk of new functionality is in PendingTaskIterator and 
PendingTaskProcessor. The rest is refactoring to adjust to the new traversal 
approach.


Diffs
-

  
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIterator.java
 PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessor.java
 67ad5d7f9909bc892301c19586561b6cdbfe79e6 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlot.java 
PRE-CREATION 
  
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCache.java
 b5a42a0a0498f06a382576e2c3a839bbb7f1d2b1 
  
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotFinder.java
 427c0de205c28540b217e817bdbab10b4af5aee4 
  src/main/java/org/apache/aurora/scheduler/async/preemptor/Preemptor.java 
77617ec9a2bcd062d5b80cd2b4c58dc80514 
  
src/main/java/org/apache/aurora/scheduler/async/preemptor/PreemptorModule.java 
1092c055588363794b37a965fb2f17a6e50d92d7 
  src/main/java/org/apache/aurora/scheduler/base/TaskGroupKey.java 
6af3949b85297043640edccc1a490906c0fcff6c 
  src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java 
d212bbdf3c37be8e1e00eb169cf40b90fe69ed5f 
  
src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskIteratorTest.java
 PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/async/preemptor/PendingTaskProcessorTest.java
 bcd1b4e854f5ea227268c73156bc97c7c937c1de 
  
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptionSlotCacheTest.java
 80bd13a192bda64759b5a93749ec47ddfeae504a 
  
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
 281f4e02650727aa5d0a35a09dcf0eb823ad1b50 
  
src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorSlotFinderTest.java
 b80e558f18b817814e4768b13ff94aa816d28543 

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


Testing
---

./gradlew -Pq build
Manual testing in vagrant


Thanks,

Maxim Khutornenko

Re: Review Request 32541: Adding client Kerberos support.

2015-04-02 Thread Aurora ReviewBot


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


Master (4b9c759) is red with this patch.
  ./build-support/jenkins/build.sh

:check
:build
:api:assemble
:api:compileTestJava UP-TO-DATE
:api:processTestResources UP-TO-DATE
:api:testClasses UP-TO-DATE
:api:test UP-TO-DATE
:api:check UP-TO-DATE
:api:build
:buildSrc:compileJava UP-TO-DATE
:buildSrc:processResources UP-TO-DATE
:buildSrc:classes UP-TO-DATE
:buildSrc:jar
:buildSrc:assemble
:buildSrc:compileTestJava UP-TO-DATE
:buildSrc:processTestResources UP-TO-DATE
:buildSrc:testClasses UP-TO-DATE
:buildSrc:test UP-TO-DATE
:buildSrc:check UP-TO-DATE
:buildSrc:build

BUILD SUCCESSFUL

Total time: 4 mins 0.994 secs
+ export PIP_DEFAULT_TIMEOUT=60
+ PIP_DEFAULT_TIMEOUT=60
+ mkdir -p third_party
+ pip install -d third_party -r /dev/fd/63
++ grep -v mesos.native 3rdparty/python/requirements.txt
Downloading/unpacking bottle==0.11.6 (from -r /dev/fd/63 (line 1))
  Saved ./third_party/bottle-0.11.6-py2.py3-none-any.whl
Downloading/unpacking CherryPy==3.6.0 (from -r /dev/fd/63 (line 2))

pip can't proceed with requirement 'CherryPy==3.6.0 (from -r /dev/fd/63 (line 
2))' due to a pre-existing build directory.
 location: /tmp/user/2395/pip_build_jenkins/CherryPy
This is likely due to a previous installation that failed.
pip is being responsible and not assuming it can delete this.
Please delete it and try again.

Storing debug log for failure in /home/jenkins/.pip/pip.log


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

- Aurora ReviewBot


On April 2, 2015, 5:33 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32541/
> ---
> 
> (Updated April 2, 2015, 5:33 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-813
> https://issues.apache.org/jira/browse/AURORA-813
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> First take on client kerberos support. The idea is to repurpose the existing 
> auth_module system to support both legacy and kerberos during the deprecation 
> period. This way the 0.8.0 client will be able to talk to pre-0.8.0 scheduler 
> and use SessionKey-based authorization. Later (in 0.9.0), the payload() will 
> be removed along with SessionKey (AURORA-1229). That will let us get rid of 
> SchedulerProxy (or reduce it substantially). The auth_module might stay 
> though to support other auth plugins (e.g. requests-ntlm or 
> requests-oauthlib).
> 
> TODO: integration e2e tests once scheduler side lands.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 11a307cdb476ebcc25ab5c6b555bed29241ea988 
>   src/main/python/apache/aurora/client/api/__init__.py 
> a81329f6f947bbea4001c3a521c1923410a51eab 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 95e553427492407743dcac31d70f392a7c1bbc02 
>   src/main/python/apache/aurora/client/cli/BUILD 
> c6b4e8a09d1315cf5defee2155a6e0c697892a30 
>   src/main/python/apache/aurora/client/cli/client.py 
> 24516d114db1743cdf600c542a27fcf5b68053a0 
>   src/main/python/apache/aurora/common/auth/BUILD 
> 966484627dab90e7606f1fc638cd0e159aee3317 
>   src/main/python/apache/aurora/common/auth/__init__.py 
> 3119fd63d3dfa28f93f219b23030059580fed098 
>   src/main/python/apache/aurora/common/auth/auth_module.py 
> 5f4116ef4cfbc407e0c50dc938870fb14e2299b4 
>   src/main/python/apache/aurora/common/auth/auth_module_manager.py 
> 73a8e5cd51edf694b971cd2c298ff406aff8c6d7 
>   src/main/python/apache/aurora/common/auth/kerberos.py PRE-CREATION 
>   src/main/python/apache/aurora/common/transport.py 
> 395f8a94d9a27aad00166a17f2528a8c0833ffdd 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 0a6194831c332a96eab62b869c4e05cfa9def058 
>   src/test/python/apache/aurora/common/test_transport.py 
> b78e0b3badfbbeecefff7b5954f3796cef4da9d8 
> 
> Diff: https://reviews.apache.org/r/32541/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python:all
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32541: Adding client Kerberos support.

2015-04-02 Thread Maxim Khutornenko


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

(Updated April 2, 2015, 5:33 p.m.)


Review request for Aurora, Kevin Sweeney and Brian Wickman.


Changes
---

Modified BUILD target to exclude kerberos module from a non-secure client.


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


Repository: aurora


Description
---

First take on client kerberos support. The idea is to repurpose the existing 
auth_module system to support both legacy and kerberos during the deprecation 
period. This way the 0.8.0 client will be able to talk to pre-0.8.0 scheduler 
and use SessionKey-based authorization. Later (in 0.9.0), the payload() will be 
removed along with SessionKey (AURORA-1229). That will let us get rid of 
SchedulerProxy (or reduce it substantially). The auth_module might stay though 
to support other auth plugins (e.g. requests-ntlm or requests-oauthlib).

TODO: integration e2e tests once scheduler side lands.


Diffs (updated)
-

  3rdparty/python/requirements.txt 11a307cdb476ebcc25ab5c6b555bed29241ea988 
  src/main/python/apache/aurora/client/api/__init__.py 
a81329f6f947bbea4001c3a521c1923410a51eab 
  src/main/python/apache/aurora/client/api/scheduler_client.py 
95e553427492407743dcac31d70f392a7c1bbc02 
  src/main/python/apache/aurora/client/cli/BUILD 
c6b4e8a09d1315cf5defee2155a6e0c697892a30 
  src/main/python/apache/aurora/client/cli/client.py 
24516d114db1743cdf600c542a27fcf5b68053a0 
  src/main/python/apache/aurora/common/auth/BUILD 
966484627dab90e7606f1fc638cd0e159aee3317 
  src/main/python/apache/aurora/common/auth/__init__.py 
3119fd63d3dfa28f93f219b23030059580fed098 
  src/main/python/apache/aurora/common/auth/auth_module.py 
5f4116ef4cfbc407e0c50dc938870fb14e2299b4 
  src/main/python/apache/aurora/common/auth/auth_module_manager.py 
73a8e5cd51edf694b971cd2c298ff406aff8c6d7 
  src/main/python/apache/aurora/common/auth/kerberos.py PRE-CREATION 
  src/main/python/apache/aurora/common/transport.py 
395f8a94d9a27aad00166a17f2528a8c0833ffdd 
  src/test/python/apache/aurora/client/api/test_scheduler_client.py 
0a6194831c332a96eab62b869c4e05cfa9def058 
  src/test/python/apache/aurora/common/test_transport.py 
b78e0b3badfbbeecefff7b5954f3796cef4da9d8 

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


Testing
---

./pants test.pytest --no-fast src/test/python:all
./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh


Thanks,

Maxim Khutornenko

Re: Review Request 32541: Adding client Kerberos support.

2015-04-02 Thread Maxim Khutornenko



> On April 1, 2015, 8:40 p.m., Kevin Sweeney wrote:
> > src/main/python/apache/aurora/client/cli/BUILD, line 29
> > 
> >
> > How about "kaurora"
> 
> Maxim Khutornenko wrote:
> This is a joke, right? :) If not I am really struggling how it would be 
> more informative.
> 
> Kevin Sweeney wrote:
> Some Kerberized tools have a convention of being prefixed with a "k" (su 
> => ksu, rsh => krsh, passwd => kpasswd). I think the ergonomics are better 
> (no underscores in command names), but don't feel strongly about it.

Since this isn't a command line tool, I'd rather keep the current name as more 
descriptive.


- Maxim


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


On April 2, 2015, 1:10 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32541/
> ---
> 
> (Updated April 2, 2015, 1:10 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-813
> https://issues.apache.org/jira/browse/AURORA-813
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> First take on client kerberos support. The idea is to repurpose the existing 
> auth_module system to support both legacy and kerberos during the deprecation 
> period. This way the 0.8.0 client will be able to talk to pre-0.8.0 scheduler 
> and use SessionKey-based authorization. Later (in 0.9.0), the payload() will 
> be removed along with SessionKey (AURORA-1229). That will let us get rid of 
> SchedulerProxy (or reduce it substantially). The auth_module might stay 
> though to support other auth plugins (e.g. requests-ntlm or 
> requests-oauthlib).
> 
> TODO: integration e2e tests once scheduler side lands.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 11a307cdb476ebcc25ab5c6b555bed29241ea988 
>   src/main/python/apache/aurora/client/api/__init__.py 
> a81329f6f947bbea4001c3a521c1923410a51eab 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 95e553427492407743dcac31d70f392a7c1bbc02 
>   src/main/python/apache/aurora/client/cli/BUILD 
> c6b4e8a09d1315cf5defee2155a6e0c697892a30 
>   src/main/python/apache/aurora/client/cli/client.py 
> 24516d114db1743cdf600c542a27fcf5b68053a0 
>   src/main/python/apache/aurora/common/auth/BUILD 
> 966484627dab90e7606f1fc638cd0e159aee3317 
>   src/main/python/apache/aurora/common/auth/__init__.py 
> 3119fd63d3dfa28f93f219b23030059580fed098 
>   src/main/python/apache/aurora/common/auth/auth_module.py 
> 5f4116ef4cfbc407e0c50dc938870fb14e2299b4 
>   src/main/python/apache/aurora/common/auth/auth_module_manager.py 
> 73a8e5cd51edf694b971cd2c298ff406aff8c6d7 
>   src/main/python/apache/aurora/common/auth/kerberos.py PRE-CREATION 
>   src/main/python/apache/aurora/common/transport.py 
> 395f8a94d9a27aad00166a17f2528a8c0833ffdd 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 0a6194831c332a96eab62b869c4e05cfa9def058 
>   src/test/python/apache/aurora/common/test_transport.py 
> b78e0b3badfbbeecefff7b5954f3796cef4da9d8 
> 
> Diff: https://reviews.apache.org/r/32541/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python:all
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32559: Add Kerberos support to the scheduler

2015-04-02 Thread Bill Farner

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

src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizeHeaderToken.java

What does this mean?  Perhaps the missing context is " 
will do this part".

src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizeHeaderToken.java

Marking this method as deprecated is slightly confusing.  I assume it's 
only because you don't want other code in the project to call it?  If that's 
the case, i suggest you don't use deprecated as the signal and instead docs + 
code review.

src/main/java/org/apache/aurora/scheduler/http/api/security/IniShiroRealmModule.java

``

src/main/java/org/apache/aurora/scheduler/http/api/security/IniShiroRealmModule.java

"it" is ambiguous.  Consider rewording: "Another filter may still be used 
for authentication, in which case the ini file can still be used to provide 
authorization configuration."

src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5Realm.java

comment formatting is off

src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5Realm.java

nonNull?

src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java

These are pure magic to me.  Please doc.

src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java

Why is this `Optional` if it's required?  Ditto below.

src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java

This is a question of convention - what's the upside to addError/return as 
opposed to throwing?

src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java

Given the behavior i see, i'd prefer a constant and String.format over the 
resource file and use of StringTemplate.

If there are reasons for retaining this complexity, please leave a comment 
justifying it.

src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java

s/jass/jaas/

src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java

Why is this in a `finally`?  Seems like any exception is fatal, and there's 
no more work to do.  Use of a finally block makes the expectations of this code 
confusing.

src/main/java/org/apache/aurora/scheduler/http/api/security/Kerberos5ShiroRealmModule.java

remove newline

src/main/java/org/apache/aurora/scheduler/http/api/security/ShiroKerberosAuthenticationFilter.java

Please use a consistent exception logging convention - the exception logged 
on :67 is different.

src/test/sh/org/apache/aurora/e2e/test_kerberos_end_to_end.sh

Please invoke this from the existing end-to-end test script to avoid 
another qualification step for developers.

- Bill Farner

On March 27, 2015, 8:50 p.m., Kevin Sweeney wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32559/
> ---
> 
> (Updated March 27, 2015, 8:50 p.m.)
> 
> 
> Review request for Aurora, Joshua Cohen and Bill Farner.
> 
> 
> Bugs: AURORA-812
> https://issues.apache.org/jira/browse/AURORA-812
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> Support authenticating to the scheduler API with Kerberos.
> 
> 
> Diffs
> -
> 
>   config/findbugs/excludeFilter.xml 5ff5f87d6bb7d8bf3d9ecb1bc6c6b1a524d1bf15 
>   examples/vagrant/provision-dev-cluster.sh 
> ae500436e703140065e5c16fc0e38dbe3214e69f 
>   examples/vagrant/upstart/aurora-scheduler-kerberos.conf PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/ApiSecurityModule.java
>  ec6a02c4086ee0d5a7529083030d978ea889f677 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/AuthorizeHeaderToken.java
>  PRE-CREATION 
>   
> src/main/java/org/apache/aurora/scheduler/http/api/security/IniShiroRealmModule.java
>

Re: Review Request 32541: Adding client Kerberos support.

2015-04-02 Thread Kevin Sweeney



> On April 1, 2015, 1:40 p.m., Kevin Sweeney wrote:
> > src/main/python/apache/aurora/client/cli/BUILD, line 29
> > 
> >
> > How about "kaurora"
> 
> Maxim Khutornenko wrote:
> This is a joke, right? :) If not I am really struggling how it would be 
> more informative.

Some Kerberized tools have a convention of being prefixed with a "k" (su => 
ksu, rsh => krsh, passwd => kpasswd). I think the ergonomics are better (no 
underscores in command names), but don't feel strongly about it.


- Kevin


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


On April 1, 2015, 6:10 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32541/
> ---
> 
> (Updated April 1, 2015, 6:10 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Brian Wickman.
> 
> 
> Bugs: AURORA-813
> https://issues.apache.org/jira/browse/AURORA-813
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> First take on client kerberos support. The idea is to repurpose the existing 
> auth_module system to support both legacy and kerberos during the deprecation 
> period. This way the 0.8.0 client will be able to talk to pre-0.8.0 scheduler 
> and use SessionKey-based authorization. Later (in 0.9.0), the payload() will 
> be removed along with SessionKey (AURORA-1229). That will let us get rid of 
> SchedulerProxy (or reduce it substantially). The auth_module might stay 
> though to support other auth plugins (e.g. requests-ntlm or 
> requests-oauthlib).
> 
> TODO: integration e2e tests once scheduler side lands.
> 
> 
> Diffs
> -
> 
>   3rdparty/python/requirements.txt 11a307cdb476ebcc25ab5c6b555bed29241ea988 
>   src/main/python/apache/aurora/client/api/__init__.py 
> a81329f6f947bbea4001c3a521c1923410a51eab 
>   src/main/python/apache/aurora/client/api/scheduler_client.py 
> 95e553427492407743dcac31d70f392a7c1bbc02 
>   src/main/python/apache/aurora/client/cli/BUILD 
> c6b4e8a09d1315cf5defee2155a6e0c697892a30 
>   src/main/python/apache/aurora/client/cli/client.py 
> 24516d114db1743cdf600c542a27fcf5b68053a0 
>   src/main/python/apache/aurora/common/auth/BUILD 
> 966484627dab90e7606f1fc638cd0e159aee3317 
>   src/main/python/apache/aurora/common/auth/__init__.py 
> 3119fd63d3dfa28f93f219b23030059580fed098 
>   src/main/python/apache/aurora/common/auth/auth_module.py 
> 5f4116ef4cfbc407e0c50dc938870fb14e2299b4 
>   src/main/python/apache/aurora/common/auth/auth_module_manager.py 
> 73a8e5cd51edf694b971cd2c298ff406aff8c6d7 
>   src/main/python/apache/aurora/common/auth/kerberos.py PRE-CREATION 
>   src/main/python/apache/aurora/common/transport.py 
> 395f8a94d9a27aad00166a17f2528a8c0833ffdd 
>   src/test/python/apache/aurora/client/api/test_scheduler_client.py 
> 0a6194831c332a96eab62b869c4e05cfa9def058 
>   src/test/python/apache/aurora/common/test_transport.py 
> b78e0b3badfbbeecefff7b5954f3796cef4da9d8 
> 
> Diff: https://reviews.apache.org/r/32541/diff/
> 
> 
> Testing
> ---
> 
> ./pants test.pytest --no-fast src/test/python:all
> ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 32597: Improving async preemptor efficiency.

Re: Review Request 32806: Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.

Re: Review Request 32806: Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.

Re: Review Request 32806: Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.

Review Request 32806: Remove use of LocalServiceRegistry, simplify plumbing of HTTP address.

Re: Review Request 32802: Add a benchmark for getRoleSummary.

Re: Review Request 32802: Add a benchmark for getRoleSummary.

Review Request 32802: Add a benchmark for getRoleSummary.

Re: Review Request 32597: Improving async preemptor efficiency.

Re: Review Request 32597: Improving async preemptor efficiency.

Re: Review Request 32597: Improving async preemptor efficiency.

Re: Review Request 32541: Adding client Kerberos support.

Re: Review Request 32541: Adding client Kerberos support.

Re: Review Request 32541: Adding client Kerberos support.

Re: Review Request 32559: Add Kerberos support to the scheduler

Re: Review Request 32541: Adding client Kerberos support.

16 matches

Site Navigation

Mail list logo

Footer information