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 89FD517B15 for ; Tue, 26 Jan 2016 14:57:45 +0000 (UTC) Received: (qmail 40191 invoked by uid 500); 26 Jan 2016 14:57:45 -0000 Delivered-To: apmail-aurora-reviews-archive@aurora.apache.org Received: (qmail 40143 invoked by uid 500); 26 Jan 2016 14:57:45 -0000 Mailing-List: contact reviews-help@aurora.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: reviews@aurora.apache.org Delivered-To: mailing list reviews@aurora.apache.org Received: (qmail 40120 invoked by uid 99); 26 Jan 2016 14:57:45 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 26 Jan 2016 14:57:45 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id C38D9288CEC; Tue, 26 Jan 2016 14:57:44 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============0585227080666278374==" MIME-Version: 1.0 Subject: Re: Review Request 42801: `TaskHistoryPruner` controls Lifecycle directly. From: Aurora ReviewBot To: Bill Farner , Zameer Manji Cc: Aurora , John Sirois , Aurora ReviewBot Date: Tue, 26 Jan 2016 14:57:44 -0000 Message-ID: <20160126145744.25059.98125@reviews.apache.org> X-ReviewBoard-URL: https://reviews.apache.org/ Auto-Submitted: auto-generated Sender: Aurora ReviewBot X-ReviewGroup: Aurora X-Auto-Response-Suppress: DR, RN, OOF, AutoReply X-ReviewRequest-URL: https://reviews.apache.org/r/42801/ X-Sender: Aurora ReviewBot References: <20160126145301.25059.74803@reviews.apache.org> In-Reply-To: <20160126145301.25059.74803@reviews.apache.org> Reply-To: Aurora ReviewBot X-ReviewRequest-Repository: aurora --===============0585227080666278374== 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/42801/#review116356 ----------------------------------------------------------- Can only test reviews with 1 parent, found 2: https://reviews.apache.org/api/review-requests/42639/ https://reviews.apache.org/api/review-requests/42332/ I will refresh this build result if you post a review containing "@ReviewBot retry" - Aurora ReviewBot On Jan. 26, 2016, 2:53 p.m., John Sirois wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/42801/ > ----------------------------------------------------------- > > (Updated Jan. 26, 2016, 2:53 p.m.) > > > Review request for Aurora, Bill Farner and Zameer Manji. > > > Bugs: AURORA-1593 > https://issues.apache.org/jira/browse/AURORA-1593 > > > Repository: aurora > > > Description > ------- > > This was the original idea in https://reviews.apache.org/r/42332. > > Mixing the active scheduler `Service` lifecycle with the `EventBus` > lifecycle proves tricky - prune events are fired before scheduler active > services are started. Instead of queueing up prune events to wait for > service start or re-engineering service / event bus interaction, returns > to the orignal behavior, manipulating the `Lifecycle` directly. > > Also kill a confusing unused EventSink discovered during analyis of all > pub-sub event sourcing that might interact with the `TaskHistoryPruner`. > > src/main/java/org/apache/aurora/GuavaUtils.java | 18 ------------------ > src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java | 5 ----- > src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java | 1 - > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java | 24 +++++++++++++----------- > src/test/java/org/apache/aurora/LifecycleShutdownListenerTest.java | 17 +++++++++++++++-- > src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java | 4 ---- > src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java | 24 ++++++++---------------- > 7 files changed, 36 insertions(+), 57 deletions(-) > > > Diffs > ----- > > src/main/java/org/apache/aurora/GuavaUtils.java 7d569e000c358e4f27397168843afdc0e9471ad6 > src/main/java/org/apache/aurora/scheduler/SchedulerLifecycle.java 5ba5e73464336014d5d49885a080dc98bebfeefb > src/main/java/org/apache/aurora/scheduler/pruning/PruningModule.java efdfbdaa4ea00469faf2c33a8bc12051685f87ca > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java 5441630522b3855a3b2036b5ff66fe980728ec68 > src/test/java/org/apache/aurora/LifecycleShutdownListenerTest.java d91dc277efebf658947382614f77aff15b3a251b > src/test/java/org/apache/aurora/scheduler/SchedulerLifecycleTest.java bab45670d66dfed23dd6e0339687166c9be44ba4 > src/test/java/org/apache/aurora/scheduler/pruning/TaskHistoryPrunerTest.java ffeee1bebdb403be9179a8160e4d9a01aaf1f56b > > Diff: https://reviews.apache.org/r/42801/diff/ > > > Testing > ------- > > Locally green: > ``` > ./gradlew -Pq build > ./src/test/sh/org/apache/aurora/e2e/test_end_to_end.sh > ``` > It's the latter - e2e (krb part) - that was the only automated testing > revealing the problem previously. > > > Thanks, > > John Sirois > > --===============0585227080666278374==--