aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maxim Khutornenko" <ma...@apache.org>
Subject Re: Review Request 26899: Require StateManager callers to open their own transactions.
Date Tue, 21 Oct 2014 01:25:04 GMT

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

Ship it!



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/26899/#comment98233>

    FWIW "|=" is a standard way to record boolean True in a result. This feels lees readable
in my opinion.



src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
<https://reviews.apache.org/r/26899/#comment98235>

    Would be great to update unit tests to validate this change.


- Maxim Khutornenko


On Oct. 21, 2014, 1:09 a.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26899/
> -----------------------------------------------------------
> 
> (Updated Oct. 21, 2014, 1:09 a.m.)
> 
> 
> Review request for Aurora, Kevin Sweeney and Maxim Khutornenko.
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> Apologies for the large diff - `StateManager` has a large fan-out.  This is a pure functional
no-op - just moving the call to `storage.write` up in the stack.
> 
> This change is to prepare for AURORA-871 [1], as well as generally make use of `StateManager`
safer.  With the API prior to this diff, it would be easy to unintentionally attempt a fateful
read->write lock upgrade.  This firms up the API by requiring the caller to bring their
own transaction, removing the implementation detail that a write transaction is opened behind
the scenes.
> 
> [1] https://issues.apache.org/jira/browse/AURORA-871
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java 250c2df8113adfd62b3a7e124f7994156c82b5f7

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

>   src/main/java/org/apache/aurora/scheduler/async/TaskHistoryPruner.java 345cd8959045302fe3711e22396f5f7244a88c44

>   src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java 1189ed0318ae0cf9663f0fa41775c4dd625bb397

>   src/main/java/org/apache/aurora/scheduler/async/TaskThrottler.java ca6129c7e0225530336c88f91a1451892f2ce234

>   src/main/java/org/apache/aurora/scheduler/async/TaskTimeout.java 89140223f4f3acd02ade6fb95734744ef19d89bc

>   src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 9388657ccb904364e460ec612c3da562b8952d7e

>   src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 21cfebdf11e0652e192cf08e35c8581b1246f7b5

>   src/main/java/org/apache/aurora/scheduler/state/StateManager.java 3a2fd279c2953d564b7fddabf31afda001bb3dfe

>   src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 58b94c2f2f3bac00f0692579974e8bdf159b6e40

>   src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 4db9be86f2e7db08d12e0182914a7c5130301b13

>   src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java e792d23d6bb13b4e61b078beea6d063f72f0d8fc

>   src/main/java/org/apache/aurora/scheduler/updater/InstanceAction.java 3774c851b58e747acf25735d24334408b1c5386d

>   src/main/java/org/apache/aurora/scheduler/updater/InstanceActionHandler.java f4363aa8ce9ef9f583b52251f351c8c971ef8119

>   src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java 6ec130f4a9a5075b34452efb27c8fd0f08f93a63

>   src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 083a63543e5f9041f13fc6be66877f7173a5bf32

>   src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 8ee84cda8670d117e2efa2d1a114da6f0d8315d6

>   src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java b3e4ae39067b1dfb632f5d685d69fcbd7d4705da

>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 6534329a92bf005223fa8907cbe4a8a3a511e142

>   src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 9970948bace4c0ecbc51d6fc79270d77fb17bf87

>   src/test/java/org/apache/aurora/scheduler/async/TaskThrottlerTest.java a28e512afde584fb94ff7686d7a3e3fbb51f8b7b

>   src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java e79327c3b8ead01495e063e5c0e9270568e16f69

>   src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java b6b1bcbf8080eb7d1e7eca4a486cc063f28db75d

>   src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
4d96761e04a342ad3564bdba4afdc889f27ac123 
>   src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java cf4a015a040338a642fb07eec1fb7b5c11058fe5

>   src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java c48cbae4864127e7799917182439f7670285b0d3

>   src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
f36a88fda3539553800bd727c3d2a77a54f1e71c 
>   src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java f9ed46f56bb11e9c158268c16f29557f3e99c84e

>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java f739e6d1b1af6eea4875e03d32bfe88cef87b3ff

> 
> Diff: https://reviews.apache.org/r/26899/diff/
> 
> 
> Testing
> -------
> 
>     ./gradlew build -Pq
> 
> 
> Thanks,
> 
> Bill Farner
> 
>


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