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 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 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 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.,

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 30225: Modifying update controller to support heartbeats.

2015-02-10 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review71879 --- I like the shape of this, thanks for working through a few

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

2015-02-09 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review71729 --- Ship it! Pulse logic LGTM. - David McLaughlin On Feb. 7, 2015,

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

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

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

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

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

2015-02-05 Thread Maxim Khutornenko
On Feb. 5, 2015, 6:07 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 308 https://reviews.apache.org/r/30225/diff/5/?file=849498#file849498line308 Deferrment is not quite what i had in mind. I was thinking something

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

2015-02-05 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review71245 ---

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

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

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

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

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

2015-02-04 Thread Maxim Khutornenko
On Feb. 4, 2015, 11:34 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 273 https://reviews.apache.org/r/30225/diff/4/?file=848246#file848246line273 I think avoid acquisition of a write lock here is a good goal to aim

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

2015-02-04 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review71063 --- I suggest you skip to the big comment before paying attention to

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

2015-02-04 Thread Bill Farner
On Feb. 4, 2015, 11:34 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 273 https://reviews.apache.org/r/30225/diff/4/?file=848246#file848246line273 I think avoid acquisition of a write lock here is a good goal to aim

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

2015-02-04 Thread Maxim Khutornenko
On Feb. 4, 2015, 11:34 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 273 https://reviews.apache.org/r/30225/diff/4/?file=848246#file848246line273 I think avoid acquisition of a write lock here is a good goal to aim

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

2015-02-04 Thread Bill Farner
On Feb. 4, 2015, 11:34 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 273 https://reviews.apache.org/r/30225/diff/4/?file=848246#file848246line273 I think avoid acquisition of a write lock here is a good goal to aim

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

2015-02-04 Thread Maxim Khutornenko
On Feb. 4, 2015, 11:34 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 273 https://reviews.apache.org/r/30225/diff/4/?file=848246#file848246line273 I think avoid acquisition of a write lock here is a good goal to aim

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

2015-02-04 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review71114 ---

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

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

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

2015-02-04 Thread David McLaughlin
On Feb. 4, 2015, 11:34 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 273 https://reviews.apache.org/r/30225/diff/4/?file=848246#file848246line273 I think avoid acquisition of a write lock here is a good goal to aim

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

2015-02-04 Thread Maxim Khutornenko
On Feb. 4, 2015, 11:34 p.m., Bill Farner wrote: api/src/main/thrift/org/apache/aurora/gen/api.thrift, line 565 https://reviews.apache.org/r/30225/diff/4/?file=848240#file848240line565 Maybe s/BLOCKED/AWAITING_PULSE/? That would at least self-document and avoid

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

2015-02-04 Thread Maxim Khutornenko
On Feb. 5, 2015, 1:15 a.m., David McLaughlin wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 293 https://reviews.apache.org/r/30225/diff/4/?file=848246#file848246line293 I think we still want to update the last pulse time even if it's

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

2015-02-04 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/ --- (Updated Feb. 5, 2015, 2:34 a.m.) Review request for Aurora, David McLaughlin,

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

2015-02-03 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review70913 --- Master (8bcb2ba) is red with this patch.

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

2015-02-03 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review70904 --- This patch does not apply cleanly on master (9fe6d54), do you need

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

2015-01-29 Thread David McLaughlin
On Jan. 28, 2015, 7:41 p.m., David McLaughlin wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, lines 259-263 https://reviews.apache.org/r/30225/diff/1/?file=832014#file832014line259 I am unsure why this is being called inside pulse. Once

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

2015-01-28 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review70076 --- Ship it! Looks reasonable to me (caveat being I'm not super

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

2015-01-28 Thread Maxim Khutornenko
On Jan. 28, 2015, 7:41 p.m., David McLaughlin wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, lines 259-263 https://reviews.apache.org/r/30225/diff/1/?file=832014#file832014line259 I am unsure why this is being called inside pulse. Once

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

2015-01-28 Thread David McLaughlin
On Jan. 28, 2015, 7:41 p.m., David McLaughlin wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, lines 259-263 https://reviews.apache.org/r/30225/diff/1/?file=832014#file832014line259 I am unsure why this is being called inside pulse. Once

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

2015-01-28 Thread David McLaughlin
On Jan. 28, 2015, 7:41 p.m., David McLaughlin wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, lines 259-263 https://reviews.apache.org/r/30225/diff/1/?file=832014#file832014line259 I am unsure why this is being called inside pulse. Once

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

2015-01-28 Thread Maxim Khutornenko
On Jan. 28, 2015, 7:41 p.m., David McLaughlin wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, lines 259-263 https://reviews.apache.org/r/30225/diff/1/?file=832014#file832014line259 I am unsure why this is being called inside pulse. Once

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

2015-01-28 Thread David McLaughlin
On Jan. 28, 2015, 7:41 p.m., David McLaughlin wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, lines 259-263 https://reviews.apache.org/r/30225/diff/1/?file=832014#file832014line259 I am unsure why this is being called inside pulse. Once

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

2015-01-28 Thread David McLaughlin
On Jan. 28, 2015, 7:41 p.m., David McLaughlin wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, lines 259-263 https://reviews.apache.org/r/30225/diff/1/?file=832014#file832014line259 I am unsure why this is being called inside pulse. Once

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

2015-01-28 Thread Maxim Khutornenko
On Jan. 28, 2015, 7:41 p.m., David McLaughlin wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, lines 259-263 https://reviews.apache.org/r/30225/diff/1/?file=832014#file832014line259 I am unsure why this is being called inside pulse. Once

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

2015-01-28 Thread David McLaughlin
On Jan. 27, 2015, 5:49 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 227 https://reviews.apache.org/r/30225/diff/1/?file=832014#file832014line227 Given how sensitive we are to storage lock contention, is there any

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

2015-01-28 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review70058 ---

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

2015-01-27 Thread Maxim Khutornenko
On Jan. 27, 2015, 5:49 p.m., Joshua Cohen wrote: src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, line 227 https://reviews.apache.org/r/30225/diff/1/?file=832014#file832014line227 Given how sensitive we are to storage lock contention, is there any

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

2015-01-27 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review69825 --- Ping. - Maxim Khutornenko On Jan. 23, 2015, 8:37 p.m., Maxim

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

2015-01-27 Thread Joshua Cohen
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review69834 ---

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

2015-01-23 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30225/#review69460 --- Master (3fa004b) is red with this patch.