Hi, Yes, now you are on the right way.
It is ok if checkpointReadLock() throws runtime exception and in your PR reason become obvious. Now, it is easy to handle exceptions thrown from checkpointReadLock() method on node stop if needed. TtlManager looks correctly ignore NodeStopping exception, but I'd avoid logging of warning message with stacktrace here. I've left comments in ticket. Normally, this change shouldn't affect any other behavior and you shouldn't be bother to catch this exception on every checkpointReadLock call. Those places where checkpointReadLock is called from, now fixed and will correctly handle exception with cause (NodeStoppingExcpetion) if they tried to do this before, of cause. checkpointReadLock() could throw IgniteException before your changes. So changing type RuntimeException -> IgniteException shouldn't have any affect (otherwise it is a bug of incorrect exception handling). Please run TC test and check if there is no new failures. I see nothing in GridCacheAdapter#getAllAsync0 that may be affected by your changes. On Tue, May 29, 2018 at 8:05 PM, Dmitriy Setrakyan <[email protected]> wrote: > As suggested before, please do not put blank ticket numbers into subjects > because no one understands ticket numbers. Please add titles or some other > context to the subject. This will improve the level of engagement from the > community. > > I have changed the subject of this thread, let's continue the discussion > here. > > D. > > On Tue, May 29, 2018 at 9:10 AM, Александр Меньшиков <[email protected] > > wrote: > >> Hi, Andrew. >> >> You suggested continuing the discussion on dev-list about IGNITE-8238 [1] >> I have reworked my PR [2]. Have I moved in the right direction? >> >> I see 57 usages of `checkpointReadLock` in the core module (without >> tests). >> And it still unclear for me should I catch all exceptions from >> `checkpointReadLock` or not? >> Some places look okay like usage inside overrides of `GridWorker#body`. >> But for example `GridCacheAdapter#getAllAsync0` makes me worry. >> >> [1] https://issues.apache.org/jira/browse/IGNITE-8238 >> [2] https://github.com/apache/ignite/pull/3993/files >> > > -- Best regards, Andrey V. Mashenkov
