gemmellr commented on code in PR #5145:
URL: https://github.com/apache/activemq-artemis/pull/5145#discussion_r1715062415
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ChannelImpl.java:
##########
@@ -452,6 +452,14 @@ public Packet sendBlocking(final Packet packet, byte
expectedPacket) throws Acti
return sendBlocking(packet, -1, expectedPacket);
}
+ @Override
+ public Packet sendBlocking(final Packet packet,
+ final int reconnectID,
+ byte expectedPacket) throws ActiveMQException {
Review Comment:
expectedPacket could be final too
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/quorum/QuorumManager.java:
##########
@@ -348,19 +377,42 @@ public synchronized void voteComplete() {
}
}
- private Vote sendQuorumVote(ClusterControl clusterControl, SimpleString
handler, Vote vote) {
+ private Vote sendQuorumVote(ClusterControl clusterControl, SimpleString
handler, SimpleString oldHandlerName, Vote vote) {
try {
final ClientSessionFactoryInternal sessionFactory =
clusterControl.getSessionFactory();
final String remoteAddress =
sessionFactory.getConnection().getRemoteAddress();
ActiveMQServerLogger.LOGGER.sendingQuorumVoteRequest(remoteAddress,
vote.toString());
- QuorumVoteReplyMessage replyMessage = (QuorumVoteReplyMessage)
clusterControl.getClusterChannel().get()
- .sendBlocking(new QuorumVoteMessage(handler, vote),
PacketImpl.QUORUM_VOTE_REPLY);
+
+ QuorumVoteReplyMessage replyMessage = null;
+
+ Channel clusterChannel = clusterControl.getClusterChannel().get();
+
+
+ // We first try the current packet with a medium timeout
+ replyMessage = (QuorumVoteReplyMessage)
clusterChannel.sendBlocking(new QuorumVoteMessage(handler, vote), -1,
PacketImpl.QUORUM_VOTE_REPLY, VOTE_RESPONSE_TIMEOUT, false);
+ logger.trace("This is the reply message from the current version =
{}", replyMessage);
+
+ // if no response, we try the previous versions, with still a medium
timeout
+ if (replyMessage == null && oldHandlerName != null) {
+ replyMessage = (QuorumVoteReplyMessage)
clusterChannel.sendBlocking(new QuorumVoteMessage(oldHandlerName, vote), -1,
PacketImpl.QUORUM_VOTE_REPLY, VOTE_RESPONSE_TIMEOUT, false);
+ logger.trace("This is the reply message from the older version =
{}", replyMessage);
+ }
+
+ if (replyMessage == null) {
+ // if still no response, we will try one last time with the
blocking timeout configuration, and we would fail with a regular exception if
that still an issue
+ replyMessage = (QuorumVoteReplyMessage)
clusterChannel.sendBlocking(new QuorumVoteMessage(handler, vote),
PacketImpl.QUORUM_VOTE_REPLY);
+ logger.trace("This is the reply message from the older version =
{}", replyMessage);
Review Comment:
This log message doesnt seem to match with the line above, which suggests
the current one is being used.
##########
tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/realServer/RolledUpgradeTest.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * 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.activemq.artemis.tests.compatibility.realServer;
+
+import java.io.File;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.api.core.Pair;
+import org.apache.activemq.artemis.api.core.management.SimpleManagement;
+import org.apache.activemq.artemis.util.ServerUtil;
+import org.apache.activemq.artemis.utils.FileUtil;
+import org.apache.activemq.artemis.utils.SpawnedVMSupport;
+import org.apache.activemq.artemis.utils.Wait;
+import org.apache.activemq.artemis.utils.cli.helper.HelperBase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RolledUpgradeTest extends CompatBaseTest {
+
+ private static final Logger logger =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TWO_THIRTY_ZIP =
"./target/old-releases/apache-artemis-2.30.0-bin.zip";
+ private static final String TWO_THIRTY_FIVE_ZIP =
"./target/old-releases/apache-artemis-2.35.0-bin.zip";
+
+ //private static String sourceHome = HelperBase.getHome().getAbsolutePath();
+
+ public static final String LIVE_0 = "RolledUpgradeTest/live0";
+ public static final String LIVE_1 = "RolledUpgradeTest/live1";
+ public static final String LIVE_2 = "RolledUpgradeTest/live2";
+
+ public static final String BKP_0 = "RolledUpgradeTest/bkp0";
+ public static final String BKP_1 = "RolledUpgradeTest/bkp1";
+ public static final String BKP_2 = "RolledUpgradeTest/bkp2";
+
+ Process live0Process;
+ Process live1Process;
+ Process live2Process;
+
+ Process bkp0Process;
+ Process bkp1Process;
+ Process bkp2Process;
+
+ private static String getHost(int nodeID) {
+ return "localhost";
+ }
+
+ private static int getPort(int nodeID) {
+ return 61616 + nodeID;
+ }
+
+ private static int getPortOffeset(int nodeID) {
+ return nodeID;
+ }
+
+ private static void newServer(File homeLocation,
+ String location,
+ int serverID,
+ String replicaGroupName,
+ boolean live,
+ int[] connectedNodes) throws Exception {
+ File serverLocation = getFileServerLocation(location);
+ List<String> parameters = new ArrayList<>();
+ deleteDirectory(serverLocation);
+
+ StringBuilder clusterList = new StringBuilder();
+ for (int i = 0; i < connectedNodes.length; i++) {
+ if (i > 0) {
+ clusterList.append(",");
+ }
+ clusterList.append("tcp://" + getHost(connectedNodes[i]) + ":" +
getPort(connectedNodes[i]));
+ }
+
+ parameters.add(homeLocation.getAbsolutePath() + "/bin/artemis");
+ parameters.add("create");
+ parameters.add("--silent");
+ parameters.add("--user");
+ parameters.add("guest");
+ parameters.add("--password");
+ parameters.add("guest");
+ parameters.add("--port-offset");
+ parameters.add(String.valueOf(getPortOffeset(serverID)));
+ parameters.add("--allow-anonymous");
+ parameters.add("--no-web");
+ parameters.add("--no-autotune");
+ parameters.add("--host");
+ parameters.add("localhost");
+ parameters.add("--clustered");
+ parameters.add("--staticCluster");
+ parameters.add(clusterList.toString());
+ parameters.add("--replicated");
+ if (!live) {
+ parameters.add("--slave");
+ }
+ parameters.add("--no-amqp-acceptor");
+ parameters.add("--no-mqtt-acceptor");
+ parameters.add("--no-hornetq-acceptor");
+ parameters.add("--no-stomp-acceptor");
+ parameters.add(serverLocation.getAbsolutePath());
+
+ ProcessBuilder processBuilder = new ProcessBuilder();
+ processBuilder.command(parameters.toArray(new
String[parameters.size()]));
+ Process process = processBuilder.start();
+ SpawnedVMSupport.spawnLoggers(null, null, "ArtemisCreate", true, true,
process);
+ Assertions.assertTrue(process.waitFor(10, TimeUnit.SECONDS));
Review Comment:
When the JUnit 5 update was done most Assertions usage changed to simply the
assertFoo method rather than a prefixed Assertions.assertFoo, would nice to
keep things consistent (plus its more succinct/readable and easier to backport
to older versions)
##########
tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/realServer/RolledUpgradeTest.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * 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.activemq.artemis.tests.compatibility.realServer;
+
+import java.io.File;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.api.core.Pair;
+import org.apache.activemq.artemis.api.core.management.SimpleManagement;
+import org.apache.activemq.artemis.util.ServerUtil;
+import org.apache.activemq.artemis.utils.FileUtil;
+import org.apache.activemq.artemis.utils.SpawnedVMSupport;
+import org.apache.activemq.artemis.utils.Wait;
+import org.apache.activemq.artemis.utils.cli.helper.HelperBase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RolledUpgradeTest extends CompatBaseTest {
+
+ private static final Logger logger =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TWO_THIRTY_ZIP =
"./target/old-releases/apache-artemis-2.30.0-bin.zip";
+ private static final String TWO_THIRTY_FIVE_ZIP =
"./target/old-releases/apache-artemis-2.35.0-bin.zip";
+
+ //private static String sourceHome = HelperBase.getHome().getAbsolutePath();
+
+ public static final String LIVE_0 = "RolledUpgradeTest/live0";
+ public static final String LIVE_1 = "RolledUpgradeTest/live1";
+ public static final String LIVE_2 = "RolledUpgradeTest/live2";
+
+ public static final String BKP_0 = "RolledUpgradeTest/bkp0";
+ public static final String BKP_1 = "RolledUpgradeTest/bkp1";
+ public static final String BKP_2 = "RolledUpgradeTest/bkp2";
+
+ Process live0Process;
+ Process live1Process;
+ Process live2Process;
+
+ Process bkp0Process;
+ Process bkp1Process;
+ Process bkp2Process;
+
+ private static String getHost(int nodeID) {
+ return "localhost";
+ }
+
+ private static int getPort(int nodeID) {
+ return 61616 + nodeID;
+ }
+
+ private static int getPortOffeset(int nodeID) {
+ return nodeID;
+ }
+
+ private static void newServer(File homeLocation,
+ String location,
+ int serverID,
+ String replicaGroupName,
+ boolean live,
+ int[] connectedNodes) throws Exception {
+ File serverLocation = getFileServerLocation(location);
+ List<String> parameters = new ArrayList<>();
+ deleteDirectory(serverLocation);
+
+ StringBuilder clusterList = new StringBuilder();
+ for (int i = 0; i < connectedNodes.length; i++) {
+ if (i > 0) {
+ clusterList.append(",");
+ }
+ clusterList.append("tcp://" + getHost(connectedNodes[i]) + ":" +
getPort(connectedNodes[i]));
+ }
+
+ parameters.add(homeLocation.getAbsolutePath() + "/bin/artemis");
+ parameters.add("create");
+ parameters.add("--silent");
+ parameters.add("--user");
+ parameters.add("guest");
+ parameters.add("--password");
+ parameters.add("guest");
+ parameters.add("--port-offset");
+ parameters.add(String.valueOf(getPortOffeset(serverID)));
+ parameters.add("--allow-anonymous");
+ parameters.add("--no-web");
+ parameters.add("--no-autotune");
+ parameters.add("--host");
+ parameters.add("localhost");
+ parameters.add("--clustered");
+ parameters.add("--staticCluster");
+ parameters.add(clusterList.toString());
+ parameters.add("--replicated");
+ if (!live) {
+ parameters.add("--slave");
+ }
+ parameters.add("--no-amqp-acceptor");
+ parameters.add("--no-mqtt-acceptor");
+ parameters.add("--no-hornetq-acceptor");
+ parameters.add("--no-stomp-acceptor");
+ parameters.add(serverLocation.getAbsolutePath());
+
+ ProcessBuilder processBuilder = new ProcessBuilder();
+ processBuilder.command(parameters.toArray(new
String[parameters.size()]));
+ Process process = processBuilder.start();
+ SpawnedVMSupport.spawnLoggers(null, null, "ArtemisCreate", true, true,
process);
+ Assertions.assertTrue(process.waitFor(10, TimeUnit.SECONDS));
+
+ File brokerXml = new File(serverLocation, "/etc/broker.xml");
+
+ if (live) {
+ boolean replacedMaster = FileUtil.findReplace(brokerXml,
"</primary>", " <group-name>" + replicaGroupName + "</group-name>\n" + "
<check-for-live-server>true</check-for-live-server>\n" + "
<quorum-size>2</quorum-size>\n" + " </primary>");
+ replacedMaster |= FileUtil.findReplace(brokerXml, "</master>", "
<group-name>" + replicaGroupName + "</group-name>\n" + "
<check-for-live-server>true</check-for-live-server>\n" + "
<quorum-size>2</quorum-size>\n" + " </master>");
+
+ Assertions.assertTrue(replacedMaster, "couldn't find either master or
primary on broker.xml");
+ } else {
+ boolean replacedSlave = FileUtil.findReplace(brokerXml, "<slave/>",
"<slave>\n" + " <group-name>" + replicaGroupName +
"</group-name>\n" + " <allow-failback>false</allow-failback>\n" +
" <quorum-size>2</quorum-size>\n" + " </slave>");
+
+ replacedSlave |= FileUtil.findReplace(brokerXml, "<backup/>",
"<backup>\n" + " <group-name>" + replicaGroupName +
"</group-name>\n" + " <allow-failback>false</allow-failback>\n" +
" <quorum-size>2</quorum-size>\n" + " </backup>");
+ Assertions.assertTrue(replacedSlave, "couldn't find slave on backup
to replace on broker.xml");
+ }
+
+ }
+
+ public static void createServers(File sourceHome) throws Exception {
+ newServer(sourceHome, LIVE_0, 0, "live0", true, new int[]{1, 2, 3, 4,
5});
+ newServer(sourceHome, LIVE_1, 1, "live1", true, new int[]{0, 2, 3, 4,
5});
+ newServer(sourceHome, LIVE_2, 2, "live2", true, new int[]{0, 1, 3, 4,
5});
+ newServer(sourceHome, BKP_0, 3, "live0", false, new int[]{0, 1, 2, 4,
5});
+ newServer(sourceHome, BKP_1, 4, "live1", false, new int[]{0, 1, 2, 3,
5});
+ newServer(sourceHome, BKP_2, 5, "live2", false, new int[]{0, 1, 2, 3,
4});
+ }
+
+ private void upgrade(File home, File instance) throws Exception {
+ ProcessBuilder upgradeBuilder = new ProcessBuilder();
+ upgradeBuilder.command(home.getAbsolutePath() + "/bin/artemis",
"upgrade", instance.getAbsolutePath());
+ Process process = upgradeBuilder.start();
+ SpawnedVMSupport.spawnLoggers(null, null, null, true, true, process);
+ Assertions.assertTrue(process.waitFor(10, TimeUnit.SECONDS));
+
+
+ /*File brokerXml = new File(instance, "/etc/broker.xml");
+ FileUtil.findReplace(brokerXml, "master", "primary");
+ FileUtil.findReplace(brokerXml, "<slave>", "");
+ FileUtil.findReplace(brokerXml, "</slave>", ""); */
+ }
+
+ @Test
+ public void testRolledUpgrade_2_30() throws Exception {
+ testRolledOnZipDistribution(TWO_THIRTY_ZIP, "apache-artemis-2.30.0");
+ }
+
+ @Test
+ public void testRolledUpgrade_2_35() throws Exception {
+ testRolledOnZipDistribution(TWO_THIRTY_FIVE_ZIP,
"apache-artemis-2.35.0");
+ }
+
+ private void testRolledOnZipDistribution(String zip, String destination)
throws Exception {
+ File twoThirtyZip = new File(zip);
+ Assumptions.assumeTrue(twoThirtyZip.exists());
+ File distFolder = new File("./target/old-releases", destination);
+ unzipJava(twoThirtyZip, new File("./target/old-releases/"), distFolder);
+ testRolledUpgrade(distFolder);
+ }
+
+ public void testRolledUpgrade(File distribution) throws Exception {
Review Comment:
private?
##########
tests/compatibility-tests/pom.xml:
##########
@@ -265,6 +271,39 @@
</plugins>
</build>
<profiles>
+ <profile>
+ <id>compatibility-real-servers</id>
Review Comment:
What about _compatibility-tests-distribution_ so that it shares a nicer
prefix with the other profile, and is more descriptive of the difference
between them?
Also, with this having its own profile, its likely not going to actually run
in any CI jobs currently, that intended?
##########
tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/realServer/RolledUpgradeTest.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * 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.activemq.artemis.tests.compatibility.realServer;
+
+import java.io.File;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.api.core.Pair;
+import org.apache.activemq.artemis.api.core.management.SimpleManagement;
+import org.apache.activemq.artemis.util.ServerUtil;
+import org.apache.activemq.artemis.utils.FileUtil;
+import org.apache.activemq.artemis.utils.SpawnedVMSupport;
+import org.apache.activemq.artemis.utils.Wait;
+import org.apache.activemq.artemis.utils.cli.helper.HelperBase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RolledUpgradeTest extends CompatBaseTest {
+
+ private static final Logger logger =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TWO_THIRTY_ZIP =
"./target/old-releases/apache-artemis-2.30.0-bin.zip";
+ private static final String TWO_THIRTY_FIVE_ZIP =
"./target/old-releases/apache-artemis-2.35.0-bin.zip";
+
+ //private static String sourceHome = HelperBase.getHome().getAbsolutePath();
+
+ public static final String LIVE_0 = "RolledUpgradeTest/live0";
+ public static final String LIVE_1 = "RolledUpgradeTest/live1";
+ public static final String LIVE_2 = "RolledUpgradeTest/live2";
+
+ public static final String BKP_0 = "RolledUpgradeTest/bkp0";
+ public static final String BKP_1 = "RolledUpgradeTest/bkp1";
+ public static final String BKP_2 = "RolledUpgradeTest/bkp2";
+
+ Process live0Process;
+ Process live1Process;
+ Process live2Process;
+
+ Process bkp0Process;
+ Process bkp1Process;
+ Process bkp2Process;
+
+ private static String getHost(int nodeID) {
+ return "localhost";
+ }
+
+ private static int getPort(int nodeID) {
+ return 61616 + nodeID;
+ }
+
+ private static int getPortOffeset(int nodeID) {
+ return nodeID;
+ }
+
+ private static void newServer(File homeLocation,
+ String location,
+ int serverID,
+ String replicaGroupName,
+ boolean live,
+ int[] connectedNodes) throws Exception {
+ File serverLocation = getFileServerLocation(location);
+ List<String> parameters = new ArrayList<>();
+ deleteDirectory(serverLocation);
+
+ StringBuilder clusterList = new StringBuilder();
+ for (int i = 0; i < connectedNodes.length; i++) {
+ if (i > 0) {
+ clusterList.append(",");
+ }
+ clusterList.append("tcp://" + getHost(connectedNodes[i]) + ":" +
getPort(connectedNodes[i]));
+ }
+
+ parameters.add(homeLocation.getAbsolutePath() + "/bin/artemis");
+ parameters.add("create");
+ parameters.add("--silent");
+ parameters.add("--user");
+ parameters.add("guest");
+ parameters.add("--password");
+ parameters.add("guest");
+ parameters.add("--port-offset");
+ parameters.add(String.valueOf(getPortOffeset(serverID)));
+ parameters.add("--allow-anonymous");
+ parameters.add("--no-web");
+ parameters.add("--no-autotune");
+ parameters.add("--host");
+ parameters.add("localhost");
+ parameters.add("--clustered");
+ parameters.add("--staticCluster");
+ parameters.add(clusterList.toString());
+ parameters.add("--replicated");
+ if (!live) {
+ parameters.add("--slave");
+ }
+ parameters.add("--no-amqp-acceptor");
+ parameters.add("--no-mqtt-acceptor");
+ parameters.add("--no-hornetq-acceptor");
+ parameters.add("--no-stomp-acceptor");
+ parameters.add(serverLocation.getAbsolutePath());
+
+ ProcessBuilder processBuilder = new ProcessBuilder();
+ processBuilder.command(parameters.toArray(new
String[parameters.size()]));
+ Process process = processBuilder.start();
+ SpawnedVMSupport.spawnLoggers(null, null, "ArtemisCreate", true, true,
process);
+ Assertions.assertTrue(process.waitFor(10, TimeUnit.SECONDS));
+
+ File brokerXml = new File(serverLocation, "/etc/broker.xml");
+
+ if (live) {
+ boolean replacedMaster = FileUtil.findReplace(brokerXml,
"</primary>", " <group-name>" + replicaGroupName + "</group-name>\n" + "
<check-for-live-server>true</check-for-live-server>\n" + "
<quorum-size>2</quorum-size>\n" + " </primary>");
+ replacedMaster |= FileUtil.findReplace(brokerXml, "</master>", "
<group-name>" + replicaGroupName + "</group-name>\n" + "
<check-for-live-server>true</check-for-live-server>\n" + "
<quorum-size>2</quorum-size>\n" + " </master>");
+
+ Assertions.assertTrue(replacedMaster, "couldn't find either master or
primary on broker.xml");
+ } else {
+ boolean replacedSlave = FileUtil.findReplace(brokerXml, "<slave/>",
"<slave>\n" + " <group-name>" + replicaGroupName +
"</group-name>\n" + " <allow-failback>false</allow-failback>\n" +
" <quorum-size>2</quorum-size>\n" + " </slave>");
+
+ replacedSlave |= FileUtil.findReplace(brokerXml, "<backup/>",
"<backup>\n" + " <group-name>" + replicaGroupName +
"</group-name>\n" + " <allow-failback>false</allow-failback>\n" +
" <quorum-size>2</quorum-size>\n" + " </backup>");
+ Assertions.assertTrue(replacedSlave, "couldn't find slave on backup
to replace on broker.xml");
+ }
+
+ }
+
+ public static void createServers(File sourceHome) throws Exception {
+ newServer(sourceHome, LIVE_0, 0, "live0", true, new int[]{1, 2, 3, 4,
5});
+ newServer(sourceHome, LIVE_1, 1, "live1", true, new int[]{0, 2, 3, 4,
5});
+ newServer(sourceHome, LIVE_2, 2, "live2", true, new int[]{0, 1, 3, 4,
5});
+ newServer(sourceHome, BKP_0, 3, "live0", false, new int[]{0, 1, 2, 4,
5});
+ newServer(sourceHome, BKP_1, 4, "live1", false, new int[]{0, 1, 2, 3,
5});
+ newServer(sourceHome, BKP_2, 5, "live2", false, new int[]{0, 1, 2, 3,
4});
+ }
+
+ private void upgrade(File home, File instance) throws Exception {
+ ProcessBuilder upgradeBuilder = new ProcessBuilder();
+ upgradeBuilder.command(home.getAbsolutePath() + "/bin/artemis",
"upgrade", instance.getAbsolutePath());
+ Process process = upgradeBuilder.start();
+ SpawnedVMSupport.spawnLoggers(null, null, null, true, true, process);
+ Assertions.assertTrue(process.waitFor(10, TimeUnit.SECONDS));
+
+
+ /*File brokerXml = new File(instance, "/etc/broker.xml");
+ FileUtil.findReplace(brokerXml, "master", "primary");
+ FileUtil.findReplace(brokerXml, "<slave>", "");
+ FileUtil.findReplace(brokerXml, "</slave>", ""); */
+ }
+
+ @Test
+ public void testRolledUpgrade_2_30() throws Exception {
+ testRolledOnZipDistribution(TWO_THIRTY_ZIP, "apache-artemis-2.30.0");
+ }
+
+ @Test
+ public void testRolledUpgrade_2_35() throws Exception {
+ testRolledOnZipDistribution(TWO_THIRTY_FIVE_ZIP,
"apache-artemis-2.35.0");
+ }
+
+ private void testRolledOnZipDistribution(String zip, String destination)
throws Exception {
+ File twoThirtyZip = new File(zip);
+ Assumptions.assumeTrue(twoThirtyZip.exists());
Review Comment:
Similar comment as before around dropping _Assumptions._ prefix
##########
tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/realServer/RolledUpgradeTest.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * 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.activemq.artemis.tests.compatibility.realServer;
+
+import java.io.File;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.api.core.Pair;
+import org.apache.activemq.artemis.api.core.management.SimpleManagement;
+import org.apache.activemq.artemis.util.ServerUtil;
+import org.apache.activemq.artemis.utils.FileUtil;
+import org.apache.activemq.artemis.utils.SpawnedVMSupport;
+import org.apache.activemq.artemis.utils.Wait;
+import org.apache.activemq.artemis.utils.cli.helper.HelperBase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RolledUpgradeTest extends CompatBaseTest {
+
+ private static final Logger logger =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TWO_THIRTY_ZIP =
"./target/old-releases/apache-artemis-2.30.0-bin.zip";
+ private static final String TWO_THIRTY_FIVE_ZIP =
"./target/old-releases/apache-artemis-2.35.0-bin.zip";
+
+ //private static String sourceHome = HelperBase.getHome().getAbsolutePath();
+
+ public static final String LIVE_0 = "RolledUpgradeTest/live0";
+ public static final String LIVE_1 = "RolledUpgradeTest/live1";
+ public static final String LIVE_2 = "RolledUpgradeTest/live2";
+
+ public static final String BKP_0 = "RolledUpgradeTest/bkp0";
+ public static final String BKP_1 = "RolledUpgradeTest/bkp1";
+ public static final String BKP_2 = "RolledUpgradeTest/bkp2";
+
+ Process live0Process;
+ Process live1Process;
+ Process live2Process;
+
+ Process bkp0Process;
+ Process bkp1Process;
+ Process bkp2Process;
+
+ private static String getHost(int nodeID) {
+ return "localhost";
+ }
+
+ private static int getPort(int nodeID) {
+ return 61616 + nodeID;
+ }
+
+ private static int getPortOffeset(int nodeID) {
+ return nodeID;
+ }
+
+ private static void newServer(File homeLocation,
+ String location,
+ int serverID,
+ String replicaGroupName,
+ boolean live,
+ int[] connectedNodes) throws Exception {
+ File serverLocation = getFileServerLocation(location);
+ List<String> parameters = new ArrayList<>();
+ deleteDirectory(serverLocation);
+
+ StringBuilder clusterList = new StringBuilder();
+ for (int i = 0; i < connectedNodes.length; i++) {
+ if (i > 0) {
+ clusterList.append(",");
+ }
+ clusterList.append("tcp://" + getHost(connectedNodes[i]) + ":" +
getPort(connectedNodes[i]));
+ }
+
+ parameters.add(homeLocation.getAbsolutePath() + "/bin/artemis");
+ parameters.add("create");
+ parameters.add("--silent");
+ parameters.add("--user");
+ parameters.add("guest");
+ parameters.add("--password");
+ parameters.add("guest");
+ parameters.add("--port-offset");
+ parameters.add(String.valueOf(getPortOffeset(serverID)));
+ parameters.add("--allow-anonymous");
+ parameters.add("--no-web");
+ parameters.add("--no-autotune");
+ parameters.add("--host");
+ parameters.add("localhost");
+ parameters.add("--clustered");
+ parameters.add("--staticCluster");
+ parameters.add(clusterList.toString());
+ parameters.add("--replicated");
+ if (!live) {
+ parameters.add("--slave");
+ }
+ parameters.add("--no-amqp-acceptor");
+ parameters.add("--no-mqtt-acceptor");
+ parameters.add("--no-hornetq-acceptor");
+ parameters.add("--no-stomp-acceptor");
+ parameters.add(serverLocation.getAbsolutePath());
+
+ ProcessBuilder processBuilder = new ProcessBuilder();
+ processBuilder.command(parameters.toArray(new
String[parameters.size()]));
+ Process process = processBuilder.start();
+ SpawnedVMSupport.spawnLoggers(null, null, "ArtemisCreate", true, true,
process);
+ Assertions.assertTrue(process.waitFor(10, TimeUnit.SECONDS));
+
+ File brokerXml = new File(serverLocation, "/etc/broker.xml");
+
+ if (live) {
+ boolean replacedMaster = FileUtil.findReplace(brokerXml,
"</primary>", " <group-name>" + replicaGroupName + "</group-name>\n" + "
<check-for-live-server>true</check-for-live-server>\n" + "
<quorum-size>2</quorum-size>\n" + " </primary>");
+ replacedMaster |= FileUtil.findReplace(brokerXml, "</master>", "
<group-name>" + replicaGroupName + "</group-name>\n" + "
<check-for-live-server>true</check-for-live-server>\n" + "
<quorum-size>2</quorum-size>\n" + " </master>");
+
+ Assertions.assertTrue(replacedMaster, "couldn't find either master or
primary on broker.xml");
+ } else {
+ boolean replacedSlave = FileUtil.findReplace(brokerXml, "<slave/>",
"<slave>\n" + " <group-name>" + replicaGroupName +
"</group-name>\n" + " <allow-failback>false</allow-failback>\n" +
" <quorum-size>2</quorum-size>\n" + " </slave>");
+
+ replacedSlave |= FileUtil.findReplace(brokerXml, "<backup/>",
"<backup>\n" + " <group-name>" + replicaGroupName +
"</group-name>\n" + " <allow-failback>false</allow-failback>\n" +
" <quorum-size>2</quorum-size>\n" + " </backup>");
+ Assertions.assertTrue(replacedSlave, "couldn't find slave on backup
to replace on broker.xml");
+ }
+
+ }
+
+ public static void createServers(File sourceHome) throws Exception {
+ newServer(sourceHome, LIVE_0, 0, "live0", true, new int[]{1, 2, 3, 4,
5});
+ newServer(sourceHome, LIVE_1, 1, "live1", true, new int[]{0, 2, 3, 4,
5});
+ newServer(sourceHome, LIVE_2, 2, "live2", true, new int[]{0, 1, 3, 4,
5});
+ newServer(sourceHome, BKP_0, 3, "live0", false, new int[]{0, 1, 2, 4,
5});
+ newServer(sourceHome, BKP_1, 4, "live1", false, new int[]{0, 1, 2, 3,
5});
+ newServer(sourceHome, BKP_2, 5, "live2", false, new int[]{0, 1, 2, 3,
4});
+ }
+
+ private void upgrade(File home, File instance) throws Exception {
+ ProcessBuilder upgradeBuilder = new ProcessBuilder();
+ upgradeBuilder.command(home.getAbsolutePath() + "/bin/artemis",
"upgrade", instance.getAbsolutePath());
+ Process process = upgradeBuilder.start();
+ SpawnedVMSupport.spawnLoggers(null, null, null, true, true, process);
+ Assertions.assertTrue(process.waitFor(10, TimeUnit.SECONDS));
+
+
+ /*File brokerXml = new File(instance, "/etc/broker.xml");
+ FileUtil.findReplace(brokerXml, "master", "primary");
+ FileUtil.findReplace(brokerXml, "<slave>", "");
+ FileUtil.findReplace(brokerXml, "</slave>", ""); */
Review Comment:
Remove leftover?
##########
tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/realServer/CompatBaseTest.java:
##########
@@ -0,0 +1,75 @@
+/*
+ * 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.activemq.artemis.tests.compatibility.realServer;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.FileOutputStream;
+import java.lang.invoke.MethodHandles;
+import java.util.zip.ZipEntry;
+import java.util.zip.ZipInputStream;
+
+import org.apache.activemq.artemis.utils.RealServerTestBase;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class CompatBaseTest extends RealServerTestBase {
Review Comment:
Given the only thing in here is a static method, would it not be nicer as a
utility class rather than part of the test class hierarchy?
EDIT: or seeing that all it does is unzip, would using the
maven-dependency-plugin _unpack_ goal be simpler?
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/quorum/QuorumVote.java:
##########
@@ -36,6 +39,7 @@ public QuorumVote(SimpleString name) {
*
* @return the vote to use
*/
+
Review Comment:
Erroneous newline
##########
tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/realServer/RolledUpgradeTest.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * 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.activemq.artemis.tests.compatibility.realServer;
+
+import java.io.File;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.api.core.Pair;
+import org.apache.activemq.artemis.api.core.management.SimpleManagement;
+import org.apache.activemq.artemis.util.ServerUtil;
+import org.apache.activemq.artemis.utils.FileUtil;
+import org.apache.activemq.artemis.utils.SpawnedVMSupport;
+import org.apache.activemq.artemis.utils.Wait;
+import org.apache.activemq.artemis.utils.cli.helper.HelperBase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RolledUpgradeTest extends CompatBaseTest {
+
+ private static final Logger logger =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TWO_THIRTY_ZIP =
"./target/old-releases/apache-artemis-2.30.0-bin.zip";
+ private static final String TWO_THIRTY_FIVE_ZIP =
"./target/old-releases/apache-artemis-2.35.0-bin.zip";
+
+ //private static String sourceHome = HelperBase.getHome().getAbsolutePath();
+
+ public static final String LIVE_0 = "RolledUpgradeTest/live0";
+ public static final String LIVE_1 = "RolledUpgradeTest/live1";
+ public static final String LIVE_2 = "RolledUpgradeTest/live2";
+
+ public static final String BKP_0 = "RolledUpgradeTest/bkp0";
+ public static final String BKP_1 = "RolledUpgradeTest/bkp1";
+ public static final String BKP_2 = "RolledUpgradeTest/bkp2";
+
+ Process live0Process;
+ Process live1Process;
+ Process live2Process;
+
+ Process bkp0Process;
+ Process bkp1Process;
+ Process bkp2Process;
+
+ private static String getHost(int nodeID) {
+ return "localhost";
+ }
+
+ private static int getPort(int nodeID) {
+ return 61616 + nodeID;
+ }
+
+ private static int getPortOffeset(int nodeID) {
+ return nodeID;
+ }
+
+ private static void newServer(File homeLocation,
+ String location,
+ int serverID,
+ String replicaGroupName,
+ boolean live,
+ int[] connectedNodes) throws Exception {
+ File serverLocation = getFileServerLocation(location);
+ List<String> parameters = new ArrayList<>();
+ deleteDirectory(serverLocation);
+
+ StringBuilder clusterList = new StringBuilder();
+ for (int i = 0; i < connectedNodes.length; i++) {
+ if (i > 0) {
+ clusterList.append(",");
+ }
+ clusterList.append("tcp://" + getHost(connectedNodes[i]) + ":" +
getPort(connectedNodes[i]));
+ }
+
+ parameters.add(homeLocation.getAbsolutePath() + "/bin/artemis");
+ parameters.add("create");
+ parameters.add("--silent");
+ parameters.add("--user");
+ parameters.add("guest");
+ parameters.add("--password");
+ parameters.add("guest");
+ parameters.add("--port-offset");
+ parameters.add(String.valueOf(getPortOffeset(serverID)));
+ parameters.add("--allow-anonymous");
+ parameters.add("--no-web");
+ parameters.add("--no-autotune");
+ parameters.add("--host");
+ parameters.add("localhost");
+ parameters.add("--clustered");
+ parameters.add("--staticCluster");
+ parameters.add(clusterList.toString());
+ parameters.add("--replicated");
+ if (!live) {
+ parameters.add("--slave");
+ }
+ parameters.add("--no-amqp-acceptor");
+ parameters.add("--no-mqtt-acceptor");
+ parameters.add("--no-hornetq-acceptor");
+ parameters.add("--no-stomp-acceptor");
+ parameters.add(serverLocation.getAbsolutePath());
+
+ ProcessBuilder processBuilder = new ProcessBuilder();
+ processBuilder.command(parameters.toArray(new
String[parameters.size()]));
+ Process process = processBuilder.start();
+ SpawnedVMSupport.spawnLoggers(null, null, "ArtemisCreate", true, true,
process);
+ Assertions.assertTrue(process.waitFor(10, TimeUnit.SECONDS));
+
+ File brokerXml = new File(serverLocation, "/etc/broker.xml");
+
+ if (live) {
+ boolean replacedMaster = FileUtil.findReplace(brokerXml,
"</primary>", " <group-name>" + replicaGroupName + "</group-name>\n" + "
<check-for-live-server>true</check-for-live-server>\n" + "
<quorum-size>2</quorum-size>\n" + " </primary>");
+ replacedMaster |= FileUtil.findReplace(brokerXml, "</master>", "
<group-name>" + replicaGroupName + "</group-name>\n" + "
<check-for-live-server>true</check-for-live-server>\n" + "
<quorum-size>2</quorum-size>\n" + " </master>");
+
+ Assertions.assertTrue(replacedMaster, "couldn't find either master or
primary on broker.xml");
+ } else {
+ boolean replacedSlave = FileUtil.findReplace(brokerXml, "<slave/>",
"<slave>\n" + " <group-name>" + replicaGroupName +
"</group-name>\n" + " <allow-failback>false</allow-failback>\n" +
" <quorum-size>2</quorum-size>\n" + " </slave>");
+
+ replacedSlave |= FileUtil.findReplace(brokerXml, "<backup/>",
"<backup>\n" + " <group-name>" + replicaGroupName +
"</group-name>\n" + " <allow-failback>false</allow-failback>\n" +
" <quorum-size>2</quorum-size>\n" + " </backup>");
+ Assertions.assertTrue(replacedSlave, "couldn't find slave on backup
to replace on broker.xml");
+ }
+
+ }
+
+ public static void createServers(File sourceHome) throws Exception {
+ newServer(sourceHome, LIVE_0, 0, "live0", true, new int[]{1, 2, 3, 4,
5});
+ newServer(sourceHome, LIVE_1, 1, "live1", true, new int[]{0, 2, 3, 4,
5});
+ newServer(sourceHome, LIVE_2, 2, "live2", true, new int[]{0, 1, 3, 4,
5});
+ newServer(sourceHome, BKP_0, 3, "live0", false, new int[]{0, 1, 2, 4,
5});
+ newServer(sourceHome, BKP_1, 4, "live1", false, new int[]{0, 1, 2, 3,
5});
+ newServer(sourceHome, BKP_2, 5, "live2", false, new int[]{0, 1, 2, 3,
4});
+ }
+
+ private void upgrade(File home, File instance) throws Exception {
+ ProcessBuilder upgradeBuilder = new ProcessBuilder();
+ upgradeBuilder.command(home.getAbsolutePath() + "/bin/artemis",
"upgrade", instance.getAbsolutePath());
+ Process process = upgradeBuilder.start();
+ SpawnedVMSupport.spawnLoggers(null, null, null, true, true, process);
+ Assertions.assertTrue(process.waitFor(10, TimeUnit.SECONDS));
+
+
+ /*File brokerXml = new File(instance, "/etc/broker.xml");
+ FileUtil.findReplace(brokerXml, "master", "primary");
+ FileUtil.findReplace(brokerXml, "<slave>", "");
+ FileUtil.findReplace(brokerXml, "</slave>", ""); */
+ }
+
+ @Test
+ public void testRolledUpgrade_2_30() throws Exception {
+ testRolledOnZipDistribution(TWO_THIRTY_ZIP, "apache-artemis-2.30.0");
+ }
+
+ @Test
+ public void testRolledUpgrade_2_35() throws Exception {
+ testRolledOnZipDistribution(TWO_THIRTY_FIVE_ZIP,
"apache-artemis-2.35.0");
+ }
+
+ private void testRolledOnZipDistribution(String zip, String destination)
throws Exception {
+ File twoThirtyZip = new File(zip);
Review Comment:
Looks like variable name needs updated to be more general
##########
artemis-core-client/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/ChannelImpl.java:
##########
@@ -460,7 +468,9 @@ public Packet sendBlocking(final Packet packet, byte
expectedPacket) throws Acti
@Override
public Packet sendBlocking(final Packet packet,
final int reconnectID,
- byte expectedPacket) throws ActiveMQException {
+ byte expectedPacket,
+ final long timeout,
+ boolean failOnTimeout) throws ActiveMQException {
Review Comment:
Should they all be final?
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/quorum/QuorumVoteServerConnect.java:
##########
@@ -28,7 +28,12 @@
*/
public class QuorumVoteServerConnect extends QuorumVote<ServerConnectVote,
Boolean> {
+
+ /** NOTE: The following String is used to identify the targetNode
implementation at other servers.
+ * Renaming such string would cause incompatibility changes. */
public static final SimpleString PRIMARY_FAILOVER_VOTE =
SimpleString.of("PrimaryFailoverQuorumVote");
+ public static final SimpleString OLD_PRIMARY_FAILOVER_NOTE =
SimpleString.of("LiveFailoverQuorumVote");
Review Comment:
Was NOTE intended, or meant to be VOTE?
##########
tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/realServer/RolledUpgradeTest.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * 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.activemq.artemis.tests.compatibility.realServer;
+
+import java.io.File;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.api.core.Pair;
+import org.apache.activemq.artemis.api.core.management.SimpleManagement;
+import org.apache.activemq.artemis.util.ServerUtil;
+import org.apache.activemq.artemis.utils.FileUtil;
+import org.apache.activemq.artemis.utils.SpawnedVMSupport;
+import org.apache.activemq.artemis.utils.Wait;
+import org.apache.activemq.artemis.utils.cli.helper.HelperBase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RolledUpgradeTest extends CompatBaseTest {
+
+ private static final Logger logger =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TWO_THIRTY_ZIP =
"./target/old-releases/apache-artemis-2.30.0-bin.zip";
+ private static final String TWO_THIRTY_FIVE_ZIP =
"./target/old-releases/apache-artemis-2.35.0-bin.zip";
+
+ //private static String sourceHome = HelperBase.getHome().getAbsolutePath();
+
+ public static final String LIVE_0 = "RolledUpgradeTest/live0";
+ public static final String LIVE_1 = "RolledUpgradeTest/live1";
+ public static final String LIVE_2 = "RolledUpgradeTest/live2";
+
+ public static final String BKP_0 = "RolledUpgradeTest/bkp0";
+ public static final String BKP_1 = "RolledUpgradeTest/bkp1";
+ public static final String BKP_2 = "RolledUpgradeTest/bkp2";
+
+ Process live0Process;
+ Process live1Process;
+ Process live2Process;
+
+ Process bkp0Process;
+ Process bkp1Process;
+ Process bkp2Process;
+
+ private static String getHost(int nodeID) {
+ return "localhost";
+ }
+
+ private static int getPort(int nodeID) {
+ return 61616 + nodeID;
+ }
+
+ private static int getPortOffeset(int nodeID) {
+ return nodeID;
+ }
+
+ private static void newServer(File homeLocation,
+ String location,
+ int serverID,
+ String replicaGroupName,
+ boolean live,
+ int[] connectedNodes) throws Exception {
+ File serverLocation = getFileServerLocation(location);
+ List<String> parameters = new ArrayList<>();
+ deleteDirectory(serverLocation);
+
+ StringBuilder clusterList = new StringBuilder();
+ for (int i = 0; i < connectedNodes.length; i++) {
+ if (i > 0) {
+ clusterList.append(",");
+ }
+ clusterList.append("tcp://" + getHost(connectedNodes[i]) + ":" +
getPort(connectedNodes[i]));
+ }
+
+ parameters.add(homeLocation.getAbsolutePath() + "/bin/artemis");
+ parameters.add("create");
+ parameters.add("--silent");
+ parameters.add("--user");
+ parameters.add("guest");
+ parameters.add("--password");
+ parameters.add("guest");
+ parameters.add("--port-offset");
+ parameters.add(String.valueOf(getPortOffeset(serverID)));
+ parameters.add("--allow-anonymous");
+ parameters.add("--no-web");
+ parameters.add("--no-autotune");
+ parameters.add("--host");
+ parameters.add("localhost");
+ parameters.add("--clustered");
+ parameters.add("--staticCluster");
+ parameters.add(clusterList.toString());
+ parameters.add("--replicated");
+ if (!live) {
+ parameters.add("--slave");
+ }
+ parameters.add("--no-amqp-acceptor");
+ parameters.add("--no-mqtt-acceptor");
+ parameters.add("--no-hornetq-acceptor");
+ parameters.add("--no-stomp-acceptor");
+ parameters.add(serverLocation.getAbsolutePath());
+
+ ProcessBuilder processBuilder = new ProcessBuilder();
+ processBuilder.command(parameters.toArray(new
String[parameters.size()]));
+ Process process = processBuilder.start();
+ SpawnedVMSupport.spawnLoggers(null, null, "ArtemisCreate", true, true,
process);
+ Assertions.assertTrue(process.waitFor(10, TimeUnit.SECONDS));
+
+ File brokerXml = new File(serverLocation, "/etc/broker.xml");
+
+ if (live) {
+ boolean replacedMaster = FileUtil.findReplace(brokerXml,
"</primary>", " <group-name>" + replicaGroupName + "</group-name>\n" + "
<check-for-live-server>true</check-for-live-server>\n" + "
<quorum-size>2</quorum-size>\n" + " </primary>");
+ replacedMaster |= FileUtil.findReplace(brokerXml, "</master>", "
<group-name>" + replicaGroupName + "</group-name>\n" + "
<check-for-live-server>true</check-for-live-server>\n" + "
<quorum-size>2</quorum-size>\n" + " </master>");
+
+ Assertions.assertTrue(replacedMaster, "couldn't find either master or
primary on broker.xml");
+ } else {
+ boolean replacedSlave = FileUtil.findReplace(brokerXml, "<slave/>",
"<slave>\n" + " <group-name>" + replicaGroupName +
"</group-name>\n" + " <allow-failback>false</allow-failback>\n" +
" <quorum-size>2</quorum-size>\n" + " </slave>");
+
+ replacedSlave |= FileUtil.findReplace(brokerXml, "<backup/>",
"<backup>\n" + " <group-name>" + replicaGroupName +
"</group-name>\n" + " <allow-failback>false</allow-failback>\n" +
" <quorum-size>2</quorum-size>\n" + " </backup>");
+ Assertions.assertTrue(replacedSlave, "couldn't find slave on backup
to replace on broker.xml");
+ }
+
+ }
+
+ public static void createServers(File sourceHome) throws Exception {
+ newServer(sourceHome, LIVE_0, 0, "live0", true, new int[]{1, 2, 3, 4,
5});
+ newServer(sourceHome, LIVE_1, 1, "live1", true, new int[]{0, 2, 3, 4,
5});
+ newServer(sourceHome, LIVE_2, 2, "live2", true, new int[]{0, 1, 3, 4,
5});
+ newServer(sourceHome, BKP_0, 3, "live0", false, new int[]{0, 1, 2, 4,
5});
+ newServer(sourceHome, BKP_1, 4, "live1", false, new int[]{0, 1, 2, 3,
5});
+ newServer(sourceHome, BKP_2, 5, "live2", false, new int[]{0, 1, 2, 3,
4});
+ }
+
+ private void upgrade(File home, File instance) throws Exception {
+ ProcessBuilder upgradeBuilder = new ProcessBuilder();
+ upgradeBuilder.command(home.getAbsolutePath() + "/bin/artemis",
"upgrade", instance.getAbsolutePath());
+ Process process = upgradeBuilder.start();
+ SpawnedVMSupport.spawnLoggers(null, null, null, true, true, process);
+ Assertions.assertTrue(process.waitFor(10, TimeUnit.SECONDS));
+
+
+ /*File brokerXml = new File(instance, "/etc/broker.xml");
+ FileUtil.findReplace(brokerXml, "master", "primary");
+ FileUtil.findReplace(brokerXml, "<slave>", "");
+ FileUtil.findReplace(brokerXml, "</slave>", ""); */
+ }
+
+ @Test
+ public void testRolledUpgrade_2_30() throws Exception {
+ testRolledOnZipDistribution(TWO_THIRTY_ZIP, "apache-artemis-2.30.0");
+ }
+
+ @Test
+ public void testRolledUpgrade_2_35() throws Exception {
+ testRolledOnZipDistribution(TWO_THIRTY_FIVE_ZIP,
"apache-artemis-2.35.0");
+ }
+
+ private void testRolledOnZipDistribution(String zip, String destination)
throws Exception {
+ File twoThirtyZip = new File(zip);
+ Assumptions.assumeTrue(twoThirtyZip.exists());
+ File distFolder = new File("./target/old-releases", destination);
+ unzipJava(twoThirtyZip, new File("./target/old-releases/"), distFolder);
+ testRolledUpgrade(distFolder);
+ }
+
+ public void testRolledUpgrade(File distribution) throws Exception {
+ createServers(distribution);
+ live0Process = startServer(LIVE_0, 0, -1);
+ live1Process = startServer(LIVE_1, 1, -1);
+ live2Process = startServer(LIVE_2, 2, -1);
+
+ ServerUtil.waitForServerToStart(0, 30_000);
+ ServerUtil.waitForServerToStart(1, 30_000);
+ ServerUtil.waitForServerToStart(2, 30_000);
+
+ SimpleManagement managementLive0 = new
SimpleManagement("tcp://localhost:61616", null, null);
+ SimpleManagement managementLive1 = new
SimpleManagement("tcp://localhost:61617", null, null);
+ SimpleManagement managementLive2 = new
SimpleManagement("tcp://localhost:61618", null, null);
+ runAfter(managementLive0::close);
+ runAfter(managementLive1::close);
+ runAfter(managementLive2::close);
+
+ SimpleManagement managementBKP0 = new
SimpleManagement("tcp://localhost:61619", null, null);
+ SimpleManagement managementBKP1 = new
SimpleManagement("tcp://localhost:61620", null, null);
+ SimpleManagement managementBKP2 = new
SimpleManagement("tcp://localhost:61621", null, null);
+
+ runAfter(managementLive0::close);
+ runAfter(managementLive1::close);
+ runAfter(managementLive2::close);
Review Comment:
Should these reference the backups rather than the lives again?
##########
tests/soak-tests/pom.xml:
##########
@@ -191,6 +191,8 @@
</dependencies>
+
+
Review Comment:
Seems like these additions can be omitted
##########
tests/compatibility-tests/pom.xml:
##########
@@ -265,6 +271,39 @@
</plugins>
</build>
<profiles>
+ <profile>
+ <id>compatibility-real-servers</id>
+ <properties>
+ <skipCompatibilityTests>false</skipCompatibilityTests>
Review Comment:
This will also enable all the other tests to be picked up by surefire even
though their deps may not have been prepared if the other profile isnt run...do
they all handle that (I see the new test handles the reverse case with an
assumptions check) or is it just the case that you _must_ run both profiles if
you run the new one?
EDIT: the answer is that the existing tests do not handle it, you need both.
Should we make it clearer (e.g a comment here) that they both need to be
enabled? E.g instead of setting this true we could have the new profile enforce
the skipCompatibilityTests prop is true, and indicate the other profile needs
to be used if it isnt.
##########
tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/realServer/RolledUpgradeTest.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * 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.activemq.artemis.tests.compatibility.realServer;
+
+import java.io.File;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.api.core.Pair;
+import org.apache.activemq.artemis.api.core.management.SimpleManagement;
+import org.apache.activemq.artemis.util.ServerUtil;
+import org.apache.activemq.artemis.utils.FileUtil;
+import org.apache.activemq.artemis.utils.SpawnedVMSupport;
+import org.apache.activemq.artemis.utils.Wait;
+import org.apache.activemq.artemis.utils.cli.helper.HelperBase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RolledUpgradeTest extends CompatBaseTest {
+
+ private static final Logger logger =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+
+ private static final String TWO_THIRTY_ZIP =
"./target/old-releases/apache-artemis-2.30.0-bin.zip";
+ private static final String TWO_THIRTY_FIVE_ZIP =
"./target/old-releases/apache-artemis-2.35.0-bin.zip";
+
+ //private static String sourceHome = HelperBase.getHome().getAbsolutePath();
Review Comment:
Remove leftover?
##########
tests/compatibility-tests/src/test/java/org/apache/activemq/artemis/tests/compatibility/realServer/RolledUpgradeTest.java:
##########
@@ -0,0 +1,295 @@
+/*
+ * 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.activemq.artemis.tests.compatibility.realServer;
+
+import java.io.File;
+import java.lang.invoke.MethodHandles;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.activemq.artemis.api.core.Pair;
+import org.apache.activemq.artemis.api.core.management.SimpleManagement;
+import org.apache.activemq.artemis.util.ServerUtil;
+import org.apache.activemq.artemis.utils.FileUtil;
+import org.apache.activemq.artemis.utils.SpawnedVMSupport;
+import org.apache.activemq.artemis.utils.Wait;
+import org.apache.activemq.artemis.utils.cli.helper.HelperBase;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Assumptions;
+import org.junit.jupiter.api.Test;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class RolledUpgradeTest extends CompatBaseTest {
Review Comment:
RollingUpgradeTest ?
##########
tests/compatibility-tests/pom.xml:
##########
@@ -265,6 +271,39 @@
</plugins>
</build>
<profiles>
+ <profile>
+ <id>compatibility-real-servers</id>
+ <properties>
+ <skipCompatibilityTests>false</skipCompatibilityTests>
+ </properties>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.activemq</groupId>
+ <artifactId>artemis-maven-plugin</artifactId>
+ <version>${project.version}</version>
+ <executions>
+ <!-- The executions of dependency-scan will calculate
dependencies for each specific version used here on this testsuite. -->
+ <execution>
+ <id>snapshot-check</id>
+ <phase>compile</phase>
+ <goals>
+ <goal>dependency-scan</goal>
+ </goals>
+ <configuration>
+ <libList>
+
<arg>org.apache.activemq:apache-artemis:zip:bin:2.30.0</arg>
+
<arg>org.apache.activemq:apache-artemis:zip:bin:2.35.0</arg>
Review Comment:
Out of interest, why 2.30.0 and 2.35.0 rather than 2.31.0 (one before the
issue was introduced in 2.32.0) and 2.36.0 (the current) ?
##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/cluster/quorum/QuorumManager.java:
##########
@@ -348,19 +377,42 @@ public synchronized void voteComplete() {
}
}
- private Vote sendQuorumVote(ClusterControl clusterControl, SimpleString
handler, Vote vote) {
+ private Vote sendQuorumVote(ClusterControl clusterControl, SimpleString
handler, SimpleString oldHandlerName, Vote vote) {
try {
final ClientSessionFactoryInternal sessionFactory =
clusterControl.getSessionFactory();
final String remoteAddress =
sessionFactory.getConnection().getRemoteAddress();
ActiveMQServerLogger.LOGGER.sendingQuorumVoteRequest(remoteAddress,
vote.toString());
- QuorumVoteReplyMessage replyMessage = (QuorumVoteReplyMessage)
clusterControl.getClusterChannel().get()
- .sendBlocking(new QuorumVoteMessage(handler, vote),
PacketImpl.QUORUM_VOTE_REPLY);
+
+ QuorumVoteReplyMessage replyMessage = null;
+
+ Channel clusterChannel = clusterControl.getClusterChannel().get();
+
+
+ // We first try the current packet with a medium timeout
+ replyMessage = (QuorumVoteReplyMessage)
clusterChannel.sendBlocking(new QuorumVoteMessage(handler, vote), -1,
PacketImpl.QUORUM_VOTE_REPLY, VOTE_RESPONSE_TIMEOUT, false);
+ logger.trace("This is the reply message from the current version =
{}", replyMessage);
+
+ // if no response, we try the previous versions, with still a medium
timeout
+ if (replyMessage == null && oldHandlerName != null) {
+ replyMessage = (QuorumVoteReplyMessage)
clusterChannel.sendBlocking(new QuorumVoteMessage(oldHandlerName, vote), -1,
PacketImpl.QUORUM_VOTE_REPLY, VOTE_RESPONSE_TIMEOUT, false);
+ logger.trace("This is the reply message from the older version =
{}", replyMessage);
+ }
Review Comment:
What happens if a reply actually does arrive, but only after these short
timeouts (2sec vs usual 30sec)? Ignored?
Also, in future could it check for a newer wire version and avoid doing this
for newer servers?
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
