> On April 7, 2014, 11:07 p.m., Kevin Sweeney wrote:
> > src/main/java/org/apache/aurora/scheduler/async/TaskGroup.java, line 62
> > <https://reviews.apache.org/r/20066/diff/3/?file=550680#file550680line62>
> >
> >     What does synchronized add here? As far as I can tell this value is 
> > immutable.

You're absolutely right.
This is a pattern i've converged towards when using intrinsic locks — 
synchronize all public methods (or otherwise externally-accessible code paths). 
 The motivation is to simplify visual scans of thread [un]safety and (to a 
lesser degree) future-proof from new functionality and copy/paste.


- Bill


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


On April 7, 2014, 3:07 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/20066/
> -----------------------------------------------------------
> 
> (Updated April 7, 2014, 3:07 p.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Bugs: AURORA-302
>     https://issues.apache.org/jira/browse/AURORA-302
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> TaskGroups suffered from (at least) two race conditions:
> 
> - When a group is added, the call to TaskGroup#push() would race (and 
> normally beat) the async Runnable execution.  However, if the latter won, it 
> would result in skipping further evaluations of the group forever.
> 
> - A task ID was removed from the TaskGroup while being evaluated.  If the 
> task was deleted during this time, the task would be added back to the 
> TaskGroup and re-evaluated later.  This case was benign since the subsequent 
> evaluation would discover that the task does not exist.
> 
> Test cases were added to TaskGroupsTest to reproduce these scenarios.
> 
> In addition to these problems, TaskGroups was still performing throttling of 
> flapping tasks, despite TaskThrottler being introduced for that purpose.  
> Removing that extra code simplifies things quite a bit.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/async/TaskGroup.java 
> ece7e3a46075edfa881c8750c491d35fdb2b98a8 
>   src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java 
> 6c95c6f7695cb8126105818528900cb09887ad3e 
>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 
> 263235ce7c279f98920d7681f6162458be40593c 
>   src/test/java/org/apache/aurora/scheduler/async/TaskGroupsTest.java 
> PRE-CREATION 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 
> 65e00f70cc860b2d225335e4487add2a903e72bb 
>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 
> ce03abc5248dbc95306c759a04828830e3057b81 
> 
> Diff: https://reviews.apache.org/r/20066/diff/
> 
> 
> Testing
> -------
> 
> ./gradlew build
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to