sergey-chugunov-1985 commented on code in PR #12598: URL: https://github.com/apache/ignite/pull/12598#discussion_r2652718310
########## modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryClusterMetricsHolderMessage.java: ########## @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.spi.discovery.tcp.messages; + +import org.apache.ignite.cluster.ClusterMetrics; +import org.apache.ignite.internal.managers.discovery.DiscoveryMessageFactory; +import org.apache.ignite.internal.processors.cluster.NodeMetricsMessage; +import org.apache.ignite.internal.util.typedef.internal.S; + +/** + * Utility container message of the node metrics. Is not a pure {@link TcpDiscoveryAbstractMessage}. + * Reuses Communication's {@link NodeMetricsMessage}. + */ +public class TcpDiscoveryClusterMetricsHolderMessage extends NodeMetricsMessage { Review Comment: ```suggestion public class TcpDiscoveryNodeMetricsMessage extends NodeMetricsMessage { ``` ########## modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/messages/TcpDiscoveryClusterMetricsHolderMessage.java: ########## @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.ignite.spi.discovery.tcp.messages; + +import org.apache.ignite.cluster.ClusterMetrics; +import org.apache.ignite.internal.managers.discovery.DiscoveryMessageFactory; +import org.apache.ignite.internal.processors.cluster.NodeMetricsMessage; +import org.apache.ignite.internal.util.typedef.internal.S; + +/** + * Utility container message of the node metrics. Is not a pure {@link TcpDiscoveryAbstractMessage}. Review Comment: ```suggestion * We cannot directly reuse `NodeMetricsMessage` in Discovery as it is registered in a message factory of Communication component * and thus is unavailable in Discovery. * We have to extend `NodeMetricsMessage` and register this subclass in message factory of Discovery component. ``` ########## modules/core/src/main/java/org/apache/ignite/spi/discovery/tcp/TcpDiscoveryIoSession.java: ########## @@ -185,19 +184,39 @@ <T> T readMessage() throws IgniteCheckedException, IOException { boolean finished; - do { - // Should be cleared before first operation. - msgBuf.clear(); + msgBuf.clear(); - int read = in.read(msgBuf.array(), 0, msgBuf.limit()); + do { + int read = in.read(msgBuf.array(), msgBuf.position(), msgBuf.remaining()); if (read == -1) throw new EOFException("Connection closed before message was fully read."); - msgBuf.limit(read); + msgBuf.limit(msgBuf.position() > 0 ? msgBuf.position() + read + 1 : read); finished = msgSer.readFrom(msg, msgReader); - } while (!finished); + + // We assume that there is only one message in the socket because Discovery is a serial protocol with Review Comment: ```suggestion // We rely on the fact that Discovery only sends next message upon receiving a receipt for the previous one. This behaviour guarantees that we never read from msgBuf a beginning of a next message right after the end of the previous one. ``` -- 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]
