> 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?
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() > 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. Does the comment above change your opinion? If not, perhaps a more cross-cutting approach is stats on the states of all Services? - Bill ----------------------------------------------------------- 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 > >
