Alexey, For GridCacheIoManager#sendNoRetry it's not necessary because exception will be ignored (or will cast to String). Perhaps my message was unclear. I try to say that three methods can throw "ClusterTopologyCheckedException" without any problem.
But you are right, in some methods I can't set "retryFuture". We must ensure that exception doesn't escape away. The best option is to ignore it (or cast to String). But any way, look here: IgniteTxHandler#sendReply(UUID nodeId, GridDhtTxFinishRequest req, boolean committed, GridCacheVersion nearTxId) Inside you can found next lines: __________________________ ClusterTopologyCheckedException *cause* = new ClusterTopologyCheckedException("Primary node left grid."); res.checkCommittedError(new IgniteTxRollbackCheckedException("Failed to commit transaction " + "(transaction has been rolled back on backup node): " + req.version(), *cause*)); __________________________ How do you unsure that *"cause"* can't come to GridCacheUtils#retryTopologySafe(final Callable<S> c) ? Perhaps I don't know some rules which you use to maintain it now? 2017-01-30 12:24 GMT+03:00 Alexey Goncharuk <alexey.goncha...@gmail.com>: > Alexander, > > Do you have a use-case in mind which results in IgniteTopologyException > thrown from public API with retryFuture unset? > > retryFuture makes sense only for certain cache operations and may be set > only in certain places in the code where we already know what to wait for. > I do not see how you can initialize this future, for example, in > GridCacheIoManager#sendNoRetry(ClusterNode node, GridCacheMessage msg, > byte > plc) > > --AG > > 2017-01-27 15:45 GMT+03:00 Александр Меньшиков <sharple...@gmail.com>: > > > I found a lot of using "ClusterTopologyCheckedException" exception. In > > most > > use-case these exceptions were just ignored. It's easier to see if > > issue-4612 will be applied (I'm waiting for code review by my team > leader) > > where I renamed all unused exceptions in the "ignored". > > > > It means that in some case "retryReadyFuture" can left unset. But there > are > > code where exception is being getting from "get()" method in some > "future" > > class and don't ignored (exception is sending to method > > "GridFutureAdapter#onDone()" for example). > > > > So I decided not to do a deep global analysis of code and make some > simple > > rules. > > 1. If some method "X" throws ClusterTopologyCheckedException and ignores > > it > > (or converts to String, there are some variants to lose exception like > > object) in all methods that call the method "X", then we can don't set > > "retryReadyFuture" for optimization. But this should be clarified in > > javadoc. > > 2. But if exception escapes at least once like object: we must set > > "retryReadyFuture" in that method. > > > > A few times when call tree was simple, I went deeper. > > > > So I found only three methods which can throw > > "ClusterTopologyCheckedException" without setting "retryReadyFuture": > > > > 1. IgfsFragmentizerManager#sendWithRetries(UUID nodeId, > > IgfsCommunicationMessage msg) > > 2. GridCacheIoManager#sendNoRetry(ClusterNode node, GridCacheMessage > msg, > > byte plc) > > 3. GridCacheIoManager#sendOrderedMessage(ClusterNode node, Object topic, > > GridCacheMessage msg, byte plc, long timeout) > > > > In all other methods "ClusterTopologyCheckedException" escapes away too > > far. > > > > What do you think about this approach to make compromise between > > optimization and correctness? > > >