aurora-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Maxim Khutornenko" <ma...@apache.org>
Subject Re: Review Request 35793: DbTaskStore: delete unreferenced job keys.
Date Tue, 23 Jun 2015 18:58:58 GMT

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



src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java (line 132)
<https://reviews.apache.org/r/35793/#comment141632>

    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



src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java (line 135)
<https://reviews.apache.org/r/35793/#comment141630>

    A stat counter recording a successful removal would be nice here and below. That would
help us monitoring cleanup health.


- Maxim Khutornenko


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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message