> 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 
> > opposed to a dedicated java trigger (1)? Triggers certainly have their 
> > pitfals but they may provide a cleaner solution and let us migrate easier 
> > to pure SQL triggers when H2 implements support for them (2)
> >     
> >     (1) - http://www.h2database.com/html/features.html#triggers
> >     (2) - http://www.h2database.com/html/roadmap.html

Biggest issue is coupling to h2.  H2 only supports triggers written in java, 
implementing their API.  This would present a pretty big hurdle for using a 
different vendor.


> On June 23, 2015, 6:58 p.m., Maxim Khutornenko wrote:
> > src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java,
> >  line 136
> > <https://reviews.apache.org/r/35793/diff/2/?file=990668#file990668line136>
> >
> >     A stat counter recording a successful removal would be nice here and 
> > below. That would help us monitoring cleanup health.

What's the shelf life of that?  This seems like something we won't want to 
sprinkle in each of the many forthcoming places we need to do this, and doesn't 
seem like we'd actually be able to alert on it.


- Bill


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


On June 23, 2015, 6:28 p.m., Bill Farner wrote:
> 
> -----------------------------------------------------------
> 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 Khutornenko.
> 
> 
> Bugs: AURORA-1298
>     https://issues.apache.org/jira/browse/AURORA-1298
> 
> 
> Repository: aurora
> 
> 
> Description
> -------
> 
> DbTaskStore: delete unreferenced job keys.
> 
> 
> Diffs
> -----
> 
>   src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java 
> 30e3469a9f69091b929a9243f84036fa2fdd0539 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java 
> 3ada6286e6ef6e3302802b74eec6c46dd582dc10 
>   src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java 
> 7ee001f9c019a1e7b669ae5cec6088bf974a3746 
>   
> src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
>  8258fb102b7f5fca9635143ebaed542d43abeb9f 
>   src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql 
> 24cf52680b69e23f5ccbbcada0606975b0405d5b 
>   
> src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java 
> 63a784f843eb7edf9a13c623e5355169c7e8623b 
>   src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java 
> dda988d03634f8de582cf2b8ccdeb433c3e3de0c 
> 
> Diff: https://reviews.apache.org/r/35793/diff/
> 
> 
> Testing
> -------
> 
> Note that this removes a defensive branch wherein we checked for inbound 
> config references before attempting to delete.  With this change, we 
> proactively delete and count on foreign key constraints to prevent deletion 
> of rows that are still referenced.  I propose we adopt this as our pattern 
> for handling shared references, as it seems to be the most sane approach 
> available.
> 
> A gotcha with this case is that i do not believe mybatis provides a 
> vendor-neutral approach to identify a consistency violation, and the best 
> signal is a generic `PersistenceException`.  This is unfortunate since we 
> can't distinguish between a hopeless query with invalid syntax, a network 
> disconnection, or the anticipated case of a consistency violation.
> 
> 
> Thanks,
> 
> Bill Farner
> 
>

Reply via email to