Okay, I have changed the logging level to debug, updated master and rerun TC tests [1].
[1] https://ci.ignite.apache.org/viewType.html?buildTypeId=IgniteTests24Java8_RunAll&branch_IgniteTests24Java8=pull%2F3993%2Fhead&tab=buildTypeStatusDiv 2018-05-30 16:13 GMT+03:00 Andrey Mashenkov <andrey.mashen...@gmail.com>: > 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 <dsetrak...@apache.org> > 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, Александр Меньшиков < >> sharple...@gmail.com> 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 >
