Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Maxim Khutornenko
On Feb. 11, 2015, 1:03 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, lines 282-292 https://reviews.apache.org/r/30225/diff/6/?file=853467#file853467line282 There's a race on access to `pulseStates`. Terminating an update

Re: Review Request 30818: Support separate routes for job controller tabs.

2015-02-11 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30818/#review71999 --- src/main/resources/scheduler/assets/js/controllers.js

Re: Review Request 30818: Support separate routes for job controller tabs.

2015-02-11 Thread David McLaughlin
On Feb. 11, 2015, 6:58 p.m., David McLaughlin wrote: src/main/resources/scheduler/assets/js/controllers.js, line 338 https://reviews.apache.org/r/30818/diff/2/?file=859882#file859882line338 You should never have to do this with routes in Angular. I think this is a code smell from

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 7:19 p.m.) Review request for Aurora, David

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72014 --- Ship it!

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72022 --- Ship it! Master (7b531e9) is green with this patch.

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72016 --- Ship it! Ship It! - Bill Farner On Feb. 11, 2015, 7:19 p.m.,

Re: Review Request 28617: Implemented offer filtering for tasks with static vetoes.

2015-02-11 Thread Maxim Khutornenko
On Feb. 5, 2015, 12:01 a.m., Bill Farner wrote: Can you see any opportunity to break this diff apart? As it stands i'm having a hard time giving a thoughtful review. Perhaps you can start by introducing the `Assignment` class? Maxim Khutornenko wrote: I'd really prefer keeping

Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.

2015-02-11 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72068 --- Ship it! FWIW I do agree with Bill's comment that minimum pulse

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 11, 2015, 10:01 p.m.) Review request for Aurora, David

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Maxim Khutornenko
On Feb. 11, 2015, 8:46 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 276 https://reviews.apache.org/r/30225/diff/7/?file=860914#file860914line276 can drop the else? Sure. - Maxim

Re: Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/ --- (Updated Feb. 11, 2015, 10:52 p.m.) Review request for Aurora, Kevin Sweeney

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Maxim Khutornenko
On Feb. 11, 2015, 2:13 a.m., Bill Farner wrote: api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 630 https://reviews.apache.org/r/30325/diff/4/?file=859286#file859286line630 s/Key// Done and done. - Maxim ---

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Maxim Khutornenko
On Feb. 10, 2015, 11:57 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, lines 1609-1618 https://reviews.apache.org/r/30325/diff/4/?file=859288#file859288line1609 s/final// Done. - Maxim

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/ --- (Updated Feb. 11, 2015, 11:51 p.m.) Review request for Aurora, David

Review Request 30895: Offer filtering for static vetoes. Part 4 of 4: Benchmarks.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30895/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Repository: aurora

Review Request 30890: Offer filtering for static vetoes. Part 2 of 4: Veto groups.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30890/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72028 --- Ship it! Ship It! - Joshua Cohen On Feb. 11, 2015, 10:01 p.m.,

Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909

Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/ --- Review request for Aurora, Kevin Sweeney and Bill Farner. Bugs: AURORA-909

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Maxim Khutornenko
On Feb. 10, 2015, 7:28 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, lines 1609-1618 https://reviews.apache.org/r/30325/diff/4/?file=859288#file859288line1609 I don't know how you feel about the need for a Supplier, but

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/ --- (Updated Feb. 11, 2015, 11:35 p.m.) Review request for Aurora, David

Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.

2015-02-11 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72065 --- Ship it! pending clean test run from review bot. - Joshua Cohen

Re: Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.

2015-02-11 Thread Maxim Khutornenko
On Feb. 11, 2015, 9:59 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/state/TaskAssigner.java, line 64 https://reviews.apache.org/r/30888/diff/1/?file=861003#file861003line64 fits on one line ditto elsewhere in this file Fixed. On Feb. 11, 2015,

Re: Review Request 30890: Offer filtering for static vetoes. Part 2 of 4: Veto groups.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30890/#review72058 --- Ship it! Master (7b531e9) is green with this patch.

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/#review72059 --- This patch does not apply cleanly on master (7b531e9), do you need

Re: Review Request 30890: Offer filtering for static vetoes. Part 2 of 4: Veto groups.

2015-02-11 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30890/#review72061 --- src/main/java/org/apache/aurora/scheduler/TaskVars.java

Re: Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.

2015-02-11 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/#review72026 --- Ship it!

Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.

2015-02-11 Thread Stephan Erb
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/#review72030 --- src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java

Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.

2015-02-11 Thread Maxim Khutornenko
On Feb. 11, 2015, 11:59 p.m., David McLaughlin wrote: FWIW I do agree with Bill's comment that minimum pulse time should be enforced in scheduler rather than here. I am +1 on the scheduler check as complementary to the client when/if there is a need for it. I am -1 on using scheduler as

Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30891/#review72036 --- This patch does not apply cleanly on master (7b531e9), do you need

Re: Review Request 30225: Modifying update controller to support heartbeats.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review72041 --- Ship it! Master (7b531e9) is green with this patch.

Re: Review Request 30891: Offer filtering for static vetoes. Part 3 of 4: Offer filtering.

2015-02-11 Thread Maxim Khutornenko
On Feb. 11, 2015, 10:22 p.m., Stephan Erb wrote: src/main/java/org/apache/aurora/scheduler/async/OfferQueue.java, line 316 https://reviews.apache.org/r/30891/diff/1/?file=861043#file861043line316 The cohesion of the HostOffer class seems to be rather low. The new field

Re: Review Request 30888: Offer filtering for static vetoes. Part 1 of 4: TaskAssigner.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30888/#review72043 --- Master (7b531e9) is red with this patch.

Re: Review Request 30895: Offer filtering for static vetoes. Part 4 of 4: Benchmarks.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30895/#review72048 --- Ship it! Master (7b531e9) is green with this patch.

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Maxim Khutornenko
On Feb. 11, 2015, 11:44 p.m., David McLaughlin wrote: src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java, lines 1596-1603 https://reviews.apache.org/r/30325/diff/4/?file=859288#file859288line1596 It seems strange that the UPDATE_COORDINATOR can pulse

Re: Review Request 30325: Implementing pulseJobUpdate RPC.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30325/#review72071 --- Ship it! Master (b8f71fb) is green with this patch.

Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72091 --- @ReviewBot retry - Maxim Khutornenko On Feb. 11, 2015, 1:19

Re: Review Request 30461: Adding pulse_interval_secs into client UpdateConfig.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30461/#review72098 --- Master (61e6c35) is red with this patch.

Review Request 30913: Adding UPDATE_COORDINATOR role access into pause/resume/abort RPCs

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30913/ --- Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-1119

Review Request 30915: Increase findbugs heap size.

2015-02-11 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30915/ --- Review request for Aurora and Bill Farner. Repository: aurora Description

Re: Review Request 30710: add mesos role feature

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30710/#review72113 --- Ship it! Master (61e6c35) is green with this patch.

Re: Review Request 30710: add mesos role feature

2015-02-11 Thread lozh...@ebay.com zhang
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30710/ --- (Updated Feb. 12, 2015, 6:12 a.m.) Review request for Aurora, Joshua Cohen and

Re: Review Request 30915: Increase findbugs heap size.

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30915/#review72103 --- Ship it! Master (61e6c35) is green with this patch.

Re: Review Request 30913: Adding UPDATE_COORDINATOR role access into pause/resume/abort RPCs

2015-02-11 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30913/#review72104 --- Ship it! Master (61e6c35) is green with this patch.