[ 
https://issues.apache.org/jira/browse/ARTEMIS-3243?focusedWorklogId=619964&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-619964
 ]

ASF GitHub Bot logged work on ARTEMIS-3243:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 07/Jul/21 13:03
            Start Date: 07/Jul/21 13:03
    Worklog Time Spent: 10m 
      Work Description: gemmellr commented on a change in pull request #3633:
URL: https://github.com/apache/activemq-artemis/pull/3633#discussion_r665343844



##########
File path: 
artemis-commons/src/test/java/org/apache/activemq/artemis/utils/ByteUtilTest.java
##########
@@ -376,6 +367,21 @@ public void testIntToByte() {
       }
    }
 
+   @Test
+   public void testLongToByte() {
+      for (int i = 0; i < 1000; i++) {
+         long randomlong = RandomUtil.randomLong();
+         if (randomlong < 0) randomlong *= -1;

Review comment:
       Why actually do this? The method needs to handle negatives doesnt it? 
The whole thing with the bit shifting behaviour certainly wont make any 
difference with only positive values as the sign bit will be 0 and so either 
shift will do the same.
   
   Also this doesnt actually work for Long.MIN_VALUE since there is no positive 
equivalent for it, I think it will come out with the exact same negative value.

##########
File path: 
artemis-commons/src/test/java/org/apache/activemq/artemis/utils/ByteUtilTest.java
##########
@@ -376,6 +367,21 @@ public void testIntToByte() {
       }
    }
 
+   @Test
+   public void testLongToByte() {
+      for (int i = 0; i < 1000; i++) {

Review comment:
       1000 randoms seems excessive.  1 or two known values, corner cases etc 
would seem sufficient and better.

##########
File path: 
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/mirror/AMQPMirrorControllerTarget.java
##########
@@ -410,30 +423,33 @@ private boolean sendMessage(AMQPMessage message, 
ACKMessageOperation messageComp
                          " with internalID = " + internalIDLong + " mirror id 
" + internalMirrorID);
       }
 
-      final TransactionImpl transaction = new 
MirrorTransaction(server.getStorageManager());
-      transaction.addOperation(messageCompletionAck);
 
       routingContext.setDuplicateDetection(false); // we do our own duplicate 
detection here
 
+      byte[] duplicateIDBytes = ByteUtil.longToBytes(internalID);
+
       if (internalID != 0) {
-         byte[] duplicateIDBytes = ByteUtil.longToBytes(internalID);
          if (duplicateIDCache.contains(duplicateIDBytes)) {
             flow();
             return false;
-         } else {
-            routingContext.setTransaction(transaction);
-            duplicateIDCache.addToCache(duplicateIDBytes, transaction);
          }
-         message.setBrokerProperty(INTERNAL_ID_EXTRA_PROPERTY, internalID);
-         message.setBrokerProperty(INTERNAL_BROKER_ID_EXTRA_PROPERTY, 
internalMirrorID);
       }
 
+      message.setBrokerProperty(INTERNAL_ID_EXTRA_PROPERTY, internalID);
+      message.setBrokerProperty(INTERNAL_BROKER_ID_EXTRA_PROPERTY, 
internalMirrorID);
+
       if (internalAddress != null) {
          message.setAddress(internalAddress);
       }
 
+      final TransactionImpl transaction = new 
MirrorTransaction(server.getStorageManager());
+      transaction.addOperation(messageCompletionAck.tx); // Notice that as we 
return true on this

Review comment:
       Comment finishes mid sentence

##########
File path: 
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/connect/mirror/AMQPMirrorControllerTarget.java
##########
@@ -82,10 +82,21 @@ public static MirrorController getControllerTarget() {
       return controllerThreadLocal.get();
    }
 
-   class ACKMessageOperation extends TransactionOperationAbstract implements 
IOCallback, Runnable {
+   /** Objects of this class can be used by either transaction or by 
OoperationContext.
+    *  It is important that when you're using the tra

Review comment:
       Sentence finishes mid word.




-- 
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]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 619964)
    Time Spent: 16h  (was: 15h 50m)

> Enhance AMQP Mirror support with dual mirror
> --------------------------------------------
>
>                 Key: ARTEMIS-3243
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-3243
>             Project: ActiveMQ Artemis
>          Issue Type: Bug
>    Affects Versions: 2.17.0
>            Reporter: Clebert Suconic
>            Assignee: Clebert Suconic
>            Priority: Major
>             Fix For: 2.18.0
>
>          Time Spent: 16h
>  Remaining Estimate: 0h
>
> at the current Mirror version, we can only mirror into a single direction.
> With this enhancement the two (or more brokers) would be connected to each 
> other, each one having its own ID, and each one would send updates to the 
> other broker.
> The outcome is that if you just transferred producers and consumers from one 
> broker into the other, the fallback would be automatic and simple. No need to 
> disable and enable mirror options.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to