ignite-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Александр Меньшиков <sharple...@gmail.com>
Subject IGNITE-1948 ClusterTopologyCheckedException can return null for retryReadyFuture()
Date Fri, 27 Jan 2017 12:45:33 GMT
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
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
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?

  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message