rpuch commented on code in PR #6732: URL: https://github.com/apache/ignite-3/pull/6732#discussion_r2418551220
########## modules/cluster-management/src/test/java/org/apache/ignite/internal/cluster/management/raft/commands/CmgCommandsCompatibilityTest.java: ########## @@ -0,0 +1,249 @@ +/* + * 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.internal.cluster.management.raft.commands; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +import java.util.Base64; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import org.apache.ignite.internal.cluster.management.ClusterState; +import org.apache.ignite.internal.cluster.management.ClusterTag; +import org.apache.ignite.internal.cluster.management.network.messages.CmgMessagesFactory; +import org.apache.ignite.internal.cluster.management.network.messages.CmgMessagesSerializationRegistryInitializer; +import org.apache.ignite.internal.network.MessageSerializationRegistryImpl; +import org.apache.ignite.internal.network.serialization.MessageSerializationRegistry; +import org.apache.ignite.internal.raft.Command; +import org.apache.ignite.internal.raft.Marshaller; +import org.apache.ignite.internal.raft.util.ThreadLocalOptimizedMarshaller; +import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** + * Compatibility testing for serialization/deserialization of CMG raft commands. It is verified that deserialization of commands that were + * created on earlier versions of the product will be error-free. + * + * <p>For MAC users with aarch64 architecture, you will need to add {@code || "aarch64".equals(arch)} to the Review Comment: Would it make sense to add https://lists.apache.org/thread/67coyvm8mo7106mkndt24yqwtbvb7590 to provide context? ########## modules/cluster-management/src/test/java/org/apache/ignite/internal/cluster/management/raft/commands/CmgCommandsCompatibilityTest.java: ########## @@ -0,0 +1,249 @@ +/* + * 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.internal.cluster.management.raft.commands; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +import java.util.Base64; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import org.apache.ignite.internal.cluster.management.ClusterState; +import org.apache.ignite.internal.cluster.management.ClusterTag; +import org.apache.ignite.internal.cluster.management.network.messages.CmgMessagesFactory; +import org.apache.ignite.internal.cluster.management.network.messages.CmgMessagesSerializationRegistryInitializer; +import org.apache.ignite.internal.network.MessageSerializationRegistryImpl; +import org.apache.ignite.internal.network.serialization.MessageSerializationRegistry; +import org.apache.ignite.internal.raft.Command; +import org.apache.ignite.internal.raft.Marshaller; +import org.apache.ignite.internal.raft.util.ThreadLocalOptimizedMarshaller; +import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** + * Compatibility testing for serialization/deserialization of CMG raft commands. It is verified that deserialization of commands that were + * created on earlier versions of the product will be error-free. + * + * <p>For MAC users with aarch64 architecture, you will need to add {@code || "aarch64".equals(arch)} to the + * {@code GridUnsafe#unaligned()} for the tests to pass.</p> + */ +public class CmgCommandsCompatibilityTest extends BaseIgniteAbstractTest { + private static final MessageSerializationRegistry REGISTRY = new MessageSerializationRegistryImpl(); + + private static final Marshaller MARSHALLER = new ThreadLocalOptimizedMarshaller(REGISTRY); + + private static final CmgMessagesFactory FACTORY = new CmgMessagesFactory(); + + @BeforeAll + static void beforeAll() { + new CmgMessagesSerializationRegistryInitializer().registerFactories(REGISTRY); + } Review Comment: Why are all those static? They can be instance members. Static fields never get garbage collected, this is basically a memory leak. It doesn't seem to be a problem to create a registry, marshaller and factory per test run as they are created quickly ########## modules/cluster-management/src/test/java/org/apache/ignite/internal/cluster/management/raft/commands/CmgCommandsCompatibilityTest.java: ########## @@ -0,0 +1,249 @@ +/* + * 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.internal.cluster.management.raft.commands; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; + +import java.util.Base64; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.UUID; +import org.apache.ignite.internal.cluster.management.ClusterState; +import org.apache.ignite.internal.cluster.management.ClusterTag; +import org.apache.ignite.internal.cluster.management.network.messages.CmgMessagesFactory; +import org.apache.ignite.internal.cluster.management.network.messages.CmgMessagesSerializationRegistryInitializer; +import org.apache.ignite.internal.network.MessageSerializationRegistryImpl; +import org.apache.ignite.internal.network.serialization.MessageSerializationRegistry; +import org.apache.ignite.internal.raft.Command; +import org.apache.ignite.internal.raft.Marshaller; +import org.apache.ignite.internal.raft.util.ThreadLocalOptimizedMarshaller; +import org.apache.ignite.internal.testframework.BaseIgniteAbstractTest; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; + +/** + * Compatibility testing for serialization/deserialization of CMG raft commands. It is verified that deserialization of commands that were + * created on earlier versions of the product will be error-free. + * + * <p>For MAC users with aarch64 architecture, you will need to add {@code || "aarch64".equals(arch)} to the + * {@code GridUnsafe#unaligned()} for the tests to pass.</p> + */ +public class CmgCommandsCompatibilityTest extends BaseIgniteAbstractTest { + private static final MessageSerializationRegistry REGISTRY = new MessageSerializationRegistryImpl(); + + private static final Marshaller MARSHALLER = new ThreadLocalOptimizedMarshaller(REGISTRY); + + private static final CmgMessagesFactory FACTORY = new CmgMessagesFactory(); + + @BeforeAll + static void beforeAll() { + new CmgMessagesSerializationRegistryInitializer().registerFactories(REGISTRY); + } + + @Test + void testChangeMetaStorageInfoCommand() { + ChangeMetaStorageInfoCommand command = decodeCommand("CDADCG1zTm9kZTIIbXNOb2RlMQEr"); Review Comment: Let's extract the serialized content to constants, like `SERIALIZED_CHANGE_META_STORAGE_INFO_COMMAND`. -- 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]
