Re: Review Request 56723: Add best effort pulse timestamp recovery.

2017-02-15 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56723/#review165806 --- Ship it! Master (9ea8979) is green with this patch.

Re: Review Request 56723: Add best effort pulse timestamp recovery.

2017-02-15 Thread Zameer Manji
> On Feb. 15, 2017, 6:05 p.m., Santhosh Kumar Shanmugham wrote: > > src/main/java/org/apache/aurora/scheduler/updater/JobUpdateControllerImpl.java, > > line 288 > > > > > > Do we really need an immutable copy here?

Re: Review Request 56723: Add best effort pulse timestamp recovery.

2017-02-15 Thread Santhosh Kumar Shanmugham
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56723/#review165801 --- Fix it, then Ship it! LGTM. Minor comment.

Re: Review Request 56723: Add best effort pulse timestamp recovery.

2017-02-15 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56723/ --- (Updated Feb. 15, 2017, 6 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 56723: Add best effort pulse timestamp recovery.

2017-02-15 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56723/ --- (Updated Feb. 15, 2017, 6 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 56723: Add best effort pulse timestamp recovery.

2017-02-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56723/#review165799 --- Ship it!

Re: Review Request 56723: Add best effort pulse timestamp recovery.

2017-02-15 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56723/ --- (Updated Feb. 15, 2017, 5:53 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 56723: Add best effort pulse timestamp recovery.

2017-02-15 Thread Zameer Manji
> On Feb. 15, 2017, 2:32 p.m., David McLaughlin wrote: > > A comment on the overall approach: in a healthy system, there is usually > > only one transition from AWAITING_PULSE -> ROLLING_FORWARD. Meanwhile, > > there will have been regular pulses that keep the update in ROLLING_FORWARD > >

Re: Review Request 56691: [Patch 2/2] RFC for 2nd patch implements OfferReconciler for dynamic reservations proposal

2017-02-15 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56691/#review165783 --- This patch does not apply cleanly against RB#56690 (9ea8979), do

Re: Review Request 56691: [Patch 2/2] RFC for 2nd patch implements OfferReconciler for dynamic reservations proposal

2017-02-15 Thread Dmitriy Shirchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56691/ --- (Updated Feb. 16, 2017, 12:01 a.m.) Review request for Aurora, Mehrdad

Re: Review Request 56723: Add best effort pulse timestamp recovery.

2017-02-15 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56723/#review165781 --- Master (9ea8979) is red with this patch.

Re: Review Request 56723: Add best effort pulse timestamp recovery.

2017-02-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56723/#review165775 --- A comment on the overall approach: in a healthy system, there is

Review Request 56723: Add best effort pulse timestamp recovery.

2017-02-15 Thread Zameer Manji
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56723/ --- Review request for Aurora, David McLaughlin and Santhosh Kumar Shanmugham.

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-15 Thread Mehrdad Nurolahzade
> On Feb. 15, 2017, 9:40 a.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > lines 150-151 > > > > > > Can you explain the reasoning behind doing this in

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-15 Thread David McLaughlin
> On Feb. 15, 2017, 5:40 p.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > lines 150-151 > > > > > > Can you explain the reasoning behind doing this in

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-15 Thread Mehrdad Nurolahzade
> On Feb. 15, 2017, 9:40 a.m., David McLaughlin wrote: > > src/main/java/org/apache/aurora/scheduler/pruning/TaskHistoryPruner.java, > > lines 150-151 > > > > > > Can you explain the reasoning behind doing this in

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-15 Thread David McLaughlin
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/#review165733 ---

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-15 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/#review165726 --- Ship it! Master (9ea8979) is green with this patch.

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-15 Thread Mehrdad Nurolahzade
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56575/ --- (Updated Feb. 15, 2017, 9:07 a.m.) Review request for Aurora, David

Re: Review Request 56575: AURORA-1837 Improve task history pruning

2017-02-15 Thread Mehrdad Nurolahzade
> On Feb. 14, 2017, 4:02 p.m., Mehrdad Nurolahzade wrote: > > Looking at it as is, I'm not sure if there is much value to be gained from > > pushing this down to `TaskStore`. > > Do you see any value in pursuing this idea any further? Or shall I restore > > it to previous state? > > Zameer

Re: Review Request 56395: Change Resource Validation in ConfigurationManager so that it validates the Resource Set instead of deprecated fields

2017-02-15 Thread Nicolás Donatucci
> On Feb. 14, 2017, 7:36 a.m., Reza Motamedi wrote: > > src/main/java/org/apache/aurora/scheduler/resources/ResourceManager.java, > > line 153 > > > > > > I don't get why this method name has package path in it? Is

Re: Review Request 55951: Use --launch_info when invoking MesosContainerizer.

2017-02-15 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/55951/#review165662 --- Master (9ea8979) is red with this patch.

Re: Review Request 56691: [Patch 2/2] RFC for 2nd patch implements OfferReconciler for dynamic reservations proposal

2017-02-15 Thread Dmitriy Shirchenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/56691/ --- (Updated Feb. 15, 2017, 9:09 a.m.) Review request for Aurora, Mehrdad