Return-Path: X-Original-To: apmail-aurora-reviews-archive@minotaur.apache.org Delivered-To: apmail-aurora-reviews-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 7D734FDC9 for ; Thu, 13 Nov 2014 22:21:14 +0000 (UTC) Received: (qmail 10778 invoked by uid 500); 13 Nov 2014 22:21:14 -0000 Delivered-To: apmail-aurora-reviews-archive@aurora.apache.org Received: (qmail 10734 invoked by uid 500); 13 Nov 2014 22:21:14 -0000 Mailing-List: contact reviews-help@aurora.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@aurora.incubator.apache.org Delivered-To: mailing list reviews@aurora.incubator.apache.org Received: (qmail 10722 invoked by uid 99); 13 Nov 2014 22:21:14 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 13 Nov 2014 22:21:13 +0000 X-ASF-Spam-Status: No, hits=-1998.4 required=5.0 tests=ALL_TRUSTED,HTML_MESSAGE,RP_MATCHES_RCVD X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO mail.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with SMTP; Thu, 13 Nov 2014 22:21:12 +0000 Received: (qmail 8233 invoked by uid 99); 13 Nov 2014 22:19:37 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Thu, 13 Nov 2014 22:19:37 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id BDC381FB61D; Thu, 13 Nov 2014 22:19:39 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============6306424302822503489==" MIME-Version: 1.0 Subject: Re: Review Request 26899: Require StateManager callers to open their own transactions. From: "Bill Farner" To: "Maxim Khutornenko" , "Kevin Sweeney" Cc: "Bill Farner" , "Aurora ReviewBot" , "Aurora" Date: Thu, 13 Nov 2014 22:19:39 -0000 Message-ID: <20141113221939.9703.74071@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org Auto-Submitted: auto-generated Sender: "Bill Farner" X-ReviewGroup: Aurora X-ReviewRequest-URL: https://reviews.apache.org/r/26899/ X-Sender: "Bill Farner" References: <20141021010924.1282.19708@reviews.apache.org> In-Reply-To: <20141021010924.1282.19708@reviews.apache.org> Reply-To: "Bill Farner" X-ReviewRequest-Repository: aurora X-Virus-Checked: Checked by ClamAV on apache.org --===============6306424302822503489== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26899/ ----------------------------------------------------------- (Updated Nov. 13, 2014, 10:19 p.m.) Review request for Aurora, Kevin Sweeney and Maxim Khutornenko. Changes ------- Other priorities caused this review to go stale, so the inter-diff is probably not useful any more :-/ 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 (updated) ----- src/main/java/org/apache/aurora/scheduler/UserTaskLauncher.java e1b7d05b32bc5f6061e89fe2af901e34650f97bc src/main/java/org/apache/aurora/scheduler/async/Preemptor.java 1d337f6245530f0e22f75d5f1d301462d53dd8bb src/main/java/org/apache/aurora/scheduler/async/TaskHistoryPruner.java 345cd8959045302fe3711e22396f5f7244a88c44 src/main/java/org/apache/aurora/scheduler/async/TaskScheduler.java e2ba8b8fe978a58d1edcd01963ea020e54529353 src/main/java/org/apache/aurora/scheduler/async/TaskThrottler.java ca6129c7e0225530336c88f91a1451892f2ce234 src/main/java/org/apache/aurora/scheduler/async/TaskTimeout.java 8217c512f31d4222e855218bee8d3172767d4ba4 src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java 9388657ccb904364e460ec612c3da562b8952d7e src/main/java/org/apache/aurora/scheduler/state/MaintenanceController.java 86440ebd30bfe96262f30769d74e50c21d86f1c2 src/main/java/org/apache/aurora/scheduler/state/StateManager.java 3a2fd279c2953d564b7fddabf31afda001bb3dfe src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java 6663555ef58541afb9b6634196f1c40aff94274c src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java 77db411f338416787011868fad58986b3d614a2d src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java b2b66acee9c0789f3660469d6d504b4510af5e79 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 f918d150fd14b71db5c1d0f501423a8f05e87f71 src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java 4673e8097dade38449278424d055ef92d0d1b275 src/test/java/org/apache/aurora/scheduler/async/PreemptorImplTest.java 59bfbcb9912a10c0e7fb28b6993cae7de8004ffe src/test/java/org/apache/aurora/scheduler/async/TaskHistoryPrunerTest.java 9682c89ee1b8f10a93d5e536359df4f034cc9eee src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerImplTest.java 0e8a98c60293b8b5c23039b3c480670ed54faeb1 src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java 41709023fa95e29e45143f52ead68900c5ca6642 src/test/java/org/apache/aurora/scheduler/async/TaskThrottlerTest.java a28e512afde584fb94ff7686d7a3e3fbb51f8b7b src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java 17295acc1e818ec59253dac93915efbe5c6f1d06 src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java b6b1bcbf8080eb7d1e7eca4a486cc063f28db75d src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java 4739909d1315e39b8225e4a718820f2f5965e3cc src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java c602cbdfc09bbc7179cd4a10602f11a4c89ef605 src/test/java/org/apache/aurora/scheduler/state/TaskAssignerImplTest.java 8d69ae97e77e271e01935722a9a4b2b2d1099273 src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java 5c9ea6cf4eb4d99d94f5d61e784dd7c9c480798c src/test/java/org/apache/aurora/scheduler/updater/AddTaskTest.java f9ed46f56bb11e9c158268c16f29557f3e99c84e src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java f36e34cc56e56c8268cafa0a2af0b15feb3187e8 Diff: https://reviews.apache.org/r/26899/diff/ Testing ------- ./gradlew build -Pq Thanks, Bill Farner --===============6306424302822503489==--