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

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

                Author: ASF GitHub Bot
            Created on: 13/Jun/22 15:01
            Start Date: 13/Jun/22 15:01
    Worklog Time Spent: 10m 
      Work Description: gemmellr commented on code in PR #4101:
URL: https://github.com/apache/activemq-artemis/pull/4101#discussion_r895827972


##########
artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/QueueImpl.java:
##########
@@ -3101,29 +3108,21 @@ protected void removeMessageReference(ConsumerHolder<? 
extends Consumer> holder,
       refRemoved(ref);
    }
 
-   private void checkDepage(boolean noWait) {
-      if (pageIterator != null && pageSubscription.isPaging() && 
!depagePending && needsDepage() && (noWait ? pageIterator.tryNext() > 0 : 
pageIterator.hasNext())) {
+   private void checkDepage() {
+      if (pageIterator != null && pageSubscription.isPaging() && 
!depagePending && needsDepage() && pageIterator.tryNext() != 
PageIterator.NextResult.noElements) {
          scheduleDepage(false);
       }
    }
 
    /**
-    * This is a common check we do before scheduling depaging.. or while 
depaging.
-    * Before scheduling a depage runnable we verify if it fits / needs 
depaging.
-    * We also check for while needsDepage While depaging.
-    * This is just to avoid a copy & paste dependency
+    *
+    * This is a check on page sizing.
     *
     * @return
     */
    private boolean needsDepage() {
-      return queueMemorySize.get() < 
pageSubscription.getPagingStore().getMaxSize() &&
-         /**
-          * In most cases, one depage round following by at most 
MAX_SCHEDULED_RUNNERS deliver round,
-          * thus we just need to read MAX_DELIVERIES_IN_LOOP * 
MAX_SCHEDULED_RUNNERS messages. If we read too much, the message reference
-          * maybe discarded by gc collector in response to memory demand and 
we need to read it again at
-          * a great cost when delivering.
-          */
-         intermediateMessageReferences.size() + messageReferences.size() < 
MAX_DEPAGE_NUM;
+      return queueMemorySize.getSize() < 
pageSubscription.getPagingStore().getMaxPageReadBytes() &&
+             queueMemorySize.getElements() < 
pageSubscription.getPagingStore().getMaxPageReadMessages();

Review Comment:
   Maybe I dont understand what this is all doing then...my first reaction is, 
why not?
   
   Say a queue is configured to start paging after hitting 500MB with 100KB 
messages (i.e 5000 messages), and its max-page-read-foo is the default 
(2*page-size=)20MB/1000 msgs. When would it be expected to start depaging after 
the queue content dropping back below the 500MB it initiated? To me the obvious 
answer would be some point closely after it does. My initial understanding of 
the logic here though is that it would continue saying no until it has less 
than 20 messages left and it drops below the getMaxPageReadBytes? Leaving the 
queue paging (and accumulating yet more stuff to depage) until it is almost 
empty seems off, at which point it would again stop depaging after it reads 20 
messages worth? Meaning if the queue is active its likely to never stop paging 
once it starts until there are almost no messages for it?





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

    Worklog Id:     (was: 780852)
    Time Spent: 6h 20m  (was: 6h 10m)

> Add Option to read messages into paging based on sizing and eliminate caching
> -----------------------------------------------------------------------------
>
>                 Key: ARTEMIS-3850
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-3850
>             Project: ActiveMQ Artemis
>          Issue Type: New Feature
>    Affects Versions: 2.22.0
>            Reporter: Clebert Suconic
>            Assignee: Clebert Suconic
>            Priority: Major
>             Fix For: 2.24.0
>
>          Time Spent: 6h 20m
>  Remaining Estimate: 0h
>




--
This message was sent by Atlassian Jira
(v8.20.7#820007)

Reply via email to