> 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
> > provide a logging executor instance instead?
>
> Bill Farner wrote:
> The executor is not determined by this, just the interval. See
> `executor()` [1] for the executor being used. AbstractScheduledService shuts
> down and logs when exceptions are thrown, and our /services endpoint provides
> visibility when a service dies. Our other AbstractScheduledServices use this
> approach.
>
> [1]
> http://docs.guava-libraries.googlecode.com/git-history/release/javadoc/com/google/common/util/concurrent/AbstractScheduledService.html#executor()
Yeah, looks like they are doing it right:
```
private final Runnable task = new Runnable() {
@Override public void run() {
lock.lock();
try {
AbstractScheduledService.this.runOneIteration();
} catch (Throwable t) {
try {
shutDown();
} catch (Exception ignored) {
logger.log(Level.WARNING,
"Error while attempting to shut down the service after
failure.", ignored);
}
notifyFailed(t);
throw Throwables.propagate(t);
} finally {
lock.unlock();
}
}
};
```
> 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 collector
> > availability/health.
>
> Bill Farner wrote:
> Does the comment above change your opinion? If not, perhaps a more
> cross-cutting approach is stats on the states of all Services?
> If not, perhaps a more cross-cutting approach is stats on the states of all
> Services?
That would be ideal. Happy to accept a TODO.
- Maxim
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/35793/#review89393
-----------------------------------------------------------
On June 24, 2015, 10:46 p.m., Bill Farner wrote:
>
> -----------------------------------------------------------
> 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 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/app/SchedulerMain.java
> c31446c447c3385a4763b8a516827988e46cc480
> src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java
> 11c9c4ada400d51fc83e9e0de03108456be15fdf
> src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java
> 335d7a95e797fe940e71b10da44cbd97edea69ac
> src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java
> 2dc3034a5e4389588b5f796ff8dfb06dbc9939b8
> src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
> 30e3469a9f69091b929a9243f84036fa2fdd0539
>
> src/main/java/org/apache/aurora/scheduler/storage/db/GarbageCollectedTableMapper.java
> PRE-CREATION
> src/main/java/org/apache/aurora/scheduler/storage/db/JobKeyMapper.java
> afdaa49d6cd5c135f6e4ddda2b6a45d189560e09
> src/main/java/org/apache/aurora/scheduler/storage/db/LockKeyMapper.java
> 5e9ba823d08d2b46342e7722c9df5f2a349c97cf
>
> src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java
> PRE-CREATION
> 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/JobKeyMapper.xml
> f5829ac063272123995193caef5151e0d52d435b
>
> 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
>
> src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
> PRE-CREATION
>
> 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
>
>
