ibessonov commented on code in PR #1295:
URL: https://github.com/apache/ignite-3/pull/1295#discussion_r1011545887
##########
modules/network/src/main/java/org/apache/ignite/network/DefaultMessagingService.java:
##########
@@ -214,31 +180,31 @@ private CompletableFuture<NetworkMessage>
invoke0(@Nullable ClusterNode recipien
requestsMap.put(correlationId, responseFuture);
- InetSocketAddress address = new InetSocketAddress(addr.host(),
addr.port());
+ InetSocketAddress recipientAddress = new
InetSocketAddress(recipient.address().host(), recipient.address().port());
Review Comment:
Same here
##########
modules/network/src/main/java/org/apache/ignite/network/DefaultMessagingService.java:
##########
@@ -128,54 +115,37 @@ public void weakSend(ClusterNode recipient,
NetworkMessage msg) {
/** {@inheritDoc} */
@Override
public CompletableFuture<Void> send(ClusterNode recipient, NetworkMessage
msg) {
- return send0(recipient, recipient.address(), msg, null);
+ return send0(recipient, msg, null);
}
/** {@inheritDoc} */
@Override
public CompletableFuture<Void> respond(ClusterNode recipient,
NetworkMessage msg, long correlationId) {
- return send0(recipient, recipient.address(), msg, correlationId);
- }
-
- /** {@inheritDoc} */
- @Override
- public CompletableFuture<Void> respond(NetworkAddress addr, NetworkMessage
msg, long correlationId) {
- ClusterNode recipient = topologyService.getByAddress(addr);
- return send0(recipient, addr, msg, correlationId);
+ return send0(recipient, msg, correlationId);
}
/** {@inheritDoc} */
@Override
public CompletableFuture<NetworkMessage> invoke(ClusterNode recipient,
NetworkMessage msg, long timeout) {
- return invoke0(recipient, recipient.address(), msg, timeout);
- }
-
- /** {@inheritDoc} */
- @Override
- public CompletableFuture<NetworkMessage> invoke(NetworkAddress addr,
NetworkMessage msg, long timeout) {
- ClusterNode recipient = topologyService.getByAddress(addr);
-
- return invoke0(recipient, addr, msg, timeout);
+ return invoke0(recipient, msg, timeout);
}
/**
* Sends a message. If the target is the current node, then message will
be delivered immediately.
*
- * @param recipient Target cluster node. TODO: Maybe {@code null} due to
IGNITE-16373.
- * @param address Target address.
+ * @param recipient Target cluster node.
* @param msg Message.
* @param correlationId Correlation id. Not null iff the message is a
response to a {@link #invoke} request.
* @return Future of the send operation.
*/
- private CompletableFuture<Void> send0(@Nullable ClusterNode recipient,
NetworkAddress address, NetworkMessage msg,
- @Nullable Long correlationId) {
+ private CompletableFuture<Void> send0(ClusterNode recipient,
NetworkMessage msg, @Nullable Long correlationId) {
if (connectionManager.isStopped()) {
return failedFuture(new NodeStoppingException());
}
- InetSocketAddress addr = new InetSocketAddress(address.host(),
address.port());
+ InetSocketAddress recipientAddress = new
InetSocketAddress(recipient.address().host(), recipient.address().port());
Review Comment:
Will this be resolved every time? From the Ignite 2.x I know that it may
take a while, so we should think about it before the release. Not right now
##########
modules/network/src/main/java/org/apache/ignite/network/DefaultMessagingService.java:
##########
@@ -375,33 +334,27 @@ private long createCorrelationId() {
/**
* Checks if the target is the current node.
*
- * @param target Target cluster node. TODO: IGNITE-16373 May be {@code
null} due to the ticket.
- * @param targetSocketAddress Target's socket address.
+ * @param consistentId Target consistent ID. Can be {@code null} if the
node has not been added to the topology.
+ * @param targetAddress Target address.
* @return {@code true} if the target is the current node, {@code false}
otherwise.
*/
- private boolean isSelf(@Nullable ClusterNode target, @Nullable String
consistentId, SocketAddress targetSocketAddress) {
- String cid = consistentId;
-
- if (cid == null && target != null) {
- cid = target.name();
+ private boolean isSelf(@Nullable String consistentId, InetSocketAddress
targetAddress) {
+ if (consistentId != null) {
+ return connectionManager.consistentId().equals(consistentId);
}
- if (cid != null) {
- return connectionManager.consistentId().equals(cid);
- }
+ InetSocketAddress localAddress = connectionManager.localAddress();
Review Comment:
Again, a comment for the future. How can we be sure that there's only one
physical address for local node. Everything in this class looks suspicious.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
