[GitHub] [activemq-artemis] gtully commented on a change in pull request #3407: ARTEMIS-3067 - track application properties in memory estimate and ch…

[GitBox <gi...@apache.org>]

gtully commented on a change in pull request #3407:
URL: https://github.com/apache/activemq-artemis/pull/3407#discussion_r559754464



##########
File path: artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPStandardMessage.java
##########
@@ -124,12 +124,24 @@ protected ReadableBuffer getData() {
    @Override
    public int getMemoryEstimate() {
       if (memoryEstimate == -1) {
-         memoryEstimate = memoryOffset + (data != null ? data.capacity() : 0);
+         memoryEstimate = memoryOffset + (data != null ? data.capacity() + unmarshalledApplicationPropertiesMemoryEstimateFromData() : 0);
       }
 
       return memoryEstimate;
    }
 
+   private int unmarshalledApplicationPropertiesMemoryEstimateFromData() {
+      if (applicationProperties != null) {
+         // they have been unmarshalled, estimate memory usage based on their encoded size
+         if (remainingBodyPosition != VALUE_NOT_PRESENT) {
+            return remainingBodyPosition - applicationPropertiesPosition;

Review comment:
       the intent is to compensate for any change in the estimate, in the knowledge that it may happen for filter evaluation or for transformation. But I must not have all the possibilities covered, and it may be tricky. There is deliver and directDeliver etc. 
   The consumerTest is now sorted, but I need to investigate next org.apache.activemq.artemis.tests.integration.amqp.connect.AMQPReplicaTest#testReplicaCatchupOnQueueCreates




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org