Re: Review Request 26232: Implementing update history pruner.

2014-10-02 Thread Maxim Khutornenko
On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java, line 85 https://reviews.apache.org/r/26232/diff/2/?file=710417#file710417line85 I'm always nervous when important behavior is embedded in something

Re: Review Request 26232: Implementing update history pruner.

2014-10-02 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/ --- (Updated Oct. 2, 2014, 7:40 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 26232: Implementing update history pruner.

2014-10-02 Thread Bill Farner
On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line 147 https://reviews.apache.org/r/26232/diff/2/?file=710418#file710418line147 Keep the value rich as far down as you can, to mitigate accidental misuse:

Re: Review Request 26232: Implementing update history pruner.

2014-10-02 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/ --- (Updated Oct. 2, 2014, 7:54 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 26232: Implementing update history pruner.

2014-10-02 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/#review55266 --- Ship it!

Re: Review Request 26232: Implementing update history pruner.

2014-10-02 Thread Maxim Khutornenko
On Oct. 2, 2014, 10:39 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java, line 89 https://reviews.apache.org/r/26232/diff/4/?file=711547#file711547line89 if (!prunedUpdates.isEmpty()) { LOG.info(...); } I

Re: Review Request 26232: Implementing update history pruner.

2014-10-02 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/ --- (Updated Oct. 2, 2014, 10:51 p.m.) Review request for Aurora, David McLaughlin

Re: Review Request 26232: Implementing update history pruner.

2014-10-02 Thread Maxim Khutornenko
On Oct. 2, 2014, 11:16 p.m., David McLaughlin wrote: src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml, lines 468-490 https://reviews.apache.org/r/26232/diff/5/?file=713015#file713015line468 It'd be nice if this didn't just blanket delete all

Review Request 26232: Implementing update history pruner.

2014-10-01 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/ --- Review request for Aurora, David McLaughlin and Bill Farner. Bugs: AURORA-743

Re: Review Request 26232: Implementing update history pruner.

2014-10-01 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/#review55088 --- src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java

Re: Review Request 26232: Implementing update history pruner.

2014-10-01 Thread Maxim Khutornenko
On Oct. 1, 2014, 5:42 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 161 https://reviews.apache.org/r/26232/diff/1/?file=710137#file710137line161 We also need time-based retention. How about 1 month by default? Thought about that

Re: Review Request 26232: Implementing update history pruner.

2014-10-01 Thread Bill Farner
On Oct. 1, 2014, 5:42 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 161 https://reviews.apache.org/r/26232/diff/1/?file=710137#file710137line161 We also need time-based retention. How about 1 month by default? Maxim Khutornenko

Re: Review Request 26232: Implementing update history pruner.

2014-10-01 Thread David McLaughlin
On Oct. 1, 2014, 5:42 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 161 https://reviews.apache.org/r/26232/diff/1/?file=710137#file710137line161 We also need time-based retention. How about 1 month by default? Maxim Khutornenko

Re: Review Request 26232: Implementing update history pruner.

2014-10-01 Thread Bill Farner
On Oct. 1, 2014, 5:42 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 161 https://reviews.apache.org/r/26232/diff/1/?file=710137#file710137line161 We also need time-based retention. How about 1 month by default? Maxim Khutornenko

Re: Review Request 26232: Implementing update history pruner.

2014-10-01 Thread Maxim Khutornenko
On Oct. 1, 2014, 5:42 p.m., Bill Farner wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java, line 140 https://reviews.apache.org/r/26232/diff/1/?file=710141#file710141line140 I'm not sure how i feel about this. On one hand, it's nice that SQL

Re: Review Request 26232: Implementing update history pruner.

2014-10-01 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/26232/#review55186 ---