You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@activemq.apache.org by cl...@apache.org on 2025/02/25 19:41:30 UTC
(activemq-artemis) branch main updated: ARTEMIS-5107 using wrong value in ReplicationStartSyncMessage ctor
This is an automated email from the ASF dual-hosted git repository.
clebertsuconic pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git
The following commit(s) were added to refs/heads/main by this push:
new 3101ac59e0 ARTEMIS-5107 using wrong value in ReplicationStartSyncMessage ctor
3101ac59e0 is described below
commit 3101ac59e09c8722f5afc378162a07f518fc7fe9
Author: Justin Bertram <jb...@apache.org>
AuthorDate: Wed Jan 15 13:02:21 2025 -0600
ARTEMIS-5107 using wrong value in ReplicationStartSyncMessage ctor
The incorrect value has always been used for the `beforeTwoEighteen`
variable. However, this is not actually a problem because the
`beforeTwoEighteen` variable is not necessary. It's only job is to
prevent newer versions from sending extra data to older versions.
However, older version will simply ignore the extra data which means
the `beforeTwoEighteen` variable can be removed completely.
This same compatibility pattern is used in many places for the Core
protocol.
The tests added with the original fix successfully reproduced the
original problem and those tests still pass even with this variable
removed. Also, keep in mind that `decodeRest` is still checking the
version so that it doesn't try to read data that doesn't exist from an
older version.
---
.../artemis/core/protocol/ServerPacketDecoder.java | 2 +-
.../wireformat/ReplicationStartSyncMessage.java | 31 +++++++---------------
.../core/replication/ReplicationManager.java | 6 ++---
3 files changed, 13 insertions(+), 26 deletions(-)
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/ServerPacketDecoder.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/ServerPacketDecoder.java
index ed418927d8..9475c4c491 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/ServerPacketDecoder.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/ServerPacketDecoder.java
@@ -224,7 +224,7 @@ public class ServerPacketDecoder extends ClientPacketDecoder {
break;
}
case PacketImpl.REPLICATION_START_FINISH_SYNC: {
- packet = new ReplicationStartSyncMessage(connection.isBeforeTwoEighteen());
+ packet = new ReplicationStartSyncMessage();
break;
}
case PacketImpl.REPLICATION_SYNC_FILE: {
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/ReplicationStartSyncMessage.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/ReplicationStartSyncMessage.java
index a82b285a12..cd601bb840 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/ReplicationStartSyncMessage.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/protocol/core/impl/wireformat/ReplicationStartSyncMessage.java
@@ -40,10 +40,6 @@ public class ReplicationStartSyncMessage extends PacketImpl {
private String nodeID;
private boolean allowsAutoFailBack;
- // this is for version compatibility
- // certain versions will need to interrupt encoding and decoding after synchronizationIsFinished on the encoding depending on its value
- private final boolean beforeTwoEighteen;
-
public enum SyncDataType {
JournalBindings(AbstractJournalStorageManager.JournalContent.BINDINGS.typeByte),
JournalMessages(AbstractJournalStorageManager.JournalContent.MESSAGES.typeByte),
@@ -74,13 +70,12 @@ public class ReplicationStartSyncMessage extends PacketImpl {
}
}
- public ReplicationStartSyncMessage(boolean beforeTwoEighteen) {
+ public ReplicationStartSyncMessage() {
super(REPLICATION_START_FINISH_SYNC);
- this.beforeTwoEighteen = synchronizationIsFinished;
}
- public ReplicationStartSyncMessage(boolean beforeTwoEighteen, List<Long> filenames) {
- this(beforeTwoEighteen);
+ public ReplicationStartSyncMessage(List<Long> filenames) {
+ this();
ids = new long[filenames.size()];
for (int i = 0; i < filenames.size(); i++) {
ids[i] = filenames.get(i);
@@ -90,24 +85,20 @@ public class ReplicationStartSyncMessage extends PacketImpl {
}
- public ReplicationStartSyncMessage(boolean beforeTwoEighteen, String nodeID, long nodeDataVersion) {
- this(beforeTwoEighteen, nodeID);
+ public ReplicationStartSyncMessage(String nodeID, long nodeDataVersion) {
+ this();
+ synchronizationIsFinished = true;
+ this.nodeID = nodeID;
ids = new long[1];
ids[0] = nodeDataVersion;
dataType = SyncDataType.ActivationSequence;
}
- public ReplicationStartSyncMessage(boolean beforeTwoEighteen, String nodeID) {
- this(beforeTwoEighteen);
- synchronizationIsFinished = true;
- this.nodeID = nodeID;
- }
-
- public ReplicationStartSyncMessage(boolean beforeTwoEighteen, JournalFile[] datafiles,
+ public ReplicationStartSyncMessage(JournalFile[] datafiles,
AbstractJournalStorageManager.JournalContent contentType,
String nodeID,
boolean allowsAutoFailBack) {
- this(beforeTwoEighteen);
+ this();
this.nodeID = nodeID;
this.allowsAutoFailBack = allowsAutoFailBack;
synchronizationIsFinished = false;
@@ -148,10 +139,6 @@ public class ReplicationStartSyncMessage extends PacketImpl {
buffer.writeBoolean(synchronizationIsFinished);
buffer.writeBoolean(allowsAutoFailBack);
buffer.writeString(nodeID);
- if (beforeTwoEighteen && synchronizationIsFinished) {
- // At this point, pre 2.18.0 servers don't expect any more data to come.
- return;
- }
buffer.writeByte(dataType.code);
buffer.writeInt(ids.length);
for (long id : ids) {
diff --git a/artemis-server/src/main/java/org/apache/activemq/artemis/core/replication/ReplicationManager.java b/artemis-server/src/main/java/org/apache/activemq/artemis/core/replication/ReplicationManager.java
index 2e97840d4c..5d61b35e8f 100644
--- a/artemis-server/src/main/java/org/apache/activemq/artemis/core/replication/ReplicationManager.java
+++ b/artemis-server/src/main/java/org/apache/activemq/artemis/core/replication/ReplicationManager.java
@@ -800,7 +800,7 @@ public final class ReplicationManager implements ActiveMQComponent {
String nodeID,
boolean allowsAutoFailBack) throws ActiveMQException {
if (started)
- sendReplicatePacket(new ReplicationStartSyncMessage(remotingConnection.isBeforeTwoEighteen(), datafiles, contentType, nodeID, allowsAutoFailBack));
+ sendReplicatePacket(new ReplicationStartSyncMessage(datafiles, contentType, nodeID, allowsAutoFailBack));
}
/**
@@ -819,7 +819,7 @@ public final class ReplicationManager implements ActiveMQComponent {
}
synchronizationIsFinishedAcknowledgement.countUp();
- sendReplicatePacket(new ReplicationStartSyncMessage(remotingConnection.isBeforeTwoEighteen(), nodeID, server.getNodeManager().getNodeActivationSequence()));
+ sendReplicatePacket(new ReplicationStartSyncMessage(nodeID, server.getNodeManager().getNodeActivationSequence()));
try {
if (!synchronizationIsFinishedAcknowledgement.await(initialReplicationSyncTimeout)) {
ActiveMQReplicationTimeooutException exception = ActiveMQMessageBundle.BUNDLE.replicationSynchronizationTimeout(initialReplicationSyncTimeout);
@@ -864,7 +864,7 @@ public final class ReplicationManager implements ActiveMQComponent {
idsToSend = new ArrayList<>(largeMessages.keySet());
if (started)
- sendReplicatePacket(new ReplicationStartSyncMessage(remotingConnection.isBeforeTwoEighteen(), idsToSend));
+ sendReplicatePacket(new ReplicationStartSyncMessage(idsToSend));
}
/**
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@activemq.apache.org
For additional commands, e-mail: commits-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact