Re: Review Request 17578: When trying to schedule a task, only query once for active tasks in the job.

2014-01-30 Thread Bill Farner 


> On Jan. 31, 2014, 2:55 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java, lines 
> > 185-187
> > 
> >
> > I think this is actually somewhat a regression - in the past it was 
> > possible for a "cheap" filter to reject a task just based on a precomputed 
> > index and avoid a query altogether. Now we will always make at least one 
> > query. How would you feel about making this query lazy so that it happens 
> > the first time the info is needed rather than eagerly when it might not be 
> > needed?
> 
> Bill Farner wrote:
> In the spirit of the interfaces, you're correct.  In practice, though, 
> it's going to be exceedingly rare that we don't do this query at least once 
> in a scheduling round.  It would only happen if there are no resource offers, 
> or if all offer/task combinations have dedicated mismatches.
> 
> Thinking forward, though, laziness will avoid unexpected behavior, so 
> i've added it.

I'm going to proceed with this change so we can test this out in production 
sooner, and i believe i've addressed the issue raised.  Please don't hesitate 
to follow up with more comments!


- Bill


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


On Jan. 31, 2014, 3:28 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17578/
> ---
> 
> (Updated Jan. 31, 2014, 3:28 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-139
> https://issues.apache.org/jira/browse/AURORA-139
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The majority of the code here is related to plumbing the argument through two 
> layers (SchedulingFilter, TaskAssigner) and updating related tests.
> 
> One functional change in this diff is the removal of a filter in 
> AttributeFilter, which was unnecessary since the query was already 
> job-scoped.  The filter existed because at one point we supported job 
> configurations that could request to avoid other jobs, so the task set would 
> include other jobs.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> 72d3621055d28b7b0ec7eeca7a1e8be6ce1e5042 
>   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
> c11f4837152c3c8de07058d35a874b406ea7878f 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
> 4afc33259cc0150424f8d790a38c0d0b38fb2392 
>   
> src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
>  c7f4a1b975129c58328e42c758683ebad06ae035 
>   src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java 
> 0816d3f99ce8347d6ef705116b48abb164854c8a 
>   src/main/java/org/apache/aurora/scheduler/filter/CachedJobState.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java 
> 17fc8b99c48b3133af3d1f0c3c75d822e78757e6 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
> 999e0f7bba882a52a6b278f36c26c48a87a88958 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> 4ba1483386e8fe988a800253c44e9359561bd972 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> d8f326e45aaacbff3447c3d5b12fb4f9112a82cf 
>   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
> 025294fd51abe1eb41a6ade39b5a9fea8a8422b6 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
> 35dd666c44f39d6b13e4dc516fed8117f88db0e5 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
> 6c124c53130530881699abfd5634ba0171932afa 
>   
> src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
>  c5379fe725a5dd36dbe6217c18b281d7a3209c67 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  f3df73c1d248165bf19bdfd1305fd7a9b8190cf7 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> 154ccc4716168ad966b246280370600de92d5d7f 
> 
> Diff: https://reviews.apache.org/r/17578/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 17578: When trying to schedule a task, only query once for active tasks in the job.

2014-01-30 Thread Bill Farner 

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

(Updated Jan. 31, 2014, 3:28 a.m.)


Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


Changes
---

oh intellij, why do you forsake me


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


Repository: aurora


Description
---

The majority of the code here is related to plumbing the argument through two 
layers (SchedulingFilter, TaskAssigner) and updating related tests.

One functional change in this diff is the removal of a filter in 
AttributeFilter, which was unnecessary since the query was already job-scoped.  
The filter existed because at one point we supported job configurations that 
could request to avoid other jobs, so the task set would include other jobs.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
72d3621055d28b7b0ec7eeca7a1e8be6ce1e5042 
  src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
c11f4837152c3c8de07058d35a874b406ea7878f 
  src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
4afc33259cc0150424f8d790a38c0d0b38fb2392 
  
src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 
c7f4a1b975129c58328e42c758683ebad06ae035 
  src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java 
0816d3f99ce8347d6ef705116b48abb164854c8a 
  src/main/java/org/apache/aurora/scheduler/filter/CachedJobState.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java 
17fc8b99c48b3133af3d1f0c3c75d822e78757e6 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
999e0f7bba882a52a6b278f36c26c48a87a88958 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
4ba1483386e8fe988a800253c44e9359561bd972 
  src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
d8f326e45aaacbff3447c3d5b12fb4f9112a82cf 
  src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
025294fd51abe1eb41a6ade39b5a9fea8a8422b6 
  src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
35dd666c44f39d6b13e4dc516fed8117f88db0e5 
  src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
6c124c53130530881699abfd5634ba0171932afa 
  
src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
 c5379fe725a5dd36dbe6217c18b281d7a3209c67 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
f3df73c1d248165bf19bdfd1305fd7a9b8190cf7 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
154ccc4716168ad966b246280370600de92d5d7f 

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


Testing
---

./gradlew build


Thanks,

Bill Farner

Re: Review Request 17578: When trying to schedule a task, only query once for active tasks in the job.

2014-01-30 Thread Bill Farner 

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

(Updated Jan. 31, 2014, 3:25 a.m.)


Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


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


Repository: aurora


Description
---

The majority of the code here is related to plumbing the argument through two 
layers (SchedulingFilter, TaskAssigner) and updating related tests.

One functional change in this diff is the removal of a filter in 
AttributeFilter, which was unnecessary since the query was already job-scoped.  
The filter existed because at one point we supported job configurations that 
could request to avoid other jobs, so the task set would include other jobs.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
72d3621055d28b7b0ec7eeca7a1e8be6ce1e5042 
  src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
c11f4837152c3c8de07058d35a874b406ea7878f 
  src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
4afc33259cc0150424f8d790a38c0d0b38fb2392 
  
src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 
c7f4a1b975129c58328e42c758683ebad06ae035 
  src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java 
0816d3f99ce8347d6ef705116b48abb164854c8a 
  src/main/java/org/apache/aurora/scheduler/filter/CachedJobState.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java 
17fc8b99c48b3133af3d1f0c3c75d822e78757e6 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
999e0f7bba882a52a6b278f36c26c48a87a88958 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
4ba1483386e8fe988a800253c44e9359561bd972 
  src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
d8f326e45aaacbff3447c3d5b12fb4f9112a82cf 
  src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
025294fd51abe1eb41a6ade39b5a9fea8a8422b6 
  src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
35dd666c44f39d6b13e4dc516fed8117f88db0e5 
  src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
6c124c53130530881699abfd5634ba0171932afa 
  
src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
 c5379fe725a5dd36dbe6217c18b281d7a3209c67 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
f3df73c1d248165bf19bdfd1305fd7a9b8190cf7 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
154ccc4716168ad966b246280370600de92d5d7f 

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


Testing
---

./gradlew build


Thanks,

Bill Farner

Re: Review Request 17578: When trying to schedule a task, only query once for active tasks in the job.

2014-01-30 Thread Bill Farner 


> On Jan. 31, 2014, 2:19 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/async/Preemptor.java, line 78
> > 
> >
> > Missing @param

Thanks, fixed.


> On Jan. 31, 2014, 2:19 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/filter/CachedJobState.java, line 
> > 36
> > 
> >
> > Having an actual description of the proposed CachedJobState usage in 
> > addition to the TODO would be nice here. Specifically, documenting the 
> > acquired perf gain and data consistency guarantees.

I'm not sure what's right to document here, given the limited ability of this 
class to provide any guarantees.  I added one sentence offering a suggested 
use, happy to revisit if you'd like to see more.


> On Jan. 31, 2014, 2:19 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java, 
> > line 193
> > 
> >
> > When do we stop fighting it? :)

Ugh.  Good question, in subsequent diffs/reviews i will begin switching to 
intellij's style.


- Bill


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


On Jan. 31, 2014, 2:17 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17578/
> ---
> 
> (Updated Jan. 31, 2014, 2:17 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-139
> https://issues.apache.org/jira/browse/AURORA-139
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The majority of the code here is related to plumbing the argument through two 
> layers (SchedulingFilter, TaskAssigner) and updating related tests.
> 
> One functional change in this diff is the removal of a filter in 
> AttributeFilter, which was unnecessary since the query was already 
> job-scoped.  The filter existed because at one point we supported job 
> configurations that could request to avoid other jobs, so the task set would 
> include other jobs.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> 72d3621055d28b7b0ec7eeca7a1e8be6ce1e5042 
>   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
> c11f4837152c3c8de07058d35a874b406ea7878f 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
> 4afc33259cc0150424f8d790a38c0d0b38fb2392 
>   
> src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
>  c7f4a1b975129c58328e42c758683ebad06ae035 
>   src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java 
> 0816d3f99ce8347d6ef705116b48abb164854c8a 
>   src/main/java/org/apache/aurora/scheduler/filter/CachedJobState.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java 
> 17fc8b99c48b3133af3d1f0c3c75d822e78757e6 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
> 999e0f7bba882a52a6b278f36c26c48a87a88958 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> 4ba1483386e8fe988a800253c44e9359561bd972 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> d8f326e45aaacbff3447c3d5b12fb4f9112a82cf 
>   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
> 025294fd51abe1eb41a6ade39b5a9fea8a8422b6 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
> 35dd666c44f39d6b13e4dc516fed8117f88db0e5 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
> 6c124c53130530881699abfd5634ba0171932afa 
>   
> src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
>  c5379fe725a5dd36dbe6217c18b281d7a3209c67 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  f3df73c1d248165bf19bdfd1305fd7a9b8190cf7 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> 154ccc4716168ad966b246280370600de92d5d7f 
> 
> Diff: https://reviews.apache.org/r/17578/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 17578: When trying to schedule a task, only query once for active tasks in the job.

2014-01-30 Thread Bill Farner 


> On Jan. 31, 2014, 2:55 a.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java, lines 
> > 185-187
> > 
> >
> > I think this is actually somewhat a regression - in the past it was 
> > possible for a "cheap" filter to reject a task just based on a precomputed 
> > index and avoid a query altogether. Now we will always make at least one 
> > query. How would you feel about making this query lazy so that it happens 
> > the first time the info is needed rather than eagerly when it might not be 
> > needed?

In the spirit of the interfaces, you're correct.  In practice, though, it's 
going to be exceedingly rare that we don't do this query at least once in a 
scheduling round.  It would only happen if there are no resource offers, or if 
all offer/task combinations have dedicated mismatches.

Thinking forward, though, laziness will avoid unexpected behavior, so i've 
added it.


- Bill


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


On Jan. 31, 2014, 2:17 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17578/
> ---
> 
> (Updated Jan. 31, 2014, 2:17 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-139
> https://issues.apache.org/jira/browse/AURORA-139
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The majority of the code here is related to plumbing the argument through two 
> layers (SchedulingFilter, TaskAssigner) and updating related tests.
> 
> One functional change in this diff is the removal of a filter in 
> AttributeFilter, which was unnecessary since the query was already 
> job-scoped.  The filter existed because at one point we supported job 
> configurations that could request to avoid other jobs, so the task set would 
> include other jobs.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> 72d3621055d28b7b0ec7eeca7a1e8be6ce1e5042 
>   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
> c11f4837152c3c8de07058d35a874b406ea7878f 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
> 4afc33259cc0150424f8d790a38c0d0b38fb2392 
>   
> src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
>  c7f4a1b975129c58328e42c758683ebad06ae035 
>   src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java 
> 0816d3f99ce8347d6ef705116b48abb164854c8a 
>   src/main/java/org/apache/aurora/scheduler/filter/CachedJobState.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java 
> 17fc8b99c48b3133af3d1f0c3c75d822e78757e6 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
> 999e0f7bba882a52a6b278f36c26c48a87a88958 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> 4ba1483386e8fe988a800253c44e9359561bd972 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> d8f326e45aaacbff3447c3d5b12fb4f9112a82cf 
>   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
> 025294fd51abe1eb41a6ade39b5a9fea8a8422b6 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
> 35dd666c44f39d6b13e4dc516fed8117f88db0e5 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
> 6c124c53130530881699abfd5634ba0171932afa 
>   
> src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
>  c5379fe725a5dd36dbe6217c18b281d7a3209c67 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  f3df73c1d248165bf19bdfd1305fd7a9b8190cf7 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> 154ccc4716168ad966b246280370600de92d5d7f 
> 
> Diff: https://reviews.apache.org/r/17578/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 17578: When trying to schedule a task, only query once for active tasks in the job.

2014-01-30 Thread Kevin Sweeney 

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



src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java


I think this is actually somewhat a regression - in the past it was 
possible for a "cheap" filter to reject a task just based on a precomputed 
index and avoid a query altogether. Now we will always make at least one query. 
How would you feel about making this query lazy so that it happens the first 
time the info is needed rather than eagerly when it might not be needed?


- Kevin Sweeney


On Jan. 30, 2014, 6:17 p.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17578/
> ---
> 
> (Updated Jan. 30, 2014, 6:17 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-139
> https://issues.apache.org/jira/browse/AURORA-139
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The majority of the code here is related to plumbing the argument through two 
> layers (SchedulingFilter, TaskAssigner) and updating related tests.
> 
> One functional change in this diff is the removal of a filter in 
> AttributeFilter, which was unnecessary since the query was already 
> job-scoped.  The filter existed because at one point we supported job 
> configurations that could request to avoid other jobs, so the task set would 
> include other jobs.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> 72d3621055d28b7b0ec7eeca7a1e8be6ce1e5042 
>   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
> c11f4837152c3c8de07058d35a874b406ea7878f 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
> 4afc33259cc0150424f8d790a38c0d0b38fb2392 
>   
> src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
>  c7f4a1b975129c58328e42c758683ebad06ae035 
>   src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java 
> 0816d3f99ce8347d6ef705116b48abb164854c8a 
>   src/main/java/org/apache/aurora/scheduler/filter/CachedJobState.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java 
> 17fc8b99c48b3133af3d1f0c3c75d822e78757e6 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
> 999e0f7bba882a52a6b278f36c26c48a87a88958 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> 4ba1483386e8fe988a800253c44e9359561bd972 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> d8f326e45aaacbff3447c3d5b12fb4f9112a82cf 
>   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
> 025294fd51abe1eb41a6ade39b5a9fea8a8422b6 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
> 35dd666c44f39d6b13e4dc516fed8117f88db0e5 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
> 6c124c53130530881699abfd5634ba0171932afa 
>   
> src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
>  c5379fe725a5dd36dbe6217c18b281d7a3209c67 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  f3df73c1d248165bf19bdfd1305fd7a9b8190cf7 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> 154ccc4716168ad966b246280370600de92d5d7f 
> 
> Diff: https://reviews.apache.org/r/17578/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 17578: When trying to schedule a task, only query once for active tasks in the job.

2014-01-30 Thread Maxim Khutornenko 

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

Ship it!



src/main/java/org/apache/aurora/scheduler/async/Preemptor.java


Missing @param



src/main/java/org/apache/aurora/scheduler/filter/CachedJobState.java


Having an actual description of the proposed CachedJobState usage in 
addition to the TODO would be nice here. Specifically, documenting the acquired 
perf gain and data consistency guarantees. 



src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java


When do we stop fighting it? :)


- Maxim Khutornenko


On Jan. 31, 2014, 2:17 a.m., Bill Farner wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17578/
> ---
> 
> (Updated Jan. 31, 2014, 2:17 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-139
> https://issues.apache.org/jira/browse/AURORA-139
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The majority of the code here is related to plumbing the argument through two 
> layers (SchedulingFilter, TaskAssigner) and updating related tests.
> 
> One functional change in this diff is the removal of a filter in 
> AttributeFilter, which was unnecessary since the query was already 
> job-scoped.  The filter existed because at one point we supported job 
> configurations that could request to avoid other jobs, so the task set would 
> include other jobs.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> 72d3621055d28b7b0ec7eeca7a1e8be6ce1e5042 
>   src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
> c11f4837152c3c8de07058d35a874b406ea7878f 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
> 4afc33259cc0150424f8d790a38c0d0b38fb2392 
>   
> src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java
>  c7f4a1b975129c58328e42c758683ebad06ae035 
>   src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java 
> 0816d3f99ce8347d6ef705116b48abb164854c8a 
>   src/main/java/org/apache/aurora/scheduler/filter/CachedJobState.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java 
> 17fc8b99c48b3133af3d1f0c3c75d822e78757e6 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
> 999e0f7bba882a52a6b278f36c26c48a87a88958 
>   src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
> 4ba1483386e8fe988a800253c44e9359561bd972 
>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
> d8f326e45aaacbff3447c3d5b12fb4f9112a82cf 
>   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
> 025294fd51abe1eb41a6ade39b5a9fea8a8422b6 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
> 35dd666c44f39d6b13e4dc516fed8117f88db0e5 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
> 6c124c53130530881699abfd5634ba0171932afa 
>   
> src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
>  c5379fe725a5dd36dbe6217c18b281d7a3209c67 
>   
> src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java
>  f3df73c1d248165bf19bdfd1305fd7a9b8190cf7 
>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
> 154ccc4716168ad966b246280370600de92d5d7f 
> 
> Diff: https://reviews.apache.org/r/17578/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew build
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Re: Review Request 17578: When trying to schedule a task, only query once for active tasks in the job.

2014-01-30 Thread Bill Farner 

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

(Updated Jan. 31, 2014, 2:17 a.m.)


Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


Changes
---

Reduced the diff in SchedulingFilterImplTest.


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


Repository: aurora


Description
---

The majority of the code here is related to plumbing the argument through two 
layers (SchedulingFilter, TaskAssigner) and updating related tests.

One functional change in this diff is the removal of a filter in 
AttributeFilter, which was unnecessary since the query was already job-scoped.  
The filter existed because at one point we supported job configurations that 
could request to avoid other jobs, so the task set would include other jobs.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
72d3621055d28b7b0ec7eeca7a1e8be6ce1e5042 
  src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
c11f4837152c3c8de07058d35a874b406ea7878f 
  src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
4afc33259cc0150424f8d790a38c0d0b38fb2392 
  
src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 
c7f4a1b975129c58328e42c758683ebad06ae035 
  src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java 
0816d3f99ce8347d6ef705116b48abb164854c8a 
  src/main/java/org/apache/aurora/scheduler/filter/CachedJobState.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java 
17fc8b99c48b3133af3d1f0c3c75d822e78757e6 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
999e0f7bba882a52a6b278f36c26c48a87a88958 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
4ba1483386e8fe988a800253c44e9359561bd972 
  src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
d8f326e45aaacbff3447c3d5b12fb4f9112a82cf 
  src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
025294fd51abe1eb41a6ade39b5a9fea8a8422b6 
  src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
35dd666c44f39d6b13e4dc516fed8117f88db0e5 
  src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
6c124c53130530881699abfd5634ba0171932afa 
  
src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
 c5379fe725a5dd36dbe6217c18b281d7a3209c67 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
f3df73c1d248165bf19bdfd1305fd7a9b8190cf7 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
154ccc4716168ad966b246280370600de92d5d7f 

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


Testing
---

./gradlew build


Thanks,

Bill Farner

Review Request 17578: When trying to schedule a task, only query once for active tasks in the job.

2014-01-30 Thread Bill Farner 

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

Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.


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


Repository: aurora


Description
---

The majority of the code here is related to plumbing the argument through two 
layers (SchedulingFilter, TaskAssigner) and updating related tests.

One functional change in this diff is the removal of a filter in 
AttributeFilter, which was unnecessary since the query was already job-scoped.  
The filter existed because at one point we supported job configurations that 
could request to avoid other jobs, so the task set would include other jobs.


Diffs
-

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
72d3621055d28b7b0ec7eeca7a1e8be6ce1e5042 
  src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 
c11f4837152c3c8de07058d35a874b406ea7878f 
  src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
4afc33259cc0150424f8d790a38c0d0b38fb2392 
  
src/main/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilter.java 
c7f4a1b975129c58328e42c758683ebad06ae035 
  src/main/java/org/apache/aurora/scheduler/filter/AttributeFilter.java 
0816d3f99ce8347d6ef705116b48abb164854c8a 
  src/main/java/org/apache/aurora/scheduler/filter/CachedJobState.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/filter/ConstraintFilter.java 
17fc8b99c48b3133af3d1f0c3c75d822e78757e6 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java 
999e0f7bba882a52a6b278f36c26c48a87a88958 
  src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilterImpl.java 
4ba1483386e8fe988a800253c44e9359561bd972 
  src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 
d8f326e45aaacbff3447c3d5b12fb4f9112a82cf 
  src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 
025294fd51abe1eb41a6ade39b5a9fea8a8422b6 
  src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
35dd666c44f39d6b13e4dc516fed8117f88db0e5 
  src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
6c124c53130530881699abfd5634ba0171932afa 
  
src/test/java/org/apache/aurora/scheduler/events/NotifyingSchedulingFilterTest.java
 c5379fe725a5dd36dbe6217c18b281d7a3209c67 
  
src/test/java/org/apache/aurora/scheduler/filter/SchedulingFilterImplTest.java 
f3df73c1d248165bf19bdfd1305fd7a9b8190cf7 
  src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 
154ccc4716168ad966b246280370600de92d5d7f 

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


Testing
---

./gradlew build


Thanks,

Bill Farner