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

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

                Author: ASF GitHub Bot
            Created on: 14/Jan/21 20:47
            Start Date: 14/Jan/21 20:47
    Worklog Time Spent: 10m 
      Work Description: clebertsuconic commented on a change in pull request 
#3407:
URL: https://github.com/apache/activemq-artemis/pull/3407#discussion_r557688999



##########
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:
       it's important to always return the same size...
   
   When the message enters the system, you're adding the size of the message, 
when leaving you're returning the size after parsing the property.
   
   You need to cache the value, otherwise you would get inconsistencies found 
by the test suite here:
   
   
    
org.apache.activemq.artemis.tests.integration.client.ConsumerTest.testSendReceiveAMQP[isNetty=true,
 persistent=true]        5.4 sec 1
    
org.apache.activemq.artemis.tests.integration.client.ConsumerTest.testSendAMQPReceiveCore[isNetty=true,
 persistent=true]    5.2 sec 1
    
org.apache.activemq.artemis.tests.integration.client.ConsumerTest.testSendAMQPReceiveOpenWire[isNetty=true,
 persistent=true]        5.3 sec 1
    
org.apache.activemq.artemis.tests.integration.client.ConsumerTest.testSendReceiveAMQP[isNetty=true,
 persistent=false]       5.3 sec 1
    
org.apache.activemq.artemis.tests.integration.client.ConsumerTest.testSendAMQPReceiveCore[isNetty=true,
 persistent=false]   5.2 sec 1
    
org.apache.activemq.artemis.tests.integration.client.ConsumerTest.testSendAMQPReceiveOpenWire[isNetty=true,
 persistent=false]       5.3 sec 1
    
org.apache.activemq.artemis.tests.integration.cluster.failover.ReplicatedManyMultipleServerFailoverNoNodeGroupNameTest.testStartLiveFirst




----------------------------------------------------------------
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:
[email protected]


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

    Worklog Id:     (was: 536154)
    Time Spent: 0.5h  (was: 20m)

> AMQP applicationProperties are not part of the memoryEstimate
> -------------------------------------------------------------
>
>                 Key: ARTEMIS-3067
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-3067
>             Project: ActiveMQ Artemis
>          Issue Type: Bug
>          Components: AMQP
>    Affects Versions: 2.16.0
>            Reporter: Gary Tully
>            Assignee: Gary Tully
>            Priority: Major
>             Fix For: 2.17.0
>
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> With significant data in application properties, the decoded properties can 
> consume memory that is not tracked for paging purposes and can lead to 
> unexpected OOM.
> Duplicate detection is one cause of decoding. Use of selectors is another. 
> Otherwise they are left intact by the broker and just routed in their raw 
> format.
> When they are decoded, for whatever reason, we need to account for them is 
> some way such that paging can kick in as expected.



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

Reply via email to