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.
apache · Jan 21, 2025 · 8f0b9cb · 8f0b9cb
1 parent 1ba0b65
commit 8f0b9cb
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 26 deletions.
diff --git a/...s-server/src/main/java/org/apache/activemq/artemis/core/protocol/ServerPacketDecoder.java b/...s-server/src/main/java/org/apache/activemq/artemis/core/protocol/ServerPacketDecoder.java
@@ -229,7 +229,7 @@ private Packet slowPathDecode(ActiveMQBuffer in, byte packetType, CoreRemotingCo
             break;
          }
          case PacketImpl.REPLICATION_START_FINISH_SYNC: {
-            packet = new ReplicationStartSyncMessage(connection.isBeforeTwoEighteen());
+            packet = new ReplicationStartSyncMessage();
             break;
          }
          case PacketImpl.REPLICATION_SYNC_FILE: {

diff --git a/...ache/activemq/artemis/core/protocol/core/impl/wireformat/ReplicationStartSyncMessage.java b/...ache/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 static SyncDataType getDataType(byte code) {
       }
    }
 
-   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 ReplicationStartSyncMessage(boolean beforeTwoEighteen, List<Long> filenam
    }
 
 
-   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 void encodeRest(final ActiveMQBuffer buffer) {
       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/...server/src/main/java/org/apache/activemq/artemis/core/replication/ReplicationManager.java b/...server/src/main/java/org/apache/activemq/artemis/core/replication/ReplicationManager.java
@@ -802,7 +802,7 @@ public void sendStartSyncMessage(JournalFile[] datafiles,
                                     String nodeID,
                                     boolean allowsAutoFailBack) throws ActiveMQException {
       if (started)
-         sendReplicatePacket(new ReplicationStartSyncMessage(remotingConnection.isBeforeTwoEighteen(), datafiles, contentType, nodeID, allowsAutoFailBack));
+         sendReplicatePacket(new ReplicationStartSyncMessage(datafiles, contentType, nodeID, allowsAutoFailBack));
    }
 
    /**
@@ -821,7 +821,7 @@ public void sendSynchronizationDone(String nodeID, long initialReplicationSyncTi
          }
 
          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);
@@ -866,7 +866,7 @@ public void sendLargeMessageIdListMessage(Map<Long, Pair<String, Long>> largeMes
       idsToSend = new ArrayList<>(largeMessages.keySet());
 
       if (started)
-         sendReplicatePacket(new ReplicationStartSyncMessage(remotingConnection.isBeforeTwoEighteen(), idsToSend));
+         sendReplicatePacket(new ReplicationStartSyncMessage(idsToSend));
    }
 
    /**