Re: Review Request 26232: Implementing update history pruner.

[David McLaughlin]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55322
---

Ship it!


Ship It!

- David McLaughlin


On Oct. 2, 2014, 10:51 p.m., Maxim Khutornenko wrote:
> 
> ---
> 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 and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  2a81a94f99f242bbe400d8428ad1f1ce38a06a86 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  dbf0badbfcc19f40d9b9eeec22b7024d95a02884 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 6758b7b56e77882c67be2e39481ff76893ad1610 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[Bill Farner]

> 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
> > 
> >
> > It'd be nice if this didn't just blanket delete all old updates, 
> > especially for active jobs. There are probably a certain class of 
> > service/cron that are rarely touched and it would be nice to keep around 
> > that change history. Would it add too much complexity to try and solve this?
> 
> Maxim Khutornenko wrote:
> Well, that would require active task queries and implementation leaking 
> outside DB layer. I'd rather not attempt anything like that until we move 
> TaskStore into SQL.

Let's start with this policy and revisit.  Maxim - mind dropping a TODO in 
DbJobUpdateStore to consider retaining at least the most recent update for all 
jobs?


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55272
---


On Oct. 2, 2014, 10:51 p.m., Maxim Khutornenko wrote:
> 
> ---
> 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 and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  2a81a94f99f242bbe400d8428ad1f1ce38a06a86 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  dbf0badbfcc19f40d9b9eeec22b7024d95a02884 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 6758b7b56e77882c67be2e39481ff76893ad1610 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[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
> > 
> >
> > It'd be nice if this didn't just blanket delete all old updates, 
> > especially for active jobs. There are probably a certain class of 
> > service/cron that are rarely touched and it would be nice to keep around 
> > that change history. Would it add too much complexity to try and solve this?

Well, that would require active task queries and implementation leaking outside 
DB layer. I'd rather not attempt anything like that until we move TaskStore 
into SQL.


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55272
---


On Oct. 2, 2014, 10:51 p.m., Maxim Khutornenko wrote:
> 
> ---
> 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 and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  2a81a94f99f242bbe400d8428ad1f1ce38a06a86 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  dbf0badbfcc19f40d9b9eeec22b7024d95a02884 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 6758b7b56e77882c67be2e39481ff76893ad1610 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[David McLaughlin]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55272
---



src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml


It'd be nice if this didn't just blanket delete all old updates, especially 
for active jobs. There are probably a certain class of service/cron that are 
rarely touched and it would be nice to keep around that change history. Would 
it add too much complexity to try and solve this?


- David McLaughlin


On Oct. 2, 2014, 10:51 p.m., Maxim Khutornenko wrote:
> 
> ---
> 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 and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  2a81a94f99f242bbe400d8428ad1f1ce38a06a86 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  dbf0badbfcc19f40d9b9eeec22b7024d95a02884 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 6758b7b56e77882c67be2e39481ff76893ad1610 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[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 and Bill Farner.


Changes
---

CR comments.


Bugs: AURORA-743
https://issues.apache.org/jira/browse/AURORA-743


Repository: aurora


Description
---

The pruner runs on periodic basis and trims completed updates up to the 
guranteed per job retention count.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
  src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
  src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
c3abffe575e801cebec3572cf4aceac83a238b55 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
  
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
 c583e085e0458835d51ebf740a3b5f01b428bb25 
  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
66c91644677e7176ccf53dcfcf29a6792ec398bc 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 2a81a94f99f242bbe400d8428ad1f1ce38a06a86 
  src/main/thrift/org/apache/aurora/gen/storage.thrift 
7e502450f06abb449d06af09cc59185c6a9a2963 
  src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
  
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
dbf0badbfcc19f40d9b9eeec22b7024d95a02884 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
68df0d542e41438c0844f76fc5b9ec6996a00e8d 
  src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
e35fe23f023f5accfb2270a3634c91bec657 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
6758b7b56e77882c67be2e39481ff76893ad1610 

Diff: https://reviews.apache.org/r/26232/diff/


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 26232: Implementing update history pruner.

[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
> > 
> >
> > if (!prunedUpdates.isEmpty()) {
> >   LOG.info(...);
> > }

I actually want to log in either case for better transparency. Modified to 
support both cases.


> On Oct. 2, 2014, 10:39 p.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java,
> >  line 75
> > 
> >
> > line break above

Done.


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55266
---


On Oct. 2, 2014, 7:54 p.m., Maxim Khutornenko wrote:
> 
> ---
> 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 and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 7254588b55df9a12217c8ec172abc5976892497e 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55266
---

Ship it!



src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java


if (!prunedUpdates.isEmpty()) {
  LOG.info(...);
}



src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java


line break above


- Bill Farner


On Oct. 2, 2014, 7:54 p.m., Maxim Khutornenko wrote:
> 
> ---
> 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 and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 7254588b55df9a12217c8ec172abc5976892497e 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[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 and Bill Farner.


Changes
---

CR comments.


Bugs: AURORA-743
https://issues.apache.org/jira/browse/AURORA-743


Repository: aurora


Description
---

The pruner runs on periodic basis and trims completed updates up to the 
guranteed per job retention count.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
  src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
  src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
c3abffe575e801cebec3572cf4aceac83a238b55 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
  
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
 c583e085e0458835d51ebf740a3b5f01b428bb25 
  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
66c91644677e7176ccf53dcfcf29a6792ec398bc 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
  src/main/thrift/org/apache/aurora/gen/storage.thrift 
7e502450f06abb449d06af09cc59185c6a9a2963 
  src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
  
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
68df0d542e41438c0844f76fc5b9ec6996a00e8d 
  src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
e35fe23f023f5accfb2270a3634c91bec657 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
7254588b55df9a12217c8ec172abc5976892497e 

Diff: https://reviews.apache.org/r/26232/diff/


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 26232: Implementing update history pruner.

[Maxim Khutornenko]

> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line 
> > 147
> > 
> >
> > Keep the value rich as far down as you can, to mitigate accidental 
> > misuse:
> > Amount historyPruneThreshold
> 
> Maxim Khutornenko wrote:
> Disagree. I don't really like Amount -> long ->Amount -> long dance that 
> would require. The Amount is converted into long only once now, converted 
> into an absolute timestamp and passed down to SQL to compare against.
> 
> Bill Farner wrote:
> I'm actually suggesting you avoid the dance completely.  Keep 
> Amount in the settings object, and only translate it to a long 
> when you need a long.  The only argument i can see against this is 
> performance, which doesn't hold here.

Ah, in that case the comment was misplaced as I was reading that you wanted to 
make JobUpdateStore API Amount-aware :)


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55186
---


On Oct. 2, 2014, 7:40 p.m., Maxim Khutornenko wrote:
> 
> ---
> 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 and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 7254588b55df9a12217c8ec172abc5976892497e 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[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
> > 
> >
> > Keep the value rich as far down as you can, to mitigate accidental 
> > misuse:
> > Amount historyPruneThreshold
> 
> Maxim Khutornenko wrote:
> Disagree. I don't really like Amount -> long ->Amount -> long dance that 
> would require. The Amount is converted into long only once now, converted 
> into an absolute timestamp and passed down to SQL to compare against.

I'm actually suggesting you avoid the dance completely.  Keep Amount in the settings object, and only translate it to a long when you need a 
long.  The only argument i can see against this is performance, which doesn't 
hold here.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55186
---


On Oct. 2, 2014, 7:40 p.m., Maxim Khutornenko wrote:
> 
> ---
> 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 and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
>   src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
> 68df0d542e41438c0844f76fc5b9ec6996a00e8d 
>   
> src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
> e35fe23f023f5accfb2270a3634c91bec657 
>   src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
> 7254588b55df9a12217c8ec172abc5976892497e 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[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 and Bill Farner.


Changes
---

CR comments.


Bugs: AURORA-743
https://issues.apache.org/jira/browse/AURORA-743


Repository: aurora


Description
---

The pruner runs on periodic basis and trims completed updates up to the 
guranteed per job retention count.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
  src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
  src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
c3abffe575e801cebec3572cf4aceac83a238b55 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
  
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
 c583e085e0458835d51ebf740a3b5f01b428bb25 
  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
66c91644677e7176ccf53dcfcf29a6792ec398bc 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
  src/main/thrift/org/apache/aurora/gen/storage.thrift 
7e502450f06abb449d06af09cc59185c6a9a2963 
  src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
  
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
68df0d542e41438c0844f76fc5b9ec6996a00e8d 
  src/test/java/org/apache/aurora/scheduler/updater/FakeScheduledExecutor.java 
e35fe23f023f5accfb2270a3634c91bec657 
  src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java 
7254588b55df9a12217c8ec172abc5976892497e 

Diff: https://reviews.apache.org/r/26232/diff/


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 26232: Implementing update history pruner.

[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
> > 
> >
> > I'm always nervous when important behavior is embedded in something 
> > seemingly-less-important like logging.  Can you extract a variable to 
> > separate the two?

Kind of redundant but sure, done.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line 
> > 140
> > 
> >
> > "...last completed updates that completed less than {@code 
> > historyPruneThreshold} ago.."

Done.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java, line 
> > 147
> > 
> >
> > Keep the value rich as far down as you can, to mitigate accidental 
> > misuse:
> > Amount historyPruneThreshold

Disagree. I don't really like Amount -> long ->Amount -> long dance that would 
require. The Amount is converted into long only once now, converted into an 
absolute timestamp and passed down to SQL to compare against.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java, 
> > line 141
> > 
> >
> > s/result/pruned/

Sure.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java, 
> > line 143
> > 
> >
> > How would you feel about making the two pruning goals distinct at the 
> > mapper level?  Does that simplfiy anything?
> > 
> > - get UUIDs of all updates older than pruneThreshold
> > - get UUIDs of the the last >retainCount updates for each job
> > - delete above UUIDs.
> > 
> > I think i would find that easier to follow, at least.

This approach would require an extra SQL call (step 1) and potentially a lot 
more SQL calls in step 2 as we would now deal with raw job keys not 
pre-filtered for processing. The current algorithm is optimized to be less 
SQL-chatty and short circuits if there are no job keys to process:
- get job keys that have updates to be deleted (older than pruneThreshold 
and/or count > retainCount)
- for every key above get update UUIDs and delete them.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java,
> >  line 109
> > 
> >
> > Avoid repeating the method signature in text:
> > s/Set of u/U/

Done.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java,
> >  line 121
> > 
> >
> > How about `getPruneCandidates`?

I don't think it will be clear enough as we are selecting job keys rather then 
UUIDs here. How about selectJobKeysForPruning?


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java,
> >  line 320
> > 
> >
> > Why this rather than an explicit delete record for the affected update 
> > UUIDs?

I thought about that but decided to stick with the current solution:
- less code to maintain: deleting a set of UUIDs would require a new 
JobUpdateStore API only used by the LogStorage
- less data to store: persisting a bunch of pruned UUIDs seems redundant where 
a a single prune call restores the consistency much more effectively.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java,
> >  line 318
> > 
> >
> > If this comment remains, please elaborate on why it is not strictly 
> > necessary. (i know, but a future developer might not.)

Done.


> On Oct. 2, 2014, 2:54 a.m., Bill Farner wrote:
> > src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java,
> >  line 45
> > 
> >
> > Can you take a stab at using FakeScheduledExecutor instead of a real 
> > thread?  I don't insist on it, but i'd like to see if that pattern works 
> > out in other situations.
> > 
> > You'll have to modify FakeScheduledExecutor a bit to add support for 
> > `scheduleAtFixedRate`, but at that point all you should have to do in the 
> > test after `repla

Re: Review Request 26232: Implementing update history pruner.

[Bill Farner]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55186
---



src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java


I'm always nervous when important behavior is embedded in something 
seemingly-less-important like logging.  Can you extract a variable to separate 
the two?



src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java


"...last completed updates that completed less than {@code 
historyPruneThreshold} ago.."



src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java


Keep the value rich as far down as you can, to mitigate accidental misuse:
Amount historyPruneThreshold



src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java


s/result/pruned/



src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java


How would you feel about making the two pruning goals distinct at the 
mapper level?  Does that simplfiy anything?

- get UUIDs of all updates older than pruneThreshold
- get UUIDs of the the last >retainCount updates for each job
- delete above UUIDs.

I think i would find that easier to follow, at least.



src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java


Avoid repeating the method signature in text:
s/Set of u/U/



src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java


s/Set of j/J/



src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java


How about `getPruneCandidates`?



src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java


If this comment remains, please elaborate on why it is not strictly 
necessary. (i know, but a future developer might not.)



src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java


Why this rather than an explicit delete record for the affected update 
UUIDs?



src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java


Can you take a stab at using FakeScheduledExecutor instead of a real 
thread?  I don't insist on it, but i'd like to see if that pattern works out in 
other situations.

You'll have to modify FakeScheduledExecutor a bit to add support for 
`scheduleAtFixedRate`, but at that point all you should have to do in the test 
after `replay()` is

clock.advance(pruneInterval);


- Bill Farner


On Oct. 2, 2014, 1:23 a.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 2, 2014, 1:23 a.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/main/thrift/org/apache/aurora/gen/storage.thrift 
> 7e502450f06abb449d06af09cc59185c6a9a2963 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b

Re: Review Request 26232: Implementing update history pruner.

[Maxim Khutornenko]

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/
---

(Updated Oct. 2, 2014, 1:23 a.m.)


Review request for Aurora, David McLaughlin and Bill Farner.


Changes
---

CR comments and log store support.


Bugs: AURORA-743
https://issues.apache.org/jira/browse/AURORA-743


Repository: aurora


Description
---

The pruner runs on periodic basis and trims completed updates up to the 
guranteed per job retention count.


Diffs (updated)
-

  src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
  src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
  src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
PRE-CREATION 
  src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
c3abffe575e801cebec3572cf4aceac83a238b55 
  src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
  
src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
 c583e085e0458835d51ebf740a3b5f01b428bb25 
  src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
66c91644677e7176ccf53dcfcf29a6792ec398bc 
  
src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
 631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
  src/main/thrift/org/apache/aurora/gen/storage.thrift 
7e502450f06abb449d06af09cc59185c6a9a2963 
  src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
  
src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java 
PRE-CREATION 
  
src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java 
1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
  src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java 
68df0d542e41438c0844f76fc5b9ec6996a00e8d 

Diff: https://reviews.apache.org/r/26232/diff/


Testing
---

./gradlew -Pq build


Thanks,

Maxim Khutornenko

Re: Review Request 26232: Implementing update history pruner.

[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
> > 
> >
> > I'm not sure how i feel about this.  On one hand, it's nice that SQL 
> > handles a lot of the complexity.  On the other, it would be _really_ nice 
> > to see deleted job update UUIDs in the log.  Perhaps this function can 
> > return some details about the deleted updates?

I have refactored to return a set of IDs and log them in the pruner.


> On Oct. 1, 2014, 5:42 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java,
> >  line 63
> > 
> >
> > Nit: starting this with "Pruning completed" breaks my brain a little.  
> > How about "Pruning job update history."

Reworded.


> On Oct. 1, 2014, 5:42 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java,
> >  line 43
> > 
> >
> > maxUpdatesPerJob?

Renamed.


> On Oct. 1, 2014, 5:42 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 324
> > 
> >
> > Does this work?  I think something needs to 'pull' on the binding (by 
> > injecting) for anything to happen.

Not really. Fixed.


> On Oct. 1, 2014, 5:42 p.m., Bill Farner wrote:
> > src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java, line 161
> > 
> >
> > We also need time-based retention.  How about 1 month by default?
> 
> Maxim Khutornenko wrote:
> Thought about that but was not sure the extra complexity is worth it. 
> Also, don't we want to show update history in the UI regardless of how old it 
> is?
> 
> Bill Farner wrote:
> Given that it currently needs to fit in memory, i think not.
> 
> For example: if i create a job to test something out, then kill it and 
> never return.  Probably shouldn't retain those updates forever.
> 
> David McLaughlin wrote:
> Wouldn't the updates in that case be killed by the job garbage 
> collection/cascading deletion of the job key?
> 
> Bill Farner wrote:
> If so, that warrants validation in an integration test between 
> TaskHistoryPruner, storage, and DbJobUpdateStore.
> 
> Maxim Khutornenko wrote:
> We don't cascade deletes into the job_keys table. In fact, we don't 
> actually delete from job_keys at all at this point. We should probably 
> address this when converting TaskStore into H2.
> 
> | if i create a job to test something out, then kill it and never return. 
>  Probably shouldn't retain those updates forever.
> Good point. This alone warrants adding time-based cleanup.

Added support for time-based pruning.


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55088
---


On Oct. 1, 2014, 5:26 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 1, 2014, 5:26 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 

Re: Review Request 26232: Implementing update history pruner.

[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
> > 
> >
> > We also need time-based retention.  How about 1 month by default?
> 
> Maxim Khutornenko wrote:
> Thought about that but was not sure the extra complexity is worth it. 
> Also, don't we want to show update history in the UI regardless of how old it 
> is?
> 
> Bill Farner wrote:
> Given that it currently needs to fit in memory, i think not.
> 
> For example: if i create a job to test something out, then kill it and 
> never return.  Probably shouldn't retain those updates forever.
> 
> David McLaughlin wrote:
> Wouldn't the updates in that case be killed by the job garbage 
> collection/cascading deletion of the job key?
> 
> Bill Farner wrote:
> If so, that warrants validation in an integration test between 
> TaskHistoryPruner, storage, and DbJobUpdateStore.

We don't cascade deletes into the job_keys table. In fact, we don't actually 
delete from job_keys at all at this point. We should probably address this when 
converting TaskStore into H2.

| if i create a job to test something out, then kill it and never return.  
Probably shouldn't retain those updates forever.
Good point. This alone warrants adding time-based cleanup.


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55088
---


On Oct. 1, 2014, 5:26 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 1, 2014, 5:26 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[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
> > 
> >
> > We also need time-based retention.  How about 1 month by default?
> 
> Maxim Khutornenko wrote:
> Thought about that but was not sure the extra complexity is worth it. 
> Also, don't we want to show update history in the UI regardless of how old it 
> is?
> 
> Bill Farner wrote:
> Given that it currently needs to fit in memory, i think not.
> 
> For example: if i create a job to test something out, then kill it and 
> never return.  Probably shouldn't retain those updates forever.
> 
> David McLaughlin wrote:
> Wouldn't the updates in that case be killed by the job garbage 
> collection/cascading deletion of the job key?

If so, that warrants validation in an integration test between 
TaskHistoryPruner, storage, and DbJobUpdateStore.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55088
---


On Oct. 1, 2014, 5:26 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 1, 2014, 5:26 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[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
> > 
> >
> > We also need time-based retention.  How about 1 month by default?
> 
> Maxim Khutornenko wrote:
> Thought about that but was not sure the extra complexity is worth it. 
> Also, don't we want to show update history in the UI regardless of how old it 
> is?
> 
> Bill Farner wrote:
> Given that it currently needs to fit in memory, i think not.
> 
> For example: if i create a job to test something out, then kill it and 
> never return.  Probably shouldn't retain those updates forever.

Wouldn't the updates in that case be killed by the job garbage 
collection/cascading deletion of the job key?


- David


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55088
---


On Oct. 1, 2014, 5:26 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 1, 2014, 5:26 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[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
> > 
> >
> > We also need time-based retention.  How about 1 month by default?
> 
> Maxim Khutornenko wrote:
> Thought about that but was not sure the extra complexity is worth it. 
> Also, don't we want to show update history in the UI regardless of how old it 
> is?

Given that it currently needs to fit in memory, i think not.

For example: if i create a job to test something out, then kill it and never 
return.  Probably shouldn't retain those updates forever.


- Bill


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55088
---


On Oct. 1, 2014, 5:26 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 1, 2014, 5:26 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[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
> > 
> >
> > We also need time-based retention.  How about 1 month by default?

Thought about that but was not sure the extra complexity is worth it. Also, 
don't we want to show update history in the UI regardless of how old it is?


- Maxim


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/26232/#review55088
---


On Oct. 1, 2014, 5:26 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 1, 2014, 5:26 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>

Re: Review Request 26232: Implementing update history pruner.

[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


We also need time-based retention.  How about 1 month by default?



src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java


Does this work?  I think something needs to 'pull' on the binding (by 
injecting) for anything to happen.



src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java


maxUpdatesPerJob?



src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java


Nit: starting this with "Pruning completed" breaks my brain a little.  How 
about "Pruning job update history."



src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java


I'm not sure how i feel about this.  On one hand, it's nice that SQL 
handles a lot of the complexity.  On the other, it would be _really_ nice to 
see deleted job update UUIDs in the log.  Perhaps this function can return some 
details about the deleted updates?


- Bill Farner


On Oct. 1, 2014, 5:26 p.m., Maxim Khutornenko wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26232/
> ---
> 
> (Updated Oct. 1, 2014, 5:26 p.m.)
> 
> 
> Review request for Aurora, David McLaughlin and Bill Farner.
> 
> 
> Bugs: AURORA-743
> https://issues.apache.org/jira/browse/AURORA-743
> 
> 
> Repository: aurora
> 
> 
> Description
> ---
> 
> The pruner runs on periodic basis and trims completed updates up to the 
> guranteed per job retention count.
> 
> 
> Diffs
> -
> 
>   src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
> aa45d27391b1786ca3d5e8c928045f1b6f3cf5ef 
>   src/main/java/org/apache/aurora/scheduler/async/HistoryPruner.java 
> ebae58a04e8857c5f26d4b57c27dfcda9e14c82c 
>   src/main/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPruner.java 
> PRE-CREATION 
>   src/main/java/org/apache/aurora/scheduler/storage/JobUpdateStore.java 
> c3abffe575e801cebec3572cf4aceac83a238b55 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStore.java 
> 3db0114c5432a5b7d0b01d97c75494be9e3e99a3 
>   
> src/main/java/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.java
>  c583e085e0458835d51ebf740a3b5f01b428bb25 
>   
> src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java 
> 66c91644677e7176ccf53dcfcf29a6792ec398bc 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/JobUpdateDetailsMapper.xml
>  631ab2543dadb77b7fd1cae96adc9c0bd2f94b39 
>   src/test/java/org/apache/aurora/scheduler/async/HistoryPrunerTest.java 
> 011d9ec5d84c658c2b2e39bb0b3f9d20b5440cee 
>   
> src/test/java/org/apache/aurora/scheduler/async/JobUpdateHistoryPrunerTest.java
>  PRE-CREATION 
>   
> src/test/java/org/apache/aurora/scheduler/storage/db/DBJobUpdateStoreTest.java
>  1b1160f349d64bbcd4d20103a82f3b8eb1ca75d9 
> 
> Diff: https://reviews.apache.org/r/26232/diff/
> 
> 
> Testing
> ---
> 
> ./gradlew -Pq build
> 
> 
> Thanks,
> 
> Maxim Khutornenko
> 
>