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 <[email protected]>
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: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
