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 821E511349 for ; Wed, 17 Sep 2014 18:24:07 +0000 (UTC) Received: (qmail 41806 invoked by uid 500); 17 Sep 2014 18:24:07 -0000 Delivered-To: apmail-aurora-reviews-archive@aurora.apache.org Received: (qmail 41760 invoked by uid 500); 17 Sep 2014 18:24:07 -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 41749 invoked by uid 99); 17 Sep 2014 18:24:06 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 17 Sep 2014 18:24:06 +0000 X-ASF-Spam-Status: No, hits=-1998.5 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; Wed, 17 Sep 2014 18:24:05 +0000 Received: (qmail 41696 invoked by uid 99); 17 Sep 2014 18:23:45 -0000 Received: from reviews-vm.apache.org (HELO reviews.apache.org) (140.211.11.40) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 17 Sep 2014 18:23:45 +0000 Received: from reviews.apache.org (localhost [127.0.0.1]) by reviews.apache.org (Postfix) with ESMTP id 26A0E1DD7F9; Wed, 17 Sep 2014 18:23:43 +0000 (UTC) Content-Type: multipart/alternative; boundary="===============7218870919792659259==" MIME-Version: 1.0 Subject: Re: Review Request 25741: Remove INSTANCE_ADDED and INSTANCE_REMOVED states. From: "Bill Farner" To: "Maxim Khutornenko" , "David McLaughlin" Cc: "Bill Farner" , "Aurora" Date: Wed, 17 Sep 2014 18:23:43 -0000 Message-ID: <20140917182343.7800.90741@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/25741/ X-Sender: "Bill Farner" References: <20140917172109.7803.60153@reviews.apache.org> In-Reply-To: <20140917172109.7803.60153@reviews.apache.org> Reply-To: "Bill Farner" X-ReviewRequest-Repository: aurora X-Virus-Checked: Checked by ClamAV on apache.org --===============7218870919792659259== MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit > On Sept. 17, 2014, 5:21 p.m., David McLaughlin wrote: > > Does removing an instance take any significant amount of time? Can it fail? The concern is we'll be showing 100% progress but the update is still running when reducing the instance count. > > Bill Farner wrote: > Depends on your definition of significant. It does take a non-deterministic amount of time, since we transition to KILLING and await to see that the task is gone. It can _sort of_ fail, but not in a way that we will record an instance failure AFAICT. However, i think the behavior of ACTING -> FINISHED even in removal is nice as it matches two-step nature of the operation. > > David McLaughlin wrote: > I see, so the new behaviour is that in both scenarios the instance would end up in INSTANCE_UPDATED state? And then the client infers whether it was added or removed based on the instance count delta? Effectively, yes. The states communicate "acting" and "finished" along with direction (updating or rollback), and the caller has the information to determine what the outcome looks like (config change, added, removed). - Bill ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25741/#review53694 ----------------------------------------------------------- On Sept. 17, 2014, 5:15 p.m., Bill Farner wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25741/ > ----------------------------------------------------------- > > (Updated Sept. 17, 2014, 5:15 p.m.) > > > Review request for Aurora, David McLaughlin and Maxim Khutornenko. > > > Repository: aurora > > > Description > ------- > > Since we're changing fields in `JobUpdateConfiguration` (being renamed to `JobUpdateInstructions`) to represent the delta incurred by a job update (AURORA-718), i believe we can remove these two states, as addition/removal can be inferred from the delta. > > Furthermore, this allows the job updater's internal state machine to translate directly into the remaining `JobUpdateActions`, and hides details of the non-atomic actions necessary to move an instance from state A to state B. > > > Diffs > ----- > > src/main/resources/org/apache/aurora/scheduler/http/ui/js/services.js 5d6299f9de6eccd0f1332e11d57dfb910d956011 > src/main/thrift/org/apache/aurora/gen/api.thrift 3d0beeaed74aafcec0e24725f443e53a67f6c3a0 > src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java e09caa63bc0150d7109cb237e80b9efee441dded > src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java ca990e73d80e8456e71a97f0bdd0b6f4530d0135 > > Diff: https://reviews.apache.org/r/25741/diff/ > > > Testing > ------- > > ./gradlew build -Pq > > > Thanks, > > Bill Farner > > --===============7218870919792659259==--