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

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

                Author: ASF GitHub Bot
            Created on: 14/Apr/26 16:13
            Start Date: 14/Apr/26 16:13
    Worklog Time Spent: 10m 
      Work Description: clebertsuconic commented on code in PR #6323:
URL: https://github.com/apache/artemis/pull/6323#discussion_r3080878342


##########
artemis-protocols/artemis-amqp-protocol/src/main/java/org/apache/activemq/artemis/protocol/amqp/broker/AMQPStandardMessage.java:
##########
@@ -191,20 +187,27 @@ protected ReadableBuffer getData() {
 
    @Override
    public synchronized int getMemoryEstimate() {
-      if (memoryEstimate == -1) {
-         if (isPaged) {
-            // When the message is paged, we don't take the unmarshalled 
application properties because it could be
-            // updated at different places. We just keep the estimate simple 
when paging.
-            memoryEstimate = memoryOffset + (data != null ? data.capacity() : 
0);
-         } else {
-            memoryEstimate = memoryOffset + (data != null ? data.capacity() + 
unmarshalledApplicationPropertiesMemoryEstimateFromData(data) : 0);
-         }
-         originalEstimate = memoryEstimate;
+      if (memoryEstimate == VALUE_NOT_PRESENT) {
+         // This estimation was tested and validated through AMQPGlobalMaxTest 
on soak-tests
+         memoryEstimate = MINIMUM_ESTIMATE + (data != null ? data.capacity() + 
getApplicationPropertiesEncodingSize(data) * 2 + 
getApplicationPropertiesCount() * DataConstants.SIZE_INT : 0);
       }
 
       return memoryEstimate;
    }
 
+   private int getApplicationPropertiesEncodingSize(ReadableBuffer data) {
+      ensureScanning();
+
+      // ensureScanning will make sure we know the positions, and we will 
return the encoding used based on those positions
+      if (remainingBodyPosition != VALUE_NOT_PRESENT) {
+         return remainingBodyPosition - applicationPropertiesPosition;

Review Comment:
   yeah.. you're right..
   
   
   I added a new commit (to be squashed) addressing your comments here. if you 
could please let me know when I can squash it.. (dunno if you prefer it to be 
separate to make it easier the review).





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

    Worklog Id:     (was: 1015524)
    Time Spent: 11h 50m  (was: 11h 40m)

> Make AMQP Size immutable
> ------------------------
>
>                 Key: ARTEMIS-5573
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-5573
>             Project: Artemis
>          Issue Type: Improvement
>          Components: AMQP
>    Affects Versions: 2.41.0
>            Reporter: Clebert Suconic
>            Assignee: Clebert Suconic
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 11h 50m
>  Remaining Estimate: 0h
>
> I have had a lot of issues, even recently on races between re-evaluating a 
> message size in AMQP.
> Say a lazy decode happens at the wrong time and the memory estimates can be 
> wrong.
> We have fixed issues along the years, but this is still a fragile process 
> that is bound to fail. If an user for instance add a plugin breaking the 
> chain of events.
> For that reason the memory estimate should already include enough estimation 
> for any properties decoded and the process should be simplified.
> Less moving parts would mean less possibilities for bugs.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to