tkalkirill commented on code in PR #1204:
URL: https://github.com/apache/ignite-3/pull/1204#discussion_r995418429
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/network/messages/CmgMessageGroup.java:
##########
@@ -53,4 +53,27 @@ public class CmgMessageGroup {
* Message type for {@link SuccessResponseMessage}.
*/
public static final short SUCCESS_RESPONSE = 6;
+
+ /**
+ * Message types for RAFT commands.
+ */
+ public interface Commands {
Review Comment:
Can you add a description to the commands?
##########
modules/cluster-management/src/main/java/org/apache/ignite/internal/cluster/management/ClusterState.java:
##########
@@ -32,76 +33,32 @@
* <li>Cluster tag, unique for a particular Ignite cluster.</li>
* </ol>
*/
-public final class ClusterState implements Serializable {
- private final Set<String> cmgNodes;
-
- private final Set<String> msNodes;
-
- private final IgniteProductVersion igniteVersion;
-
- private final ClusterTag clusterTag;
-
+@Transferable(CmgMessageGroup.Commands.CLUSTER_STATE)
+public interface ClusterState extends NetworkMessage, Serializable {
/**
- * Creates a new cluster state.
- *
- * @param cmgNodes Node names that host the CMG.
- * @param metaStorageNodes Node names that host the Meta Storage.
- * @param igniteVersion Version of Ignite nodes that comprise this cluster.
- * @param clusterTag Cluster tag.
+ * Returns node names that host the CMG.
*/
- public ClusterState(
- Collection<String> cmgNodes,
- Collection<String> metaStorageNodes,
- IgniteProductVersion igniteVersion,
- ClusterTag clusterTag
- ) {
- this.cmgNodes = Set.copyOf(cmgNodes);
- this.msNodes = Set.copyOf(metaStorageNodes);
- this.igniteVersion = igniteVersion;
- this.clusterTag = clusterTag;
- }
-
- public Set<String> cmgNodes() {
- return cmgNodes;
- }
-
- public Set<String> metaStorageNodes() {
- return msNodes;
- }
-
- public IgniteProductVersion igniteVersion() {
- return igniteVersion;
- }
+ Set<String> cmgNodes();
- public ClusterTag clusterTag() {
- return clusterTag;
- }
+ /**
+ * Returns node names that host the Meta Storage.
+ */
+ Set<String> metaStorageNodes();
- @Override
- public boolean equals(Object o) {
- if (this == o) {
- return true;
- }
- if (o == null || getClass() != o.getClass()) {
- return false;
- }
- ClusterState state = (ClusterState) o;
- return cmgNodes.equals(state.cmgNodes) &&
msNodes.equals(state.msNodes) && igniteVersion.equals(state.igniteVersion)
- && clusterTag.equals(state.clusterTag);
- }
+ /**
+ * Returns a version of Ignite nodes that comprise this cluster.
+ */
+ String version();
Review Comment:
Why string and not byte array?
##########
modules/cluster-management/src/test/java/org/apache/ignite/internal/cluster/management/raft/CmgRaftGroupListenerTest.java:
##########
@@ -63,22 +62,24 @@ void tearDown() throws Exception {
*/
@Test
void testValidatedNodeIds() {
- var state = new ClusterState(
- List.of("foo"),
- List.of("bar"),
- IgniteProductVersion.CURRENT_VERSION,
- new ClusterTag("cluster")
- );
+ ClusterTag clusterTag =
MSG_FACTORY.clusterTag().clusterName("cluster").clusterId(UUID.randomUUID()).build();
Review Comment:
Same
##########
modules/network/src/main/java/org/apache/ignite/internal/network/direct/stream/DirectByteBufferStreamImplV1.java:
##########
@@ -1359,6 +1367,17 @@ public <T> T[] readObjectArray(MessageCollectionItemType
itemType, Class<T> item
@Override
public <C extends Collection<?>> C
readCollection(MessageCollectionItemType itemType,
MessageReader reader) {
+ return readCollection0(itemType, reader, ArrayList::new);
+ }
+
+ @Override
+ public <C extends Set<?>> C readSet(MessageCollectionItemType itemType,
MessageReader reader) {
+ return readCollection0(itemType, reader, HashSet::new);
+ }
+
+ @Nullable
+ private <C extends Collection<?>> C
readCollection0(MessageCollectionItemType itemType, MessageReader reader,
+ IntFunction<Collection<Object>> ctor) {
Review Comment:
Lacks description.
##########
modules/cluster-management/src/test/java/org/apache/ignite/internal/cluster/management/raft/AbstractClusterStateStorageManagerTest.java:
##########
@@ -213,4 +216,22 @@ void testValidatedNodes() {
assertThat(storageManager.getValidatedNodeIds(),
containsInAnyOrder("node2"));
}
+
+ private static ClusterTag clusterTag(String cluster) {
Review Comment:
Can these methods be taken out in a separate auxiliary class?
--
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]
