aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Bill Farner" <wfar...@apache.org>
Subject Re: Review Request 17578: When trying to schedule a task, only query once for active tasks in the job.
Date Fri, 31 Jan 2014 03:20:08 GMT


> On Jan. 31, 2014, 2:19 a.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/async/Preemptor.java, line 78
> > <https://reviews.apache.org/r/17578/diff/1/?file=457382#file457382line78>
> >
> >     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
> > <https://reviews.apache.org/r/17578/diff/1/?file=457386#file457386line36>
> >
> >     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
> > <https://reviews.apache.org/r/17578/diff/1/?file=457389#file457389line193>
> >
> >     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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message