Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-25 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35793/#review89404 --- Ship it! Ship It! - Maxim Khutornenko On June 24, 2015, 10:46

Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-25 Thread Maxim Khutornenko
On June 25, 2015, 6:05 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 254 https://reviews.apache.org/r/35793/diff/3/?file=991667#file991667line254 This will use a regular single threaded executor, right? Any chance to

Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-25 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35793/#review89393 --- Looks great! Just a couple suggestions below.

Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-25 Thread Bill Farner
On June 25, 2015, 6:05 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java, line 254 https://reviews.apache.org/r/35793/diff/3/?file=991667#file991667line254 This will use a regular single threaded executor, right? Any chance to

Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-25 Thread Bill Farner
On June 25, 2015, 6:05 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java, line 60 https://reviews.apache.org/r/35793/diff/3/?file=991672#file991672line60 I'd still recommend having a run counter to help us monitor

Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-24 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35793/ --- (Updated June 24, 2015, 10:46 p.m.) Review request for Aurora and Maxim

Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-24 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35793/#review89278 --- Ship it! Master (d28bd4f) is green with this patch.

Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-23 Thread Bill Farner
On June 23, 2015, 6:58 p.m., Maxim Khutornenko wrote: src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java, line 133 https://reviews.apache.org/r/35793/diff/2/?file=990668#file990668line133 Any particular reason you've settled on an explicit removal call as

Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-23 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35793/#review89022 ---

Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-23 Thread Bill Farner
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35793/ --- (Updated June 23, 2015, 6:28 p.m.) Review request for Aurora and Maxim

Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-23 Thread Aurora ReviewBot
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35793/#review89023 --- Ship it! Master (68c4620) is green with this patch.

Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-23 Thread Maxim Khutornenko
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35793/#review89047 --- Discussed with Bill offline and it seems like having an offline

Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.

2015-06-23 Thread Bill Farner
On June 23, 2015, 8:12 p.m., Maxim Khutornenko wrote: Discussed with Bill offline and it seems like having an offline periodic async GC processor would be a better long term solution. It would allow us to handle all cleanup in a single place but more importantly will remove the