[GitHub] ignite pull request #3993: IGNITE-8238: Operation can fails with unexpected ...
Github user asfgit closed the pull request at: https://github.com/apache/ignite/pull/3993 ---
Re: IGNITE-8238 - Operation can fails with unexpected RuntimeException when node is stopping
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 : > 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 > 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 >
Re: IGNITE-8238 - Operation can fails with unexpected RuntimeException when node is stopping
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 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, Александр Меньшиков > 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
Re: IGNITE-8238 - Operation can fails with unexpected RuntimeException when node is stopping
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, Александр Меньшиков 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 >
IGNITE-8238
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
[GitHub] ignite pull request #3993: IGNITE-8238: Operation can fails with unexpected ...
GitHub user SharplEr opened a pull request: https://github.com/apache/ignite/pull/3993 IGNITE-8238: Operation can fails with unexpected RuntimeException when node is stopping. For [IGNITE-8238](https://issues.apache.org/jira/browse/IGNITE-8238) You can merge this pull request into a Git repository by running: $ git pull https://github.com/SharplEr/ignite ignite-8238 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/ignite/pull/3993.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #3993 ---
[jira] [Created] (IGNITE-8238) Operation can fails with unexpected RuntimeException when node is stopping.
Andrew Mashenkov created IGNITE-8238: Summary: Operation can fails with unexpected RuntimeException when node is stopping. Key: IGNITE-8238 URL: https://issues.apache.org/jira/browse/IGNITE-8238 Project: Ignite Issue Type: Bug Components: general Reporter: Andrew Mashenkov Operation can fails with RuntimeException when node is stoped in other thread. PFA stacktrace. It is not clear from javadoc that operation can throws RuntimeException. We should add it to javadoc or e.g. throws IllegalStateException which already present in java cache api javadoc. -- This message was sent by Atlassian JIRA (v7.6.3#76005)
